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

["Durrant, Paul" <pdurrant@xxxxxxxxxxxx>]

> -----Original Message-----
> From: George Dunlap [mailto:george.dunlap@xxxxxxxxxx]
> Sent: 23 January 2020 14:46
> To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Jan Beulich <jbeulich@xxxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné
> <roger.pau@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxxxxx>; Ian
> Jackson <ian.jackson@xxxxxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Konrad
> Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Kevin
> Tian <kevin.tian@xxxxxxxxx>
> Subject: Re: [PATCH v3 3/3] x86 / vmx: use a 'normal' domheap page for
> APIC_DEFAULT_PHYS_BASE
> 
> 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@xxxxxxxxxx>
> 
> This is an excellent change, thank you:
> 
> Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> 
> 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.

Since I appear to need to do a v4 to re-work the allocation (assuming I can 
steal another PGC bit) I'll split things as you suggest.

  Paul

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