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

[Jan Beulich <jbeulich@xxxxxxxx>]

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