[PATCH 0/3] arm: Avoid asserting in cpu_address_space_init()

[PATCH 0/3] arm: Avoid asserting in cpu_address_space_init()

[Peter Maydell]

KVM can't support more than one address space per CPU; if you try to
create more than one then cpu_address_space_init() will assert.

The Arm CPU realize function wasn't checking for the combination of
KVM and various features that might need multiple address spaces, so
it would just blunder on and hit the assertion failure for command
lines like:
 qemu-system-aarch64  -enable-kvm -display none -cpu max -machine raspi3b
 qemu-system-aarch64  -enable-kvm -display none -machine mps3-an524

This is https://gitlab.com/qemu-project/qemu/-/issues/528

This series adds the missing checks.  It also fixes the raspi board
code to not assert when CPU creation fails, and removes a
now-duplicate check from the virt board code.

thanks
-- PMM

Peter Maydell (3):
  raspi: Use error_fatal for SoC realize errors, not error_abort
  target/arm: Avoid assertion trying to use KVM and multiple ASes
  hw/arm/virt: Delete EL3 error checksnow provided in CPU realize

 hw/arm/raspi.c   |  2 +-
 hw/arm/virt.c    |  5 -----
 target/arm/cpu.c | 23 +++++++++++++++++++++++
 3 files changed, 24 insertions(+), 6 deletions(-)

-- 
2.20.1

[PATCH 1/3] raspi: Use error_fatal for SoC realize errors, not error_abort

[Peter Maydell]

The SoC realize can fail for legitimate reasons, because it propagates
errors up from CPU realize, which in turn can be provoked by user
error in setting commandline options. Use error_fatal so we report
the error message to the user and exit, rather than asserting
via error_abort.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/raspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index b30a17871f7..0ada91c05e9 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -281,7 +281,7 @@ static void raspi_machine_init(MachineState *machine)
     object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(machine->ram));
     object_property_set_int(OBJECT(&s->soc), "board-rev", board_rev,
                             &error_abort);
-    qdev_realize(DEVICE(&s->soc), NULL, &error_abort);
+    qdev_realize(DEVICE(&s->soc), NULL, &error_fatal);
 
     /* Create and plug in the SD cards */
     di = drive_get_next(IF_SD);
-- 
2.20.1

Re: [PATCH 1/3] raspi: Use error_fatal for SoC realize errors, not error_abort

[Philippe Mathieu-Daudé]

On 8/16/21 3:58 PM, Peter Maydell wrote:
> The SoC realize can fail for legitimate reasons, because it propagates
> errors up from CPU realize, which in turn can be provoked by user
> error in setting commandline options. Use error_fatal so we report
> the error message to the user and exit, rather than asserting
> via error_abort.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/raspi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[PATCH 2/3] target/arm: Avoid assertion trying to use KVM and multiple ASes

[Peter Maydell]

KVM cannot support multiple address spaces per CPU; if you try to
create more than one then cpu_address_space_init() will assert.

In the Arm CPU realize function, detect the configurations which
would cause us to need more than one AS, and cleanly fail the
realize rather than blundering on into the assertion. This
turns this:
  $ qemu-system-aarch64  -enable-kvm -display none -cpu max -machine raspi3b
  qemu-system-aarch64: ../../softmmu/physmem.c:747: cpu_address_space_init: Assertion `asidx == 0 || !kvm_enabled()' failed.
  Aborted

into:
  $ qemu-system-aarch64  -enable-kvm -display none -machine raspi3b
  qemu-system-aarch64: Cannot enable KVM when guest CPU has EL3 enabled

and this:
  $ qemu-system-aarch64  -enable-kvm -display none -machine mps3-an524
  qemu-system-aarch64: ../../softmmu/physmem.c:747: cpu_address_space_init: Assertion `asidx == 0 || !kvm_enabled()' failed.
  Aborted

into:
  $ qemu-system-aarch64  -enable-kvm -display none -machine mps3-an524
  qemu-system-aarch64: Cannot enable KVM when using an M-profile guest CPU

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/528
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 2866dd76588..4377f3211c8 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1419,6 +1419,29 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         }
     }
 
+    if (kvm_enabled()) {
+        /*
+         * Catch all the cases which might cause us to create more than one
+         * address space for the CPU (otherwise we will assert() later in
+         * cpu_address_space_init()).
+         */
+        if (arm_feature(env, ARM_FEATURE_M)) {
+            error_setg(errp,
+                       "Cannot enable KVM when using an M-profile guest CPU");
+            return;
+        }
+        if (cpu->has_el3) {
+            error_setg(errp,
+                       "Cannot enable KVM when guest CPU has EL3 enabled");
+            return;
+        }
+        if (cpu->tag_memory) {
+            error_setg(errp,
+                       "Cannot enable KVM when guest CPUs has MTE enabled");
+            return;
+        }
+    }
+
     {
         uint64_t scale;
 
-- 
2.20.1

Re: [PATCH 2/3] target/arm: Avoid assertion trying to use KVM and multiple ASes

[Philippe Mathieu-Daudé]

On 8/16/21 3:58 PM, Peter Maydell wrote:
> KVM cannot support multiple address spaces per CPU; if you try to
> create more than one then cpu_address_space_init() will assert.
> 
> In the Arm CPU realize function, detect the configurations which
> would cause us to need more than one AS, and cleanly fail the
> realize rather than blundering on into the assertion. This
> turns this:
>   $ qemu-system-aarch64  -enable-kvm -display none -cpu max -machine raspi3b
>   qemu-system-aarch64: ../../softmmu/physmem.c:747: cpu_address_space_init: Assertion `asidx == 0 || !kvm_enabled()' failed.
>   Aborted
> 
> into:
>   $ qemu-system-aarch64  -enable-kvm -display none -machine raspi3b
>   qemu-system-aarch64: Cannot enable KVM when guest CPU has EL3 enabled
> 
> and this:
>   $ qemu-system-aarch64  -enable-kvm -display none -machine mps3-an524
>   qemu-system-aarch64: ../../softmmu/physmem.c:747: cpu_address_space_init: Assertion `asidx == 0 || !kvm_enabled()' failed.
>   Aborted
> 
> into:
>   $ qemu-system-aarch64  -enable-kvm -display none -machine mps3-an524
>   qemu-system-aarch64: Cannot enable KVM when using an M-profile guest CPU
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/528
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/cpu.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[PATCH 3/3] hw/arm/virt: Delete EL3 error checksnow provided in CPU realize

[Peter Maydell]

Now that the CPU realize function will fail cleanly if we ask for EL3
when KVM is enabled, we don't need to check for errors explicitly in
the virt board code. The reported message is slightly different;
it is now:
  qemu-system-aarch64: Cannot enable KVM when guest CPU has EL3 enabled
instead of:
  qemu-system-aarch64: mach-virt: KVM does not support Security extensions

We don't delete the MTE check because there the logic is more
complex; deleting the check would work but makes the error message
less helpful, as it would read:
  qemu-system-aarch64: MTE requested, but not supported by the guest CPU
instead of:
  qemu-system-aarch64: mach-virt: KVM does not support providing MTE to the guest CPU

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 81eda46b0bb..86c8a4ca3d7 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1852,11 +1852,6 @@ static void machvirt_init(MachineState *machine)
     }
 
     if (vms->secure) {
-        if (kvm_enabled()) {
-            error_report("mach-virt: KVM does not support Security extensions");
-            exit(1);
-        }
-
         /*
          * The Secure view of the world is the same as the NonSecure,
          * but with a few extra devices. Create it as a container region
-- 
2.20.1

Re: [PATCH 3/3] hw/arm/virt: Delete EL3 error checksnow provided in CPU realize

[Philippe Mathieu-Daudé]

On 8/16/21 3:58 PM, Peter Maydell wrote:
> Now that the CPU realize function will fail cleanly if we ask for EL3
> when KVM is enabled, we don't need to check for errors explicitly in
> the virt board code. The reported message is slightly different;
> it is now:
>   qemu-system-aarch64: Cannot enable KVM when guest CPU has EL3 enabled
> instead of:
>   qemu-system-aarch64: mach-virt: KVM does not support Security extensions
> 
> We don't delete the MTE check because there the logic is more
> complex; deleting the check would work but makes the error message
> less helpful, as it would read:
>   qemu-system-aarch64: MTE requested, but not supported by the guest CPU
> instead of:
>   qemu-system-aarch64: mach-virt: KVM does not support providing MTE to the guest CPU
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/virt.c | 5 -----
>  1 file changed, 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 0/3] arm: Avoid asserting in cpu_address_space_init()

[Richard Henderson]

On 8/16/21 3:58 AM, Peter Maydell wrote:
> Peter Maydell (3):
>    raspi: Use error_fatal for SoC realize errors, not error_abort
>    target/arm: Avoid assertion trying to use KVM and multiple ASes
>    hw/arm/virt: Delete EL3 error checksnow provided in CPU realize

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~