target/arm: Don't set HCR.RW for AArch32 only CPUs

[PATCH] target/arm: Don't set HCR.RW for AArch32 only CPUs

Posted by Peter Maydell 3 days, 2 hours ago

In commit 39ec3fc0301 we fixed a bug where we were not implementing
HCR_EL2.RW as RAO/WI for CPUs where EL1 doesn't support AArch32.
However, we got the condition wrong, so we now set this bit even on
CPUs which have no AArch64 support at all.  This is wrong because the
AArch32 HCR register defines this bit as RES0.

Correct the condition we use for forcing HCR_RW to be set.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3128
Fixes: 39ec3fc0301 ("target/arm: HCR_EL2.RW should be RAO/WI if EL1 doesn't support AArch32")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index c44294711f8..ba1f7296dd0 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3695,7 +3695,8 @@ static void do_hcr_write(CPUARMState *env, uint64_t value, uint64_t valid_mask)
     value &= valid_mask;
 
     /* RW is RAO/WI if EL1 is AArch64 only */
-    if (!cpu_isar_feature(aa64_aa32_el1, cpu)) {
+    if (arm_feature(env, ARM_FEATURE_AARCH64) &&
+        !cpu_isar_feature(aa64_aa32_el1, cpu)) {
         value |= HCR_RW;
     }
 
-- 
2.43.0

Re: [PATCH] target/arm: Don't set HCR.RW for AArch32 only CPUs

Posted by Richard Henderson 3 days, 1 hour ago

On 9/25/25 04:57, Peter Maydell wrote:
> In commit 39ec3fc0301 we fixed a bug where we were not implementing
> HCR_EL2.RW as RAO/WI for CPUs where EL1 doesn't support AArch32.
> However, we got the condition wrong, so we now set this bit even on
> CPUs which have no AArch64 support at all.  This is wrong because the
> AArch32 HCR register defines this bit as RES0.
> 
> Correct the condition we use for forcing HCR_RW to be set.
> 
> Cc:qemu-stable@nongnu.org
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/3128
> Fixes: 39ec3fc0301 ("target/arm: HCR_EL2.RW should be RAO/WI if EL1 doesn't support AArch32")
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/helper.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

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

r~

Re: [PATCH] target/arm: Don't set HCR.RW for AArch32 only CPUs

Posted by Philippe Mathieu-Daudé 3 days, 1 hour ago

On 25/9/25 13:57, Peter Maydell wrote:
> In commit 39ec3fc0301 we fixed a bug where we were not implementing
> HCR_EL2.RW as RAO/WI for CPUs where EL1 doesn't support AArch32.
> However, we got the condition wrong, so we now set this bit even on
> CPUs which have no AArch64 support at all.  This is wrong because the
> AArch32 HCR register defines this bit as RES0.
> 
> Correct the condition we use for forcing HCR_RW to be set.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3128
> Fixes: 39ec3fc0301 ("target/arm: HCR_EL2.RW should be RAO/WI if EL1 doesn't support AArch32")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/helper.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>