Re: [PATCH kvm-unit-tests 4/8] powerpc: Introduce getchar

[Paolo Bonzini <pbonzini@redhat.com>]

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?

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