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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.10.2026 16:32, Baptiste Le Duc wrote:
> On 2026-10-13 12:36 +0200, Jan Beulich wrote:
>> On 13.10.2026 12:07, Baptiste Le Duc wrote:
>>> @@ -620,6 +623,98 @@ static void __init compute_guest_isa(void)
>>>          panic("Failed to build guest \"riscv,isa\" string\n");
>>>  }
>>>  
>>> +static bool __init probe_hw_ad_update(void)
>>> +{
>>> +    bool updated = false;
>>> +    unsigned int slot = FIX_MISC;
>>> +    vaddr_t va = FIXMAP_ADDR(slot);
>>> +    struct page_info *pg;
>>> +    pte_t *entry, pte;
>>> +    mfn_t mfn;
>>> +    unsigned long tmp;
>>> +
>>> +    pg = alloc_domheap_page(NULL, 0);
>>> +    if ( !pg )
>>> +        return false;
>>
>> Why would you need a fresh page here? Can't you create a secondary mapping
>> of a page already in use (e.g. one from Xen's .data or .bss section, or the
>> stack page you're running on)?
>>
>>> +    mfn = page_to_mfn(pg);
>>> +
>>> +    arch_pmap_map(FIX_MISC, mfn);
>>> +    entry = &xen_fixmap[slot];
>>> +    pte = pte_from_mfn(mfn, PTE_VALID | PTE_READABLE);
>>> +    write_pte(entry, pte);
>>> +
>>> +    flush_tlb_one_local(va);
>>> +
>>> +    asm_inline volatile (
>>> +        "1: ld %[tmp], 0(%[va])\n"
>>> +        "   li %[updated], 1\n"
>>
>> If you mapped the stack page, you could preset a variable there to 1, load
>> from the alias mapping of that variable, and then avoid the LI altogether.
>>
>>> +        "2:\n"
>>> +        ASM_EXTABLE(1b, 2b)
>>> +        : [tmp] "=&r" (tmp), [updated] "+r" (updated)
>>> +        : [va] "r" (va)
>>> +        : "memory" );
>>> +
>>> +    arch_pmap_unmap(slot);
>>> +    free_domheap_page(pg);
>>> +
>>> +    return updated;
>>> +}
>>
>> Imo you'd better check that A also was actually set in the PTE.
> 
> If I check that A was set, can I drop the "updated" variable? The A bit
> would tell us more than whether the load succeeded.

Hmm, I'm inclined to suggest to check both.

There's one aspect to consider that I didn't think of earlier: On x86, for
example, the bit corresponding to A can also be set speculatively (the bit
corresponding to D cannot iirc). If the same is permitted on RISC-V, it
cannot be the A bit that you use for probing.

Jan