Re: [PATCH v6 1/4] xen/riscv: probe the PTE A/D bits update scheme at boot

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

On 2026-10-15 18:04 +0200, Jan Beulich wrote:
> That's what I suggested earlier, yet from replies I concluded it wouldn't
> be overly practical to have people edit DTs.
> 
> Jan

Hello Jan,

Based on Andrew's answer to your question on GitHub [1], I see two options
for handling a DT that doesn't advertise the scheme in effect. Please
feel free to choose one.

1) Trust the probe and fix up riscv_isa on a mismatch. This avoids having
people edit their DTs, and is safe as, per Andrew, the spec has only ever
allowed these two schemes:

/*
 * The privileged spec allows exactly two schemes for 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).
 *
 * Hence an implementation without Svade uses the hardware scheme (1), and one
 * that faults on clear A/D bits implements Svade, whether or not it says so.
 *
 * Rather than trusting the DTS alone, probe the scheme in effect with a write
 * through a PTE whose A and D bits are clear. If the DTS does not advertise
 * it, replace it by the probed scheme. As the spec has only ever allowed
 * these two schemes, this is safe even on implementations that predate Svade
 * and Svadu.
 */
static void __init check_ad_scheme(void)
{
    bool has_hw_scheme = probe_hw_ad_update();
    const char *scheme_name = has_hw_scheme ? "Svadu" : "Svade";
    enum riscv_isa_ext_id ext = has_hw_scheme ? RISCV_ISA_EXT_svadu
                                              : RISCV_ISA_EXT_svade;

    /* Fix DTS in case of mismatch */
    if ( !riscv_isa_extension_available(NULL, ext) )
    {
        printk(XENLOG_WARNING
               "CPU implements %s but the DTS does not advertise it\n",
               scheme_name);
        __clear_bit(RISCV_ISA_EXT_svade, riscv_isa);
        __clear_bit(RISCV_ISA_EXT_svadu, riscv_isa);
        __set_bit(ext, riscv_isa);
    }

    printk(XENLOG_DEBUG "CPU is using %s A/D bits scheme\n", scheme_name);
}

2) Panic on a mismatch, requiring the DT to be fixed. Same function, with
the last paragraph of the comment and the check becoming:

 * Rather than trusting the DTS alone, probe the scheme in effect with a write
 * through a PTE whose A and D bits are clear, check it against what the DTS
 * advertises, as described by the DT binding, and panic on a mismatch.
 */
[...]
    /* The probed scheme must match the DTS. */
    if ( !riscv_isa_extension_available(NULL, ext) )
        panic("CPU implements %s but the DTS does not advertise it\n",
              scheme_name);

Either way, Svade ends up set whenever the CPU faults on clear A/D bits,
so p2m_set_pte_flags() only has to check Svade:

    /*
     * A RISC-V implementation can either:
     * 1) Update the 'A' and 'D' PTE bits in hardware.
     * 2) Generate a page fault when 'A' and/or 'D' is clear, so that
     *    software can set them.
     *
     * (1) needs nothing more as it is handled by hardware. For (2), set 'A'
     * and 'D' up front: OpenSBI delegates such page faults to a lower
     * privilege mode, so they would be raised in Xen, which has no handler
     * for them.
     *
     * check_ad_scheme() ensures Svade is set whenever the CPU faults on
     * clear A/D bits. If both Svade and Svadu are advertised but the CPU
     * updates A/D in hardware, presetting them is harmless.
     *
     * TODO: Xen on RISC-V currently makes no use of the information that
     *       handling such page faults could provide, which would be useful
     *       for several use cases such as demand paging, cache-flushing
     *       optimizations, memory access tracking, etc.
     *
     *       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 the page faults that occur due to
     *       unset A/D bits.
     */
    if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) )
        e->pte |= PTE_ACCESSED | PTE_DIRTY;

Thanks,
Baptiste

[1] 
https://github.com/riscv/riscv-isa-manual/issues/3450#issuecomment-6201235555

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