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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.09.2026 00:35, George Dunlap wrote:
> 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.

Perhaps; I'd like to have Jürgen's and/or Andrew's input here, though.

Jan