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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.09.2026 10:50, 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

Iirc later, when discussing with me and Roger, this was somewhat adjusted.
Unfortunately the outcome of that discussion wasn't put in a reply there.

> - So, stashing the L1E might be a win, but stashing the MFN is unlikely to be.
> 
> - If we're not going to special-case this path, we have to pass an
> MFN; and if we're going to pass an MFN, it's probably better to just
> to get rid of the stashing; the extra fragility introduced doesn't pay
> for itself in terms of potential performance improvement.
> 
> Note also that by the end of the series, we add two more
> populate_perdomain_mapping() calls to the context switch path, at
> least for ASI domains, which means another two of the "expensive" MFN
> -> L1E conversions.
> 
> So v2 is doing what I understood Andy to have suggested.  I agree the
> meaning isn't 100% clear, though, so I may have misunderstood him.
> 
> As I've said, I'm not opposed to optimizing this path once we have the
> final form functional and have measured it.  Mapping the three tables
> we need to modify in vmap, and stashing both the addresses and
> pre-baked l1es, sounds like a perfectly reasonable thing to do,
> *after* we get things functional and have had a chance to measure the
> new context switch in its entirety.

And I (largely) agree. What I'm asking for (beyond feedback from those
who were involved in putting in the optimization) is that the removal
of that optimization be justified against the original commit's
reasoning.

Jan