| Message ID | 20221212142708.610090-1-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm:Set lg_page_size to 0 if either S1 or S2 asks for it | expand |
On 12/12/22 06:27, Peter Maydell wrote: > In get_phys_addr_twostage() we set the lg_page_size of the result to > the maximum of the stage 1 and stage 2 page sizes. This works for > the case where we do want to create a TLB entry, because we know the > common TLB code only creates entries of the TARGET_PAGE_SIZE and > asking for a size larger than that only means that invalidations > invalidate the whole larger area. However, if lg_page_size is > smaller than TARGET_PAGE_SIZE this effectively means "don't create a > TLB entry"; in this case if either S1 or S2 said "this covers less > than a page and can't go in a TLB" then the final result also should > be marked that way. Set the resulting page size to 0 if either > stage asked for a less-than-a-page entry, and expand the comment > to explain what's going on. > > This has no effect for VMSA because currently the VMSA lookup always > returns results that cover at least TARGET_PAGE_SIZE; however when we > add v8R support it will reuse this code path, and for v8R the S1 and > S2 results can be smaller than TARGET_PAGE_SIZE. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > Specifically, this avoids bugs for v8R if either S1 or S2 > MPU is set up with region sizes < 1K. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ > > target/arm/ptw.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index f812734bfb2..2e7826dc29b 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -2655,10 +2655,20 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, > } > > /* > - * Use the maximum of the S1 & S2 page size, so that invalidation > - * of pages > TARGET_PAGE_SIZE works correctly. > + * If either S1 or S2 returned a result smaller than TARGET_PAGE_SIZE, > + * this means "don't put this in the TLB"; in this case, return a > + * result with lg_page_size == 0 to achieve that. Otherwise, > + * use the maximum of the S1 & S2 page size, so that invalidation > + * of pages > TARGET_PAGE_SIZE works correctly. (This works even though > + * we know the combined result permissions etc only cover the minimum > + * of the S1 and S2 page size, because we know that the common TLB code > + * never actually creates TLB entries bigger than TARGET_PAGE_SIZE, > + * and passing a larger page size value only affects invalidations.) > */ > - if (result->f.lg_page_size < s1_lgpgsz) { > + if (result->f.lg_page_size < TARGET_PAGE_BITS || > + s1_lgpgsz < TARGET_PAGE_BITS) { > + result->f.lg_page_size = 0; > + } else if (result->f.lg_page_size < s1_lgpgsz) { > result->f.lg_page_size = s1_lgpgsz; > } >
diff --git a/target/arm/ptw.c b/target/arm/ptw.c index f812734bfb2..2e7826dc29b 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -2655,10 +2655,20 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, } /* - * Use the maximum of the S1 & S2 page size, so that invalidation - * of pages > TARGET_PAGE_SIZE works correctly. + * If either S1 or S2 returned a result smaller than TARGET_PAGE_SIZE, + * this means "don't put this in the TLB"; in this case, return a + * result with lg_page_size == 0 to achieve that. Otherwise, + * use the maximum of the S1 & S2 page size, so that invalidation + * of pages > TARGET_PAGE_SIZE works correctly. (This works even though + * we know the combined result permissions etc only cover the minimum + * of the S1 and S2 page size, because we know that the common TLB code + * never actually creates TLB entries bigger than TARGET_PAGE_SIZE, + * and passing a larger page size value only affects invalidations.) */ - if (result->f.lg_page_size < s1_lgpgsz) { + if (result->f.lg_page_size < TARGET_PAGE_BITS || + s1_lgpgsz < TARGET_PAGE_BITS) { + result->f.lg_page_size = 0; + } else if (result->f.lg_page_size < s1_lgpgsz) { result->f.lg_page_size = s1_lgpgsz; }
In get_phys_addr_twostage() we set the lg_page_size of the result to the maximum of the stage 1 and stage 2 page sizes. This works for the case where we do want to create a TLB entry, because we know the common TLB code only creates entries of the TARGET_PAGE_SIZE and asking for a size larger than that only means that invalidations invalidate the whole larger area. However, if lg_page_size is smaller than TARGET_PAGE_SIZE this effectively means "don't create a TLB entry"; in this case if either S1 or S2 said "this covers less than a page and can't go in a TLB" then the final result also should be marked that way. Set the resulting page size to 0 if either stage asked for a less-than-a-page entry, and expand the comment to explain what's going on. This has no effect for VMSA because currently the VMSA lookup always returns results that cover at least TARGET_PAGE_SIZE; however when we add v8R support it will reuse this code path, and for v8R the S1 and S2 results can be smaller than TARGET_PAGE_SIZE. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- Specifically, this avoids bugs for v8R if either S1 or S2 MPU is set up with region sizes < 1K. target/arm/ptw.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)