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:06, George Dunlap wrote:
> On Fri, Sep 4, 2026 at 7:54 AM Jürgen Groß <jgross@xxxxxxxx> wrote:
>>
>> On 04.09.26 08:00, Jan Beulich wrote:
>>> 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.
>>
>> At that time I implemented core scheduling in Xen. I noticed that very
>> subtle changes in the context switch path could result in unexpected large
>> performance differences. As I had the performance test for my purpose
>> already set up, I used it for Andrew's patch (which was a result of my
>> context switch path performance findings) and really did measure the
>> impressive effect of it.
>>
>> Note that you can't only count instructions, often cache effects and
>> branch predictions are dominating the performance.
> 
> Right, but:
> 
> 1. That's going to be very much hardware- and workload- dependent.
> Even on the same hardware, if you'd made a slight change in the
> workload, you might have seen a very different result; and on
> different hardware you're going to see something different again
> 
> 2. As I said, we're now adding two extra map / unmaps, which is going
> to perturb everything again.
> 
> If anything, your argument says we should wait until we've stopped
> modifying the context switch path (which won't happen until patch 49
> at least, guessing from the patch titles), and then measure things
> again to see what's actually slow.
> 
> I'm sorry Jan,

You were replying to Jürgen, though.

> 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.

Jan

> I could put back the mfn caching that was present in v1 of the series
> (which Andy said was probably not sufficient, on balance, to make the
> duplication involved worth it).  Even that I think isn't really
> sensible, but it's not too difficult to do.  To isolate the PTE
> caching effect I'd have to write an entire duplicate codepath anyway,
> and then try to duplicate Jürgen's test.  I don't think that's really
> a reasonable ask at this point in the series.
> 
>  -George