Re: [Xen-devel] [PATCH V3] xen/balloon: flush persistent kmaps in correct position

[David Vrabel <david.vrabel@xxxxxxxxxx>]

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 <wei.liu2@xxxxxxxxxx>
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 <wei.liu2@xxxxxxxxxx>
Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx # 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel