From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 06/12] xen: mark grant mapped pages as foreign Date: Wed, 7 Jan 2015 11:53:24 +0000 Message-ID: <1420631604.18631.54.camel@citrix.com> References: <1420570657-8203-1-git-send-email-david.vrabel@citrix.com> <1420570657-8203-7-git-send-email-david.vrabel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Y8pB3-0000DO-OS for xen-devel@lists.xenproject.org; Wed, 07 Jan 2015 11:53:29 +0000 In-Reply-To: <1420570657-8203-7-git-send-email-david.vrabel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Vrabel Cc: xen-devel@lists.xenproject.org, Jenny Herbert , Jenny Herbert , Boris Ostrovsky , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Tue, 2015-01-06 at 18:57 +0000, David Vrabel wrote: > From: Jenny Herbert > > Use the "foreign" page flag to mark pages that have a grant map. Use > page->private to store information of the grant (the granting domain > and the grant reference). > > Signed-off-by: Jenny Herbert > Signed-off-by: David Vrabel > --- > arch/x86/xen/p2m.c | 50 ++++++++++++++++++++++++++++++++++++++------- > include/xen/grant_table.h | 13 ++++++++++++ > 2 files changed, 56 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c > index 0d70814..22624a3 100644 > --- a/arch/x86/xen/p2m.c > +++ b/arch/x86/xen/p2m.c > @@ -648,6 +648,43 @@ bool set_phys_to_machine(unsigned long pfn, unsigned long mfn) > return true; > } > > +static int > +init_page_grant_ref(struct page *p, domid_t domid, grant_ref_t grantref) I'd be inclined to add "map" to the names somewhere, otherwise people might thing they need to call this when allocating a grant in the f.e. or other things. > +{ > +#ifdef CONFIG_X86_64 Rather than suggesting to add CONFIG_ARM_64 here I'll suggest BITS_PER_LONG >= 64. > + uint64_t gref; > + uint64_t* gref_p = &gref; > +#else > + uint64_t* gref_p = kmalloc(sizeof(uint64_t), GFP_KERNEL); Might this allocation be happening during e.g. swapping? I suppose it is backend only, and swapping to a loopback vbd would be pretty mad. If you can figure a reasonable use case for that you might want some extra GFP flags? Might this be hot enough to warrant using a specific kmem_cache? > + if (!gref) > + return -ENOMEM; > + uint64_t* gref = gref_p; > +#endif > + > + *gref_p = ((uint64_t) grantref << 32) | domid; > + p->private = gref; There is a set_page_private() macro, which doesn't seem to do much but I suppose you should use it (and page_private() for accessing, if you don't already). > @@ -182,4 +183,16 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops, > void gnttab_batch_map(struct gnttab_map_grant_ref *batch, unsigned count); > void gnttab_batch_copy(struct gnttab_copy *batch, unsigned count); > > +static inline void > +get_page_grant_ref(struct page *p, domid_t* domid, grant_ref_t* grantref) > +{ BUG_ON(!PageBlah(p))? > +#ifdef CONFIG_X86_64 > + uint64_t gref = p->private; > +#else > + uint64_t gref = *p->private; > +#endif > + *domid = gref & 0xffff; > + *grantref = gref >> 32; > +} > + > #endif /* __ASM_GNTTAB_H__ */