Re: [PATCH v5 1/3] xen/riscv: always preset A/D bits in G-stage PTEs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.10.2026 12:03, Oleksii Kurochko wrote:
> On 10/6/26 5:20 PM, Jan Beulich wrote:
>> On 05.10.2026 18:00, Baptiste Le Duc wrote:
>>> A RISC-V implementation can manage the PTE A/D bits in one of two ways:
>>>      1) Update the 'A' and 'D' PTE bits in hardware (ratified as Svadu).
>>>      2) Generate a page fault when 'A' and/or 'D' is clear, so that software
>>>         can set them (ratified as Svade).
>>>
>>> p2m_set_pte_flags() presets A/D in G-stage PTEs only when the device tree
>>> advertises Svade.
> 
> According to the code it seems like it is setting A/D bits always 
> independently on what is DT file.  Baptiste, am I missing something?
> 
>   Platforms that use scheme (2) without advertising Svade,
>>> such as the HiFive Premier P550, then hit guest-page faults that Xen does
>>> not handle.
>>
>> Is that a quirk then (to avoid the word "bug")? Since it's all DT that
>> conveys available ISA information, why can't the DT for those systems
>> simply be fixed? Depending on the answer here I might question the
>> presence of the Fixes: tag below.
> 
> I think it can at some point. Let me clarify why "at some point"...
> 
> So how the things are working now:
> 
> OpenSBI detects the available ISA extensions in two steps:
> 1. It parses the CPU nodes in the DTB. If a node has the 
> riscv,isa-extensions property, OpenSBI uses it; otherwise it falls back 
> to riscv,isa. riscv,isa-extensions describes the same extensions as 
> riscv,isa, but as a list of strings, which makes it easier to parse. 
>  From this, OpenSBI builds a per-hart bitmap of available extensions 
> (basically what we do in Xen). (Platform-specific code can also add 
> extensions to the bitmap at this step.)
> 2. It then probes a small, fixed set of extensions (Sstc, Smaia, 
> Sscofpmf, Smstateen, Ssstateen, Smcntrpmf, Sdtrig) by reading a CSR that 
> the extension introduces and checking whether the access traps. This is 
> similar to what we do for Sstc in Xen.
> 
> Step 2 only adds extensions to the bitmap. It does not check or clear 
> the extensions that came from the DT; the only exception is Zicntr, 
> which is detected purely by probing. So for extensions such as 
> Svade/Svadu, which have no CSR to probe, the DT is the only source of truth.
> Now, what OpenSBI does with Svade/Svadu specifically. First, it disables 
> hardware A/D updating by default, i.e. it clears menvcfg.ADUE:
> 
> Now, what OpenSBI does with Svade/Svadu specifically. First, it disables 
> hardware A/D updating by default, i.e. it clears menvcfg.ADUE:
> 
>    /* Disable HW A/D updating by default */
>    menvcfg_val &= ~ENVCFG_ADUE;
> 
> Then it sets ADUE again, depending on which extensions are listed in the 
> DTB:
> 
>    /*
>     * Assume only Svadu is supported when it is the only extension
>     * present in the ISA string. Svade is assumed when neither are
>     * present. When both are present we must default to Svade (see
>     * the zero reset value of FWFT.PTE_AD_HW_UPDATING).
>     */
>    if (!sbi_hart_has_extension(scratch, SBI_HART_EXT_SVADE))
>       __set_menvcfg_ext(SBI_HART_EXT_SVADU, ENVCFG_ADUE);
> 
> So all OpenSBI does is set or clear menvcfg.ADUE based on what is 
> mentined in DTB.
> 
> So, to answer your original question: yes, this can be fixed just by 
> updating the DTS. OpenSBI will then configure menvcfg.ADUE according to 
> the extensions listed there. Note that the logic above is from OpenSBI 
> v1.9. Versions v1.5–v1.8 behave the same when the DT lists only svadu, 
> only svade, or both. When neither is listed, they leave ADUE unchanged 
> instead of clearing it. Versions before v1.5 don't handle Svade/Svadu at 
> all.
> 
> But IIRC such approach was suggested in one of vX and it was rejected 
> because we can't in Xen be sure that passed DTB is correct and 
> corresponds to how real h/w is configured as specifically with ADUE bit 
> in menvcfg we can't read it in HS-mode as an exception will occur as it 
> is M-mode register. So we have to trust that OpenSBI (or other firmware 
> will configure the A/D update scheme according to what is mentioned in DTS).
> 
> So then basically the question is do we want to rely on that how OpenSBI 
> configures h/w?
> 
> It seems like it could be fine. For example, without support for the 
> FWFT extension, we can't access or change the menvcfg bit anyway. The 
> FWFT extension was ratified in 2025, so we have to trust OpenSBI anyway 
> (or introduce our own extension or port the FWFT extension to an earlier 
> OpenSBI version) before FWFT is started to be available.
> 
> With this explanation would you (Jan) be okay to go with just providing 
> proper DTS?

I can't answer this, as I lack details on where the DTS is coming from.
So far I thought this was a blob handed to Xen (as kind of a module) at
boot. What you say above makes me uncertain about that. If it is a blob
as I thought, how difficult is it for people to edit a broken blob, for
it to correctly reflect their hardware's capabilities?

If that's not "very simple", or if there's concern that people may
object to doing such editing, then I think we could (should) go without
that. All I then would ask is that the description be clarified a
little further (clarifying that absence of both extensions is a valid,
albeit historical situation).

Jan