Re: [Xen-devel] [PATCH v12 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 20/10/17 11:10, Paul Durrant wrote:
> > -----Original Message-----
> > From: Julien Grall [mailto:julien.grall@xxxxxxxxxx]
> > Sent: 20 October 2017 11:00
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; 'Jan Beulich'
> > <JBeulich@xxxxxxxx>
> > Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> > <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> > <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Roger
> > Pau Monne <roger.pau@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Stefano
> > Stabellini <sstabellini@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx; Konrad
> > Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Daniel De Graaf
> > <dgdegra@xxxxxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
> > Subject: Re: [Xen-devel] [PATCH v12 05/11] x86/mm: add
> > HYPERVISOR_memory_op to acquire guest resources
> >
> > Hi Paul,
> >
> > On 20/10/17 09:26, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > > > Sent: 20 October 2017 07:25
> > > > To: Julien Grall <julien.grall@xxxxxxxxxx>
> > > > Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> > > > <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> > > > <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Paul
> > > > Durrant <Paul.Durrant@xxxxxxxxxx>; Roger Pau Monne
> > > > <roger.pau@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Stefano Stabellini
> > > > <sstabellini@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx; Konrad
> > Rzeszutek
> > > > Wilk <konrad.wilk@xxxxxxxxxx>; Daniel De Graaf
> > <dgdegra@xxxxxxxxxxxxx>;
> > > > Tim (Xen.org) <tim@xxxxxxx>
> > > > Subject: Re: [Xen-devel] [PATCH v12 05/11] x86/mm: add
> > > > HYPERVISOR_memory_op to acquire guest resources
> > > >
> > > > > > > On 19.10.17 at 18:21, <julien.grall@xxxxxxxxxx> wrote:
> > > > > Looking a bit more at the resource you can acquire from this hypercall.
> > > > > Some of them are allocated using alloc_xenheap_page() so not assigned
> > to
> > > > > a domain.
> > > > >
> > > > > So I am not sure how you can expect a function set_foreign_p2m_entry
> > to
> > > > > take reference in that case.
> > > > Hmm, with the domain parameter added, DOMID_XEN there (for
> > > > Xen heap pages) could identify no references to be taken, if that
> > > > was really the intended behavior in that case. However, even for
> > > > Xen heap pages life time tracking ought to be done - it is for a
> > > > reason that share_xen_page_with_guest() assigns the target
> > > > domain as the owner of such pages, as that allows get_page() to
> > > > succeed for them.
> Hi Julien,
>
> > > So, nothing I'm doing here is making anything worse, right? Grant tables are
> > assigned to the guest, and IOREQ server pages are allocated with
> > alloc_domheap_page() so nothing is anonymous.
> >
> > I don't think grant tables is assigned to the guest today. They are
> > allocated using xenheap_pages() and I can't find
> > share_xen_page_with_guest().
> The guest would not be able to map them if they were not assigned in some way!
Do you mean for PV? For HVM/PVH, we don't check whether the page is 
assigned (see gnttab_map_frame).
> See the code block at 
> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/common/grant_table.c;hb=HEAD#l1716
> It calls gnttab_create_shared_page() which is what calls through to 
> share_xen_page_with_guest().
Thank you for the link, I will have a look.

> > Anyway, I discussed with Stefano about it. set_foreign_p2m_entry is
> > going to be left unimplemented on Arm until someone as time to implement
> > correctly the function.
> That makes sense. Do you still have any issues with this patch apart from the 
> cosmetic ones you spotted in the header?
No. Although, may I request to add a comment in the ARM helpers about 
the reference counting?
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel