| Message ID | 20220913135643.55728-1-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Use the max page size in a 2-stage ptw | expand |
[ Fix Marc's email address ] On 2022/9/13 21:56, Richard Henderson wrote: > We had only been reporting the stage2 page size. This causes > problems if stage1 is using a larger page size (16k, 2M, etc), > but stage2 is using a smaller page size, because cputlb does > not set large_page_{addr,mask} properly. > > Fix by using the max of the two page sizes. > > Reported-by: Marc Zyngier <marc.zyngier@arm.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > > Hi Mark, I think this will fix the issue that you mentioned on Monday. > It certainly appears to fit the bill vs the described symptoms. > > This is based on my ptw.c rewrite, full tree at > > https://gitlab.com/rth7680/qemu/-/tree/tgt-arm-rme > > Based-on: 20220822152741.1617527-1-richard.henderson@linaro.org > ("[PATCH v2 00/66] target/arm: Implement FEAT_HAFDBS") > > > r~ > > --- > target/arm/ptw.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index c81c51f60c..510939fc89 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -2509,7 +2509,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, target_ulong address, > ARMMMUFaultInfo *fi) > { > hwaddr ipa; > - int s1_prot; > + int s1_prot, s1_lgpgsz; > bool ret; > bool ipa_secure; > ARMCacheAttrs cacheattrs1; > @@ -2550,6 +2550,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, target_ulong address, > * prot and cacheattrs later. > */ > s1_prot = result->f.prot; > + s1_lgpgsz = result->f.lg_page_size; > cacheattrs1 = result->cacheattrs; > memset(result, 0, sizeof(*result)); > > @@ -2565,6 +2566,14 @@ static bool get_phys_addr_twostage(CPUARMState *env, target_ulong address, > return ret; > } > > + /* > + * Use the maximum of the S1 & S2 page size, so that invalidation > + * of pages > TARGET_PAGE_SIZE works correctly. > + */ > + if (result->f.lg_page_size < s1_lgpgsz) { > + result->f.lg_page_size = s1_lgpgsz; > + } > + > /* Combine the S1 and S2 cache attributes. */ > hcr = arm_hcr_el2_eff_secstate(env, is_secure); > if (hcr & HCR_DC) { >
On Wed, 28 Sep 2022 05:34:53 +0100, Zenghui Yu <yuzenghui@huawei.com> wrote: > > [ Fix Marc's email address ] Ah, many thanks Zenghui! I was wondering whether my discussion with Richard had any result. As it turns out, it had an almost immediate result! > > On 2022/9/13 21:56, Richard Henderson wrote: > > We had only been reporting the stage2 page size. This causes > > problems if stage1 is using a larger page size (16k, 2M, etc), > > but stage2 is using a smaller page size, because cputlb does > > not set large_page_{addr,mask} properly. > > > > Fix by using the max of the two page sizes. > > > > Reported-by: Marc Zyngier <marc.zyngier@arm.com> This is no longer a thing (and hasn't been for over 3 years! ;-). maz@kernel.org is the canonical address. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > --- > > > > Hi Mark, I think this will fix the issue that you mentioned on Monday. > > It certainly appears to fit the bill vs the described symptoms. > > > > This is based on my ptw.c rewrite, full tree at > > > > https://gitlab.com/rth7680/qemu/-/tree/tgt-arm-rme > > > > Based-on: 20220822152741.1617527-1-richard.henderson@linaro.org > > ("[PATCH v2 00/66] target/arm: Implement FEAT_HAFDBS") Thanks Richard. I'll try and give it a spin shortly. Cheers, M.
diff --git a/target/arm/ptw.c b/target/arm/ptw.c index c81c51f60c..510939fc89 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -2509,7 +2509,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, target_ulong address, ARMMMUFaultInfo *fi) { hwaddr ipa; - int s1_prot; + int s1_prot, s1_lgpgsz; bool ret; bool ipa_secure; ARMCacheAttrs cacheattrs1; @@ -2550,6 +2550,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, target_ulong address, * prot and cacheattrs later. */ s1_prot = result->f.prot; + s1_lgpgsz = result->f.lg_page_size; cacheattrs1 = result->cacheattrs; memset(result, 0, sizeof(*result)); @@ -2565,6 +2566,14 @@ static bool get_phys_addr_twostage(CPUARMState *env, target_ulong address, return ret; } + /* + * Use the maximum of the S1 & S2 page size, so that invalidation + * of pages > TARGET_PAGE_SIZE works correctly. + */ + if (result->f.lg_page_size < s1_lgpgsz) { + result->f.lg_page_size = s1_lgpgsz; + } + /* Combine the S1 and S2 cache attributes. */ hcr = arm_hcr_el2_eff_secstate(env, is_secure); if (hcr & HCR_DC) {
We had only been reporting the stage2 page size. This causes problems if stage1 is using a larger page size (16k, 2M, etc), but stage2 is using a smaller page size, because cputlb does not set large_page_{addr,mask} properly. Fix by using the max of the two page sizes. Reported-by: Marc Zyngier <marc.zyngier@arm.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- Hi Mark, I think this will fix the issue that you mentioned on Monday. It certainly appears to fit the bill vs the described symptoms. This is based on my ptw.c rewrite, full tree at https://gitlab.com/rth7680/qemu/-/tree/tgt-arm-rme Based-on: 20220822152741.1617527-1-richard.henderson@linaro.org ("[PATCH v2 00/66] target/arm: Implement FEAT_HAFDBS") r~ --- target/arm/ptw.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)