Re: [Xen-devel] [PATCH 2/3] xen/grant_table: use the new GNTTABOP_unmap_and_replace hypercall

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 23 Jul 2013, Konrad Rzeszutek Wilk wrote:
> On Sun, Jul 21, 2013 at 06:32:30PM +0100, Stefano Stabellini wrote:
> > The current GNTTABOP_unmap_and_replace (7) is deprecated, rename
> > it to GNTTABOP_unmap_and_replace_legacy. Introduce the new
> > GNTTABOP_unmap_and_replace (12) and detect at boot time which one is
> > available.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > CC: alex@xxxxxxxxxxx
> > CC: dcrisan@xxxxxxxxxxxx
> > ---
> >  drivers/xen/grant-table.c           |    8 ++++++++
> >  include/xen/grant_table.h           |    1 +
> >  include/xen/interface/grant_table.h |    8 +++++---
> >  3 files changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> > index 04c1b2d..0f7d00b 100644
> > --- a/drivers/xen/grant-table.c
> > +++ b/drivers/xen/grant-table.c
> > @@ -65,6 +65,8 @@ static grant_ref_t gnttab_free_head;
> >  static DEFINE_SPINLOCK(gnttab_list_lock);
> >  unsigned long xen_hvm_resume_frames;
> >  EXPORT_SYMBOL_GPL(xen_hvm_resume_frames);
> > +int __read_mostly gnttabop_unmap_and_replace = GNTTABOP_unmap_and_replace; 
> > +EXPORT_SYMBOL_GPL(gnttabop_unmap_and_replace);
> >  
> >  static union {
> >     struct grant_entry_v1 *v1;
> > @@ -1238,6 +1240,12 @@ int gnttab_init(void)
> >     gnttab_free_count = nr_init_grefs - NR_RESERVED_ENTRIES;
> >     gnttab_free_head  = NR_RESERVED_ENTRIES;
> >  
> > +   ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_and_replace, NULL, 1);
> > +   if (ret == -ENOSYS) {
> > +           gnttabop_unmap_and_replace = GNTTABOP_unmap_and_replace_legacy;
> > +           pr_warn("GNTTABOP_unmap_and_replace unavailable, falling back 
> > to the racy legacy version\n");
> 
> This would show up all the time on Xen 4.1 and Xen 3.6 - so a variety
> of Cloud providers would get this. Do we get any in the field diagnostic
> help with this chatty message?

Yeah, I have removed it together with the proposal of the new
hypercall.


> > +   }
> > +
> >     printk("Grant table initialized\n");
> >     return 0;
> >  
> > diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
> > index 694dcaf..867d1a7 100644
> > --- a/include/xen/grant_table.h
> > +++ b/include/xen/grant_table.h
> > @@ -179,6 +179,7 @@ int arch_gnttab_map_status(uint64_t *frames, unsigned 
> > long nr_gframes,
> >  void arch_gnttab_unmap(void *shared, unsigned long nr_gframes);
> >  
> >  extern unsigned long xen_hvm_resume_frames;
> > +extern int gnttabop_unmap_and_replace;
> >  unsigned int gnttab_max_grant_frames(void);
> >  
> >  #define gnttab_map_vaddr(map) ((void *)(map.host_virt_addr))
> > diff --git a/include/xen/interface/grant_table.h 
> > b/include/xen/interface/grant_table.h
> > index e40fae9..2a2b74f 100644
> > --- a/include/xen/interface/grant_table.h
> > +++ b/include/xen/interface/grant_table.h
> > @@ -408,15 +408,17 @@ DEFINE_GUEST_HANDLE_STRUCT(gnttab_query_size);
> >  /*
> >   * GNTTABOP_unmap_and_replace: Destroy one or more grant-reference mappings
> >   * tracked by <handle> but atomically replace the page table entry with one
> > - * pointing to the machine address under <new_addr>.  <new_addr> will be
> > - * redirected to the null entry.
> > + * pointing to the machine address under <new_addr>.
> > + * GNTTABOP_unmap_and_replace_legacy also redirects <new_addr> to the
> > + * null entry
> 
> I am not following that statement. redirects <new_addr> to the null entry?
> What is a 'null entry'? 
> Does that mean that the page table entry will have
> the value of zero?

Yep


> Or that it will have an PTE entry with the MFN of zero?
> 
> The latter means one could actually still use the PTE but would get 
> an surprise as many folks could be putting stuff in there.
> 
> >   * NOTES:
> >   *  1. The call may fail in an undefined manner if either mapping is not
> >   *     tracked by <handle>.
> >   *  2. After executing a batch of unmaps, it is guaranteed that no stale
> >   *     mappings will remain in the device or host TLBs.
> >   */
> > -#define GNTTABOP_unmap_and_replace    7
> > +#define GNTTABOP_unmap_and_replace_legacy    7
> > +#define GNTTABOP_unmap_and_replace    12
> >  struct gnttab_unmap_and_replace {
> >      /* IN parameters. */
> >      uint64_t host_addr;
> > -- 
> > 1.7.2.5
> > 
> 

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