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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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?

> +     }
> +
>       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? 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