* [PATCH v2 3/3] gnttab: clean up gnttab_set_version()
2015-06-22 11:39 [PATCH v2 0/3] gnttab: misc adjustments Jan Beulich
2015-06-22 11:44 ` [PATCH v2 1/3 resend] gnttab: fix out of range shift count Jan Beulich
2015-06-22 11:45 ` [PATCH v2 2/3] gnttab: don't silently truncate frame numbers in gnttab_set_version() Jan Beulich
@ 2015-06-22 11:46 ` Jan Beulich
2015-06-22 12:01 ` Andrew Cooper
2015-07-06 15:59 ` [PATCH v2 0/3] gnttab: misc adjustments Ian Campbell
3 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2015-06-22 11:46 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell, Keir Fraser, Ian Jackson, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 7409 bytes --]
- drop pointless nr_grant_entries() check from loop over reserved
entries (adding suitable BUILD_BUG_ON()s to validate that)
- adjust types
- rename d to currd
- formatting
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -465,10 +465,16 @@ static unsigned int nr_grant_entries(str
{
switch ( gt->gt_version )
{
+#define f2e(nr, ver) (((nr) << PAGE_SHIFT) / sizeof(grant_entry_v##ver##_t))
case 1:
- return (nr_grant_frames(gt) << PAGE_SHIFT) / sizeof(grant_entry_v1_t);
+ BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 1) <
+ GNTTAB_NR_RESERVED_ENTRIES);
+ return f2e(nr_grant_frames(gt), 1);
case 2:
- return (nr_grant_frames(gt) << PAGE_SHIFT) / sizeof(grant_entry_v2_t);
+ BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 2) <
+ GNTTAB_NR_RESERVED_ENTRIES);
+ return f2e(nr_grant_frames(gt), 2);
+#undef f2e
}
return 0;
@@ -2563,17 +2569,17 @@ static long
gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
{
gnttab_set_version_t op;
- struct domain *d = current->domain;
- struct grant_table *gt = d->grant_table;
+ struct domain *currd = current->domain;
+ struct grant_table *gt = currd->grant_table;
grant_entry_v1_t reserved_entries[GNTTAB_NR_RESERVED_ENTRIES];
- long res;
- int i;
+ int res;
+ unsigned int i;
- if (copy_from_guest(&op, uop, 1))
+ if ( copy_from_guest(&op, uop, 1) )
return -EFAULT;
res = -EINVAL;
- if (op.version != 1 && op.version != 2)
+ if ( op.version != 1 && op.version != 2 )
goto out;
res = 0;
@@ -2581,10 +2587,12 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
goto out;
write_lock(>->lock);
- /* Make sure that the grant table isn't currently in use when we
- change the version number, except for the first 8 entries which
- are allowed to be in use (xenstore/xenconsole keeps them mapped).
- (You need to change the version number for e.g. kexec.) */
+ /*
+ * Make sure that the grant table isn't currently in use when we
+ * change the version number, except for the first 8 entries which
+ * are allowed to be in use (xenstore/xenconsole keeps them mapped).
+ * (You need to change the version number for e.g. kexec.)
+ */
for ( i = GNTTAB_NR_RESERVED_ENTRIES; i < nr_grant_entries(gt); i++ )
{
if ( read_atomic(&_active_entry(gt, i).pin) != 0 )
@@ -2604,7 +2612,7 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
{
case 1:
/* XXX: We could maybe shrink the active grant table here. */
- res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
+ res = gnttab_populate_status_frames(currd, gt, nr_grant_frames(gt));
if ( res < 0)
goto out_unlock;
}
@@ -2625,66 +2633,78 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
break;
}
- /* Preserve the first 8 entries (toolstack reserved grants) */
- if ( gt->gt_version == 1 )
- {
- memcpy(reserved_entries, &shared_entry_v1(gt, 0), sizeof(reserved_entries));
- }
- else if ( gt->gt_version == 2 )
+ /* Preserve the first 8 entries (toolstack reserved grants). */
+ switch ( gt->gt_version )
{
- for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES && i < nr_grant_entries(gt); i++ )
+ case 1:
+ memcpy(reserved_entries, &shared_entry_v1(gt, 0),
+ sizeof(reserved_entries));
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
{
- int flags = status_entry(gt, i);
- flags |= shared_entry_v2(gt, i).hdr.flags;
- if ((flags & GTF_type_mask) == GTF_permit_access)
+ unsigned int flags = shared_entry_v2(gt, i).hdr.flags;
+
+ switch ( flags & GTF_type_mask )
{
- reserved_entries[i].flags = flags;
+ case GTF_permit_access:
+ reserved_entries[i].flags = flags | status_entry(gt, i);
reserved_entries[i].domid = shared_entry_v2(gt, i).hdr.domid;
reserved_entries[i].frame = shared_entry_v2(gt, i).full_page.frame;
- }
- else
- {
- if ((flags & GTF_type_mask) != GTF_invalid)
- gdprintk(XENLOG_INFO, "d%d: bad flags %x in grant %d when switching grant version\n",
- d->domain_id, flags, i);
+ break;
+ default:
+ gdprintk(XENLOG_INFO,
+ "bad flags %x in grant %u when switching version\n",
+ flags, i);
+ /* fall through */
+ case GTF_invalid:
memset(&reserved_entries[i], 0, sizeof(reserved_entries[i]));
+ break;
}
}
+ break;
}
if ( op.version < 2 && gt->gt_version == 2 )
- gnttab_unpopulate_status_frames(d, gt);
+ gnttab_unpopulate_status_frames(currd, gt);
- /* Make sure there's no crud left over in the table from the
- old version. */
+ /* Make sure there's no crud left over from the old version. */
for ( i = 0; i < nr_grant_frames(gt); i++ )
clear_page(gt->shared_raw[i]);
- /* Restore the first 8 entries (toolstack reserved grants) */
- if ( gt->gt_version != 0 && op.version == 1 )
- {
- memcpy(&shared_entry_v1(gt, 0), reserved_entries, sizeof(reserved_entries));
- }
- else if ( gt->gt_version != 0 && op.version == 2 )
+ /* Restore the first 8 entries (toolstack reserved grants). */
+ if ( gt->gt_version )
{
- for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ switch ( op.version )
{
- status_entry(gt, i) = reserved_entries[i].flags & (GTF_reading|GTF_writing);
- shared_entry_v2(gt, i).hdr.flags = reserved_entries[i].flags & ~(GTF_reading|GTF_writing);
- shared_entry_v2(gt, i).hdr.domid = reserved_entries[i].domid;
- shared_entry_v2(gt, i).full_page.frame = reserved_entries[i].frame;
+ case 1:
+ memcpy(&shared_entry_v1(gt, 0), reserved_entries, sizeof(reserved_entries));
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ {
+ status_entry(gt, i) =
+ reserved_entries[i].flags & (GTF_reading | GTF_writing);
+ shared_entry_v2(gt, i).hdr.flags =
+ reserved_entries[i].flags & ~(GTF_reading | GTF_writing);
+ shared_entry_v2(gt, i).hdr.domid =
+ reserved_entries[i].domid;
+ shared_entry_v2(gt, i).full_page.frame =
+ reserved_entries[i].frame;
+ }
+ break;
}
}
gt->gt_version = op.version;
-out_unlock:
+ out_unlock:
write_unlock(>->lock);
-out:
+ out:
op.version = gt->gt_version;
- if (__copy_to_guest(uop, &op, 1))
+ if ( __copy_to_guest(uop, &op, 1) )
res = -EFAULT;
return res;
[-- Attachment #2: gnttab-set-version-cleanup.patch --]
[-- Type: text/plain, Size: 7446 bytes --]
gnttab: clean up gnttab_set_version()
- drop pointless nr_grant_entries() check from loop over reserved
entries (adding suitable BUILD_BUG_ON()s to validate that)
- adjust types
- rename d to currd
- formatting
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -465,10 +465,16 @@ static unsigned int nr_grant_entries(str
{
switch ( gt->gt_version )
{
+#define f2e(nr, ver) (((nr) << PAGE_SHIFT) / sizeof(grant_entry_v##ver##_t))
case 1:
- return (nr_grant_frames(gt) << PAGE_SHIFT) / sizeof(grant_entry_v1_t);
+ BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 1) <
+ GNTTAB_NR_RESERVED_ENTRIES);
+ return f2e(nr_grant_frames(gt), 1);
case 2:
- return (nr_grant_frames(gt) << PAGE_SHIFT) / sizeof(grant_entry_v2_t);
+ BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 2) <
+ GNTTAB_NR_RESERVED_ENTRIES);
+ return f2e(nr_grant_frames(gt), 2);
+#undef f2e
}
return 0;
@@ -2563,17 +2569,17 @@ static long
gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
{
gnttab_set_version_t op;
- struct domain *d = current->domain;
- struct grant_table *gt = d->grant_table;
+ struct domain *currd = current->domain;
+ struct grant_table *gt = currd->grant_table;
grant_entry_v1_t reserved_entries[GNTTAB_NR_RESERVED_ENTRIES];
- long res;
- int i;
+ int res;
+ unsigned int i;
- if (copy_from_guest(&op, uop, 1))
+ if ( copy_from_guest(&op, uop, 1) )
return -EFAULT;
res = -EINVAL;
- if (op.version != 1 && op.version != 2)
+ if ( op.version != 1 && op.version != 2 )
goto out;
res = 0;
@@ -2581,10 +2587,12 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
goto out;
write_lock(>->lock);
- /* Make sure that the grant table isn't currently in use when we
- change the version number, except for the first 8 entries which
- are allowed to be in use (xenstore/xenconsole keeps them mapped).
- (You need to change the version number for e.g. kexec.) */
+ /*
+ * Make sure that the grant table isn't currently in use when we
+ * change the version number, except for the first 8 entries which
+ * are allowed to be in use (xenstore/xenconsole keeps them mapped).
+ * (You need to change the version number for e.g. kexec.)
+ */
for ( i = GNTTAB_NR_RESERVED_ENTRIES; i < nr_grant_entries(gt); i++ )
{
if ( read_atomic(&_active_entry(gt, i).pin) != 0 )
@@ -2604,7 +2612,7 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
{
case 1:
/* XXX: We could maybe shrink the active grant table here. */
- res = gnttab_populate_status_frames(d, gt, nr_grant_frames(gt));
+ res = gnttab_populate_status_frames(currd, gt, nr_grant_frames(gt));
if ( res < 0)
goto out_unlock;
}
@@ -2625,66 +2633,78 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
break;
}
- /* Preserve the first 8 entries (toolstack reserved grants) */
- if ( gt->gt_version == 1 )
- {
- memcpy(reserved_entries, &shared_entry_v1(gt, 0), sizeof(reserved_entries));
- }
- else if ( gt->gt_version == 2 )
+ /* Preserve the first 8 entries (toolstack reserved grants). */
+ switch ( gt->gt_version )
{
- for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES && i < nr_grant_entries(gt); i++ )
+ case 1:
+ memcpy(reserved_entries, &shared_entry_v1(gt, 0),
+ sizeof(reserved_entries));
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
{
- int flags = status_entry(gt, i);
- flags |= shared_entry_v2(gt, i).hdr.flags;
- if ((flags & GTF_type_mask) == GTF_permit_access)
+ unsigned int flags = shared_entry_v2(gt, i).hdr.flags;
+
+ switch ( flags & GTF_type_mask )
{
- reserved_entries[i].flags = flags;
+ case GTF_permit_access:
+ reserved_entries[i].flags = flags | status_entry(gt, i);
reserved_entries[i].domid = shared_entry_v2(gt, i).hdr.domid;
reserved_entries[i].frame = shared_entry_v2(gt, i).full_page.frame;
- }
- else
- {
- if ((flags & GTF_type_mask) != GTF_invalid)
- gdprintk(XENLOG_INFO, "d%d: bad flags %x in grant %d when switching grant version\n",
- d->domain_id, flags, i);
+ break;
+ default:
+ gdprintk(XENLOG_INFO,
+ "bad flags %x in grant %u when switching version\n",
+ flags, i);
+ /* fall through */
+ case GTF_invalid:
memset(&reserved_entries[i], 0, sizeof(reserved_entries[i]));
+ break;
}
}
+ break;
}
if ( op.version < 2 && gt->gt_version == 2 )
- gnttab_unpopulate_status_frames(d, gt);
+ gnttab_unpopulate_status_frames(currd, gt);
- /* Make sure there's no crud left over in the table from the
- old version. */
+ /* Make sure there's no crud left over from the old version. */
for ( i = 0; i < nr_grant_frames(gt); i++ )
clear_page(gt->shared_raw[i]);
- /* Restore the first 8 entries (toolstack reserved grants) */
- if ( gt->gt_version != 0 && op.version == 1 )
- {
- memcpy(&shared_entry_v1(gt, 0), reserved_entries, sizeof(reserved_entries));
- }
- else if ( gt->gt_version != 0 && op.version == 2 )
+ /* Restore the first 8 entries (toolstack reserved grants). */
+ if ( gt->gt_version )
{
- for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ switch ( op.version )
{
- status_entry(gt, i) = reserved_entries[i].flags & (GTF_reading|GTF_writing);
- shared_entry_v2(gt, i).hdr.flags = reserved_entries[i].flags & ~(GTF_reading|GTF_writing);
- shared_entry_v2(gt, i).hdr.domid = reserved_entries[i].domid;
- shared_entry_v2(gt, i).full_page.frame = reserved_entries[i].frame;
+ case 1:
+ memcpy(&shared_entry_v1(gt, 0), reserved_entries, sizeof(reserved_entries));
+ break;
+ case 2:
+ for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
+ {
+ status_entry(gt, i) =
+ reserved_entries[i].flags & (GTF_reading | GTF_writing);
+ shared_entry_v2(gt, i).hdr.flags =
+ reserved_entries[i].flags & ~(GTF_reading | GTF_writing);
+ shared_entry_v2(gt, i).hdr.domid =
+ reserved_entries[i].domid;
+ shared_entry_v2(gt, i).full_page.frame =
+ reserved_entries[i].frame;
+ }
+ break;
}
}
gt->gt_version = op.version;
-out_unlock:
+ out_unlock:
write_unlock(>->lock);
-out:
+ out:
op.version = gt->gt_version;
- if (__copy_to_guest(uop, &op, 1))
+ if ( __copy_to_guest(uop, &op, 1) )
res = -EFAULT;
return res;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread