* [PATCH 0/2] target/arm: Fix the TTBR table base address in its 52-bit layout
@ 2026-10-02 7:09 Fuad Tabba
2026-10-02 7:09 ` [PATCH 1/2] target/arm: Clear TTBR[5:0] from a 52-bit table base address Fuad Tabba
2026-10-02 7:09 ` [PATCH 2/2] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set Fuad Tabba
0 siblings, 2 replies; 9+ messages in thread
From: Fuad Tabba @ 2026-10-02 7:09 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel, Richard Henderson, Will Deacon,
Itaru Kitayama, Fuad Tabba
Hi folks,
Two fixes to how get_phys_addr_lpae() forms the initial table base
address when TTBR uses its 52-bit layout, each with a TCG test.
The first stops TTBR[5:4] leaking into the base address of a table
smaller than 64 bytes. KVM's page_fault_test hangs on it under TCG in
its 16KB, 52-bit PA guest mode. Itaru reported the 16KB hang on kvmarm
last year [1].
The second uses the 52-bit layout whenever TCR.DS is set, as the
architecture does, not only with a 52-bit OA. With a smaller OA a
non-zero TTBR[5:2] is then an Address size fault.
Based on QEMU master (f7ada39eda).
Cheers,
/fuad
[1] https://lore.kernel.org/kvmarm/B4C0AC3E-6A4F-4A7B-B7BC-81207539115E@linux.dev/
Fuad Tabba (2):
target/arm: Clear TTBR[5:0] from a 52-bit table base address
target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set
target/arm/ptw.c | 15 +-
tests/tcg/aarch64/system/meson.build | 6 +
tests/tcg/aarch64/system/ttbr-baddr.c | 194 ++++++++++++++++++++++++++
3 files changed, 209 insertions(+), 6 deletions(-)
create mode 100644 tests/tcg/aarch64/system/ttbr-baddr.c
--
2.39.5
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] target/arm: Clear TTBR[5:0] from a 52-bit table base address
2026-10-02 7:09 [PATCH 0/2] target/arm: Fix the TTBR table base address in its 52-bit layout Fuad Tabba
@ 2026-10-02 7:09 ` Fuad Tabba
2026-10-05 20:06 ` Gustavo Romero
2026-10-02 7:09 ` [PATCH 2/2] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set Fuad Tabba
1 sibling, 1 reply; 9+ messages in thread
From: Fuad Tabba @ 2026-10-02 7:09 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel, Richard Henderson, Will Deacon,
Itaru Kitayama, Fuad Tabba
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 <itaru.kitayama@linux.dev>
Link: https://lore.kernel.org/kvmarm/B4C0AC3E-6A4F-4A7B-B7BC-81207539115E@linux.dev/
Fixes: 7a928f43d872 ("target/arm: Implement FEAT_LPA")
Cc: qemu-stable@nongnu.org
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
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 <fuad.tabba@linux.dev>
+ *
+ * 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) {
+ ml_printf("SKIP: no 52-bit PA or no FEAT_LPA2 with 16KB\n");
+ return 0;
+ }
+
+ /*
+ * 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set
2026-10-02 7:09 [PATCH 0/2] target/arm: Fix the TTBR table base address in its 52-bit layout Fuad Tabba
2026-10-02 7:09 ` [PATCH 1/2] target/arm: Clear TTBR[5:0] from a 52-bit table base address Fuad Tabba
@ 2026-10-02 7:09 ` Fuad Tabba
2026-10-06 11:24 ` Peter Maydell
1 sibling, 1 reply; 9+ messages in thread
From: Fuad Tabba @ 2026-10-02 7:09 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel, Richard Henderson, Will Deacon,
Itaru Kitayama, Fuad Tabba
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: qemu-stable@nongnu.org
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
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.
*
- * 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.
*/
- 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;
diff --git a/tests/tcg/aarch64/system/ttbr-baddr.c b/tests/tcg/aarch64/system/ttbr-baddr.c
index 7360684268..6a15c55fae 100644
--- a/tests/tcg/aarch64/system/ttbr-baddr.c
+++ b/tests/tcg/aarch64/system/ttbr-baddr.c
@@ -34,10 +34,13 @@
* 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.
+ * Stage 2 with VTCR.DS: 16KB granule, 47-bit IPA, mapping RAM flat.
*/
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 s2ds_l1[2048] __attribute__((aligned(16384)));
+static uint64_t s2ds_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)));
@@ -46,20 +49,27 @@ static uint64_t s2_l2_high[8192] __attribute__((aligned(65536)));
#define TCR_T1SZ(x) ((uint64_t)(x) << 16)
#define TCR_TG0_16K (2UL << 14)
#define TCR_TG1_16K (1UL << 30)
+#define TCR_IPS_48 (5UL << 32)
#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_16K (2UL << 14)
#define VTCR_TG0_64K (1UL << 14)
+#define VTCR_PS_48 (5UL << 16)
#define VTCR_PS_52 (6UL << 16)
#define VTCR_RES1 (1UL << 31)
+#define VTCR_DS (1UL << 32)
#define HCR_VM (1UL << 0)
#define HCR_RW (1UL << 31)
#define PAR_F (1UL << 0)
+#define PAR_FST(par) (((par) >> 1) & 0x3f)
#define PAR_PA(par) ((par) & 0xfffffffff000UL)
+#define FST_ADDR_SIZE_L0 0x00
+
#define S2_BLOCK ((1 << 10) | (3 << 6) | (0xf << 2) | 1)
static void tlb_flush(void)
@@ -78,6 +88,17 @@ static uint64_t at(uint64_t ttbr1)
return par;
}
+static uint64_t at_s1(uint64_t ttbr1)
+{
+ uint64_t par;
+
+ write_sysreg(ttbr1_el1, ttbr1);
+ tlb_flush();
+ asm volatile("at s1e1r, %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 */
@@ -85,6 +106,10 @@ static void setup_tables(void)
s1_l1[0] = (uint64_t)s1_l2 | 3;
s1_l2[(RAM_BASE >> 25) & 0x7ff] = RAM_BASE | (1 << 10) | 1;
+ /* Stage 2 with VTCR.DS: RAM_BASE -> RAM_BASE, 32MB block, AF, RW */
+ s2ds_l1[0] = (uint64_t)s2ds_l2 | 3;
+ s2ds_l2[(RAM_BASE >> 25) & 0x7ff] = RAM_BASE | S2_BLOCK;
+
/* 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;
@@ -100,6 +125,15 @@ static int check(const char *name, uint64_t par)
return !ok;
}
+/* A level 0 Address size fault */
+static int check_fault(const char *name, uint64_t par)
+{
+ int ok = (par & PAR_F) && PAR_FST(par) == FST_ADDR_SIZE_L0;
+
+ 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);
@@ -124,6 +158,25 @@ int main(void)
write_sysreg(mair_el1, 0xff);
write_sysreg(sctlr_el1, read_sysreg(sctlr_el1) | 1);
+ /*
+ * TCR.DS with a 48-bit OA: TTBR[5:2] are base address bits [51:48],
+ * so setting any of them is a level 0 Address size fault.
+ */
+ write_sysreg(hcr_el2, HCR_RW);
+ write_sysreg(tcr_el1, tcr | TCR_IPS_48);
+ ret |= check("ips48", at_s1(base));
+ ret |= check_fault("ips48 ttbr[2]", at_s1(base | (1 << 2)));
+ ret |= check_fault("ips48 ttbr[4]", at_s1(base | (1 << 4)));
+
+ /* The same for VTTBR, with VTCR.DS and a 48-bit PS */
+ write_sysreg(vtcr_el2, VTCR_T0SZ(17) | VTCR_SL0_L1 | VTCR_TG0_16K |
+ VTCR_PS_48 | VTCR_RES1 | VTCR_DS);
+ write_sysreg(hcr_el2, HCR_RW | HCR_VM);
+ write_sysreg(vttbr_el2, (uint64_t)s2ds_l1);
+ ret |= check("ps48", at(base));
+ write_sysreg(vttbr_el2, (uint64_t)s2ds_l1 | (1 << 2));
+ ret |= check_fault("ps48 vttbr[2]", at(base));
+
/*
* 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
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] target/arm: Clear TTBR[5:0] from a 52-bit table base address
2026-10-02 7:09 ` [PATCH 1/2] target/arm: Clear TTBR[5:0] from a 52-bit table base address Fuad Tabba
@ 2026-10-05 20:06 ` Gustavo Romero
2026-10-05 21:44 ` Fuad Tabba
0 siblings, 1 reply; 9+ messages in thread
From: Gustavo Romero @ 2026-10-05 20:06 UTC (permalink / raw)
To: Fuad Tabba
Cc: Peter Maydell, qemu-arm, qemu-devel, Richard Henderson,
Will Deacon, Itaru Kitayama, Fuad Tabba
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 <itaru.kitayama@linux.dev>
> Link: https://lore.kernel.org/kvmarm/B4C0AC3E-6A4F-4A7B-B7BC-81207539115E@linux.dev/
> Fixes: 7a928f43d872 ("target/arm: Implement FEAT_LPA")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
> 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 <fuad.tabba@linux.dev>
> + *
> + * 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
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] target/arm: Clear TTBR[5:0] from a 52-bit table base address
2026-10-05 20:06 ` Gustavo Romero
@ 2026-10-05 21:44 ` Fuad Tabba
2026-10-06 9:03 ` Peter Maydell
0 siblings, 1 reply; 9+ messages in thread
From: Fuad Tabba @ 2026-10-05 21:44 UTC (permalink / raw)
To: Gustavo Romero
Cc: Peter Maydell, qemu-arm, qemu-devel, Richard Henderson,
Will Deacon, Itaru Kitayama, Fuad Tabba
Hi Gustavo,
On Mon, 05 Oct 2026 21:06:44 +0100, Gustavo Romero <gromero@redhat.com> wrote:
[...]
> > diff --git a/tests/tcg/aarch64/system/ttbr-baddr.c b/tests/tcg/aarch64/system/ttbr-baddr.c
[...]
> > +int main(void)
[...]
> > + /* 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) */
>
> ?
Yes, the test's 52-bit PA and IPA both require FEAT_LPA. I'll take
your comment for v2.
> > + ml_printf("SKIP: no 52-bit PA or no FEAT_LPA2 with 16KB\n");
> > + return 0;
>
> Return 'ret' here for consistence?
Sure, I'll change it in v2.
> 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.
Thanks! I kept each test case with the fix it covers, so each patch
carries a test case that fails without its fix. Both forms have gone
into target/arm, including my last contribution. Happy to split if you
prefer. I'll wait for more comments before doing that.
Cheers,
/fuad
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] target/arm: Clear TTBR[5:0] from a 52-bit table base address
2026-10-05 21:44 ` Fuad Tabba
@ 2026-10-06 9:03 ` Peter Maydell
2026-10-06 9:27 ` Fuad Tabba
0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2026-10-06 9:03 UTC (permalink / raw)
To: Fuad Tabba
Cc: Gustavo Romero, qemu-arm, qemu-devel, Richard Henderson,
Will Deacon, Itaru Kitayama, Fuad Tabba
On Mon, 5 Oct 2026 at 22:44, Fuad Tabba <fuad.tabba@linux.dev> wrote:
> On Mon, 05 Oct 2026 21:06:44 +0100, Gustavo Romero <gromero@redhat.com> wrote:
> > 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.
>
> Thanks! I kept each test case with the fix it covers, so each patch
> carries a test case that fails without its fix. Both forms have gone
> into target/arm, including my last contribution. Happy to split if you
> prefer. I'll wait for more comments before doing that.
Personally I prefer "fix commit first, test commit second", especially
for cases like this where the fix is a one-liner and the test case is
150 lines; but I don't object to single-commit strongly enough to
require splitting it if it wouldn't otherwise require a respin.
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] target/arm: Clear TTBR[5:0] from a 52-bit table base address
2026-10-06 9:03 ` Peter Maydell
@ 2026-10-06 9:27 ` Fuad Tabba
0 siblings, 0 replies; 9+ messages in thread
From: Fuad Tabba @ 2026-10-06 9:27 UTC (permalink / raw)
To: Peter Maydell
Cc: Gustavo Romero, qemu-arm, qemu-devel, Richard Henderson,
Will Deacon, Itaru Kitayama
Hi Peter,
On Tue, 6 Oct 2026 at 11:03, Peter Maydell <peter.maydell@linaro.org> wrote:
[...]
> Personally I prefer "fix commit first, test commit second", especially
> for cases like this where the fix is a one-liner and the test case is
> 150 lines; but I don't object to single-commit strongly enough to
> require splitting it if it wouldn't otherwise require a respin.
Since Gistavo had other feedback, I'm happy to respin with this.
Cheers,
/fuad
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set
2026-10-02 7:09 ` [PATCH 2/2] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set Fuad Tabba
@ 2026-10-06 11:24 ` Peter Maydell
2026-10-06 13:20 ` Fuad Tabba
0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2026-10-06 11:24 UTC (permalink / raw)
To: Fuad Tabba
Cc: qemu-arm, qemu-devel, Richard Henderson, Will Deacon,
Itaru Kitayama, Fuad Tabba
On Fri, 2 Oct 2026 at 08:09, Fuad Tabba <fuad.tabba@linux.dev> 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: qemu-stable@nongnu.org
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set
2026-10-06 11:24 ` Peter Maydell
@ 2026-10-06 13:20 ` Fuad Tabba
0 siblings, 0 replies; 9+ messages in thread
From: Fuad Tabba @ 2026-10-06 13:20 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, qemu-devel, Richard Henderson, Will Deacon,
Itaru Kitayama
Hi Peter,
On Tue, 06 Oct 2026 12:24:17 +0100, Peter Maydell
<peter.maydell@linaro.org> wrote:
[...]
> > /*
> > - * 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?
I'll keep it in v2.
> > *
> > - * 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).
I'll put it back in v2, updated to the current pseudocode. v2 will
also have the test in its own patch, after the two fixes.
Cheers,
/fuad
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-07 9:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-02 7:09 [PATCH 0/2] target/arm: Fix the TTBR table base address in its 52-bit layout Fuad Tabba
2026-10-02 7:09 ` [PATCH 1/2] target/arm: Clear TTBR[5:0] from a 52-bit table base address Fuad Tabba
2026-10-05 20:06 ` Gustavo Romero
2026-10-05 21:44 ` Fuad Tabba
2026-10-06 9:03 ` Peter Maydell
2026-10-06 9:27 ` Fuad Tabba
2026-10-02 7:09 ` [PATCH 2/2] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set Fuad Tabba
2026-10-06 11:24 ` Peter Maydell
2026-10-06 13:20 ` Fuad Tabba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.