On Fri, 2 Oct 2026 at 08:09, Fuad Tabba <[email protected]> wrote:
>
> With TCR.DS set, TTBR[5:2] hold bits [51:48] of the translation table
> base address, whatever the OA size. If the effective OA size is below
> 52 bits, setting any of them is a level 0 Address size fault
> (AArch64_S1TTBaseAddress(), AArch64_S1Walk()).
>
> get_phys_addr_lpae() uses the 52-bit layout only when the OA size is
> over 48 bits. With TCR.DS set and an IPS of 48 bits, it takes TTBR[5:2]
> as base address bits [5:2] instead. A guest that sets them gets no
> fault: the walk either succeeds with TTBR[5:2] ignored, or reads its
> first descriptor from the wrong address. VTTBR walks with VTCR.DS take
> the same path.
>
> Use the 52-bit layout whenever TCR.DS is set, and check the whole base
> address against the OA size. Add TCR.DS and VTCR.DS cases with a 48-bit
> OA to the test.
>
> Fixes: ef56c2425e5f ("target/arm: Implement FEAT_LPA2")
> Cc: [email protected]
> Signed-off-by: Fuad Tabba <[email protected]>
> ---
>  target/arm/ptw.c                      | 16 ++++----
>  tests/tcg/aarch64/system/ttbr-baddr.c | 53 +++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 6cff52d592..16f5fdfe0b 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -2107,18 +2107,18 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
> S1Translate *ptw,
>      descaddr = extract64(ttbr, 0, 48);
>
>      /*
> -     * With a 52-bit OA (FEAT_LPA or FEAT_LPA2), bits [51:48] of descaddr are
> -     * in [5:2] of TTBR, and bits [5:0] of the base address are zero: a table
> -     * under 64 bytes is still 64-byte aligned (R_KBLCR).
> +     * With a 52-bit OA, or with TCR.DS, bits [51:48] of the base address are
> +     * in [5:2] of TTBR, and bits [5:0] of the base address are zero: the
> +     * table is at least 64-byte aligned.

You put that R_KBLCR reference into this comment in patch 1, so
why are you deleting it again in patch 2?

>       *
> -     * Otherwise, if the base address is out of range, raise 
> AddressSizeFault.
> -     * In the pseudocode, this is !IsZero(baseregister<47:outputsize>),
> -     * but we've just cleared the bits above 47, so simplify the test.
> +     * If the base address is out of range, raise AddressSizeFault, as
> +     * AArch64_OAOutOfRange() does in the pseudocode.

Please don't drop information that's still relevant. This comment is
remarking that although the pseudocode is doing an IsZero check only
on the bits of the address between the max physaddr size and the
outputsize, QEMU is doing a check on all the upper bits, and saying
why that's OK. This is still the case, although the pseudocode has
changed a little so AArch64_OAOutOfRange()'s test is now:
   return !IsZero(address[NUM_PABITS-1:oasize]);
(and NUM_PABITS is 56).

>       */
> -    if (outputsize > 48) {
> +    if (outputsize > 48 || param.ds) {
>          descaddr &= ~MAKE_64BIT_MASK(0, 6);
>          descaddr |= extract64(ttbr, 2, 4) << 48;
> -    } else if (descaddr >> outputsize) {
> +    }
> +    if (descaddr >> outputsize) {
>          level = 0;
>          fi->type = ARMFault_AddressSize;
>          goto do_fault;

The code changes here look correct (compare AArch64_S1TTBaseAddress()
which checks walkparams.ds == '1' || (complex expr for FEAT_LPA)).

thanks
-- PMM

Reply via email to