On 02.09.2026 11:43, George Dunlap wrote: > From: George Dunlap <[email protected]> > > Currently, map_domain_page() cannot be called in the context switch > path. However, Xen already needs to update the slot of an incoming PV > vcpu's GDT during context switch; and when we soon switch to per-vCPU > root pagetables, we'll have to modify two more places. > > Xen currently solves the problem by special-casing the GDT/LDT L1 > tables to be allocated from the xenheap, and stashing a pointer to its > address in the xenheap in the domain struct. Rather than add more Xen > pagetable pages to the xenheap, introduce a version of map_domain_page > which can be called from the context switch path. > > The reason map_domain_page() cannot be called from the context switch > path is x86's lazy context-switch state. Mapcache mappings are > created in the page-tables that are loaded on the pCPU. When Xen is > in a lazy context-switch state, current is the idle vCPU while the > previously-running vCPU's page-tables remain loaded. If in this > state, another pcpu wants access to the lazily-swapped-out vcpu's > state, it will send a FLUSH_VCPU_STATE IPI to the processor, which > will call sync_local_execstate(). > > sync_local_execstate() is implemented internally by calling a full > __context_switch(). In addition to copying the processor state into > the vcpu structure, this also switches the loaded pagetables to the > idle vcpu's, which would in turn cause mappings created before the IPI > to disappear mid-use. Therefore, mappings cannot be held in the > mapcache when a FLUSH_VCPU_STATE IPI may execute. To this end, > map_domain_page() calls sync_local_execstate() itself proactively when > it detects a lazy context-switch state. This guarantees that the > pagetables will remain consistent at least until the next context > switch. > > But of course, that synchronization must not be triggered from the > context switch path itself: sync_local_execstate() ends up in > __context_switch(), so a call made while a context switch is in > progress would recurse, and the assertions along that path (current > being the idle vCPU) don't hold there either. > > A full synchronization is sufficient to prevent a FLUSH_VCPU_STATE IPI > from switching the pagetables; however, it is not necessary. It > suffices to maintain interrupts disabled from before the page is > mapped until after it is unmapped. This condition is satisfied for > the mappings used on the context switch path. > > Introduce {,un}map_domain_page_irqoff() variants for callers which > guarantee that interrupts remain disabled from the map until the > matching unmap. Under that guarantee the synchronization is > unnecessary rather than merely inconvenient: no IPI can be delivered > while the mapping is in use, so the lazy state cannot change under the > caller's feet, and this_cpu(pgtable_vcpu) accurately identifies the > mapcache to use (see 622c9a5ba95d "x86/mm: accurately track which vCPU > page-tables are loaded"). The variants assert that interrupts are > disabled on entry; the rest of the contract remains the caller's > responsibility. > > This will be used by the next patch, which introduces a function which > will be used to modify the incoming vCPU's per-domain mappings from > within __context_switch(); it will also be used in future ASI > patches (tearing down and establishing per-CPU stack mappings during > context switch). > > No functional change for existing callers. > > Assisted-by: Claude Code:claude-fable-5 > Signed-off-by: George Dunlap <[email protected]>
Reviewed-by: Jan Beulich <[email protected]> with one aspect for further consideration: > @@ -59,7 +68,7 @@ static inline struct vcpu *mapcache_current_vcpu(void) > #define MAPCACHE_L1ENT(idx) \ > __linear_l1_table[l1_linear_offset(MAPCACHE_VIRT_START + > pfn_to_paddr(idx))] > > -void *map_domain_page(mfn_t mfn) > +static void *do_map_domain_page(mfn_t mfn, bool irqs_off) > { do_...() commonly (but sadly not consistently) mark top-level hypercall handlers. Personally I'd prefer if the "do" (but not the underscore) were dropped here and ... > @@ -165,7 +174,19 @@ void *map_domain_page(mfn_t mfn) > return (void *)MAPCACHE_VIRT_START + pfn_to_paddr(idx); > } > > -void unmap_domain_page(const void *ptr) > +void *map_domain_page(mfn_t mfn) > +{ > + return do_map_domain_page(mfn, false); > +} > + > +void *map_domain_page_irqoff(mfn_t mfn) > +{ > + ASSERT(!local_irq_is_enabled()); > + > + return do_map_domain_page(mfn, true); > +} > + > +static void do_unmap_domain_page(const void *ptr, bool irqs_off) ... here. Identifiers with a single leading underscore (and no following upper-case letter) are designated for use by static functions, after all. Jan
