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

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

On 10/7/26 11:13, Teddy Astie wrote:
> Le 07/10/2026 à 10:24, Baptiste Le Duc a écrit :
> > On 10/7/26 00:50, Teddy Astie wrote:
> > > Le 05/10/2026 à 18:02, Baptiste Le Duc a écrit :
> > > > 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. Platforms that use scheme (2) without advertising 
> > > > Svade,
> > > > such as the HiFive Premier P550, then hit guest-page faults that 
> > > > Xen does
> > > > not handle.
> > > I second what Jan said (this is likely a firmware bug, given that 
> > > the enumerated behavior is not consistent with the specification, as 
> > > existence of Svadu/Svade alters observable behavior).
> > Thanks, agreed on both points.
> >
> > I'll reword the commit message: it shouldn't imply that Svade in the 
> > DT means faults will occur, since with Svadu also present that 
> > depends on henvcfg.ADUE.
> > On the firmware-bug point: I don't think the P550 is really 
> > inconsistent with the spec. Svade and Svadu didn't exist when the 
> > P550 core was designed. They were only ratified later, to name the 
> > two behaviors that already existed in the wild. Before that, the 
> > privileged spec just allowed either: hardware updating A/D, or 
> > raising a page fault. The P550 implements the second, and there was 
> > no extension it could advertise for it.
> > Therefore, the DT lacks svade because the extension didn't exist when 
> > the platform description was written, not because the vendor 
> > misdescribed anything.
> At least in latest specification, absence of Svade implies that A/D 
> bit is updated on page-table access.
> The spec also insist on the no-Svade behavior.
>
> > 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.
> >
> > (... hardware updates A/D bit)
>
> The no-Svade behavior can then be re-introduced with Svadu with ADUE=1
>
> > If the Svadu extension is implemented, the ADUE bit controls whether
> > hardware updating of PTE A/D bits is enabled for VS-stage address
> > translation. When ADUE=1, hardware updating of PTE A/D bits is enabled
> > during VS-stage address translation, and the implementation behaves as
> > though the Svade extension were not implemented for VS-mode address
> > translation. When ADUE=0, the implementation behaves as though Svade
> > were implemented for VS-stage address translation. If Svadu is not
> > implemented, ADUE is read-only zero.
>
> The specification is unfortunately written a bit sideways, the base 
> behavior happens to be "opt-in" through another extension. 
I'm not sure this holds for the !Svade && !Svadu case.

In fact, in that case, my strict reading of the text from spec above is :
"If the Svadu extension is implemented" then:
        - "ADUE=0 => Svade"


Therefore, in !Svade && !Svadu case, if we followed the spec, we would 
felt into "When the Svade extension is not implemented, the following 
scheme applies [...] -> hardware updates A/D bits"
However, it is not the case of the P550 that doesn't support hw updates 
of A/D bits...

Do you read it the same way, or am I missing something?

> But that's specification, and it's possible that firmware follows a 
> earlier revision of it that has unspecified A/D semantics instead. But 
> in this case, the firmware is out of sync with current specifications 
> and probably wants to be updated.
> > I agree that, where Svadu is available, Xen should ideally configure 
> > a consistent scheme (ADUE=1) and use ADUE=0 only for specific needs 
> > such as dirty tracking. But Xen doesn't support Svadu today, so that 
> > is a separate piece of work which I'd prefer to do later.
> > In the meantime, presetting A/D in the G-stage PTEs is the only way 
> > to avoid the faults on hardware like the P550, and it is harmless on 
> > hardware that updates A/D itself.
> Looks good to me.
> >