Re: [PATCH v3 12/18] x86/mm: let Xen's page-table walker operate on a given root

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Tue, Oct 13, 2026 at 09:37:53PM +0100, George Dunlap wrote:
> On Thu, Oct 8, 2026 at 3:59 PM Roger Pau Monné <roger@xxxxxxxxxxxxxx> wrote:
> >
> > On Wed, Oct 07, 2026 at 11:40:45AM +0100, George Dunlap wrote:
> > > map_pages_to_xen() and modify_xen_mappings() maintain Xen's own mappings,
> > > walking from idle_pg_table, whose Xen slots every other root page-table
> > > copies.  A later patch introduces a second directmap hierarchy: page
> > > tables for the directmap virtual address range that some guest contexts
> > > use in place of idle_pg_table's.  It needs the same maintenance at the
> > > same virtual addresses.
> >
> > I assume that "second directmap hierarchy" refers to the sparse
> > directmap approach?
> 
> Yes.  Should I change it to "a second, sparse directmap hierarchy"?  I
> think raising that we're going to be maintaining parallel hierarchies
> is important.

I think that would be clearer.

> 
> > > Give virt_to_xen_l{3,2,1}e() the L4 to walk from, and move the bodies of
> > > map_pages_to_xen() and modify_xen_mappings() into map_pages_in() and
> > > modify_mappings_in(), which take the root as well.  The public functions
> > > pass idle_pg_table.  The EFI runtime-services page-tables mirror
> > > idle_pg_table's L4 entries only, so a newly allocated L3 is propagated
> > > to them only when the root is idle_pg_table.
> > >
> > > TLB flushing is unaffected: it is by virtual address, and the same
> > > addresses are mapped in either hierarchy.
> >
> > The virtual address space is just part of it, so far
> > map_pages_to_xen() has been doing global TLB flushes, because all CPUs
> > used the same page-tables for the Xen slots that map_pages_to_xen()
> > modifies.  With the change to pass different L4, there's also the
> > assumption that the Xen slots on those L4 will be different, and hence
> > not all CPUs might need flushing?
> >
> > Maybe I'm thinking ahead too much - but if this is going to be used to
> > manage the sparse directmap we might want to avoid IPIs to CPUs that
> > are running with the full directmap when the sparse one is changed,
> > as those are useless.
> 
> Yes; changes to both "views" (e.g., map_pages_to_xen()) still need to
> flush on all CPUs; but changes only to the sparse "view" (e.g.,
> removing pages on free_xenheap_pages()) could send an IPI only to CPUs
> with the sparse view running on it.  We could keep a cpumask with
> which CPUs currently have sparse directmaps loaded, and only IPI
> those.
> 
> That said, maintaining the cpumask isn't free; it would cause some
> cacheline bouncing as vCPUs are scheduled in and out; and since the
> idle vCPU has the full directmap, even a system with all "normal"
> domains using the sparse map would still need to update it constantly.
> Not sure how often free_xenheap_pages() happens, but if it's not very
> frequent, it might not be worth the cache consistency overhead (and
> the extra complication).  I'd leave this as a potential future
> improvement.

Worth adding a TODO comment somewhere about the fact that
map_pages_to_xen() is likely doing too many flushes when used to
manage the sparse directmap.  Without going into the more complex
approach of tracking which CPUs are running with the sparse directmap,
we could do batch flushes in add_to_sparse_view() for example, as the
flush could be pulled out of the map_pages_in() loop.

Regards, Roger.