diff mbox series

[v2,2/2] target/arm: Catch invalid kvm state also for hvf

Message ID 20220620192242.70573-2-agraf@csgraf.de
State New
Headers show
Series [v2,1/2] accel: Introduce current_accel_name() | expand

Commit Message

Alexander Graf June 20, 2022, 7:22 p.m. UTC
Some features such as running in EL3 or running M profile code are
incompatible with virtualization as QEMU implements it today. To prevent
users from picking invalid configurations on other virt solutions like
Hvf, let's run the same checks there too.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1073
Signed-off-by: Alexander Graf <agraf@csgraf.de>

---

v1 -> v2:

  - Use current_accel_name()
  - Use !tcg_enabled()
---
 target/arm/cpu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Richard Henderson June 20, 2022, 7:46 p.m. UTC | #1
On 6/20/22 12:22, Alexander Graf wrote:
> Some features such as running in EL3 or running M profile code are
> incompatible with virtualization as QEMU implements it today. To prevent
> users from picking invalid configurations on other virt solutions like
> Hvf, let's run the same checks there too.
> 
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1073
> Signed-off-by: Alexander Graf<agraf@csgraf.de>
> 
> ---
> 
> v1 -> v2:
> 
>    - Use current_accel_name()
>    - Use !tcg_enabled()
> ---
>   target/arm/cpu.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)

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

r~
Peter Maydell June 21, 2022, 11:41 a.m. UTC | #2
On Mon, 20 Jun 2022 at 20:22, Alexander Graf <agraf@csgraf.de> wrote:
>
> Some features such as running in EL3 or running M profile code are
> incompatible with virtualization as QEMU implements it today. To prevent
> users from picking invalid configurations on other virt solutions like
> Hvf, let's run the same checks there too.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1073
> Signed-off-by: Alexander Graf <agraf@csgraf.de>


> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1490,7 +1490,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>          }
>      }
>
> -    if (kvm_enabled()) {
> +    if (!tcg_enabled()) {

I'm a bit surprised we don't need to also have "&& !qtest_enabled()",
but I guess if "make check" works then we're fine :-)

-- PMM
Peter Maydell June 21, 2022, 5:34 p.m. UTC | #3
On Tue, 21 Jun 2022 at 12:41, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 20 Jun 2022 at 20:22, Alexander Graf <agraf@csgraf.de> wrote:
> >
> > Some features such as running in EL3 or running M profile code are
> > incompatible with virtualization as QEMU implements it today. To prevent
> > users from picking invalid configurations on other virt solutions like
> > Hvf, let's run the same checks there too.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1073
> > Signed-off-by: Alexander Graf <agraf@csgraf.de>
>
>
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1490,7 +1490,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> >          }
> >      }
> >
> > -    if (kvm_enabled()) {
> > +    if (!tcg_enabled()) {
>
> I'm a bit surprised we don't need to also have "&& !qtest_enabled()",
> but I guess if "make check" works then we're fine :-)

In fact you do need to handle qtest here too, otherwise
lots of tests in 'make check' barf on the unexpected error...

I'm going to squash the following fix in and take the patchset into
target-arm.next. (PS: you forgot the cover letter, I think ?)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 0862dcd63cb..d9c4a9f56d2 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -39,6 +39,7 @@
 #include "hw/boards.h"
 #endif
 #include "sysemu/tcg.h"
+#include "sysemu/qtest.h"
 #include "sysemu/hw_accel.h"
 #include "kvm_arm.h"
 #include "disas/capstone.h"
@@ -1490,8 +1491,12 @@ static void arm_cpu_realizefn(DeviceState *dev,
Error **errp)
         }
     }

-    if (!tcg_enabled()) {
+    if (!tcg_enabled() && !qtest_enabled()) {
         /*
+         * We assume that no accelerator except TCG (and the "not really an
+         * accelerator" qtest) can handle these features, because Arm hardware
+         * virtualization can't virtualize them.
+         *
          * 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()).


thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 1b5d535788..0862dcd63c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1490,7 +1490,7 @@  static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         }
     }
 
-    if (kvm_enabled()) {
+    if (!tcg_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
@@ -1498,17 +1498,20 @@  static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
          */
         if (arm_feature(env, ARM_FEATURE_M)) {
             error_setg(errp,
-                       "Cannot enable KVM when using an M-profile guest CPU");
+                       "Cannot enable %s when using an M-profile guest CPU",
+                       current_accel_name());
             return;
         }
         if (cpu->has_el3) {
             error_setg(errp,
-                       "Cannot enable KVM when guest CPU has EL3 enabled");
+                       "Cannot enable %s when guest CPU has EL3 enabled",
+                       current_accel_name());
             return;
         }
         if (cpu->tag_memory) {
             error_setg(errp,
-                       "Cannot enable KVM when guest CPUs has MTE enabled");
+                       "Cannot enable %s when guest CPUs has MTE enabled",
+                       current_accel_name());
             return;
         }
     }