Re: [PATCH 3/7] x86/pv: use populate_perdomain_mapping() to map the Xen GDT

On 20/08/2026 6:43 pm, George Dunlap wrote:
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 1774966305..2fcaa413f7 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -71,10 +71,10 @@ DEFINE_PER_CPU(uint64_t, efer);
>  static DEFINE_PER_CPU(unsigned long, last_extable_addr);
>  
>  DEFINE_PER_CPU_READ_MOSTLY(seg_desc_t *, gdt);
> -DEFINE_PER_CPU_READ_MOSTLY(l1_pgentry_t, gdt_l1e);
> +DEFINE_PER_CPU_READ_MOSTLY(mfn_t, gdt_mfn);
>  #ifdef CONFIG_PV32
>  DEFINE_PER_CPU_READ_MOSTLY(seg_desc_t *, compat_gdt);
> -DEFINE_PER_CPU_READ_MOSTLY(l1_pgentry_t, compat_gdt_l1e);
> +DEFINE_PER_CPU_READ_MOSTLY(mfn_t, compat_gdt_mfn);
>  #endif

I am not convinced by this change.

The reason we had the L1e stashed in the first place is because mfn <->
maddr conversions are expensive.  Stashing the L1e in this way proved to
be a win in the context switch path, despite the fragility it added by
needing to maintain a second form of the same data.

mfn <-> maddr conversions have changed expense since the optimisation
was first put in, but one form is now even more expensive than when the
optimisation was first put in.

Stashing the L1e means doing the conversion once at boot.  Anything else
means doing it on every context switch path.

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.

~Andrew