[kvm-unit-tests,v2,14/16] arm/run: Allow Migration tests

Let's link getchar.o to use puts and getchar from the
tests.

Then allow tests belonging to the migration group to
trigger the migration from the test code by putting
"migrate" into the uart. Then the code can wait for the
migration completion by using getchar().

The __getchar implement is minimalist as it just reads the
data register. It is just meant to read the single character
emitted at the end of the migration by the runner script.

It is not meant to read more data (FIFOs are not enabled).

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 arm/Makefile.common |  2 +-
 arm/run             |  2 +-
 lib/arm/io.c        | 13 +++++++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)

On Fri, Jan 10, 2020 at 03:54:10PM +0100, Eric Auger wrote:
> Let's link getchar.o to use puts and getchar from the
> tests.
> 
> Then allow tests belonging to the migration group to
> trigger the migration from the test code by putting
> "migrate" into the uart. Then the code can wait for the
> migration completion by using getchar().
> 
> The __getchar implement is minimalist as it just reads the
> data register. It is just meant to read the single character
> emitted at the end of the migration by the runner script.
> 
> It is not meant to read more data (FIFOs are not enabled).
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  arm/Makefile.common |  2 +-
>  arm/run             |  2 +-
>  lib/arm/io.c        | 13 +++++++++++++
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index 7cc0f04..327f112 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -32,7 +32,7 @@ CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib
>  asm-offsets = lib/$(ARCH)/asm-offsets.h
>  include $(SRCDIR)/scripts/asm-offsets.mak
>  
> -cflatobjs += lib/util.o
> +cflatobjs += lib/util.o lib/getchar.o
>  cflatobjs += lib/alloc_phys.o
>  cflatobjs += lib/alloc_page.o
>  cflatobjs += lib/vmalloc.o
> diff --git a/arm/run b/arm/run
> index 277db9b..a390ca5 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -61,6 +61,6 @@ fi
>  M+=",accel=$ACCEL"
>  command="$qemu -nodefaults $M -cpu $processor $chr_testdev $pci_testdev"
>  command+=" -display none -serial stdio -kernel"
> -command="$(timeout_cmd) $command"
> +command="$(migration_cmd) $(timeout_cmd) $command"
>  
>  run_qemu $command "$@"
> diff --git a/lib/arm/io.c b/lib/arm/io.c
> index 99fd315..ed89e19 100644
> --- a/lib/arm/io.c
> +++ b/lib/arm/io.c
> @@ -87,6 +87,19 @@ void puts(const char *s)
>  	spin_unlock(&uart_lock);
>  }
>  
> +/*
> + * Minimalist implementation for migration completion detection.
> + * Needs to be improved for more advanced Rx cases

Please add that QEMU supports reading a maximum of 16 characters in
this minimal mode. We could even add a counter and then assert if
we try to read 16 or more.

> + */
> +int __getchar(void)
> +{
> +	int ret;
> +
> +	ret = readb(uart0_base);
> +	if (!ret)
> +		return -1;
> +	return ret;
> +}

What about taking the lock?

>  
>  /*
>   * Defining halt to take 'code' as an argument guarantees that it will
> -- 
> 2.20.1
> 

What do you think of the __getchar below?

Thanks,
drew


diff --git a/lib/arm/io.c b/lib/arm/io.c
index 99fd31560084..77beb331d6b2 100644
--- a/lib/arm/io.c
+++ b/lib/arm/io.c
@@ -79,6 +79,31 @@ void io_init(void)
 	chr_testdev_init();
 }
 
+static int ____getchar(void)
+{
+	int c;
+
+	spin_lock(&uart_lock);
+	c = readb(uart0_base);
+	spin_unlock(&uart_lock);
+
+	return c ?: -1;
+}
+
+int __getchar(void)
+{
+	static int count;
+	int c;
+
+	if ((c = ____getchar()) != -1)
+		++count;
+
+	/* Without special UART initialization we can only read 16 characters. */
+	assert(count < 16);
+
+	return c;
+}
+
 void puts(const char *s)
 {
 	spin_lock(&uart_lock);

Hi Drew,
On 1/13/20 7:40 PM, Andrew Jones wrote:
> On Fri, Jan 10, 2020 at 03:54:10PM +0100, Eric Auger wrote:
>> Let's link getchar.o to use puts and getchar from the
>> tests.
>>
>> Then allow tests belonging to the migration group to
>> trigger the migration from the test code by putting
>> "migrate" into the uart. Then the code can wait for the
>> migration completion by using getchar().
>>
>> The __getchar implement is minimalist as it just reads the
>> data register. It is just meant to read the single character
>> emitted at the end of the migration by the runner script.
>>
>> It is not meant to read more data (FIFOs are not enabled).
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  arm/Makefile.common |  2 +-
>>  arm/run             |  2 +-
>>  lib/arm/io.c        | 13 +++++++++++++
>>  3 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/arm/Makefile.common b/arm/Makefile.common
>> index 7cc0f04..327f112 100644
>> --- a/arm/Makefile.common
>> +++ b/arm/Makefile.common
>> @@ -32,7 +32,7 @@ CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib
>>  asm-offsets = lib/$(ARCH)/asm-offsets.h
>>  include $(SRCDIR)/scripts/asm-offsets.mak
>>  
>> -cflatobjs += lib/util.o
>> +cflatobjs += lib/util.o lib/getchar.o
>>  cflatobjs += lib/alloc_phys.o
>>  cflatobjs += lib/alloc_page.o
>>  cflatobjs += lib/vmalloc.o
>> diff --git a/arm/run b/arm/run
>> index 277db9b..a390ca5 100755
>> --- a/arm/run
>> +++ b/arm/run
>> @@ -61,6 +61,6 @@ fi
>>  M+=",accel=$ACCEL"
>>  command="$qemu -nodefaults $M -cpu $processor $chr_testdev $pci_testdev"
>>  command+=" -display none -serial stdio -kernel"
>> -command="$(timeout_cmd) $command"
>> +command="$(migration_cmd) $(timeout_cmd) $command"
>>  
>>  run_qemu $command "$@"
>> diff --git a/lib/arm/io.c b/lib/arm/io.c
>> index 99fd315..ed89e19 100644
>> --- a/lib/arm/io.c
>> +++ b/lib/arm/io.c
>> @@ -87,6 +87,19 @@ void puts(const char *s)
>>  	spin_unlock(&uart_lock);
>>  }
>>  
>> +/*
>> + * Minimalist implementation for migration completion detection.
>> + * Needs to be improved for more advanced Rx cases
> 
> Please add that QEMU supports reading a maximum of 16 characters in
> this minimal mode. We could even add a counter and then assert if
> we try to read 16 or more.
> 
>> + */
>> +int __getchar(void)
>> +{
>> +	int ret;
>> +
>> +	ret = readb(uart0_base);
>> +	if (!ret)
>> +		return -1;
>> +	return ret;
>> +}
> 
> What about taking the lock?
> 
>>  
>>  /*
>>   * Defining halt to take 'code' as an argument guarantees that it will
>> -- 
>> 2.20.1
>>
> 
> What do you think of the __getchar below?
> 
> Thanks,
> drew
> 
> 
> diff --git a/lib/arm/io.c b/lib/arm/io.c
> index 99fd31560084..77beb331d6b2 100644
> --- a/lib/arm/io.c
> +++ b/lib/arm/io.c
> @@ -79,6 +79,31 @@ void io_init(void)
>  	chr_testdev_init();
>  }
>  
> +static int ____getchar(void)
> +{
> +	int c;
> +
> +	spin_lock(&uart_lock);
> +	c = readb(uart0_base);
> +	spin_unlock(&uart_lock);
> +
> +	return c ?: -1;
> +}
> +
> +int __getchar(void)
> +{
> +	static int count;
> +	int c;
> +
> +	if ((c = ____getchar()) != -1)
> +		++count;
> +
> +	/* Without special UART initialization we can only read 16 characters. */
> +	assert(count < 16);
> +
> +	return c;
> +}
> +
>  void puts(const char *s)
>  {
>  	spin_lock(&uart_lock);
> 
Yep, I will take your improved version.

Thanks

Eric