On Thu, 2009-12-03 at 12:01 +0000, Steven Smith wrote:
> > Oh, I see what you meant... in the proper resume case (as opposed to the
> > cancelled suspend/checkpoint case I was thinking of) there should be no
> > grant tables in use at this point so most devices should, in theory, be
> > able to reconnect using v1 grants, any drivers which require v2 grant
> > tables need to check for them in their resume hook as well as at start
> > of day.
> >
> > Unfortunately frontend devices tear down their grant entries after the
> > resume rather than before the suspend (I presume this has to do with
> > faster checkpointing?) which means they could be trying to clear an
> > entry of the wrong layout, leading the unbounded badness that the
> > comment refers to.
> Actually, I think that'd be okay. Drivers should be clearing grant
> table entries through the gnttab_end_* functions, which always check
> grant_table_version and do the right thing.
In the case where you have gone v1->v2 then the entries would have been
setup while grante_table_version==1 layout but by the time gnttab_end_*
is called grant_table_version==2 and the wrong thing happens.
>
> > I think the choices are basically:
> > * Always latch to either v1 or v2 at start of day, if we can't get
> > the version we want then panic (this is a stronger restriction
> > than the current code which will try to upgrade to v2 on resume)
> Yeah, that probably counts as a bug in the current code. If we boot
> on a Xen which only supports v1 tabled then we should probably stick
> with v1 until we shut down.
>
> panic()ing when v2 isn't available sounds like overkill, though.
It's only if you've already used v2 in this guest.
> Would something like this work?
I don't think so, because I don't think changing the grant table layout
is safe.
I'm experimenting with this:
Subject: xen: latch grant-table layout at start of day.
It is not possible to switch grant table layout over a save restore
since entries setup before with the old layout cannot be torn down under
the new layout.
Also add a grant_table.version parameter to allow the user to force a
particular layout (e.g. if they know they need to migrate to v1 only
hosts)
Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index c653970..a449ef4 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -77,6 +77,7 @@ static grant_status_t *grstatus;
static struct gnttab_free_callback *gnttab_free_callback_list;
static int grant_table_version;
+module_param_named(version, grant_table_version, int, 0);
static int gnttab_expand(unsigned int req_entries);
@@ -697,24 +698,29 @@ static void gnttab_request_version(void)
int rc;
struct gnttab_set_version gsv;
- gsv.version = 2;
+ /*
+ * If we have already used a particular layout (e.g. before
+ * suspend/resume) then we must continue to use that
+ * version. Otherwise try and use v2.
+ */
+ gsv.version = grant_table_version ? : 2;
rc = HYPERVISOR_grant_table_op(GNTTABOP_set_version, &gsv, 1);
- if (rc == 0)
- grant_table_version = 2;
- else {
- if (grant_table_version == 2) {
- /*
- * If we've already used version 2 features,
- * but then suddenly discover that they're not
- * available (e.g. migrating to an older
- * version of Xen), almost unbounded badness
- * can happen.
- */
- panic("we need grant tables version 2, but only version
1 is
available");
- }
- grant_table_version = 1;
- }
- printk(KERN_INFO "Grant tables using version %d layout.\n",
grant_table_version);
+ BUG_ON(rc == -EFAULT);
+
+ /*
+ * Compatibility with hypervisors which do not support >= v2
+ * grant tables.
+ */
+ if (rc == -ENOSYS)
+ gsv.version = 1;
+ else if (rc != 0)
+ printk(KERN_WARNING "Error %d requesting v%d grant table
layout, got
v%d\n",
+ rc, grant_table_version ? : 2, gsv.version);
+
+ if (grant_table_version && grant_table_version != gsv.version)
+ panic("unable to set required v%d grant table layout",
grant_table_version);
+
+ grant_table_version = gsv.version;
}
static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
@@ -964,7 +970,7 @@ static int __devinit gnttab_init(void)
gnttab_free_count = nr_init_grefs - NR_RESERVED_ENTRIES;
gnttab_free_head = NR_RESERVED_ENTRIES;
- printk("Grant table initialized\n");
+ printk("Grant table initialized using v%d layout\n",
grant_table_version);
return 0;
ini_nomem:
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home