[PATCH 0/9] single-binary: Restrict scope of TARGET_PAGE_BITS_MIN

[PATCH 0/9] single-binary: Restrict scope of TARGET_PAGE_BITS_MIN

[Richard Henderson]

With this, TARGET_PAGE_BITS_MIN no longer exists outside of
page-vary-target.c, as that's the only place that needs the
information.

Based-on: 20250318213209.2579218-1-richard.henderson@linaro.org
("[PATCH v2 00/42] accel/tcg, codebase: Build once patches")
Based-on: 20250325224403.4011975-1-richard.henderson@linaro.org
("[PATCH v2 00/11] target/avr: Increase page size")
Based-on: 20250328175526.368121-1-richard.henderson@linaro.org
("[PATCH 0/3] target/mips: Revert TARGET_PAGE_BITS_VARY and bug fixes")

Which is a lot, so for avoidance of doubt:
https://gitlab.com/rth7680/qemu/-/commit/c8b593f1a907794b5767274cb3f5c70985638397

r~

Richard Henderson (9):
  include/exec: Move tb_{,set_}page_addr[01] to translation-block.h
  accel/tcg: Move get_page_addr_code* declarations
  accel/tcg: Remove page_protect
  accel/tcg: Remove cpu-all.h, exec-all.h from tb-internal.h
  accel/tcg: Build translator.c twice
  accel/tcg: Split out tlb-bounds.h
  include/exec: Redefine tlb-flags with absolute values
  page-vary: Move and rename qemu_target_page_bits_min
  page-vary: Restrict scope of TARGET_PAGE_BITS_MIN

 accel/tcg/internal-common.h      | 34 +++++++++++++
 accel/tcg/tb-internal.h          | 38 +--------------
 accel/tcg/tlb-bounds.h           | 32 ++++++++++++
 include/exec/cpu-defs.h          | 10 +---
 include/exec/exec-all.h          | 83 --------------------------------
 include/exec/page-vary.h         |  9 ++++
 include/exec/poison.h            |  1 +
 include/exec/target_page.h       |  2 -
 include/exec/tlb-flags.h         | 68 ++++++++++++--------------
 include/exec/translation-block.h | 50 +++++++++++++++++++
 include/qemu/osdep.h             |  6 +++
 include/user/page-protection.h   |  1 -
 target/alpha/cpu-param.h         |  1 -
 target/arm/cpu-param.h           |  3 +-
 target/ppc/cpu-param.h           |  1 -
 accel/tcg/cputlb.c               |  2 +
 accel/tcg/tb-maint.c             |  1 +
 accel/tcg/translate-all.c        |  1 +
 accel/tcg/translator.c           | 15 +++---
 accel/tcg/user-exec.c            |  2 +-
 migration/savevm.c               |  6 +--
 page-target.c                    |  5 --
 page-vary-target.c               | 48 ++++++++++++++++--
 accel/tcg/meson.build            |  2 +-
 24 files changed, 230 insertions(+), 191 deletions(-)
 create mode 100644 accel/tcg/tlb-bounds.h

-- 
2.43.0

[PATCH 1/9] include/exec: Move tb_{, set_}page_addr[01] to translation-block.h

[Richard Henderson]

Move the accessor functions for TranslationBlock
into the header related to the structure.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/exec-all.h          | 49 -------------------------------
 include/exec/translation-block.h | 50 ++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 19b0eda44a..fcad3446fe 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -123,55 +123,6 @@ int probe_access_full_mmu(CPUArchState *env, vaddr addr, int size,
 #endif /* !CONFIG_USER_ONLY */
 #endif /* CONFIG_TCG */
 
-static inline tb_page_addr_t tb_page_addr0(const TranslationBlock *tb)
-{
-#ifdef CONFIG_USER_ONLY
-    return tb->itree.start;
-#else
-    return tb->page_addr[0];
-#endif
-}
-
-static inline tb_page_addr_t tb_page_addr1(const TranslationBlock *tb)
-{
-#ifdef CONFIG_USER_ONLY
-    tb_page_addr_t next = tb->itree.last & TARGET_PAGE_MASK;
-    return next == (tb->itree.start & TARGET_PAGE_MASK) ? -1 : next;
-#else
-    return tb->page_addr[1];
-#endif
-}
-
-static inline void tb_set_page_addr0(TranslationBlock *tb,
-                                     tb_page_addr_t addr)
-{
-#ifdef CONFIG_USER_ONLY
-    tb->itree.start = addr;
-    /*
-     * To begin, we record an interval of one byte.  When the translation
-     * loop encounters a second page, the interval will be extended to
-     * include the first byte of the second page, which is sufficient to
-     * allow tb_page_addr1() above to work properly.  The final corrected
-     * interval will be set by tb_page_add() from tb->size before the
-     * node is added to the interval tree.
-     */
-    tb->itree.last = addr;
-#else
-    tb->page_addr[0] = addr;
-#endif
-}
-
-static inline void tb_set_page_addr1(TranslationBlock *tb,
-                                     tb_page_addr_t addr)
-{
-#ifdef CONFIG_USER_ONLY
-    /* Extend the interval to the first byte of the second page.  See above. */
-    tb->itree.last = addr;
-#else
-    tb->page_addr[1] = addr;
-#endif
-}
-
 /* TranslationBlock invalidate API */
 void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
 void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t last);
diff --git a/include/exec/translation-block.h b/include/exec/translation-block.h
index 3c69bc71a9..8b8e730561 100644
--- a/include/exec/translation-block.h
+++ b/include/exec/translation-block.h
@@ -13,6 +13,7 @@
 #include "exec/vaddr.h"
 #ifdef CONFIG_USER_ONLY
 #include "qemu/interval-tree.h"
+#include "exec/target_page.h"
 #endif
 
 /*
@@ -157,4 +158,53 @@ static inline uint32_t tb_cflags(const TranslationBlock *tb)
 bool tcg_cflags_has(CPUState *cpu, uint32_t flags);
 void tcg_cflags_set(CPUState *cpu, uint32_t flags);
 
+static inline tb_page_addr_t tb_page_addr0(const TranslationBlock *tb)
+{
+#ifdef CONFIG_USER_ONLY
+    return tb->itree.start;
+#else
+    return tb->page_addr[0];
+#endif
+}
+
+static inline tb_page_addr_t tb_page_addr1(const TranslationBlock *tb)
+{
+#ifdef CONFIG_USER_ONLY
+    tb_page_addr_t next = tb->itree.last & TARGET_PAGE_MASK;
+    return next == (tb->itree.start & TARGET_PAGE_MASK) ? -1 : next;
+#else
+    return tb->page_addr[1];
+#endif
+}
+
+static inline void tb_set_page_addr0(TranslationBlock *tb,
+                                     tb_page_addr_t addr)
+{
+#ifdef CONFIG_USER_ONLY
+    tb->itree.start = addr;
+    /*
+     * To begin, we record an interval of one byte.  When the translation
+     * loop encounters a second page, the interval will be extended to
+     * include the first byte of the second page, which is sufficient to
+     * allow tb_page_addr1() above to work properly.  The final corrected
+     * interval will be set by tb_page_add() from tb->size before the
+     * node is added to the interval tree.
+     */
+    tb->itree.last = addr;
+#else
+    tb->page_addr[0] = addr;
+#endif
+}
+
+static inline void tb_set_page_addr1(TranslationBlock *tb,
+                                     tb_page_addr_t addr)
+{
+#ifdef CONFIG_USER_ONLY
+    /* Extend the interval to the first byte of the second page.  See above. */
+    tb->itree.last = addr;
+#else
+    tb->page_addr[1] = addr;
+#endif
+}
+
 #endif /* EXEC_TRANSLATION_BLOCK_H */
-- 
2.43.0

[PATCH 2/9] accel/tcg: Move get_page_addr_code* declarations

[Richard Henderson]

Move the declarations from exec/exec-all.h to the
private accel/tcg/internal-common.h.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/internal-common.h | 34 ++++++++++++++++++++++++++++++++++
 include/exec/exec-all.h     | 34 ----------------------------------
 accel/tcg/translator.c      |  1 +
 3 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
index 9b6ab3a8cc..2f00560d10 100644
--- a/accel/tcg/internal-common.h
+++ b/accel/tcg/internal-common.h
@@ -74,4 +74,38 @@ uint32_t curr_cflags(CPUState *cpu);
 
 void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);
 
+/**
+ * get_page_addr_code_hostp()
+ * @env: CPUArchState
+ * @addr: guest virtual address of guest code
+ *
+ * See get_page_addr_code() (full-system version) for documentation on the
+ * return value.
+ *
+ * Sets *@hostp (when @hostp is non-NULL) as follows.
+ * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
+ * to the host address where @addr's content is kept.
+ *
+ * Note: this function can trigger an exception.
+ */
+tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
+                                        void **hostp);
+
+/**
+ * get_page_addr_code()
+ * @env: CPUArchState
+ * @addr: guest virtual address of guest code
+ *
+ * If we cannot translate and execute from the entire RAM page, or if
+ * the region is not backed by RAM, returns -1. Otherwise, returns the
+ * ram_addr_t corresponding to the guest code at @addr.
+ *
+ * Note: this function can trigger an exception.
+ */
+static inline tb_page_addr_t get_page_addr_code(CPUArchState *env,
+                                                vaddr addr)
+{
+    return get_page_addr_code_hostp(env, addr, NULL);
+}
+
 #endif
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index fcad3446fe..f52a680f42 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -143,40 +143,6 @@ struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
                                              hwaddr index, MemTxAttrs attrs);
 #endif
 
-/**
- * get_page_addr_code_hostp()
- * @env: CPUArchState
- * @addr: guest virtual address of guest code
- *
- * See get_page_addr_code() (full-system version) for documentation on the
- * return value.
- *
- * Sets *@hostp (when @hostp is non-NULL) as follows.
- * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
- * to the host address where @addr's content is kept.
- *
- * Note: this function can trigger an exception.
- */
-tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
-                                        void **hostp);
-
-/**
- * get_page_addr_code()
- * @env: CPUArchState
- * @addr: guest virtual address of guest code
- *
- * If we cannot translate and execute from the entire RAM page, or if
- * the region is not backed by RAM, returns -1. Otherwise, returns the
- * ram_addr_t corresponding to the guest code at @addr.
- *
- * Note: this function can trigger an exception.
- */
-static inline tb_page_addr_t get_page_addr_code(CPUArchState *env,
-                                                vaddr addr)
-{
-    return get_page_addr_code_hostp(env, addr, NULL);
-}
-
 #if !defined(CONFIG_USER_ONLY)
 
 MemoryRegionSection *
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 7ef04fc597..307a513487 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -17,6 +17,7 @@
 #include "exec/translator.h"
 #include "exec/plugin-gen.h"
 #include "tcg/tcg-op-common.h"
+#include "internal-common.h"
 #include "internal-target.h"
 #include "disas/disas.h"
 #include "tb-internal.h"
-- 
2.43.0

[PATCH 3/9] accel/tcg: Remove page_protect

[Richard Henderson]

Merge the user-only page_protect function with the user-only
implementation of tb_lock_page0.  This avoids pulling
page-protection.h into tb-internal.h.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/tb-internal.h        | 11 +++--------
 include/user/page-protection.h |  1 -
 accel/tcg/user-exec.c          |  2 +-
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/accel/tcg/tb-internal.h b/accel/tcg/tb-internal.h
index 68aa8d17f4..f7c2073e29 100644
--- a/accel/tcg/tb-internal.h
+++ b/accel/tcg/tb-internal.h
@@ -51,28 +51,23 @@
 
 #endif /* CONFIG_SOFTMMU */
 
+void tb_lock_page0(tb_page_addr_t);
+
 #ifdef CONFIG_USER_ONLY
-#include "user/page-protection.h"
 /*
  * For user-only, page_protect sets the page read-only.
  * Since most execution is already on read-only pages, and we'd need to
  * account for other TBs on the same page, defer undoing any page protection
  * until we receive the write fault.
  */
-static inline void tb_lock_page0(tb_page_addr_t p0)
-{
-    page_protect(p0);
-}
-
 static inline void tb_lock_page1(tb_page_addr_t p0, tb_page_addr_t p1)
 {
-    page_protect(p1);
+    tb_lock_page0(p1);
 }
 
 static inline void tb_unlock_page1(tb_page_addr_t p0, tb_page_addr_t p1) { }
 static inline void tb_unlock_pages(TranslationBlock *tb) { }
 #else
-void tb_lock_page0(tb_page_addr_t);
 void tb_lock_page1(tb_page_addr_t, tb_page_addr_t);
 void tb_unlock_page1(tb_page_addr_t, tb_page_addr_t);
 void tb_unlock_pages(TranslationBlock *);
diff --git a/include/user/page-protection.h b/include/user/page-protection.h
index 51daa18648..d5c8748d49 100644
--- a/include/user/page-protection.h
+++ b/include/user/page-protection.h
@@ -16,7 +16,6 @@
 #include "exec/target_long.h"
 #include "exec/translation-block.h"
 
-void page_protect(tb_page_addr_t page_addr);
 int page_unprotect(tb_page_addr_t address, uintptr_t pc);
 
 int page_get_flags(target_ulong address);
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 667c5e0354..72a9809c2d 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -657,7 +657,7 @@ target_ulong page_find_range_empty(target_ulong min, target_ulong max,
     }
 }
 
-void page_protect(tb_page_addr_t address)
+void tb_lock_page0(tb_page_addr_t address)
 {
     PageFlagsNode *p;
     target_ulong start, last;
-- 
2.43.0

[PATCH 4/9] accel/tcg: Remove cpu-all.h, exec-all.h from tb-internal.h

[Richard Henderson]

Not used by tb-internal.h, but add an include for
target_page.h in tb-maint.c.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/tb-internal.h | 2 --
 accel/tcg/tb-maint.c    | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/accel/tcg/tb-internal.h b/accel/tcg/tb-internal.h
index f7c2073e29..f9a06bcbab 100644
--- a/accel/tcg/tb-internal.h
+++ b/accel/tcg/tb-internal.h
@@ -9,8 +9,6 @@
 #ifndef ACCEL_TCG_TB_INTERNAL_TARGET_H
 #define ACCEL_TCG_TB_INTERNAL_TARGET_H
 
-#include "exec/cpu-all.h"
-#include "exec/exec-all.h"
 #include "exec/translation-block.h"
 
 /*
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index d5899ad047..df3438e190 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -26,6 +26,7 @@
 #include "exec/page-protection.h"
 #include "exec/mmap-lock.h"
 #include "exec/tb-flush.h"
+#include "exec/target_page.h"
 #include "tb-internal.h"
 #include "system/tcg.h"
 #include "tcg/tcg.h"
-- 
2.43.0

[PATCH 5/9] accel/tcg: Build translator.c twice

[Richard Henderson]

Drop some unnecessary includes.  Change the offsetof expressions
to be based on CPUState instead of ArchCPU.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/translator.c | 14 ++++++--------
 accel/tcg/meson.build  |  2 +-
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 307a513487..36a6a9e040 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -11,14 +11,13 @@
 #include "qemu/bswap.h"
 #include "qemu/log.h"
 #include "qemu/error-report.h"
-#include "exec/exec-all.h"
 #include "exec/cpu-ldst-common.h"
 #include "exec/cpu-mmu-index.h"
+#include "exec/target_page.h"
 #include "exec/translator.h"
 #include "exec/plugin-gen.h"
 #include "tcg/tcg-op-common.h"
 #include "internal-common.h"
-#include "internal-target.h"
 #include "disas/disas.h"
 #include "tb-internal.h"
 
@@ -26,8 +25,7 @@ static void set_can_do_io(DisasContextBase *db, bool val)
 {
     QEMU_BUILD_BUG_ON(sizeof_field(CPUState, neg.can_do_io) != 1);
     tcg_gen_st8_i32(tcg_constant_i32(val), tcg_env,
-                    offsetof(ArchCPU, parent_obj.neg.can_do_io) -
-                    offsetof(ArchCPU, env));
+                    offsetof(CPUState, neg.can_do_io) - sizeof(CPUState));
 }
 
 bool translator_io_start(DisasContextBase *db)
@@ -50,8 +48,8 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
     if ((cflags & CF_USE_ICOUNT) || !(cflags & CF_NOIRQ)) {
         count = tcg_temp_new_i32();
         tcg_gen_ld_i32(count, tcg_env,
-                       offsetof(ArchCPU, parent_obj.neg.icount_decr.u32)
-                       - offsetof(ArchCPU, env));
+                       offsetof(CPUState, neg.icount_decr.u32) -
+                       sizeof(CPUState));
     }
 
     if (cflags & CF_USE_ICOUNT) {
@@ -80,8 +78,8 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
 
     if (cflags & CF_USE_ICOUNT) {
         tcg_gen_st16_i32(count, tcg_env,
-                         offsetof(ArchCPU, parent_obj.neg.icount_decr.u16.low)
-                         - offsetof(ArchCPU, env));
+                         offsetof(CPUState, neg.icount_decr.u16.low) -
+                         sizeof(CPUState));
     }
 
     return icount_start_insn;
diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
index 72d4acfe5e..047afa49a2 100644
--- a/accel/tcg/meson.build
+++ b/accel/tcg/meson.build
@@ -8,6 +8,7 @@ tcg_ss.add(files(
   'cpu-exec-common.c',
   'tcg-runtime.c',
   'tcg-runtime-gvec.c',
+  'translator.c',
 ))
 if get_option('plugins')
   tcg_ss.add(files('plugin-gen.c'))
@@ -22,7 +23,6 @@ tcg_specific_ss.add(files(
   'cpu-exec.c',
   'tb-maint.c',
   'translate-all.c',
-  'translator.c',
 ))
 tcg_specific_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
 specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_specific_ss)
-- 
2.43.0

[PATCH 6/9] accel/tcg: Split out tlb-bounds.h

[Richard Henderson]

The CPU_TLB_DYN_{MIN,MAX}_BITS definitions are not required
outside of cputlb.c and translate-all.c.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/tb-internal.h   | 27 ---------------------------
 accel/tcg/tlb-bounds.h    | 32 ++++++++++++++++++++++++++++++++
 accel/tcg/cputlb.c        |  1 +
 accel/tcg/translate-all.c |  1 +
 4 files changed, 34 insertions(+), 27 deletions(-)
 create mode 100644 accel/tcg/tlb-bounds.h

diff --git a/accel/tcg/tb-internal.h b/accel/tcg/tb-internal.h
index f9a06bcbab..08538e2896 100644
--- a/accel/tcg/tb-internal.h
+++ b/accel/tcg/tb-internal.h
@@ -22,33 +22,6 @@
  */
 #define GETPC_ADJ   2
 
-#ifdef CONFIG_SOFTMMU
-
-#define CPU_TLB_DYN_MIN_BITS 6
-#define CPU_TLB_DYN_DEFAULT_BITS 8
-
-# if HOST_LONG_BITS == 32
-/* Make sure we do not require a double-word shift for the TLB load */
-#  define CPU_TLB_DYN_MAX_BITS (32 - TARGET_PAGE_BITS)
-# else /* HOST_LONG_BITS == 64 */
-/*
- * Assuming TARGET_PAGE_BITS==12, with 2**22 entries we can cover 2**(22+12) ==
- * 2**34 == 16G of address space. This is roughly what one would expect a
- * TLB to cover in a modern (as of 2018) x86_64 CPU. For instance, Intel
- * Skylake's Level-2 STLB has 16 1G entries.
- * Also, make sure we do not size the TLB past the guest's address space.
- */
-#  ifdef TARGET_PAGE_BITS_VARY
-#   define CPU_TLB_DYN_MAX_BITS                                  \
-    MIN(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS)
-#  else
-#   define CPU_TLB_DYN_MAX_BITS                                  \
-    MIN_CONST(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS)
-#  endif
-# endif
-
-#endif /* CONFIG_SOFTMMU */
-
 void tb_lock_page0(tb_page_addr_t);
 
 #ifdef CONFIG_USER_ONLY
diff --git a/accel/tcg/tlb-bounds.h b/accel/tcg/tlb-bounds.h
new file mode 100644
index 0000000000..efd34d4793
--- /dev/null
+++ b/accel/tcg/tlb-bounds.h
@@ -0,0 +1,32 @@
+/*
+ * softmmu size bounds
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef ACCEL_TCG_TLB_BOUNDS_H
+#define ACCEL_TCG_TLB_BOUNDS_H
+
+#define CPU_TLB_DYN_MIN_BITS 6
+#define CPU_TLB_DYN_DEFAULT_BITS 8
+
+# if HOST_LONG_BITS == 32
+/* Make sure we do not require a double-word shift for the TLB load */
+#  define CPU_TLB_DYN_MAX_BITS (32 - TARGET_PAGE_BITS)
+# else /* HOST_LONG_BITS == 64 */
+/*
+ * Assuming TARGET_PAGE_BITS==12, with 2**22 entries we can cover 2**(22+12) ==
+ * 2**34 == 16G of address space. This is roughly what one would expect a
+ * TLB to cover in a modern (as of 2018) x86_64 CPU. For instance, Intel
+ * Skylake's Level-2 STLB has 16 1G entries.
+ * Also, make sure we do not size the TLB past the guest's address space.
+ */
+#  ifdef TARGET_PAGE_BITS_VARY
+#   define CPU_TLB_DYN_MAX_BITS                                  \
+    MIN(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS)
+#  else
+#   define CPU_TLB_DYN_MAX_BITS                                  \
+    MIN_CONST(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS)
+#  endif
+# endif
+
+#endif /* ACCEL_TCG_TLB_BOUNDS_H */
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 28c47d4872..a717f357d5 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -41,6 +41,7 @@
 #include "trace.h"
 #include "tb-hash.h"
 #include "tb-internal.h"
+#include "tlb-bounds.h"
 #include "internal-common.h"
 #include "internal-target.h"
 #ifdef CONFIG_PLUGIN
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index bb161ae61a..87fb6c51d3 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -47,6 +47,7 @@
 #include "exec/page-protection.h"
 #include "exec/mmap-lock.h"
 #include "tb-internal.h"
+#include "tlb-bounds.h"
 #include "exec/translator.h"
 #include "exec/tb-flush.h"
 #include "qemu/bitmap.h"
-- 
2.43.0

[PATCH 7/9] include/exec: Redefine tlb-flags with absolute values

[Richard Henderson]

Don't base the values on TARGET_PAGE_BITS_MIN, but do verify
that TLB_FLAGS_MASK does not overlap minimum page size.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/tlb-flags.h | 68 +++++++++++++++++++---------------------
 accel/tcg/cputlb.c       |  2 ++
 2 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/include/exec/tlb-flags.h b/include/exec/tlb-flags.h
index 54a6bae768..357e79095c 100644
--- a/include/exec/tlb-flags.h
+++ b/include/exec/tlb-flags.h
@@ -19,54 +19,29 @@
 #ifndef TLB_FLAGS_H
 #define TLB_FLAGS_H
 
-#include "exec/cpu-defs.h"
+/*
+ * Flags returned for lookup of a TLB virtual address.
+ */
 
 #ifdef CONFIG_USER_ONLY
 
 /*
- * Allow some level of source compatibility with softmmu.  We do not
- * support any of the more exotic features, so only invalid pages may
- * be signaled by probe_access_flags().
+ * Allow some level of source compatibility with softmmu.
+ * Invalid is set when the page does not have requested permissions.
+ * MMIO is set when we want the target helper to use the functional
+ * interface for load/store so that plugins see the access.
  */
-#define TLB_INVALID_MASK    (1 << (TARGET_PAGE_BITS_MIN - 1))
-#define TLB_MMIO            (1 << (TARGET_PAGE_BITS_MIN - 2))
-#define TLB_WATCHPOINT      0
+#define TLB_INVALID_MASK     (1 << 0)
+#define TLB_MMIO             (1 << 1)
+#define TLB_WATCHPOINT       0
 
 #else
 
-/*
- * Flags stored in the low bits of the TLB virtual address.
- * These are defined so that fast path ram access is all zeros.
- * The flags all must be between TARGET_PAGE_BITS and
- * maximum address alignment bit.
- *
- * Use TARGET_PAGE_BITS_MIN so that these bits are constant
- * when TARGET_PAGE_BITS_VARY is in effect.
- *
- * The count, if not the placement of these bits is known
- * to tcg/tcg-op-ldst.c, check_max_alignment().
- */
-/* Zero if TLB entry is valid.  */
-#define TLB_INVALID_MASK    (1 << (TARGET_PAGE_BITS_MIN - 1))
-/*
- * Set if TLB entry references a clean RAM page.  The iotlb entry will
- * contain the page physical address.
- */
-#define TLB_NOTDIRTY        (1 << (TARGET_PAGE_BITS_MIN - 2))
-/* Set if the slow path must be used; more flags in CPUTLBEntryFull. */
-#define TLB_FORCE_SLOW      (1 << (TARGET_PAGE_BITS_MIN - 3))
-
-/*
- * Use this mask to check interception with an alignment mask
- * in a TCG backend.
- */
-#define TLB_FLAGS_MASK \
-    (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_FORCE_SLOW)
-
 /*
  * Flags stored in CPUTLBEntryFull.slow_flags[x].
  * TLB_FORCE_SLOW must be set in CPUTLBEntry.addr_idx[x].
  */
+
 /* Set if TLB entry requires byte swap.  */
 #define TLB_BSWAP            (1 << 0)
 /* Set if TLB entry contains a watchpoint.  */
@@ -82,6 +57,27 @@
     (TLB_BSWAP | TLB_WATCHPOINT | TLB_CHECK_ALIGNED | \
      TLB_DISCARD_WRITE | TLB_MMIO)
 
+/*
+ * Flags stored in CPUTLBEntry.addr_idx[x].
+ * These must be above the largest alignment (64 bytes),
+ * and below the smallest page size (1024 bytes).
+ * This leaves bits [9:6] available for use.
+ */
+
+/* Zero if TLB entry is valid.  */
+#define TLB_INVALID_MASK     (1 << 6)
+/* Set if TLB entry references a clean RAM page.  */
+#define TLB_NOTDIRTY         (1 << 7)
+/* Set if the slow path must be used; more flags in CPUTLBEntryFull. */
+#define TLB_FORCE_SLOW       (1 << 8)
+
+/*
+ * Use this mask to check interception with an alignment mask
+ * in a TCG backend.
+ */
+#define TLB_FLAGS_MASK \
+    (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_FORCE_SLOW)
+
 /* The two sets of flags must not overlap. */
 QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & TLB_SLOW_FLAGS_MASK);
 
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index a717f357d5..39314e86f3 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -49,6 +49,8 @@
 #endif
 #include "tcg/tcg-ldst.h"
 
+QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & ((1u < TARGET_PAGE_BITS_MIN) - 1));
+
 /* DEBUG defines, enable DEBUG_TLB_LOG to log to the CPU_LOG_MMU target */
 /* #define DEBUG_TLB */
 /* #define DEBUG_TLB_LOG */
-- 
2.43.0

[PATCH 8/9] page-vary: Move and rename qemu_target_page_bits_min

[Richard Henderson]

Rename to migration_legacy_page_bits, to make it clear that
we cannot change the value without causing a migration break.
Move to page-vary.h and page-vary-target.c.
Define via TARGET_PAGE_BITS if not TARGET_PAGE_BITS_VARY.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/page-vary.h   | 9 +++++++++
 include/exec/target_page.h | 1 -
 migration/savevm.c         | 6 +++---
 page-target.c              | 5 -----
 page-vary-target.c         | 9 +++++++++
 5 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/include/exec/page-vary.h b/include/exec/page-vary.h
index 54ddde308a..101c25911c 100644
--- a/include/exec/page-vary.h
+++ b/include/exec/page-vary.h
@@ -49,4 +49,13 @@ bool set_preferred_target_page_bits(int bits);
  */
 void finalize_target_page_bits(void);
 
+/**
+ * migration_legacy_page_bits
+ *
+ * For migration compatibility with qemu v2.9, prior to the introduction
+ * of the configuration/target-page-bits section, return the value of
+ * TARGET_PAGE_BITS that the target had then.
+ */
+int migration_legacy_page_bits(void);
+
 #endif /* EXEC_PAGE_VARY_H */
diff --git a/include/exec/target_page.h b/include/exec/target_page.h
index 8e89e5cbe6..e4bd7f7767 100644
--- a/include/exec/target_page.h
+++ b/include/exec/target_page.h
@@ -63,7 +63,6 @@ static inline int qemu_target_page_bits(void)
     return TARGET_PAGE_BITS;
 }
 
-int qemu_target_page_bits_min(void);
 size_t qemu_target_pages_to_MiB(size_t pages);
 
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index c33200a33f..0c12e373b4 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -50,6 +50,7 @@
 #include "system/cpus.h"
 #include "system/memory.h"
 #include "exec/target_page.h"
+#include "exec/page-vary.h"
 #include "trace.h"
 #include "qemu/iov.h"
 #include "qemu/job.h"
@@ -339,7 +340,7 @@ static int configuration_pre_load(void *opaque)
      * predates the variable-target-page-bits support and is using the
      * minimum possible value for this CPU.
      */
-    state->target_page_bits = qemu_target_page_bits_min();
+    state->target_page_bits = migration_legacy_page_bits();
     return 0;
 }
 
@@ -462,8 +463,7 @@ static const VMStateInfo vmstate_info_capability = {
  */
 static bool vmstate_target_page_bits_needed(void *opaque)
 {
-    return qemu_target_page_bits()
-        > qemu_target_page_bits_min();
+    return qemu_target_page_bits() > migration_legacy_page_bits();
 }
 
 static const VMStateDescription vmstate_target_page_bits = {
diff --git a/page-target.c b/page-target.c
index 321e43d06f..8fcd5443b5 100644
--- a/page-target.c
+++ b/page-target.c
@@ -9,11 +9,6 @@
 #include "qemu/osdep.h"
 #include "exec/target_page.h"
 
-int qemu_target_page_bits_min(void)
-{
-    return TARGET_PAGE_BITS_MIN;
-}
-
 /* Convert target pages to MiB (2**20). */
 size_t qemu_target_pages_to_MiB(size_t pages)
 {
diff --git a/page-vary-target.c b/page-vary-target.c
index 84ddeb7c26..6251d948cf 100644
--- a/page-vary-target.c
+++ b/page-vary-target.c
@@ -23,6 +23,15 @@
 #include "exec/page-vary.h"
 #include "exec/target_page.h"
 
+int migration_legacy_page_bits(void)
+{
+#ifdef TARGET_PAGE_BITS_VARY
+    return TARGET_PAGE_BITS_MIN;
+#else
+    return TARGET_PAGE_BITS;
+#endif
+}
+
 bool set_preferred_target_page_bits(int bits)
 {
 #ifdef TARGET_PAGE_BITS_VARY
-- 
2.43.0

[PATCH 9/9] page-vary: Restrict scope of TARGET_PAGE_BITS_MIN

[Richard Henderson]

The only place we really need to know the minimum is within
page-vary-target.c.  Rename the target/arm TARGET_PAGE_BITS_MIN
to TARGE_PAGE_BITS_LEGACY to emphasize what it really means.
Move the assertions related to minimum page size as well.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu-defs.h    | 10 ++--------
 include/exec/poison.h      |  1 +
 include/exec/target_page.h |  1 -
 include/qemu/osdep.h       |  6 ++++++
 target/alpha/cpu-param.h   |  1 -
 target/arm/cpu-param.h     |  3 +--
 target/ppc/cpu-param.h     |  1 -
 accel/tcg/cputlb.c         |  1 -
 page-vary-target.c         | 39 +++++++++++++++++++++++++++++++++++---
 9 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 9f955f53fd..e01acb7c90 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -34,14 +34,8 @@
 #ifndef TARGET_VIRT_ADDR_SPACE_BITS
 # error TARGET_VIRT_ADDR_SPACE_BITS must be defined in cpu-param.h
 #endif
-#ifndef TARGET_PAGE_BITS
-# ifdef TARGET_PAGE_BITS_VARY
-#  ifndef TARGET_PAGE_BITS_MIN
-#   error TARGET_PAGE_BITS_MIN must be defined in cpu-param.h
-#  endif
-# else
-#  error TARGET_PAGE_BITS must be defined in cpu-param.h
-# endif
+#if !defined(TARGET_PAGE_BITS) && !defined(TARGET_PAGE_BITS_VARY)
+# error TARGET_PAGE_BITS must be defined in cpu-param.h
 #endif
 
 #include "exec/target_long.h"
diff --git a/include/exec/poison.h b/include/exec/poison.h
index 4180a5a489..c4f7ee22bf 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -44,6 +44,7 @@
 #pragma GCC poison TARGET_FMT_lu
 
 #pragma GCC poison TARGET_PHYS_ADDR_SPACE_BITS
+#pragma GCC poison TARGET_PAGE_BITS_LEGACY
 
 #pragma GCC poison CONFIG_ALPHA_DIS
 #pragma GCC poison CONFIG_HPPA_DIS
diff --git a/include/exec/target_page.h b/include/exec/target_page.h
index e4bd7f7767..ca0ebbc8bb 100644
--- a/include/exec/target_page.h
+++ b/include/exec/target_page.h
@@ -41,7 +41,6 @@ extern const TargetPageBits target_page;
 # endif
 # define TARGET_PAGE_SIZE    (-(int)TARGET_PAGE_MASK)
 #else
-# define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
 # define TARGET_PAGE_SIZE    (1 << TARGET_PAGE_BITS)
 # define TARGET_PAGE_MASK    ((TARGET_PAGE_TYPE)-1 << TARGET_PAGE_BITS)
 #endif
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 4397a90680..321a52d7f0 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -50,6 +50,12 @@
  */
 #pragma GCC poison TARGET_WORDS_BIGENDIAN
 
+/*
+ * TARGET_PAGE_BITS_MIN was repaced by TARGET_PAGE_BITS_LEGACY
+ * for system mode.  Prevent it from creeping back in.
+ */
+#pragma GCC poison TARGET_PAGE_BITS_MIN
+
 #include "qemu/compiler.h"
 
 /* Older versions of C++ don't get definitions of various macros from
diff --git a/target/alpha/cpu-param.h b/target/alpha/cpu-param.h
index ff06e41497..63989e71c0 100644
--- a/target/alpha/cpu-param.h
+++ b/target/alpha/cpu-param.h
@@ -18,7 +18,6 @@
  * a 4k minimum to match x86 host, which can minimize emulation issues.
  */
 # define TARGET_PAGE_BITS_VARY
-# define TARGET_PAGE_BITS_MIN 12
 # define TARGET_VIRT_ADDR_SPACE_BITS  63
 #else
 # define TARGET_PAGE_BITS 13
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index 896b35bd6d..a7ae42d17d 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -24,7 +24,6 @@
 # else
 /* Allow user-only to vary page size from 4k */
 #  define TARGET_PAGE_BITS_VARY
-#  define TARGET_PAGE_BITS_MIN  12
 # endif
 # else
 #  define TARGET_PAGE_BITS 12
@@ -35,7 +34,7 @@
  * have to support 1K tiny pages.
  */
 # define TARGET_PAGE_BITS_VARY
-# define TARGET_PAGE_BITS_MIN  10
+# define TARGET_PAGE_BITS_LEGACY 10
 #endif /* !CONFIG_USER_ONLY */
 
 /* ARM processors have a weak memory model */
diff --git a/target/ppc/cpu-param.h b/target/ppc/cpu-param.h
index 6c4525fdf3..553ad2f4c6 100644
--- a/target/ppc/cpu-param.h
+++ b/target/ppc/cpu-param.h
@@ -33,7 +33,6 @@
 #ifdef CONFIG_USER_ONLY
 /* Allow user-only to vary page size from 4k */
 # define TARGET_PAGE_BITS_VARY
-# define TARGET_PAGE_BITS_MIN 12
 #else
 # define TARGET_PAGE_BITS 12
 #endif
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 39314e86f3..0de46903dd 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -49,7 +49,6 @@
 #endif
 #include "tcg/tcg-ldst.h"
 
-QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & ((1u < TARGET_PAGE_BITS_MIN) - 1));
 
 /* DEBUG defines, enable DEBUG_TLB_LOG to log to the CPU_LOG_MMU target */
 /* #define DEBUG_TLB */
diff --git a/page-vary-target.c b/page-vary-target.c
index 6251d948cf..d83f9a6a90 100644
--- a/page-vary-target.c
+++ b/page-vary-target.c
@@ -23,19 +23,45 @@
 #include "exec/page-vary.h"
 #include "exec/target_page.h"
 
+
+/*
+ * For system mode, the minimum comes from the number of bits
+ * required for maximum alignment (6) and the number of bits
+ * required for TLB_FLAGS_MASK (3).
+ *
+ * For user mode, TARGET_PAGE_BITS_VARY is a hack to allow the target
+ * page size to match the host page size.  Mostly, this reduces the
+ * ordinary target page size to run on a host with 4KiB pages (i.e. x86).
+ * There is no true minimum required by the implementation, but keep the
+ * same minimum as for system mode for sanity.
+ * See linux-user/mmap.c, mmap_h_lt_g and mmap_h_gt_g.
+ */
+#define TARGET_PAGE_BITS__MIN 9
+
+#ifndef TARGET_PAGE_BITS_VARY
+QEMU_BUILD_BUG_ON(TARGET_PAGE_BITS < TARGET_PAGE_BITS__MIN);
+#endif
+
+#ifndef CONFIG_USER_ONLY
+#include "exec/tlb-flags.h"
+
+QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & ((1u < TARGET_PAGE_BITS__MIN) - 1));
+
 int migration_legacy_page_bits(void)
 {
 #ifdef TARGET_PAGE_BITS_VARY
-    return TARGET_PAGE_BITS_MIN;
+    QEMU_BUILD_BUG_ON(TARGET_PAGE_BITS_LEGACY < TARGET_PAGE_BITS__MIN);
+    return TARGET_PAGE_BITS_LEGACY;
 #else
     return TARGET_PAGE_BITS;
 #endif
 }
+#endif
 
 bool set_preferred_target_page_bits(int bits)
 {
+    assert(bits >= TARGET_PAGE_BITS__MIN);
 #ifdef TARGET_PAGE_BITS_VARY
-    assert(bits >= TARGET_PAGE_BITS_MIN);
     return set_preferred_target_page_bits_common(bits);
 #else
     return true;
@@ -44,5 +70,12 @@ bool set_preferred_target_page_bits(int bits)
 
 void finalize_target_page_bits(void)
 {
-    finalize_target_page_bits_common(TARGET_PAGE_BITS_MIN);
+#ifndef TARGET_PAGE_BITS_VARY
+    finalize_target_page_bits_common(TARGET_PAGE_BITS);
+#elif defined(CONFIG_USER_ONLY)
+    assert(target_page.bits != 0);
+    finalize_target_page_bits_common(target_page.bits);
+#else
+    finalize_target_page_bits_common(TARGET_PAGE_BITS_LEGACY);
+#endif
 }
-- 
2.43.0

Re: [PATCH 1/9] include/exec: Move tb_{,set_}page_addr[01] to translation-block.h

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> Move the accessor functions for TranslationBlock
> into the header related to the structure.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/exec-all.h          | 49 -------------------------------
>   include/exec/translation-block.h | 50 ++++++++++++++++++++++++++++++++
>   2 files changed, 50 insertions(+), 49 deletions(-)
> 
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 19b0eda44a..fcad3446fe 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -123,55 +123,6 @@ int probe_access_full_mmu(CPUArchState *env, vaddr addr, int size,
>   #endif /* !CONFIG_USER_ONLY */
>   #endif /* CONFIG_TCG */
>   
> -static inline tb_page_addr_t tb_page_addr0(const TranslationBlock *tb)
> -{
> -#ifdef CONFIG_USER_ONLY
> -    return tb->itree.start;
> -#else
> -    return tb->page_addr[0];
> -#endif
> -}
> -
> -static inline tb_page_addr_t tb_page_addr1(const TranslationBlock *tb)
> -{
> -#ifdef CONFIG_USER_ONLY
> -    tb_page_addr_t next = tb->itree.last & TARGET_PAGE_MASK;
> -    return next == (tb->itree.start & TARGET_PAGE_MASK) ? -1 : next;
> -#else
> -    return tb->page_addr[1];
> -#endif
> -}
> -
> -static inline void tb_set_page_addr0(TranslationBlock *tb,
> -                                     tb_page_addr_t addr)
> -{
> -#ifdef CONFIG_USER_ONLY
> -    tb->itree.start = addr;
> -    /*
> -     * To begin, we record an interval of one byte.  When the translation
> -     * loop encounters a second page, the interval will be extended to
> -     * include the first byte of the second page, which is sufficient to
> -     * allow tb_page_addr1() above to work properly.  The final corrected
> -     * interval will be set by tb_page_add() from tb->size before the
> -     * node is added to the interval tree.
> -     */
> -    tb->itree.last = addr;
> -#else
> -    tb->page_addr[0] = addr;
> -#endif
> -}
> -
> -static inline void tb_set_page_addr1(TranslationBlock *tb,
> -                                     tb_page_addr_t addr)
> -{
> -#ifdef CONFIG_USER_ONLY
> -    /* Extend the interval to the first byte of the second page.  See above. */
> -    tb->itree.last = addr;
> -#else
> -    tb->page_addr[1] = addr;
> -#endif
> -}
> -
>   /* TranslationBlock invalidate API */
>   void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
>   void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t last);
> diff --git a/include/exec/translation-block.h b/include/exec/translation-block.h
> index 3c69bc71a9..8b8e730561 100644
> --- a/include/exec/translation-block.h
> +++ b/include/exec/translation-block.h
> @@ -13,6 +13,7 @@
>   #include "exec/vaddr.h"
>   #ifdef CONFIG_USER_ONLY
>   #include "qemu/interval-tree.h"
> +#include "exec/target_page.h"
>   #endif
>   
>   /*
> @@ -157,4 +158,53 @@ static inline uint32_t tb_cflags(const TranslationBlock *tb)
>   bool tcg_cflags_has(CPUState *cpu, uint32_t flags);
>   void tcg_cflags_set(CPUState *cpu, uint32_t flags);
>   
> +static inline tb_page_addr_t tb_page_addr0(const TranslationBlock *tb)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    return tb->itree.start;
> +#else
> +    return tb->page_addr[0];
> +#endif
> +}
> +
> +static inline tb_page_addr_t tb_page_addr1(const TranslationBlock *tb)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    tb_page_addr_t next = tb->itree.last & TARGET_PAGE_MASK;
> +    return next == (tb->itree.start & TARGET_PAGE_MASK) ? -1 : next;
> +#else
> +    return tb->page_addr[1];
> +#endif
> +}
> +
> +static inline void tb_set_page_addr0(TranslationBlock *tb,
> +                                     tb_page_addr_t addr)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    tb->itree.start = addr;
> +    /*
> +     * To begin, we record an interval of one byte.  When the translation
> +     * loop encounters a second page, the interval will be extended to
> +     * include the first byte of the second page, which is sufficient to
> +     * allow tb_page_addr1() above to work properly.  The final corrected
> +     * interval will be set by tb_page_add() from tb->size before the
> +     * node is added to the interval tree.
> +     */
> +    tb->itree.last = addr;
> +#else
> +    tb->page_addr[0] = addr;
> +#endif
> +}
> +
> +static inline void tb_set_page_addr1(TranslationBlock *tb,
> +                                     tb_page_addr_t addr)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    /* Extend the interval to the first byte of the second page.  See above. */
> +    tb->itree.last = addr;
> +#else
> +    tb->page_addr[1] = addr;
> +#endif
> +}
> +
>   #endif /* EXEC_TRANSLATION_BLOCK_H */

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 2/9] accel/tcg: Move get_page_addr_code* declarations

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> Move the declarations from exec/exec-all.h to the
> private accel/tcg/internal-common.h.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/internal-common.h | 34 ++++++++++++++++++++++++++++++++++
>   include/exec/exec-all.h     | 34 ----------------------------------
>   accel/tcg/translator.c      |  1 +
>   3 files changed, 35 insertions(+), 34 deletions(-)
> 
> diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
> index 9b6ab3a8cc..2f00560d10 100644
> --- a/accel/tcg/internal-common.h
> +++ b/accel/tcg/internal-common.h
> @@ -74,4 +74,38 @@ uint32_t curr_cflags(CPUState *cpu);
>   
>   void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);
>   
> +/**
> + * get_page_addr_code_hostp()
> + * @env: CPUArchState
> + * @addr: guest virtual address of guest code
> + *
> + * See get_page_addr_code() (full-system version) for documentation on the
> + * return value.
> + *
> + * Sets *@hostp (when @hostp is non-NULL) as follows.
> + * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
> + * to the host address where @addr's content is kept.
> + *
> + * Note: this function can trigger an exception.
> + */
> +tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
> +                                        void **hostp);
> +
> +/**
> + * get_page_addr_code()
> + * @env: CPUArchState
> + * @addr: guest virtual address of guest code
> + *
> + * If we cannot translate and execute from the entire RAM page, or if
> + * the region is not backed by RAM, returns -1. Otherwise, returns the
> + * ram_addr_t corresponding to the guest code at @addr.
> + *
> + * Note: this function can trigger an exception.
> + */
> +static inline tb_page_addr_t get_page_addr_code(CPUArchState *env,
> +                                                vaddr addr)
> +{
> +    return get_page_addr_code_hostp(env, addr, NULL);
> +}
> +
>   #endif
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index fcad3446fe..f52a680f42 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -143,40 +143,6 @@ struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
>                                                hwaddr index, MemTxAttrs attrs);
>   #endif
>   
> -/**
> - * get_page_addr_code_hostp()
> - * @env: CPUArchState
> - * @addr: guest virtual address of guest code
> - *
> - * See get_page_addr_code() (full-system version) for documentation on the
> - * return value.
> - *
> - * Sets *@hostp (when @hostp is non-NULL) as follows.
> - * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
> - * to the host address where @addr's content is kept.
> - *
> - * Note: this function can trigger an exception.
> - */
> -tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
> -                                        void **hostp);
> -
> -/**
> - * get_page_addr_code()
> - * @env: CPUArchState
> - * @addr: guest virtual address of guest code
> - *
> - * If we cannot translate and execute from the entire RAM page, or if
> - * the region is not backed by RAM, returns -1. Otherwise, returns the
> - * ram_addr_t corresponding to the guest code at @addr.
> - *
> - * Note: this function can trigger an exception.
> - */
> -static inline tb_page_addr_t get_page_addr_code(CPUArchState *env,
> -                                                vaddr addr)
> -{
> -    return get_page_addr_code_hostp(env, addr, NULL);
> -}
> -
>   #if !defined(CONFIG_USER_ONLY)
>   
>   MemoryRegionSection *
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 7ef04fc597..307a513487 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -17,6 +17,7 @@
>   #include "exec/translator.h"
>   #include "exec/plugin-gen.h"
>   #include "tcg/tcg-op-common.h"
> +#include "internal-common.h"
>   #include "internal-target.h"
>   #include "disas/disas.h"
>   #include "tb-internal.h"

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 3/9] accel/tcg: Remove page_protect

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> Merge the user-only page_protect function with the user-only
> implementation of tb_lock_page0.  This avoids pulling
> page-protection.h into tb-internal.h.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tb-internal.h        | 11 +++--------
>   include/user/page-protection.h |  1 -
>   accel/tcg/user-exec.c          |  2 +-
>   3 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/accel/tcg/tb-internal.h b/accel/tcg/tb-internal.h
> index 68aa8d17f4..f7c2073e29 100644
> --- a/accel/tcg/tb-internal.h
> +++ b/accel/tcg/tb-internal.h
> @@ -51,28 +51,23 @@
>   
>   #endif /* CONFIG_SOFTMMU */
>   
> +void tb_lock_page0(tb_page_addr_t);
> +
>   #ifdef CONFIG_USER_ONLY
> -#include "user/page-protection.h"
>   /*
>    * For user-only, page_protect sets the page read-only.
>    * Since most execution is already on read-only pages, and we'd need to
>    * account for other TBs on the same page, defer undoing any page protection
>    * until we receive the write fault.
>    */
> -static inline void tb_lock_page0(tb_page_addr_t p0)
> -{
> -    page_protect(p0);
> -}
> -
>   static inline void tb_lock_page1(tb_page_addr_t p0, tb_page_addr_t p1)
>   {
> -    page_protect(p1);
> +    tb_lock_page0(p1);
>   }
>   
>   static inline void tb_unlock_page1(tb_page_addr_t p0, tb_page_addr_t p1) { }
>   static inline void tb_unlock_pages(TranslationBlock *tb) { }
>   #else
> -void tb_lock_page0(tb_page_addr_t);
>   void tb_lock_page1(tb_page_addr_t, tb_page_addr_t);
>   void tb_unlock_page1(tb_page_addr_t, tb_page_addr_t);
>   void tb_unlock_pages(TranslationBlock *);
> diff --git a/include/user/page-protection.h b/include/user/page-protection.h
> index 51daa18648..d5c8748d49 100644
> --- a/include/user/page-protection.h
> +++ b/include/user/page-protection.h
> @@ -16,7 +16,6 @@
>   #include "exec/target_long.h"
>   #include "exec/translation-block.h"
>   
> -void page_protect(tb_page_addr_t page_addr);
>   int page_unprotect(tb_page_addr_t address, uintptr_t pc);
>   
>   int page_get_flags(target_ulong address);
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 667c5e0354..72a9809c2d 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -657,7 +657,7 @@ target_ulong page_find_range_empty(target_ulong min, target_ulong max,
>       }
>   }
>   
> -void page_protect(tb_page_addr_t address)
> +void tb_lock_page0(tb_page_addr_t address)
>   {
>       PageFlagsNode *p;
>       target_ulong start, last;

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 4/9] accel/tcg: Remove cpu-all.h, exec-all.h from tb-internal.h

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> Not used by tb-internal.h, but add an include for
> target_page.h in tb-maint.c.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tb-internal.h | 2 --
>   accel/tcg/tb-maint.c    | 1 +
>   2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/accel/tcg/tb-internal.h b/accel/tcg/tb-internal.h
> index f7c2073e29..f9a06bcbab 100644
> --- a/accel/tcg/tb-internal.h
> +++ b/accel/tcg/tb-internal.h
> @@ -9,8 +9,6 @@
>   #ifndef ACCEL_TCG_TB_INTERNAL_TARGET_H
>   #define ACCEL_TCG_TB_INTERNAL_TARGET_H
>   
> -#include "exec/cpu-all.h"
> -#include "exec/exec-all.h"
>   #include "exec/translation-block.h"
>   
>   /*
> diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
> index d5899ad047..df3438e190 100644
> --- a/accel/tcg/tb-maint.c
> +++ b/accel/tcg/tb-maint.c
> @@ -26,6 +26,7 @@
>   #include "exec/page-protection.h"
>   #include "exec/mmap-lock.h"
>   #include "exec/tb-flush.h"
> +#include "exec/target_page.h"
>   #include "tb-internal.h"
>   #include "system/tcg.h"
>   #include "tcg/tcg.h"

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 5/9] accel/tcg: Build translator.c twice

[Philippe Mathieu-Daudé]

On 28/3/25 21:04, Richard Henderson wrote:
> Drop some unnecessary includes.  Change the offsetof expressions
> to be based on CPUState instead of ArchCPU.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/translator.c | 14 ++++++--------
>   accel/tcg/meson.build  |  2 +-
>   2 files changed, 7 insertions(+), 9 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 6/9] accel/tcg: Split out tlb-bounds.h

[Philippe Mathieu-Daudé]

On 28/3/25 21:04, Richard Henderson wrote:
> The CPU_TLB_DYN_{MIN,MAX}_BITS definitions are not required
> outside of cputlb.c and translate-all.c.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tb-internal.h   | 27 ---------------------------
>   accel/tcg/tlb-bounds.h    | 32 ++++++++++++++++++++++++++++++++
>   accel/tcg/cputlb.c        |  1 +
>   accel/tcg/translate-all.c |  1 +
>   4 files changed, 34 insertions(+), 27 deletions(-)
>   create mode 100644 accel/tcg/tlb-bounds.h

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 7/9] include/exec: Redefine tlb-flags with absolute values

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> Don't base the values on TARGET_PAGE_BITS_MIN, but do verify
> that TLB_FLAGS_MASK does not overlap minimum page size.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/tlb-flags.h | 68 +++++++++++++++++++---------------------
>   accel/tcg/cputlb.c       |  2 ++
>   2 files changed, 34 insertions(+), 36 deletions(-)
> 
> diff --git a/include/exec/tlb-flags.h b/include/exec/tlb-flags.h
> index 54a6bae768..357e79095c 100644
> --- a/include/exec/tlb-flags.h
> +++ b/include/exec/tlb-flags.h
> @@ -19,54 +19,29 @@
>   #ifndef TLB_FLAGS_H
>   #define TLB_FLAGS_H
>   
> -#include "exec/cpu-defs.h"
> +/*
> + * Flags returned for lookup of a TLB virtual address.
> + */
>   
>   #ifdef CONFIG_USER_ONLY
>   
>   /*
> - * Allow some level of source compatibility with softmmu.  We do not
> - * support any of the more exotic features, so only invalid pages may
> - * be signaled by probe_access_flags().
> + * Allow some level of source compatibility with softmmu.
> + * Invalid is set when the page does not have requested permissions.
> + * MMIO is set when we want the target helper to use the functional
> + * interface for load/store so that plugins see the access.
>    */
> -#define TLB_INVALID_MASK    (1 << (TARGET_PAGE_BITS_MIN - 1))
> -#define TLB_MMIO            (1 << (TARGET_PAGE_BITS_MIN - 2))
> -#define TLB_WATCHPOINT      0
> +#define TLB_INVALID_MASK     (1 << 0)
> +#define TLB_MMIO             (1 << 1)
> +#define TLB_WATCHPOINT       0
>   
>   #else
>   
> -/*
> - * Flags stored in the low bits of the TLB virtual address.
> - * These are defined so that fast path ram access is all zeros.
> - * The flags all must be between TARGET_PAGE_BITS and
> - * maximum address alignment bit.
> - *
> - * Use TARGET_PAGE_BITS_MIN so that these bits are constant
> - * when TARGET_PAGE_BITS_VARY is in effect.
> - *
> - * The count, if not the placement of these bits is known
> - * to tcg/tcg-op-ldst.c, check_max_alignment().
> - */
> -/* Zero if TLB entry is valid.  */
> -#define TLB_INVALID_MASK    (1 << (TARGET_PAGE_BITS_MIN - 1))
> -/*
> - * Set if TLB entry references a clean RAM page.  The iotlb entry will
> - * contain the page physical address.
> - */
> -#define TLB_NOTDIRTY        (1 << (TARGET_PAGE_BITS_MIN - 2))
> -/* Set if the slow path must be used; more flags in CPUTLBEntryFull. */
> -#define TLB_FORCE_SLOW      (1 << (TARGET_PAGE_BITS_MIN - 3))
> -
> -/*
> - * Use this mask to check interception with an alignment mask
> - * in a TCG backend.
> - */
> -#define TLB_FLAGS_MASK \
> -    (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_FORCE_SLOW)
> -
>   /*
>    * Flags stored in CPUTLBEntryFull.slow_flags[x].
>    * TLB_FORCE_SLOW must be set in CPUTLBEntry.addr_idx[x].
>    */
> +
>   /* Set if TLB entry requires byte swap.  */
>   #define TLB_BSWAP            (1 << 0)
>   /* Set if TLB entry contains a watchpoint.  */
> @@ -82,6 +57,27 @@
>       (TLB_BSWAP | TLB_WATCHPOINT | TLB_CHECK_ALIGNED | \
>        TLB_DISCARD_WRITE | TLB_MMIO)
>   
> +/*
> + * Flags stored in CPUTLBEntry.addr_idx[x].
> + * These must be above the largest alignment (64 bytes),
> + * and below the smallest page size (1024 bytes).
> + * This leaves bits [9:6] available for use.
> + */
> +
> +/* Zero if TLB entry is valid.  */
> +#define TLB_INVALID_MASK     (1 << 6)
> +/* Set if TLB entry references a clean RAM page.  */
> +#define TLB_NOTDIRTY         (1 << 7)
> +/* Set if the slow path must be used; more flags in CPUTLBEntryFull. */
> +#define TLB_FORCE_SLOW       (1 << 8)
> +
> +/*
> + * Use this mask to check interception with an alignment mask
> + * in a TCG backend.
> + */
> +#define TLB_FLAGS_MASK \
> +    (TLB_INVALID_MASK | TLB_NOTDIRTY | TLB_FORCE_SLOW)
> +
>   /* The two sets of flags must not overlap. */
>   QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & TLB_SLOW_FLAGS_MASK);
>   
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index a717f357d5..39314e86f3 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -49,6 +49,8 @@
>   #endif
>   #include "tcg/tcg-ldst.h"
>   
> +QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & ((1u < TARGET_PAGE_BITS_MIN) - 1));
> +
>   /* DEBUG defines, enable DEBUG_TLB_LOG to log to the CPU_LOG_MMU target */
>   /* #define DEBUG_TLB */
>   /* #define DEBUG_TLB_LOG */

Hurrah!

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Maybe we could define MEMOP_MAX_ALIGNMENT, reuse use that in 
check_max_alignment(), and add a compile time check here as well.

We can as well mention in a comment that all architectures now have the 
same placement for those flags, simplifying MMU management when we'll 
mix several architectures.

Re: [PATCH 5/9] accel/tcg: Build translator.c twice

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> Drop some unnecessary includes.  Change the offsetof expressions
> to be based on CPUState instead of ArchCPU.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/translator.c | 14 ++++++--------
>   accel/tcg/meson.build  |  2 +-
>   2 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 307a513487..36a6a9e040 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -11,14 +11,13 @@
>   #include "qemu/bswap.h"
>   #include "qemu/log.h"
>   #include "qemu/error-report.h"
> -#include "exec/exec-all.h"
>   #include "exec/cpu-ldst-common.h"
>   #include "exec/cpu-mmu-index.h"
> +#include "exec/target_page.h"
>   #include "exec/translator.h"
>   #include "exec/plugin-gen.h"
>   #include "tcg/tcg-op-common.h"
>   #include "internal-common.h"
> -#include "internal-target.h"
>   #include "disas/disas.h"
>   #include "tb-internal.h"
>   
> @@ -26,8 +25,7 @@ static void set_can_do_io(DisasContextBase *db, bool val)
>   {
>       QEMU_BUILD_BUG_ON(sizeof_field(CPUState, neg.can_do_io) != 1);
>       tcg_gen_st8_i32(tcg_constant_i32(val), tcg_env,
> -                    offsetof(ArchCPU, parent_obj.neg.can_do_io) -
> -                    offsetof(ArchCPU, env));
> +                    offsetof(CPUState, neg.can_do_io) - sizeof(CPUState));
>   }
>   
>   bool translator_io_start(DisasContextBase *db)
> @@ -50,8 +48,8 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
>       if ((cflags & CF_USE_ICOUNT) || !(cflags & CF_NOIRQ)) {
>           count = tcg_temp_new_i32();
>           tcg_gen_ld_i32(count, tcg_env,
> -                       offsetof(ArchCPU, parent_obj.neg.icount_decr.u32)
> -                       - offsetof(ArchCPU, env));
> +                       offsetof(CPUState, neg.icount_decr.u32) -
> +                       sizeof(CPUState));
>       }
>   
>       if (cflags & CF_USE_ICOUNT) {
> @@ -80,8 +78,8 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags)
>   
>       if (cflags & CF_USE_ICOUNT) {
>           tcg_gen_st16_i32(count, tcg_env,
> -                         offsetof(ArchCPU, parent_obj.neg.icount_decr.u16.low)
> -                         - offsetof(ArchCPU, env));
> +                         offsetof(CPUState, neg.icount_decr.u16.low) -
> +                         sizeof(CPUState));
>       }
>   
>       return icount_start_insn;
> diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
> index 72d4acfe5e..047afa49a2 100644
> --- a/accel/tcg/meson.build
> +++ b/accel/tcg/meson.build
> @@ -8,6 +8,7 @@ tcg_ss.add(files(
>     'cpu-exec-common.c',
>     'tcg-runtime.c',
>     'tcg-runtime-gvec.c',
> +  'translator.c',
>   ))
>   if get_option('plugins')
>     tcg_ss.add(files('plugin-gen.c'))
> @@ -22,7 +23,6 @@ tcg_specific_ss.add(files(
>     'cpu-exec.c',
>     'tb-maint.c',
>     'translate-all.c',
> -  'translator.c',
>   ))
>   tcg_specific_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
>   specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_specific_ss)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 6/9] accel/tcg: Split out tlb-bounds.h

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> The CPU_TLB_DYN_{MIN,MAX}_BITS definitions are not required
> outside of cputlb.c and translate-all.c.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tb-internal.h   | 27 ---------------------------
>   accel/tcg/tlb-bounds.h    | 32 ++++++++++++++++++++++++++++++++
>   accel/tcg/cputlb.c        |  1 +
>   accel/tcg/translate-all.c |  1 +
>   4 files changed, 34 insertions(+), 27 deletions(-)
>   create mode 100644 accel/tcg/tlb-bounds.h
> 
> diff --git a/accel/tcg/tb-internal.h b/accel/tcg/tb-internal.h
> index f9a06bcbab..08538e2896 100644
> --- a/accel/tcg/tb-internal.h
> +++ b/accel/tcg/tb-internal.h
> @@ -22,33 +22,6 @@
>    */
>   #define GETPC_ADJ   2
>   
> -#ifdef CONFIG_SOFTMMU
> -
> -#define CPU_TLB_DYN_MIN_BITS 6
> -#define CPU_TLB_DYN_DEFAULT_BITS 8
> -
> -# if HOST_LONG_BITS == 32
> -/* Make sure we do not require a double-word shift for the TLB load */
> -#  define CPU_TLB_DYN_MAX_BITS (32 - TARGET_PAGE_BITS)
> -# else /* HOST_LONG_BITS == 64 */
> -/*
> - * Assuming TARGET_PAGE_BITS==12, with 2**22 entries we can cover 2**(22+12) ==
> - * 2**34 == 16G of address space. This is roughly what one would expect a
> - * TLB to cover in a modern (as of 2018) x86_64 CPU. For instance, Intel
> - * Skylake's Level-2 STLB has 16 1G entries.
> - * Also, make sure we do not size the TLB past the guest's address space.
> - */
> -#  ifdef TARGET_PAGE_BITS_VARY
> -#   define CPU_TLB_DYN_MAX_BITS                                  \
> -    MIN(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS)
> -#  else
> -#   define CPU_TLB_DYN_MAX_BITS                                  \
> -    MIN_CONST(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS)
> -#  endif
> -# endif
> -
> -#endif /* CONFIG_SOFTMMU */
> -
>   void tb_lock_page0(tb_page_addr_t);
>   
>   #ifdef CONFIG_USER_ONLY
> diff --git a/accel/tcg/tlb-bounds.h b/accel/tcg/tlb-bounds.h
> new file mode 100644
> index 0000000000..efd34d4793
> --- /dev/null
> +++ b/accel/tcg/tlb-bounds.h
> @@ -0,0 +1,32 @@
> +/*
> + * softmmu size bounds
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +
> +#ifndef ACCEL_TCG_TLB_BOUNDS_H
> +#define ACCEL_TCG_TLB_BOUNDS_H
> +
> +#define CPU_TLB_DYN_MIN_BITS 6
> +#define CPU_TLB_DYN_DEFAULT_BITS 8
> +
> +# if HOST_LONG_BITS == 32
> +/* Make sure we do not require a double-word shift for the TLB load */
> +#  define CPU_TLB_DYN_MAX_BITS (32 - TARGET_PAGE_BITS)
> +# else /* HOST_LONG_BITS == 64 */
> +/*
> + * Assuming TARGET_PAGE_BITS==12, with 2**22 entries we can cover 2**(22+12) ==
> + * 2**34 == 16G of address space. This is roughly what one would expect a
> + * TLB to cover in a modern (as of 2018) x86_64 CPU. For instance, Intel
> + * Skylake's Level-2 STLB has 16 1G entries.
> + * Also, make sure we do not size the TLB past the guest's address space.
> + */
> +#  ifdef TARGET_PAGE_BITS_VARY
> +#   define CPU_TLB_DYN_MAX_BITS                                  \
> +    MIN(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS)
> +#  else
> +#   define CPU_TLB_DYN_MAX_BITS                                  \
> +    MIN_CONST(22, TARGET_VIRT_ADDR_SPACE_BITS - TARGET_PAGE_BITS)
> +#  endif
> +# endif
> +
> +#endif /* ACCEL_TCG_TLB_BOUNDS_H */
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 28c47d4872..a717f357d5 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -41,6 +41,7 @@
>   #include "trace.h"
>   #include "tb-hash.h"
>   #include "tb-internal.h"
> +#include "tlb-bounds.h"
>   #include "internal-common.h"
>   #include "internal-target.h"
>   #ifdef CONFIG_PLUGIN
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index bb161ae61a..87fb6c51d3 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -47,6 +47,7 @@
>   #include "exec/page-protection.h"
>   #include "exec/mmap-lock.h"
>   #include "tb-internal.h"
> +#include "tlb-bounds.h"
>   #include "exec/translator.h"
>   #include "exec/tb-flush.h"
>   #include "qemu/bitmap.h"

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 8/9] page-vary: Move and rename qemu_target_page_bits_min

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> Rename to migration_legacy_page_bits, to make it clear that
> we cannot change the value without causing a migration break.
> Move to page-vary.h and page-vary-target.c.
> Define via TARGET_PAGE_BITS if not TARGET_PAGE_BITS_VARY.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/page-vary.h   | 9 +++++++++
>   include/exec/target_page.h | 1 -
>   migration/savevm.c         | 6 +++---
>   page-target.c              | 5 -----
>   page-vary-target.c         | 9 +++++++++
>   5 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/include/exec/page-vary.h b/include/exec/page-vary.h
> index 54ddde308a..101c25911c 100644
> --- a/include/exec/page-vary.h
> +++ b/include/exec/page-vary.h
> @@ -49,4 +49,13 @@ bool set_preferred_target_page_bits(int bits);
>    */
>   void finalize_target_page_bits(void);
>   
> +/**
> + * migration_legacy_page_bits
> + *
> + * For migration compatibility with qemu v2.9, prior to the introduction
> + * of the configuration/target-page-bits section, return the value of
> + * TARGET_PAGE_BITS that the target had then.
> + */
> +int migration_legacy_page_bits(void);
> +
>   #endif /* EXEC_PAGE_VARY_H */
> diff --git a/include/exec/target_page.h b/include/exec/target_page.h
> index 8e89e5cbe6..e4bd7f7767 100644
> --- a/include/exec/target_page.h
> +++ b/include/exec/target_page.h
> @@ -63,7 +63,6 @@ static inline int qemu_target_page_bits(void)
>       return TARGET_PAGE_BITS;
>   }
>   
> -int qemu_target_page_bits_min(void);
>   size_t qemu_target_pages_to_MiB(size_t pages);
>   
>   #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index c33200a33f..0c12e373b4 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -50,6 +50,7 @@
>   #include "system/cpus.h"
>   #include "system/memory.h"
>   #include "exec/target_page.h"
> +#include "exec/page-vary.h"
>   #include "trace.h"
>   #include "qemu/iov.h"
>   #include "qemu/job.h"
> @@ -339,7 +340,7 @@ static int configuration_pre_load(void *opaque)
>        * predates the variable-target-page-bits support and is using the
>        * minimum possible value for this CPU.
>        */
> -    state->target_page_bits = qemu_target_page_bits_min();
> +    state->target_page_bits = migration_legacy_page_bits();
>       return 0;
>   }
>   
> @@ -462,8 +463,7 @@ static const VMStateInfo vmstate_info_capability = {
>    */
>   static bool vmstate_target_page_bits_needed(void *opaque)
>   {
> -    return qemu_target_page_bits()
> -        > qemu_target_page_bits_min();
> +    return qemu_target_page_bits() > migration_legacy_page_bits();
>   }
>   
>   static const VMStateDescription vmstate_target_page_bits = {
> diff --git a/page-target.c b/page-target.c
> index 321e43d06f..8fcd5443b5 100644
> --- a/page-target.c
> +++ b/page-target.c
> @@ -9,11 +9,6 @@
>   #include "qemu/osdep.h"
>   #include "exec/target_page.h"
>   
> -int qemu_target_page_bits_min(void)
> -{
> -    return TARGET_PAGE_BITS_MIN;
> -}
> -
>   /* Convert target pages to MiB (2**20). */
>   size_t qemu_target_pages_to_MiB(size_t pages)
>   {
> diff --git a/page-vary-target.c b/page-vary-target.c
> index 84ddeb7c26..6251d948cf 100644
> --- a/page-vary-target.c
> +++ b/page-vary-target.c
> @@ -23,6 +23,15 @@
>   #include "exec/page-vary.h"
>   #include "exec/target_page.h"
>   
> +int migration_legacy_page_bits(void)
> +{
> +#ifdef TARGET_PAGE_BITS_VARY
> +    return TARGET_PAGE_BITS_MIN;
> +#else
> +    return TARGET_PAGE_BITS;
> +#endif
> +}
> +
>   bool set_preferred_target_page_bits(int bits)
>   {
>   #ifdef TARGET_PAGE_BITS_VARY

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 9/9] page-vary: Restrict scope of TARGET_PAGE_BITS_MIN

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> The only place we really need to know the minimum is within
> page-vary-target.c.  Rename the target/arm TARGET_PAGE_BITS_MIN
> to TARGE_PAGE_BITS_LEGACY to emphasize what it really means.
> Move the assertions related to minimum page size as well.
> 

s/TARGE_PAGE_BITS_LEGACY/TARGET_PAGE_BITS_LEGACY

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/cpu-defs.h    | 10 ++--------
>   include/exec/poison.h      |  1 +
>   include/exec/target_page.h |  1 -
>   include/qemu/osdep.h       |  6 ++++++
>   target/alpha/cpu-param.h   |  1 -
>   target/arm/cpu-param.h     |  3 +--
>   target/ppc/cpu-param.h     |  1 -
>   accel/tcg/cputlb.c         |  1 -
>   page-vary-target.c         | 39 +++++++++++++++++++++++++++++++++++---
>   9 files changed, 46 insertions(+), 17 deletions(-)
> 
> diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
> index 9f955f53fd..e01acb7c90 100644
> --- a/include/exec/cpu-defs.h
> +++ b/include/exec/cpu-defs.h
> @@ -34,14 +34,8 @@
>   #ifndef TARGET_VIRT_ADDR_SPACE_BITS
>   # error TARGET_VIRT_ADDR_SPACE_BITS must be defined in cpu-param.h
>   #endif
> -#ifndef TARGET_PAGE_BITS
> -# ifdef TARGET_PAGE_BITS_VARY
> -#  ifndef TARGET_PAGE_BITS_MIN
> -#   error TARGET_PAGE_BITS_MIN must be defined in cpu-param.h
> -#  endif
> -# else
> -#  error TARGET_PAGE_BITS must be defined in cpu-param.h
> -# endif
> +#if !defined(TARGET_PAGE_BITS) && !defined(TARGET_PAGE_BITS_VARY)
> +# error TARGET_PAGE_BITS must be defined in cpu-param.h
>   #endif
>   
>   #include "exec/target_long.h"
> diff --git a/include/exec/poison.h b/include/exec/poison.h
> index 4180a5a489..c4f7ee22bf 100644
> --- a/include/exec/poison.h
> +++ b/include/exec/poison.h
> @@ -44,6 +44,7 @@
>   #pragma GCC poison TARGET_FMT_lu
>   
>   #pragma GCC poison TARGET_PHYS_ADDR_SPACE_BITS
> +#pragma GCC poison TARGET_PAGE_BITS_LEGACY
>   
>   #pragma GCC poison CONFIG_ALPHA_DIS
>   #pragma GCC poison CONFIG_HPPA_DIS
> diff --git a/include/exec/target_page.h b/include/exec/target_page.h
> index e4bd7f7767..ca0ebbc8bb 100644
> --- a/include/exec/target_page.h
> +++ b/include/exec/target_page.h
> @@ -41,7 +41,6 @@ extern const TargetPageBits target_page;
>   # endif
>   # define TARGET_PAGE_SIZE    (-(int)TARGET_PAGE_MASK)
>   #else
> -# define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
>   # define TARGET_PAGE_SIZE    (1 << TARGET_PAGE_BITS)
>   # define TARGET_PAGE_MASK    ((TARGET_PAGE_TYPE)-1 << TARGET_PAGE_BITS)
>   #endif
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 4397a90680..321a52d7f0 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -50,6 +50,12 @@
>    */
>   #pragma GCC poison TARGET_WORDS_BIGENDIAN
>   
> +/*
> + * TARGET_PAGE_BITS_MIN was repaced by TARGET_PAGE_BITS_LEGACY
> + * for system mode.  Prevent it from creeping back in.
> + */

s/repaced/replaced

> +#pragma GCC poison TARGET_PAGE_BITS_MIN
> +
>   #include "qemu/compiler.h"
>   
>   /* Older versions of C++ don't get definitions of various macros from
> diff --git a/target/alpha/cpu-param.h b/target/alpha/cpu-param.h
> index ff06e41497..63989e71c0 100644
> --- a/target/alpha/cpu-param.h
> +++ b/target/alpha/cpu-param.h
> @@ -18,7 +18,6 @@
>    * a 4k minimum to match x86 host, which can minimize emulation issues.
>    */
>   # define TARGET_PAGE_BITS_VARY
> -# define TARGET_PAGE_BITS_MIN 12
>   # define TARGET_VIRT_ADDR_SPACE_BITS  63
>   #else
>   # define TARGET_PAGE_BITS 13
> diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
> index 896b35bd6d..a7ae42d17d 100644
> --- a/target/arm/cpu-param.h
> +++ b/target/arm/cpu-param.h
> @@ -24,7 +24,6 @@
>   # else
>   /* Allow user-only to vary page size from 4k */
>   #  define TARGET_PAGE_BITS_VARY
> -#  define TARGET_PAGE_BITS_MIN  12
>   # endif
>   # else
>   #  define TARGET_PAGE_BITS 12
> @@ -35,7 +34,7 @@
>    * have to support 1K tiny pages.
>    */
>   # define TARGET_PAGE_BITS_VARY
> -# define TARGET_PAGE_BITS_MIN  10
> +# define TARGET_PAGE_BITS_LEGACY 10
>   #endif /* !CONFIG_USER_ONLY */
>   
>   /* ARM processors have a weak memory model */
> diff --git a/target/ppc/cpu-param.h b/target/ppc/cpu-param.h
> index 6c4525fdf3..553ad2f4c6 100644
> --- a/target/ppc/cpu-param.h
> +++ b/target/ppc/cpu-param.h
> @@ -33,7 +33,6 @@
>   #ifdef CONFIG_USER_ONLY
>   /* Allow user-only to vary page size from 4k */
>   # define TARGET_PAGE_BITS_VARY
> -# define TARGET_PAGE_BITS_MIN 12
>   #else
>   # define TARGET_PAGE_BITS 12
>   #endif
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 39314e86f3..0de46903dd 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -49,7 +49,6 @@
>   #endif
>   #include "tcg/tcg-ldst.h"
>   
> -QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & ((1u < TARGET_PAGE_BITS_MIN) - 1));
>   
>   /* DEBUG defines, enable DEBUG_TLB_LOG to log to the CPU_LOG_MMU target */
>   /* #define DEBUG_TLB */
> diff --git a/page-vary-target.c b/page-vary-target.c
> index 6251d948cf..d83f9a6a90 100644
> --- a/page-vary-target.c
> +++ b/page-vary-target.c
> @@ -23,19 +23,45 @@
>   #include "exec/page-vary.h"
>   #include "exec/target_page.h"
>   
> +
> +/*
> + * For system mode, the minimum comes from the number of bits
> + * required for maximum alignment (6) and the number of bits
> + * required for TLB_FLAGS_MASK (3).
> + *
> + * For user mode, TARGET_PAGE_BITS_VARY is a hack to allow the target
> + * page size to match the host page size.  Mostly, this reduces the
> + * ordinary target page size to run on a host with 4KiB pages (i.e. x86).
> + * There is no true minimum required by the implementation, but keep the
> + * same minimum as for system mode for sanity.
> + * See linux-user/mmap.c, mmap_h_lt_g and mmap_h_gt_g.
> + */
> +#define TARGET_PAGE_BITS__MIN 9
> +
> +#ifndef TARGET_PAGE_BITS_VARY
> +QEMU_BUILD_BUG_ON(TARGET_PAGE_BITS < TARGET_PAGE_BITS__MIN);
> +#endif
> +
> +#ifndef CONFIG_USER_ONLY
> +#include "exec/tlb-flags.h"
> +
> +QEMU_BUILD_BUG_ON(TLB_FLAGS_MASK & ((1u < TARGET_PAGE_BITS__MIN) - 1));
> +
>   int migration_legacy_page_bits(void)
>   {
>   #ifdef TARGET_PAGE_BITS_VARY
> -    return TARGET_PAGE_BITS_MIN;
> +    QEMU_BUILD_BUG_ON(TARGET_PAGE_BITS_LEGACY < TARGET_PAGE_BITS__MIN);
> +    return TARGET_PAGE_BITS_LEGACY;
>   #else
>       return TARGET_PAGE_BITS;
>   #endif
>   }
> +#endif
>   
>   bool set_preferred_target_page_bits(int bits)
>   {
> +    assert(bits >= TARGET_PAGE_BITS__MIN);
>   #ifdef TARGET_PAGE_BITS_VARY
> -    assert(bits >= TARGET_PAGE_BITS_MIN);
>       return set_preferred_target_page_bits_common(bits);
>   #else
>       return true;
> @@ -44,5 +70,12 @@ bool set_preferred_target_page_bits(int bits)
>   
>   void finalize_target_page_bits(void)
>   {
> -    finalize_target_page_bits_common(TARGET_PAGE_BITS_MIN);
> +#ifndef TARGET_PAGE_BITS_VARY
> +    finalize_target_page_bits_common(TARGET_PAGE_BITS);
> +#elif defined(CONFIG_USER_ONLY)
> +    assert(target_page.bits != 0);
> +    finalize_target_page_bits_common(target_page.bits);
> +#else
> +    finalize_target_page_bits_common(TARGET_PAGE_BITS_LEGACY);
> +#endif
>   }

Great!

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Any chance we would rename TARGET_PAGE_BITS__MIN? (MIN_ALL? ALL_MIN? 
ARCH_MIN? any other idea)
I know it's restricted to this file only, but the __ is surprising.

Re: [PATCH 0/9] single-binary: Restrict scope of TARGET_PAGE_BITS_MIN

[Pierrick Bouvier]

On 3/28/25 13:04, Richard Henderson wrote:
> With this, TARGET_PAGE_BITS_MIN no longer exists outside of
> page-vary-target.c, as that's the only place that needs the
> information.
> 
> Based-on: 20250318213209.2579218-1-richard.henderson@linaro.org
> ("[PATCH v2 00/42] accel/tcg, codebase: Build once patches")
> Based-on: 20250325224403.4011975-1-richard.henderson@linaro.org
> ("[PATCH v2 00/11] target/avr: Increase page size")
> Based-on: 20250328175526.368121-1-richard.henderson@linaro.org
> ("[PATCH 0/3] target/mips: Revert TARGET_PAGE_BITS_VARY and bug fixes")
> 
> Which is a lot, so for avoidance of doubt:
> https://gitlab.com/rth7680/qemu/-/commit/c8b593f1a907794b5767274cb3f5c70985638397
> 

I'll rebase my hw/arm single binary series 'single-binary: start make 
hw/arm/ common' on top of this series, so we can continue to expand the 
house of cards for this topic.

Feel free to pick the cpu-all cleanup part if it's interesting and ready 
for you.

Regards,
Pierrick

Re: [PATCH 9/9] page-vary: Restrict scope of TARGET_PAGE_BITS_MIN

[Richard Henderson]

On 3/28/25 15:38, Pierrick Bouvier wrote:
> Any chance we would rename TARGET_PAGE_BITS__MIN? (MIN_ALL? ALL_MIN? ARCH_MIN? any other 
> idea)
> I know it's restricted to this file only, but the __ is surprising.

I could drop the poisoning.  Since there have only ever been two targets using 
TARGET_PAGE_BITS_VARY, it's unlikely creep back in unawares.


r~