All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] target/arm: Fix the TTBR table base address in its 52-bit layout
@ 2026-10-07  8:00 Fuad Tabba
  2026-10-07  8:00 ` [PATCH v2 1/3] target/arm: Clear TTBR[5:0] from a 52-bit table base address Fuad Tabba
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Fuad Tabba @ 2026-10-07  8:00 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, qemu-devel, Richard Henderson, Gustavo Romero,
	Will Deacon, Itaru Kitayama, Fuad Tabba

Hi folks,

Changes since v1 [1]:
  - Moved the test into its own patch, after the two fixes. (Gustavo)
  - Various fixes. (Peter, Gustavo)

Two fixes to how get_phys_addr_lpae() forms the initial table base
address when TTBR uses its 52-bit layout, then a TCG test for both.

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 [2].

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://msgid.link/20261001175003.ttbr-baddr-52bit-0-fuad.tabba@linux.dev
[2] https://msgid.link/B4C0AC3E-6A4F-4A7B-B7BC-81207539115E@linux.dev

Fuad Tabba (3):
  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
  tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout

 target/arm/ptw.c                      |  18 ++-
 tests/tcg/aarch64/system/meson.build  |   6 +
 tests/tcg/aarch64/system/ttbr-baddr.c | 197 ++++++++++++++++++++++++++
 3 files changed, 215 insertions(+), 6 deletions(-)
 create mode 100644 tests/tcg/aarch64/system/ttbr-baddr.c

-- 
2.39.5



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/3] target/arm: Clear TTBR[5:0] from a 52-bit table base address
  2026-10-07  8:00 [PATCH v2 0/3] target/arm: Fix the TTBR table base address in its 52-bit layout Fuad Tabba
@ 2026-10-07  8:00 ` Fuad Tabba
  2026-10-07 16:55   ` Gustavo Romero
  2026-10-07  8:00 ` [PATCH v2 2/3] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set Fuad Tabba
  2026-10-07  8:00 ` [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout Fuad Tabba
  2 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2026-10-07  8:00 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, qemu-devel, Richard Henderson, Gustavo Romero,
	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.

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 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

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;
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set
  2026-10-07  8:00 [PATCH v2 0/3] target/arm: Fix the TTBR table base address in its 52-bit layout Fuad Tabba
  2026-10-07  8:00 ` [PATCH v2 1/3] target/arm: Clear TTBR[5:0] from a 52-bit table base address Fuad Tabba
@ 2026-10-07  8:00 ` Fuad Tabba
  2026-10-07 16:55   ` Gustavo Romero
  2026-10-07  8:00 ` [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout Fuad Tabba
  2 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2026-10-07  8:00 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, qemu-devel, Richard Henderson, Gustavo Romero,
	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.

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 | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 6cff52d592..2e67d02493 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2107,18 +2107,21 @@ 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 (FEAT_LPA or FEAT_LPA2), or with TCR.DS set at any OA
+     * size, 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 the base address is out of range, raise AddressSizeFault.
+     * In the pseudocode, this is !IsZero(address<NUM_PABITS-1:outputsize>)
+     * in AArch64_OAOutOfRange(), but descaddr has no bits set above 51,
+     * so simplify the test.
      */
-    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;
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout
  2026-10-07  8:00 [PATCH v2 0/3] target/arm: Fix the TTBR table base address in its 52-bit layout Fuad Tabba
  2026-10-07  8:00 ` [PATCH v2 1/3] target/arm: Clear TTBR[5:0] from a 52-bit table base address Fuad Tabba
  2026-10-07  8:00 ` [PATCH v2 2/3] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set Fuad Tabba
@ 2026-10-07  8:00 ` Fuad Tabba
  2026-10-07 16:56   ` Gustavo Romero
  2 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2026-10-07  8:00 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, qemu-devel, Richard Henderson, Gustavo Romero,
	Will Deacon, Itaru Kitayama, Fuad Tabba

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.

Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 tests/tcg/aarch64/system/meson.build  |   6 +
 tests/tcg/aarch64/system/ttbr-baddr.c | 197 ++++++++++++++++++++++++++
 2 files changed, 203 insertions(+)
 create mode 100644 tests/tcg/aarch64/system/ttbr-baddr.c

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..b36c14aa29
--- /dev/null
+++ b/tests/tcg/aarch64/system/ttbr-baddr.c
@@ -0,0 +1,197 @@
+/*
+ * 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.
+ * 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)));
+
+#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_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)
+{
+    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 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 */
+    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 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;
+    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;
+}
+
+/* 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);
+    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 (FEAT_LPA) and 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 ret;
+    }
+
+    /*
+     * 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);
+
+    /*
+     * 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
+     * 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] 11+ messages in thread

* Re: [PATCH v2 1/3] target/arm: Clear TTBR[5:0] from a 52-bit table base address
  2026-10-07  8:00 ` [PATCH v2 1/3] target/arm: Clear TTBR[5:0] from a 52-bit table base address Fuad Tabba
@ 2026-10-07 16:55   ` Gustavo Romero
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Romero @ 2026-10-07 16:55 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Peter Maydell, qemu-arm, qemu-devel, Richard Henderson,
	Will Deacon, Itaru Kitayama, Fuad Tabba

Hi Fuad,

Thanks for separating the fixes from the tests.

On Wed, Oct 07, 2026 at 09:00:17AM +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.
> 
> 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 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> 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;
> -- 
> 2.39.5
> 

Reviewed-by: Gustavo Romero <gromero@redhat.com>


Cheers,
Gustavo



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set
  2026-10-07  8:00 ` [PATCH v2 2/3] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set Fuad Tabba
@ 2026-10-07 16:55   ` Gustavo Romero
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Romero @ 2026-10-07 16:55 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Peter Maydell, qemu-arm, qemu-devel, Richard Henderson,
	Will Deacon, Itaru Kitayama, Fuad Tabba

Hi Fuad,

On Wed, Oct 07, 2026 at 09:00:18AM +0100, Fuad Tabba 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.
> 
> 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 | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 6cff52d592..2e67d02493 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -2107,18 +2107,21 @@ 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 (FEAT_LPA or FEAT_LPA2), or with TCR.DS set at any OA
> +     * size, 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 the base address is out of range, raise AddressSizeFault.
> +     * In the pseudocode, this is !IsZero(address<NUM_PABITS-1:outputsize>)
> +     * in AArch64_OAOutOfRange(), but descaddr has no bits set above 51,
> +     * so simplify the test.
>       */
> -    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;
> -- 
> 2.39.5
>

Reviewed-by: Gustavo Romero <gromero@redhat.com>


Cheers,
Gustavo



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout
  2026-10-07  8:00 ` [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout Fuad Tabba
@ 2026-10-07 16:56   ` Gustavo Romero
  2026-10-07 21:29     ` Fuad Tabba
  0 siblings, 1 reply; 11+ messages in thread
From: Gustavo Romero @ 2026-10-07 16:56 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Peter Maydell, qemu-arm, qemu-devel, Richard Henderson,
	Will Deacon, Itaru Kitayama, Fuad Tabba

Hi Fuad,

On Wed, Oct 07, 2026 at 09:00:19AM +0100, Fuad Tabba 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 ...".

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


> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
>  tests/tcg/aarch64/system/meson.build  |   6 +
>  tests/tcg/aarch64/system/ttbr-baddr.c | 197 ++++++++++++++++++++++++++
>  2 files changed, 203 insertions(+)
>  create mode 100644 tests/tcg/aarch64/system/ttbr-baddr.c
> 
> 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..b36c14aa29
> --- /dev/null
> +++ b/tests/tcg/aarch64/system/ttbr-baddr.c
> @@ -0,0 +1,197 @@
> +/*
> + * 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.
> + * 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)));
> +
> +#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_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)
> +{
> +    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 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 */
> +    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 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;
> +    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;
> +}
> +
> +/* 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);
> +    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 (FEAT_LPA) and 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 ret;
> +    }
> +
> +    /*
> +     * 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);
> +
> +    /*
> +     * 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

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?


> +     * 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
> 

Otherwise:

Reviewed-by: Gustavo Romero <gromero@redhat.com>

As always, free feel to respin it only when you get more review comments.

We probably want an additional review of the series from Richard ;)


Cheers,
Gustavo



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout
  2026-10-07 16:56   ` Gustavo Romero
@ 2026-10-07 21:29     ` Fuad Tabba
  2026-10-08 16:40       ` Gustavo Romero
  0 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2026-10-07 21:29 UTC (permalink / raw)
  To: Gustavo Romero
  Cc: Peter Maydell, qemu-arm, qemu-devel, Richard Henderson,
	Will Deacon, Itaru Kitayama

Hi Gustavo,

On Wed, 07 Oct 2026 17:56:30 +0100, Gustavo Romero <gromero@redhat.com> 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout
  2026-10-07 21:29     ` Fuad Tabba
@ 2026-10-08 16:40       ` Gustavo Romero
  2026-10-08 18:01         ` Fuad Tabba
  0 siblings, 1 reply; 11+ messages in thread
From: Gustavo Romero @ 2026-10-08 16:40 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Peter Maydell, qemu-arm, qemu-devel, Richard Henderson,
	Will Deacon, Itaru Kitayama

Hi Fuad,

On Wed, Oct 07, 2026 at 11:29:11PM +0200, Fuad Tabba wrote:
> Hi Gustavo,
> 
> On Wed, 07 Oct 2026 17:56:30 +0100, Gustavo Romero <gromero@redhat.com> 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.
>          */

Yep, that's good.

However, I missed one thing that I think is important to mention and that
confused me after a while not looking at these TCG tests:

RAM_BASE is precisely defined as 1 << 30 because the way we link the tests based
on the memory layout defined in tests/tcg/aarch64/system/kernel.ld, right?

That's the bit that also confused me, like, how Stage 2 entries:

s2_l2[RAM_BASE >> 29] = RAM_BASE | S2_BLOCK;
s2_l2_high[RAM_BASE >> 29] = RAM_BASE | S2_BLOCK 

are taking us back to s1_l0. How s1_l0 ends up related to RAM_BASE :)

Hence, I think it's worth mentioning that relationship also in this text. How
about something like:

"RAM_BASE is chosen to match the memory layout defined in kernel.ld. As a
result, s1_l0 resides at an offset from RAM_BASE after linking."

Wdyt?


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

np! Sure ;)


Cheers,
Gustavo



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout
  2026-10-08 16:40       ` Gustavo Romero
@ 2026-10-08 18:01         ` Fuad Tabba
  2026-10-09  8:13           ` Gustavo Romero
  0 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2026-10-08 18:01 UTC (permalink / raw)
  To: Gustavo Romero
  Cc: Peter Maydell, qemu-arm, qemu-devel, Richard Henderson,
	Will Deacon, Itaru Kitayama

Hi Gustavo,

On Thu, 08 Oct 2026 17:40:27 +0100, Gustavo Romero <gromero@redhat.com> wrote:
[...]
> RAM_BASE is precisely defined as 1 << 30 because the way we link the tests based
> on the memory layout defined in tests/tcg/aarch64/system/kernel.ld, right?

Yes. The tables are static, so they're linked a few MB above RAM_BASE,
inside the blocks both stages map at RAM_BASE (and stage 2 again at
HIGH_IPA | RAM_BASE).

> That's the bit that also confused me, like, how Stage 2 entries:
>
> s2_l2[RAM_BASE >> 29] = RAM_BASE | S2_BLOCK;
> s2_l2_high[RAM_BASE >> 29] = RAM_BASE | S2_BLOCK
>
> are taking us back to s1_l0. How s1_l0 ends up related to RAM_BASE :)
>
> Hence, I think it's worth mentioning that relationship also in this text. How
> about something like:
>
> "RAM_BASE is chosen to match the memory layout defined in kernel.ld. As a
> result, s1_l0 resides at an offset from RAM_BASE after linking."
>
> Wdyt?

Agree. The stage 1 and VTCR.DS stage 2 mappings depend on it too, so
I've put it on the RAM_BASE define for v3 rather than in that comment:

        /*
         * kernel.ld links the test at 1GB, so the tables below lie within the
         * blocks mapped at RAM_BASE.
         */
        #define RAM_BASE    0x40000000UL

Cheers,
/fuad


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout
  2026-10-08 18:01         ` Fuad Tabba
@ 2026-10-09  8:13           ` Gustavo Romero
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Romero @ 2026-10-09  8:13 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Peter Maydell, qemu-arm, qemu-devel, Richard Henderson,
	Will Deacon, Itaru Kitayama

Hi Faud,

On Thu, Oct 08, 2026 at 07:01:41PM +0100, Fuad Tabba wrote:
> Hi Gustavo,
> 
> On Thu, 08 Oct 2026 17:40:27 +0100, Gustavo Romero <gromero@redhat.com> wrote:
> [...]
> > RAM_BASE is precisely defined as 1 << 30 because the way we link the tests based
> > on the memory layout defined in tests/tcg/aarch64/system/kernel.ld, right?
> 
> Yes. The tables are static, so they're linked a few MB above RAM_BASE,
> inside the blocks both stages map at RAM_BASE (and stage 2 again at
> HIGH_IPA | RAM_BASE).
> 
> > That's the bit that also confused me, like, how Stage 2 entries:
> >
> > s2_l2[RAM_BASE >> 29] = RAM_BASE | S2_BLOCK;
> > s2_l2_high[RAM_BASE >> 29] = RAM_BASE | S2_BLOCK
> >
> > are taking us back to s1_l0. How s1_l0 ends up related to RAM_BASE :)
> >
> > Hence, I think it's worth mentioning that relationship also in this text. How
> > about something like:
> >
> > "RAM_BASE is chosen to match the memory layout defined in kernel.ld. As a
> > result, s1_l0 resides at an offset from RAM_BASE after linking."
> >
> > Wdyt?
> 
> Agree. The stage 1 and VTCR.DS stage 2 mappings depend on it too, so
> I've put it on the RAM_BASE define for v3 rather than in that comment:
> 
>         /*
>          * kernel.ld links the test at 1GB, so the tables below lie within the
>          * blocks mapped at RAM_BASE.
>          */
>         #define RAM_BASE    0x40000000UL

Excellent. Thank you.


Cheers,
Gustavo



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-10-09  8:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-07  8:00 [PATCH v2 0/3] target/arm: Fix the TTBR table base address in its 52-bit layout Fuad Tabba
2026-10-07  8:00 ` [PATCH v2 1/3] target/arm: Clear TTBR[5:0] from a 52-bit table base address Fuad Tabba
2026-10-07 16:55   ` Gustavo Romero
2026-10-07  8:00 ` [PATCH v2 2/3] target/arm: Use the 52-bit TTBR base address layout when TCR.DS is set Fuad Tabba
2026-10-07 16:55   ` Gustavo Romero
2026-10-07  8:00 ` [PATCH v2 3/3] tests/tcg/aarch64: Add a test for the 52-bit TTBR base address layout Fuad Tabba
2026-10-07 16:56   ` Gustavo Romero
2026-10-07 21:29     ` Fuad Tabba
2026-10-08 16:40       ` Gustavo Romero
2026-10-08 18:01         ` Fuad Tabba
2026-10-09  8:13           ` Gustavo Romero

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.