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 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
> 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?

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