Re: [PATCH 04/18] xen: Preserve reserved grant entries when switching versions

From: Ian Campbell <Ian.Campbell@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
Subject: Re: [PATCH 04/18] xen: Preserve reserved grant entries when switching versions
Date: Thu, 12 Jan 2012 09:56:11 +0000	[thread overview]
Message-ID: <1326362172.17210.224.camel@zakaz.uk.xensource.com> (raw)
In-Reply-To: <1326361791.17210.221.camel@zakaz.uk.xensource.com>

On Thu, 2012-01-12 at 09:49 +0000, Ian Campbell wrote:
> On Thu, 2012-01-12 at 08:53 +0000, Jan Beulich wrote:
> > >>> On 11.01.12 at 18:21, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> > > --- a/xen/common/grant_table.c
> > > +++ b/xen/common/grant_table.c
> > > @@ -2106,6 +2106,7 @@ gnttab_set_version(XEN_GUEST_HANDLE(gnttab_set_version_t uop))
> > >      struct domain *d = current->domain;
> > >      struct grant_table *gt = d->grant_table;
> > >      struct active_grant_entry *act;
> > > +    grant_entry_v1_t reserved_entries[GNTTAB_NR_RESERVED_ENTRIES];
> > 
> > How much stack space does this consume? (Or in other words, where
> > does GNTTAB_NR_RESERVED_ENTRIES get defined? It's not in this
> > patch, I can't find it in earlier patches, and it also doesn't exist in the
> > current staging tree.)
> 
> I could have sworn I'd seen this somewhere but I can't for the life of
> me find it now :-/

Found it in patch #6.

+/* External tools reserve first few grant table entries. */
+#define GNTTAB_NR_RESERVED_ENTRIES     8
+#define GNTTAB_RESERVED_CONSOLE        0
+#define GNTTAB_RESERVED_XENSTORE       1
+/* (the next 6 are reserved for future use) */

These patches need to be reordered to make sure each step builds.

Ian.

> 
> One place I did find it is in the Linux gnttab driver, but that's not
> canonical.
> 
> Ian.
> 
> > 
> > Jan
> > 
> > >      long res;
> > >      int i;
> > >  
> > > @@ -2126,7 +2127,7 @@ 
> > > gnttab_set_version(XEN_GUEST_HANDLE(gnttab_set_version_t uop))
> > >      /* (You need to change the version number for e.g. kexec.) */
> > >      if ( gt->gt_version != 0 )
> > >      {
> > > -        for ( i = 0; i < nr_grant_entries(gt); i++ )
> > > +        for ( i = GNTTAB_NR_RESERVED_ENTRIES; i < nr_grant_entries(gt); i++ 
> > > )
> > >          {
> > >              act = &active_entry(gt, i);
> > >              if ( act->pin != 0 )
> > > @@ -2151,15 +2152,38 @@ 
> > > gnttab_set_version(XEN_GUEST_HANDLE(gnttab_set_version_t uop))
> > >              goto out_unlock;
> > >      }
> > >  
> > > +    /* Preserve the first 8 entries (toolstack reserved grants) */
> > > +    if (gt->gt_version == 1) {
> > > +        memcpy(reserved_entries, gt->shared_v1[0], sizeof(reserved_entries));
> > > +    } else if (gt->gt_version == 2) {
> > > +        for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES && i < 
> > > nr_grant_entries(gt); i++ )
> > > +        {
> > > +            reserved_entries[i].flags = shared_entry_v2(gt, i).hdr.flags;
> > > +            reserved_entries[i].domid = shared_entry_v2(gt, i).hdr.domid;
> > > +            reserved_entries[i].frame = shared_entry_v2(gt, 
> > > i).full_page.frame;
> > > +            reserved_entries[i].flags |= status_entry(gt, i);
> > > +        }
> > > +    }
> > > +
> > >      if ( op.version < 2 && gt->gt_version == 2 )
> > >          gnttab_unpopulate_status_frames(d, gt);
> > >  
> > > -    if ( op.version != gt->gt_version )
> > > -    {
> > > -        /* Make sure there's no crud left over in the table from the
> > > -           old version. */
> > > -        for ( i = 0; i < nr_grant_frames(gt); i++ )
> > > -            memset(gt->shared_raw[i], 0, PAGE_SIZE);
> > > +    /* Make sure there's no crud left over in the table from the
> > > +       old version. */
> > > +    for ( i = 0; i < nr_grant_frames(gt); i++ )
> > > +        memset(gt->shared_raw[i], 0, PAGE_SIZE);
> > > +
> > > +    /* Restore the first 8 entries (toolstack reserved grants) */
> > > +    if (gt->gt_version != 0 && op.version == 1) {
> > > +        memcpy(gt->shared_v1[0], reserved_entries, sizeof(reserved_entries));
> > > +    } else if (gt->gt_version != 0 && op.version == 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;
> > > +        }
> > >      }
> > >  
> > >      gt->gt_version = op.version;
> > > -- 
> > > 1.7.7.5
> > > 
> > > 
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@lists.xensource.com 
> > > http://lists.xensource.com/xen-devel 
> > 
> > 
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel