qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-8.2 1/3] target/arm: Do all "ARM_FEATURE_X implies Y" che


From: Richard Henderson
Subject: Re: [PATCH for-8.2 1/3] target/arm: Do all "ARM_FEATURE_X implies Y" checks in post_init
Date: Tue, 25 Jul 2023 16:32:26 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0

On 7/24/23 10:43, Peter Maydell wrote:
Where architecturally one ARM_FEATURE_X flag implies another
ARM_FEATURE_Y, we allow the CPU init function to only set X, and then
set Y for it.  Currently we do this in two places -- we set a few
flags in arm_cpu_post_init() because we need them to decide which
properties to create on the CPU object, and then we do the rest in
arm_cpu_realizefn().  However, this is fragile, because it's easy to
add a new property and not notice that this means that an X-implies-Y
check now has to move from realize to post-init.

As a specific example, the pmsav7-dregion property is conditional
on ARM_FEATURE_PMSA && ARM_FEATURE_V7, which means it won't appear
on the Cortex-M33 and -M55, because they set ARM_FEATURE_V8 and
rely on V8-implies-V7, which doesn't happen until the realizefn.

Move all of these X-implies-Y checks into a new function, which
we call at the top of arm_cpu_post_init(), so the feature bits
are available at that point.

This does now give us the reverse issue, that if there's a feature
bit which is enabled or disabled by the setting of a property then
then X-implies-Y features that are dependent on that property need to
be in realize, not in this new function.  But the only one of those
is the "EL3 implies VBAR" which is already in the right place, so
putting things this way round seems better to me.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
  target/arm/cpu.c | 176 +++++++++++++++++++++++++----------------------
  1 file changed, 94 insertions(+), 82 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 69e2bde3c2d..58301c4b7d8 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1356,17 +1356,105 @@ unsigned int gt_cntfrq_period_ns(ARMCPU *cpu)
        NANOSECONDS_PER_SECOND / cpu->gt_cntfrq_hz : 1;
  }
+static void arm_cpu_propagate_feature_implications(ARMCPU *cpu)
+{
+    CPUARMState *env = &cpu->env;
+    bool no_aa32 = false;
+    /*
+     * Some features automatically imply others: set the feature

Spacing after local vars.

+    if (arm_feature(env, ARM_FEATURE_V7VE)) {
+        /* v7 Virtualization Extensions. In real hardware this implies

Should fix the comment formatting.

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

I thought I had tried this myself at some point, and ran into a problem. But I can't recall the specifics now.


r~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]