Hi Fuad,
On Fri, Oct 02, 2026 at 08:09:19AM +0100, Fuad Tabba wrote:
> get_phys_addr_lpae() can read the first descriptor of a walk from the
> wrong address. This happens with a 52-bit OA when the initial lookup
> table has fewer than eight entries and its base address has bit 51 set,
> or bit 50 for a two-entry table. The walk then faults or translates
> through whatever that address holds. VTTBR walks for stage 2 take the
> same path.
>
> For example, the 16KB granule with a 48-bit VA has a two-entry level 0
> (FEAT_LPA2). The 64KB granule with a 44-bit VA has a four-entry level 1
> (FEAT_LPA). KVM's page_fault_test hangs on this in its 16KB, 52-bit PA
> guest mode under TCG, as reported on kvmarm.
>
> With a 52-bit OA, TTBR[5:2] hold bits [51:48] of the base address. Such
> a table is 64-byte aligned (R_KBLCR), so bits [5:0] of the base address
> are zero. get_phys_addr_lpae() ORs TTBR[5:2] into bits [51:48], but
> then clears only the bits in indexmask. For these tables indexmask
> covers fewer than six bits, so TTBR[5] stays in the address, and
> TTBR[4] too for a two-entry table.
>
> Clear TTBR[5:0] when forming a 52-bit table base address, and add a
> TCG test that walks a two-entry table through an IPA with bit 50 set,
> which stage 2 maps back onto the table.
>
> Reported-by: Itaru Kitayama <[email protected]>
> Link:
> https://lore.kernel.org/kvmarm/[email protected]/
> Fixes: 7a928f43d872 ("target/arm: Implement FEAT_LPA")
> Cc: [email protected]
> Signed-off-by: Fuad Tabba <[email protected]>
> ---
> target/arm/ptw.c | 5 +-
> tests/tcg/aarch64/system/meson.build | 6 ++
> tests/tcg/aarch64/system/ttbr-baddr.c | 141 ++++++++++++++++++++++++++
> 3 files changed, 151 insertions(+), 1 deletion(-)
> create mode 100644 tests/tcg/aarch64/system/ttbr-baddr.c
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index de0435a58b..6cff52d592 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -2107,13 +2107,16 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> S1Translate *ptw,
> descaddr = extract64(ttbr, 0, 48);
>
> /*
> - * For FEAT_LPA and PS=6, bits [51:48] of descaddr are in [5:2] of TTBR.
> + * 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).
> *
> * 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 (outputsize > 48) {
> + descaddr &= ~MAKE_64BIT_MASK(0, 6);
> descaddr |= extract64(ttbr, 2, 4) << 48;
> } else if (descaddr >> outputsize) {
> level = 0;
> diff --git a/tests/tcg/aarch64/system/meson.build
> b/tests/tcg/aarch64/system/meson.build
> index f51feb253f..270d4d6c31 100644
> --- a/tests/tcg/aarch64/system/meson.build
> +++ b/tests/tcg/aarch64/system/meson.build
> @@ -79,6 +79,12 @@ tests += {
> '-semihosting-config', 'enable=on,arg=2',
> qemu_base_args]
> },
> + 'ttbr-baddr.c': {
> + 'cflags': cflags,
> + 'qemu_args': ['-M', 'virt,virtualization=on', '-cpu', 'max',
> + '-semihosting-config', 'enable=on,arg=2',
> + qemu_base_args]
> + },
> }
>
> tests += {
> diff --git a/tests/tcg/aarch64/system/ttbr-baddr.c
> b/tests/tcg/aarch64/system/ttbr-baddr.c
> new file mode 100644
> index 0000000000..7360684268
> --- /dev/null
> +++ b/tests/tcg/aarch64/system/ttbr-baddr.c
> @@ -0,0 +1,141 @@
> +/*
> + * TTBR base address with a 52-bit layout and a small initial lookup table
> + *
> + * Copyright (c) 2026 Google LLC
> + * Author: Fuad Tabba <[email protected]>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include <stdint.h>
> +#include <minilib.h>
> +
> +/* from Linux's include/linux/stringify.h */
> +#define __stringify_1(x...) #x
> +#define __stringify(x...) __stringify_1(x)
> +
> +#define read_sysreg(r) ({ \
> + uint64_t __val; \
> + asm volatile("mrs %0, " __stringify(r) : "=r" (__val)); \
> + __val; \
> +})
> +
> +#define write_sysreg(r, v) do { \
> + uint64_t __val = (uint64_t)(v); \
> + asm volatile("msr " __stringify(r) ", %x0" \
> + : : "rZ" (__val)); \
> +} while (0)
> +
> +#define RAM_BASE 0x40000000UL
> +#define HIGH_IPA (1UL << 50)
> +#define TEST_VA (0xffff000000000000UL | RAM_BASE)
> +
> +/*
> + * Stage 1: 16KB granule, 48-bit VA, so level 0 has two entries.
> + * Stage 2: 64KB granule, 52-bit IPA, mapping RAM at its own address and
> + * again at HIGH_IPA.
> + */
> +static uint64_t s1_l0[2048] __attribute__((aligned(16384)));
> +static uint64_t s1_l1[2048] __attribute__((aligned(16384)));
> +static uint64_t s1_l2[2048] __attribute__((aligned(16384)));
> +static uint64_t s2_l1[1024] __attribute__((aligned(65536)));
> +static uint64_t s2_l2[8192] __attribute__((aligned(65536)));
> +static uint64_t s2_l2_high[8192] __attribute__((aligned(65536)));
> +
> +#define TCR_T0SZ(x) ((uint64_t)(x) << 0)
> +#define TCR_T1SZ(x) ((uint64_t)(x) << 16)
> +#define TCR_TG0_16K (2UL << 14)
> +#define TCR_TG1_16K (1UL << 30)
> +#define TCR_IPS_52 (6UL << 32)
> +#define TCR_DS (1UL << 59)
> +
> +#define VTCR_T0SZ(x) ((uint64_t)(x) << 0)
> +#define VTCR_SL0_L1 (2UL << 6)
> +#define VTCR_TG0_64K (1UL << 14)
> +#define VTCR_PS_52 (6UL << 16)
> +#define VTCR_RES1 (1UL << 31)
> +
> +#define HCR_VM (1UL << 0)
> +#define HCR_RW (1UL << 31)
> +
> +#define PAR_F (1UL << 0)
> +#define PAR_PA(par) ((par) & 0xfffffffff000UL)
> +#define S2_BLOCK ((1 << 10) | (3 << 6) | (0xf << 2) | 1)
> +
> +static void tlb_flush(void)
> +{
> + asm volatile("dsb sy; tlbi alle1; dsb sy; isb" : : : "memory");
> +}
> +
> +static uint64_t at(uint64_t ttbr1)
> +{
> + uint64_t par;
> +
> + write_sysreg(ttbr1_el1, ttbr1);
> + tlb_flush();
> + asm volatile("at s12e1r, %1; isb; mrs %0, par_el1"
> + : "=r" (par) : "r" (TEST_VA));
> + return par;
> +}
> +
> +static void setup_tables(void)
> +{
> + /* Stage 1: TEST_VA -> RAM_BASE, 32MB block, AF */
> + s1_l0[0] = (uint64_t)s1_l1 | 3;
> + s1_l1[0] = (uint64_t)s1_l2 | 3;
> + s1_l2[(RAM_BASE >> 25) & 0x7ff] = RAM_BASE | (1 << 10) | 1;
> +
> + /* Stage 2: 512MB blocks at RAM_BASE and HIGH_IPA | RAM_BASE, AF, RW */
> + s2_l1[0] = (uint64_t)s2_l2 | 3;
> + s2_l1[HIGH_IPA >> 42] = (uint64_t)s2_l2_high | 3;
> + s2_l2[RAM_BASE >> 29] = RAM_BASE | S2_BLOCK;
> + s2_l2_high[RAM_BASE >> 29] = RAM_BASE | S2_BLOCK;
> +}
> +
> +static int check(const char *name, uint64_t par)
> +{
> + int ok = !(par & PAR_F) && PAR_PA(par) == RAM_BASE;
> +
> + ml_printf("%s: PAR_EL1=%lx %s\n", name, par, ok ? "ok" : "FAIL");
> + return !ok;
> +}
> +
> +int main(void)
> +{
> + uint64_t mmfr0 = read_sysreg(id_aa64mmfr0_el1);
> + uint64_t tcr = TCR_T0SZ(16) | TCR_T1SZ(16) | TCR_TG0_16K | TCR_TG1_16K |
> + TCR_DS;
> + uint64_t base = (uint64_t)s1_l0;
> + int ret = 0;
> +
> + ml_printf("TTBR base address test\n");
> +
> + /* PARange 52 bits, TGran16 with 52-bit addresses (FEAT_LPA2) */
> + if ((mmfr0 & 0xf) != 6 || ((mmfr0 >> 20) & 0xf) != 2) {
I understand you are checking PARange == 6 here for checking if FEAT_LPA (not
FEAT_LPA2) is also available? If so, maybe change the comment to:
/* PARange 52 bits (FEAT_LPA) and TGran16 with 52-bit addresses (FEAT_LPA2) */
?
> + ml_printf("SKIP: no 52-bit PA or no FEAT_LPA2 with 16KB\n");
> + return 0;
Return 'ret' here for consistence?
That's a nice test.
I'm wondering if the fixes should be separated from the tests as we usually do,
but feel free to wait for collecting more input from the other reviewers about
it.
Cheers,
Gustavo
> + }
> +
> + /*
> + * The test runs at EL2 (arg=2) and walks the EL1&0 regime with AT, so
> + * the EL1 MMU settings below only affect those walks.
> + */
> + setup_tables();
> + write_sysreg(mair_el1, 0xff);
> + write_sysreg(sctlr_el1, read_sysreg(sctlr_el1) | 1);
> +
> + /*
> + * 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
> + * TTBR[4] does not also offset the two-entry table by 0x10.
> + */
> + write_sysreg(vtcr_el2, VTCR_T0SZ(12) | VTCR_SL0_L1 | VTCR_TG0_64K |
> + VTCR_PS_52 | VTCR_RES1);
> + write_sysreg(vttbr_el2, (uint64_t)s2_l1);
> + write_sysreg(hcr_el2, HCR_RW | HCR_VM);
> + write_sysreg(tcr_el1, tcr | TCR_IPS_52);
> + ret |= check("ips52", at(base));
> + ret |= check("ips52 ttbr[4]", at(base | (1 << 4)));
> +
> + return ret;
> +}
> --
> 2.39.5
>
>
