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

[George Dunlap <dunlapg@xxxxxxxxx>]

On Fri, Sep 4, 2026 at 11:34 AM Roger Pau Monné <roger@xxxxxxxxxxxxxx> wrote:
>
> On Fri, Sep 04, 2026 at 09:50:02AM +0100, George Dunlap wrote:
> > On Fri, Sep 4, 2026 at 9:29 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > > > your position here is really inconsistent:  You wave
> > > > away a partial pagetable walk with three map/unmap operations on the
> > > > context switch path as something we'll have to do in the interim, and
> > > > can optimize later, but are now threatening to make me add in
> > > > special-case codepaths and run tests to save a few memory reads and
> > > > shifts.
> > >
> > > I think you misunderstood. There was a concern raised already on v1,
> > > and that concern wasn't covered by the patch description. In my initial
> > > reply I said "Functionally the change looks okay to me" for a reason,
> > > after all.
> >
> > To quote Andy's mail:
> >
> > <<<
> >
> > So what this patch is doing is still keeping the double copy (the
> > fragility) but reintroducing the expensive part of the operation into
> > the context switch path.  If you can't keep it being L1e, there's
> > probably no point keeping the optimisation at all.
> >
> > >>>
> >
> > Basically what I took from this is;
> >
> > - Andy thinks stashing any intermediate form (whether L1E or MFN) has
> > a technical cost (two copies that could potentially go out of sync,
> > thus "fragility")
> >
> > - Andy thinks that the expensive part of the conversion is the MFN ->
> > L1E conversion, not the vaddr -> MFN conversion
>
> We later discussed this, and the assumption was that the expensive
> part was the vaddr -> MFN translation, as that's where PDX is
> involved.  I expect crafting a PTE shouldn't be expensive at all, but
> maybe there's something I'm missing here.
>
> The original patch cached the MFN in an attempt to not remove the
> optimization, because my understanding was that the possible expensive
> part was the PDX translation.  Then again I don't have real
> measurements to back up any of the claims above, and hence it might
> all be plain wrong.

I ran some tests on my NUC.  Measuring cycles for *just*
update_xen_slot_in_full_gdt(), using the previous version (stashed
xenheap pointer + stashed l1e), and then mapcache walk with {stashed
mfn, full conversion} x {default, PDX forced on}.

xenheap+l1e: 38 cycles
mapcache walk, stashed mfn (v1), no PDX: 98 cycles
mapcache walk, full convervion, no PDX: 97 cycles
mapcache walk, stashed mfn (v1), PDX forced on: 128 cycles
mapcache walk, full conversion: 122 cycles

The variance is pretty high, so basically the mfn stashing didn't have
any statistically significant effect.

I'll leave it doing the full conversion for now.

 -George