[kvm-unit-tests PATCH v3 0/5] arm/arm64: Add support for running under kvmtool

[kvm-unit-tests PATCH v3 0/5] arm/arm64: Add support for running under kvmtool

[Alexandru Elisei]

kvm-unit-tests is designed to be run with QEMU as the virtual machine
monitor. It relies on devices emulated by QEMU (like isa-debug-exit or
testdev) and it makes certain assumptions based on the implicit QEMU
virtual environment configuration (like the serial base address).

kvmtool [1] is a lightweight virtual machine monitor for running KVM
guests. kvmtool has reduced complexity compared to QEMU and is easily
hackable.

This patch series aims to make it possible to run kvm-unit-tests using
kvmtool on the arm and arm64 architectures, with two caveats:

(1) When terminating a test, the userspace process won't exit with an exit
code that signals the success or failure of the test. Output from the test
can still be parsed to determine the outcome of the test.

(2) kvmtool has been designed to work with a linux guest and it
automatically generates the command line arguments for a Linux kernel. The
arm/arm64 selftest and gic tests will fail if unexpected command line
arguments are found. To get around this limitation, the test binary needs
to be loaded using the --firmware option introduced by kvmtool in commit
5e4b563d75b9 ("arm: Allow command line for firmware"). This option
suppresses the automatic kernel command line and can be used to run all
tests, not just the tests that require specific arguments.

The run scripts haven't been modified. To run a test under kvmtool, one
needs to launch kvmtool manually. For example, to run the timer test the
following command can be used:

lkvm run --cpus 1 --console serial --firmware timer.flat.

To run the gicv3-ipi test:

lkvm run --cpus 8 --console serial --params "ipi" --irqchip gicv3 \
    --firmware gic.flat

Changes in v3:
* Updated cover letter with information about the kvmtool --firmware
  option.
* Gathered Reviewed-by tags.
* Renamed the config.h define UART_EARLY_BASE to CONFIG_UART_EARLY_BASE and
  made the necessary casts in lib/arm/io.c

Changes in v2:
* Generate lib/config.h when configuring kvm-unit-tests; arm/arm64 uses it
  to get the UART address.
* Added --vmm configure option for arm/arm64 which will set the UART
  address in lib/config.h when the tests are run under QEMU or kvmtool.
* Renamed psci_sys_reset() to psci_system_reset().
* Dropped patches that allowed a test to ignore unexpected command line
  arguments.

Summary:
* Patches 1, 2 and 3 add support for configuring kvm-unit-tests on arm and
  arm64 to use the ns16550a UART emulated by kvmtool.
* Patches 4 and 5 provide an alternative mechanism for terminating the
  virtual machine by using PSCI.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git/
[2] https://www.spinics.net/lists/kvm-arm/msg34352.html

Alexandru Elisei (5):
  lib: arm: Use UART address from generated config.h
  configure: arm/arm64: Add --vmm option with no effect
  lib: arm: Use ns16550a UART when --vmm=kvmtool
  lib: arm: Implement PSCI SYSTEM_OFF in psci_system_off()
  lib: arm: Fallback to psci_system_off() in exit()

 configure          | 32 ++++++++++++++++++++++++++++++++
 Makefile           |  2 +-
 lib/arm/asm/psci.h |  3 ++-
 lib/arm/io.c       | 41 ++++++++++++++++++++++++++---------------
 lib/arm/psci.c     |  8 +++++++-
 .gitignore         |  1 +
 6 files changed, 69 insertions(+), 18 deletions(-)

-- 
2.17.0

[kvm-unit-tests PATCH v3 1/5] lib: arm: Use UART address from generated config.h

[Alexandru Elisei]

Generate lib/config.h when configuring kvm-unit-tests. The file is empty
for all architectures except for arm and arm64, where it is used to store
the UART base address. This removes the hardcoded address from lib/arm/io.c
and provides a mechanism for using different UART addresses in the future.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 configure    | 17 +++++++++++++++++
 Makefile     |  2 +-
 lib/arm/io.c | 12 ++++++------
 .gitignore   |  1 +
 4 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index df8581e3a906..b4732c6c04e4 100755
--- a/configure
+++ b/configure
@@ -198,3 +198,20 @@ ENVIRON_DEFAULT=$environ_default
 ERRATATXT=errata.txt
 U32_LONG_FMT=$u32_long
 EOF
+
+cat <<EOF > lib/config.h
+#ifndef CONFIG_H
+#define CONFIG_H 1
+/*
+ * Generated file. DO NOT MODIFY.
+ *
+ */
+EOF
+if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
+cat <<EOF >> lib/config.h
+
+#define CONFIG_UART_EARLY_BASE 0x09000000
+
+EOF
+fi
+echo "#endif" >> lib/config.h
diff --git a/Makefile b/Makefile
index e9f02272e156..643af05678ad 100644
--- a/Makefile
+++ b/Makefile
@@ -115,7 +115,7 @@ libfdt_clean:
 	$(LIBFDT_objdir)/.*.d
 
 distclean: clean libfdt_clean
-	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
+	$(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
 	$(RM) -r tests logs logs.old
 
 cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
diff --git a/lib/arm/io.c b/lib/arm/io.c
index d2c1a07c19ee..e84a5c89fcb1 100644
--- a/lib/arm/io.c
+++ b/lib/arm/io.c
@@ -11,6 +11,7 @@
 #include <libcflat.h>
 #include <devicetree.h>
 #include <chr-testdev.h>
+#include <config.h>
 #include <asm/spinlock.h>
 #include <asm/io.h>
 
@@ -18,6 +19,7 @@
 
 extern void halt(int code);
 
+static struct spinlock uart_lock;
 /*
  * Use this guess for the pl011 base in order to make an attempt at
  * having earlier printf support. We'll overwrite it with the real
@@ -25,10 +27,8 @@ extern void halt(int code);
  * the address we expect QEMU's mach-virt machine type to put in
  * its generated device tree.
  */
-#define UART_EARLY_BASE 0x09000000UL
-
-static struct spinlock uart_lock;
-static volatile u8 *uart0_base = (u8 *)UART_EARLY_BASE;
+#define UART_EARLY_BASE (u8 *)(unsigned long)CONFIG_UART_EARLY_BASE
+static volatile u8 *uart0_base = UART_EARLY_BASE;
 
 static void uart0_init(void)
 {
@@ -58,10 +58,10 @@ static void uart0_init(void)
 
 	uart0_base = ioremap(base.addr, base.size);
 
-	if (uart0_base != (u8 *)UART_EARLY_BASE) {
+	if (uart0_base != UART_EARLY_BASE) {
 		printf("WARNING: early print support may not work. "
 		       "Found uart at %p, but early base is %p.\n",
-			uart0_base, (u8 *)UART_EARLY_BASE);
+			uart0_base, UART_EARLY_BASE);
 	}
 }
 
diff --git a/.gitignore b/.gitignore
index 2405a8087ae5..483f7c7a09ea 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,7 @@ patches
 cscope.*
 *.swp
 /lib/asm
+/lib/config.h
 /config.mak
 /*-run
 /msr.out
-- 
2.17.0

[kvm-unit-tests PATCH v3 2/5] configure: arm/arm64: Add --vmm option with no effect

[Alexandru Elisei]

Add configuration option --vmm to specify the virtual machine manager.
Valid choices are 'qemu' and 'kvmtool', the default being 'qemu'. This
option is only available for the arm and arm64 architectures and does
nothing for now.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
I opted to keep the exclamation mark in the error messages because I think
it makes them stand out.

 configure | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/configure b/configure
index b4732c6c04e4..d1dd7d5830eb 100755
--- a/configure
+++ b/configure
@@ -16,6 +16,7 @@ endian=""
 pretty_print_stacks=yes
 environ_default=yes
 u32_long=
+vmm="qemu"
 
 usage() {
     cat <<-EOF
@@ -24,6 +25,8 @@ usage() {
 	Options include:
 	    --arch=ARCH            architecture to compile for ($arch)
 	    --processor=PROCESSOR  processor to compile for ($arch)
+	    --vmm=VMM              virtual machine monitor to compile for (qemu
+	                           or kvmtool, default is qemu) (arm/arm64 only)
 	    --cross-prefix=PREFIX  cross compiler prefix
 	    --cc=CC		   c compiler to use ($cc)
 	    --cxx=CXX		   c++ compiler to use ($cxx)
@@ -56,6 +59,9 @@ while [[ "$1" = -* ]]; do
         --processor)
 	    processor="$arg"
 	    ;;
+	--vmm)
+	    vmm="$arg"
+	    ;;
 	--cross-prefix)
 	    cross_prefix="$arg"
 	    ;;
@@ -108,6 +114,10 @@ if [ "$arch" = "i386" ] || [ "$arch" = "x86_64" ]; then
     testdir=x86
 elif [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
     testdir=arm
+    if [ "$vmm" != "qemu" ] && [ "$vmm" != "kvmtool" ]; then
+        echo '--vmm must be one of "qemu" or "kvmtool"!'
+        usage
+    fi
 elif [ "$arch" = "ppc64" ]; then
     testdir=powerpc
     firmware="$testdir/boot_rom.bin"
-- 
2.17.0

[kvm-unit-tests PATCH v3 3/5] lib: arm: Use ns16550a UART when --vmm=kvmtool

[Alexandru Elisei]

When kvm-unit-tests is configured with --vmm=kvmtool, use the address for
the ns16550a UART that kvmtool emulates. When the virtual machine manager
is QEMU, use the address for the pl011 UART, as before.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
 configure    |  9 +++++++--
 lib/arm/io.c | 27 ++++++++++++++++++---------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index d1dd7d5830eb..30112a812d0b 100755
--- a/configure
+++ b/configure
@@ -114,7 +114,11 @@ if [ "$arch" = "i386" ] || [ "$arch" = "x86_64" ]; then
     testdir=x86
 elif [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
     testdir=arm
-    if [ "$vmm" != "qemu" ] && [ "$vmm" != "kvmtool" ]; then
+    if [ "$vmm" = "qemu" ]; then
+        arm_uart_early_addr=0x09000000
+    elif [ "$vmm" = "kvmtool" ]; then
+        arm_uart_early_addr=0x3f8
+    else
         echo '--vmm must be one of "qemu" or "kvmtool"!'
         usage
     fi
@@ -216,11 +220,12 @@ cat <<EOF > lib/config.h
  * Generated file. DO NOT MODIFY.
  *
  */
+
 EOF
 if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
 cat <<EOF >> lib/config.h
 
-#define CONFIG_UART_EARLY_BASE 0x09000000
+#define CONFIG_UART_EARLY_BASE ${arm_uart_early_addr}
 
 EOF
 fi
diff --git a/lib/arm/io.c b/lib/arm/io.c
index e84a5c89fcb1..e55b8b854869 100644
--- a/lib/arm/io.c
+++ b/lib/arm/io.c
@@ -21,10 +21,10 @@ extern void halt(int code);
 
 static struct spinlock uart_lock;
 /*
- * Use this guess for the pl011 base in order to make an attempt at
+ * Use this guess for the uart base in order to make an attempt at
  * having earlier printf support. We'll overwrite it with the real
  * base address that we read from the device tree later. This is
- * the address we expect QEMU's mach-virt machine type to put in
+ * the address we expect the virtual machine manager to put in
  * its generated device tree.
  */
 #define UART_EARLY_BASE (u8 *)(unsigned long)CONFIG_UART_EARLY_BASE
@@ -32,22 +32,31 @@ static volatile u8 *uart0_base = UART_EARLY_BASE;
 
 static void uart0_init(void)
 {
-	const char *compatible = "arm,pl011";
+	/*
+	 * kvm-unit-tests uses the uart only for output. Both uart models have
+	 * the TX register at offset 0 from the base address, so there is no
+	 * need to treat them separately.
+	 */
+	const char *compatible[] = {"arm,pl011", "ns16550a"};
 	struct dt_pbus_reg base;
-	int ret;
+	int i, ret;
 
 	ret = dt_get_default_console_node();
 	assert(ret >= 0 || ret == -FDT_ERR_NOTFOUND);
 
 	if (ret == -FDT_ERR_NOTFOUND) {
 
-		ret = dt_pbus_get_base_compatible(compatible, &base);
-		assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
+		for (i = 0; i < ARRAY_SIZE(compatible); i++) {
+			ret = dt_pbus_get_base_compatible(compatible[i], &base);
+			assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
+
+			if (ret == 0)
+				break;
+		}
 
 		if (ret) {
-			printf("%s: %s not found in the device tree, "
-				"aborting...\n",
-				__func__, compatible);
+			printf("%s: Compatible uart not found in the device tree, "
+				"aborting...\n", __func__);
 			abort();
 		}
 
-- 
2.17.0

[kvm-unit-tests PATCH v3 4/5] lib: arm: Implement PSCI SYSTEM_OFF in psci_system_off()

[Alexandru Elisei]

A new function, psci_system_off(), is added which implements the PSCI
SYSTEM_OFF function. A call causes the hypervisor to terminate the virtual
machine.

We take this opportunity to rename psci_sys_reset() to psci_system_reset()
to match the name of the PSCI function SYSTEM_RESET that it implements.

Consumers for the function will be added in a later patch.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
 lib/arm/asm/psci.h | 3 ++-
 lib/arm/psci.c     | 8 +++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h
index ed51708fd265..7b956bf5987d 100644
--- a/lib/arm/asm/psci.h
+++ b/lib/arm/asm/psci.h
@@ -6,8 +6,9 @@
 extern int psci_invoke(unsigned long function_id, unsigned long arg0,
 		       unsigned long arg1, unsigned long arg2);
 extern int psci_cpu_on(unsigned long cpuid, unsigned long entry_point);
-extern void psci_sys_reset(void);
+extern void psci_system_reset(void);
 extern int cpu_psci_cpu_boot(unsigned int cpu);
 extern void cpu_psci_cpu_die(void);
+extern void psci_system_off(void);
 
 #endif /* _ASMARM_PSCI_H_ */
diff --git a/lib/arm/psci.c b/lib/arm/psci.c
index 119f74e57e91..c3d399064ae3 100644
--- a/lib/arm/psci.c
+++ b/lib/arm/psci.c
@@ -48,7 +48,13 @@ void cpu_psci_cpu_die(void)
 	printf("CPU%d unable to power off (error = %d)\n", smp_processor_id(), err);
 }
 
-void psci_sys_reset(void)
+void psci_system_reset(void)
 {
 	psci_invoke(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
 }
+
+void psci_system_off(void)
+{
+	int err = psci_invoke(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
+	printf("CPU%d unable to do system off (error = %d)\n", smp_processor_id(), err);
+}
-- 
2.17.0

[kvm-unit-tests PATCH v3 5/5] lib: arm: Fallback to psci_system_off() in exit()

[Alexandru Elisei]

On arm and arm64, kvm-unit-tests uses the QEMU chr-testdev device to shut
down the virtual machine at the end of a test. The function
psci_system_off() provides another mechanism for terminating the virtual
machine.

chr-testdev is implemented on top of virtio console. If the virtual machine
manager doesn't emulate a virtio console, then chr_testdev_exit() will
fail. If this happens, try to use psci_system_off() to terminate the test.

This patch makes it possible for a virtual machine manager which doesn't
have support for chr-testdev, but has been configured not to emulate a
virtio console, to gracefully terminate a virtual machine after a test has
been completed.

There is one limitation to using psci_system_off() to terminate a test:
chr-testdev allows kvm-unit-tests to specify an exit code;
psci_system_off() has no such mechanism.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
 lib/arm/io.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/arm/io.c b/lib/arm/io.c
index e55b8b854869..8226b765bdc5 100644
--- a/lib/arm/io.c
+++ b/lib/arm/io.c
@@ -12,6 +12,7 @@
 #include <devicetree.h>
 #include <chr-testdev.h>
 #include <config.h>
+#include <asm/psci.h>
 #include <asm/spinlock.h>
 #include <asm/io.h>
 
@@ -91,6 +92,7 @@ void puts(const char *s)
 void exit(int code)
 {
 	chr_testdev_exit(code);
+	psci_system_off();
 	halt(code);
 	__builtin_unreachable();
 }
-- 
2.17.0

Re: [kvm-unit-tests PATCH v3 1/5] lib: arm: Use UART address from generated config.h

[Andrew Jones]

On Mon, Feb 04, 2019 at 01:44:08PM +0000, Alexandru Elisei wrote:
> Generate lib/config.h when configuring kvm-unit-tests. The file is empty
> for all architectures except for arm and arm64, where it is used to store
> the UART base address. This removes the hardcoded address from lib/arm/io.c
> and provides a mechanism for using different UART addresses in the future.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  configure    | 17 +++++++++++++++++
>  Makefile     |  2 +-
>  lib/arm/io.c | 12 ++++++------
>  .gitignore   |  1 +
>  4 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/configure b/configure
> index df8581e3a906..b4732c6c04e4 100755
> --- a/configure
> +++ b/configure
> @@ -198,3 +198,20 @@ ENVIRON_DEFAULT=$environ_default
>  ERRATATXT=errata.txt
>  U32_LONG_FMT=$u32_long
>  EOF
> +
> +cat <<EOF > lib/config.h
> +#ifndef CONFIG_H
> +#define CONFIG_H 1
> +/*
> + * Generated file. DO NOT MODIFY.
> + *
> + */
> +EOF
> +if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
> +cat <<EOF >> lib/config.h
> +
> +#define CONFIG_UART_EARLY_BASE 0x09000000
> +
> +EOF
> +fi
> +echo "#endif" >> lib/config.h
> diff --git a/Makefile b/Makefile
> index e9f02272e156..643af05678ad 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -115,7 +115,7 @@ libfdt_clean:
>  	$(LIBFDT_objdir)/.*.d
>  
>  distclean: clean libfdt_clean
> -	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> +	$(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
>  	$(RM) -r tests logs logs.old
>  
>  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
> diff --git a/lib/arm/io.c b/lib/arm/io.c
> index d2c1a07c19ee..e84a5c89fcb1 100644
> --- a/lib/arm/io.c
> +++ b/lib/arm/io.c
> @@ -11,6 +11,7 @@
>  #include <libcflat.h>
>  #include <devicetree.h>
>  #include <chr-testdev.h>
> +#include <config.h>
>  #include <asm/spinlock.h>
>  #include <asm/io.h>
>  
> @@ -18,6 +19,7 @@
>  
>  extern void halt(int code);
>  
> +static struct spinlock uart_lock;
>  /*
>   * Use this guess for the pl011 base in order to make an attempt at
>   * having earlier printf support. We'll overwrite it with the real
> @@ -25,10 +27,8 @@ extern void halt(int code);
>   * the address we expect QEMU's mach-virt machine type to put in
>   * its generated device tree.
>   */
> -#define UART_EARLY_BASE 0x09000000UL
> -
> -static struct spinlock uart_lock;
> -static volatile u8 *uart0_base = (u8 *)UART_EARLY_BASE;
> +#define UART_EARLY_BASE (u8 *)(unsigned long)CONFIG_UART_EARLY_BASE
> +static volatile u8 *uart0_base = UART_EARLY_BASE;
>  
>  static void uart0_init(void)
>  {
> @@ -58,10 +58,10 @@ static void uart0_init(void)
>  
>  	uart0_base = ioremap(base.addr, base.size);
>  
> -	if (uart0_base != (u8 *)UART_EARLY_BASE) {
> +	if (uart0_base != UART_EARLY_BASE) {
>  		printf("WARNING: early print support may not work. "
>  		       "Found uart at %p, but early base is %p.\n",
> -			uart0_base, (u8 *)UART_EARLY_BASE);
> +			uart0_base, UART_EARLY_BASE);
>  	}
>  }
>  
> diff --git a/.gitignore b/.gitignore
> index 2405a8087ae5..483f7c7a09ea 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -10,6 +10,7 @@ patches
>  cscope.*
>  *.swp
>  /lib/asm
> +/lib/config.h
>  /config.mak
>  /*-run
>  /msr.out
> -- 
> 2.17.0
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [kvm-unit-tests PATCH v3 1/5] lib: arm: Use UART address from generated config.h

[Alexandru Elisei]

On 2/4/19 2:00 PM, Andrew Jones wrote:
> On Mon, Feb 04, 2019 at 01:44:08PM +0000, Alexandru Elisei wrote:
>> Generate lib/config.h when configuring kvm-unit-tests. The file is empty
>> for all architectures except for arm and arm64, where it is used to store
>> the UART base address. This removes the hardcoded address from lib/arm/io.c
>> and provides a mechanism for using different UART addresses in the future.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>  configure    | 17 +++++++++++++++++
>>  Makefile     |  2 +-
>>  lib/arm/io.c | 12 ++++++------
>>  .gitignore   |  1 +
>>  4 files changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/configure b/configure
>> index df8581e3a906..b4732c6c04e4 100755
>> --- a/configure
>> +++ b/configure
>> @@ -198,3 +198,20 @@ ENVIRON_DEFAULT=$environ_default
>>  ERRATATXT=errata.txt
>>  U32_LONG_FMT=$u32_long
>>  EOF
>> +
>> +cat <<EOF > lib/config.h
>> +#ifndef CONFIG_H
>> +#define CONFIG_H 1
>> +/*
>> + * Generated file. DO NOT MODIFY.
>> + *
>> + */
>> +EOF
>> +if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
>> +cat <<EOF >> lib/config.h
>> +
>> +#define CONFIG_UART_EARLY_BASE 0x09000000
>> +
>> +EOF
>> +fi
>> +echo "#endif" >> lib/config.h
>> diff --git a/Makefile b/Makefile
>> index e9f02272e156..643af05678ad 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -115,7 +115,7 @@ libfdt_clean:
>>  	$(LIBFDT_objdir)/.*.d
>>  
>>  distclean: clean libfdt_clean
>> -	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
>> +	$(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
>>  	$(RM) -r tests logs logs.old
>>  
>>  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
>> diff --git a/lib/arm/io.c b/lib/arm/io.c
>> index d2c1a07c19ee..e84a5c89fcb1 100644
>> --- a/lib/arm/io.c
>> +++ b/lib/arm/io.c
>> @@ -11,6 +11,7 @@
>>  #include <libcflat.h>
>>  #include <devicetree.h>
>>  #include <chr-testdev.h>
>> +#include <config.h>
>>  #include <asm/spinlock.h>
>>  #include <asm/io.h>
>>  
>> @@ -18,6 +19,7 @@
>>  
>>  extern void halt(int code);
>>  
>> +static struct spinlock uart_lock;
>>  /*
>>   * Use this guess for the pl011 base in order to make an attempt at
>>   * having earlier printf support. We'll overwrite it with the real
>> @@ -25,10 +27,8 @@ extern void halt(int code);
>>   * the address we expect QEMU's mach-virt machine type to put in
>>   * its generated device tree.
>>   */
>> -#define UART_EARLY_BASE 0x09000000UL
>> -
>> -static struct spinlock uart_lock;
>> -static volatile u8 *uart0_base = (u8 *)UART_EARLY_BASE;
>> +#define UART_EARLY_BASE (u8 *)(unsigned long)CONFIG_UART_EARLY_BASE
>> +static volatile u8 *uart0_base = UART_EARLY_BASE;
>>  
>>  static void uart0_init(void)
>>  {
>> @@ -58,10 +58,10 @@ static void uart0_init(void)
>>  
>>  	uart0_base = ioremap(base.addr, base.size);
>>  
>> -	if (uart0_base != (u8 *)UART_EARLY_BASE) {
>> +	if (uart0_base != UART_EARLY_BASE) {
>>  		printf("WARNING: early print support may not work. "
>>  		       "Found uart at %p, but early base is %p.\n",
>> -			uart0_base, (u8 *)UART_EARLY_BASE);
>> +			uart0_base, UART_EARLY_BASE);
>>  	}
>>  }
>>  
>> diff --git a/.gitignore b/.gitignore
>> index 2405a8087ae5..483f7c7a09ea 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -10,6 +10,7 @@ patches
>>  cscope.*
>>  *.swp
>>  /lib/asm
>> +/lib/config.h
>>  /config.mak
>>  /*-run
>>  /msr.out
>> -- 
>> 2.17.0
>>
> Reviewed-by: Andrew Jones <drjones@redhat.com>

Thank you for taking the time to review the patches!

What is my next step?

Re: [kvm-unit-tests PATCH v3 0/5] arm/arm64: Add support for running under kvmtool

[Andrew Jones]

On Mon, Feb 04, 2019 at 01:44:07PM +0000, Alexandru Elisei wrote:
> kvm-unit-tests is designed to be run with QEMU as the virtual machine
> monitor. It relies on devices emulated by QEMU (like isa-debug-exit or
> testdev) and it makes certain assumptions based on the implicit QEMU
> virtual environment configuration (like the serial base address).
> 
> kvmtool [1] is a lightweight virtual machine monitor for running KVM
> guests. kvmtool has reduced complexity compared to QEMU and is easily
> hackable.
> 
> This patch series aims to make it possible to run kvm-unit-tests using
> kvmtool on the arm and arm64 architectures, with two caveats:
> 
> (1) When terminating a test, the userspace process won't exit with an exit
> code that signals the success or failure of the test. Output from the test
> can still be parsed to determine the outcome of the test.
> 
> (2) kvmtool has been designed to work with a linux guest and it
> automatically generates the command line arguments for a Linux kernel. The
> arm/arm64 selftest and gic tests will fail if unexpected command line
> arguments are found. To get around this limitation, the test binary needs
> to be loaded using the --firmware option introduced by kvmtool in commit
> 5e4b563d75b9 ("arm: Allow command line for firmware"). This option
> suppresses the automatic kernel command line and can be used to run all
> tests, not just the tests that require specific arguments.
> 
> The run scripts haven't been modified. To run a test under kvmtool, one
> needs to launch kvmtool manually. For example, to run the timer test the
> following command can be used:
> 
> lkvm run --cpus 1 --console serial --firmware timer.flat.
> 
> To run the gicv3-ipi test:
> 
> lkvm run --cpus 8 --console serial --params "ipi" --irqchip gicv3 \
>     --firmware gic.flat

This looks good to me. I tested with lkvm and saw that all tests were able
to run and pass, except pci-test since lkvm doesn't have pci-testdev. The
only other issue was selftest.vectors-user which ends in user mode and
thus can't make the PSCI call to exit. The following patch can get that
to work if lkvm users want it

diff --git a/arm/selftest.c b/arm/selftest.c
index ea5101ef7217..7ba3f02a9b9d 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -272,10 +272,18 @@ static bool check_svc(void)
 }
 #endif
 
+static void user_psci_off(struct pt_regs *regs, unsigned int esr)
+{
+       psci_system_off();
+       halt();
+}
+
 static void check_vectors(void *arg __unused)
 {
        report("und", check_und());
        report("svc", check_svc());
+       if (is_user())
+               install_exception_handler(EL0_SYNC_64, ESR_EL1_EC_UNKNOWN, user_psci_off);
        exit(report_summary());
 }


Anyway, thanks for submitting these patches to enable another user of
kvm-unit-tests.

drew


> 
> Changes in v3:
> * Updated cover letter with information about the kvmtool --firmware
>   option.
> * Gathered Reviewed-by tags.
> * Renamed the config.h define UART_EARLY_BASE to CONFIG_UART_EARLY_BASE and
>   made the necessary casts in lib/arm/io.c
> 
> Changes in v2:
> * Generate lib/config.h when configuring kvm-unit-tests; arm/arm64 uses it
>   to get the UART address.
> * Added --vmm configure option for arm/arm64 which will set the UART
>   address in lib/config.h when the tests are run under QEMU or kvmtool.
> * Renamed psci_sys_reset() to psci_system_reset().
> * Dropped patches that allowed a test to ignore unexpected command line
>   arguments.
> 
> Summary:
> * Patches 1, 2 and 3 add support for configuring kvm-unit-tests on arm and
>   arm64 to use the ns16550a UART emulated by kvmtool.
> * Patches 4 and 5 provide an alternative mechanism for terminating the
>   virtual machine by using PSCI.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git/
> [2] https://www.spinics.net/lists/kvm-arm/msg34352.html
> 
> Alexandru Elisei (5):
>   lib: arm: Use UART address from generated config.h
>   configure: arm/arm64: Add --vmm option with no effect
>   lib: arm: Use ns16550a UART when --vmm=kvmtool
>   lib: arm: Implement PSCI SYSTEM_OFF in psci_system_off()
>   lib: arm: Fallback to psci_system_off() in exit()
> 
>  configure          | 32 ++++++++++++++++++++++++++++++++
>  Makefile           |  2 +-
>  lib/arm/asm/psci.h |  3 ++-
>  lib/arm/io.c       | 41 ++++++++++++++++++++++++++---------------
>  lib/arm/psci.c     |  8 +++++++-
>  .gitignore         |  1 +
>  6 files changed, 69 insertions(+), 18 deletions(-)
> 
> -- 
> 2.17.0
> 

Re: [kvm-unit-tests PATCH v3 1/5] lib: arm: Use UART address from generated config.h

[Andrew Jones]

On Mon, Feb 04, 2019 at 02:17:05PM +0000, Alexandru Elisei wrote:
> On 2/4/19 2:00 PM, Andrew Jones wrote:
> > On Mon, Feb 04, 2019 at 01:44:08PM +0000, Alexandru Elisei wrote:
> >> Generate lib/config.h when configuring kvm-unit-tests. The file is empty
> >> for all architectures except for arm and arm64, where it is used to store
> >> the UART base address. This removes the hardcoded address from lib/arm/io.c
> >> and provides a mechanism for using different UART addresses in the future.
> >>
> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> >> ---
> >>  configure    | 17 +++++++++++++++++
> >>  Makefile     |  2 +-
> >>  lib/arm/io.c | 12 ++++++------
> >>  .gitignore   |  1 +
> >>  4 files changed, 25 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/configure b/configure
> >> index df8581e3a906..b4732c6c04e4 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -198,3 +198,20 @@ ENVIRON_DEFAULT=$environ_default
> >>  ERRATATXT=errata.txt
> >>  U32_LONG_FMT=$u32_long
> >>  EOF
> >> +
> >> +cat <<EOF > lib/config.h
> >> +#ifndef CONFIG_H
> >> +#define CONFIG_H 1
> >> +/*
> >> + * Generated file. DO NOT MODIFY.
> >> + *
> >> + */
> >> +EOF
> >> +if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
> >> +cat <<EOF >> lib/config.h
> >> +
> >> +#define CONFIG_UART_EARLY_BASE 0x09000000
> >> +
> >> +EOF
> >> +fi
> >> +echo "#endif" >> lib/config.h
> >> diff --git a/Makefile b/Makefile
> >> index e9f02272e156..643af05678ad 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -115,7 +115,7 @@ libfdt_clean:
> >>  	$(LIBFDT_objdir)/.*.d
> >>  
> >>  distclean: clean libfdt_clean
> >> -	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> >> +	$(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
> >>  	$(RM) -r tests logs logs.old
> >>  
> >>  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
> >> diff --git a/lib/arm/io.c b/lib/arm/io.c
> >> index d2c1a07c19ee..e84a5c89fcb1 100644
> >> --- a/lib/arm/io.c
> >> +++ b/lib/arm/io.c
> >> @@ -11,6 +11,7 @@
> >>  #include <libcflat.h>
> >>  #include <devicetree.h>
> >>  #include <chr-testdev.h>
> >> +#include <config.h>
> >>  #include <asm/spinlock.h>
> >>  #include <asm/io.h>
> >>  
> >> @@ -18,6 +19,7 @@
> >>  
> >>  extern void halt(int code);
> >>  
> >> +static struct spinlock uart_lock;
> >>  /*
> >>   * Use this guess for the pl011 base in order to make an attempt at
> >>   * having earlier printf support. We'll overwrite it with the real
> >> @@ -25,10 +27,8 @@ extern void halt(int code);
> >>   * the address we expect QEMU's mach-virt machine type to put in
> >>   * its generated device tree.
> >>   */
> >> -#define UART_EARLY_BASE 0x09000000UL
> >> -
> >> -static struct spinlock uart_lock;
> >> -static volatile u8 *uart0_base = (u8 *)UART_EARLY_BASE;
> >> +#define UART_EARLY_BASE (u8 *)(unsigned long)CONFIG_UART_EARLY_BASE
> >> +static volatile u8 *uart0_base = UART_EARLY_BASE;
> >>  
> >>  static void uart0_init(void)
> >>  {
> >> @@ -58,10 +58,10 @@ static void uart0_init(void)
> >>  
> >>  	uart0_base = ioremap(base.addr, base.size);
> >>  
> >> -	if (uart0_base != (u8 *)UART_EARLY_BASE) {
> >> +	if (uart0_base != UART_EARLY_BASE) {
> >>  		printf("WARNING: early print support may not work. "
> >>  		       "Found uart at %p, but early base is %p.\n",
> >> -			uart0_base, (u8 *)UART_EARLY_BASE);
> >> +			uart0_base, UART_EARLY_BASE);
> >>  	}
> >>  }
> >>  
> >> diff --git a/.gitignore b/.gitignore
> >> index 2405a8087ae5..483f7c7a09ea 100644
> >> --- a/.gitignore
> >> +++ b/.gitignore
> >> @@ -10,6 +10,7 @@ patches
> >>  cscope.*
> >>  *.swp
> >>  /lib/asm
> >> +/lib/config.h
> >>  /config.mak
> >>  /*-run
> >>  /msr.out
> >> -- 
> >> 2.17.0
> >>
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> 
> Thank you for taking the time to review the patches!
> 
> What is my next step?
>

Wait for Paolo or Radim to merge :-)  I've added them to CC.

drew

Re: [kvm-unit-tests PATCH v3 0/5] arm/arm64: Add support for running under kvmtool

[Alexandru Elisei]

On 2/4/19 2:39 PM, Andrew Jones wrote:
> On Mon, Feb 04, 2019 at 01:44:07PM +0000, Alexandru Elisei wrote:
>> kvm-unit-tests is designed to be run with QEMU as the virtual machine
>> monitor. It relies on devices emulated by QEMU (like isa-debug-exit or
>> testdev) and it makes certain assumptions based on the implicit QEMU
>> virtual environment configuration (like the serial base address).
>>
>> kvmtool [1] is a lightweight virtual machine monitor for running KVM
>> guests. kvmtool has reduced complexity compared to QEMU and is easily
>> hackable.
>>
>> This patch series aims to make it possible to run kvm-unit-tests using
>> kvmtool on the arm and arm64 architectures, with two caveats:
>>
>> (1) When terminating a test, the userspace process won't exit with an exit
>> code that signals the success or failure of the test. Output from the test
>> can still be parsed to determine the outcome of the test.
>>
>> (2) kvmtool has been designed to work with a linux guest and it
>> automatically generates the command line arguments for a Linux kernel. The
>> arm/arm64 selftest and gic tests will fail if unexpected command line
>> arguments are found. To get around this limitation, the test binary needs
>> to be loaded using the --firmware option introduced by kvmtool in commit
>> 5e4b563d75b9 ("arm: Allow command line for firmware"). This option
>> suppresses the automatic kernel command line and can be used to run all
>> tests, not just the tests that require specific arguments.
>>
>> The run scripts haven't been modified. To run a test under kvmtool, one
>> needs to launch kvmtool manually. For example, to run the timer test the
>> following command can be used:
>>
>> lkvm run --cpus 1 --console serial --firmware timer.flat.
>>
>> To run the gicv3-ipi test:
>>
>> lkvm run --cpus 8 --console serial --params "ipi" --irqchip gicv3 \
>>     --firmware gic.flat
> This looks good to me. I tested with lkvm and saw that all tests were able
> to run and pass, except pci-test since lkvm doesn't have pci-testdev. The
> only other issue was selftest.vectors-user which ends in user mode and
> thus can't make the PSCI call to exit. The following patch can get that
> to work if lkvm users want it
>
> diff --git a/arm/selftest.c b/arm/selftest.c
> index ea5101ef7217..7ba3f02a9b9d 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -272,10 +272,18 @@ static bool check_svc(void)
>  }
>  #endif
>  
> +static void user_psci_off(struct pt_regs *regs, unsigned int esr)
> +{
> +       psci_system_off();
I was wondering if it's worth checking that the cause for the unknown exception
was actually the instruction hvc #0 from user space. Getting the function id
from x0/r0 is trivial, and we could also check the opcode from the memory
location pointed to by the pc.
> +       halt();
> +}
> +
>  static void check_vectors(void *arg __unused)
>  {
>         report("und", check_und());
>         report("svc", check_svc());
> +       if (is_user())
> +               install_exception_handler(EL0_SYNC_64, ESR_EL1_EC_UNKNOWN, user_psci_off);
>         exit(report_summary());
>  }

Thank you for posting this, when I started playing with kvm-unit-tests I noticed
that selftest-vectors-user isn't working, but I totally forgot about it. I am
interested in the patch, do you want to write the patch yourself? If not, I can
pick it up and submit it.

>
>
> Anyway, thanks for submitting these patches to enable another user of
> kvm-unit-tests.
>
> drew
>
>
>> Changes in v3:
>> * Updated cover letter with information about the kvmtool --firmware
>>   option.
>> * Gathered Reviewed-by tags.
>> * Renamed the config.h define UART_EARLY_BASE to CONFIG_UART_EARLY_BASE and
>>   made the necessary casts in lib/arm/io.c
>>
>> Changes in v2:
>> * Generate lib/config.h when configuring kvm-unit-tests; arm/arm64 uses it
>>   to get the UART address.
>> * Added --vmm configure option for arm/arm64 which will set the UART
>>   address in lib/config.h when the tests are run under QEMU or kvmtool.
>> * Renamed psci_sys_reset() to psci_system_reset().
>> * Dropped patches that allowed a test to ignore unexpected command line
>>   arguments.
>>
>> Summary:
>> * Patches 1, 2 and 3 add support for configuring kvm-unit-tests on arm and
>>   arm64 to use the ns16550a UART emulated by kvmtool.
>> * Patches 4 and 5 provide an alternative mechanism for terminating the
>>   virtual machine by using PSCI.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git/
>> [2] https://www.spinics.net/lists/kvm-arm/msg34352.html
>>
>> Alexandru Elisei (5):
>>   lib: arm: Use UART address from generated config.h
>>   configure: arm/arm64: Add --vmm option with no effect
>>   lib: arm: Use ns16550a UART when --vmm=kvmtool
>>   lib: arm: Implement PSCI SYSTEM_OFF in psci_system_off()
>>   lib: arm: Fallback to psci_system_off() in exit()
>>
>>  configure          | 32 ++++++++++++++++++++++++++++++++
>>  Makefile           |  2 +-
>>  lib/arm/asm/psci.h |  3 ++-
>>  lib/arm/io.c       | 41 ++++++++++++++++++++++++++---------------
>>  lib/arm/psci.c     |  8 +++++++-
>>  .gitignore         |  1 +
>>  6 files changed, 69 insertions(+), 18 deletions(-)
>>
>> -- 
>> 2.17.0
>>

Re: [kvm-unit-tests PATCH v3 0/5] arm/arm64: Add support for running under kvmtool

[Andrew Jones]

On Tue, Feb 05, 2019 at 12:05:35PM +0000, Alexandru Elisei wrote:
> On 2/4/19 2:39 PM, Andrew Jones wrote:
> > On Mon, Feb 04, 2019 at 01:44:07PM +0000, Alexandru Elisei wrote:
> >> kvm-unit-tests is designed to be run with QEMU as the virtual machine
> >> monitor. It relies on devices emulated by QEMU (like isa-debug-exit or
> >> testdev) and it makes certain assumptions based on the implicit QEMU
> >> virtual environment configuration (like the serial base address).
> >>
> >> kvmtool [1] is a lightweight virtual machine monitor for running KVM
> >> guests. kvmtool has reduced complexity compared to QEMU and is easily
> >> hackable.
> >>
> >> This patch series aims to make it possible to run kvm-unit-tests using
> >> kvmtool on the arm and arm64 architectures, with two caveats:
> >>
> >> (1) When terminating a test, the userspace process won't exit with an exit
> >> code that signals the success or failure of the test. Output from the test
> >> can still be parsed to determine the outcome of the test.
> >>
> >> (2) kvmtool has been designed to work with a linux guest and it
> >> automatically generates the command line arguments for a Linux kernel. The
> >> arm/arm64 selftest and gic tests will fail if unexpected command line
> >> arguments are found. To get around this limitation, the test binary needs
> >> to be loaded using the --firmware option introduced by kvmtool in commit
> >> 5e4b563d75b9 ("arm: Allow command line for firmware"). This option
> >> suppresses the automatic kernel command line and can be used to run all
> >> tests, not just the tests that require specific arguments.
> >>
> >> The run scripts haven't been modified. To run a test under kvmtool, one
> >> needs to launch kvmtool manually. For example, to run the timer test the
> >> following command can be used:
> >>
> >> lkvm run --cpus 1 --console serial --firmware timer.flat.
> >>
> >> To run the gicv3-ipi test:
> >>
> >> lkvm run --cpus 8 --console serial --params "ipi" --irqchip gicv3 \
> >>     --firmware gic.flat
> > This looks good to me. I tested with lkvm and saw that all tests were able
> > to run and pass, except pci-test since lkvm doesn't have pci-testdev. The
> > only other issue was selftest.vectors-user which ends in user mode and
> > thus can't make the PSCI call to exit. The following patch can get that
> > to work if lkvm users want it
> >
> > diff --git a/arm/selftest.c b/arm/selftest.c
> > index ea5101ef7217..7ba3f02a9b9d 100644
> > --- a/arm/selftest.c
> > +++ b/arm/selftest.c
> > @@ -272,10 +272,18 @@ static bool check_svc(void)
> >  }
> >  #endif
> >  
> > +static void user_psci_off(struct pt_regs *regs, unsigned int esr)
> > +{
> > +       psci_system_off();
> I was wondering if it's worth checking that the cause for the unknown exception
> was actually the instruction hvc #0 from user space. Getting the function id
> from x0/r0 is trivial, and we could also check the opcode from the memory
> location pointed to by the pc.
> > +       halt();
> > +}
> > +
> >  static void check_vectors(void *arg __unused)
> >  {
> >         report("und", check_und());
> >         report("svc", check_svc());
> > +       if (is_user())
> > +               install_exception_handler(EL0_SYNC_64, ESR_EL1_EC_UNKNOWN, user_psci_off);
> >         exit(report_summary());
> >  }
> 
> Thank you for posting this, when I started playing with kvm-unit-tests I noticed
> that selftest-vectors-user isn't working, but I totally forgot about it. I am
> interested in the patch, do you want to write the patch yourself? If not, I can
> pick it up and submit it.

I'll post it as 6/5 of this series. Have you reviewed/tested it? I.e. any
tags you'd like me to add to the posting?

Thanks,
drew

> 
> >
> >
> > Anyway, thanks for submitting these patches to enable another user of
> > kvm-unit-tests.
> >
> > drew
> >
> >
> >> Changes in v3:
> >> * Updated cover letter with information about the kvmtool --firmware
> >>   option.
> >> * Gathered Reviewed-by tags.
> >> * Renamed the config.h define UART_EARLY_BASE to CONFIG_UART_EARLY_BASE and
> >>   made the necessary casts in lib/arm/io.c
> >>
> >> Changes in v2:
> >> * Generate lib/config.h when configuring kvm-unit-tests; arm/arm64 uses it
> >>   to get the UART address.
> >> * Added --vmm configure option for arm/arm64 which will set the UART
> >>   address in lib/config.h when the tests are run under QEMU or kvmtool.
> >> * Renamed psci_sys_reset() to psci_system_reset().
> >> * Dropped patches that allowed a test to ignore unexpected command line
> >>   arguments.
> >>
> >> Summary:
> >> * Patches 1, 2 and 3 add support for configuring kvm-unit-tests on arm and
> >>   arm64 to use the ns16550a UART emulated by kvmtool.
> >> * Patches 4 and 5 provide an alternative mechanism for terminating the
> >>   virtual machine by using PSCI.
> >>
> >> [1] https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git/
> >> [2] https://www.spinics.net/lists/kvm-arm/msg34352.html
> >>
> >> Alexandru Elisei (5):
> >>   lib: arm: Use UART address from generated config.h
> >>   configure: arm/arm64: Add --vmm option with no effect
> >>   lib: arm: Use ns16550a UART when --vmm=kvmtool
> >>   lib: arm: Implement PSCI SYSTEM_OFF in psci_system_off()
> >>   lib: arm: Fallback to psci_system_off() in exit()
> >>
> >>  configure          | 32 ++++++++++++++++++++++++++++++++
> >>  Makefile           |  2 +-
> >>  lib/arm/asm/psci.h |  3 ++-
> >>  lib/arm/io.c       | 41 ++++++++++++++++++++++++++---------------
> >>  lib/arm/psci.c     |  8 +++++++-
> >>  .gitignore         |  1 +
> >>  6 files changed, 69 insertions(+), 18 deletions(-)
> >>
> >> -- 
> >> 2.17.0
> >>

Re: [kvm-unit-tests PATCH v3 0/5] arm/arm64: Add support for running under kvmtool

[Alexandru Elisei]

On 2/5/19 12:38 PM, Andrew Jones wrote:
> On Tue, Feb 05, 2019 at 12:05:35PM +0000, Alexandru Elisei wrote:
>> On 2/4/19 2:39 PM, Andrew Jones wrote:
>>> On Mon, Feb 04, 2019 at 01:44:07PM +0000, Alexandru Elisei wrote:
>>>> kvm-unit-tests is designed to be run with QEMU as the virtual machine
>>>> monitor. It relies on devices emulated by QEMU (like isa-debug-exit or
>>>> testdev) and it makes certain assumptions based on the implicit QEMU
>>>> virtual environment configuration (like the serial base address).
>>>>
>>>> kvmtool [1] is a lightweight virtual machine monitor for running KVM
>>>> guests. kvmtool has reduced complexity compared to QEMU and is easily
>>>> hackable.
>>>>
>>>> This patch series aims to make it possible to run kvm-unit-tests using
>>>> kvmtool on the arm and arm64 architectures, with two caveats:
>>>>
>>>> (1) When terminating a test, the userspace process won't exit with an exit
>>>> code that signals the success or failure of the test. Output from the test
>>>> can still be parsed to determine the outcome of the test.
>>>>
>>>> (2) kvmtool has been designed to work with a linux guest and it
>>>> automatically generates the command line arguments for a Linux kernel. The
>>>> arm/arm64 selftest and gic tests will fail if unexpected command line
>>>> arguments are found. To get around this limitation, the test binary needs
>>>> to be loaded using the --firmware option introduced by kvmtool in commit
>>>> 5e4b563d75b9 ("arm: Allow command line for firmware"). This option
>>>> suppresses the automatic kernel command line and can be used to run all
>>>> tests, not just the tests that require specific arguments.
>>>>
>>>> The run scripts haven't been modified. To run a test under kvmtool, one
>>>> needs to launch kvmtool manually. For example, to run the timer test the
>>>> following command can be used:
>>>>
>>>> lkvm run --cpus 1 --console serial --firmware timer.flat.
>>>>
>>>> To run the gicv3-ipi test:
>>>>
>>>> lkvm run --cpus 8 --console serial --params "ipi" --irqchip gicv3 \
>>>>     --firmware gic.flat
>>> This looks good to me. I tested with lkvm and saw that all tests were able
>>> to run and pass, except pci-test since lkvm doesn't have pci-testdev. The
>>> only other issue was selftest.vectors-user which ends in user mode and
>>> thus can't make the PSCI call to exit. The following patch can get that
>>> to work if lkvm users want it
>>>
>>> diff --git a/arm/selftest.c b/arm/selftest.c
>>> index ea5101ef7217..7ba3f02a9b9d 100644
>>> --- a/arm/selftest.c
>>> +++ b/arm/selftest.c
>>> @@ -272,10 +272,18 @@ static bool check_svc(void)
>>>  }
>>>  #endif
>>>  
>>> +static void user_psci_off(struct pt_regs *regs, unsigned int esr)
>>> +{
>>> +       psci_system_off();
>> I was wondering if it's worth checking that the cause for the unknown exception
>> was actually the instruction hvc #0 from user space. Getting the function id
>> from x0/r0 is trivial, and we could also check the opcode from the memory
>> location pointed to by the pc.
>>> +       halt();
>>> +}
>>> +
>>>  static void check_vectors(void *arg __unused)
>>>  {
>>>         report("und", check_und());
>>>         report("svc", check_svc());
>>> +       if (is_user())
>>> +               install_exception_handler(EL0_SYNC_64, ESR_EL1_EC_UNKNOWN, user_psci_off);
>>>         exit(report_summary());
>>>  }
>> Thank you for posting this, when I started playing with kvm-unit-tests I noticed
>> that selftest-vectors-user isn't working, but I totally forgot about it. I am
>> interested in the patch, do you want to write the patch yourself? If not, I can
>> pick it up and submit it.
> I'll post it as 6/5 of this series. Have you reviewed/tested it? I.e. any
> tags you'd like me to add to the posting?

Commit 96d79976f7795 ("mark exit() and abort() as non-returning functions")
added __builtin_unreachable() after functions that are not expected to return.
You might want to add it after halt().

But other than that:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>

>
> Thanks,
> drew
>
>>>
>>> Anyway, thanks for submitting these patches to enable another user of
>>> kvm-unit-tests.
>>>
>>> drew
>>>
>>>
>>>> Changes in v3:
>>>> * Updated cover letter with information about the kvmtool --firmware
>>>>   option.
>>>> * Gathered Reviewed-by tags.
>>>> * Renamed the config.h define UART_EARLY_BASE to CONFIG_UART_EARLY_BASE and
>>>>   made the necessary casts in lib/arm/io.c
>>>>
>>>> Changes in v2:
>>>> * Generate lib/config.h when configuring kvm-unit-tests; arm/arm64 uses it
>>>>   to get the UART address.
>>>> * Added --vmm configure option for arm/arm64 which will set the UART
>>>>   address in lib/config.h when the tests are run under QEMU or kvmtool.
>>>> * Renamed psci_sys_reset() to psci_system_reset().
>>>> * Dropped patches that allowed a test to ignore unexpected command line
>>>>   arguments.
>>>>
>>>> Summary:
>>>> * Patches 1, 2 and 3 add support for configuring kvm-unit-tests on arm and
>>>>   arm64 to use the ns16550a UART emulated by kvmtool.
>>>> * Patches 4 and 5 provide an alternative mechanism for terminating the
>>>>   virtual machine by using PSCI.
>>>>
>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git/
>>>> [2] https://www.spinics.net/lists/kvm-arm/msg34352.html
>>>>
>>>> Alexandru Elisei (5):
>>>>   lib: arm: Use UART address from generated config.h
>>>>   configure: arm/arm64: Add --vmm option with no effect
>>>>   lib: arm: Use ns16550a UART when --vmm=kvmtool
>>>>   lib: arm: Implement PSCI SYSTEM_OFF in psci_system_off()
>>>>   lib: arm: Fallback to psci_system_off() in exit()
>>>>
>>>>  configure          | 32 ++++++++++++++++++++++++++++++++
>>>>  Makefile           |  2 +-
>>>>  lib/arm/asm/psci.h |  3 ++-
>>>>  lib/arm/io.c       | 41 ++++++++++++++++++++++++++---------------
>>>>  lib/arm/psci.c     |  8 +++++++-
>>>>  .gitignore         |  1 +
>>>>  6 files changed, 69 insertions(+), 18 deletions(-)
>>>>
>>>> -- 
>>>> 2.17.0
>>>>

[PATCH 6/5] arm/arm64: selftest.vectors-user: clean up PSCI exit

[Andrew Jones]

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---

The original posting didn't consider arm32. I kept Alexandru's tags
for this new version though, as the tweaks should be safe. Also
added __builtin_unreachable as suggested by Alexandru.

 arm/selftest.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arm/selftest.c b/arm/selftest.c
index ea5101ef7217..28a17f7a7531 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -17,6 +17,13 @@
 #include <asm/smp.h>
 #include <asm/barrier.h>
 
+static void __user_psci_system_off(void)
+{
+	psci_system_off();
+	halt();
+	__builtin_unreachable();
+}
+
 static void check_setup(int argc, char **argv)
 {
 	int nr_tests = 0, len, i;
@@ -154,6 +161,11 @@ static bool check_svc(void)
 
 	return svc_works;
 }
+
+static void user_psci_system_off(struct pt_regs *regs)
+{
+	__user_psci_system_off();
+}
 #elif defined(__aarch64__)
 
 /*
@@ -270,12 +282,25 @@ static bool check_svc(void)
 
 	return svc_works;
 }
+
+static void user_psci_system_off(struct pt_regs *regs, unsigned int esr)
+{
+	__user_psci_system_off();
+}
 #endif
 
 static void check_vectors(void *arg __unused)
 {
 	report("und", check_und());
 	report("svc", check_svc());
+	if (is_user()) {
+#ifdef __arm__
+		install_exception_handler(EXCPTN_UND, user_psci_system_off);
+#else
+		install_exception_handler(EL0_SYNC_64, ESR_EL1_EC_UNKNOWN,
+					  user_psci_system_off);
+#endif
+	}
 	exit(report_summary());
 }
 
-- 
2.18.1

Re: [kvm-unit-tests PATCH v3 1/5] lib: arm: Use UART address from generated config.h

[Alexandru Elisei]

On 2/4/19 2:40 PM, Andrew Jones wrote:
> On Mon, Feb 04, 2019 at 02:17:05PM +0000, Alexandru Elisei wrote:
>> On 2/4/19 2:00 PM, Andrew Jones wrote:
>>> On Mon, Feb 04, 2019 at 01:44:08PM +0000, Alexandru Elisei wrote:
>>>> Generate lib/config.h when configuring kvm-unit-tests. The file is empty
>>>> for all architectures except for arm and arm64, where it is used to store
>>>> the UART base address. This removes the hardcoded address from lib/arm/io.c
>>>> and provides a mechanism for using different UART addresses in the future.
>>>>
>>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>>> ---
>>>>  configure    | 17 +++++++++++++++++
>>>>  Makefile     |  2 +-
>>>>  lib/arm/io.c | 12 ++++++------
>>>>  .gitignore   |  1 +
>>>>  4 files changed, 25 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/configure b/configure
>>>> index df8581e3a906..b4732c6c04e4 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -198,3 +198,20 @@ ENVIRON_DEFAULT=$environ_default
>>>>  ERRATATXT=errata.txt
>>>>  U32_LONG_FMT=$u32_long
>>>>  EOF
>>>> +
>>>> +cat <<EOF > lib/config.h
>>>> +#ifndef CONFIG_H
>>>> +#define CONFIG_H 1
>>>> +/*
>>>> + * Generated file. DO NOT MODIFY.
>>>> + *
>>>> + */
>>>> +EOF
>>>> +if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then
>>>> +cat <<EOF >> lib/config.h
>>>> +
>>>> +#define CONFIG_UART_EARLY_BASE 0x09000000
>>>> +
>>>> +EOF
>>>> +fi
>>>> +echo "#endif" >> lib/config.h
>>>> diff --git a/Makefile b/Makefile
>>>> index e9f02272e156..643af05678ad 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -115,7 +115,7 @@ libfdt_clean:
>>>>  	$(LIBFDT_objdir)/.*.d
>>>>  
>>>>  distclean: clean libfdt_clean
>>>> -	$(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head
>>>> +	$(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head
>>>>  	$(RM) -r tests logs logs.old
>>>>  
>>>>  cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic
>>>> diff --git a/lib/arm/io.c b/lib/arm/io.c
>>>> index d2c1a07c19ee..e84a5c89fcb1 100644
>>>> --- a/lib/arm/io.c
>>>> +++ b/lib/arm/io.c
>>>> @@ -11,6 +11,7 @@
>>>>  #include <libcflat.h>
>>>>  #include <devicetree.h>
>>>>  #include <chr-testdev.h>
>>>> +#include <config.h>
>>>>  #include <asm/spinlock.h>
>>>>  #include <asm/io.h>
>>>>  
>>>> @@ -18,6 +19,7 @@
>>>>  
>>>>  extern void halt(int code);
>>>>  
>>>> +static struct spinlock uart_lock;
>>>>  /*
>>>>   * Use this guess for the pl011 base in order to make an attempt at
>>>>   * having earlier printf support. We'll overwrite it with the real
>>>> @@ -25,10 +27,8 @@ extern void halt(int code);
>>>>   * the address we expect QEMU's mach-virt machine type to put in
>>>>   * its generated device tree.
>>>>   */
>>>> -#define UART_EARLY_BASE 0x09000000UL
>>>> -
>>>> -static struct spinlock uart_lock;
>>>> -static volatile u8 *uart0_base = (u8 *)UART_EARLY_BASE;
>>>> +#define UART_EARLY_BASE (u8 *)(unsigned long)CONFIG_UART_EARLY_BASE
>>>> +static volatile u8 *uart0_base = UART_EARLY_BASE;
>>>>  
>>>>  static void uart0_init(void)
>>>>  {
>>>> @@ -58,10 +58,10 @@ static void uart0_init(void)
>>>>  
>>>>  	uart0_base = ioremap(base.addr, base.size);
>>>>  
>>>> -	if (uart0_base != (u8 *)UART_EARLY_BASE) {
>>>> +	if (uart0_base != UART_EARLY_BASE) {
>>>>  		printf("WARNING: early print support may not work. "
>>>>  		       "Found uart at %p, but early base is %p.\n",
>>>> -			uart0_base, (u8 *)UART_EARLY_BASE);
>>>> +			uart0_base, UART_EARLY_BASE);
>>>>  	}
>>>>  }
>>>>  
>>>> diff --git a/.gitignore b/.gitignore
>>>> index 2405a8087ae5..483f7c7a09ea 100644
>>>> --- a/.gitignore
>>>> +++ b/.gitignore
>>>> @@ -10,6 +10,7 @@ patches
>>>>  cscope.*
>>>>  *.swp
>>>>  /lib/asm
>>>> +/lib/config.h
>>>>  /config.mak
>>>>  /*-run
>>>>  /msr.out
>>>> -- 
>>>> 2.17.0
>>>>
>>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>> Thank you for taking the time to review the patches!
>>
>> What is my next step?
>>
> Wait for Paolo or Radim to merge :-)  I've added them to CC.

Hi,

Any updates?

Thank you,

Alex

>
> drew

Re: [kvm-unit-tests PATCH v3 1/5] lib: arm: Use UART address from generated config.h

[Vladimir Murzin]

... snip

>>>
>>> What is my next step?
>>>
>> Wait for Paolo or Radim to merge :-)  I've added them to CC.
> 
> Hi,
> 
> Any updates?
> 

Pabl/Radim, these patches have been on a list for several weeks. It
looks to me that Alex addressed all points raised by Andrew.

Do you think they can be merged in foreseeable feature or you have
concerns about them? 


Cheers
Vladimir