1. Patchew
  2. QEMU
  3. View series

[PATCH-for-8.2] target/arm/cpu: Allow logging disabled CPU features at UNIMP level

Philippe Mathieu-Daudé posted 1 patch 9 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230810161329.7453-1-philmd@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/cpu.c | 60 ++++++++++++++++++++++++++++++++----------------
1 file changed, 40 insertions(+), 20 deletions(-)
[PATCH-for-8.2] target/arm/cpu: Allow logging disabled CPU features at UNIMP level
Posted by Philippe Mathieu-Daudé 9 months, 2 weeks ago 
Some CPU features aren't strictly required to run guest code (such
debugging features), so we don't model them. To prevent the guest
to access non-existent system registers, we disable the feature bit
in the ID registers (see commit 7d8c283e10 "target/arm: Suppress more
TCG unimplemented features in ID registers").

Since it might be useful to know when a CPU supposed to implement a
feature has it disabled in QEMU, provide the ability to log disabled
features at the UNIMP level:

  $ qemu-system-aarch64 -M virt -d unimp -S -cpu neoverse-v1
  QEMU 8.0.92 monitor - type 'help' for more information
  CPU#0: Disabling unimplemented feature 'FEAT_SPE (Statistical Profiling Extension)' for neoverse-v1
  CPU#0: Disabling unimplemented feature 'FEAT_TRF (Self-hosted Trace Extension)' for neoverse-v1
  CPU#0: Disabling unimplemented feature 'FEAT_TRF (Self-hosted Trace Extension)' for neoverse-v1
  CPU#0: Disabling unimplemented feature 'Trace Macrocell system register access' for neoverse-v1
  CPU#0: Disabling unimplemented feature 'Trace Macrocell system register access' for neoverse-v1
  CPU#0: Disabling unimplemented feature 'Memory-mapped Trace' for neoverse-v1
  CPU#0: Disabling unimplemented feature 'FEAT_AMU (Activity Monitors Extension)' for neoverse-v1
  CPU#0: Disabling unimplemented feature 'FEAT_AMU (Activity Monitors Extension)' for neoverse-v1
  CPU#0: Disabling unimplemented feature 'FEAT_MPAM (Memory Partitioning and Monitoring Extension)' for neoverse-v1
  CPU#0: Disabling unimplemented feature 'FEAT_NV (Nested Virtualization)' for neoverse-v1
  (qemu) q

  $ qemu-system-aarch64 -M xlnx-zcu102 -trace arm_disable\* -S -d unimp -monitor stdio -cpu cortex-a76
  QEMU 8.0.92 monitor - type 'help' for more information
  CPU#0: Disabling unimplemented feature 'Memory-mapped Trace' for cortex-a53
  CPU#1: Disabling unimplemented feature 'Memory-mapped Trace' for cortex-a53
  CPU#2: Disabling unimplemented feature 'Memory-mapped Trace' for cortex-a53
  CPU#3: Disabling unimplemented feature 'Memory-mapped Trace' for cortex-a53
  (qemu) q

Refactor the FIELD_DPxx() calls into the DISABLE_UNIMP_CPU_FEATURE()
macro where we call qemu_log(UNIMP).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
There is another series reworking cpu_typename() helpers, so
meanwhile I simply added a static arm_cpu_typename() here.
---
 target/arm/cpu.c | 60 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 93c28d50e5..becd2d2690 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1580,6 +1580,24 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp)
     }
 }
 
+/* callers must free the returned string with g_free() */
+static char *arm_cpu_typename(ARMCPU *cpu)
+{
+    const char *cpu_type = object_get_typename(OBJECT(cpu));
+
+    return g_strndup(cpu_type, strlen(cpu_type) - strlen(ARM_CPU_TYPE_SUFFIX));
+}
+
+#define DISABLE_UNIMP_CPU_FEATURE(size, cpu, storage, reg, field, desc) \
+    if (FIELD_EX##size(cpu->isar.storage, reg, field)) { \
+        g_autofree char *cpu_type = arm_cpu_typename(cpu); \
+        \
+        qemu_log_mask(LOG_UNIMP, \
+                      "CPU#%d: Disabling unimplemented feature '%s' for %s\n", \
+                      CPU(cpu)->cpu_index, desc, cpu_type); \
+        cpu->isar.storage = FIELD_DP##size(cpu->isar.storage, reg, field, 0); \
+    }
+
 static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -2075,32 +2093,34 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
          * try to access the non-existent system registers for them.
          */
         /* FEAT_SPE (Statistical Profiling Extension) */
-        cpu->isar.id_aa64dfr0 =
-            FIELD_DP64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, PMSVER, 0);
+        DISABLE_UNIMP_CPU_FEATURE(64, cpu, id_aa64dfr0, ID_AA64DFR0, PMSVER,
+                                  "FEAT_SPE (Statistical Profiling Extension)");
+
         /* FEAT_TRF (Self-hosted Trace Extension) */
-        cpu->isar.id_aa64dfr0 =
-            FIELD_DP64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, TRACEFILT, 0);
-        cpu->isar.id_dfr0 =
-            FIELD_DP32(cpu->isar.id_dfr0, ID_DFR0, TRACEFILT, 0);
+        DISABLE_UNIMP_CPU_FEATURE(64, cpu, id_aa64dfr0, ID_AA64DFR0, TRACEFILT,
+                                  "FEAT_TRF (Self-hosted Trace Extension)");
+        DISABLE_UNIMP_CPU_FEATURE(32, cpu, id_dfr0, ID_DFR0, TRACEFILT,
+                                  "FEAT_TRF (Self-hosted Trace Extension)");
         /* Trace Macrocell system register access */
-        cpu->isar.id_aa64dfr0 =
-            FIELD_DP64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, TRACEVER, 0);
-        cpu->isar.id_dfr0 =
-            FIELD_DP32(cpu->isar.id_dfr0, ID_DFR0, COPTRC, 0);
+        DISABLE_UNIMP_CPU_FEATURE(64, cpu, id_aa64dfr0, ID_AA64DFR0, TRACEVER,
+                                  "Trace Macrocell system register access");
+        DISABLE_UNIMP_CPU_FEATURE(32, cpu, id_dfr0, ID_DFR0, COPTRC,
+                                  "Trace Macrocell system register access");
         /* Memory mapped trace */
-        cpu->isar.id_dfr0 =
-            FIELD_DP32(cpu->isar.id_dfr0, ID_DFR0, MMAPTRC, 0);
+        DISABLE_UNIMP_CPU_FEATURE(32, cpu, id_dfr0, ID_DFR0, MMAPTRC,
+                                  "Memory-mapped Trace");
         /* FEAT_AMU (Activity Monitors Extension) */
-        cpu->isar.id_aa64pfr0 =
-            FIELD_DP64(cpu->isar.id_aa64pfr0, ID_AA64PFR0, AMU, 0);
-        cpu->isar.id_pfr0 =
-            FIELD_DP32(cpu->isar.id_pfr0, ID_PFR0, AMU, 0);
+        DISABLE_UNIMP_CPU_FEATURE(64, cpu, id_aa64pfr0, ID_AA64PFR0, AMU,
+                                  "FEAT_AMU (Activity Monitors Extension)");
+        DISABLE_UNIMP_CPU_FEATURE(32, cpu, id_pfr0, ID_PFR0, AMU,
+                                  "FEAT_AMU (Activity Monitors Extension)");
         /* FEAT_MPAM (Memory Partitioning and Monitoring Extension) */
-        cpu->isar.id_aa64pfr0 =
-            FIELD_DP64(cpu->isar.id_aa64pfr0, ID_AA64PFR0, MPAM, 0);
+        DISABLE_UNIMP_CPU_FEATURE(64, cpu, id_aa64pfr0, ID_AA64PFR0, MPAM,
+                                  "FEAT_MPAM (Memory Partitioning"
+                                  " and Monitoring Extension)");
         /* FEAT_NV (Nested Virtualization) */
-        cpu->isar.id_aa64mmfr2 =
-            FIELD_DP64(cpu->isar.id_aa64mmfr2, ID_AA64MMFR2, NV, 0);
+        DISABLE_UNIMP_CPU_FEATURE(64, cpu, id_aa64mmfr2, ID_AA64MMFR2, NV,
+                                  "FEAT_NV (Nested Virtualization)");
     }
 
     /* MPU can be configured out of a PMSA CPU either by setting has-mpu
-- 
2.38.1
Re: [PATCH-for-8.2] target/arm/cpu: Allow logging disabled CPU features at UNIMP level
Posted by Peter Maydell 9 months, 2 weeks ago 
On Thu, 10 Aug 2023 at 17:13, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Some CPU features aren't strictly required to run guest code (such
> debugging features), so we don't model them. To prevent the guest
> to access non-existent system registers, we disable the feature bit
> in the ID registers (see commit 7d8c283e10 "target/arm: Suppress more
> TCG unimplemented features in ID registers").
>
> Since it might be useful to know when a CPU supposed to implement a
> feature has it disabled in QEMU, provide the ability to log disabled
> features at the UNIMP level:
>
>   $ qemu-system-aarch64 -M virt -d unimp -S -cpu neoverse-v1
>   QEMU 8.0.92 monitor - type 'help' for more information
>   CPU#0: Disabling unimplemented feature 'FEAT_SPE (Statistical Profiling Extension)' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'FEAT_TRF (Self-hosted Trace Extension)' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'FEAT_TRF (Self-hosted Trace Extension)' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'Trace Macrocell system register access' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'Trace Macrocell system register access' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'Memory-mapped Trace' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'FEAT_AMU (Activity Monitors Extension)' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'FEAT_AMU (Activity Monitors Extension)' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'FEAT_MPAM (Memory Partitioning and Monitoring Extension)' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'FEAT_NV (Nested Virtualization)' for neoverse-v1
>   (qemu) q
>
>   $ qemu-system-aarch64 -M xlnx-zcu102 -trace arm_disable\* -S -d unimp -monitor stdio -cpu cortex-a76
>   QEMU 8.0.92 monitor - type 'help' for more information
>   CPU#0: Disabling unimplemented feature 'Memory-mapped Trace' for cortex-a53
>   CPU#1: Disabling unimplemented feature 'Memory-mapped Trace' for cortex-a53
>   CPU#2: Disabling unimplemented feature 'Memory-mapped Trace' for cortex-a53
>   CPU#3: Disabling unimplemented feature 'Memory-mapped Trace' for cortex-a53
>   (qemu) q

Generally the 'unimp' logging is "the guest tried to use some
feature we don't implement". Here we log about the fact we
don't implement the feature, even if the guest never tries
to use the feature at all...

thanks
-- PMM
Re: [PATCH-for-8.2] target/arm/cpu: Allow logging disabled CPU features at UNIMP level
Posted by Alex Bennée 9 months, 2 weeks ago 
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Some CPU features aren't strictly required to run guest code (such
> debugging features), so we don't model them. To prevent the guest
> to access non-existent system registers, we disable the feature bit
> in the ID registers (see commit 7d8c283e10 "target/arm: Suppress more
> TCG unimplemented features in ID registers").
>
> Since it might be useful to know when a CPU supposed to implement a
> feature has it disabled in QEMU, provide the ability to log disabled
> features at the UNIMP level:
>
>   $ qemu-system-aarch64 -M virt -d unimp -S -cpu neoverse-v1
>   QEMU 8.0.92 monitor - type 'help' for more information
>   CPU#0: Disabling unimplemented feature 'FEAT_SPE (Statistical Profiling Extension)' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'FEAT_TRF (Self-hosted Trace Extension)' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'FEAT_TRF (Self-hosted Trace Extension)' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'Trace Macrocell system register access' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'Trace Macrocell system register access' for neoverse-v1
>   CPU#0: Disabling unimplemented feature 'Memory-mapped Trace' for
>   neoverse-v1

Don't these also depend on FEAT_?

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.