Re: [Xen-devel] [PATCH 06/12] xen: mark grant mapped pages as foreign
On Fri, 2015-01-09 at 16:39 +, David Vrabel wrote: > > ... or stubbed out for arches which don't need this (which might include > > arm*?). > > I'm reasonably certain that this is required for HVM and ARM guests as > well. The grant copy will still fail to get the page by gfn since the > mfn is owned by a different domain. Yes, I'd somehow convinced myself this snippet was to do with the m2p_override aspect of the series, which it's not. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 06/12] xen: mark grant mapped pages as foreign
On Fri, 9 Jan 2015, David Vrabel wrote:
> On 09/01/15 16:19, Ian Campbell wrote:
> > On Fri, 2015-01-09 at 16:03 +, Stefano Stabellini wrote:
> >> On Tue, 6 Jan 2015, 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)
> >>> +{
> >>> +#ifdef CONFIG_X86_64
> >>> + uint64_t gref;
> >>> + uint64_t* gref_p = &gref;
> >>> +#else
> >>> + uint64_t* gref_p = kmalloc(sizeof(uint64_t), GFP_KERNEL);
> >>> + if (!gref)
> >>> + return -ENOMEM;
> >>> + uint64_t* gref = gref_p;
> >>> +#endif
> >>> +
> >>> + *gref_p = ((uint64_t) grantref << 32) | domid;
> >>> + p->private = gref;
> >>> +
> >>> + WARN_ON(PagePrivate(p));
> >>> + WARN_ON(PageForeign(p));
> >>> + SetPagePrivate(p);
> >>> + SetPageForeign(p);
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static void
> >>> +clear_page_grant_ref(struct page *p)
> >>> +{
> >>> + WARN_ON(!PagePrivate(p));
> >>> + WARN_ON(!PageForeign(p));
> >>> +
> >>> +#ifndef CONFIG_X86_64
> >>> + kfree(p->private);
> >>> +#endif
> >>> + p->private = 0;
> >>> + ClearPagePrivate(p);
> >>> + ClearPageForeign(p);
> >>> +}
> >>
> >> Given that get_page_grant_ref is used by netback, these functions need
> >> to be made arch-independent, moved to an arch-independent code location
> >> and called appropriately.
>
> I've fixed this already.
>
> > ... or stubbed out for arches which don't need this (which might include
> > arm*?).
>
> I'm reasonably certain that this is required for HVM and ARM guests as
> well. The grant copy will still fail to get the page by gfn since the
> mfn is owned by a different domain.
I agree
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 06/12] xen: mark grant mapped pages as foreign
On 09/01/15 16:19, Ian Campbell wrote:
> On Fri, 2015-01-09 at 16:03 +, Stefano Stabellini wrote:
>> On Tue, 6 Jan 2015, 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)
>>> +{
>>> +#ifdef CONFIG_X86_64
>>> + uint64_t gref;
>>> + uint64_t* gref_p = &gref;
>>> +#else
>>> + uint64_t* gref_p = kmalloc(sizeof(uint64_t), GFP_KERNEL);
>>> + if (!gref)
>>> + return -ENOMEM;
>>> + uint64_t* gref = gref_p;
>>> +#endif
>>> +
>>> + *gref_p = ((uint64_t) grantref << 32) | domid;
>>> + p->private = gref;
>>> +
>>> + WARN_ON(PagePrivate(p));
>>> + WARN_ON(PageForeign(p));
>>> + SetPagePrivate(p);
>>> + SetPageForeign(p);
>>> + return 0;
>>> +}
>>> +
>>> +static void
>>> +clear_page_grant_ref(struct page *p)
>>> +{
>>> + WARN_ON(!PagePrivate(p));
>>> + WARN_ON(!PageForeign(p));
>>> +
>>> +#ifndef CONFIG_X86_64
>>> + kfree(p->private);
>>> +#endif
>>> + p->private = 0;
>>> + ClearPagePrivate(p);
>>> + ClearPageForeign(p);
>>> +}
>>
>> Given that get_page_grant_ref is used by netback, these functions need
>> to be made arch-independent, moved to an arch-independent code location
>> and called appropriately.
I've fixed this already.
> ... or stubbed out for arches which don't need this (which might include
> arm*?).
I'm reasonably certain that this is required for HVM and ARM guests as
well. The grant copy will still fail to get the page by gfn since the
mfn is owned by a different domain.
David
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 06/12] xen: mark grant mapped pages as foreign
On Fri, 2015-01-09 at 16:03 +, Stefano Stabellini wrote:
> On Tue, 6 Jan 2015, 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)
> > +{
> > +#ifdef CONFIG_X86_64
> > + uint64_t gref;
> > + uint64_t* gref_p = &gref;
> > +#else
> > + uint64_t* gref_p = kmalloc(sizeof(uint64_t), GFP_KERNEL);
> > + if (!gref)
> > + return -ENOMEM;
> > + uint64_t* gref = gref_p;
> > +#endif
> > +
> > + *gref_p = ((uint64_t) grantref << 32) | domid;
> > + p->private = gref;
> > +
> > + WARN_ON(PagePrivate(p));
> > + WARN_ON(PageForeign(p));
> > + SetPagePrivate(p);
> > + SetPageForeign(p);
> > + return 0;
> > +}
> > +
> > +static void
> > +clear_page_grant_ref(struct page *p)
> > +{
> > + WARN_ON(!PagePrivate(p));
> > + WARN_ON(!PageForeign(p));
> > +
> > +#ifndef CONFIG_X86_64
> > + kfree(p->private);
> > +#endif
> > + p->private = 0;
> > + ClearPagePrivate(p);
> > + ClearPageForeign(p);
> > +}
>
> Given that get_page_grant_ref is used by netback, these functions need
> to be made arch-independent, moved to an arch-independent code location
> and called appropriately.
... or stubbed out for arches which don't need this (which might include
arm*?).
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 06/12] xen: mark grant mapped pages as foreign
On Tue, 6 Jan 2015, 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)
> +{
> +#ifdef CONFIG_X86_64
> + uint64_t gref;
> + uint64_t* gref_p = &gref;
> +#else
> + uint64_t* gref_p = kmalloc(sizeof(uint64_t), GFP_KERNEL);
> + if (!gref)
> + return -ENOMEM;
> + uint64_t* gref = gref_p;
> +#endif
> +
> + *gref_p = ((uint64_t) grantref << 32) | domid;
> + p->private = gref;
> +
> + WARN_ON(PagePrivate(p));
> + WARN_ON(PageForeign(p));
> + SetPagePrivate(p);
> + SetPageForeign(p);
> + return 0;
> +}
> +
> +static void
> +clear_page_grant_ref(struct page *p)
> +{
> + WARN_ON(!PagePrivate(p));
> + WARN_ON(!PageForeign(p));
> +
> +#ifndef CONFIG_X86_64
> + kfree(p->private);
> +#endif
> + p->private = 0;
> + ClearPagePrivate(p);
> + ClearPageForeign(p);
> +}
Given that get_page_grant_ref is used by netback, these functions need
to be made arch-independent, moved to an arch-independent code location
and called appropriately.
> int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops,
> struct gnttab_map_grant_ref *kmap_ops,
> struct page **pages, unsigned int count)
> @@ -681,11 +718,12 @@ int set_foreign_p2m_mapping(struct gnttab_map_grant_ref
> *map_ops,
> }
> pfn = page_to_pfn(pages[i]);
>
> - WARN_ON(PagePrivate(pages[i]));
> - WARN(pfn_to_mfn(pfn) != INVALID_P2M_ENTRY, "page must be
> ballooned");
> + ret = init_page_grant_ref(pages[i],
> + map_ops[i].dom, map_ops[i].ref);
> + if (ret < 0)
> + goto out;
>
> - SetPagePrivate(pages[i]);
> - set_page_private(pages[i], mfn);
> + WARN(pfn_to_mfn(pfn) != INVALID_P2M_ENTRY, "page must be
> ballooned");
>
> if (unlikely(!set_phys_to_machine(pfn, FOREIGN_FRAME(mfn {
> ret = -ENOMEM;
> @@ -716,9 +754,7 @@ int clear_foreign_p2m_mapping(struct
> gnttab_unmap_grant_ref *unmap_ops,
> goto out;
> }
>
> - set_page_private(pages[i], INVALID_P2M_ENTRY);
> - WARN_ON(!PagePrivate(pages[i]));
> - ClearPagePrivate(pages[i]);
> + clear_page_grant_ref(pages[i]);
> set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
> }
> if (kunmap_ops)
> diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
> index 7235d8f..381f007 100644
> --- a/include/xen/grant_table.h
> +++ b/include/xen/grant_table.h
> @@ -45,6 +45,7 @@
> #include
>
> #include
> +#include
>
> #define GNTTAB_RESERVED_XENSTORE 1
>
> @@ -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)
> +{
> +#ifdef CONFIG_X86_64
> + uint64_t gref = p->private;
> +#else
> + uint64_t gref = *p->private;
> +#endif
> + *domid = gref & 0x;
> + *grantref = gref >> 32;
> +}
> +
> #endif /* __ASM_GNTTAB_H__ */
> --
> 1.7.10.4
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 06/12] xen: mark grant mapped pages as foreign
On Tue, 2015-01-06 at 18:57 +, 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 & 0x;
> + *grantref = gref >> 32;
> +}
> +
> #endif /* __ASM_GNTTAB_H__ */
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 06/12] xen: mark grant mapped pages as foreign
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)
+{
+#ifdef CONFIG_X86_64
+ uint64_t gref;
+ uint64_t* gref_p = &gref;
+#else
+ uint64_t* gref_p = kmalloc(sizeof(uint64_t), GFP_KERNEL);
+ if (!gref)
+ return -ENOMEM;
+ uint64_t* gref = gref_p;
+#endif
+
+ *gref_p = ((uint64_t) grantref << 32) | domid;
+ p->private = gref;
+
+ WARN_ON(PagePrivate(p));
+ WARN_ON(PageForeign(p));
+ SetPagePrivate(p);
+ SetPageForeign(p);
+ return 0;
+}
+
+static void
+clear_page_grant_ref(struct page *p)
+{
+ WARN_ON(!PagePrivate(p));
+ WARN_ON(!PageForeign(p));
+
+#ifndef CONFIG_X86_64
+ kfree(p->private);
+#endif
+ p->private = 0;
+ ClearPagePrivate(p);
+ ClearPageForeign(p);
+}
+
int set_foreign_p2m_mapping(struct gnttab_map_grant_ref *map_ops,
struct gnttab_map_grant_ref *kmap_ops,
struct page **pages, unsigned int count)
@@ -681,11 +718,12 @@ int set_foreign_p2m_mapping(struct gnttab_map_grant_ref
*map_ops,
}
pfn = page_to_pfn(pages[i]);
- WARN_ON(PagePrivate(pages[i]));
- WARN(pfn_to_mfn(pfn) != INVALID_P2M_ENTRY, "page must be
ballooned");
+ ret = init_page_grant_ref(pages[i],
+ map_ops[i].dom, map_ops[i].ref);
+ if (ret < 0)
+ goto out;
- SetPagePrivate(pages[i]);
- set_page_private(pages[i], mfn);
+ WARN(pfn_to_mfn(pfn) != INVALID_P2M_ENTRY, "page must be
ballooned");
if (unlikely(!set_phys_to_machine(pfn, FOREIGN_FRAME(mfn {
ret = -ENOMEM;
@@ -716,9 +754,7 @@ int clear_foreign_p2m_mapping(struct gnttab_unmap_grant_ref
*unmap_ops,
goto out;
}
- set_page_private(pages[i], INVALID_P2M_ENTRY);
- WARN_ON(!PagePrivate(pages[i]));
- ClearPagePrivate(pages[i]);
+ clear_page_grant_ref(pages[i]);
set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
}
if (kunmap_ops)
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index 7235d8f..381f007 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -45,6 +45,7 @@
#include
#include
+#include
#define GNTTAB_RESERVED_XENSTORE 1
@@ -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)
+{
+#ifdef CONFIG_X86_64
+ uint64_t gref = p->private;
+#else
+ uint64_t gref = *p->private;
+#endif
+ *domid = gref & 0x;
+ *grantref = gref >> 32;
+}
+
#endif /* __ASM_GNTTAB_H__ */
--
1.7.10.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
