Mailing List Archive: [PATCH 1/2] xen/granttable: Support sub-page grants

[PATCH 1/2] xen/granttable: Support sub-page grants

Dec 6, 2011, 2:56 AM

Post #1 of 14 (315 views)

From: Annie Li <annie.li@oracle.com>

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

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index bd325fd..7a0f4d1 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -261,6 +261,47 @@ int gnttab_grant_foreign_access(domid_t domid, unsigned long frame,
}
EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);

+int gnttab_grant_foreign_access_subpage_v2(domid_t domid, unsigned long frame,
+ int flags, unsigned page_off,
+ unsigned length)
+{
+ int ref;
+
+ ref = get_free_entries(1);
+ if (unlikely(ref < 0))
+ return -ENOSPC;
+
+ gnttab_grant_foreign_access_ref_subpage_v2(ref, domid, frame, flags,
+ page_off, length);
+
+ return ref;
+}
+EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage_v2);
+
+void gnttab_grant_foreign_access_ref_subpage_v2(grant_ref_t ref, domid_t domid,
+ unsigned long frame, int flags,
+ unsigned page_off,
+ unsigned length)
+{
+ BUG_ON(flags & (GTF_accept_transfer | GTF_reading |
+ GTF_writing | GTF_transitive));
+ BUG_ON(grant_table_version == 1);
+ 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;
+}
+EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_ref_subpage_v2);
+
+bool gnttab_subpage_trans_grants_available(void)
+{
+ return grant_table_version == 2;
+}
+EXPORT_SYMBOL_GPL(gnttab_subpage_trans_grants_available);
+
static int gnttab_query_foreign_access_v1(grant_ref_t ref)
{
return gnttab_shared.v1[ref].flags & (GTF_reading|GTF_writing);
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index fea4954..7e43652 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_v2(domid_t domid, unsigned long frame,
+ int flags, unsigned page_off,
+ unsigned length);
+
+/*
+ * Are sub-page or transitive grants available on this version of Xen? Returns
+ * true if they are, and false if they're not.
+ */
+bool gnttab_subpage_trans_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);
+void gnttab_grant_foreign_access_ref_subpage_v2(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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

Ian.Campbell at citrix

Dec 6, 2011, 3:42 AM

Post #2 of 14 (310 views)

Permalink

On Tue, 2011-12-06 at 10:56 +0000, annie.li@oracle.com wrote:
> From: Annie Li <annie.li@oracle.com>
>
> -- 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).
>

Almost all my comments will apply to the transitive grants patch too.

> Signed-off-by: Annie Li <annie.li@oracle.com>
> ---
> drivers/xen/grant-table.c | 41 +++++++++++++++++++++++++++++++++++++++++
> include/xen/grant_table.h | 13 +++++++++++++
> 2 files changed, 54 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index bd325fd..7a0f4d1 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -261,6 +261,47 @@ int gnttab_grant_foreign_access(domid_t domid, unsigned long frame,
> }
> EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
>
> +int gnttab_grant_foreign_access_subpage_v2(domid_t domid, unsigned long frame,
> + int flags, unsigned page_off,
> + unsigned length)

Please drop the v2 suffixes on the public functions.

Any reason not to route these via the ops table for consistency with all
the other ops? Then your availability check becomes a test for NULL fn
pointer rather than a specific version.

> +{
> + int ref;
> +
> + ref = get_free_entries(1);
> + if (unlikely(ref < 0))
> + return -ENOSPC;
> +
> + gnttab_grant_foreign_access_ref_subpage_v2(ref, domid, frame, flags,
> + page_off, length);
> +
> + return ref;
> +}
> +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage_v2);
> +
> +void gnttab_grant_foreign_access_ref_subpage_v2(grant_ref_t ref, domid_t domid,
> + unsigned long frame, int flags,
> + unsigned page_off,
> + unsigned length)
> +{
> + BUG_ON(flags & (GTF_accept_transfer | GTF_reading |
> + GTF_writing | GTF_transitive));
> + BUG_ON(grant_table_version == 1);

Returning -Esomething might be less drastic? ENOSYS perhaps?

> + 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;
> +}
> +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_ref_subpage_v2);
> +
> +bool gnttab_subpage_trans_grants_available(void)
> +{
> + return grant_table_version == 2;
> +}

It's not clear this adds anything over and above letting the user query
the grant table version. It's hard to tell since there are no users
presented here though. Perhaps separate subpage and transitive functions
would be cleaner?

> +EXPORT_SYMBOL_GPL(gnttab_subpage_trans_grants_available);
> +
> static int gnttab_query_foreign_access_v1(grant_ref_t ref)
> {
> return gnttab_shared.v1[ref].flags & (GTF_reading|GTF_writing);
> diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
> index fea4954..7e43652 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_v2(domid_t domid, unsigned long frame,
> + int flags, unsigned page_off,
> + unsigned length);

> +
> +/*
> + * Are sub-page or transitive grants available on this version of Xen? Returns
> + * true if they are, and false if they're not.
> + */
> +bool gnttab_subpage_trans_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);
> +void gnttab_grant_foreign_access_ref_subpage_v2(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);

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

jeremy at goop

Dec 6, 2011, 9:20 AM

Post #3 of 14 (308 views)

Permalink

On 12/06/2011 03:42 AM, Ian Campbell wrote:
>> +{
>> + int ref;
>> +
>> + ref = get_free_entries(1);
>> + if (unlikely(ref < 0))
>> + return -ENOSPC;
>> +
>> + gnttab_grant_foreign_access_ref_subpage_v2(ref, domid, frame, flags,
>> + page_off, length);
>> +
>> + return ref;
>> +}
>> +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage_v2);
>> +
>> +void gnttab_grant_foreign_access_ref_subpage_v2(grant_ref_t ref, domid_t domid,
>> + unsigned long frame, int flags,
>> + unsigned page_off,
>> + unsigned length)
>> +{
>> + BUG_ON(flags & (GTF_accept_transfer | GTF_reading |
>> + GTF_writing | GTF_transitive));
>> + BUG_ON(grant_table_version == 1);
> Returning -Esomething might be less drastic? ENOSYS perhaps?

Yeah, BUG_ON shouldn't be used for API misuse unless there's absolutely
no other way to handle it.

>
>> + 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;
>> +}
>> +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_ref_subpage_v2);
>> +
>> +bool gnttab_subpage_trans_grants_available(void)
>> +{
>> + return grant_table_version == 2;
>> +}
> It's not clear this adds anything over and above letting the user query
> the grant table version. It's hard to tell since there are no users
> presented here though. Perhaps separate subpage and transitive functions
> would be cleaner?

Well, in general, specifically testing for features rather than
interface versions is better.

J

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

annie.li at oracle

Dec 6, 2011, 7:36 PM

Post #4 of 14 (316 views)

Permalink

Thanks for your reviewing, Ian.
>> EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
>>
>> +int gnttab_grant_foreign_access_subpage_v2(domid_t domid, unsigned long frame,
>> + int flags, unsigned page_off,
>> + unsigned length)
> Please drop the v2 suffixes on the public functions.
OK, the initial interface is without v2 suffixes. It was added in order
to reminder user the interfaces are only available for grant table v2.
But I am fine to remove it, and following ops fn pointers are better.
> Any reason not to route these via the ops table for consistency with all
> the other ops? Then your availability check becomes a test for NULL fn
> pointer rather than a specific version.
Ok, it is good.
How about following implements?

gnttab_v1_ops = {
...
.access_subpage = NULL;
.access_ref_subpage = NULL;
.access_trans = NULL;
.access_ref_trans = NULL;
}

gnttab_v2_ops = {
...
.access_subpage = access_subpage_v2;
.access_ref_subpage = access_ref_subpage_v2;
.access_trans = access_trans_v2;
.access_ref_trans = access_ref_trans_v2;
}

gnttab_request_version()
{
.....
if(v2)
gnttab_interface = &gnttab_v2_ops;
else
gnttab_interface = &gnttab_v1_ops;
.....
}

int gnttab_grant_foreign_access_subpage()
{
if(gnttab_interface->access_subpage != NULL)
return gnttab_interface->access_subpage;
return Esomething;
}

Same operations for access_ref_subpage, access_trans and access_ref_trans.

bool gnttab_subpage_available()
{
return (gnttab_interface->access_subpage != NULL);
}

bool gnttab_subpage_available()
{
return (gnttab_interface->access_trans != NULL);
}

Thanks
Annie

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

annie.li at oracle

Dec 6, 2011, 7:36 PM

Post #5 of 14 (313 views)

Permalink

>> Returning -Esomething might be less drastic? ENOSYS perhaps?
> Yeah, BUG_ON shouldn't be used for API misuse unless there's absolutely
> no other way to handle it.
OK, will change it, thanks.
>>> + 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;
>>> +}
>>> +EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_ref_subpage_v2);
>>> +
>>> +bool gnttab_subpage_trans_grants_available(void)
>>> +{
>>> + return grant_table_version == 2;
>>> +}
>> It's not clear this adds anything over and above letting the user query
>> the grant table version. It's hard to tell since there are no users
>> presented here though. Perhaps separate subpage and transitive functions
>> would be cleaner?
> Well, in general, specifically testing for features rather than
> interface versions is better.
Ok, will split it into 2 functions and check the function pointer
instead of grant table version, thanks.

Thanks
Annie

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

Paul.Durrant at citrix

Dec 7, 2011, 12:59 AM

Post #6 of 14 (315 views)

Permalink

> -----Original Message-----
> From: ANNIE LI [mailto:annie.li@oracle.com]
> Sent: 07 December 2011 03:36
> To: Ian Campbell
> Cc: xen-devel@lists.xensource.com; linux-kernel@vger.kernel.org;
> konrad.wilk@oracle.com; jeremy@goop.org; kurt.hackel@oracle.com;
> Paul Durrant
> Subject: Re: [PATCH 1/2] xen/granttable: Support sub-page grants
>
> Thanks for your reviewing, Ian.
> >> EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
> >>
> >> +int gnttab_grant_foreign_access_subpage_v2(domid_t domid,
> unsigned long frame,
> >> + int flags, unsigned page_off,
> >> + unsigned length)
> > Please drop the v2 suffixes on the public functions.
> OK, the initial interface is without v2 suffixes. It was added in
> order to reminder user the interfaces are only available for grant
> table v2.
> But I am fine to remove it, and following ops fn pointers are
> better.
> > Any reason not to route these via the ops table for consistency
> with
> > all the other ops? Then your availability check becomes a test for
> > NULL fn pointer rather than a specific version.
> Ok, it is good.
> How about following implements?
>
> gnttab_v1_ops = {
> ...
> .access_subpage = NULL;
> .access_ref_subpage = NULL;
> .access_trans = NULL;
> .access_ref_trans = NULL;
> }
>
> gnttab_v2_ops = {
> ...
> .access_subpage = access_subpage_v2;
> .access_ref_subpage = access_ref_subpage_v2; .access_trans =
> access_trans_v2; .access_ref_trans = access_ref_trans_v2; }
>

Do you need ops for the ref and non-ref functions? I would have thought you could just have the ref ones since the all the non-ref variants do is allocate and then call the ref variant.

Paul

> gnttab_request_version()
> {
> .....
> if(v2)
> gnttab_interface = &gnttab_v2_ops;
> else
> gnttab_interface = &gnttab_v1_ops; .....
> }
>
> int gnttab_grant_foreign_access_subpage()
> {
> if(gnttab_interface->access_subpage != NULL)
> return gnttab_interface->access_subpage;
> return Esomething;
> }
>
> Same operations for access_ref_subpage, access_trans and
> access_ref_trans.
>
> bool gnttab_subpage_available()
> {
> return (gnttab_interface->access_subpage != NULL); }
>
> bool gnttab_subpage_available()
> {
> return (gnttab_interface->access_trans != NULL); }
>
> Thanks
> Annie
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

Ian.Campbell at citrix

Dec 7, 2011, 1:56 AM

Post #7 of 14 (311 views)

Permalink

On Wed, 2011-12-07 at 03:36 +0000, ANNIE LI wrote:
> Thanks for your reviewing, Ian.
> >> EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
> >>
> >> +int gnttab_grant_foreign_access_subpage_v2(domid_t domid, unsigned long frame,
> >> + int flags, unsigned page_off,
> >> + unsigned length)
> > Please drop the v2 suffixes on the public functions.
> OK, the initial interface is without v2 suffixes. It was added in order
> to reminder user the interfaces are only available for grant table v2.
> But I am fine to remove it, and following ops fn pointers are better.
> > Any reason not to route these via the ops table for consistency with all
> > the other ops? Then your availability check becomes a test for NULL fn
> > pointer rather than a specific version.
> Ok, it is good.
> How about following implements?

Looks to be along the right lines. Thanks.

> gnttab_v1_ops = {
> ...
> .access_subpage = NULL;
> .access_ref_subpage = NULL;
> .access_trans = NULL;
> .access_ref_trans = NULL;
> }

I think you can omit these since NULL is the default but perhaps
explicitly listing them is useful in a self documenting type way.

[...]
>
> Same operations for access_ref_subpage, access_trans and access_ref_trans.
>
> bool gnttab_subpage_available()
> {
> return (gnttab_interface->access_subpage != NULL);
> }
>
> bool gnttab_subpage_available()
Typo: ..trans..
> {
> return (gnttab_interface->access_trans != NULL);
> }

Ian.

>
> Thanks
> Annie

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

Ian.Campbell at citrix

Dec 7, 2011, 1:57 AM

Post #8 of 14 (305 views)

Permalink

On Wed, 2011-12-07 at 08:59 +0000, Paul Durrant wrote:
> > gnttab_v2_ops = {
> > ...
> > .access_subpage = access_subpage_v2;
> > .access_ref_subpage = access_ref_subpage_v2; .access_trans =
> > access_trans_v2; .access_ref_trans = access_ref_trans_v2; }
> >
>
>
> Do you need ops for the ref and non-ref functions? I would have
> thought you could just have the ref ones since the all the non-ref
> variants do is allocate and then call the ref variant.

Good point. It appears that all the existing ops are only present in the
ref form.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

annie.li at oracle

Dec 7, 2011, 2:27 AM

Post #9 of 14 (310 views)

Permalink

On 2011-12-7 17:56, Ian Campbell wrote:
> On Wed, 2011-12-07 at 03:36 +0000, ANNIE LI wrote:
>> Thanks for your reviewing, Ian.
>>>> EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
>>>>
>>>> +int gnttab_grant_foreign_access_subpage_v2(domid_t domid, unsigned long frame,
>>>> + int flags, unsigned page_off,
>>>> + unsigned length)
>>> Please drop the v2 suffixes on the public functions.
>> OK, the initial interface is without v2 suffixes. It was added in order
>> to reminder user the interfaces are only available for grant table v2.
>> But I am fine to remove it, and following ops fn pointers are better.
>>> Any reason not to route these via the ops table for consistency with all
>>> the other ops? Then your availability check becomes a test for NULL fn
>>> pointer rather than a specific version.
>> Ok, it is good.
>> How about following implements?
> Looks to be along the right lines. Thanks.
>
>> gnttab_v1_ops = {
>> ...
>> .access_subpage = NULL;
>> .access_ref_subpage = NULL;
>> .access_trans = NULL;
>> .access_ref_trans = NULL;
>> }
> I think you can omit these since NULL is the default but perhaps
> explicitly listing them is useful in a self documenting type way.
>
> [...]
OK, I can delete those.
>> Same operations for access_ref_subpage, access_trans and access_ref_trans.
>>
>> bool gnttab_subpage_available()
>> {
>> return (gnttab_interface->access_subpage != NULL);
>> }
>>
>> bool gnttab_subpage_available()
> Typo: ..trans..
Thanks for pointing out this.

Thanks
Annie
>> {
>> return (gnttab_interface->access_trans != NULL);
>> }
> Ian.
>
>> Thanks
>> Annie
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

annie.li at oracle

Dec 7, 2011, 2:27 AM

Post #10 of 14 (309 views)

Permalink

On 2011-12-7 17:57, Ian Campbell wrote:
> On Wed, 2011-12-07 at 08:59 +0000, Paul Durrant wrote:
>>> gnttab_v2_ops = {
>>> ...
>>> .access_subpage = access_subpage_v2;
>>> .access_ref_subpage = access_ref_subpage_v2; .access_trans =
>>> access_trans_v2; .access_ref_trans = access_ref_trans_v2; }
>>>
>>
>> Do you need ops for the ref and non-ref functions? I would have
>> thought you could just have the ref ones since the all the non-ref
>> variants do is allocate and then call the ref variant.
> Good point. It appears that all the existing ops are only present in the
> ref form.
So I will add two elements: access_ref_subpage and access_ref_trans into
gnttab_v2_ops.

Thanks
Annie
> Ian.
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

Ian.Campbell at citrix

Dec 7, 2011, 2:33 AM

Post #11 of 14 (308 views)

Permalink

On Wed, 2011-12-07 at 10:27 +0000, ANNIE LI wrote:
>
> On 2011-12-7 17:57, Ian Campbell wrote:
> > On Wed, 2011-12-07 at 08:59 +0000, Paul Durrant wrote:
> >>> gnttab_v2_ops = {
> >>> ...
> >>> .access_subpage = access_subpage_v2;
> >>> .access_ref_subpage = access_ref_subpage_v2; .access_trans =
> >>> access_trans_v2; .access_ref_trans = access_ref_trans_v2; }
> >>>
> >>
> >> Do you need ops for the ref and non-ref functions? I would have
> >> thought you could just have the ref ones since the all the non-ref
> >> variants do is allocate and then call the ref variant.
> > Good point. It appears that all the existing ops are only present in the
> > ref form.
> So I will add two elements: access_ref_subpage and access_ref_trans into
> gnttab_v2_ops.

The existing convention seems to be for _ref to be a suffix, although
it's only actually used on the end_*_ref ones.
+ int (*end_foreign_access_ref)(grant_ref_t, int);
+ unsigned long (*end_foreign_transfer_ref)(grant_ref_t);

Also it occurs to me that access_* sounds like something which uses a
ref rather than something which sets one up. The existing hook to setup
a normal grant is called "update_entry". Perhaps
update_{subpage,transitive}_entry?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

annie.li at oracle

Dec 7, 2011, 2:36 AM

Post #12 of 14 (309 views)

Permalink

> +void gnttab_grant_foreign_access_ref_subpage_v2(grant_ref_t ref, domid_t domid,
> + unsigned long frame, int flags,
> + unsigned page_off,
> + unsigned length)
> +{
> + BUG_ON(flags& (GTF_accept_transfer | GTF_reading |
> + GTF_writing | GTF_transitive));
This is slightly changed from the initial code, welcome your suggestions.

Initial condition is:
BUG_ON(flags & (GTF_accept_transfer | GTF_reading | GTF_writing |
GTF_sub_page | GTF_permit_access));

GTF_sub_page | GTF_permit_access will be set later in this function, so
it is necessary to verify this flag here.
GTF_transitive and GTF_sub_page can not be enabled at same time, so
GTF_transitive is checked here to avoid those two flags are both enabled.

Same code was changed in corresponding transitive function.

Thanks
Annie
> + BUG_ON(grant_table_version == 1);
> + 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;
> +}
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

annie.li at oracle

Dec 7, 2011, 2:50 AM

Post #13 of 14 (310 views)

Permalink

> The existing convention seems to be for _ref to be a suffix, although
> it's only actually used on the end_*_ref ones.
> + int (*end_foreign_access_ref)(grant_ref_t, int);
> + unsigned long (*end_foreign_transfer_ref)(grant_ref_t);
>
> Also it occurs to me that access_* sounds like something which uses a
> ref rather than something which sets one up. The existing hook to setup
> a normal grant is called "update_entry". Perhaps
> update_{subpage,transitive}_entry?
Yes, you are right.
Just like the existing code:
gnttab_grant_foreign_access VS gnttab_grant_foreign_access_subpage
update_entry in gnttab_grant_foreign_access_ref VS
update_{subpage,transitive}_entry in
gnttab_grant_foreign_access_{subpage,trans}_ref

Thanks
Annie

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/2] xen/granttable: Support sub-page grants [ In reply to ]

konrad at darnok

Dec 7, 2011, 11:41 AM

Post #14 of 14 (309 views)

Permalink

> >
> > gnttab_v1_ops = {
> > ...
> > .access_subpage = NULL;
> > .access_ref_subpage = NULL;
> > .access_trans = NULL;
> > .access_ref_trans = NULL;
> > }

You don't really need that. If you just leave it empty, they are all set
to NULL.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel