All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V3 0/2] xen: patches for supporting sub-page and transitive grants
@ 2011-12-09 11:32 annie.li
  2011-12-09 11:32 ` [PATCH V3 1/2] xen/granttable: Support sub-page grants annie.li
  2011-12-09 11:33 ` [PATCH V3 2/2] xen/granttable: Support transitive grants annie.li
  0 siblings, 2 replies; 8+ messages in thread
From: annie.li @ 2011-12-09 11:32 UTC (permalink / raw)
  To: xen-devel, linux-kernel, konrad.wilk, jeremy, paul.durrant,
	Ian.Campbell, annie.li
  Cc: kurt.hackel

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 |  141 +++++++++++++++++++++++++++++++++++++++++++++
 include/xen/grant_table.h |   25 ++++++++
 2 files changed, 166 insertions(+), 0 deletions(-)

Thanks
Annie
-- 
1.7.6.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V3 1/2] xen/granttable: Support sub-page grants
  2011-12-09 11:32 [PATCH V3 0/2] xen: patches for supporting sub-page and transitive grants annie.li
@ 2011-12-09 11:32 ` annie.li
  2011-12-09 17:38   ` Ian Campbell
  2011-12-09 11:33 ` [PATCH V3 2/2] xen/granttable: Support transitive grants annie.li
  1 sibling, 1 reply; 8+ messages in thread
From: annie.li @ 2011-12-09 11:32 UTC (permalink / raw)
  To: xen-devel, linux-kernel, konrad.wilk, jeremy, paul.durrant,
	Ian.Campbell, annie.li
  Cc: kurt.hackel

    -- 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 |   71 +++++++++++++++++++++++++++++++++++++++++++++
 include/xen/grant_table.h |   13 ++++++++
 2 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index bd325fd..0ac16fa 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,66 @@ 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, rc;
+
+	ref = get_free_entries(1);
+	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;
+	}
+
+	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 +883,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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH V3 2/2] xen/granttable: Support transitive grants
  2011-12-09 11:32 [PATCH V3 0/2] xen: patches for supporting sub-page and transitive grants annie.li
  2011-12-09 11:32 ` [PATCH V3 1/2] xen/granttable: Support sub-page grants annie.li
@ 2011-12-09 11:33 ` annie.li
  1 sibling, 0 replies; 8+ messages in thread
From: annie.li @ 2011-12-09 11:33 UTC (permalink / raw)
  To: xen-devel, linux-kernel, konrad.wilk, jeremy, paul.durrant,
	Ian.Campbell, annie.li
  Cc: kurt.hackel

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 |   70 +++++++++++++++++++++++++++++++++++++++++++++
 include/xen/grant_table.h |   12 ++++++++
 2 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 0ac16fa..7835ffc 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;
@@ -331,6 +343,63 @@ 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, rc;
+
+	ref = get_free_entries(1);
+	if (unlikely(ref < 0))
+		return -ENOSPC;
+
+	rc = gnttab_grant_foreign_access_trans_ref(ref, domid, flags,
+						   trans_domid, trans_gref);
+	if (rc < 0) {
+		put_free_entry(ref);
+		return rc;
+	}
+
+	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);
@@ -884,6 +953,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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V3 1/2] xen/granttable: Support sub-page grants
  2011-12-09 11:32 ` [PATCH V3 1/2] xen/granttable: Support sub-page grants annie.li
@ 2011-12-09 17:38   ` Ian Campbell
  2011-12-11  6:05     ` ANNIE LI
  2011-12-12  3:16     ` ANNIE LI
  0 siblings, 2 replies; 8+ messages in thread
From: Ian Campbell @ 2011-12-09 17:38 UTC (permalink / raw)
  To: annie.li
  Cc: xen-devel, linux-kernel, konrad.wilk, jeremy, Paul Durrant, kurt.hackel

On Fri, 2011-12-09 at 11:32 +0000, annie.li@oracle.com wrote:
> -- 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 |   71 +++++++++++++++++++++++++++++++++++++++++++++
>  include/xen/grant_table.h |   13 ++++++++
>  2 files changed, 84 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index bd325fd..0ac16fa 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);

Please can you name the arguments here and then refer to them by name in
the comments instead of all this "First parameter", "second one" stuff.

Similarly for the existing comments sorry I didn't notice this in
previous review.

Ian.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3 1/2] xen/granttable: Support sub-page grants
  2011-12-09 17:38   ` Ian Campbell
@ 2011-12-11  6:05     ` ANNIE LI
  2011-12-12  3:16     ` ANNIE LI
  1 sibling, 0 replies; 8+ messages in thread
From: ANNIE LI @ 2011-12-11  6:05 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel, linux-kernel, konrad.wilk, jeremy, Paul Durrant, kurt.hackel



On 2011-12-10 1:38, Ian Campbell wrote:
> On Fri, 2011-12-09 at 11:32 +0000, annie.li@oracle.com wrote:
>> -- 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 |   71 +++++++++++++++++++++++++++++++++++++++++++++
>>   include/xen/grant_table.h |   13 ++++++++
>>   2 files changed, 84 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
>> index bd325fd..0ac16fa 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);
> Please can you name the arguments here and then refer to them by name in
> the comments instead of all this "First parameter", "second one" stuff.
>
> Similarly for the existing comments sorry I didn't notice this in
> previous review.
Ok, I will do this in another separate patch.

Thanks,
Annie.
> Ian.
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3 1/2] xen/granttable: Support sub-page grants
  2011-12-09 17:38   ` Ian Campbell
  2011-12-11  6:05     ` ANNIE LI
@ 2011-12-12  3:16     ` ANNIE LI
  2011-12-12  7:10       ` Ian Campbell
  1 sibling, 1 reply; 8+ messages in thread
From: ANNIE LI @ 2011-12-12  3:16 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel, linux-kernel, konrad.wilk, jeremy, Paul Durrant, kurt.hackel


> Please can you name the arguments here and then refer to them by name in
> the comments instead of all this "First parameter", "second one" stuff.
>
> Similarly for the existing comments sorry I didn't notice this in
> previous review.
Please check following comments in gnttab_ops, I changed the "First 
parameter", "second one" into parameter name. However, it looks not very 
consistent with parameters format of function fn since only parameter 
type exists, not name.

struct gnttab_ops {
         /*
          * Mapping a list of frames for storing grant entries. Frames 
parameter
          * is used to store grant table address when grant table being 
setup,
          * nr_gframes is the number of frames to map grant table. Returning
          * GNTST_okay means success and negative value means failure.
          */
         int (*map_frames)(unsigned long *, unsigned int);
         /*
          * Release a list of frames which are mapped in map_frames for 
grant
          * entry status.
          */
         void (*unmap_frames)(void);
         /*
          * Introducing a valid entry into the grant table, granting the 
frame
          * of this grant entry to domain for accessing, or transfering, or
          * transitively accessing. Ref parameter is reference of this 
introduced
          * grant entry, domid is id of granted domain, frame is the 
page frame
          * to be granted, and flags is status of the grant entry to be 
updated.
          */
         void (*update_entry)(grant_ref_t, domid_t, unsigned long, 
unsigned);
         /*
          * Stop granting a grant entry to domain for accessing. Ref 
parameter is
          * reference of a grant entry whose grant access will be stopped,
          * readonly is not in use in this function. If the grant entry is
          * currently mapped for reading or writing, just return 
failure(==0)
          * directly and don't tear down the grant access. Otherwise, 
stop grant
          * access for this entry and return success(==1).
          */
         int (*end_foreign_access_ref)(grant_ref_t, int);
         /*
          * Stop granting a grant entry to domain for transfer. If 
tranfer has
          * not started, just reclaim the grant entry and return 
failure(==0).
          * Otherwise, wait for the transfer to complete and then return the
          * frame.
          */
         unsigned long (*end_foreign_transfer_ref)(grant_ref_t);
         /*
          * Query the status of a grant entry. Ref parameter is reference of
          * queried grant entry, return value is the status of queried 
entry.
          * Detailed status(writing/reading) can be gotten from the 
return value
          * 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. Ref parameter is grant entry reference
          * number, domid is id of grantee domain, frame is frame address of
          * subpage grant, flags is grant type and flag information, 
page_off is
          * offset of the range of bytes, and length is length of bytes 
to be
          * accessed.
          */
         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. Ref parameter is an available 
grant entry
          * reference on domain A, domid is id of domain C which 
accesses grant
          * entry transitively, flags is grant type and flag information,
          * trans_domid is id of domain B whose grant entry is finally 
accessed
          * transitively, trans_gref is grant entry transitive reference of
          * domain B.
          */
         void (*update_trans_entry)(grant_ref_t, domid_t, int, domid_t,
                                    grant_ref_t);
Thanks
Annie
> Ian.
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3 1/2] xen/granttable: Support sub-page grants
  2011-12-12  3:16     ` ANNIE LI
@ 2011-12-12  7:10       ` Ian Campbell
  2011-12-12  7:32         ` ANNIE LI
  0 siblings, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2011-12-12  7:10 UTC (permalink / raw)
  To: ANNIE LI
  Cc: xen-devel, linux-kernel, konrad.wilk, jeremy, Paul Durrant, kurt.hackel

On Mon, 2011-12-12 at 03:16 +0000, ANNIE LI wrote:
> > Please can you name the arguments here and then refer to them by name in
> > the comments instead of all this "First parameter", "second one" stuff.
> >
> > Similarly for the existing comments sorry I didn't notice this in
> > previous review.
> Please check following comments in gnttab_ops, I changed the "First 
> parameter", "second one" into parameter name. However, it looks not very 
> consistent with parameters format of function fn since only parameter 
> type exists, not name.

You can give the parameters names in the function pointers too, that's
what I was suggesting.

e.g.:
         int (*map_frames)(unsigned long *frames, unsigned int nr);

Ian.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3 1/2] xen/granttable: Support sub-page grants
  2011-12-12  7:10       ` Ian Campbell
@ 2011-12-12  7:32         ` ANNIE LI
  0 siblings, 0 replies; 8+ messages in thread
From: ANNIE LI @ 2011-12-12  7:32 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel, linux-kernel, konrad.wilk, jeremy, Paul Durrant, kurt.hackel



On 2011-12-12 15:10, Ian Campbell wrote:
> On Mon, 2011-12-12 at 03:16 +0000, ANNIE LI wrote:
>>> Please can you name the arguments here and then refer to them by name in
>>> the comments instead of all this "First parameter", "second one" stuff.
>>>
>>> Similarly for the existing comments sorry I didn't notice this in
>>> previous review.
>> Please check following comments in gnttab_ops, I changed the "First
>> parameter", "second one" into parameter name. However, it looks not very
>> consistent with parameters format of function fn since only parameter
>> type exists, not name.
> You can give the parameters names in the function pointers too, that's
> what I was suggesting.
>
> e.g.:
>           int (*map_frames)(unsigned long *frames, unsigned int nr);
>
Ok, I will correct comments and add parameter names of existing function 
in a third patch.
Sub-page and trans patches should contain corrected comments and 
parameter names directly.

Thanks
Annie
> Ian.
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-12-12  7:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-09 11:32 [PATCH V3 0/2] xen: patches for supporting sub-page and transitive grants annie.li
2011-12-09 11:32 ` [PATCH V3 1/2] xen/granttable: Support sub-page grants annie.li
2011-12-09 17:38   ` Ian Campbell
2011-12-11  6:05     ` ANNIE LI
2011-12-12  3:16     ` ANNIE LI
2011-12-12  7:10       ` Ian Campbell
2011-12-12  7:32         ` ANNIE LI
2011-12-09 11:33 ` [PATCH V3 2/2] xen/granttable: Support transitive grants annie.li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.