* [PATCH] target/arm: Don't assert in regime_is_user() for E10 mmuidx values
@ 2024-10-17 17:23 Peter Maydell
2024-10-17 17:45 ` Richard Henderson
2024-10-18 10:21 ` Alex Bennée
0 siblings, 2 replies; 3+ messages in thread
From: Peter Maydell @ 2024-10-17 17:23 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: qemu-stable
In regime_is_user() we assert if we're passed an ARMMMUIdx_E10_*
mmuidx value. This used to make sense because we only used this
function in ptw.c and would never use it on this kind of stage 1+2
mmuidx, only for an individual stage 1 or stage 2 mmuidx.
However, when we implemented FEAT_E0PD we added a callsite in
aa64_va_parameters(), which means this can now be called for
stage 1+2 mmuidx values if the guest sets the TCG_ELX.{E0PD0,E0PD1}
bits to enable use of the feature. This will then result in
an assertion failure later, for instance on a TLBI operation:
#6 0x00007ffff6d0e70f in g_assertion_message_expr
(domain=0x0, file=0x55555676eeba "../../target/arm/internals.h", line=978, func=0x555556771d48 <__func__.5> "regime_is_user", expr=<optimised out>)
at ../../../glib/gtestutils.c:3279
#7 0x0000555555f286d2 in regime_is_user (env=0x555557f2fe00, mmu_idx=ARMMMUIdx_E10_0) at ../../target/arm/internals.h:978
#8 0x0000555555f3e31c in aa64_va_parameters (env=0x555557f2fe00, va=18446744073709551615, mmu_idx=ARMMMUIdx_E10_0, data=true, el1_is_aa32=false)
at ../../target/arm/helper.c:12048
#9 0x0000555555f3163b in tlbi_aa64_get_range (env=0x555557f2fe00, mmuidx=ARMMMUIdx_E10_0, value=106721347371041) at ../../target/arm/helper.c:5214
#10 0x0000555555f317e8 in do_rvae_write (env=0x555557f2fe00, value=106721347371041, idxmap=21, synced=true) at ../../target/arm/helper.c:5260
#11 0x0000555555f31925 in tlbi_aa64_rvae1is_write (env=0x555557f2fe00, ri=0x555557fbeae0, value=106721347371041) at ../../target/arm/helper.c:5302
#12 0x0000555556036f8f in helper_set_cp_reg64 (env=0x555557f2fe00, rip=0x555557fbeae0, value=106721347371041) at ../../target/arm/tcg/op_helper.c:965
Since we do know whether these mmuidx values are for usermode
or not, we can easily make regime_is_user() handle them:
ARMMMUIdx_E10_0 is user, and the other two are not.
Cc: qemu-stable@nongnu.org
Fixes: e4c93e44ab103f ("target/arm: Implement FEAT_E0PD")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/internals.h | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 299a96a81a7..fd8f7c82aa3 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -963,6 +963,7 @@ static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
{
switch (mmu_idx) {
+ case ARMMMUIdx_E10_0:
case ARMMMUIdx_E20_0:
case ARMMMUIdx_Stage1_E0:
case ARMMMUIdx_MUser:
@@ -972,10 +973,6 @@ static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
return true;
default:
return false;
- case ARMMMUIdx_E10_0:
- case ARMMMUIdx_E10_1:
- case ARMMMUIdx_E10_1_PAN:
- g_assert_not_reached();
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] target/arm: Don't assert in regime_is_user() for E10 mmuidx values
2024-10-17 17:23 [PATCH] target/arm: Don't assert in regime_is_user() for E10 mmuidx values Peter Maydell
@ 2024-10-17 17:45 ` Richard Henderson
2024-10-18 10:21 ` Alex Bennée
1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2024-10-17 17:45 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: qemu-stable
On 10/17/24 10:23, Peter Maydell wrote:
> In regime_is_user() we assert if we're passed an ARMMMUIdx_E10_*
> mmuidx value. This used to make sense because we only used this
> function in ptw.c and would never use it on this kind of stage 1+2
> mmuidx, only for an individual stage 1 or stage 2 mmuidx.
>
> However, when we implemented FEAT_E0PD we added a callsite in
> aa64_va_parameters(), which means this can now be called for
> stage 1+2 mmuidx values if the guest sets the TCG_ELX.{E0PD0,E0PD1}
> bits to enable use of the feature. This will then result in
> an assertion failure later, for instance on a TLBI operation:
>
> #6 0x00007ffff6d0e70f in g_assertion_message_expr
> (domain=0x0, file=0x55555676eeba "../../target/arm/internals.h", line=978, func=0x555556771d48 <__func__.5> "regime_is_user", expr=<optimised out>)
> at ../../../glib/gtestutils.c:3279
> #7 0x0000555555f286d2 in regime_is_user (env=0x555557f2fe00, mmu_idx=ARMMMUIdx_E10_0) at ../../target/arm/internals.h:978
> #8 0x0000555555f3e31c in aa64_va_parameters (env=0x555557f2fe00, va=18446744073709551615, mmu_idx=ARMMMUIdx_E10_0, data=true, el1_is_aa32=false)
> at ../../target/arm/helper.c:12048
> #9 0x0000555555f3163b in tlbi_aa64_get_range (env=0x555557f2fe00, mmuidx=ARMMMUIdx_E10_0, value=106721347371041) at ../../target/arm/helper.c:5214
> #10 0x0000555555f317e8 in do_rvae_write (env=0x555557f2fe00, value=106721347371041, idxmap=21, synced=true) at ../../target/arm/helper.c:5260
> #11 0x0000555555f31925 in tlbi_aa64_rvae1is_write (env=0x555557f2fe00, ri=0x555557fbeae0, value=106721347371041) at ../../target/arm/helper.c:5302
> #12 0x0000555556036f8f in helper_set_cp_reg64 (env=0x555557f2fe00, rip=0x555557fbeae0, value=106721347371041) at ../../target/arm/tcg/op_helper.c:965
>
> Since we do know whether these mmuidx values are for usermode
> or not, we can easily make regime_is_user() handle them:
> ARMMMUIdx_E10_0 is user, and the other two are not.
>
> Cc:qemu-stable@nongnu.org
> Fixes: e4c93e44ab103f ("target/arm: Implement FEAT_E0PD")
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> target/arm/internals.h | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] target/arm: Don't assert in regime_is_user() for E10 mmuidx values
2024-10-17 17:23 [PATCH] target/arm: Don't assert in regime_is_user() for E10 mmuidx values Peter Maydell
2024-10-17 17:45 ` Richard Henderson
@ 2024-10-18 10:21 ` Alex Bennée
1 sibling, 0 replies; 3+ messages in thread
From: Alex Bennée @ 2024-10-18 10:21 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, qemu-stable
Peter Maydell <peter.maydell@linaro.org> writes:
> In regime_is_user() we assert if we're passed an ARMMMUIdx_E10_*
> mmuidx value. This used to make sense because we only used this
> function in ptw.c and would never use it on this kind of stage 1+2
> mmuidx, only for an individual stage 1 or stage 2 mmuidx.
>
> However, when we implemented FEAT_E0PD we added a callsite in
> aa64_va_parameters(), which means this can now be called for
> stage 1+2 mmuidx values if the guest sets the TCG_ELX.{E0PD0,E0PD1}
> bits to enable use of the feature. This will then result in
> an assertion failure later, for instance on a TLBI operation:
>
> #6 0x00007ffff6d0e70f in g_assertion_message_expr
> (domain=0x0, file=0x55555676eeba "../../target/arm/internals.h", line=978, func=0x555556771d48 <__func__.5> "regime_is_user", expr=<optimised out>)
> at ../../../glib/gtestutils.c:3279
> #7 0x0000555555f286d2 in regime_is_user (env=0x555557f2fe00, mmu_idx=ARMMMUIdx_E10_0) at ../../target/arm/internals.h:978
> #8 0x0000555555f3e31c in aa64_va_parameters (env=0x555557f2fe00, va=18446744073709551615, mmu_idx=ARMMMUIdx_E10_0, data=true, el1_is_aa32=false)
> at ../../target/arm/helper.c:12048
> #9 0x0000555555f3163b in tlbi_aa64_get_range (env=0x555557f2fe00, mmuidx=ARMMMUIdx_E10_0, value=106721347371041) at ../../target/arm/helper.c:5214
> #10 0x0000555555f317e8 in do_rvae_write (env=0x555557f2fe00, value=106721347371041, idxmap=21, synced=true) at ../../target/arm/helper.c:5260
> #11 0x0000555555f31925 in tlbi_aa64_rvae1is_write (env=0x555557f2fe00, ri=0x555557fbeae0, value=106721347371041) at ../../target/arm/helper.c:5302
> #12 0x0000555556036f8f in helper_set_cp_reg64 (env=0x555557f2fe00, rip=0x555557fbeae0, value=106721347371041) at ../../target/arm/tcg/op_helper.c:965
>
> Since we do know whether these mmuidx values are for usermode
> or not, we can easily make regime_is_user() handle them:
> ARMMMUIdx_E10_0 is user, and the other two are not.
>
> Cc: qemu-stable@nongnu.org
> Fixes: e4c93e44ab103f ("target/arm: Implement FEAT_E0PD")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/arm/internals.h | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 299a96a81a7..fd8f7c82aa3 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -963,6 +963,7 @@ static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
> static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
> {
> switch (mmu_idx) {
> + case ARMMMUIdx_E10_0:
> case ARMMMUIdx_E20_0:
> case ARMMMUIdx_Stage1_E0:
> case ARMMMUIdx_MUser:
> @@ -972,10 +973,6 @@ static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
> return true;
> default:
> return false;
> - case ARMMMUIdx_E10_0:
> - case ARMMMUIdx_E10_1:
> - case ARMMMUIdx_E10_1_PAN:
> - g_assert_not_reached();
> }
> }
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-18 10:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-17 17:23 [PATCH] target/arm: Don't assert in regime_is_user() for E10 mmuidx values Peter Maydell
2024-10-17 17:45 ` Richard Henderson
2024-10-18 10:21 ` Alex Bennée
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).