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

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Julien Grall [mailto:julien.grall@linaro.org]
> Sent: 20 October 2017 11:00
> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich'
> <JBeulich@suse.com>
> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Roger
> Pau Monne <roger.pau@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Stefano
> Stabellini <sstabellini@kernel.org>; xen-devel@lists.xenproject.org; Konrad
> Rzeszutek Wilk <konrad.wilk@oracle.com>; Daniel De Graaf
> <dgdegra@tycho.nsa.gov>; Tim (Xen.org) <tim@xen.org>
> 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@suse.com]
> >> Sent: 20 October 2017 07:25
> >> To: Julien Grall <julien.grall@linaro.org>
> >> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper
> >> <Andrew.Cooper3@citrix.com>; George Dunlap
> >> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Paul
> >> Durrant <Paul.Durrant@citrix.com>; Roger Pau Monne
> >> <roger.pau@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Stefano Stabellini
> >> <sstabellini@kernel.org>; xen-devel@lists.xenproject.org; Konrad
> Rzeszutek
> >> Wilk <konrad.wilk@oracle.com>; Daniel De Graaf
> <dgdegra@tycho.nsa.gov>;
> >> Tim (Xen.org) <tim@xen.org>
> >> 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@linaro.org> 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!
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().

> 
> 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?

Cheers,

  Paul

> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel