Re: [Xen-devel] [PATCH v3 3/3] x86 / vmx: use a 'normal' domheap page for APIC_DEFAULT_PHYS_BASE

[George Dunlap <george.dunlap@citrix.com>]

On 1/23/20 2:03 PM, Paul Durrant wrote:
> vmx_alloc_vlapic_mapping() currently contains some very odd looking code
> that allocates a MEMF_no_owner domheap page and then shares with the guest
> as if it were a xenheap page. This then requires vmx_free_vlapic_mapping()
> to call a special function in the mm code: free_shared_domheap_page().
> 
> By using a 'normal' domheap page (i.e. by not passing MEMF_no_owner to
> alloc_domheap_page()), the odd looking code in vmx_alloc_vlapic_mapping()
> can simply use get_page_and_type() to set up a writable mapping before
> insertion in the P2M and vmx_free_vlapic_mapping() can simply release the
> page using put_page_alloc_ref() followed by put_page_and_type(). This
> then allows free_shared_domheap_page() to be purged.
> 
> There is, however, some fall-out from this simplification:
> 
> - alloc_domheap_page() will now call assign_pages() and run into the fact
>   that 'max_pages' is not set until some time after domain_create(). To
>   avoid an allocation failure, domain_create() is modified to set
>   max_pages to an initial value, sufficient to cover any domheap
>   allocations required to complete domain creation. The value will be
>   set to the 'real' max_pages when the tool-stack later performs the
>   XEN_DOMCTL_max_mem operation, thus allowing the rest of the domain's
>   memory to be allocated.
> 
> - Because the domheap page is no longer a pseudo-xenheap page, the
>   reference counting will prevent the domain from being destroyed. Thus
>   the call to vmx_free_vlapic_mapping() is moved from the
>   domain_destroy() method into the domain_relinquish_resources() method.
>   Whilst in the area, make the domain_destroy() method an optional
>   alternative_vcall() (since it will no longer peform any function in VMX
>   and is stubbed in SVM anyway).
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

This is an excellent change, thank you:

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

My only comment is that this would have been a bit easier to review
broken down into probably three patches: 1) making domain_destroy
optional, 2) moving vmx teardown to a relinquish_resources call 3) using
"normal" pages".  But I don't think it's worth a re-send just for that.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel