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

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

Hi, 

Can you possibly configure your mailer to indent quoted text?  Your
emails are a bit tricky to read without it. 

At 08:30 +0000 on 10 Dec (1291969810), Jui-Hao Chiang wrote:
> I just checked the value of PGT_none, and it is actually equal to 0,
> so there seems nothing wrong with the original checking.

What I meant was: the current check looks at the type and the count.
AFAICS it ought to be only checking the count and ignoring the type.
That is:

  if((x & (PGT_type_mask | PGT_count_mask)) != PGT_none)

should be:

  if ( (x & PGT_count_mask) != 0 )


> The reason that I swap step3 and step2 is because step3 doesn't modify any 
> value while step2 does.
> If checking count_info fails, then just abort. Else if count_info is correct, 
> then go checking and modifying type_info.

I think the check needs to be after the type change.  Otherwise you
could race with another refcount

> I found one patch of xenpaging as the following, but not sure it's exactly 
> the same as my question.
> Cuts from their article 
> http://thread.gmane.org/gmane.comp.emulators.xen.devel/91768/focus=91770
> "qemu will just keep mapping pages and not release them, which causes 
> problems for the memory pager (since the page is mapped, it won't get paged 
> out)"
> 
> AFAIK, the qemu can not always map the entire address space of HVM guest, so 
> Map Cache feature tends to map HVM's memory on-demand/partially. The 
> flush-cache command will trigger qemu_invalidate_map_cache(), which in turn 
> calls munmap() on all mapped virtual addresses. Am I on the right track?

Possibly.  Where are you putting the code that scans for duplicate pages?

> > 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).
> 
> 
> Thanks for your remind "the page needs to be unshared again if qemu tries to 
> map it again".
> There exists several hypercalls to perform the memory mapping, e.g. 
> mmu_update, update_va_mapping, and I will check on them.
> Of course, only the RW mapping should be taken care, right?

No, any foreign mapping needs to be handled, including grant tables,
because there's no guaranteed way to fix them up when the sharing is
undone later. 

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