annie.li@oracle.com
2011-Dec-08 09:35 UTC
[PATCH V2 0/2] xen: patches for supporting sub-page and transitive grants
Hi Following two patches introduce and implement interfaces for sub-page and transitive grants, and they are based on linux-next branch of linux/kernel/git/konrad/xen.git(3.2.0-rc2+). Sub-page and transitive grants are new types of grant table V2. Descriptions for those patches: Through sub-page grant table interfaces, a domain can grant another domain access to a range of bytes within a page, and Xen will then prevent the grantee domain accessing outside that range. For obvious reasons, it isn''t possible to map these grant references, and domains are expected to use the grant copy hypercall instead. Through transitive grant interfaces, a domain can create grant reference which redirects to another grant reference, so that any attempt to access the first grant reference will be redirected to the second one. This is used to implement receiver-side copy on inter-domain traffic: rather than copying the packet in dom0, dom0 creates a transitive grant referencing the original transmit buffer, and passes that to the receiving domain. Annie Li (2): xen/granttable: Support sub-page grants xen/granttable: Support transitive grants drivers/xen/grant-table.c | 147 +++++++++++++++++++++++++++++++++++++++++++++ include/xen/grant_table.h | 25 ++++++++ 2 files changed, 172 insertions(+), 0 deletions(-) Thanks Annie -- 1.7.6.4
annie.li@oracle.com
2011-Dec-08 09:37 UTC
[PATCH V2 1/2] xen/granttable: Support sub-page grants
-- They can''t be used to map the page (so can only be used in a
GNTTABOP_copy
hypercall).
-- It''s possible to grant access with a finer granularity than
whole pages.
-- Xen guarantees that they can be revoked quickly (a normal map grant can
only be revoked with the cooperation of the domain which has been granted
access).
Signed-off-by: Annie Li <annie.li@oracle.com>
---
drivers/xen/grant-table.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
include/xen/grant_table.h | 13 ++++++++
2 files changed, 87 insertions(+), 0 deletions(-)
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index bd325fd..4a10e3f 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -120,6 +120,16 @@ struct gnttab_ops {
* by bit operations.
*/
int (*query_foreign_access)(grant_ref_t);
+ /*
+ * Grant a domain to access a range of bytes within the page referred by
+ * an available grant entry. First parameter is grant entry reference
+ * number, second one is id of grantee domain, third one is frame
+ * address of subpage grant, forth one is grant type and flag
+ * information, fifth one is offset of the range of bytes, and last one
+ * is length of bytes to be accessed.
+ */
+ void (*update_subpage_entry)(grant_ref_t, domid_t, unsigned long, int,
+ unsigned, unsigned);
};
static struct gnttab_ops *gnttab_interface;
@@ -261,6 +271,69 @@ int gnttab_grant_foreign_access(domid_t domid, unsigned
long frame,
}
EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
+void gnttab_update_subpage_entry_v2(grant_ref_t ref, domid_t domid,
+ unsigned long frame, int flags,
+ unsigned page_off,
+ unsigned length)
+{
+ gnttab_shared.v2[ref].sub_page.frame = frame;
+ gnttab_shared.v2[ref].sub_page.page_off = page_off;
+ gnttab_shared.v2[ref].sub_page.length = length;
+ gnttab_shared.v2[ref].hdr.domid = domid;
+ wmb();
+ gnttab_shared.v2[ref].hdr.flags + GTF_permit_access | GTF_sub_page | flags;
+}
+
+int gnttab_grant_foreign_access_subpage_ref(grant_ref_t ref, domid_t domid,
+ unsigned long frame, int flags,
+ unsigned page_off,
+ unsigned length)
+{
+ if (flags & (GTF_accept_transfer | GTF_reading |
+ GTF_writing | GTF_transitive))
+ return -EPERM;
+
+ if (gnttab_interface->update_subpage_entry == NULL)
+ return -ENOSYS;
+
+ gnttab_interface->update_subpage_entry(ref, domid, frame, flags,
+ page_off, length);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage_ref);
+
+int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned long frame,
+ int flags, unsigned page_off,
+ unsigned length)
+{
+ int ref;
+
+ if (flags & (GTF_accept_transfer | GTF_reading |
+ GTF_writing | GTF_transitive))
+ return -EPERM;
+
+ if (gnttab_interface->update_subpage_entry == NULL)
+ return -ENOSYS;
+
+ ref = get_free_entries(1);
+ if (unlikely(ref < 0))
+ return -ENOSPC;
+
+ gnttab_interface->update_subpage_entry(ref, domid, frame, flags,
+ page_off, length);
+
+ return ref;
+}
+EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage);
+
+bool gnttab_subpage_grants_available(void)
+{
+ return gnttab_interface->update_subpage_entry != NULL;
+}
+EXPORT_SYMBOL_GPL(gnttab_subpage_grants_available);
+
static int gnttab_query_foreign_access_v1(grant_ref_t ref)
{
return gnttab_shared.v1[ref].flags & (GTF_reading|GTF_writing);
@@ -813,6 +886,7 @@ static struct gnttab_ops gnttab_v2_ops = {
.end_foreign_access_ref = gnttab_end_foreign_access_ref_v2,
.end_foreign_transfer_ref = gnttab_end_foreign_transfer_ref_v2,
.query_foreign_access = gnttab_query_foreign_access_v2,
+ .update_subpage_entry = gnttab_update_subpage_entry_v2,
};
static void gnttab_request_version(void)
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index fea4954..2b492b9 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -62,6 +62,15 @@ int gnttab_resume(void);
int gnttab_grant_foreign_access(domid_t domid, unsigned long frame,
int readonly);
+int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned long frame,
+ int flags, unsigned page_off,
+ unsigned length);
+
+/*
+ * Are sub-page grants available on this version of Xen? Returns true if they
+ * are, and false if they''re not.
+ */
+bool gnttab_subpage_grants_available(void);
/*
* End access through the given grant reference, iff the grant entry is no
@@ -108,6 +117,10 @@ void gnttab_cancel_free_callback(struct
gnttab_free_callback *callback);
void gnttab_grant_foreign_access_ref(grant_ref_t ref, domid_t domid,
unsigned long frame, int readonly);
+int gnttab_grant_foreign_access_subpage_ref(grant_ref_t ref, domid_t domid,
+ unsigned long frame, int flags,
+ unsigned page_off,
+ unsigned length);
void gnttab_grant_foreign_transfer_ref(grant_ref_t, domid_t domid,
unsigned long pfn);
--
1.7.6.4
annie.li@oracle.com
2011-Dec-08 09:37 UTC
[PATCH V2 2/2] xen/granttable: Support transitive grants
These allow a domain A which has been granted access on a page of domain
B''s
memory to issue domain C with a copy-grant on the same page. This is useful
e.g. for forwarding packets between domains.
Signed-off-by: Annie Li <annie.li@oracle.com>
---
drivers/xen/grant-table.c | 73 +++++++++++++++++++++++++++++++++++++++++++++
include/xen/grant_table.h | 12 +++++++
2 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 4a10e3f..db3e7c0 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -130,6 +130,18 @@ struct gnttab_ops {
*/
void (*update_subpage_entry)(grant_ref_t, domid_t, unsigned long, int,
unsigned, unsigned);
+ /*
+ * Redirect an available grant entry on domain A to another grant
+ * reference of domain B, then allow domain C to use grant reference
+ * of domain B transitively. First parameter is an available grant entry
+ * reference on domain A, second one is id of domain C which accesses
+ * grant entry transitively, third one is grant type and flag
+ * information, forth one is id of domain B whose grant entry is finally
+ * accessed transitively, last one is grant entry transitive reference
+ * of domain B.
+ */
+ void (*update_trans_entry)(grant_ref_t, domid_t, int, domid_t,
+ grant_ref_t);
};
static struct gnttab_ops *gnttab_interface;
@@ -334,6 +346,66 @@ bool gnttab_subpage_grants_available(void)
}
EXPORT_SYMBOL_GPL(gnttab_subpage_grants_available);
+void gnttab_update_trans_entry_v2(grant_ref_t ref, domid_t domid,
+ int flags, domid_t trans_domid,
+ grant_ref_t trans_gref)
+{
+ gnttab_shared.v2[ref].transitive.trans_domid = trans_domid;
+ gnttab_shared.v2[ref].transitive.gref = trans_gref;
+ gnttab_shared.v2[ref].hdr.domid = domid;
+ wmb();
+ gnttab_shared.v2[ref].hdr.flags + GTF_permit_access | GTF_transitive |
flags;
+}
+
+int gnttab_grant_foreign_access_trans_ref(grant_ref_t ref, domid_t domid,
+ int flags, domid_t trans_domid,
+ grant_ref_t trans_gref)
+{
+ if (flags & (GTF_accept_transfer | GTF_reading |
+ GTF_writing | GTF_sub_page))
+ return -EPERM;
+
+ if (gnttab_interface->update_trans_entry == NULL)
+ return -ENOSYS;
+
+ gnttab_interface->update_trans_entry(ref, domid, flags, trans_domid,
+ trans_gref);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_trans_ref);
+
+int gnttab_grant_foreign_access_trans(domid_t domid, int flags,
+ domid_t trans_domid,
+ grant_ref_t trans_gref)
+{
+ int ref;
+
+ if (flags & (GTF_accept_transfer | GTF_reading |
+ GTF_writing | GTF_sub_page))
+ return -EPERM;
+
+ if (gnttab_interface->update_trans_entry == NULL)
+ return -ENOSYS;
+
+ ref = get_free_entries(1);
+ if (unlikely(ref < 0))
+ return -ENOSPC;
+
+ gnttab_interface->update_trans_entry(ref, domid, flags, trans_domid,
+ trans_gref);
+
+ return ref;
+}
+EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_trans);
+
+bool gnttab_trans_grants_available(void)
+{
+ return gnttab_interface->update_trans_entry != NULL;
+}
+EXPORT_SYMBOL_GPL(gnttab_trans_grants_available);
+
static int gnttab_query_foreign_access_v1(grant_ref_t ref)
{
return gnttab_shared.v1[ref].flags & (GTF_reading|GTF_writing);
@@ -887,6 +959,7 @@ static struct gnttab_ops gnttab_v2_ops = {
.end_foreign_transfer_ref = gnttab_end_foreign_transfer_ref_v2,
.query_foreign_access = gnttab_query_foreign_access_v2,
.update_subpage_entry = gnttab_update_subpage_entry_v2,
+ .update_trans_entry = gnttab_update_trans_entry_v2,
};
static void gnttab_request_version(void)
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index 2b492b9..f1e17b7 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -65,6 +65,9 @@ int gnttab_grant_foreign_access(domid_t domid, unsigned long
frame,
int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned long frame,
int flags, unsigned page_off,
unsigned length);
+int gnttab_grant_foreign_access_trans(domid_t domid, int flags,
+ domid_t trans_domid,
+ grant_ref_t trans_gref);
/*
* Are sub-page grants available on this version of Xen? Returns true if they
@@ -73,6 +76,12 @@ int gnttab_grant_foreign_access_subpage(domid_t domid,
unsigned long frame,
bool gnttab_subpage_grants_available(void);
/*
+ * Are transitive grants available on this version of Xen? Returns true if
they
+ * are, and false if they''re not.
+ */
+bool gnttab_trans_grants_available(void);
+
+/*
* End access through the given grant reference, iff the grant entry is no
* longer in use. Return 1 if the grant entry was freed, 0 if it is still in
* use.
@@ -121,6 +130,9 @@ int gnttab_grant_foreign_access_subpage_ref(grant_ref_t ref,
domid_t domid,
unsigned long frame, int flags,
unsigned page_off,
unsigned length);
+int gnttab_grant_foreign_access_trans_ref(grant_ref_t ref, domid_t domid,
+ int flags, domid_t trans_domid,
+ grant_ref_t trans_gref);
void gnttab_grant_foreign_transfer_ref(grant_ref_t, domid_t domid,
unsigned long pfn);
--
1.7.6.4
Paul Durrant
2011-Dec-08 09:49 UTC
RE: [PATCH V2 1/2] xen/granttable: Support sub-page grants
> -----Original Message----- > From: annie.li@oracle.com [mailto:annie.li@oracle.com] > Sent: 08 December 2011 09:37 > To: xen-devel@lists.xensource.com; linux-kernel@vger.kernel.org; > konrad.wilk@oracle.com; jeremy@goop.org; Paul Durrant; Ian Campbell > Cc: kurt.hackel@oracle.com; annie.li@oracle.com > Subject: [PATCH V2 1/2] xen/granttable: Support sub-page grants > > -- They can''t be used to map the page (so can only be used in a > GNTTABOP_copy > hypercall). > -- It''s possible to grant access with a finer granularity than > whole pages. > -- Xen guarantees that they can be revoked quickly (a normal map > grant can > only be revoked with the cooperation of the domain which has > been granted > access). > > Signed-off-by: Annie Li <annie.li@oracle.com> > --- > drivers/xen/grant-table.c | 74 > +++++++++++++++++++++++++++++++++++++++++++++ > include/xen/grant_table.h | 13 ++++++++ > 2 files changed, 87 insertions(+), 0 deletions(-) > > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c > index bd325fd..4a10e3f 100644 > --- a/drivers/xen/grant-table.c > +++ b/drivers/xen/grant-table.c > @@ -120,6 +120,16 @@ struct gnttab_ops { > * by bit operations. > */ > int (*query_foreign_access)(grant_ref_t); > + /* > + * Grant a domain to access a range of bytes within the page > referred by > + * an available grant entry. First parameter is grant entry > reference > + * number, second one is id of grantee domain, third one is > frame > + * address of subpage grant, forth one is grant type and flag > + * information, fifth one is offset of the range of bytes, > and last one > + * is length of bytes to be accessed. > + */ > + void (*update_subpage_entry)(grant_ref_t, domid_t, unsigned > long, int, > + unsigned, unsigned); > }; > > static struct gnttab_ops *gnttab_interface; @@ -261,6 +271,69 @@ > int gnttab_grant_foreign_access(domid_t domid, unsigned long frame, > } EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access); > > +void gnttab_update_subpage_entry_v2(grant_ref_t ref, domid_t domid, > + unsigned long frame, int flags, > + unsigned page_off, > + unsigned length) > +{ > + gnttab_shared.v2[ref].sub_page.frame = frame; > + gnttab_shared.v2[ref].sub_page.page_off = page_off; > + gnttab_shared.v2[ref].sub_page.length = length; > + gnttab_shared.v2[ref].hdr.domid = domid; > + wmb(); > + gnttab_shared.v2[ref].hdr.flags > + GTF_permit_access | GTF_sub_page | > flags; } > + > +int gnttab_grant_foreign_access_subpage_ref(grant_ref_t ref, > domid_t domid, > + unsigned long frame, int > flags, > + unsigned page_off, > + unsigned length) > +{ > + if (flags & (GTF_accept_transfer | GTF_reading | > + GTF_writing | GTF_transitive)) > + return -EPERM; > + > + if (gnttab_interface->update_subpage_entry == NULL) > + return -ENOSYS; > + > + gnttab_interface->update_subpage_entry(ref, domid, frame, > flags, > + page_off, length); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage_ref); > + > +int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned > long frame, > + int flags, unsigned page_off, > + unsigned length) > +{ > + int ref; > + > + if (flags & (GTF_accept_transfer | GTF_reading | > + GTF_writing | GTF_transitive)) > + return -EPERM; > + > + if (gnttab_interface->update_subpage_entry == NULL) > + return -ENOSYS; > + > + ref = get_free_entries(1); > + if (unlikely(ref < 0)) > + return -ENOSPC; > + > + gnttab_interface->update_subpage_entry(ref, domid, frame, > flags, > + page_off, length); > + > + return ref; > +} > +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage);There''s quite a lot of duplicated code here. What about something along the lines of: #define get_free_entry() get_free_entries(1) int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned long frame, int flags, unsigned page_off, unsigned length) { int ref; ref = get_free_entry(); if (unlikely(ref < 0)) return -ENOSPC; rc = gnttab_grant_foreign_access_subpage_ref(ref, domid, frame, flags, page_off, length); if (rc < 0) put_free_entry(ref); return (rc < 0) rc : ref; } EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage); I think this is more akin to the format for existing non-ref variants.> + > +bool gnttab_subpage_grants_available(void) > +{ > + return gnttab_interface->update_subpage_entry != NULL; } > +EXPORT_SYMBOL_GPL(gnttab_subpage_grants_available); > + > static int gnttab_query_foreign_access_v1(grant_ref_t ref) { > return gnttab_shared.v1[ref].flags & > (GTF_reading|GTF_writing); @@ -813,6 +886,7 @@ static struct > gnttab_ops gnttab_v2_ops = { > .end_foreign_access_ref > gnttab_end_foreign_access_ref_v2, > .end_foreign_transfer_ref > gnttab_end_foreign_transfer_ref_v2, > .query_foreign_access > gnttab_query_foreign_access_v2, > + .update_subpage_entry > gnttab_update_subpage_entry_v2, > }; > > static void gnttab_request_version(void) diff --git > a/include/xen/grant_table.h b/include/xen/grant_table.h index > fea4954..2b492b9 100644 > --- a/include/xen/grant_table.h > +++ b/include/xen/grant_table.h > @@ -62,6 +62,15 @@ int gnttab_resume(void); > > int gnttab_grant_foreign_access(domid_t domid, unsigned long frame, > int readonly); > +int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned > long frame, > + int flags, unsigned page_off, > + unsigned length); > + > +/* > + * Are sub-page grants available on this version of Xen? Returns > true > +if they > + * are, and false if they''re not. > + */ > +bool gnttab_subpage_grants_available(void); > > /* > * End access through the given grant reference, iff the grant > entry is no @@ -108,6 +117,10 @@ void > gnttab_cancel_free_callback(struct gnttab_free_callback *callback); > > void gnttab_grant_foreign_access_ref(grant_ref_t ref, domid_t > domid, > unsigned long frame, int readonly); > +int gnttab_grant_foreign_access_subpage_ref(grant_ref_t ref, > domid_t domid, > + unsigned long frame, int > flags, > + unsigned page_off, > + unsigned length); > > void gnttab_grant_foreign_transfer_ref(grant_ref_t, domid_t domid, > unsigned long pfn); > -- > 1.7.6.4
Paul Durrant
2011-Dec-08 09:51 UTC
RE: [PATCH V2 2/2] xen/granttable: Support transitive grants
> -----Original Message----- > From: annie.li@oracle.com [mailto:annie.li@oracle.com] > Sent: 08 December 2011 09:38 > To: xen-devel@lists.xensource.com; linux-kernel@vger.kernel.org; > konrad.wilk@oracle.com; jeremy@goop.org; Paul Durrant; Ian Campbell > Cc: kurt.hackel@oracle.com; annie.li@oracle.com > Subject: [PATCH V2 2/2] xen/granttable: Support transitive grants > > These allow a domain A which has been granted access on a page of > domain B''s memory to issue domain C with a copy-grant on the same > page. This is useful e.g. for forwarding packets between domains. > > Signed-off-by: Annie Li <annie.li@oracle.com> > --- > drivers/xen/grant-table.c | 73 > +++++++++++++++++++++++++++++++++++++++++++++ > include/xen/grant_table.h | 12 +++++++ > 2 files changed, 85 insertions(+), 0 deletions(-) > > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c > index 4a10e3f..db3e7c0 100644 > --- a/drivers/xen/grant-table.c > +++ b/drivers/xen/grant-table.c > @@ -130,6 +130,18 @@ struct gnttab_ops { > */ > void (*update_subpage_entry)(grant_ref_t, domid_t, unsigned > long, int, > unsigned, unsigned); > + /* > + * Redirect an available grant entry on domain A to another > grant > + * reference of domain B, then allow domain C to use grant > reference > + * of domain B transitively. First parameter is an available > grant entry > + * reference on domain A, second one is id of domain C which > accesses > + * grant entry transitively, third one is grant type and flag > + * information, forth one is id of domain B whose grant entry > is finally > + * accessed transitively, last one is grant entry transitive > reference > + * of domain B. > + */ > + void (*update_trans_entry)(grant_ref_t, domid_t, int, > domid_t, > + grant_ref_t); > }; > > static struct gnttab_ops *gnttab_interface; @@ -334,6 +346,66 @@ > bool gnttab_subpage_grants_available(void) > } > EXPORT_SYMBOL_GPL(gnttab_subpage_grants_available); > > +void gnttab_update_trans_entry_v2(grant_ref_t ref, domid_t domid, > + int flags, domid_t trans_domid, > + grant_ref_t trans_gref) > +{ > + gnttab_shared.v2[ref].transitive.trans_domid = trans_domid; > + gnttab_shared.v2[ref].transitive.gref = trans_gref; > + gnttab_shared.v2[ref].hdr.domid = domid; > + wmb(); > + gnttab_shared.v2[ref].hdr.flags > + GTF_permit_access | GTF_transitive | > flags; } > + > +int gnttab_grant_foreign_access_trans_ref(grant_ref_t ref, domid_t > domid, > + int flags, domid_t trans_domid, > + grant_ref_t trans_gref) > +{ > + if (flags & (GTF_accept_transfer | GTF_reading | > + GTF_writing | GTF_sub_page)) > + return -EPERM; > + > + if (gnttab_interface->update_trans_entry == NULL) > + return -ENOSYS; > + > + gnttab_interface->update_trans_entry(ref, domid, flags, > trans_domid, > + trans_gref); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_trans_ref); > + > +int gnttab_grant_foreign_access_trans(domid_t domid, int flags, > + domid_t trans_domid, > + grant_ref_t trans_gref) > +{ > + int ref; > + > + if (flags & (GTF_accept_transfer | GTF_reading | > + GTF_writing | GTF_sub_page)) > + return -EPERM; > + > + if (gnttab_interface->update_trans_entry == NULL) > + return -ENOSYS; > + > + ref = get_free_entries(1); > + if (unlikely(ref < 0)) > + return -ENOSPC; > + > + gnttab_interface->update_trans_entry(ref, domid, flags, > trans_domid, > + trans_gref); > + > + return ref; > +} > +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_trans); > +I have the same opinion here as with the other patch. The non-ref variant should allocate and then call the ref variant.> +bool gnttab_trans_grants_available(void) > +{ > + return gnttab_interface->update_trans_entry != NULL; } > +EXPORT_SYMBOL_GPL(gnttab_trans_grants_available); > + > static int gnttab_query_foreign_access_v1(grant_ref_t ref) { > return gnttab_shared.v1[ref].flags & > (GTF_reading|GTF_writing); @@ -887,6 +959,7 @@ static struct > gnttab_ops gnttab_v2_ops = { > .end_foreign_transfer_ref > gnttab_end_foreign_transfer_ref_v2, > .query_foreign_access > gnttab_query_foreign_access_v2, > .update_subpage_entry > gnttab_update_subpage_entry_v2, > + .update_trans_entry = gnttab_update_trans_entry_v2, > }; > > static void gnttab_request_version(void) diff --git > a/include/xen/grant_table.h b/include/xen/grant_table.h index > 2b492b9..f1e17b7 100644 > --- a/include/xen/grant_table.h > +++ b/include/xen/grant_table.h > @@ -65,6 +65,9 @@ int gnttab_grant_foreign_access(domid_t domid, > unsigned long frame, int > gnttab_grant_foreign_access_subpage(domid_t domid, unsigned long > frame, > int flags, unsigned page_off, > unsigned length); > +int gnttab_grant_foreign_access_trans(domid_t domid, int flags, > + domid_t trans_domid, > + grant_ref_t trans_gref); > > /* > * Are sub-page grants available on this version of Xen? Returns > true if they @@ -73,6 +76,12 @@ int > gnttab_grant_foreign_access_subpage(domid_t domid, unsigned long > frame, bool gnttab_subpage_grants_available(void); > > /* > + * Are transitive grants available on this version of Xen? Returns > +true if they > + * are, and false if they''re not. > + */ > +bool gnttab_trans_grants_available(void); > + > +/* > * End access through the given grant reference, iff the grant > entry is no > * longer in use. Return 1 if the grant entry was freed, 0 if it > is still in > * use. > @@ -121,6 +130,9 @@ int > gnttab_grant_foreign_access_subpage_ref(grant_ref_t ref, domid_t > domid, > unsigned long frame, int > flags, > unsigned page_off, > unsigned length); > +int gnttab_grant_foreign_access_trans_ref(grant_ref_t ref, domid_t > domid, > + int flags, domid_t trans_domid, > + grant_ref_t trans_gref); > > void gnttab_grant_foreign_transfer_ref(grant_ref_t, domid_t domid, > unsigned long pfn); > -- > 1.7.6.4
>> + >> +int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned >> long frame, >> + int flags, unsigned page_off, >> + unsigned length) >> +{ >> + int ref; >> + >> + if (flags& (GTF_accept_transfer | GTF_reading | >> + GTF_writing | GTF_transitive)) >> + return -EPERM; >> + >> + if (gnttab_interface->update_subpage_entry == NULL) >> + return -ENOSYS; >> + >> + ref = get_free_entries(1); >> + if (unlikely(ref< 0)) >> + return -ENOSPC; >> + >> + gnttab_interface->update_subpage_entry(ref, domid, frame, >> flags, >> + page_off, length); >> + >> + return ref; >> +} >> +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage); > There''s quite a lot of duplicated code here. What about something along the lines of: > > #define get_free_entry() get_free_entries(1) > > int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned long frame, > int flags, unsigned page_off, > unsigned length) > { > int ref; > > ref = get_free_entry(); > if (unlikely(ref< 0)) > return -ENOSPC; > > rc = gnttab_grant_foreign_access_subpage_ref(ref, domid, frame, flags, page_off, length); > if (rc< 0) > put_free_entry(ref); > > return (rc< 0) rc : ref; > } > EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage); > > I think this is more akin to the format for existing non-ref variants. >I hesitated between those two implement ways before sending them out. Those two ways all have shortcoming, one has duplicated code, but is simple when condition does not meet. The other way you pointed out added more put_free_entry process when _ref function fails. Thanks Annie
Paul Durrant
2011-Dec-08 10:04 UTC
RE: [PATCH V2 1/2] xen/granttable: Support sub-page grants
> -----Original Message----- > From: ANNIE LI [mailto:annie.li@oracle.com] > Sent: 08 December 2011 10:02 > To: Paul Durrant > Cc: xen-devel@lists.xensource.com; linux-kernel@vger.kernel.org; > konrad.wilk@oracle.com; jeremy@goop.org; Ian Campbell; > kurt.hackel@oracle.com > Subject: Re: [PATCH V2 1/2] xen/granttable: Support sub-page grants > > > >> + > >> +int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned > >> long frame, > >> + int flags, unsigned page_off, > >> + unsigned length) > >> +{ > >> + int ref; > >> + > >> + if (flags& (GTF_accept_transfer | GTF_reading | > >> + GTF_writing | GTF_transitive)) > >> + return -EPERM; > >> + > >> + if (gnttab_interface->update_subpage_entry == NULL) > >> + return -ENOSYS; > >> + > >> + ref = get_free_entries(1); > >> + if (unlikely(ref< 0)) > >> + return -ENOSPC; > >> + > >> + gnttab_interface->update_subpage_entry(ref, domid, frame, > >> flags, > >> + page_off, length); > >> + > >> + return ref; > >> +} > >> +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage); > > There''s quite a lot of duplicated code here. What about something > along the lines of: > > > > #define get_free_entry() get_free_entries(1) > > > > int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned > long frame, > > int flags, unsigned page_off, > > unsigned length) > > { > > int ref; > > > > ref = get_free_entry(); > > if (unlikely(ref< 0)) > > return -ENOSPC; > > > > rc = gnttab_grant_foreign_access_subpage_ref(ref, domid, > frame, flags, page_off, length); > > if (rc< 0) > > put_free_entry(ref); > > > > return (rc< 0) rc : ref; > > } > > EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage); > > > > I think this is more akin to the format for existing non-ref > variants. > > > I hesitated between those two implement ways before sending them > out. > Those two ways all have shortcoming, one has duplicated code, but is > simple when condition does not meet. The other way you pointed out > added more put_free_entry process when _ref function fails. >I say let''s be optimistic :-) Yes, there''s more overhead in the failure case, but that''s not what we should be optimising for. Paul
> > > I say let''s be optimistic :-) Yes, there''s more overhead in the failure case, but that''s not what we should be optimising for. >From positive side, it is OK. I agree with you since the failure case does not occurs frequently, the other goodness is the code style can keep identical with gnttab_grant_foreign_access and gnttab_grant_foreign_access_ref. thanks. Thanks Annie
Hi Paul,>>> #define get_free_entry() get_free_entries(1)Is this necessary? Maybe you defined this to keep consistence with put_free_entry(ref)? But other functions such as gnttab_grant_foreign_transfer and gnttab_grant_foreign_access all call get_free_entries(1). Maybe it is better to keep initial get_free_entries(1) code? Thanks Annie
On 2011-12-9 13:19, annie li wrote:> Hi Paul, > >>>> #define get_free_entry() get_free_entries(1) > Is this necessary? Maybe you defined this to keep consistence with > put_free_entry(ref)? > But other functions such as gnttab_grant_foreign_transfer and > gnttab_grant_foreign_access all call get_free_entries(1). Maybe it is > better to keep initial get_free_entries(1) code?Another approach is doing those work in a separate patch -- changing get_free_entries to get_free_entry in following 4 functions: gnttab_grant_foreign_access gnttab_grant_foreign_access_subpage gnttab_grant_foreign_access_trans gnttab_grant_foreign_transfer Thanks Annie
Ian Campbell
2011-Dec-09 08:37 UTC
Re: [PATCH V2 1/2] xen/granttable: Support sub-page grants
On Fri, 2011-12-09 at 07:45 +0000, annie li wrote:> > On 2011-12-9 13:19, annie li wrote: > > Hi Paul, > > > >>>> #define get_free_entry() get_free_entries(1) > > Is this necessary? Maybe you defined this to keep consistence with > > put_free_entry(ref)? > > But other functions such as gnttab_grant_foreign_transfer and > > gnttab_grant_foreign_access all call get_free_entries(1). Maybe it is > > better to keep initial get_free_entries(1) code? > Another approach is doing those work in a separate patch -- changing > get_free_entries to get_free_entry in following 4 functions:I think you shouldn''t get too bogged down in get_free_entry() vs get_free_entries(1) for the purposes of this patch series. Ian.> gnttab_grant_foreign_access > gnttab_grant_foreign_access_subpage > gnttab_grant_foreign_access_trans > gnttab_grant_foreign_transfer > > Thanks > Annie
Paul Durrant
2011-Dec-09 08:48 UTC
RE: [PATCH V2 1/2] xen/granttable: Support sub-page grants
> -----Original Message----- > From: Ian Campbell > Sent: 09 December 2011 08:37 > To: annie li > Cc: Paul Durrant; xen-devel@lists.xensource.com; linux- > kernel@vger.kernel.org; konrad.wilk@oracle.com; jeremy@goop.org; > kurt.hackel@oracle.com > Subject: Re: [PATCH V2 1/2] xen/granttable: Support sub-page grants > > On Fri, 2011-12-09 at 07:45 +0000, annie li wrote: > > > > On 2011-12-9 13:19, annie li wrote: > > > Hi Paul, > > > > > >>>> #define get_free_entry() get_free_entries(1) > > > Is this necessary? Maybe you defined this to keep consistence > with > > > put_free_entry(ref)? > > > But other functions such as gnttab_grant_foreign_transfer and > > > gnttab_grant_foreign_access all call get_free_entries(1). Maybe > it > > > is better to keep initial get_free_entries(1) code? > > Another approach is doing those work in a separate patch -- > changing > > get_free_entries to get_free_entry in following 4 functions: > > I think you shouldn't get too bogged down in get_free_entry() vs > get_free_entries(1) for the purposes of this patch series. >Annie, Yes, don't worry about get_free_entry(). I only defined it for the sake of symmetry in the code I posted. Paul
On 2011-12-9 16:48, Paul Durrant wrote:>> -----Original Message----- >> From: Ian Campbell >> Sent: 09 December 2011 08:37 >> To: annie li >> Cc: Paul Durrant; xen-devel@lists.xensource.com; linux- >> kernel@vger.kernel.org; konrad.wilk@oracle.com; jeremy@goop.org; >> kurt.hackel@oracle.com >> Subject: Re: [PATCH V2 1/2] xen/granttable: Support sub-page grants >> >> On Fri, 2011-12-09 at 07:45 +0000, annie li wrote: >> >>> On 2011-12-9 13:19, annie li wrote: >>> >>>> Hi Paul, >>>> >>>> >>>>>>> #define get_free_entry() get_free_entries(1) >>>>>>> >>>> Is this necessary? Maybe you defined this to keep consistence >>>> >> with >> >>>> put_free_entry(ref)? >>>> But other functions such as gnttab_grant_foreign_transfer and >>>> gnttab_grant_foreign_access all call get_free_entries(1). Maybe >>>> >> it >> >>>> is better to keep initial get_free_entries(1) code? >>>> >>> Another approach is doing those work in a separate patch -- >>> >> changing >> >>> get_free_entries to get_free_entry in following 4 functions: >>> >> I think you shouldn''t get too bogged down in get_free_entry() vs >> get_free_entries(1) for the purposes of this patch series. >> >> > > Annie, > > Yes, don''t worry about get_free_entry(). I only defined it for the sake of symmetry in the code I posted. >OK, will keep get_free_entries(1) unchanged. Thanks Annie> Paul >