Re: [Xen-devel] [PATCH v11 10/11] common: add a new mappable resource type: XENMEM_resource_grant_table

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:jbeulich@xxxxxxxx]
> Sent: 17 October 2017 07:43
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian
> Jackson <Ian.Jackson@xxxxxxxxxx>; sstabellini@xxxxxxxxxx; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; konrad.wilk@xxxxxxxxxx; Tim (Xen.org)
> <tim@xxxxxxx>
> Subject: Re: [PATCH v11 10/11] common: add a new mappable resource
> type: XENMEM_resource_grant_table
> 
> >>> Paul Durrant <paul.durrant@xxxxxxxxxx> 10/12/17 6:28 PM >>>
> >@@ -1608,7 +1608,8 @@ fault:
> >}
> >
> >static int
> >-gnttab_populate_status_frames(struct domain *d, struct grant_table *gt,
> >+gnttab_populate_status_frames(struct domain *d,
> >+                              struct grant_table *gt,
> >unsigned int req_nr_frames)
> 
> What is this change about?
> 

It must have crept in accidentally. I'll get rid of it.

> >+int gnttab_get_grant_frame(struct domain *d, unsigned long idx,
> >+                           mfn_t *mfn)
> >+{
> >+    struct grant_table *gt = d->grant_table;
> >+    int rc;
> >+
> >+    /* write lock required as version may change and/or table may grow */
> >+    grant_write_lock(gt);
> >+
> >+    rc = (gt->gt_version == 2 &&
> >+          idx > XENMAPIDX_grant_table_status) ?
> 
> I don't understand this check - why does XENMAPIDX_grant_table_status
> matter here at all? Same in gnttab_get_status_frame() then.
> 

Well, the current legal range of grant table frames for v2 is 0 - (1 << 
XENMAPIDX_grant_table_status) whereas it appears that for v1 there is no limit. 
As for status frames, they are a v2-only concept but I agree that the range 
check there is wrong.

  Paul

> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel