Re: [PATCH v4] VMX: use a single, global APIC access page

[Tim Deegan <tim@xxxxxxx>]

Hi,

Apologies for not sending comments before, and thanks for the ping.

At 12:40 +0200 on 12 Apr (1618231248), Jan Beulich wrote:
> The address of this page is used by the CPU only to recognize when to
> access the virtual APIC page instead. No accesses would ever go to this
> page. It only needs to be present in the (CPU) page tables so that
> address translation will produce its address as result for respective
> accesses.
> 
> By making this page global, we also eliminate the need to refcount it,
> or to assign it to any domain in the first place.

What is the aim here?  To save 4k per domain?  It seems to come out
about even for adding and removing code. 

> --- a/xen/arch/x86/mm/shadow/set.c
> +++ b/xen/arch/x86/mm/shadow/set.c
> @@ -94,6 +94,22 @@ shadow_get_page_from_l1e(shadow_l1e_t sl
>      ASSERT(!sh_l1e_is_magic(sl1e));
>      ASSERT(shadow_mode_refcounts(d));
>  
> +    /*
> +     * VMX'es APIC access MFN is just a surrogate page.  It doesn't actually
> +     * get accessed, and hence there's no need to refcount it (and 
> refcounting
> +     * would fail, due to the page having no owner).
> +     */
> +    if ( mfn_valid(mfn = shadow_l1e_get_mfn(sl1e)) )

Would it be better to check specifically for mfn == apic_access_mfn
(and apic_access_mfn != 0, I guess)?  If we want this behaviour for
for all un-owned PGC_extra MFNs it would be good to explain that in the
comments.

Cheers,

Tim.