From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH 2/3] xen/grant_table: use the new GNTTABOP_unmap_and_replace hypercall Date: Tue, 23 Jul 2013 17:53:05 +0100 Message-ID: References: <1374427951-24126-2-git-send-email-stefano.stabellini@eu.citrix.com> <20130723140848.GA4389@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130723140848.GA4389@phenom.dumpdata.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: Konrad Rzeszutek Wilk Cc: xen-devel@lists.xensource.com, Ian.Campbell@citrix.com, Stefano Stabellini , david.vrabel@citrix.com, alex@alex.org.uk, dcrisan@flexiant.com List-Id: xen-devel@lists.xenproject.org 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 > > 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); > > > > 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 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 am not following that statement. redirects 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 . > > * 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 > > >