On Wed, Feb 14, 2018 at 12:43:40PM +0100, Paolo Bonzini wrote:
> On 08/02/2018 13:59, Andrew Jones wrote:
> > On Thu, Feb 08, 2018 at 12:06:07PM +0100, Thomas Huth wrote:
> >> On 07.02.2018 20:03, Andrew Jones wrote:
> >>> Move the unit test specific h_get_term_char() to powerpc common code
> >>> and call it getchar(). Other architectures may want to implement
> >>> getchar() too (ARM will in a coming patch), so put the prototype in
> >>> libcflat.h
> >>
> >> Using the libc name getchar() here sounds wrong. h_get_term_char()
> >> behaves differently compared to the libc getchar(), e.g. it does not
> >> block if there are no characters available.
> >> I think you should either name this function differently, or implement a
> >> behavior that is more close to the libc getchar() to avoid future confusion.
> >
> > Good point. I guess I should rename it, because we'd need to anyway in
> > order to share getchar()'s implementation among other architectures,
> > unless we duplicated the looping and EOF returning. Any suggestion for
> > the name? __getchar() or stdin_readb()?
>
> Like this?
Precisely, although I'm not sure it needs its own file. Since it might be
nice to eventually clean up (remove?) libcflat.h, maybe we should create
lib/stdio.[ch] and lib/stdlib.[ch] files. Implementations for libc
functions (like getchar), that have their prototypes in lib/stdio.h, could
all get thrown together in lib/stdio.c.
Could be nice to define EOF in lib/stdio.h too.
Thanks,
drew
>
> diff --git a/lib/getchar.c b/lib/getchar.c
> new file mode 100644
> index 0000000..26f6b6b
> --- /dev/null
> +++ b/lib/getchar.c
> @@ -0,0 +1,11 @@
> +#include "libcflat.h"
> +#include "asm/barrier.h"
> +
> +int getchar(void)
> +{
> + int c;
> +
> + while ((c = __getchar()) == -1)
> + cpu_relax();
> + return c;
> +}
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index c680b69..cc56553 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -82,6 +82,7 @@ typedef u64 phys_addr_t;
> #define INVALID_PHYS_ADDR (~(phys_addr_t)0)
>
> extern void puts(const char *s);
> +extern int __getchar(void);
> extern int getchar(void);
> extern void exit(int code);
> extern void abort(void);
> diff --git a/lib/powerpc/hcall.c b/lib/powerpc/hcall.c
> index d65af93..7b05265 100644
> --- a/lib/powerpc/hcall.c
> +++ b/lib/powerpc/hcall.c
> @@ -32,7 +32,7 @@ void putchar(int c)
> hcall(H_PUT_TERM_CHAR, vty, nr_chars, chars);
> }
>
> -int getchar(void)
> +int __getchar(void)
> {
> register unsigned long r3 asm("r3") = H_GET_TERM_CHAR;
> register unsigned long r4 asm("r4") = 0; /* 0 == default vty */
> @@ -41,5 +41,5 @@ int getchar(void)
> asm volatile (" sc 1 " : "+r"(r3), "+r"(r4), "=r"(r5)
> : "r"(r3), "r"(r4));
>
> - return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : 0;
> + return r3 == H_SUCCESS && r4 > 0 ? r5 >> 48 : -1;
> }
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 21dea40..63dc166 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -30,6 +30,7 @@ asm-offsets = lib/$(ARCH)/asm-offsets.h
> include $(SRCDIR)/scripts/asm-offsets.mak
>
> cflatobjs += lib/util.o
> +cflatobjs += lib/getchar.o
> cflatobjs += lib/alloc_phys.o
> cflatobjs += lib/alloc.o
> cflatobjs += lib/devicetree.o
> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index 5b5a540..6744bd8 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -285,8 +285,7 @@ int main(int argc, char **argv)
>
> if (pause) {
> puts("Now migrate the VM, then press a key to continue...\n");
> - while (getchar() == 0)
> - cpu_relax();
> + (void) getchar();
> } else {
> puts("Sleeping...\n");
> handle_exception(0x900, &dec_except_handler, NULL);
>
>
> Paolo
