[PATCH v11 1/1] 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 that it is missing from "riscv,isa" along with
the probed result.

Introduce probe_hw_ad_update(): alias the first page of the Xen image
read-only through the FIX_MISC fixmap slot with its A and D bits clear,
then load from it under an exception table fixup. Hardware updating is in
effect if the load completes and the A bit is then set. The probe fails
otherwise, whether the load faults or completes leaving the A bit clear.
The A bit being set by a speculative access still implies Svadu, as Svade
only allows A to be set by software.

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>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
Changes since v10:
- collect Reviewed-by and Acked-by.
- reword the comments around the probe load and the A bit check.
- shorten the warning printed when the extension is probed.
---
Changes since v9:
- add a fence between the load and reading the PTE back, as nothing
  else orders the A bit update before the read.
- explain why set_fixmap() can't be used and why any RAM page can be
  aliased.
---
Changes since v8:
- probe through a read-only alias of the Xen image and check the A bit,
  as A being set by a speculative access still implies Svadu.
---
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..dff590f68a 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 reading through an alias
+ * mapping whose A and D bits are clear. Observing A set, even by a speculative
+ * access, implies Svadu, as Svade only allows A to be set by software.
+ */
+static bool __init probe_hw_ad_update(void)
+{
+    vaddr_t va = FIXMAP_ADDR(FIX_MISC);
+    pte_t *entry = &xen_fixmap[FIX_MISC];
+    bool a_set = false;
+
+    ASSERT(!pte_is_valid(*entry));
+
+    /*
+     * set_fixmap() can't be used, as pt_update_entry() always sets A and D.
+     *
+     * A and D belong to the PTE, not to the page, so a new PTE with A and D
+     * clear can point to any RAM page, whatever its other mappings. Use the
+     * first page of the Xen image, and map it read-only so that Xen can't be
+     * corrupted through this alias.
+     */
+    write_pte(entry, pte_from_mfn(maddr_to_mfn(virt_to_maddr(_start)),
+                                  PTE_VALID | PTE_READABLE));
+    flush_tlb_one_local(va);
+
+    /* Read from the alias, handling page fault */
+    asm_inline goto (
+        "1: lw zero, 0(%[va])\n"
+        ASM_EXTABLE(1b, %l[out])
+        :
+        : [va] "r" (va)
+        : "memory"
+        : out );
+
+    /* Check to see if hardware set A. */
+    RISCV_FENCE(r, r);
+    a_set = read_pte(entry).pte & PTE_ACCESSED;
+
+ out:
+    write_pte(entry, (pte_t){});
+    flush_tlb_one_local(va);
+
+    return a_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
+               "Svade/Svadu missing from \"riscv,isa\" - probed as %s\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