Re: [PATCH v2 05/14] x86/pv: update guest LDT mappings using {populate,destroy}_perdomain_mapping()

[George Dunlap <dunlapg@xxxxxxxxx>]

On Mon, Sep 7, 2026 at 5:06 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 02.09.2026 11:43, George Dunlap wrote:
> > From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >
> > Until two patches ago, update_xen_slot_in_full_gdt() used the stashed
>
> With wording at the start here and ...
>
> > pointer in d->arch.pv.gdt_ldt_l1tab to update the incoming vCPU's page
> > tables with Xen's GDT; this was previously necessary because
> > map_domain_page() couldn't be called in a context switch.  Having a
> > handy pointer to an always-mapped version of the GDT/LDT L1 table,
> > other sites which modify the table started using it for convenience,
> > even if they weren't called from within a context switch.  These
> > include pv_map_ldt_shadow_page() and pv_destroy_ldt().
> >
> > Continue the process of switching users of the stashed reference to use
> > populate_perdomain_mapping() instead.
> >
> > pv_map_ldt_shadow_page() is, by definition, always modifying the
> > currently-running vCPU: it runs from the #PF handler for a descriptor
> > fetch on the guest's behalf, and running the guest implies its page
> > tables are loaded.  So it could simply write the linear recursive
> > mappings directly.  Go through populate_perdomain_mapping() anyway, to
> > keep a single writer for the per-domain area.
> >
> > For pv_destroy_ldt(), use destroy_perdomain_mapping().
> >
> > Previously, pv_destroy_ldt() used the L1 LDT entries themselves to
> > determine which MFNs to drop type and count references to.  Rather
> > than reading from the stashed L1, keep the MFNs corresponding to L1
> > slots in an array in the vCPU structure, as we do in the GDT case.
> > (Note that unlike the GDT case, these are not part of a public ABI, so
> > can be mfn_t, avoiding a recast-and-copy.)
> >
> > Note that mappings_dropped (the return value of pv_destroy_ldt()) now
> > reflects the *number of valid MFNs in this array*, not *the number of
> > non-empty L1 entries*.  This introduces an invariant we must maintain:
> > pv_map_ldt_shadow_page() writes both the array entry and the mapping,
> > and pv_destroy_ldt() clears both, so the two stay in lockstep.
> >
> > Also note that, unlike pv_destroy_gdt() from the previous patch,
>
> ... here adjusted as per the comment on the earlier patch, ...
>
> > pv_destroy_ldt() doesn't fill in the values with zero_l1e (see
> > 61031e64d3), so there's no change here.
> >
> > 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>
>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> On the basis that ...
>
> > --- a/xen/arch/x86/include/asm/domain.h
> > +++ b/xen/arch/x86/include/asm/domain.h
> > @@ -541,6 +541,8 @@ struct pv_vcpu
> >      struct trap_info *trap_ctxt;
> >
> >      unsigned long gdt_frames[FIRST_RESERVED_GDT_PAGE];
> > +    /* Max LDT entries is 8192, so 8192 * 8 = 64KiB (16 pages). */
> > +    mfn_t ldt_frames[16];
> >      unsigned long ldt_base;
> >      unsigned int gdt_ents, ldt_ents;
>
> ... this not really insignificant size increase is okay-ish as long as
> struct hvm_vcpu is about three times the size (i.e. is still more than
> double the size after this change).

FYI it looks like by the end of the whole series struct pv_vcpu has
net zero change: we add this array, but then move the mapcache
structure out into arch_vcpu.  In turn arch_vcpu by the end is an
extra 256 bytes, but with some rearrangement, we can reduce padding
and increase struct vcpu by only 192 bytes.

 -George