From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH V3] xen/balloon: flush persistent kmaps in correct position Date: Tue, 18 Mar 2014 17:40:55 +0000 Message-ID: <53288527.8070003@citrix.com> References: <1394899907-2529-1-git-send-email-wei.liu2@citrix.com> <20140318134728.GA16807@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WPy0c-0006jS-AN for xen-devel@lists.xenproject.org; Tue, 18 Mar 2014 17:41:12 +0000 In-Reply-To: <20140318134728.GA16807@zion.uk.xensource.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: Wei Liu Cc: xen-devel@lists.xenproject.org, Boris Ostrovsky , David Vrabel List-Id: xen-devel@lists.xenproject.org On 18/03/14 13:47, Wei Liu wrote: > On Sat, Mar 15, 2014 at 04:11:47PM +0000, Wei Liu wrote: >> >> --- a/drivers/xen/balloon.c >> +++ b/drivers/xen/balloon.c >> @@ -404,6 +404,15 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) >> frame_list[i] = pfn_to_mfn(pfn); >> >> scrub_page(page); >> + } >> + >> + /* Ensure that ballooned highmem pages don't have kmaps. */ >> + kmap_flush_unused(); >> + flush_tlb_all(); >> + >> + /* No more mappings: invalidate P2M and add to balloon. */ >> + for (i = 0; i < nr_pages; i++) { >> + pfn = mfn_to_pfn(frame_list[i]); >> > > + page = pfn_to_page(pfn); // missing in this patch > > This missing line causes PageHighMem to test on the wrong page. > > David, you can either take V3 and add this line, or take V4. Wei, how about this? I expanded the comment, left the tlb flush after changing the mappings, and shuffled a few bits around to avoid a few pfn/mfn/page conversions. Can you test it, please? Thanks. 8<--------------------------------- >>From e0c38006cfd0395d500a66d2ab07dfd1327d140d Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Sat, 15 Mar 2014 16:11:47 +0000 Subject: [PATCH] xen/balloon: flush persistent kmaps in correct position Xen balloon driver will update ballooned out pages' P2M entries to point to scratch page for PV guests. In 24f69373e2 ("xen/balloon: don't alloc page while non-preemptible", kmap_flush_unused was moved after updating P2M table. In that case for 32 bit PV guest we might end up with P2M X -----> S (S is mfn of balloon scratch page) M2P Y -----> X (Y is mfn in persistent kmap entry) kmap_flush_unused() iterates through all the PTEs in the kmap address space, using pte_to_page() to obtain the page. If the p2m and the m2p are inconsistent the incorrect page is returned. This will clear page->address on the wrong page which may cause subsequent oopses if that page is currently kmap'ed. Move the flush back between get_page and __set_phys_to_machine to fix this. Signed-off-by: Wei Liu Signed-off-by: David Vrabel Cc: stable@vger.kernel.org # 3.12+ --- drivers/xen/balloon.c | 24 ++++++++++++++++++------ 1 files changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c index 37d06ea..3e27d11 100644 --- a/drivers/xen/balloon.c +++ b/drivers/xen/balloon.c @@ -399,12 +399,26 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) state = BP_EAGAIN; break; } + scrub_page(page); - pfn = page_to_pfn(page); - frame_list[i] = pfn_to_mfn(pfn); + frame_list[i] = page_to_pfn(page); + } - scrub_page(page); + /* + * Ensure that ballooned highmem pages don't have kmaps. + * + * Do this before changing the p2m as kmap_flush_unused() + * reads PTEs to obtain pages (and hence needs the original + * p2m entry). + */ + kmap_flush_unused(); + /* Update direct mapping, invalidate P2M, and add to balloon. */ + for (i = 0; i < nr_pages; i++) { + pfn = frame_list[i]; + frame_list[i] = pfn_to_mfn(pfn); + page = pfn_to_page(pfn); + #ifdef CONFIG_XEN_HAVE_PVMMU /* * Ballooned out frames are effectively replaced with @@ -429,11 +443,9 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) } #endif - balloon_append(pfn_to_page(pfn)); + balloon_append(page); } - /* Ensure that ballooned highmem pages don't have kmaps. */ - kmap_flush_unused(); flush_tlb_all(); set_xen_guest_handle(reservation.extent_start, frame_list); -- 1.7.2.5