[PATCH] target/arm: arm_pamax() no longer needs to do feature propagation

[PATCH] target/arm: arm_pamax() no longer needs to do feature propagation

[Peter Maydell]

In arm_pamax(), we need to cope with the virt board calling this
function on a CPU object which has been inited but not realize.
We used to do propagation of feature-flag implications (such as
"V7VE implies LPAE") at realize, so we have some code in arm_pamax()
which manually checks for both V7VE and LPAE feature flags.

In commit b8f7959f28c4f36 we moved the feature propagation for
almost all features from realize to post-init. That means that
now when the virt board calls arm_pamax(), the feature propagation
has been done. So we can drop the manual propagation handling
and check only for the feature we actually care about, which
is ARM_FEATURE_LPAE.

Retain the comment that the virt board is calling this function
with a not completely realized CPU object, because that is a
potential beartrap for later changes which is worth calling out.

(Note that b8f7959f28c4f36 actually fixed a bug in the arm_pamax()
handling: arm_pamax() was missing a check for ARM_FEATURE_V8, so it
incorrectly thought that the qemu-system-arm 'max' CPU did not have
LPAE and turned off 'highmem' support in the virt board.  Following
b8f7959f28c4f36 qemu-system-arm 'max' is treated the same as
'cortex-a15' and other v7 LPAE CPUs, because the generic feature
propagation code does correctly propagate V8 -> V7VE -> LPAE.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/ptw.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 1762b058aec..0da1caae117 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -95,7 +95,10 @@ static const uint8_t pamax_map[] = {
     [6] = 52,
 };
 
-/* The cpu-specific constant value of PAMax; also used by hw/arm/virt. */
+/*
+ * The cpu-specific constant value of PAMax; also used by hw/arm/virt.
+ * Note that machvirt_init calls this on a CPU that is inited but not realized!
+ */
 unsigned int arm_pamax(ARMCPU *cpu)
 {
     if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
@@ -110,13 +113,8 @@ unsigned int arm_pamax(ARMCPU *cpu)
         return pamax_map[parange];
     }
 
-    /*
-     * In machvirt_init, we call arm_pamax on a cpu that is not fully
-     * initialized, so we can't rely on the propagation done in realize.
-     */
-    if (arm_feature(&cpu->env, ARM_FEATURE_LPAE) ||
-        arm_feature(&cpu->env, ARM_FEATURE_V7VE)) {
-        /* v7 with LPAE */
+    if (arm_feature(&cpu->env, ARM_FEATURE_LPAE)) {
+        /* v7 or v8 with LPAE */
         return 40;
     }
     /* Anything else */
-- 
2.34.1

Re: [PATCH] target/arm: arm_pamax() no longer needs to do feature propagation

[Richard Henderson]

On 1/10/24 01:38, Peter Maydell wrote:
> In arm_pamax(), we need to cope with the virt board calling this
> function on a CPU object which has been inited but not realize.
> We used to do propagation of feature-flag implications (such as
> "V7VE implies LPAE") at realize, so we have some code in arm_pamax()
> which manually checks for both V7VE and LPAE feature flags.
> 
> In commit b8f7959f28c4f36 we moved the feature propagation for
> almost all features from realize to post-init. That means that
> now when the virt board calls arm_pamax(), the feature propagation
> has been done. So we can drop the manual propagation handling
> and check only for the feature we actually care about, which
> is ARM_FEATURE_LPAE.
> 
> Retain the comment that the virt board is calling this function
> with a not completely realized CPU object, because that is a
> potential beartrap for later changes which is worth calling out.
> 
> (Note that b8f7959f28c4f36 actually fixed a bug in the arm_pamax()
> handling: arm_pamax() was missing a check for ARM_FEATURE_V8, so it
> incorrectly thought that the qemu-system-arm 'max' CPU did not have
> LPAE and turned off 'highmem' support in the virt board.  Following
> b8f7959f28c4f36 qemu-system-arm 'max' is treated the same as
> 'cortex-a15' and other v7 LPAE CPUs, because the generic feature
> propagation code does correctly propagate V8 -> V7VE -> LPAE.)
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/ptw.c | 14 ++++++--------
>   1 file changed, 6 insertions(+), 8 deletions(-)

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

r~