Re: [Xen-devel] [PATCH][Xen 4.0-testing.hg] fix small bugs of memory sharing

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

At 08:14 +0000 on 09 Dec (1291882483), Jui-Hao Chiang wrote:
> It seems the candidate page for nomination usually has (type=none, count=1), 
> and it's ok for page_make_sharable() to make it (type=none, count=2) 
> afterwards.
> However, when we have a page (type=none, count=2), the page_make_sharable() 
> will make it wrong as the following steps:
> (step1) get_page() increases count (type=none, count=3)
> (step2) cmpxchg loops changes type (type=8400000000000001, count=3, actually 
> the real value of count_info is 0x8000000000000002)
> (step3) Checking count is greater than 2? Oops!.... abort without recovering 
> type back to none

OK, I think that's the problem.  Pages with type-count 0 basically have
no type so page_make_sharable() is wrong to insist on its having
type=none; it should only check that the typecount is zero.

Can you try changing the test in the cmpxchg loop only to check  
for ((x & PGT_count_mask) != 0)? 

> (Correct me if I am wrong please !!)
> From my observation, the normal page mapped by a single gfn of a domain will 
> have count=1 (PGT_allocated) and type=0.
> The page_make_sharable() will use get_page() before checking ci >=2.
> So it's ok for PGT_allocated page, but not ok if ci =2 before get_page().

Yes, that's what it's enforcing. 

> After some tracing, I found a scenario for the page (ci=2, ti=0).
> It seems the stub domain for a HVM guest will try to map some of HVM's memory 
> into its own address space using do_mmu_update(), which increases the ci from 
> 1 to 2 without changing the type or marking it as shared. For a 1GB 64-bit 
> HVM Centos 5.5 guest (pae=1, hap=1), around 250MB will become ci=2 after 
> booting into user space prompt.
> 
> I wonder the following two things
> (1) stub domain does this to perform I/O for HVM guest? can someone point out 
> where this code is?

Yes; the code is in the qemu sources.

> (2) is there a way or any place to unmap the memory and make the page count 
> back to 1?

IIRC it's possible to ask qemu to unmap the guest's memory but not to
synchronously wait for that to happen (because that would be a priority
inversion at best and a deadlock at worst).  So that doesn't really
help.

If you're trying to do page deduplication you might try putting some of
the logic into qemu, where it can make sure it unmaps the page before
asking to share it. 

> Assume my previous guess for stub domain is right.  Then if a page
> from the previous scenario is made sharable and its mapped mfn is
> freed (when sharing two pages, the later one's mfn will be discarded),
> will the stub domain refer to the old discarded mfn if no unmapping is
> performed?

Yes, and that's definitely not good.  AFAICS, qemu has to drop all its
mappings for the sharing to be safe, and the page needs to be unshared
again if qemu tries to map it again.  Otherwise I/O from one VM could
pollute another VM (or in your case, I/O to one page could overwrite
another page). 

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel