Re: [PATCH v2 03/14] x86/pv: use populate_perdomain_mapping() to map the Xen GDT

[George Dunlap <dunlapg@xxxxxxxxx>]

On Thu, Sep 3, 2026 at 5:11 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 02.09.2026 11:43, George Dunlap wrote:
> > From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >
> > Currently, update_xen_slot_in_full_gdt() uses the stashed direct-map
> > pointer in d->arch.pv.gdt_ldt_l1tab to update the incoming vcpu's
> > page tables with Xen's GDT, by writing a stashed per-cpu copy of a
> > pre-baked L1 entry (either 64-bit or compat version).
> >
> > Switch this to using populate_perdomain_mapping(), which doesn't rely
> > on the stashed address of the l1 page in the direct map.  Rather than
> > also stashing a pre-baked value for the payload, compute the mfn from
> > the per-cpu GDT pointer at use: the conversion is a handful of cycles
> > on a path costing thousands, and computing at use removes the
> > parallel {,compat_}gdt_l1e bookkeeping along with its boot-ordering
> > constraint (the cached value could only be generated after Xen's
> > physical relocation, and had to be in place before the first context
> > switch; a use-time lookup is correct by construction).  The flags on
> > the final mapping are identical.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > Assisted-by: Claude Code:claude-fable-5, Claude Code:claude-opus-4-8
> > Signed-off-by: George Dunlap <gwd@xxxxxxxxxxxxxx>
> > ---
> > Changes in v2:
> > - Drop the {,compat_}gdt_mfn caching entirely (suggested by Andrew
> >    Cooper): compute virt_to_mfn() from the per-cpu GDT pointer at use.
> >    The PDX lookup behind it measures ~5-10 cycles warm against a
> >    ~1,500-cycle context switch, and this removes the double
> >    bookkeeping and the after-relocation caching constraint.  The
> >    cached-MFN assertion goes with the cache: a use-time computation
> >    from a live pointer needs no staleness check.
>
> This looks to contradict what 564d261687c0 ("x86/ctxt-switch: Document
> and improve GDT handling") used as justification to put in place the
> caching. Also Cc-ing Jürgen, who also was involved there, for possible
> further insight.
>
> Functionally the change looks okay to me, but the above will need
> sorting, at the very least by specifically discussing why effectively
> undoing that earlier change is okay.

So looking back at the thread, Jürgen measured a 14% improvement for
something that might be described as a microbenchmark before and after
the patch (a benchmark purposely trying to set up an unusual scenario
to maximize the effect of context switch overhead, not one to
represent a typical workflow).  But are the numbers really plausible?
Even at an implausible 100k switches/s across the box, saving 100
cycles per switch is about 0.04% of eight 3 GHz cores.

At any rate, we're already adding several map/unmap operations, and
about to add several more.  Keeping the PTE caching would require
adding a separate path that can write just PTEs, which then will
potentially further complication future paths where we need to make
sure we handle both domain-wide perdomain areas and per-vcpu areas.
If it were easy I would already have been keeping it.

I'd be inclined to say: Since we're going to be adding more
populate_perdomain_mapping() calls anyway, let's do it the simple
correct way first; and then explore the idea of stashing mfns of
frequently-mapped L1s (rather than having to walk L3 -> L2 -> L1); and
at that time look into stashing baked l1es to avoid conversions.

 -George