[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 3/4] xen/riscv: fix A/D bits in Xen's page-table mappings



On 2026-10-13 12:43 +0200, Jan Beulich wrote:
> On 13.10.2026 12:07, Baptiste Le Duc wrote:
> > Xen does not handle page faults caused by clear A/D bits, so it presets
> > them when creating PTEs. pt_update_entry() does so, but three places build
> > leaf PTEs directly without going through it: setup_initial_mapping() (the
> > boot page tables), check_pgtbl_mode_support() (the temporary root entry
> > used to probe SATP mode support) and arch_pmap_map() (the fixmap).
> > 
> > Without Svadu, all three fault on first access. For
> > check_pgtbl_mode_support(), the fault is raised by the instruction fetch
> > right after the SATP write, before any trap handler is set.
> > 
> > Add PTE_ACCESSED and PTE_DIRTY to PAGE_HYPERVISOR_RO, and build
> > PAGE_HYPERVISOR_RW and PAGE_HYPERVISOR_RX on top of it. PTE_DIRTY is set in
> > all cases for consistency with pt_update_entry() which sets it at runtime.
> > 
> > This fixes arch_pmap_map() for free, since it already builds its PTE from
> > PAGE_HYPERVISOR_RW. Switch setup_initial_mapping() to use these macros for
> > its default, text and rodata permissions, and check_pgtbl_mode_support() to
> > use PAGE_HYPERVISOR_RX for its temporary root entry. The latter drops
> > PTE_WRITABLE, going from RWX to RX, but this is harmless, as that entry
> > only has to make the current instruction stream fetchable between the two
> > CSR_SATP writes, and nothing writes through it.
> > 
> > Drop the now-redundant PTE_LEAF_DEFAULT, since converting the last
> > open-coded site above leaves it with no user outside page.h itself.
> > 
> > pte_is_table() and pte_is_mapping() both assert that a PTE doesn't use one
> > of the two reserved encodings, (V=1, W=1, R=0) and (V=1, X=1, W=1, R=0), by
> > masking it with PAGE_HYPERVISOR_RW. Now that PAGE_HYPERVISOR_RW also
> > carries A and D, a reserved PTE with A or D set would no longer be caught.
> > Mask with the V, R and W bits explicitly instead, and factor the check out
> > into pte_has_reserved_rwx().
> > 
> > Fixes: e66003e7be19 ("xen/riscv: introduce setup_initial_pages")
> > Assisted-by: Claude:claude-opus-5
> > Signed-off-by: Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>
> > Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > ---
> > Changes since v5:
> > - no changes, already reviewed and waiting to be merged.
> 
> I'm puzzled by this, also seen in the next patch: For one, nothing is said
> anywhere that patches 3 and 4 are independent of patches 1 and 2, and
> hence could go in ahead of those. And then, formally, neither patch is
> ready for committing - Oleksii's R-b isn't enough at this point. I would
> have ack-ed the last patch if that didn't - contextually - depend on the
> one here. I'm only going to ack the one here once either it is clarified
> that it's independent of the first two patches, or once agreement was
> reached on those earlier patches. (If this was independent, though, I
> would have expected it to have got moved to the front of the series, to
> disambiguate whether it may go in already.)
Sorry I wasn't aware of that. I will put the last patch first in the
next revision as it is independent of the other ones. Thanks for
clarifying.

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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.