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

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