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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/15/26 10:41 AM, Jan Beulich wrote:
> On 15.10.2026 10:28, Baptiste Le Duc wrote:
> > On 2026-10-15 09:14 +0200, Jan Beulich wrote:
> > > On 14.10.2026 18:45, Baptiste Le Duc wrote:
> > > > On 2026-10-14 11:09:58+02:00, Jan Beulich wrote:
> > > > > On 14.10.2026 11:00, Baptiste Le Duc wrote:
> > > > >
> > > > > In which case it would need to be
> > > > >
> > > > >      if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) ||
> > > > >           !riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svadu) )
> > > > >          e->pte |= PTE_ACCESSED | PTE_DIRTY;
> > > > >
> > > > > albeit I have to admit that permitting two contradicting extensions to
> > > > > be exposed at the same time is irritating.
> > > > >
> > > > > > I see two ways to address this:
> > > > > >
> > > > > > 1) Keep checking Svade:
> > > > > >
> > > > > >      if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) )
> > > > > >          e->pte |= PTE_ACCESSED | PTE_DIRTY;
> > > > > >
> > > > > >      After resolve_ad_scheme(), the Svade bit is set in every case where
> > > > > >      software has to preset the A/D bits. The only imprecise case is both
> > > > > >      extensions advertised with hardware updating already enabled by
> > > > > >      firmware, where presetting A/D is unnecessary but harmless.
> > > > > >
> > > > > > 2) Record the probe result in a global, e.g.
> > > > > >
> > > > > >      static bool __ro_after_init hw_ad_update;
> > > > > >
> > > > > >      and use:
> > > > > >
> > > > > >      if ( !hw_ad_update )
> > > > > >          e->pte |= PTE_ACCESSED | PTE_DIRTY;
> > > > > >
> > > > > >      This is exact in all cases, at the cost of a new global that mostly
> > > > > >      duplicates what resolve_ad_scheme() already records in the ISA bitmap.
> > > > > >
> > > > > > I discussed with Oleksii and we are fine with either. What do you prefer?
> > > > > As to 1), see above. For 2), while maybe okay, I think we're better off
> > > > Sorry, I don't fully follow the issue with 1). Compared to what you proposed,
> > > > it only differs when neither Svade nor Svadu is recorded, which cannot
> > > > happen as resolve_ad_scheme() runs before and always records one of the
> > > > two.
> > > Yet previously we said that recording extensions (as such, i.e. later also
> > > possibly exposing them to guests) when they weren't really advertised may
> > > not be the right thing to do. I.e. imo the probing may be used to drive
> > But we also said that platforms (such as the P550) may implement an
> > extension without advertising it, as a RISC-V spec maintainer
> > confirmed in [1].
> >
> > [1] 
> > https://github.com/riscv/riscv-isa-manual/issues/3450#issuecomment-6076851535
> What Andrew says there suggests to me that the behavior you observe on P550
What was mentioned in [1]:
```
Implementations that trap when A/D bits are not set do implement Svade, 
whether or not they explicitly say they do.
```

Sounds like a default behavior for our case and so Svade should be taken 
as a default.
> is against the spec. That shouldn't really be the case though, as nothing
> was said in the spec at all until Svade and Svadu were actually introduced.
Actually, earlier versions of the Privileged ISA specification (e.g., 
v1.11 and v1.12) already explicitly described both behaviors:
"Two schemes to manage the A and D bits are permitted:
1. 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.
2. When a virtual page is accessed and the A bit is clear, or is written 
and the D bit is clear, the implementation sets the corresponding bit(s) 
in the PTE. The PTE update must be atomic with respect to other accesses 
to the PTE, and must atomically check that the PTE is valid and grants 
sufficient permissions. Updates of the A bit may be performed as a 
result of speculation, but updates to the D bit must be exact..."
What is now formalized as Svade corresponds to the first scheme 
(software management via page faults), while the second scheme 
(hardware-managed updates) became Svadu.
~ Oleksii

> An update to the spec should imo never invalidate previously conforming
> implementations. I guess I'll comment there.
>
> > > Xen's internal behavior, but I'm unconvinced that it should also lead to
> > > marking extensions as available.
> > Therefore, as far as I understood, you're not convinced of this:
> >
> >      /* Neither advertised: trust the probe. */
> >      __set_bit(has_hw_update ? RISCV_ISA_EXT_svadu : RISCV_ISA_EXT_svade,
> >                riscv_isa);
> >
> > If so, then, the only solution I have in mind in what I proposed in 2)
> > with the global variable that store the result of the real probe.
> > Am I correct?
> Possibly. What I'm missing is some clarification towards the underlying
> principle which we want to follow not just here.
>
> Jan