diff mbox series

target/arm: Use the max page size in a 2-stage ptw

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

Commit Message

Richard Henderson Sept. 13, 2022, 1:56 p.m. UTC
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(-)

Comments

Zenghui Yu Sept. 28, 2022, 4:34 a.m. UTC | #1
[ 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) {
>
Marc Zyngier Sept. 28, 2022, 6:52 a.m. UTC | #2
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 mbox series

Patch

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) {