From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965085Ab3HIP01 (ORCPT ); Fri, 9 Aug 2013 11:26:27 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:17543 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964779Ab3HIP0Z (ORCPT ); Fri, 9 Aug 2013 11:26:25 -0400 Date: Fri, 9 Aug 2013 11:26:01 -0400 From: Konrad Rzeszutek Wilk To: Stefano Stabellini Cc: xen-devel@lists.xensource.com, linux-kernel@vger.kernel.org, dcrisan@flexiant.com, alex@alex.org.uk, ian.campbell@citrix.com Subject: Re: [PATCH v4 2/2] xen/m2p: use GNTTABOP_unmap_and_replace to reinstate the original mapping Message-ID: <20130809152601.GB5637@phenom.dumpdata.com> References: <1375627181-14948-2-git-send-email-stefano.stabellini@eu.citrix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1375627181-14948-2-git-send-email-stefano.stabellini@eu.citrix.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: acsinet21.oracle.com [141.146.126.237] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Aug 04, 2013 at 03:39:41PM +0100, Stefano Stabellini wrote: > GNTTABOP_unmap_grant_ref unmaps a grant and replaces it with a 0 > mapping instead of reinstating the original mapping. > Doing so separately would be racy. > > To unmap a grant and reinstate the original mapping atomically we use > GNTTABOP_unmap_and_replace. > GNTTABOP_unmap_and_replace doesn't work with GNTMAP_contains_pte, so > don't use it for kmaps. GNTTABOP_unmap_and_replace zeroes the mapping > passed in new_addr so we have to reinstate it, however that is a > per-cpu mapping only used for balloon scratch pages, so we can be sure that > it's not going to be accessed while the mapping is not valid. This looks to be depend on a new structure, which is not in Linux kernel? Are you missing a dependency patch? Shouldn't we use some logic to figure out which hypercall to use if the hypervisor does not support it? > > Signed-off-by: Stefano Stabellini > Reviewed-by: David Vrabel > CC: alex@alex.org.uk > CC: dcrisan@flexiant.com > --- > arch/x86/xen/p2m.c | 22 +++++++++++++++------- > drivers/xen/gntdev.c | 11 ++--------- > 2 files changed, 17 insertions(+), 16 deletions(-) > > diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c > index 95fb2aa..0d4ec35 100644 > --- a/arch/x86/xen/p2m.c > +++ b/arch/x86/xen/p2m.c > @@ -161,6 +161,7 @@ > #include > #include > #include > +#include > #include > > #include "multicalls.h" > @@ -967,7 +968,10 @@ int m2p_remove_override(struct page *page, > if (kmap_op != NULL) { > if (!PageHighMem(page)) { > struct multicall_space mcs; > - struct gnttab_unmap_grant_ref *unmap_op; > + struct gnttab_unmap_and_replace *unmap_op; > + struct page *scratch_page = get_balloon_scratch_page(); > + unsigned long scratch_page_address = (unsigned long) > + __va(page_to_pfn(scratch_page) << PAGE_SHIFT); > > /* > * It might be that we queued all the m2p grant table > @@ -990,21 +994,25 @@ int m2p_remove_override(struct page *page, > } > > mcs = xen_mc_entry( > - sizeof(struct gnttab_unmap_grant_ref)); > + sizeof(struct gnttab_unmap_and_replace)); > unmap_op = mcs.args; > unmap_op->host_addr = kmap_op->host_addr; > + unmap_op->new_addr = scratch_page_address; > unmap_op->handle = kmap_op->handle; > - unmap_op->dev_bus_addr = 0; > > MULTI_grant_table_op(mcs.mc, > - GNTTABOP_unmap_grant_ref, unmap_op, 1); > + GNTTABOP_unmap_and_replace, unmap_op, 1); > > xen_mc_issue(PARAVIRT_LAZY_MMU); > > - set_pte_at(&init_mm, address, ptep, > - pfn_pte(pfn, PAGE_KERNEL)); > - __flush_tlb_single(address); > + mcs = __xen_mc_entry(0); > + MULTI_update_va_mapping(mcs.mc, scratch_page_address, > + pfn_pte(page_to_pfn(get_balloon_scratch_page()), > + PAGE_KERNEL_RO), 0); > + xen_mc_issue(PARAVIRT_LAZY_MMU); > + > kmap_op->host_addr = 0; > + put_balloon_scratch_page(); > } > } > > diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c > index 3c8803f..51f4c95 100644 > --- a/drivers/xen/gntdev.c > +++ b/drivers/xen/gntdev.c > @@ -270,19 +270,12 @@ static int map_grant_pages(struct grant_map *map) > * with find_grant_ptes. > */ > for (i = 0; i < map->count; i++) { > - unsigned level; > unsigned long address = (unsigned long) > pfn_to_kaddr(page_to_pfn(map->pages[i])); > - pte_t *ptep; > - u64 pte_maddr = 0; > BUG_ON(PageHighMem(map->pages[i])); > > - ptep = lookup_address(address, &level); > - pte_maddr = arbitrary_virt_to_machine(ptep).maddr; > - gnttab_set_map_op(&map->kmap_ops[i], pte_maddr, > - map->flags | > - GNTMAP_host_map | > - GNTMAP_contains_pte, > + gnttab_set_map_op(&map->kmap_ops[i], address, > + map->flags | GNTMAP_host_map, > map->grants[i].ref, > map->grants[i].domid); > } > -- > 1.7.2.5 >