Hi Gustavo,

On Wed, 07 Oct 2026 17:56:30 +0100, Gustavo Romero <[email protected]> wrote:
[...]
> > With a 52-bit OA, walk a two-entry level 0 table through an IPA with
> > bit 50 set, which stage 2 maps back onto the table, so the walk
> > succeeds only if TTBR[4] is cleared from bit 4 of the base address.
> > With TCR.DS or VTCR.DS and a 48-bit OA, check that setting TTBR[5:2]
> > is a level 0 Address size fault.
>
> Maybe move this commit message to ttbr-baddr.c header and just replace it with
> something like "Add a test for checking if with a 52-bit OA a walk ...".

Sure, I'll move it to the file header in v3.

> I like better the test split done in v1. There there are two tests and the
> changes are clear, helping to understand which parts of the test are related 
> to
> each fix.
>
> So the organization I think is better for this series is:
>
> Fix 0
> Test 0
> Fix 1
> Test 1

Ack.

[...]

> > +    /*
> > +     * 52-bit OA: TTBR[4] is base address bit 50. Through stage 2, the
> > +     * table at HIGH_IPA | s1_l0 is s1_l0, so the walk succeeds only if
>
> I struggled a bit to make sense of this part: "Through Stage 2, the table
> HIGH_IPA | s1_l0 is s1_l0 ...", not so much for the duplication of s1_l0 in 
> the
> text but because Stage 2 uses the s2_l* tables. Any chance you could enhance a
> the explanation here?

The level 0 table address is an IPA, and the s2_l2_high mapping is
what brings it back to s1_l0. How about:

        /*
         * 52-bit OA: TTBR[4] is base address bit 50, so with it set the
         * level 0 table is at IPA HIGH_IPA | s1_l0. Stage 2 maps that IPA
         * onto s1_l0 (through s2_l2_high), so the walk succeeds only if
         * TTBR[4] does not also offset the two-entry table by 0x10.
         */

Thanks for the review. I'll respin once Richard and Peter have had a look.

Cheers,
/fuad

Reply via email to