Re: [Xen-devel] Re: [PATCH] xen: reduce severity of message about using v1 grant tables.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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