qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG
@ 2025-10-02 10:16 Peter Maydell
  2025-10-02 10:16 ` [RFC PATCH 1/2] target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64 disabled Peter Maydell
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Peter Maydell @ 2025-10-02 10:16 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Clément Chigot

This patchset relaxes our current constraint that we only permit
-cpu foo,aarch64=off on KVM CPUs, so that you can also use this
to run a TCG CPU with aarch64 disabled. This is useful because
currently if you want a 32-bit TCG CPU you're limited to either
'max' in qemu-system-arm or else the old v7-only CPUs like a15.

I had a look at this last year, but never actually got the changes
into a completed state before I moved onto other things. Clément
asked about this the other day, so I figured I'd send out the
patches I had.

The series is RFC because:
 * I haven't tested it enough; in particular I don't think
   I checked that the "clear the AArch64 ID register values"
   patch doesn't break KVM aarch64=off (including not breaking
   migration). If it does we might have to make the "clear regs"
   only be done for TCG, but that seems a bit hacky...
 * I haven't checked that we forbid weird property combos like
   '-cpu max,aarch64=off,sve=on'

But I did do the work of looking through the codebase at where
we test ARM_FEATURE_AARCH64 to confirm that it really is the
right thing to test and we weren't using it any places where we
should instead have been checking ARM_FEATURE_V8 or something
else instead.

thanks
-- PMM

Peter Maydell (2):
  target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64
    disabled
  target/arm: Allow 'aarch64=off' to be set for TCG CPUs

 docs/system/arm/cpu-features.rst |  7 ++--
 target/arm/cpu-features.h        |  5 +++
 target/arm/cpu.h                 |  3 +-
 target/arm/cpu.c                 | 61 +++++++++++++++++++++++++++++---
 tests/qtest/arm-cpu-features.c   |  8 ++---
 5 files changed, 70 insertions(+), 14 deletions(-)

-- 
2.43.0



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

* [RFC PATCH 1/2] target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64 disabled
  2025-10-02 10:16 [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG Peter Maydell
@ 2025-10-02 10:16 ` Peter Maydell
  2025-10-10 15:44   ` Peter Maydell
  2025-10-02 10:16 ` [RFC PATCH 2/2] target/arm: Allow 'aarch64=off' to be set for TCG CPUs Peter Maydell
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2025-10-02 10:16 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Clément Chigot

If we create a normally-AArch64 CPU and configure it with
aarch64=off, this will by default leave all the AArch64 ID register
values in its ARMISARegisters struct untouched.  That in turn means
that tests of cpu_isar_feature(aa64_something, cpu) will return true.

Until now we have had a design policy that you shouldn't check an
aa64_ feature unless you know that the CPU has AArch64; but this is
quite fragile as it's easy to forget and only causes a problem in the
corner case where AArch64 was turned off.  In particular, when we
extend the ability to disable AArch64 from only KVM to also TCG there
are many more aa64 feature check points which we would otherwise have
to audit for whether they needed to be guarded with a check on
ARM_FEATURE_AARCH64.

Instead, make the CPU realize function zero out all the 64-bit ID
registers if the CPU doesn't have AArch64; this will make aa64_
feature tests generally return false.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h |  3 ++-
 target/arm/cpu.c | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 2b9585dc80a..b9f5d70ea35 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1056,7 +1056,8 @@ struct ArchCPU {
      * Note that if you add an ID register to the ARMISARegisters struct
      * you need to also update the 32-bit and 64-bit versions of the
      * kvm_arm_get_host_cpu_features() function to correctly populate the
-     * field by reading the value from the KVM vCPU.
+     * field by reading the value from the KVM vCPU. If it is an AArch64
+     * ID register then you also must update arm_clear_aarch64_idregs().
      */
     struct ARMISARegisters {
         uint32_t mvfr0;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 30e29fd3153..e0376b02119 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1592,6 +1592,27 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp)
     }
 }
 
+static void arm_clear_aarch64_idregs(ARMCPU *cpu)
+{
+    /* Zero out all the AArch64 ID registers in ARMISARegisters */
+    SET_IDREG(&cpu->isar, ID_AA64ISAR0, 0);
+    SET_IDREG(&cpu->isar, ID_AA64ISAR1, 0);
+    SET_IDREG(&cpu->isar, ID_AA64ISAR2, 0);
+    SET_IDREG(&cpu->isar, ID_AA64PFR0, 0);
+    SET_IDREG(&cpu->isar, ID_AA64PFR1, 0);
+    SET_IDREG(&cpu->isar, ID_AA64PFR2, 0);
+    SET_IDREG(&cpu->isar, ID_AA64MMFR0, 0);
+    SET_IDREG(&cpu->isar, ID_AA64MMFR1, 0);
+    SET_IDREG(&cpu->isar, ID_AA64MMFR2, 0);
+    SET_IDREG(&cpu->isar, ID_AA64MMFR3, 0);
+    SET_IDREG(&cpu->isar, ID_AA64DFR0, 0);
+    SET_IDREG(&cpu->isar, ID_AA64DFR1, 0);
+    SET_IDREG(&cpu->isar, ID_AA64AFR0, 0);
+    SET_IDREG(&cpu->isar, ID_AA64AFR1, 0);
+    SET_IDREG(&cpu->isar, ID_AA64ZFR0, 0);
+    SET_IDREG(&cpu->isar, ID_AA64SMFR0, 0);
+}
+
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -1718,6 +1739,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (!arm_feature(env, ARM_FEATURE_AARCH64)) {
+        arm_clear_aarch64_idregs(cpu);
+    }
+
 #ifdef CONFIG_USER_ONLY
     /*
      * User mode relies on IC IVAU instructions to catch modification of
-- 
2.43.0



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

* [RFC PATCH 2/2] target/arm: Allow 'aarch64=off' to be set for TCG CPUs
  2025-10-02 10:16 [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG Peter Maydell
  2025-10-02 10:16 ` [RFC PATCH 1/2] target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64 disabled Peter Maydell
@ 2025-10-02 10:16 ` Peter Maydell
  2025-10-03  9:28 ` [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG Clément Chigot
  2025-10-15 21:46 ` Richard Henderson
  3 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2025-10-02 10:16 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Clément Chigot

Allow the 'aarch64=off' property, which is currently KVM-only, to
be set for TCG CPUs also.

Note that we don't permit it on the qemu-aarch64 user-mode binary:
this makes no sense as that executable can only handle AArch64
syscalls (and it would also assert at startup since it doesn't
compile in the A32-specific GDB xml files like arm-neon.xml).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 docs/system/arm/cpu-features.rst |  7 ++++---
 target/arm/cpu-features.h        |  5 +++++
 target/arm/cpu.c                 | 36 ++++++++++++++++++++++++++++----
 tests/qtest/arm-cpu-features.c   |  8 ++-----
 4 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
index 37d5dfd15b3..7c339f1fb71 100644
--- a/docs/system/arm/cpu-features.rst
+++ b/docs/system/arm/cpu-features.rst
@@ -24,9 +24,10 @@ QEMU's support may be limited for some CPU features, only partially
 supporting the feature or only supporting the feature under certain
 configurations.  For example, the ``aarch64`` CPU feature, which, when
 disabled, enables the optional AArch32 CPU feature, is only supported
-when using the KVM accelerator and when running on a host CPU type that
-supports the feature.  While ``aarch64`` currently only works with KVM,
-it could work with TCG.  CPU features that are specific to KVM are
+on the TCG accelerator and when the KVM accelerator is running on a
+host CPU type that supports the feature.
+
+CPU features that are inherently specific to KVM are
 prefixed with "kvm-" and are described in "KVM VCPU Features".
 
 CPU Feature Probing
diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index 602f6a88e53..201fb93ac7e 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -1066,6 +1066,11 @@ static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
     return FIELD_EX64_IDREG(id, ID_AA64PFR0, EL2) >= 2;
 }
 
+static inline bool isar_feature_aa64_aa32_el3(const ARMISARegisters *id)
+{
+    return FIELD_EX64_IDREG(id, ID_AA64PFR0, EL3) >= 2;
+}
+
 static inline bool isar_feature_aa64_ras(const ARMISARegisters *id)
 {
     return FIELD_EX64_IDREG(id, ID_AA64PFR0, RAS) != 0;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index e0376b02119..79d71b2e8d2 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1220,10 +1220,38 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
      * uniform execution state like do_interrupt.
      */
     if (value == false) {
-        if (!kvm_enabled() || !kvm_arm_aarch32_supported()) {
-            error_setg(errp, "'aarch64' feature cannot be disabled "
-                             "unless KVM is enabled and 32-bit EL1 "
-                             "is supported");
+        if (kvm_enabled()) {
+            if (!kvm_arm_aarch32_supported()) {
+                error_setg(errp, "'aarch64' feature cannot be disabled for KVM "
+                           "because this host does not support 32-bit EL1");
+                return;
+            }
+        } else if (tcg_enabled()) {
+#ifdef CONFIG_USER_ONLY
+            error_setg(errp, "'aarch64' feature cannot be disabled for "
+                       "usermode emulator qemu-aarch64; use qemu-arm instead");
+            return;
+#else
+            bool aa32_at_highest_el;
+            if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
+                aa32_at_highest_el = cpu_isar_feature(aa64_aa32_el3, cpu);
+            } else if (arm_feature(&cpu->env, ARM_FEATURE_EL2)) {
+                aa32_at_highest_el = cpu_isar_feature(aa64_aa32_el2, cpu);
+            } else {
+                aa32_at_highest_el = cpu_isar_feature(aa64_aa32_el1, cpu);
+            }
+
+            if (!aa32_at_highest_el) {
+                error_setg(errp, "'aarch64' feature cannot be disabled for "
+                           "this TCG CPU because it does not support 32-bit "
+                           "execution at its highest implemented exception "
+                           "level");
+                return;
+            }
+#endif
+        } else {
+            error_setg(errp, "'aarch64' feature cannot be disabled for "
+                       "this accelerator");
             return;
         }
         unset_feature(&cpu->env, ARM_FEATURE_AARCH64);
diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index eb8ddebffbf..66009c3900a 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -493,12 +493,8 @@ static void test_query_cpu_model_expansion(const void *data)
         sve_tests_default(qts, "max");
         pauth_tests_default(qts, "max");
 
-        /* Test that features that depend on KVM generate errors without. */
-        assert_error(qts, "max",
-                     "'aarch64' feature cannot be disabled "
-                     "unless KVM is enabled and 32-bit EL1 "
-                     "is supported",
-                     "{ 'aarch64': false }");
+        /* TCG allows us to turn off AArch64 on the 'max' CPU type */
+        assert_set_feature(qts, "max", "aarch64", false);
     }
 
     qtest_quit(qts);
-- 
2.43.0



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

* Re: [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG
  2025-10-02 10:16 [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG Peter Maydell
  2025-10-02 10:16 ` [RFC PATCH 1/2] target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64 disabled Peter Maydell
  2025-10-02 10:16 ` [RFC PATCH 2/2] target/arm: Allow 'aarch64=off' to be set for TCG CPUs Peter Maydell
@ 2025-10-03  9:28 ` Clément Chigot
  2025-10-15 21:46 ` Richard Henderson
  3 siblings, 0 replies; 6+ messages in thread
From: Clément Chigot @ 2025-10-03  9:28 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-arm, qemu-devel

On Thu, Oct 2, 2025 at 12:16 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset relaxes our current constraint that we only permit
> -cpu foo,aarch64=off on KVM CPUs, so that you can also use this
> to run a TCG CPU with aarch64 disabled. This is useful because
> currently if you want a 32-bit TCG CPU you're limited to either
> 'max' in qemu-system-arm or else the old v7-only CPUs like a15.
>
> I had a look at this last year, but never actually got the changes
> into a completed state before I moved onto other things. Clément
> asked about this the other day, so I figured I'd send out the
> patches I had.

Thanks for the patches.
I've been able to test a couple of our internal testsuites over ARM
Linux (based on Yocto images) using both `cortex-a53,aarch64=off` and
`max,aarch64=off` on the virt machine. Everything went right including
the migration features we are using (basically, "migrate" to file and
later -incoming from that file). Hence,

Tested-by: Clément Chigot <chigot@adacore.com>

I'll let more relevant people do the review though.

Side note, IMO it would make sense to add another test under
function/arm/virt asserting this feature.

Thanks,
Clément

> The series is RFC because:
>  * I haven't tested it enough; in particular I don't think
>    I checked that the "clear the AArch64 ID register values"
>    patch doesn't break KVM aarch64=off (including not breaking
>    migration). If it does we might have to make the "clear regs"
>    only be done for TCG, but that seems a bit hacky...
>  * I haven't checked that we forbid weird property combos like
>    '-cpu max,aarch64=off,sve=on'
>
> But I did do the work of looking through the codebase at where
> we test ARM_FEATURE_AARCH64 to confirm that it really is the
> right thing to test and we weren't using it any places where we
> should instead have been checking ARM_FEATURE_V8 or something
> else instead.
>
> thanks
> -- PMM
>
> Peter Maydell (2):
>   target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64
>     disabled
>   target/arm: Allow 'aarch64=off' to be set for TCG CPUs
>
>  docs/system/arm/cpu-features.rst |  7 ++--
>  target/arm/cpu-features.h        |  5 +++
>  target/arm/cpu.h                 |  3 +-
>  target/arm/cpu.c                 | 61 +++++++++++++++++++++++++++++---
>  tests/qtest/arm-cpu-features.c   |  8 ++---
>  5 files changed, 70 insertions(+), 14 deletions(-)
>
> --
> 2.43.0
>


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

* Re: [RFC PATCH 1/2] target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64 disabled
  2025-10-02 10:16 ` [RFC PATCH 1/2] target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64 disabled Peter Maydell
@ 2025-10-10 15:44   ` Peter Maydell
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2025-10-10 15:44 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Clément Chigot

On Thu, 2 Oct 2025 at 11:16, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> If we create a normally-AArch64 CPU and configure it with
> aarch64=off, this will by default leave all the AArch64 ID register
> values in its ARMISARegisters struct untouched.  That in turn means
> that tests of cpu_isar_feature(aa64_something, cpu) will return true.
>
> Until now we have had a design policy that you shouldn't check an
> aa64_ feature unless you know that the CPU has AArch64; but this is
> quite fragile as it's easy to forget and only causes a problem in the
> corner case where AArch64 was turned off.  In particular, when we
> extend the ability to disable AArch64 from only KVM to also TCG there
> are many more aa64 feature check points which we would otherwise have
> to audit for whether they needed to be guarded with a check on
> ARM_FEATURE_AARCH64.
>
> Instead, make the CPU realize function zero out all the 64-bit ID
> registers if the CPU doesn't have AArch64; this will make aa64_
> feature tests generally return false.

I did some testing, and this doesn't break a KVM AArch32
vcpu, so in that sense it's OK, but it is a bit odd, because
from KVM's point of view the 64-bit ID registers still exist
and must have their correct values (especially, for VM
state load/save). This works because save/load goes through
the cpreg arrays, and doesn't affect cpu->isar. But it
means that QEMU has a different idea of the ID regs than
KVM does.

So I think the better approach here is that we do only
clear these ID registers for the TCG case. For TCG, an
aarch64=off CPU really does not have any aarch64 at all.
For the other accelerators (which is probably only ever
going to be KVM), an aarch64=off CPU is one which has
AArch64 at a higher EL, it's just the VM can never get
to that higher EL. That seems like enough of a difference
to justify treating them differently here (i.e. only
call arm_clear_aarch64_idregs() if tcg_enabled().)

thanks
-- PMM


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

* Re: [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG
  2025-10-02 10:16 [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG Peter Maydell
                   ` (2 preceding siblings ...)
  2025-10-03  9:28 ` [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG Clément Chigot
@ 2025-10-15 21:46 ` Richard Henderson
  3 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2025-10-15 21:46 UTC (permalink / raw)
  To: qemu-devel

On 10/2/25 03:16, Peter Maydell wrote:
> Peter Maydell (2):
>    target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64
>      disabled
>    target/arm: Allow 'aarch64=off' to be set for TCG CPUs

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

r~


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

end of thread, other threads:[~2025-10-15 21:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-02 10:16 [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG Peter Maydell
2025-10-02 10:16 ` [RFC PATCH 1/2] target/arm: Clear AArch64 ID regs from ARMISARegisters if AArch64 disabled Peter Maydell
2025-10-10 15:44   ` Peter Maydell
2025-10-02 10:16 ` [RFC PATCH 2/2] target/arm: Allow 'aarch64=off' to be set for TCG CPUs Peter Maydell
2025-10-03  9:28 ` [RFC PATCH 0/2] target/arm: Allow aarch64=off for TCG Clément Chigot
2025-10-15 21:46 ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).