On Mon, 24 Oct 2022 at 06:19, Richard Henderson
<richard.hender...@linaro.org> 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 <m...@kernel.org>
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
So when I was reviewing the v8R patchset I re-found this
change in the code, and had some questions about it that
I wasn't thinking about the first time...
> @@ -2639,6 +2640,14 @@ static bool get_phys_addr_twostage(CPUARMState *env,
> S1Translate *ptw,
> 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) {
Firstly, what if the lg_page_size is < TARGET_PAGE_SIZE ?
I think this can't happen for VMSA, but for PMSA it will
when the region (in either S1 or S2) is less than the page size
(in which case lg_page_size is 0). Presumably in this case we
want to set the result's lg_page_size to also be 0 to
preserve the "don't put this in the TLB" effect.
Secondly, how does this work for VMSA? Suppose that stage 1
is using 4K pages and stage 2 is using 64K pages. We will then
claim here that the result lg_page_size is 64K, but the
attributes and mapping in the result are only valid for
the 4K page that we looked up in stage 1 -- the surrounding
4K pages could have entirely different permissions/mapping.
thanks
-- PMM
