[PATCH v8 4/5] xen/riscv: probe the A/D update scheme if the DT advertises neither

[Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>]

p2m_set_pte_flags() presets the A/D bits of G-stage PTEs only when
RISCV_ISA_EXT_svade is set, which is correct only if the device tree is.

The privileged spec allows exactly two schemes for updating the PTE A and D
bits: hardware sets them on access and on write, or accessing a page whose
A and/or D bit is clear raises a page fault so that software can set them
(ratified as Svade). Every implementation uses one of them, but platforms
designed before Svade and Svadu were ratified, such as the HiFive Premier
P550, may advertise neither.

Trust the device tree when it advertises Svade and/or Svadu, as for any
other extension. When it advertises neither, probe the scheme in effect
and record the matching extension in riscv_isa, so that such device trees
can be used as-is, and warn naming the extension to add to "riscv,isa".

Introduce probe_hw_ad_update(): alias a local variable through the FIX_MISC
fixmap slot with its A and D bits clear, then store to it under an
exception table fixup. Hardware updating is in effect if the store
completes and the D bit is then set. The probe fails otherwise, whether the
store faults or completes leaving the D bit clear. The D bit is checked
rather than the A bit, as the A bit may be set speculatively. The PTE is
written directly rather than through set_fixmap(), which presets the A/D
bits.

Add Svadu to the known ISA extensions so it can be told apart from Svade.
The comment above the enum asks for new extensions to go at the bottom to
avoid shuffling, a leftover from Linux where the IDs were macros with
explicit values. Nothing depends on the enum values, so have the comment
say that multi-letter extensions follow the order of riscv_isa_ext[].

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>
---
Changes since v7:
- probe the A/D update scheme when the device tree advertises neither
  Svade nor Svadu and record the probed extension, instead of panicking.
- drop Suggested-by.
- update the patch's title/body accordingly.
---
Changes since v6:
- drop the probe and trust the device tree, only panic when it advertises
  neither Svade nor Svadu.
- update the patch's title/body accordingly.
- put svadu between svade and svpbmt in riscv_isa_ext_id.
- rewrite the enum riscv_isa_ext_id comment.
---
Changes since v5:
- new patch, probe the A/D bits update scheme instead of trusting the
  device tree alone.
---
 xen/arch/riscv/cpufeature.c             | 69 +++++++++++++++++++++++++++++++++
 xen/arch/riscv/include/asm/cpufeature.h | 13 ++++---
 2 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c
index 3d5bbd2228..49cbd0a6fe 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/fixmap.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),
 };
 
@@ -630,6 +632,51 @@ const char *get_guest_isa_str(void)
     return guest_isa_str;
 }
 
+/*
+ * Probe whether hardware updates the PTE A/D bits by writing to a local
+ * variable through an alias mapping whose A and D bits are clear.
+ *
+ * Check the D bit rather than the A bit, as the A bit may be set
+ * speculatively while the D bit can't.
+ *
+ * Under Svade, the store raises a page fault instead, so use the extable to
+ * handle it.
+ */
+static bool __init probe_hw_ad_update(void)
+{
+    unsigned int probe_target;
+    vaddr_t va = FIXMAP_ADDR(FIX_MISC) + PAGE_OFFSET(&probe_target);
+    pte_t *entry = &xen_fixmap[FIX_MISC];
+    bool d_set = false;
+
+    ASSERT(!pte_is_valid(*entry));
+
+    /*
+     * Any page in use will do, so alias the stack page this code runs on,
+     * with A and D clear. Store to a variable of its own, so that nothing
+     * the compiler relies on gets overwritten.
+     */
+    write_pte(entry, pte_from_mfn(maddr_to_mfn(virt_to_maddr(&probe_target)),
+                                  PTE_VALID | PTE_READABLE | PTE_WRITABLE));
+    flush_tlb_one_local(va);
+
+    asm_inline goto (
+        "1: sw zero, 0(%[va])\n"
+        ASM_EXTABLE(1b, %l[out])
+        :
+        : [va] "r" (va)
+        : "memory"
+        : out );
+
+    d_set = read_pte(entry).pte & PTE_DIRTY;
+
+ out:
+    write_pte(entry, (pte_t){});
+    flush_tlb_one_local(va);
+
+    return d_set;
+}
+
 void __init riscv_fill_hwcap(void)
 {
     unsigned int i;
@@ -674,6 +721,28 @@ void __init riscv_fill_hwcap(void)
                "Zihintpause unavailable: cpu_relax() gives the CPU no hint; "
                "wire up this platform's pause equivalent in cpu_relax()\n");
 
+    /*
+     * The privileged spec allows exactly two schemes for updating the PTE A
+     * and D bits:
+     *  1) hardware sets them on access and on write;
+     *  2) accessing a page whose A and/or D bit is clear raises a page fault,
+     *     so that software can set them (ratified as Svade).
+     *
+     * Every implementation uses one of them, but may predate the extensions
+     * and advertise neither. In that case, trust the hardware probe.
+     */
+    if ( !riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) &&
+         !riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu) )
+    {
+        bool hw_update = probe_hw_ad_update();
+
+        __set_bit(hw_update ? RISCV_ISA_EXT_svadu : RISCV_ISA_EXT_svade,
+                  riscv_isa);
+        printk(XENLOG_WARNING
+               "Device tree advertises neither Svade nor Svadu, probed %s: add 
it to \"riscv,isa\"\n",
+               hw_update ? "svadu" : "svade");
+    }
+
     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..b0a23be18e 100644
--- a/xen/arch/riscv/include/asm/cpufeature.h
+++ b/xen/arch/riscv/include/asm/cpufeature.h
@@ -8,13 +8,13 @@
 #include <xen/types.h>
 
 /*
- * These macros represent the logical IDs of each multi-letter RISC-V ISA
- * extension and are used in the ISA bitmap. The logical IDs start from
- * RISCV_ISA_EXT_BASE, which allows the 0-25 range to be reserved for single
- * letter extensions and are used in enum riscv_isa_ext_id.
+ * Logical IDs of RISC-V ISA extensions, used as bit positions in the ISA
+ * bitmap.
  *
- * New extensions should just be added to the bottom, rather than added
- * alphabetically, in order to avoid unnecessary shuffling.
+ * Single-letter extensions use IDs below RISCV_ISA_EXT_BASE and are sorted
+ * alphabetically. Multi-letter extensions start from RISCV_ISA_EXT_BASE and
+ * follow the same order as riscv_isa_ext[] (see the ordering rules described
+ * above it).
  */
 #define RISCV_ISA_EXT_BASE  26
 
@@ -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
 };

-- 
2.56.0