From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 2/3] xen/grant_table: use the new GNTTABOP_unmap_and_replace hypercall Date: Sun, 21 Jul 2013 18:44:46 +0100 Message-ID: <1374428686.4127.45.camel@hastur.hellion.org.uk> References: <1374427951-24126-2-git-send-email-stefano.stabellini@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1374427951-24126-2-git-send-email-stefano.stabellini@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: dcrisan@flexiant.com, xen-devel@lists.xensource.com, david.vrabel@citrix.com, alex@alex.org.uk List-Id: xen-devel@lists.xenproject.org On Sun, 2013-07-21 at 18:32 +0100, Stefano Stabellini wrote: > The current GNTTABOP_unmap_and_replace (7) is deprecated, rename > it to GNTTABOP_unmap_and_replace_legacy. We would normally call this foo_compat. > Introduce the new > GNTTABOP_unmap_and_replace (12) and detect at boot time which one is > available. I don't think this exists yet, is there a h/v patch under separate cover? > Signed-off-by: Stefano Stabellini > CC: alex@alex.org.uk > CC: dcrisan@flexiant.com > --- > 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); I can see this getting set but not used, does it really need to be exported? > > 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"); Perhaps WARN_ONCE? > + } > + > 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 but atomically replace the page table entry with one > - * pointing to the machine address under . will be > - * redirected to the null entry. > + * pointing to the machine address under . > + * GNTTABOP_unmap_and_replace_legacy also redirects to the > + * null entry I wonder if rather than _legacy (or _compat) if the old behaviour might be well described as "unmap_and_swap", since I think that is what clearing new_addr means? > > * NOTES: > * 1. The call may fail in an undefined manner if either mapping is not > * tracked by . > * 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;