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

[Jan Beulich <jbeulich@suse.com>]

On 02.09.2026 11:43, George Dunlap wrote:
> From: Roger Pau Monné <roger.pau@citrix.com>
> 
> 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@citrix.com>
> Assisted-by: Claude Code:claude-fable-5, Claude Code:claude-opus-4-8
> Signed-off-by: George Dunlap <gwd@xenproject.org>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

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

Jan