[PATCH v3 0/6] xen/riscv: fix boot on missing extensions and MMU setup bugs

[PATCH v3 0/6] xen/riscv: fix boot on missing extensions and MMU setup bugs

[Baptiste Le Duc]

This series introduces bugs fixes that were found while bringing up CI
support for the HiFive Premier P550 board with a basic smoke test. The
board-support series itself will follow separately as it depends on
PLIC/vPLIC and dom0less support that have not been upstreamed yet. This
series carries only the independent fixes found along the way, none of them
need the board-support series to apply.

This series:
    1: Docs: sync required ISA extensions with required_extensions[]
    2: Rename PTE "permissions" to "pte_flags"
    3: Fix A/D bit handling in G-stage mappings
    4: Fix A/D bit handling in Xen's page-table mappings
    5: Use pte_is_valid() in pte_is_mapping()
    6: Make Zihintpause no longer a required extension

CI pipeline:
https://gitlab.com/xen-project/people/baptleduc/xen/-/pipelines/2894144143

---
Changes since v2:
- Drop v2 patches 5 and 6, already committed:
    d8ca89f26c8a ("xen/riscv: flush speculatively cached Bare-mode TLB entries in turn_on_mmu()")
    08a898b93d21 ("xen/riscv: fix level_map_mask truncation on load_start")
- Drop "xen/riscv: make Svpbmt no longer a required extension".
  It relaxed Svpbmt so boards without it, like the HiFive P550, could boot.
  But without Svpbmt, DMA-noncoherent devices do not snoop the CPU caches
  and can read stale data from DRAM. Mainline Linux has the same problem,
  which is explained and reproducible in [1]. Fixing it needs a lot of
  machinery we do not need yet, and Svpbmt is mandatory in RVA23, so
  keeping it required is simpler and safer. An RFC will follow with a fix
  inspired by [2].
- Add three new patches:
    docs/riscv: sync required ISA extensions with required_extensions[]
    xen/riscv: rename PTE "permissions" to "pte_flags"
    xen/riscv: use pte_is_valid() in pte_is_mapping()
- Address review comments.

[1] https://github.com/davidlohr/vanilla-kernel-sifive-p550/tree/master
[2] https://lwn.net/Articles/996819/
---
Changes since v1:
- address ML comments
- rename some patchs
- add new patch-fix: 242dd1f890e4 ("xen/riscv: fix level_map_mask
  truncation on load_start") discovered when working on Spacemit K3 support

To: xen-devel@lists.xenproject.org
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Anthony PERARD <anthony.perard@vates.tech>
Cc: Michal Orzel <michal.orzel@amd.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Roger Pau Monné <roger@xenproject.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Alistair Francis <alistair.francis@wdc.com>
Cc: Connor Davis <connojdavis@gmail.com>
Cc: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Cc: Zheng Zhang <Zheng Zhang <zhangzheng@iscas.ac.cn>

---
Baptiste Le Duc (5):
      xen/riscv: rename PTE "permissions" to "pte_flags"
      xen/riscv: fix A/D bit handling in G-stage mappings
      xen/riscv: fix A/D bits in Xen's page-table mappings
      xen/riscv: use pte_is_valid() in pte_is_mapping()
      xen/riscv: make Zihintpause no longer a required extension

Oleksii Kurochko (1):
      docs/riscv: sync required ISA extensions with required_extensions[]

 docs/misc/riscv/booting.txt             | 27 +++++++++++-----
 xen/arch/riscv/cpufeature.c             | 56 ++++++++++++++++++++++++++++++++-
 xen/arch/riscv/include/asm/cpufeature.h |  1 +
 xen/arch/riscv/include/asm/mm.h         |  5 +--
 xen/arch/riscv/include/asm/page.h       | 35 +++++++++++----------
 xen/arch/riscv/include/asm/sbi.h        |  8 +++++
 xen/arch/riscv/mm.c                     | 11 +++----
 xen/arch/riscv/p2m.c                    | 44 ++++++++------------------
 8 files changed, 123 insertions(+), 64 deletions(-)
---
base-commit: 4c1aba82cff95ab11f7fe4af8965a06703537288
change-id: 20260902-riscv-fix-boot-missing-ext-a79c23bad694

Best regards,
--  
Baptiste Le Duc <baptiste.le-duc@vates.tech>

[PATCH v3 1/6] docs/riscv: sync required ISA extensions with required_extensions[]

[Baptiste Le Duc]

From: Oleksii Kurochko <oleksii.kurochko@gmail.com>

booting.txt listed only H, Zbb, Zihintpause and Svpbmt as required, while
required_extensions[] in cpufeature.c also checks for I, M, A, Zicsr,
Zifencei and, with CONFIG_RISCV_ISA_C=y, C. As the panic message printed
for a missing extension points to booting.txt, document the missing ones.

Also add a section pointing to riscv_isa_ext[] for the optional extensions
Xen recognises, and a comment above required_extensions[] to keep it in
sync with the document.

No functional change.

Suggested-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
Changes since v2:
- new patch
---
 docs/misc/riscv/booting.txt | 24 ++++++++++++++++++++----
 xen/arch/riscv/cpufeature.c |  1 +
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/docs/misc/riscv/booting.txt b/docs/misc/riscv/booting.txt
index e100bde575..c99ccafa74 100644
--- a/docs/misc/riscv/booting.txt
+++ b/docs/misc/riscv/booting.txt
@@ -1,8 +1,12 @@
 System requirements
 ===================
 
-The following extensions are expected to be supported by a system on which
-Xen is run:
+The following extensions are required by Xen. Xen panics at boot if any of
+them is missing from the riscv,isa property of the CPU nodes:
+- I, M, A, Zicsr, Zifencei:
+  Base ISA Xen is compiled for.
+- C:
+  Required only when Xen is built with CONFIG_RISCV_ISA_C=y.
 - H:
   Provides additional instructions and CSRs that control the new stage of
   address translation and support hosting a guest OS in virtual S-mode
@@ -18,7 +22,19 @@ Xen is run:
 - Zihintpause:
   On a system that doesn't have this extension, cpu_relax() should be
   implemented properly.
-- SVPBMT is mandatory to enable changing the memory attributes of a page.
-  For platforms that do not support SVPBMT, it is necessary to introduce a
+- Svpbmt:
+  Mandatory to enable changing the memory attributes of a page.
+  For platforms that do not support Svpbmt, it is necessary to introduce a
   similar mechanism as described in:
   https://lore.kernel.org/all/20241102000843.1301099-1-samuel.holland@sifive.com/
+
+Note: this list must be kept in sync with required_extensions[] in
+      xen/arch/riscv/cpufeature.c.
+
+Optional extensions
+===================
+
+Other extensions recognised by Xen are listed in riscv_isa_ext[] in
+xen/arch/riscv/cpufeature.c. Xen uses them when present, but they are not
+needed to boot. The second column of that table says whether the extension
+is also exposed to guests.
diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c
index aaf544d13f..2d12dffae7 100644
--- a/xen/arch/riscv/cpufeature.c
+++ b/xen/arch/riscv/cpufeature.c
@@ -203,6 +203,7 @@ static const struct riscv_isa_ext_entry __initconstrel riscv_isa_ext[] = {
     RISCV_ISA_EXT_ENTRY(svpbmt,         NONE),
 };
 
+/* Keep in sync with docs/misc/riscv/booting.txt. */
 static const struct riscv_isa_ext_data __initconst required_extensions[] = {
     RISCV_ISA_EXT_DATA(i),
     RISCV_ISA_EXT_DATA(m),

-- 
2.55.0

[PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Baptiste Le Duc]

paddr_to_pte()'s "permissions" parameter, the matching local in
setup_initial_mapping() and p2m_set_permission() don't only deal with
permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
PTE_DIRTY.

Rename them to "pte_flags" and p2m_set_pte_flags() respectively.

No functional change.

Requested-by: Jan Beulich <jbeulich@suse.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
Changes since v2:
- new patch
---
 xen/arch/riscv/include/asm/mm.h | 5 +++--
 xen/arch/riscv/mm.c             | 8 ++++----
 xen/arch/riscv/p2m.c            | 4 ++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 9e28c24954..1ac66283ec 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -22,9 +22,10 @@ extern vaddr_t directmap_virt_start;
 #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
 
 static inline pte_t paddr_to_pte(paddr_t paddr,
-                                 unsigned int permissions)
+                                 unsigned int pte_flags)
 {
-    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) | permissions };
+    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) |
+                            pte_flags };
 }
 
 static inline paddr_t pte_to_paddr(pte_t pte)
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 610d111945..0e26c5751f 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -140,7 +140,7 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
         case 1: /* Level 0 */
             {
                 unsigned long paddr = (page_addr - map_start) + pa_start;
-                unsigned int permissions = PTE_LEAF_DEFAULT;
+                unsigned int pte_flags = PTE_LEAF_DEFAULT;
                 unsigned long addr = is_identity_mapping
                                      ? page_addr : virt_to_maddr(page_addr);
                 pte_t pte_to_be_written;
@@ -149,13 +149,13 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
 
                 if ( is_kernel_text(addr) ||
                      is_kernel_inittext(addr) )
-                        permissions =
+                        pte_flags =
                             PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;
 
                 if ( is_kernel_rodata(addr) )
-                    permissions = PTE_READABLE | PTE_VALID;
+                    pte_flags = PTE_READABLE | PTE_VALID;
 
-                pte_to_be_written = paddr_to_pte(paddr, permissions);
+                pte_to_be_written = paddr_to_pte(paddr, pte_flags);
 
                 if ( !pte_is_valid(pgtbl[index]) )
                     pgtbl[index] = pte_to_be_written;
diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
index 1cea86512c..f7b380b90a 100644
--- a/xen/arch/riscv/p2m.c
+++ b/xen/arch/riscv/p2m.c
@@ -584,7 +584,7 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
     p2m_write_pte(p, pte, clean_cache);
 }
 
-static void p2m_set_permission(pte_t *e, p2m_type_t t)
+static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
 {
     e->pte &= ~PTE_ACCESS_MASK;
 
@@ -676,7 +676,7 @@ static pte_t p2m_pte_from_mfn(mfn_t mfn, p2m_type_t t,
             break;
         }
 
-        p2m_set_permission(&e, t);
+        p2m_set_pte_flags(&e, t);
         p2m_set_type(&e, t, ctx);
     }
     else

-- 
2.55.0

[PATCH v3 3/6] xen/riscv: fix A/D bit handling in G-stage mappings

[Baptiste Le Duc]

There are two schemes for managing the PTE A/D bits: either a page fault is
raised when an access requires A or D to be set (ratified as Svade), or
hardware updates the bits itself (ratified as Svadu).

p2m_set_pte_flags() presets the A/D bits in G-stage PTEs only when Svade is
present in the device tree. When neither Svade nor Svadu is present, this
causes an unhandled page fault.

The four possible Svade/Svadu combinations in the device tree mean (see
[1]):
- neither present: the scheme is unknown, so the A/D bits have to be
  preset. This is harmless if hardware actually updates them itself.
- only Svade present: page faults, so the A/D bits have to be preset.
- only Svadu present: hardware updates the A/D bits.
- both present: hardware updating is off at boot and has to be enabled
  through the SBI FWFT extension, which Xen doesn't support yet, so the A/D
  bits have to be preset.

Hence preset the A/D bits in p2m_set_pte_flags() unless only Svadu is
present, i.e. when (!svadu || svade). Add riscv_resolve_ad_scheme(), called
once from riscv_fill_hwcap(), to warn when both are present but SBI FWFT is
missing, as dropping 'svade' from the DT is then the only way to get Svadu.

[1] https://lwn.net/Articles/980016/

Fixes: ff14053983b0 ("xen/riscv: Implement p2m_pte_from_mfn() and support PBMT configuration")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
Changes since v2:
- change commit title.
- preset A/D bits in p2m_set_pte_flags() if (!svadu || svade), instead of
setting RISCV_ISA_EXT_svade in the riscv_isa bitmap for cases 1, 2 and 4.
- riscv_resolve_ad_scheme() now only warns when both Svade and Svadu are
present and SBI FWFT is missing; drop the dead 'svadu && svade' operand.
- repeat XENLOG_WARNING on each line of the warning.
- drop ASSERT(svade != svadu) from p2m_set_pte_flags().
- only add the sbi_probe_extension() declaration to sbi.h.
- rework the comment in p2m_set_pte_flags().
---
Changes since v1:
- change commit title
- expose RISCV_ISA_EXT_svadu so the two extensions can be told apart.
- move the Svade/Svadu resolution to a new riscv_resolve_ad_scheme(),
  called once from riscv_fill_hwcap().
- expose sbi_probe_extension() (was static) to probe for SBI FWFT.
- stop presetting A/D bits unconditionally in p2m_set_permission(), do it
  only when Svade is present.
---
 xen/arch/riscv/cpufeature.c             | 43 +++++++++++++++++++++++++++++++++
 xen/arch/riscv/include/asm/cpufeature.h |  1 +
 xen/arch/riscv/include/asm/sbi.h        |  8 ++++++
 xen/arch/riscv/p2m.c                    | 40 +++++++++---------------------
 4 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c
index 2d12dffae7..ee35c7a17c 100644
--- a/xen/arch/riscv/cpufeature.c
+++ b/xen/arch/riscv/cpufeature.c
@@ -20,6 +20,7 @@
 
 #include <asm/cpufeature.h>
 #include <asm/csr.h>
+#include <asm/sbi.h>
 
 #ifdef CONFIG_ACPI
 # error "cpufeature.c functions should be updated to support ACPI"
@@ -200,6 +201,7 @@ static const struct riscv_isa_ext_entry __initconstrel riscv_isa_ext[] = {
     RISCV_ISA_EXT_ENTRY(ssaia,          ANY),
     RISCV_ISA_EXT_ENTRY(sstc,           NONE),
     RISCV_ISA_EXT_ENTRY(svade,          NONE),
+    RISCV_ISA_EXT_ENTRY(svadu,          NONE),
     RISCV_ISA_EXT_ENTRY(svpbmt,         NONE),
 };
 
@@ -500,6 +502,45 @@ static void __init riscv_fill_hwcap_from_isa_string(void)
     }
 }
 
+/*
+ * Svade and Svadu extensions represent two schemes for managing the PTE A/D
+ * bits. When the PTE A/D bits need to be set, the Svade extension indicates
+ * that a page fault will be raised. In contrast, the Svadu extension supports
+ * hardware updating of the PTE A/D bits.
+ *
+ * There are 4 possible combinations of these extensions in the device tree.
+ * The default hardware behavior for each is:
+ *
+ * 1) Neither Svade nor Svadu present in DT => It is technically unknown
+ *    whether the platform uses Svade or Svadu. Xen should be prepared to
+ *    handle either hardware updating of the PTE A/D bits or page faults when
+ *    they need updating.
+ *
+ * 2) Only Svade present in DT => Xen must assume Svade to be always enabled.
+ *
+ * 3) Only Svadu present in DT => Xen must assume Svadu to be always enabled.
+ *
+ * 4) Both Svade and Svadu present in DT => Xen must assume Svadu is turned off
+ *    at boot time, so it presets the A/D bits. To use Svadu, the supervisor
+ *    must explicitly enable it using the SBI FWFT extension.
+ *
+ * The Svade extension is mandatory and the Svadu extension is optional in the
+ * RVA23 profile. Platforms wanting to take advantage of Svadu can choose
+ * option 3. Platforms aware of the profile can choose option 4, and Xen won't
+ * get the benefit of Svadu until the SBI FWFT extension is available.
+ */
+static void __init riscv_resolve_ad_scheme(void)
+{
+    bool svade = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade);
+    bool svadu = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu);
+
+    if ( svadu && svade )
+        if ( sbi_probe_extension(SBI_EXT_FWFT) <= 0 )
+            printk(XENLOG_WARNING "RISC-V: Both Svade and Svadu detected, but SBI FWFT is missing.\n"
+                XENLOG_WARNING "RISC-V: Defaulting to software A/D updates (Svade).\n"
+                XENLOG_WARNING "RISC-V: To force hardware A/D updates (Svadu), remove 'svade' from DT.\n");
+}
+
 static bool __init has_isa_extensions_property(void)
 {
     const struct dt_device_node *cpus = dt_find_node_by_path("/cpus");
@@ -664,6 +705,8 @@ void __init riscv_fill_hwcap(void)
         __set_bit(RISCV_ISA_EXT_sstc, riscv_isa);
     }
 
+    riscv_resolve_ad_scheme();
+
     for ( i = 0; i < req_extns_amount; i++ )
     {
         const struct riscv_isa_ext_data ext = required_extensions[i];
diff --git a/xen/arch/riscv/include/asm/cpufeature.h b/xen/arch/riscv/include/asm/cpufeature.h
index 2973eb13a5..7a448d6111 100644
--- a/xen/arch/riscv/include/asm/cpufeature.h
+++ b/xen/arch/riscv/include/asm/cpufeature.h
@@ -41,6 +41,7 @@ enum riscv_isa_ext_id {
     RISCV_ISA_EXT_ssaia,
     RISCV_ISA_EXT_sstc,
     RISCV_ISA_EXT_svade,
+    RISCV_ISA_EXT_svadu,
     RISCV_ISA_EXT_svpbmt,
     RISCV_ISA_EXT_MAX
 };
diff --git a/xen/arch/riscv/include/asm/sbi.h b/xen/arch/riscv/include/asm/sbi.h
index 1952868e96..4efa166603 100644
--- a/xen/arch/riscv/include/asm/sbi.h
+++ b/xen/arch/riscv/include/asm/sbi.h
@@ -30,6 +30,7 @@
 #define SBI_EXT_BASE                    0x10
 #define SBI_EXT_RFENCE                  0x52464E43
 #define SBI_EXT_TIME                    0x54494D45
+#define SBI_EXT_FWFT                    0x46574654
 
 /* SBI function IDs for BASE extension */
 #define SBI_EXT_BASE_GET_SPEC_VERSION   0x0
@@ -138,6 +139,13 @@ int sbi_remote_hfence_gvma(const cpumask_t *cpu_mask, vaddr_t start,
 int sbi_remote_hfence_gvma_vmid(const cpumask_t *cpu_mask, vaddr_t start,
                                 size_t size, unsigned long vmid);
 
+/*
+ * Check if an SBI extension ID is supported or not.
+ *
+ * @return: > 0 if supported, 0 if not, negative errno on SBI error.
+ */
+int sbi_probe_extension(long extid);
+
 /*
  * Initialize SBI library
  *
diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
index f7b380b90a..5c8e480050 100644
--- a/xen/arch/riscv/p2m.c
+++ b/xen/arch/riscv/p2m.c
@@ -586,42 +586,24 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
 
 static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
 {
+    bool svade = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade);
+    bool svadu = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu);
+
     e->pte &= ~PTE_ACCESS_MASK;
 
     e->pte |= PTE_USER;
 
     /*
-     * Two schemes to manage the A and D bits are defined:
-     *   • The Svade extension: when a virtual page is accessed and the A bit
-     *     is clear, or is written and the D bit is clear, a page-fault
-     *     exception is raised.
-     *   • When the Svade extension is not implemented, the following scheme
-     *     applies.
-     *     When a virtual page is accessed and the A bit is clear, the PTE is
-     *     updated to set the A bit. When the virtual page is written and the
-     *     D bit is clear, the PTE is updated to set the D bit. When G-stage
-     *     address translation is in use and is not Bare, the G-stage virtual
-     *     pages may be accessed or written by implicit accesses to VS-level
-     *     memory management data structures, such as page tables.
-     * Thereby to avoid a page-fault in case of Svade is available, it is
-     * necessary to set A and D bits.
-     *
-     * TODO: For now, it’s fine to simply set the A/D bits, since OpenSBI
-     *       delegates page faults to a lower privilege mode and so OpenSBI
-     *       isn't expect to handle page-faults occured in lower modes.
-     *       By setting the A/D bits here, page faults that would otherwise
-     *       be generated due to unset A/D bits will not occur in Xen.
-     *
-     *       Currently, Xen on RISC-V does not make use of the information
-     *       that could be obtained from handling such page faults, which
-     *       could otherwise be useful for several use cases such as demand
-     *       paging, cache-flushing optimizations, memory access tracking,etc.
+     * A RISC-V implementation can choose to either:
+     * 1) Update 'A' and 'D' PTE bits in hardware.
+     * 2) Generate page fault when 'A' and/or 'D' PTE bits are not set so that
+     *    software can update these bits.
      *
-     *       To support the more general case and the optimizations mentioned
-     *       above, it would be better to stop setting the A/D bits here and
-     *       instead handle page faults that occur due to unset A/D bits.
+     * Xen supports both options mentioned above: unless the platform guarantees
+     * (1), i.e. only Svadu is present, set 'A' and 'D' so that (2) never
+     * faults.
      */
-    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) )
+    if ( !svadu || svade )
         e->pte |= PTE_ACCESSED | PTE_DIRTY;
 
     switch ( t )

-- 
2.55.0

[PATCH v3 4/6] xen/riscv: fix A/D bits in Xen's page-table mappings

[Baptiste Le Duc]

Xen does not handle page faults caused by clear A/D bits, so it presets
them when creating PTEs. pt_update_entry() does so, but
setup_initial_mapping() (the boot page tables) and arch_pmap_map() (the
fixmap) build leaf PTEs directly without going through it. Without Svadu,
both would fault on first access.

Add PTE_ACCESSED and PTE_DIRTY to PAGE_HYPERVISOR_RO, and build
PAGE_HYPERVISOR_RW and PAGE_HYPERVISOR_RX on top of it. PTE_DIRTY is set in
all cases for consistency with pt_update_entry() which sets it at runtime.

This fixes arch_pmap_map() for free, since it already builds its PTE from
PAGE_HYPERVISOR_RW. Switch setup_initial_mapping() to use these macros for
its default, text and rodata permissions, and for the temporary root entry
built by check_pgtbl_mode_support(), instead of the equivalent raw bit
lists. The latter drops PTE_WRITABLE, going from RWX to RX, but this is
harmless, as that entry only has to make the current instruction stream
fetchable between the two CSR_SATP writes used to probe SATP mode support,
and nothing writes through it.

Drop the now-redundant PTE_LEAF_DEFAULT, since converting the last
open-coded site above leaves it with no user outside page.h itself.

pte_is_table() and pte_is_mapping() both assert that a PTE doesn't use one
of the two reserved encodings, (V=1, W=1, R=0) and (V=1, X=1, W=1, R=0), by
masking it with PAGE_HYPERVISOR_RW. Now that PAGE_HYPERVISOR_RW also
carries A and D, a reserved PTE with A or D set would no longer be caught.
Mask with the V, R and W bits explicitly instead, and factor the check out
into pte_is_reserved().

Fixes: e66003e7be19 ("xen/riscv: introduce setup_initial_pages")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
Changes since v2:
- add fixes commit ref.
- add A/D bits to PAGE_HYPERVISOR_RO and derive PAGE_HYPERVISOR_RW/RX
  from it for consistency with runtime.
- introduce pte_is_reserved() to factor out the reserved-encoding assert
  shared by pte_is_table() and pte_is_mapping().
- reword commit title
---
Changes since v1:
- change commit title
- mention in patch message that arch_pmap_map() is fixed too, via the
  PAGE_HYPERVISOR_RW change, not just setup_initial_mapping().
- convert check_pgtbl_mode_support()'s temporary root entry to
  PAGE_HYPERVISOR_RX, as it's harmless.
- drop PTE_LEAF_DEFAULT entirely instead of keeping it, now that no site
  open-codes it anymore.
- drop the pte_is_table() comment line that referenced PAGE_HYPERVISOR_RW,
  now stale.
---
 xen/arch/riscv/include/asm/page.h | 33 +++++++++++++++++----------------
 xen/arch/riscv/mm.c               |  9 ++++-----
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/xen/arch/riscv/include/asm/page.h b/xen/arch/riscv/include/asm/page.h
index b465a90325..7772e2f572 100644
--- a/xen/arch/riscv/include/asm/page.h
+++ b/xen/arch/riscv/include/asm/page.h
@@ -46,12 +46,12 @@
 #define PTE_PBMT_NOCACHE            BIT(61, UL)
 #define PTE_PBMT_IO                 BIT(62, UL)
 
-#define PTE_LEAF_DEFAULT            (PTE_VALID | PTE_READABLE | PTE_WRITABLE)
 #define PTE_TABLE                   (PTE_VALID)
 
-#define PAGE_HYPERVISOR_RO          (PTE_VALID | PTE_READABLE)
-#define PAGE_HYPERVISOR_RW          (PTE_VALID | PTE_READABLE | PTE_WRITABLE)
-#define PAGE_HYPERVISOR_RX          (PTE_VALID | PTE_READABLE | PTE_EXECUTABLE)
+#define PAGE_HYPERVISOR_RO          (PTE_VALID | PTE_READABLE | \
+                                     PTE_ACCESSED | PTE_DIRTY)
+#define PAGE_HYPERVISOR_RW          (PAGE_HYPERVISOR_RO | PTE_WRITABLE)
+#define PAGE_HYPERVISOR_RX          (PAGE_HYPERVISOR_RO | PTE_EXECUTABLE)
 
 #define PAGE_HYPERVISOR             PAGE_HYPERVISOR_RW
 /*
@@ -161,31 +161,32 @@ static inline bool pte_is_valid(pte_t p)
  *      X W R Meaning
  *      0 0 0 Pointer to next level of page table.
  *      0 0 1 Read-only page.
- *      0 1 0 Reserved for future use.
+ *      0 1 0 Reserved for future use. [1]
  *      0 1 1 Read-write page.
  *      1 0 0 Execute-only page.
  *      1 0 1 Read-execute page.
- *      1 1 0 Reserved for future use.
+ *      1 1 0 Reserved for future use. [2]
  *      1 1 1 Read-write-execute page.
+ *
+ *   So if V=1 and W=1 then R also needs to be 1 as R = 0 is reserved for
+ *   future use ([1], [2]).
  */
+static inline bool pte_is_reserved(pte_t p)
+{
+    return (p.pte & (PTE_VALID | PTE_READABLE | PTE_WRITABLE)) ==
+           (PTE_VALID | PTE_WRITABLE);
+}
+
 static inline bool pte_is_table(pte_t p)
 {
-    /*
-     * According to the spec if V=1 and W=1 then R also needs to be 1 as
-     * R = 0 is reserved for future use ( look at the Table 4.5 ) so check
-     * in ASSERT that if (V==1 && W==1) then R isn't 0.
-     *
-     * PAGE_HYPERVISOR_RW contains PTE_VALID too.
-     */
-    ASSERT(((p.pte & PAGE_HYPERVISOR_RW) != (PTE_VALID | PTE_WRITABLE)));
+    ASSERT(!pte_is_reserved(p));
 
     return ((p.pte & (PTE_VALID | PTE_ACCESS_MASK)) == PTE_VALID);
 }
 
 static inline bool pte_is_mapping(pte_t p)
 {
-    /* See pte_is_table() */
-    ASSERT(((p.pte & PAGE_HYPERVISOR_RW) != (PTE_VALID | PTE_WRITABLE)));
+    ASSERT(!pte_is_reserved(p));
 
     return (p.pte & PTE_VALID) && (p.pte & PTE_ACCESS_MASK);
 }
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 0e26c5751f..b7b9bff0bf 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -140,7 +140,7 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
         case 1: /* Level 0 */
             {
                 unsigned long paddr = (page_addr - map_start) + pa_start;
-                unsigned int pte_flags = PTE_LEAF_DEFAULT;
+                unsigned int pte_flags = PAGE_HYPERVISOR_RW;
                 unsigned long addr = is_identity_mapping
                                      ? page_addr : virt_to_maddr(page_addr);
                 pte_t pte_to_be_written;
@@ -149,11 +149,10 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
 
                 if ( is_kernel_text(addr) ||
                      is_kernel_inittext(addr) )
-                        pte_flags =
-                            PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;
+                    pte_flags = PAGE_HYPERVISOR_RX;
 
                 if ( is_kernel_rodata(addr) )
-                    pte_flags = PTE_READABLE | PTE_VALID;
+                    pte_flags = PAGE_HYPERVISOR_RO;
 
                 pte_to_be_written = paddr_to_pte(paddr, pte_flags);
 
@@ -198,7 +197,7 @@ static bool __init check_pgtbl_mode_support(struct mmu_desc *mmu_desc,
 
     index = pt_index(page_table_level, aligned_load_start);
     stage1_pgtbl_root[index] = paddr_to_pte(aligned_load_start,
-                                            PTE_LEAF_DEFAULT | PTE_EXECUTABLE);
+                                            PAGE_HYPERVISOR_RX);
 
     sfence_vma();
     csr_write(CSR_SATP,

-- 
2.55.0

[PATCH v3 5/6] xen/riscv: use pte_is_valid() in pte_is_mapping()

[Baptiste Le Duc]

Use pte_is_valid() instead of open-coding the PTE_VALID check.

No functional change.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
Changes since v2:
- new patch.
---
 xen/arch/riscv/include/asm/page.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/riscv/include/asm/page.h b/xen/arch/riscv/include/asm/page.h
index 7772e2f572..d357223ce2 100644
--- a/xen/arch/riscv/include/asm/page.h
+++ b/xen/arch/riscv/include/asm/page.h
@@ -188,7 +188,7 @@ static inline bool pte_is_mapping(pte_t p)
 {
     ASSERT(!pte_is_reserved(p));
 
-    return (p.pte & PTE_VALID) && (p.pte & PTE_ACCESS_MASK);
+    return pte_is_valid(p) && (p.pte & PTE_ACCESS_MASK);
 }
 
 static inline bool pte_is_superpage(pte_t p, unsigned int level)

-- 
2.55.0

[PATCH v3 6/6] xen/riscv: make Zihintpause no longer a required extension

[Baptiste Le Duc]

Zihintpause is in required_extensions[] so Xen panics when it is missing,
while it does not strictly require hardware support for it.

The PAUSE hint (Zihintpause extension) is encoded as FENCE W, 0
(0x0100000F). In accordance with the RISC-V Unprivileged ISA, HINTs are
encoded in the space of valid standard instructions. On platforms without
Zihintpause support, executing 0x0100000F is treated as a standard FENCE
with an empty successor set, which acts as a NOP and never generates an
illegal instruction trap.

Zihintpause is therefore purely an optimization hint. Drop it from
required_extensions[] so Xen can still boot on systems that don't advertise
it, and print a warning in that case.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
Changes since v2:
- rewrite commit message
- print a warning at boot when Zihintpause is unavailable
- drop Zihintpause requirement from booting.txt
---
Changes since v1:
- rewrite commit message.
---
 docs/misc/riscv/booting.txt |  3 ---
 xen/arch/riscv/cpufeature.c | 12 +++++++++++-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/docs/misc/riscv/booting.txt b/docs/misc/riscv/booting.txt
index c99ccafa74..14212ed62f 100644
--- a/docs/misc/riscv/booting.txt
+++ b/docs/misc/riscv/booting.txt
@@ -19,9 +19,6 @@ them is missing from the riscv,isa property of the CPU nodes:
   a very simple sequence.
   The similar issue occurs with other __builtin_<bitop>, so it is needed to
   provide a generic version of bitops in RISC-V bitops.h
-- Zihintpause:
-  On a system that doesn't have this extension, cpu_relax() should be
-  implemented properly.
 - Svpbmt:
   Mandatory to enable changing the memory attributes of a page.
   For platforms that do not support Svpbmt, it is necessary to introduce a
diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c
index ee35c7a17c..91404daac5 100644
--- a/xen/arch/riscv/cpufeature.c
+++ b/xen/arch/riscv/cpufeature.c
@@ -216,7 +216,6 @@ static const struct riscv_isa_ext_data __initconst required_extensions[] = {
     RISCV_ISA_EXT_DATA(h),
     RISCV_ISA_EXT_DATA(zicsr),
     RISCV_ISA_EXT_DATA(zifencei),
-    RISCV_ISA_EXT_DATA(zihintpause),
     RISCV_ISA_EXT_DATA(zbb),
     RISCV_ISA_EXT_DATA(svpbmt),
 };
@@ -707,6 +706,17 @@ void __init riscv_fill_hwcap(void)
 
     riscv_resolve_ad_scheme();
 
+    /*
+     * Zihintpause isn't mandatory: the encoding used by cpu_relax() is a
+     * HINT which executes as a no-op on hardware without the extension.
+     * Report it, as a platform may provide its own way to hint a spin-wait
+     * loop, which then has to be wired up in cpu_relax().
+     */
+    if ( !riscv_isa_extension_available(NULL, RISCV_ISA_EXT_zihintpause) )
+        printk(XENLOG_WARNING
+               "Zihintpause unavailable: cpu_relax() gives the CPU no hint; "
+               "wire up this platform's pause equivalent in cpu_relax()\n");
+
     for ( i = 0; i < req_extns_amount; i++ )
     {
         const struct riscv_isa_ext_data ext = required_extensions[i];

-- 
2.55.0

Re: [PATCH v3 1/6] docs/riscv: sync required ISA extensions with required_extensions[]

[Jan Beulich]

On 29.09.2026 18:32, Baptiste Le Duc wrote:
> From: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> booting.txt listed only H, Zbb, Zihintpause and Svpbmt as required, while
> required_extensions[] in cpufeature.c also checks for I, M, A, Zicsr,
> Zifencei and, with CONFIG_RISCV_ISA_C=y, C. As the panic message printed
> for a missing extension points to booting.txt, document the missing ones.
> 
> Also add a section pointing to riscv_isa_ext[] for the optional extensions
> Xen recognises, and a comment above required_extensions[] to keep it in
> sync with the document.
> 
> No functional change.
> 
> Suggested-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v3 1/6] docs/riscv: sync required ISA extensions with required_extensions[]

[Oleksii Kurochko]

On 9/29/26 6:32 PM, Baptiste Le Duc wrote:
> From: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> booting.txt listed only H, Zbb, Zihintpause and Svpbmt as required, while
> required_extensions[] in cpufeature.c also checks for I, M, A, Zicsr,
> Zifencei and, with CONFIG_RISCV_ISA_C=y, C. As the panic message printed
> for a missing extension points to booting.txt, document the missing ones.
> 
> Also add a section pointing to riscv_isa_ext[] for the optional extensions
> Xen recognises, and a comment above required_extensions[] to keep it in
> sync with the document.
> 
> No functional change.
> 
> Suggested-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>

IDK if it make sense for this patch but just in case:

Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

Re: [PATCH v3 6/6] xen/riscv: make Zihintpause no longer a required extension

[Oleksii Kurochko]

On 9/29/26 6:32 PM, Baptiste Le Duc wrote:
> Zihintpause is in required_extensions[] so Xen panics when it is missing,
> while it does not strictly require hardware support for it.
> 
> The PAUSE hint (Zihintpause extension) is encoded as FENCE W, 0
> (0x0100000F). In accordance with the RISC-V Unprivileged ISA, HINTs are
> encoded in the space of valid standard instructions. On platforms without
> Zihintpause support, executing 0x0100000F is treated as a standard FENCE
> with an empty successor set, which acts as a NOP and never generates an
> illegal instruction trap.
> 
> Zihintpause is therefore purely an optimization hint. Drop it from
> required_extensions[] so Xen can still boot on systems that don't advertise
> it, and print a warning in that case.
> 
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> ---
> Changes since v2:
> - rewrite commit message
> - print a warning at boot when Zihintpause is unavailable
> - drop Zihintpause requirement from booting.txt
> ---
> Changes since v1:
> - rewrite commit message.
> ---
LGTM:

Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

Re: [PATCH v3 5/6] xen/riscv: use pte_is_valid() in pte_is_mapping()

[Oleksii Kurochko]

On 9/29/26 6:32 PM, Baptiste Le Duc wrote:
> Use pte_is_valid() instead of open-coding the PTE_VALID check.
> 
> No functional change.
> 
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>

LGTM:
  Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

Re: [PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Jan Beulich]

On 29.09.2026 18:32, Baptiste Le Duc wrote:
> paddr_to_pte()'s "permissions" parameter, the matching local in
> 
> setup_initial_mapping() and p2m_set_permission() don't only deal with
> 
> permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
> 
> PTE_DIRTY.
> 
> 
> 
> Rename them to "pte_flags" and p2m_set_pte_flags() respectively.
> 
> 
> 
> No functional change.
> 
> 
> 
> Requested-by: Jan Beulich <jbeulich@suse.com>
> 
> Assisted-by: Claude:claude-opus-5
> 
> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>

In principle
Acked-by: Jan Beulich <jbeulich@suse.com>

I won't, however, be able to apply this patch as-is. As you can see both above
and below, extra newlines were inserted everywhere on the way here. The list
archive doesn't show this issue, but instead shows undue wrapped lines. It is
also only this one patch in the series which has this issue. You will need to
resend for me to properly consume.

Jan

> ---
> 
> Changes since v2:
> 
> - new patch
> 
> ---
> 
>  xen/arch/riscv/include/asm/mm.h | 5 +++--
> 
>  xen/arch/riscv/mm.c             | 8 ++++----
> 
>  xen/arch/riscv/p2m.c            | 4 ++--
> 
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> 
> 
> diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
> 
> index 9e28c24954..1ac66283ec 100644
> 
> --- a/xen/arch/riscv/include/asm/mm.h
> 
> +++ b/xen/arch/riscv/include/asm/mm.h
> 
> @@ -22,9 +22,10 @@ extern vaddr_t directmap_virt_start;
> 
>  #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
> 
>  
> 
>  static inline pte_t paddr_to_pte(paddr_t paddr,
> 
> -                                 unsigned int permissions)
> 
> +                                 unsigned int pte_flags)
> 
>  {
> 
> -    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) | permissions };
> 
> +    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) |
> 
> +                            pte_flags };
> 
>  }
> 
>  
> 
>  static inline paddr_t pte_to_paddr(pte_t pte)
> 
> diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
> 
> index 610d111945..0e26c5751f 100644
> 
> --- a/xen/arch/riscv/mm.c
> 
> +++ b/xen/arch/riscv/mm.c
> 
> @@ -140,7 +140,7 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
> 
>          case 1: /* Level 0 */
> 
>              {
> 
>                  unsigned long paddr = (page_addr - map_start) + pa_start;
> 
> -                unsigned int permissions = PTE_LEAF_DEFAULT;
> 
> +                unsigned int pte_flags = PTE_LEAF_DEFAULT;
> 
>                  unsigned long addr = is_identity_mapping
> 
>                                       ? page_addr : virt_to_maddr(page_addr);
> 
>                  pte_t pte_to_be_written;
> 
> @@ -149,13 +149,13 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
> 
>  
> 
>                  if ( is_kernel_text(addr) ||
> 
>                       is_kernel_inittext(addr) )
> 
> -                        permissions =
> 
> +                        pte_flags =
> 
>                              PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;
> 
>  
> 
>                  if ( is_kernel_rodata(addr) )
> 
> -                    permissions = PTE_READABLE | PTE_VALID;
> 
> +                    pte_flags = PTE_READABLE | PTE_VALID;
> 
>  
> 
> -                pte_to_be_written = paddr_to_pte(paddr, permissions);
> 
> +                pte_to_be_written = paddr_to_pte(paddr, pte_flags);
> 
>  
> 
>                  if ( !pte_is_valid(pgtbl[index]) )
> 
>                      pgtbl[index] = pte_to_be_written;
> 
> diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
> 
> index 1cea86512c..f7b380b90a 100644
> 
> --- a/xen/arch/riscv/p2m.c
> 
> +++ b/xen/arch/riscv/p2m.c
> 
> @@ -584,7 +584,7 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
> 
>      p2m_write_pte(p, pte, clean_cache);
> 
>  }
> 
>  
> 
> -static void p2m_set_permission(pte_t *e, p2m_type_t t)
> 
> +static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
> 
>  {
> 
>      e->pte &= ~PTE_ACCESS_MASK;
> 
>  
> 
> @@ -676,7 +676,7 @@ static pte_t p2m_pte_from_mfn(mfn_t mfn, p2m_type_t t,
> 
>              break;
> 
>          }
> 
>  
> 
> -        p2m_set_permission(&e, t);
> 
> +        p2m_set_pte_flags(&e, t);
> 
>          p2m_set_type(&e, t, ctx);
> 
>      }
> 
>      else
> 
> 
> 

Re: [PATCH v3 3/6] xen/riscv: fix A/D bit handling in G-stage mappings

[Jan Beulich]

On 29.09.2026 18:32, Baptiste Le Duc wrote:
> @@ -500,6 +502,45 @@ static void __init riscv_fill_hwcap_from_isa_string(void)
>      }
>  }
>  
> +/*
> + * Svade and Svadu extensions represent two schemes for managing the PTE A/D
> + * bits. When the PTE A/D bits need to be set, the Svade extension indicates
> + * that a page fault will be raised. In contrast, the Svadu extension supports
> + * hardware updating of the PTE A/D bits.
> + *
> + * There are 4 possible combinations of these extensions in the device tree.
> + * The default hardware behavior for each is:
> + *
> + * 1) Neither Svade nor Svadu present in DT => It is technically unknown
> + *    whether the platform uses Svade or Svadu. Xen should be prepared to
> + *    handle either hardware updating of the PTE A/D bits or page faults when
> + *    they need updating.
> + *
> + * 2) Only Svade present in DT => Xen must assume Svade to be always enabled.
> + *
> + * 3) Only Svadu present in DT => Xen must assume Svadu to be always enabled.
> + *
> + * 4) Both Svade and Svadu present in DT => Xen must assume Svadu is turned off
> + *    at boot time, so it presets the A/D bits. To use Svadu, the supervisor
> + *    must explicitly enable it using the SBI FWFT extension.
> + *
> + * The Svade extension is mandatory and the Svadu extension is optional in the
> + * RVA23 profile. Platforms wanting to take advantage of Svadu can choose
> + * option 3. Platforms aware of the profile can choose option 4, and Xen won't
> + * get the benefit of Svadu until the SBI FWFT extension is available.
> + */
> +static void __init riscv_resolve_ad_scheme(void)
> +{
> +    bool svade = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade);
> +    bool svadu = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu);
> +
> +    if ( svadu && svade )
> +        if ( sbi_probe_extension(SBI_EXT_FWFT) <= 0 )

Such successive if()s want folding.

> +            printk(XENLOG_WARNING "RISC-V: Both Svade and Svadu detected, but SBI FWFT is missing.\n"
> +                XENLOG_WARNING "RISC-V: Defaulting to software A/D updates (Svade).\n"
> +                XENLOG_WARNING "RISC-V: To force hardware A/D updates (Svadu), remove 'svade' from DT.\n");

Nit: Indentation, and no full stop at the end of log messages please.

Overall - what use is this? Nothing will be logged if FWFT is present, yet
there's no real use of the extension. I.e. even in that case you default
to svade. Further, removing svadu from DT doesn't alter hardware behavior.
How can that be a useful suggestion?

> --- a/xen/arch/riscv/p2m.c
> +++ b/xen/arch/riscv/p2m.c
> @@ -586,42 +586,24 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
>  
>  static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
>  {
> +    bool svade = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade);
> +    bool svadu = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu);
> +
>      e->pte &= ~PTE_ACCESS_MASK;
>  
>      e->pte |= PTE_USER;
>  
>      /*
> -     * Two schemes to manage the A and D bits are defined:
> -     *   • The Svade extension: when a virtual page is accessed and the A bit
> -     *     is clear, or is written and the D bit is clear, a page-fault
> -     *     exception is raised.
> -     *   • When the Svade extension is not implemented, the following scheme
> -     *     applies.
> -     *     When a virtual page is accessed and the A bit is clear, the PTE is
> -     *     updated to set the A bit. When the virtual page is written and the
> -     *     D bit is clear, the PTE is updated to set the D bit. When G-stage
> -     *     address translation is in use and is not Bare, the G-stage virtual
> -     *     pages may be accessed or written by implicit accesses to VS-level
> -     *     memory management data structures, such as page tables.
> -     * Thereby to avoid a page-fault in case of Svade is available, it is
> -     * necessary to set A and D bits.
> -     *
> -     * TODO: For now, it’s fine to simply set the A/D bits, since OpenSBI
> -     *       delegates page faults to a lower privilege mode and so OpenSBI
> -     *       isn't expect to handle page-faults occured in lower modes.
> -     *       By setting the A/D bits here, page faults that would otherwise
> -     *       be generated due to unset A/D bits will not occur in Xen.
> -     *
> -     *       Currently, Xen on RISC-V does not make use of the information
> -     *       that could be obtained from handling such page faults, which
> -     *       could otherwise be useful for several use cases such as demand
> -     *       paging, cache-flushing optimizations, memory access tracking,etc.
> +     * A RISC-V implementation can choose to either:
> +     * 1) Update 'A' and 'D' PTE bits in hardware.
> +     * 2) Generate page fault when 'A' and/or 'D' PTE bits are not set so that
> +     *    software can update these bits.
>       *
> -     *       To support the more general case and the optimizations mentioned
> -     *       above, it would be better to stop setting the A/D bits here and
> -     *       instead handle page faults that occur due to unset A/D bits.
> +     * Xen supports both options mentioned above: unless the platform guarantees
> +     * (1), i.e. only Svadu is present, set 'A' and 'D' so that (2) never
> +     * faults.
>       */
> -    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) )
> +    if ( !svadu || svade )
>          e->pte |= PTE_ACCESSED | PTE_DIRTY;

I may have asked this already when the original conditional was introduced:
What use is it to leave A and D clear, when we don't otherwise consume the
bits? This way hardware has to issue more (atomic) writes, i.e. performance
suffers for no gain.

Jan

Re: [PATCH v3 4/6] xen/riscv: fix A/D bits in Xen's page-table mappings

[Jan Beulich]

On 29.09.2026 18:32, Baptiste Le Duc wrote:
> Xen does not handle page faults caused by clear A/D bits, so it presets
> them when creating PTEs. pt_update_entry() does so, but
> setup_initial_mapping() (the boot page tables) and arch_pmap_map() (the
> fixmap) build leaf PTEs directly without going through it. Without Svadu,
> both would fault on first access.
> 
> Add PTE_ACCESSED and PTE_DIRTY to PAGE_HYPERVISOR_RO, and build
> PAGE_HYPERVISOR_RW and PAGE_HYPERVISOR_RX on top of it. PTE_DIRTY is set in
> all cases for consistency with pt_update_entry() which sets it at runtime.
> 
> This fixes arch_pmap_map() for free, since it already builds its PTE from
> PAGE_HYPERVISOR_RW. Switch setup_initial_mapping() to use these macros for
> its default, text and rodata permissions, and for the temporary root entry
> built by check_pgtbl_mode_support(), instead of the equivalent raw bit
> lists. The latter drops PTE_WRITABLE, going from RWX to RX, but this is
> harmless, as that entry only has to make the current instruction stream
> fetchable between the two CSR_SATP writes used to probe SATP mode support,
> and nothing writes through it.
> 
> Drop the now-redundant PTE_LEAF_DEFAULT, since converting the last
> open-coded site above leaves it with no user outside page.h itself.
> 
> pte_is_table() and pte_is_mapping() both assert that a PTE doesn't use one
> of the two reserved encodings, (V=1, W=1, R=0) and (V=1, X=1, W=1, R=0), by
> masking it with PAGE_HYPERVISOR_RW. Now that PAGE_HYPERVISOR_RW also
> carries A and D, a reserved PTE with A or D set would no longer be caught.
> Mask with the V, R and W bits explicitly instead, and factor the check out
> into pte_is_reserved().
> 
> Fixes: e66003e7be19 ("xen/riscv: introduce setup_initial_pages")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Baptiste Le Duc]

On 2026-09-30 14:07:34+02:00, Jan Beulich wrote:
> On 29.09.2026 18:32, Baptiste Le Duc wrote:
> 
> > paddr_to_pte()'s "permissions" parameter, the matching local in
> > 
> > setup_initial_mapping() and p2m_set_permission() don't only deal with
> > 
> > permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
> > 
> > PTE_DIRTY.
> > 
> > 
> > 
> > Rename them to "pte_flags" and p2m_set_pte_flags() respectively.
> > 
> > 
> > 
> > No functional change.
> > 
> > 
> > 
> > Requested-by: Jan Beulich <jbeulich@suse.com>
> > 
> > Assisted-by: Claude:claude-opus-5
> > 
> > Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> 
> In principle
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> I won't, however, be able to apply this patch as-is. As you can see both above
> and below, extra newlines were inserted everywhere on the way here. The list
> archive doesn't show this issue, but instead shows undue wrapped lines. It is
> also only this one patch in the series which has this issue. You will need to
> resend for me to properly consume.
> 
It's weird, it appears my original message didn't contain such `\r`. We
took a look with Anthony Perard and it seems to be an issue from Xen's server
side. I will try to resend it, just to see.
Thanks

-- 
Baptiste Le Duc <baptiste.le-duc@vates.tech>

[PATCH v3.1 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Baptiste Le Duc]

paddr_to_pte()'s "permissions" parameter, the matching local in
setup_initial_mapping() and p2m_set_permission() don't only deal with
permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
PTE_DIRTY.

Rename them to "pte_flags" and p2m_set_pte_flags() respectively.

No functional change.

Requested-by: Jan Beulich <jbeulich@suse.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
Changes since v2:
- new patch
---
 xen/arch/riscv/include/asm/mm.h | 5 +++--
 xen/arch/riscv/mm.c             | 8 ++++----
 xen/arch/riscv/p2m.c            | 4 ++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 9e28c24954..1ac66283ec 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -22,9 +22,10 @@ extern vaddr_t directmap_virt_start;
 #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
 
 static inline pte_t paddr_to_pte(paddr_t paddr,
-                                 unsigned int permissions)
+                                 unsigned int pte_flags)
 {
-    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) | permissions };
+    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) |
+                            pte_flags };
 }
 
 static inline paddr_t pte_to_paddr(pte_t pte)
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 610d111945..0e26c5751f 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -140,7 +140,7 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
         case 1: /* Level 0 */
             {
                 unsigned long paddr = (page_addr - map_start) + pa_start;
-                unsigned int permissions = PTE_LEAF_DEFAULT;
+                unsigned int pte_flags = PTE_LEAF_DEFAULT;
                 unsigned long addr = is_identity_mapping
                                      ? page_addr : virt_to_maddr(page_addr);
                 pte_t pte_to_be_written;
@@ -149,13 +149,13 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
 
                 if ( is_kernel_text(addr) ||
                      is_kernel_inittext(addr) )
-                        permissions =
+                        pte_flags =
                             PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;
 
                 if ( is_kernel_rodata(addr) )
-                    permissions = PTE_READABLE | PTE_VALID;
+                    pte_flags = PTE_READABLE | PTE_VALID;
 
-                pte_to_be_written = paddr_to_pte(paddr, permissions);
+                pte_to_be_written = paddr_to_pte(paddr, pte_flags);
 
                 if ( !pte_is_valid(pgtbl[index]) )
                     pgtbl[index] = pte_to_be_written;
diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
index 1cea86512c..f7b380b90a 100644
--- a/xen/arch/riscv/p2m.c
+++ b/xen/arch/riscv/p2m.c
@@ -584,7 +584,7 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
     p2m_write_pte(p, pte, clean_cache);
 }
 
-static void p2m_set_permission(pte_t *e, p2m_type_t t)
+static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
 {
     e->pte &= ~PTE_ACCESS_MASK;
 
@@ -676,7 +676,7 @@ static pte_t p2m_pte_from_mfn(mfn_t mfn, p2m_type_t t,
             break;
         }
 
-        p2m_set_permission(&e, t);
+        p2m_set_pte_flags(&e, t);
         p2m_set_type(&e, t, ctx);
     }
     else

-- 
2.55.0

Re: [PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Jan Beulich]

On 30.09.2026 14:42, Baptiste Le Duc wrote:
> On 2026-09-30 14:07:34+02:00, Jan Beulich wrote:
>> On 29.09.2026 18:32, Baptiste Le Duc wrote:
>>
>>> paddr_to_pte()'s "permissions" parameter, the matching local in
>>>
>>> setup_initial_mapping() and p2m_set_permission() don't only deal with
>>>
>>> permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
>>>
>>> PTE_DIRTY.
>>>
>>>
>>>
>>> Rename them to "pte_flags" and p2m_set_pte_flags() respectively.
>>>
>>>
>>>
>>> No functional change.
>>>
>>>
>>>
>>> Requested-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> Assisted-by: Claude:claude-opus-5
>>>
>>> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>
>> In principle
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>
>> I won't, however, be able to apply this patch as-is. As you can see both above
>> and below, extra newlines were inserted everywhere on the way here. The list
>> archive doesn't show this issue, but instead shows undue wrapped lines. It is
>> also only this one patch in the series which has this issue. You will need to
>> resend for me to properly consume.
>>
> It's weird, it appears my original message didn't contain such `\r`. We
> took a look with Anthony Perard and it seems to be an issue from Xen's server
> side. I will try to resend it, just to see.

Sadly same issue there.

Jan

Re: [PATCH v3.1 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Baptiste Le Duc]

On 2026-09-30 14:46 +0200, Baptiste Le Duc wrote:

Ok it didn't fix the problem. I said Xen's server, but it's in fact, our
company's server, sorry. We are going to change SMTP provider soon, I
hope it will fix the problem...

-- 
Baptiste Le Duc <baptiste.le-duc@vates.tech>

[PATCH RESEND v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Anthony PERARD]

From: Baptiste Le Duc <baptiste.le-duc@vates.tech>

paddr_to_pte()'s "permissions" parameter, the matching local in
setup_initial_mapping() and p2m_set_permission() don't only deal with
permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
PTE_DIRTY.

Rename them to "pte_flags" and p2m_set_pte_flags() respectively.

No functional change.

Requested-by: Jan Beulich <jbeulich@suse.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
 xen/arch/riscv/include/asm/mm.h | 5 +++--
 xen/arch/riscv/mm.c             | 8 ++++----
 xen/arch/riscv/p2m.c            | 4 ++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 9e28c2495462..1ac66283ec9b 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -22,9 +22,10 @@ extern vaddr_t directmap_virt_start;
 #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
 
 static inline pte_t paddr_to_pte(paddr_t paddr,
-                                 unsigned int permissions)
+                                 unsigned int pte_flags)
 {
-    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) | permissions };
+    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) |
+                            pte_flags };
 }
 
 static inline paddr_t pte_to_paddr(pte_t pte)
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 610d11194588..0e26c5751f17 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -140,7 +140,7 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
         case 1: /* Level 0 */
             {
                 unsigned long paddr = (page_addr - map_start) + pa_start;
-                unsigned int permissions = PTE_LEAF_DEFAULT;
+                unsigned int pte_flags = PTE_LEAF_DEFAULT;
                 unsigned long addr = is_identity_mapping
                                      ? page_addr : virt_to_maddr(page_addr);
                 pte_t pte_to_be_written;
@@ -149,13 +149,13 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
 
                 if ( is_kernel_text(addr) ||
                      is_kernel_inittext(addr) )
-                        permissions =
+                        pte_flags =
                             PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;
 
                 if ( is_kernel_rodata(addr) )
-                    permissions = PTE_READABLE | PTE_VALID;
+                    pte_flags = PTE_READABLE | PTE_VALID;
 
-                pte_to_be_written = paddr_to_pte(paddr, permissions);
+                pte_to_be_written = paddr_to_pte(paddr, pte_flags);
 
                 if ( !pte_is_valid(pgtbl[index]) )
                     pgtbl[index] = pte_to_be_written;
diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
index 1cea86512c8c..f7b380b90a4e 100644
--- a/xen/arch/riscv/p2m.c
+++ b/xen/arch/riscv/p2m.c
@@ -584,7 +584,7 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
     p2m_write_pte(p, pte, clean_cache);
 }
 
-static void p2m_set_permission(pte_t *e, p2m_type_t t)
+static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
 {
     e->pte &= ~PTE_ACCESS_MASK;
 
@@ -676,7 +676,7 @@ static pte_t p2m_pte_from_mfn(mfn_t mfn, p2m_type_t t,
             break;
         }
 
-        p2m_set_permission(&e, t);
+        p2m_set_pte_flags(&e, t);
         p2m_set_type(&e, t, ctx);
     }
     else
-- 
Anthony PERARD

Re: [PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Oleksii Kurochko]

On 9/30/26 2:07 PM, Jan Beulich wrote:
> On 29.09.2026 18:32, Baptiste Le Duc wrote:
>> paddr_to_pte()'s "permissions" parameter, the matching local in
>>
>> setup_initial_mapping() and p2m_set_permission() don't only deal with
>>
>> permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
>>
>> PTE_DIRTY.
>>
>>
>>
>> Rename them to "pte_flags" and p2m_set_pte_flags() respectively.
>>
>>
>>
>> No functional change.
>>
>>
>>
>> Requested-by: Jan Beulich <jbeulich@suse.com>
>>
>> Assisted-by: Claude:claude-opus-5
>>
>> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> 
> In principle
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> I won't, however, be able to apply this patch as-is. As you can see both above
> and below, extra newlines were inserted everywhere on the way here. The list
> archive doesn't show this issue, but instead shows undue wrapped lines. It is
> also only this one patch in the series which has this issue. You will need to
> resend for me to properly consume.

I had the same issue but I resolved it with a small bash script.

~ Oleksii

Re: [PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Oleksii Kurochko]

On 9/29/26 6:32 PM, Baptiste Le Duc wrote:
> paddr_to_pte()'s "permissions" parameter, the matching local in

Nit: s/the matching local/the matching local variable?

> setup_initial_mapping() and p2m_set_permission() don't only deal with
> permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
> PTE_DIRTY.
> 
> Rename them to "pte_flags" and p2m_set_pte_flags() respectively.
> 
> No functional change.
> 
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> ---
> Changes since v2:
> - new patch
> ---
>   xen/arch/riscv/include/asm/mm.h | 5 +++--
>   xen/arch/riscv/mm.c             | 8 ++++----
>   xen/arch/riscv/p2m.c            | 4 ++--
>   3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
> index 9e28c24954..1ac66283ec 100644
> --- a/xen/arch/riscv/include/asm/mm.h
> +++ b/xen/arch/riscv/include/asm/mm.h
> @@ -22,9 +22,10 @@ extern vaddr_t directmap_virt_start;
>   #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
>   
>   static inline pte_t paddr_to_pte(paddr_t paddr,
> -                                 unsigned int permissions)
> +                                 unsigned int pte_flags)
>   {
> -    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) | permissions };
> +    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) |
> +                            pte_flags };

Nit: it could be one line.

>   }
>   
>   static inline paddr_t pte_to_paddr(pte_t pte)
> diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
> index 610d111945..0e26c5751f 100644
> --- a/xen/arch/riscv/mm.c
> +++ b/xen/arch/riscv/mm.c
> @@ -140,7 +140,7 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
>           case 1: /* Level 0 */
>               {
>                   unsigned long paddr = (page_addr - map_start) + pa_start;
> -                unsigned int permissions = PTE_LEAF_DEFAULT;
> +                unsigned int pte_flags = PTE_LEAF_DEFAULT;
>                   unsigned long addr = is_identity_mapping
>                                        ? page_addr : virt_to_maddr(page_addr);
>                   pte_t pte_to_be_written;
> @@ -149,13 +149,13 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
>   
>                   if ( is_kernel_text(addr) ||
>                        is_kernel_inittext(addr) )
> -                        permissions =
> +                        pte_flags =
>                               PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;

Nit: it could be one line now.

>   
>                   if ( is_kernel_rodata(addr) )
> -                    permissions = PTE_READABLE | PTE_VALID;
> +                    pte_flags = PTE_READABLE | PTE_VALID;
>   
> -                pte_to_be_written = paddr_to_pte(paddr, permissions);
> +                pte_to_be_written = paddr_to_pte(paddr, pte_flags);
>   
>                   if ( !pte_is_valid(pgtbl[index]) )
>                       pgtbl[index] = pte_to_be_written;
> diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
> index 1cea86512c..f7b380b90a 100644
> --- a/xen/arch/riscv/p2m.c
> +++ b/xen/arch/riscv/p2m.c
> @@ -584,7 +584,7 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
>       p2m_write_pte(p, pte, clean_cache);
>   }
>   
> -static void p2m_set_permission(pte_t *e, p2m_type_t t)
> +static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
>   {
>       e->pte &= ~PTE_ACCESS_MASK;
>   
> @@ -676,7 +676,7 @@ static pte_t p2m_pte_from_mfn(mfn_t mfn, p2m_type_t t,
>               break;
>           }
>   
> -        p2m_set_permission(&e, t);
> +        p2m_set_pte_flags(&e, t);
>           p2m_set_type(&e, t, ctx);
>       }
>       else
> 

I am okay to go without last two Nit(s):

Reveiwed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

Re: [PATCH v3 3/6] xen/riscv: fix A/D bit handling in G-stage mappings

[Oleksii Kurochko]

On 9/30/26 2:20 PM, Jan Beulich wrote:
> On 29.09.2026 18:32, Baptiste Le Duc wrote:
>> @@ -500,6 +502,45 @@ static void __init riscv_fill_hwcap_from_isa_string(void)
>>       }
>>   }
>>   
>> +/*
>> + * Svade and Svadu extensions represent two schemes for managing the PTE A/D
>> + * bits. When the PTE A/D bits need to be set, the Svade extension indicates
>> + * that a page fault will be raised. In contrast, the Svadu extension supports
>> + * hardware updating of the PTE A/D bits.
>> + *
>> + * There are 4 possible combinations of these extensions in the device tree.
>> + * The default hardware behavior for each is:
>> + *
>> + * 1) Neither Svade nor Svadu present in DT => It is technically unknown
>> + *    whether the platform uses Svade or Svadu. Xen should be prepared to
>> + *    handle either hardware updating of the PTE A/D bits or page faults when
>> + *    they need updating.
>> + *
>> + * 2) Only Svade present in DT => Xen must assume Svade to be always enabled.
>> + *
>> + * 3) Only Svadu present in DT => Xen must assume Svadu to be always enabled.
>> + *
>> + * 4) Both Svade and Svadu present in DT => Xen must assume Svadu is turned off
>> + *    at boot time, so it presets the A/D bits. To use Svadu, the supervisor
>> + *    must explicitly enable it using the SBI FWFT extension.
>> + *
>> + * The Svade extension is mandatory and the Svadu extension is optional in the
>> + * RVA23 profile. Platforms wanting to take advantage of Svadu can choose
>> + * option 3. Platforms aware of the profile can choose option 4, and Xen won't
>> + * get the benefit of Svadu until the SBI FWFT extension is available.
>> + */
>> +static void __init riscv_resolve_ad_scheme(void)
>> +{
>> +    bool svade = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade);
>> +    bool svadu = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu);
>> +
>> +    if ( svadu && svade )
>> +        if ( sbi_probe_extension(SBI_EXT_FWFT) <= 0 )
> 
> Such successive if()s want folding.
> 
>> +            printk(XENLOG_WARNING "RISC-V: Both Svade and Svadu detected, but SBI FWFT is missing.\n"
>> +                XENLOG_WARNING "RISC-V: Defaulting to software A/D updates (Svade).\n"
>> +                XENLOG_WARNING "RISC-V: To force hardware A/D updates (Svadu), remove 'svade' from DT.\n");
> 
> Nit: Indentation, and no full stop at the end of log messages please.
> 
> Overall - what use is this? Nothing will be logged if FWFT is present, yet
> there's no real use of the extension. I.e. even in that case you default
> to svade. Further, removing svadu from DT doesn't alter hardware behavior.
> How can that be a useful suggestion?

Oh, right, that what I missed when suggested that to just remove from 
DT... It won't really affect the hardware behavior.

Then it will be easy just to drop this function and set A/D uncondtionally.

> 
>> --- a/xen/arch/riscv/p2m.c
>> +++ b/xen/arch/riscv/p2m.c
>> @@ -586,42 +586,24 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
>>   
>>   static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
>>   {
>> +    bool svade = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade);
>> +    bool svadu = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu);
>> +
>>       e->pte &= ~PTE_ACCESS_MASK;
>>   
>>       e->pte |= PTE_USER;
>>   
>>       /*
>> -     * Two schemes to manage the A and D bits are defined:
>> -     *   • The Svade extension: when a virtual page is accessed and the A bit
>> -     *     is clear, or is written and the D bit is clear, a page-fault
>> -     *     exception is raised.
>> -     *   • When the Svade extension is not implemented, the following scheme
>> -     *     applies.
>> -     *     When a virtual page is accessed and the A bit is clear, the PTE is
>> -     *     updated to set the A bit. When the virtual page is written and the
>> -     *     D bit is clear, the PTE is updated to set the D bit. When G-stage
>> -     *     address translation is in use and is not Bare, the G-stage virtual
>> -     *     pages may be accessed or written by implicit accesses to VS-level
>> -     *     memory management data structures, such as page tables.
>> -     * Thereby to avoid a page-fault in case of Svade is available, it is
>> -     * necessary to set A and D bits.
>> -     *
>> -     * TODO: For now, it’s fine to simply set the A/D bits, since OpenSBI
>> -     *       delegates page faults to a lower privilege mode and so OpenSBI
>> -     *       isn't expect to handle page-faults occured in lower modes.
>> -     *       By setting the A/D bits here, page faults that would otherwise
>> -     *       be generated due to unset A/D bits will not occur in Xen.
>> -     *
>> -     *       Currently, Xen on RISC-V does not make use of the information
>> -     *       that could be obtained from handling such page faults, which
>> -     *       could otherwise be useful for several use cases such as demand
>> -     *       paging, cache-flushing optimizations, memory access tracking,etc.
>> +     * A RISC-V implementation can choose to either:
>> +     * 1) Update 'A' and 'D' PTE bits in hardware.
>> +     * 2) Generate page fault when 'A' and/or 'D' PTE bits are not set so that
>> +     *    software can update these bits.
>>        *
>> -     *       To support the more general case and the optimizations mentioned
>> -     *       above, it would be better to stop setting the A/D bits here and
>> -     *       instead handle page faults that occur due to unset A/D bits.
>> +     * Xen supports both options mentioned above: unless the platform guarantees
>> +     * (1), i.e. only Svadu is present, set 'A' and 'D' so that (2) never
>> +     * faults.
>>        */
>> -    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) )
>> +    if ( !svadu || svade )
>>           e->pte |= PTE_ACCESSED | PTE_DIRTY;
> 
> I may have asked this already when the original conditional was introduced:
> What use is it to leave A and D clear, when we don't otherwise consume the
> bits? This way hardware has to issue more (atomic) writes, i.e. performance
> suffers for no gain.

It will be done only once, won't it? After that, if the A/D bits aren't 
cleared, there shouldn't be any performance impact, so it will basically 
behave the same as setting the A/D bits in software.

The idea was that, if Svadu is available, it should be the hardware's 
job to set the A/D bits. That way, when the A/D bits eventually start 
being used for some purpose in Xen, we won't miss an unconditional write 
of the A/D bits when Svadu is available.

If you think it's enough to just have the A/D bits set unconditionally, 
I'm okay with that, and we can go that way.

~ Oleksii

Re: [PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Jan Beulich]

On 30.09.2026 15:42, Oleksii Kurochko wrote:
> On 9/29/26 6:32 PM, Baptiste Le Duc wrote:
>> paddr_to_pte()'s "permissions" parameter, the matching local in
> 
> Nit: s/the matching local/the matching local variable?

What else could "local" on its own mean here? I think it's common
shorthand for "local variable".

>> setup_initial_mapping() and p2m_set_permission() don't only deal with
>> permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
>> PTE_DIRTY.
>>
>> Rename them to "pte_flags" and p2m_set_pte_flags() respectively.
>>
>> No functional change.
>>
>> Requested-by: Jan Beulich <jbeulich@suse.com>
>> Assisted-by: Claude:claude-opus-5
>> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>> ---
>> Changes since v2:
>> - new patch
>> ---
>>   xen/arch/riscv/include/asm/mm.h | 5 +++--
>>   xen/arch/riscv/mm.c             | 8 ++++----
>>   xen/arch/riscv/p2m.c            | 4 ++--
>>   3 files changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
>> index 9e28c24954..1ac66283ec 100644
>> --- a/xen/arch/riscv/include/asm/mm.h
>> +++ b/xen/arch/riscv/include/asm/mm.h
>> @@ -22,9 +22,10 @@ extern vaddr_t directmap_virt_start;
>>   #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
>>   
>>   static inline pte_t paddr_to_pte(paddr_t paddr,
>> -                                 unsigned int permissions)
>> +                                 unsigned int pte_flags)
>>   {
>> -    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) | permissions };
>> +    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) |
>> +                            pte_flags };
> 
> Nit: it could be one line.

Not if all the blanks are to be kept.

> I am okay to go without last two Nit(s):
> 
> Reveiwed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Please clarify whether you insist on the description change for the tag
to be applied.

Jan

Re: [PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Oleksii Kurochko]

On 9/30/26 3:54 PM, Jan Beulich wrote:
> On 30.09.2026 15:42, Oleksii Kurochko wrote:
>> On 9/29/26 6:32 PM, Baptiste Le Duc wrote:
>>> paddr_to_pte()'s "permissions" parameter, the matching local in
>>
>> Nit: s/the matching local/the matching local variable?
> 
> What else could "local" on its own mean here? I think it's common
> shorthand for "local variable".

I didn't think in that way. Now I agree that it is fine.

> 
>>> setup_initial_mapping() and p2m_set_permission() don't only deal with
>>> permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
>>> PTE_DIRTY.
>>>
>>> Rename them to "pte_flags" and p2m_set_pte_flags() respectively.
>>>
>>> No functional change.
>>>
>>> Requested-by: Jan Beulich <jbeulich@suse.com>
>>> Assisted-by: Claude:claude-opus-5
>>> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>> ---
>>> Changes since v2:
>>> - new patch
>>> ---
>>>    xen/arch/riscv/include/asm/mm.h | 5 +++--
>>>    xen/arch/riscv/mm.c             | 8 ++++----
>>>    xen/arch/riscv/p2m.c            | 4 ++--
>>>    3 files changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
>>> index 9e28c24954..1ac66283ec 100644
>>> --- a/xen/arch/riscv/include/asm/mm.h
>>> +++ b/xen/arch/riscv/include/asm/mm.h
>>> @@ -22,9 +22,10 @@ extern vaddr_t directmap_virt_start;
>>>    #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
>>>    
>>>    static inline pte_t paddr_to_pte(paddr_t paddr,
>>> -                                 unsigned int permissions)
>>> +                                 unsigned int pte_flags)
>>>    {
>>> -    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) | permissions };
>>> +    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) |
>>> +                            pte_flags };
>>
>> Nit: it could be one line.
> 
> Not if all the blanks are to be kept.
> 
>> I am okay to go without last two Nit(s):
>>
>> Reveiwed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> Please clarify whether you insist on the description change for the tag
> to be applied.
> 
Considering the your comment above, I agree local is just shorthand for 
'local variable' so I am not insisting on the description change.

~ Oleksii

Re: [PATCH v3 3/6] xen/riscv: fix A/D bit handling in G-stage mappings

[Jan Beulich]

On 30.09.2026 15:53, Oleksii Kurochko wrote:
> On 9/30/26 2:20 PM, Jan Beulich wrote:
>> On 29.09.2026 18:32, Baptiste Le Duc wrote:
>>> --- a/xen/arch/riscv/p2m.c
>>> +++ b/xen/arch/riscv/p2m.c
>>> @@ -586,42 +586,24 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
>>>   
>>>   static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
>>>   {
>>> +    bool svade = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade);
>>> +    bool svadu = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu);
>>> +
>>>       e->pte &= ~PTE_ACCESS_MASK;
>>>   
>>>       e->pte |= PTE_USER;
>>>   
>>>       /*
>>> -     * Two schemes to manage the A and D bits are defined:
>>> -     *   • The Svade extension: when a virtual page is accessed and the A bit
>>> -     *     is clear, or is written and the D bit is clear, a page-fault
>>> -     *     exception is raised.
>>> -     *   • When the Svade extension is not implemented, the following scheme
>>> -     *     applies.
>>> -     *     When a virtual page is accessed and the A bit is clear, the PTE is
>>> -     *     updated to set the A bit. When the virtual page is written and the
>>> -     *     D bit is clear, the PTE is updated to set the D bit. When G-stage
>>> -     *     address translation is in use and is not Bare, the G-stage virtual
>>> -     *     pages may be accessed or written by implicit accesses to VS-level
>>> -     *     memory management data structures, such as page tables.
>>> -     * Thereby to avoid a page-fault in case of Svade is available, it is
>>> -     * necessary to set A and D bits.
>>> -     *
>>> -     * TODO: For now, it’s fine to simply set the A/D bits, since OpenSBI
>>> -     *       delegates page faults to a lower privilege mode and so OpenSBI
>>> -     *       isn't expect to handle page-faults occured in lower modes.
>>> -     *       By setting the A/D bits here, page faults that would otherwise
>>> -     *       be generated due to unset A/D bits will not occur in Xen.
>>> -     *
>>> -     *       Currently, Xen on RISC-V does not make use of the information
>>> -     *       that could be obtained from handling such page faults, which
>>> -     *       could otherwise be useful for several use cases such as demand
>>> -     *       paging, cache-flushing optimizations, memory access tracking,etc.
>>> +     * A RISC-V implementation can choose to either:
>>> +     * 1) Update 'A' and 'D' PTE bits in hardware.
>>> +     * 2) Generate page fault when 'A' and/or 'D' PTE bits are not set so that
>>> +     *    software can update these bits.
>>>        *
>>> -     *       To support the more general case and the optimizations mentioned
>>> -     *       above, it would be better to stop setting the A/D bits here and
>>> -     *       instead handle page faults that occur due to unset A/D bits.
>>> +     * Xen supports both options mentioned above: unless the platform guarantees
>>> +     * (1), i.e. only Svadu is present, set 'A' and 'D' so that (2) never
>>> +     * faults.
>>>        */
>>> -    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) )
>>> +    if ( !svadu || svade )
>>>           e->pte |= PTE_ACCESSED | PTE_DIRTY;
>>
>> I may have asked this already when the original conditional was introduced:
>> What use is it to leave A and D clear, when we don't otherwise consume the
>> bits? This way hardware has to issue more (atomic) writes, i.e. performance
>> suffers for no gain.
> 
> It will be done only once, won't it? After that, if the A/D bits aren't 
> cleared, there shouldn't be any performance impact, so it will basically 
> behave the same as setting the A/D bits in software.
> 
> The idea was that, if Svadu is available, it should be the hardware's 
> job to set the A/D bits. That way, when the A/D bits eventually start 
> being used for some purpose in Xen, we won't miss an unconditional write 
> of the A/D bits when Svadu is available.
> 
> If you think it's enough to just have the A/D bits set unconditionally, 
> I'm okay with that, and we can go that way.

I think starting simple (i.e. unconditional) here is the way to go. Making
the setting of one or both flags conditional can be left to whenever that
would become a necessity. Note that on x86 we haven't seen a need in all
the time (for Xen's own page tables that is).

Jan

Re: [PATCH v3 3/6] xen/riscv: fix A/D bit handling in G-stage mappings

[Oleksii Kurochko]

On 9/30/26 3:58 PM, Jan Beulich wrote:
> On 30.09.2026 15:53, Oleksii Kurochko wrote:
>> On 9/30/26 2:20 PM, Jan Beulich wrote:
>>> On 29.09.2026 18:32, Baptiste Le Duc wrote:
>>>> --- a/xen/arch/riscv/p2m.c
>>>> +++ b/xen/arch/riscv/p2m.c
>>>> @@ -586,42 +586,24 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
>>>>    
>>>>    static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
>>>>    {
>>>> +    bool svade = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade);
>>>> +    bool svadu = riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu);
>>>> +
>>>>        e->pte &= ~PTE_ACCESS_MASK;
>>>>    
>>>>        e->pte |= PTE_USER;
>>>>    
>>>>        /*
>>>> -     * Two schemes to manage the A and D bits are defined:
>>>> -     *   • The Svade extension: when a virtual page is accessed and the A bit
>>>> -     *     is clear, or is written and the D bit is clear, a page-fault
>>>> -     *     exception is raised.
>>>> -     *   • When the Svade extension is not implemented, the following scheme
>>>> -     *     applies.
>>>> -     *     When a virtual page is accessed and the A bit is clear, the PTE is
>>>> -     *     updated to set the A bit. When the virtual page is written and the
>>>> -     *     D bit is clear, the PTE is updated to set the D bit. When G-stage
>>>> -     *     address translation is in use and is not Bare, the G-stage virtual
>>>> -     *     pages may be accessed or written by implicit accesses to VS-level
>>>> -     *     memory management data structures, such as page tables.
>>>> -     * Thereby to avoid a page-fault in case of Svade is available, it is
>>>> -     * necessary to set A and D bits.
>>>> -     *
>>>> -     * TODO: For now, it’s fine to simply set the A/D bits, since OpenSBI
>>>> -     *       delegates page faults to a lower privilege mode and so OpenSBI
>>>> -     *       isn't expect to handle page-faults occured in lower modes.
>>>> -     *       By setting the A/D bits here, page faults that would otherwise
>>>> -     *       be generated due to unset A/D bits will not occur in Xen.
>>>> -     *
>>>> -     *       Currently, Xen on RISC-V does not make use of the information
>>>> -     *       that could be obtained from handling such page faults, which
>>>> -     *       could otherwise be useful for several use cases such as demand
>>>> -     *       paging, cache-flushing optimizations, memory access tracking,etc.
>>>> +     * A RISC-V implementation can choose to either:
>>>> +     * 1) Update 'A' and 'D' PTE bits in hardware.
>>>> +     * 2) Generate page fault when 'A' and/or 'D' PTE bits are not set so that
>>>> +     *    software can update these bits.
>>>>         *
>>>> -     *       To support the more general case and the optimizations mentioned
>>>> -     *       above, it would be better to stop setting the A/D bits here and
>>>> -     *       instead handle page faults that occur due to unset A/D bits.
>>>> +     * Xen supports both options mentioned above: unless the platform guarantees
>>>> +     * (1), i.e. only Svadu is present, set 'A' and 'D' so that (2) never
>>>> +     * faults.
>>>>         */
>>>> -    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) )
>>>> +    if ( !svadu || svade )
>>>>            e->pte |= PTE_ACCESSED | PTE_DIRTY;
>>>
>>> I may have asked this already when the original conditional was introduced:
>>> What use is it to leave A and D clear, when we don't otherwise consume the
>>> bits? This way hardware has to issue more (atomic) writes, i.e. performance
>>> suffers for no gain.
>>
>> It will be done only once, won't it? After that, if the A/D bits aren't
>> cleared, there shouldn't be any performance impact, so it will basically
>> behave the same as setting the A/D bits in software.
>>
>> The idea was that, if Svadu is available, it should be the hardware's
>> job to set the A/D bits. That way, when the A/D bits eventually start
>> being used for some purpose in Xen, we won't miss an unconditional write
>> of the A/D bits when Svadu is available.
>>
>> If you think it's enough to just have the A/D bits set unconditionally,
>> I'm okay with that, and we can go that way.
> 
> I think starting simple (i.e. unconditional) here is the way to go. Making
> the setting of one or both flags conditional can be left to whenever that
> would become a necessity. Note that on x86 we haven't seen a need in all
> the time (for Xen's own page tables that is).

Okay, then lets set them unconditionally for now.

~ Oleksii

Re: [PATCH v3 4/6] xen/riscv: fix A/D bits in Xen's page-table mappings

[Oleksii Kurochko]

On 9/29/26 6:32 PM, Baptiste Le Duc wrote:
> Xen does not handle page faults caused by clear A/D bits, so it presets
> them when creating PTEs. pt_update_entry() does so, but
> setup_initial_mapping() (the boot page tables) and arch_pmap_map() (the
> fixmap) build leaf PTEs directly without going through it. 

If I am not mistaken then not all places are mentioned here:

... does so, but three places
build leaf PTEs directly without going through it:
setup_initial_mapping() (the boot page tables), check_pgtbl_mode_support()
(the temporary root entry used to probe SATP mode support) and
arch_pmap_map() (the fixmap).

Without Svadu,
> both would fault on first access.

After this I think it makes sense to add also about 
check_pgtbl_mode_support():

For check_pgtbl_mode_support() the fault happens on the
instruction fetch right after the CSR_SATP write, before any trap
handler is set up.

> 
> Add PTE_ACCESSED and PTE_DIRTY to PAGE_HYPERVISOR_RO, and build
> PAGE_HYPERVISOR_RW and PAGE_HYPERVISOR_RX on top of it. PTE_DIRTY is set in
> all cases for consistency with pt_update_entry() which sets it at runtime.
> 
> This fixes arch_pmap_map() for free, since it already builds its PTE from
> PAGE_HYPERVISOR_RW. Switch setup_initial_mapping() to use these macros for
> its default, text and rodata permissions, and for the temporary root entry
> built by check_pgtbl_mode_support(), instead of the equivalent raw bit
> lists.

... but you added PTE_ACCESSED | PTE_DIRTY to PAGE_HYPERVISOR_* so it 
isn't really "equivalent raw bit lists".

So it seems like this part should be dropped. My suggestion is ...


  The latter drops PTE_WRITABLE, going from RWX to RX, but this is
> harmless, as that entry only has to make the current instruction stream
> fetchable between the two CSR_SATP writes used to probe SATP mode support,
> and nothing writes through it.

...

This fixes arch_pmap_map() for free, since it already builds its PTE 
from PAGE_HYPERVISOR_RW. Switch setup_initial_mapping() to use these 
macros for its default, text and rodata permissions, and 
check_pgtbl_mode_support() to use PAGE_HYPERVISOR_RX for its temporary 
root entry. The latter drops PTE_WRITABLE, going from RWX to RX, but 
this is harmless, as that entry only has to make the current instruction 
stream fetchable between the two CSR_SATP writes, and nothing writes 
through it.

> 
> Drop the now-redundant PTE_LEAF_DEFAULT, since converting the last
> open-coded site above leaves it with no user outside page.h itself.
> 
> pte_is_table() and pte_is_mapping() both assert that a PTE doesn't use one
> of the two reserved encodings, (V=1, W=1, R=0) and (V=1, X=1, W=1, R=0), by
> masking it with PAGE_HYPERVISOR_RW. Now that PAGE_HYPERVISOR_RW also
> carries A and D, a reserved PTE with A or D set would no longer be caught.
> Mask with the V, R and W bits explicitly instead, and factor the check out
> into pte_is_reserved().
> 
> Fixes: e66003e7be19 ("xen/riscv: introduce setup_initial_pages")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> ---
> Changes since v2:
> - add fixes commit ref.
> - add A/D bits to PAGE_HYPERVISOR_RO and derive PAGE_HYPERVISOR_RW/RX
>    from it for consistency with runtime.
> - introduce pte_is_reserved() to factor out the reserved-encoding assert
>    shared by pte_is_table() and pte_is_mapping().
> - reword commit title
> ---
> Changes since v1:
> - change commit title
> - mention in patch message that arch_pmap_map() is fixed too, via the
>    PAGE_HYPERVISOR_RW change, not just setup_initial_mapping().
> - convert check_pgtbl_mode_support()'s temporary root entry to
>    PAGE_HYPERVISOR_RX, as it's harmless.
> - drop PTE_LEAF_DEFAULT entirely instead of keeping it, now that no site
>    open-codes it anymore.
> - drop the pte_is_table() comment line that referenced PAGE_HYPERVISOR_RW,
>    now stale.
> ---
>   xen/arch/riscv/include/asm/page.h | 33 +++++++++++++++++----------------
>   xen/arch/riscv/mm.c               |  9 ++++-----
>   2 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/riscv/include/asm/page.h b/xen/arch/riscv/include/asm/page.h
> index b465a90325..7772e2f572 100644
> --- a/xen/arch/riscv/include/asm/page.h
> +++ b/xen/arch/riscv/include/asm/page.h
> @@ -46,12 +46,12 @@
>   #define PTE_PBMT_NOCACHE            BIT(61, UL)
>   #define PTE_PBMT_IO                 BIT(62, UL)
>   
> -#define PTE_LEAF_DEFAULT            (PTE_VALID | PTE_READABLE | PTE_WRITABLE)
>   #define PTE_TABLE                   (PTE_VALID)
>   
> -#define PAGE_HYPERVISOR_RO          (PTE_VALID | PTE_READABLE)
> -#define PAGE_HYPERVISOR_RW          (PTE_VALID | PTE_READABLE | PTE_WRITABLE)
> -#define PAGE_HYPERVISOR_RX          (PTE_VALID | PTE_READABLE | PTE_EXECUTABLE)
> +#define PAGE_HYPERVISOR_RO          (PTE_VALID | PTE_READABLE | \
> +                                     PTE_ACCESSED | PTE_DIRTY)
> +#define PAGE_HYPERVISOR_RW          (PAGE_HYPERVISOR_RO | PTE_WRITABLE)
> +#define PAGE_HYPERVISOR_RX          (PAGE_HYPERVISOR_RO | PTE_EXECUTABLE)
>   
>   #define PAGE_HYPERVISOR             PAGE_HYPERVISOR_RW
>   /*
> @@ -161,31 +161,32 @@ static inline bool pte_is_valid(pte_t p)
>    *      X W R Meaning
>    *      0 0 0 Pointer to next level of page table.
>    *      0 0 1 Read-only page.
> - *      0 1 0 Reserved for future use.
> + *      0 1 0 Reserved for future use. [1]
>    *      0 1 1 Read-write page.
>    *      1 0 0 Execute-only page.
>    *      1 0 1 Read-execute page.
> - *      1 1 0 Reserved for future use.
> + *      1 1 0 Reserved for future use. [2]
>    *      1 1 1 Read-write-execute page.
> + *
> + *   So if V=1 and W=1 then R also needs to be 1 as R = 0 is reserved for
> + *   future use ([1], [2]).
>    */
> +static inline bool pte_is_reserved(pte_t p)
> +{
> +    return (p.pte & (PTE_VALID | PTE_READABLE | PTE_WRITABLE)) ==
> +           (PTE_VALID | PTE_WRITABLE);
> +}
> +
Nit:

The function checks specifically for reserved R/W/X permission bit 
encodings where W=1 and R=0 (encodings 0b010 and 0b110 in Table 25 
"Encoding of PTE R/W/X fields" of the RISC-V Privileged ISA 
Specification). Per the specification, writable pages must also be 
marked readable (W=1 requires R=1 for valid leaf PTEs).

However, naming this function `pte_is_reserved()` can be ambiguous 
because the RISC-V PTE format contains several other types of reserved 
fields:
1. Bits 54–60 (and bit 63 without Svnapot) are "Reserved for future 
standard use".
2. Bits 9:8 (RSW) are "Reserved for supervisor software" and ignored by 
hardware.
3. PBMT = 0b11 is "Reserved for future standard use" under the Svpbmt 
extension.

To avoid confusion between reserved R/W/X permission encodings and 
reserved PTE bitfields/attributes, it would be much clearer to make the 
function name explicitly reflect that it checks for reserved R/W/X 
permission encodings.

My suggestion will be pte_has_reserved_rwx() or pte_has_reserved_perms().

I am not going to insist on the name change (but it would be nice to 
have) so but with commit message updated:
  Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

Re: [PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Jan Beulich]

On 30.09.2026 15:42, Oleksii Kurochko wrote:
> On 9/29/26 6:32 PM, Baptiste Le Duc wrote:
>> @@ -149,13 +149,13 @@ static void __init setup_initial_mapping(struct mmu_desc *mmu_desc,
>>   
>>                   if ( is_kernel_text(addr) ||
>>                        is_kernel_inittext(addr) )
>> -                        permissions =
>> +                        pte_flags =
>>                               PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;
> 
> Nit: it could be one line now.

I'll do not just that, but also correct indentation at the same time.
With correct indentation it could have been one line already before.

Jan