All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] accel: Introduce current_accel_name()
@ 2022-06-20 19:22 Alexander Graf
  2022-06-20 19:22 ` [PATCH v2 2/2] target/arm: Catch invalid kvm state also for hvf Alexander Graf
  2022-06-20 19:46 ` [PATCH v2 1/2] accel: Introduce current_accel_name() Richard Henderson
  0 siblings, 2 replies; 6+ messages in thread
From: Alexander Graf @ 2022-06-20 19:22 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Richard Henderson

We need to fetch the name of the current accelerator in flexible error
messages more going forward. Let's create a helper that gives it to us
without casting in the target code.

Signed-off-by: Alexander Graf <agraf@csgraf.de>
---
 accel/accel-common.c | 8 ++++++++
 include/qemu/accel.h | 1 +
 softmmu/vl.c         | 3 +--
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/accel/accel-common.c b/accel/accel-common.c
index 7b8ec7e0f7..50035bda55 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -49,6 +49,14 @@ AccelClass *accel_find(const char *opt_name)
     return ac;
 }
 
+/* Return the name of the current accelerator */
+const char *current_accel_name(void)
+{
+    AccelClass *ac = ACCEL_GET_CLASS(current_accel());
+
+    return ac->name;
+}
+
 static void accel_init_cpu_int_aux(ObjectClass *klass, void *opaque)
 {
     CPUClass *cc = CPU_CLASS(klass);
diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index 4f4c283f6f..be56da1b99 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -68,6 +68,7 @@ typedef struct AccelClass {
 
 AccelClass *accel_find(const char *opt_name);
 AccelState *current_accel(void);
+const char *current_accel_name(void);
 
 void accel_init_interfaces(AccelClass *ac);
 
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 54e920ada1..3dca5936c7 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2271,8 +2271,7 @@ static void configure_accelerators(const char *progname)
     }
 
     if (init_failed && !qtest_chrdev) {
-        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
-        error_report("falling back to %s", ac->name);
+        error_report("falling back to %s", current_accel_name());
     }
 
     if (icount_enabled() && !tcg_enabled()) {
-- 
2.32.1 (Apple Git-133)



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] target/arm: Catch invalid kvm state also for hvf
  2022-06-20 19:22 [PATCH v2 1/2] accel: Introduce current_accel_name() Alexander Graf
@ 2022-06-20 19:22 ` Alexander Graf
  2022-06-20 19:46   ` Richard Henderson
  2022-06-21 11:41   ` Peter Maydell
  2022-06-20 19:46 ` [PATCH v2 1/2] accel: Introduce current_accel_name() Richard Henderson
  1 sibling, 2 replies; 6+ messages in thread
From: Alexander Graf @ 2022-06-20 19:22 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Richard Henderson

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(-)

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;
         }
     }
-- 
2.32.1 (Apple Git-133)



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] accel: Introduce current_accel_name()
  2022-06-20 19:22 [PATCH v2 1/2] accel: Introduce current_accel_name() Alexander Graf
  2022-06-20 19:22 ` [PATCH v2 2/2] target/arm: Catch invalid kvm state also for hvf Alexander Graf
@ 2022-06-20 19:46 ` Richard Henderson
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2022-06-20 19:46 UTC (permalink / raw)
  To: Alexander Graf, Peter Maydell; +Cc: qemu-arm, qemu-devel

On 6/20/22 12:22, Alexander Graf wrote:
> We need to fetch the name of the current accelerator in flexible error
> messages more going forward. Let's create a helper that gives it to us
> without casting in the target code.
> 
> Signed-off-by: Alexander Graf<agraf@csgraf.de>
> ---
>   accel/accel-common.c | 8 ++++++++
>   include/qemu/accel.h | 1 +
>   softmmu/vl.c         | 3 +--
>   3 files changed, 10 insertions(+), 2 deletions(-)

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

r~


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] target/arm: Catch invalid kvm state also for hvf
  2022-06-20 19:22 ` [PATCH v2 2/2] target/arm: Catch invalid kvm state also for hvf Alexander Graf
@ 2022-06-20 19:46   ` Richard Henderson
  2022-06-21 11:41   ` Peter Maydell
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2022-06-20 19:46 UTC (permalink / raw)
  To: Alexander Graf, Peter Maydell; +Cc: qemu-arm, qemu-devel

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~


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] target/arm: Catch invalid kvm state also for hvf
  2022-06-20 19:22 ` [PATCH v2 2/2] target/arm: Catch invalid kvm state also for hvf Alexander Graf
  2022-06-20 19:46   ` Richard Henderson
@ 2022-06-21 11:41   ` Peter Maydell
  2022-06-21 17:34     ` Peter Maydell
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2022-06-21 11:41 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-arm, qemu-devel, Richard Henderson

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] target/arm: Catch invalid kvm state also for hvf
  2022-06-21 11:41   ` Peter Maydell
@ 2022-06-21 17:34     ` Peter Maydell
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2022-06-21 17:34 UTC (permalink / raw)
  To: Alexander Graf; +Cc: qemu-arm, qemu-devel, Richard Henderson

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-06-21 17:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-20 19:22 [PATCH v2 1/2] accel: Introduce current_accel_name() Alexander Graf
2022-06-20 19:22 ` [PATCH v2 2/2] target/arm: Catch invalid kvm state also for hvf Alexander Graf
2022-06-20 19:46   ` Richard Henderson
2022-06-21 11:41   ` Peter Maydell
2022-06-21 17:34     ` Peter Maydell
2022-06-20 19:46 ` [PATCH v2 1/2] accel: Introduce current_accel_name() Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.