| Message ID | 20231031173723.26582-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Correctly propagate stage 1 BTI guarded bit in a two-stage walk | expand |
On 10/31/23 10:37, Peter Maydell wrote: > In a two-stage translation, the result of the BTI guarded bit should > be the guarded bit from the first stage of translation, as there is > no BTI guard information in stage two. Our code tried to do this, > but got it wrong, because we currently have two fields where the GP > bit information might live (ARMCacheAttrs::guarded and > CPUTLBEntryFull::extra::arm::guarded), and we were storing the GP bit > in the latter during the stage 1 walk but trying to copy the former > in combine_cacheattrs(). > > Remove the duplicated storage, and always use the field in > CPUTLBEntryFull; correctly propagate the stage 1 value to the output > in get_phys_addr_twostage(). > > Note for stable backports: in v8.0 and earlier the field is named > result->f.guarded, not result->f.extra.arm.guarded. > > Cc: qemu-stable@nongnu.org > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1950 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/arm/internals.h | 1 - > target/arm/ptw.c | 7 +++++-- > 2 files changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/arm/internals.h b/target/arm/internals.h index f7224e6f4d9..c837506e448 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -1181,7 +1181,6 @@ typedef struct ARMCacheAttrs { unsigned int attrs:8; unsigned int shareability:2; /* as in the SH field of the VMSAv8-64 PTEs */ bool is_s2_format:1; - bool guarded:1; /* guarded bit of the v8-64 PTE */ } ARMCacheAttrs; /* Fields that are valid upon success. */ diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 53713e03006..1762b058aec 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -3032,7 +3032,6 @@ static ARMCacheAttrs combine_cacheattrs(uint64_t hcr, assert(!s1.is_s2_format); ret.is_s2_format = false; - ret.guarded = s1.guarded; if (s1.attrs == 0xf0) { tagged = true; @@ -3175,7 +3174,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, hwaddr ipa; int s1_prot, s1_lgpgsz; ARMSecuritySpace in_space = ptw->in_space; - bool ret, ipa_secure; + bool ret, ipa_secure, s1_guarded; ARMCacheAttrs cacheattrs1; ARMSecuritySpace ipa_space; uint64_t hcr; @@ -3202,6 +3201,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, */ s1_prot = result->f.prot; s1_lgpgsz = result->f.lg_page_size; + s1_guarded = result->f.extra.arm.guarded; cacheattrs1 = result->cacheattrs; memset(result, 0, sizeof(*result)); @@ -3252,6 +3252,9 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, result->cacheattrs = combine_cacheattrs(hcr, cacheattrs1, result->cacheattrs); + /* No BTI GP information in stage 2, we just use the S1 value */ + result->f.extra.arm.guarded = s1_guarded; + /* * Check if IPA translates to secure or non-secure PA space. * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA.
In a two-stage translation, the result of the BTI guarded bit should be the guarded bit from the first stage of translation, as there is no BTI guard information in stage two. Our code tried to do this, but got it wrong, because we currently have two fields where the GP bit information might live (ARMCacheAttrs::guarded and CPUTLBEntryFull::extra::arm::guarded), and we were storing the GP bit in the latter during the stage 1 walk but trying to copy the former in combine_cacheattrs(). Remove the duplicated storage, and always use the field in CPUTLBEntryFull; correctly propagate the stage 1 value to the output in get_phys_addr_twostage(). Note for stable backports: in v8.0 and earlier the field is named result->f.guarded, not result->f.extra.arm.guarded. Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1950 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 1 - target/arm/ptw.c | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-)