mapping problems in xenpaging

mapping problems in xenpaging

[zhen shi]

[-- Attachment #1.1: Type: text/plain, Size: 1403 bytes --]

 Hi,Olaf,

 When we analyze and test xenpaging,we found there are some problems between
mapping and xenpaging.
 1) When mapping firstly, then do xenpaging,and the code paths have resolved
the problems.It's OK.
 2) The problems exists if we do address mapping firstly then go to
xenpaging,and our confusions are as followings:
   a) If the domU's memory is directly mapped to dom0,such as the hypercall
from pv driver,then it will build a related page-table in dom0,which will
not change p2m-type.
      and then do the xenpaging to page out the domU's memory pages whose
gfn address have been already mapped to dom0;So it will cause some problems
when dom0
      accesses these pages.Because these pages are paged-out,and dom0 cannot
tell the p2mt before access the pages.
  b)The another situation is that if xen has mapped the domU's page, and get
the mfn according to pfn_to_mfn.But then the page's p2mt is changed by
others, so when xen
    accesses the page ,it will cause problems such as BSOD or reboot.Because
the operations of getting mfn and accessing the page are not
atomic.and the situation exists
    in many code paths .
   According to the above-mentioned points,do you have any suggestions about
what to do to avoid these situations.We have thought these two problems,but
currently have no
  good method to resolve.

  I am looking forward to hearing from you. Thank you very much!  :)

[-- Attachment #1.2: Type: text/html, Size: 1652 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: mapping problems in xenpaging

[Olaf Hering]

On Thu, Sep 29, zhen shi wrote:

>  Hi,Olaf,
>  
>  When we analyze and test xenpaging,we found there are some problems between
> mapping and xenpaging.
>  1) When mapping firstly, then do xenpaging,and the code paths have resolved
> the problems.It's OK.
>  2) The problems exists if we do address mapping firstly then go to
> xenpaging,and our confusions are as followings:
>    a) If the domU's memory is directly mapped to dom0,such as the hypercall
> from pv driver,then it will build a related page-table in dom0,which will not
> change p2m-type.
>       and then do the xenpaging to page out the domU's memory pages whose gfn
> address have been already mapped to dom0;So it will cause some problems when
> dom0
>       accesses these pages.Because these pages are paged-out,and dom0 cannot
> tell the p2mt before access the pages.

I'm not entirely sure what you do. xenpaging runs in dom0 and is able to
map paged-out pages. It uses that to trigger a page-in, see
tools/xenpaging/pagein.c in xen-unstable.hg

>   b)The another situation is that if xen has mapped the domU's page, and get
> the mfn according to pfn_to_mfn.But then the page's p2mt is changed by others,
> so when xen
>     accesses the page ,it will cause problems such as BSOD or reboot.Because
> the operations of getting mfn and accessing the page are not
> atomic.and the situation exists
>     in many code paths .

Can you be more specific what you mean? Xen doesnt seem to have a
pfn_to_mfn function, only the tools have some helper macros of that name.


Olaf

Re: Re: mapping problems in xenpaging

[Adin Scannell]

>>  When we analyze and test xenpaging,we found there are some problems between
>> mapping and xenpaging.
>>  1) When mapping firstly, then do xenpaging,and the code paths have resolved
>> the problems.It's OK.
>>  2) The problems exists if we do address mapping firstly then go to
>> xenpaging,and our confusions are as followings:
>>    a) If the domU's memory is directly mapped to dom0,such as the hypercall
>> from pv driver,then it will build a related page-table in dom0,which will not
>> change p2m-type.
>>       and then do the xenpaging to page out the domU's memory pages whose gfn
>> address have been already mapped to dom0;So it will cause some problems when
>> dom0
>>       accesses these pages.Because these pages are paged-out,and dom0 cannot
>> tell the p2mt before access the pages.
>
> I'm not entirely sure what you do. xenpaging runs in dom0 and is able to
> map paged-out pages. It uses that to trigger a page-in, see
> tools/xenpaging/pagein.c in xen-unstable.hg

Here's my take...

Xenpaging doesn't allow selection of pages that have been mapped by
other domains (as in p2m.c):

 669 int p2m_mem_paging_nominate(struct domain *d, unsigned long gfn)
....
 693     /* Check page count and type */
 694     page = mfn_to_page(mfn);
 695     if ( (page->count_info & (PGC_count_mask | PGC_allocated)) !=
 696          (1 | PGC_allocated) )
 697         goto out;

*However*, I think that the problem Zhen is describing still exists:
1) xenpaging nominates a page, it is successful.
2) dom0 maps the same page (a process other than xenpaging, which will
also map it).
3) xenpaging evicts the page, successfully.

I've experienced a few nasty crashes, and I think this could account
for a couple (but certainly not all)... I think that the solution may
be to repeat the refcount check in paging_evict, and roll back the
nomination gracefully if the race is detected. Thoughts?

>>   b)The another situation is that if xen has mapped the domU's page, and get
>> the mfn according to pfn_to_mfn.But then the page's p2mt is changed by others,
>> so when xen
>>     accesses the page ,it will cause problems such as BSOD or reboot.Because
>> the operations of getting mfn and accessing the page are not
>> atomic.and the situation exists
>>     in many code paths .

I believe I have recreated this problem a few times, resulting in
various crashes... unfortunately, there is somewhat of an implicit
assumption throughout the code that when you grab an mfn via
gfn_to_mfn, that mfn won't disappear underneath you (for example, see
vmx_load_pdptrs).  Really, you want something like gfn_to_mfn_getpage,
where the underlying page has its refcount bumped so that it won't be
nominated/evicted while you map and use the page, then you must put it
back when you're done.  I hope to look into helping fix some of these
paging bugs soon.

Cheers,
-Adin

Re: Re: mapping problems in xenpaging

[Tim Deegan]

At 17:02 -0400 on 30 Sep (1317402151), Adin Scannell wrote:
> I believe I have recreated this problem a few times, resulting in
> various crashes... unfortunately, there is somewhat of an implicit
> assumption throughout the code that when you grab an mfn via
> gfn_to_mfn, that mfn won't disappear underneath you (for example, see
> vmx_load_pdptrs).  Really, you want something like gfn_to_mfn_getpage,
> where the underlying page has its refcount bumped so that it won't be
> nominated/evicted while you map and use the page, then you must put it
> back when you're done.  

Quite right - there are a lot of places that assume that a p2m mapping
won't change underfoot, and the right answer will involve reference
counting of some kind - either better integration with the underlying
refcount/typecount system or some reference count in the p2m. 

The tricky part will be what to do when a p2m update can't be made
because of one of those refcounts. 

Tim.

Re: Re: mapping problems in xenpaging

[Olaf Hering]

On Fri, Sep 30, Adin Scannell wrote:

> >>  When we analyze and test xenpaging,we found there are some problems between
> >> mapping and xenpaging.
> >>  1) When mapping firstly, then do xenpaging,and the code paths have resolved
> >> the problems.It's OK.
> >>  2) The problems exists if we do address mapping firstly then go to
> >> xenpaging,and our confusions are as followings:
> >>    a) If the domU's memory is directly mapped to dom0,such as the hypercall
> >> from pv driver,then it will build a related page-table in dom0,which will not
> >> change p2m-type.
> >>       and then do the xenpaging to page out the domU's memory pages whose gfn
> >> address have been already mapped to dom0;So it will cause some problems when
> >> dom0
> >>       accesses these pages.Because these pages are paged-out,and dom0 cannot
> >> tell the p2mt before access the pages.
> >
> > I'm not entirely sure what you do. xenpaging runs in dom0 and is able to
> > map paged-out pages. It uses that to trigger a page-in, see
> > tools/xenpaging/pagein.c in xen-unstable.hg
> 
> Here's my take...
> 
> Xenpaging doesn't allow selection of pages that have been mapped by
> other domains (as in p2m.c):
> 
>  669 int p2m_mem_paging_nominate(struct domain *d, unsigned long gfn)
> ....
>  693     /* Check page count and type */
>  694     page = mfn_to_page(mfn);
>  695     if ( (page->count_info & (PGC_count_mask | PGC_allocated)) !=
>  696          (1 | PGC_allocated) )
>  697         goto out;
> 
> *However*, I think that the problem Zhen is describing still exists:
> 1) xenpaging nominates a page, it is successful.
> 2) dom0 maps the same page (a process other than xenpaging, which will
> also map it).
> 3) xenpaging evicts the page, successfully.
> 
> I've experienced a few nasty crashes, and I think this could account
> for a couple (but certainly not all)... I think that the solution may
> be to repeat the refcount check in paging_evict, and roll back the
> nomination gracefully if the race is detected. Thoughts?

Are there really code paths that touch a mfn without going through the
p2m functions? If so I will copy the check and update xenpaging.

Olaf

Re: Re: mapping problems in xenpaging

[Tim Deegan]

At 16:56 +0200 on 03 Oct (1317660976), Olaf Hering wrote:
> On Fri, Sep 30, Adin Scannell wrote:
> 
> > >>  When we analyze and test xenpaging,we found there are some problems between
> > >> mapping and xenpaging.
> > >>  1) When mapping firstly, then do xenpaging,and the code paths have resolved
> > >> the problems.It's OK.
> > >>  2) The problems exists if we do address mapping firstly then go to
> > >> xenpaging,and our confusions are as followings:
> > >>    a) If the domU's memory is directly mapped to dom0,such as the hypercall
> > >> from pv driver,then it will build a related page-table in dom0,which will not
> > >> change p2m-type.
> > >>       and then do the xenpaging to page out the domU's memory pages whose gfn
> > >> address have been already mapped to dom0;So it will cause some problems when
> > >> dom0
> > >>       accesses these pages.Because these pages are paged-out,and dom0 cannot
> > >> tell the p2mt before access the pages.
> > >
> > > I'm not entirely sure what you do. xenpaging runs in dom0 and is able to
> > > map paged-out pages. It uses that to trigger a page-in, see
> > > tools/xenpaging/pagein.c in xen-unstable.hg
> > 
> > Here's my take...
> > 
> > Xenpaging doesn't allow selection of pages that have been mapped by
> > other domains (as in p2m.c):
> > 
> >  669 int p2m_mem_paging_nominate(struct domain *d, unsigned long gfn)
> > ....
> >  693     /* Check page count and type */
> >  694     page = mfn_to_page(mfn);
> >  695     if ( (page->count_info & (PGC_count_mask | PGC_allocated)) !=
> >  696          (1 | PGC_allocated) )
> >  697         goto out;
> > 
> > *However*, I think that the problem Zhen is describing still exists:
> > 1) xenpaging nominates a page, it is successful.
> > 2) dom0 maps the same page (a process other than xenpaging, which will
> > also map it).
> > 3) xenpaging evicts the page, successfully.
> > 
> > I've experienced a few nasty crashes, and I think this could account
> > for a couple (but certainly not all)... I think that the solution may
> > be to repeat the refcount check in paging_evict, and roll back the
> > nomination gracefully if the race is detected. Thoughts?
> 
> Are there really code paths that touch a mfn without going through the
> p2m functions? If so I will copy the check and update xenpaging.

No, but there are race conditions where CPU A could to the p2m lookup,
then CPU B nominates the page and changes its p2m entry, then CPU A
completes the mapping.  In the extreme case, detecting this in the
eviction code is also subject to the same race; some sort of atomic
lookup-and-get-reference operation seems like a better fix.

Tim.

Re: Re: mapping problems in xenpaging

[zhen shi]

[-- Attachment #1.1: Type: text/plain, Size: 3158 bytes --]

2011/10/6 Tim Deegan <tim@xen.org>

> At 16:56 +0200 on 03 Oct (1317660976), Olaf Hering wrote:
> > On Fri, Sep 30, Adin Scannell wrote:
> >
> > > >>  When we analyze and test xenpaging,we found there are some
> problems between
> > > >> mapping and xenpaging.
> > > >>  1) When mapping firstly, then do xenpaging,and the code paths have
> resolved
> > > >> the problems.It's OK.
> > > >>  2) The problems exists if we do address mapping firstly then go to
> > > >> xenpaging,and our confusions are as followings:
> > > >>    a) If the domU's memory is directly mapped to dom0,such as the
> hypercall
> > > >> from pv driver,then it will build a related page-table in dom0,which
> will not
> > > >> change p2m-type.
> > > >>       and then do the xenpaging to page out the domU's memory pages
> whose gfn
> > > >> address have been already mapped to dom0;So it will cause some
> problems when
> > > >> dom0
> > > >>       accesses these pages.Because these pages are paged-out,and
> dom0 cannot
> > > >> tell the p2mt before access the pages.
> > > >
> > > > I'm not entirely sure what you do. xenpaging runs in dom0 and is able
> to
> > > > map paged-out pages. It uses that to trigger a page-in, see
> > > > tools/xenpaging/pagein.c in xen-unstable.hg
> > >
> > > Here's my take...
> > >
> > > Xenpaging doesn't allow selection of pages that have been mapped by
> > > other domains (as in p2m.c):
> > >
> > >  669 int p2m_mem_paging_nominate(struct domain *d, unsigned long gfn)
> > > ....
> > >  693     /* Check page count and type */
> > >  694     page = mfn_to_page(mfn);
> > >  695     if ( (page->count_info & (PGC_count_mask | PGC_allocated)) !=
> > >  696          (1 | PGC_allocated) )
> > >  697         goto out;
>
>     I wonder if pages have been mapped by other domains,then the
page->count_info will be added.I have analyzed  xc_map_foreign_pages()
function,and have not figured out how to add the page->count_info
by xc_map_foreign_pages().and the page->count_info decreases in munmap().


> > > *However*, I think that the problem Zhen is describing still exists:
> > > 1) xenpaging nominates a page, it is successful.
> > > 2) dom0 maps the same page (a process other than xenpaging, which will
> > > also map it).
> > > 3) xenpaging evicts the page, successfully.
> > >
> > > I've experienced a few nasty crashes, and I think this could account
> > > for a couple (but certainly not all)... I think that the solution may
> > > be to repeat the refcount check in paging_evict, and roll back the
> > > nomination gracefully if the race is detected. Thoughts?
>


> > Are there really code paths that touch a mfn without going through the
> > p2m functions? If so I will copy the check and update xenpaging.
>
> >No, but there are race conditions where CPU A could to the p2m lookup,
> >then CPU B nominates the page and changes its p2m entry, then CPU A
> >completes the mapping.  In the extreme case, detecting this in the
> >eviction code is also subject to the same race; some sort of atomic
> >lookup-and-get-reference operation seems like a better fix.
>

  Tim , Olaf and Adin, do you have any good ideas about the above
situation.

[-- Attachment #1.2: Type: text/html, Size: 4322 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: mapping problems in xenpaging

[zhen shi]

[-- Attachment #1.1: Type: text/plain, Size: 2683 bytes --]

Olaf,maybe I didn't described the problems clearly.I will give an example.
a) In xen_vga_vram_map() of vga.c from tools/ioemu-qemu-xen/hw,  it uses
xc_map_foreign_pages() to map a page's gfn address to dom0. If then the page
is paged out and changed to zero page in xenpaging, and dom0 access the page
such as using the mapped address, it will make mistakes.Am I right?
In brief,I mean there may be some conflicts in xc_map_foreign_pages from
other functions and xenpaging feature when they access the same page.

b) In create_grant_pte_mapping() of mm.c from /xen/arch/x86, it
uses gmfn_to_mfn() to get mfn, and then executes map_domain_page(mfn). At
the same time, the page is paged_out and the mfn is changed to INVALID_MFN.
So that in create_grant_pte_mapping () when it goes to mfn_to_page(mfn), it
will make a mistake.Because xen didn't judge the mfn and thought the mfn was
original.
I mean there may be some conflicts of operations after getting the mfn in
xen but the page is paged_out in the meantime.


2011/9/30 Olaf Hering <olaf@aepfle.de>

> On Thu, Sep 29, zhen shi wrote:
>
> >  Hi,Olaf,
> >
> >  When we analyze and test xenpaging,we found there are some
> problems between
> > mapping and xenpaging.
> >  1) When mapping firstly, then do xenpaging,and the code paths have
> resolved
> > the problems.It's OK.
> >  2) The problems exists if we do address mapping firstly then go to
> > xenpaging,and our confusions are as followings:
> >    a) If the domU's memory is directly mapped to dom0,such as the
> hypercall
> > from pv driver,then it will build a related page-table in dom0,which will
> not
> > change p2m-type.
> >       and then do the xenpaging to page out the domU's memory pages whose
> gfn
> > address have been already mapped to dom0;So it will cause some problems
> when
> > dom0
> >       accesses these pages.Because these pages are paged-out,and dom0
> cannot
> > tell the p2mt before access the pages.
>
> I'm not entirely sure what you do. xenpaging runs in dom0 and is able to
> map paged-out pages. It uses that to trigger a page-in, see
> tools/xenpaging/pagein.c in xen-unstable.hg
>
> >   b)The another situation is that if xen has mapped the domU's page, and
> get
> > the mfn according to pfn_to_mfn.But then the page's p2mt is changed by
> others,
> > so when xen
> >     accesses the page ,it will cause problems such as BSOD or
> reboot.Because
> > the operations of getting mfn and accessing the page are not
> > atomic.and the situation exists
> >     in many code paths .
>
> Can you be more specific what you mean? Xen doesnt seem to have a
> pfn_to_mfn function, only the tools have some helper macros of that name.
>
>
> Olaf
>

[-- Attachment #1.2: Type: text/html, Size: 3284 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: Re: mapping problems in xenpaging

[Andres Lagar Cavilla]

I have a proposal. I'd like to hear from the list what they think.

- 1. change p2m lock to a read/write lock
- 2. Make lookups (gfn_to_mfn_* family) take a read lock. All current
callers of p2m_lock will become write lockers.
- 3. Change the gfn_to_mfn_* family to get_page on the mfn obtained,
while holding the read lock.
- 4. Have all lookup callers put_page on the obtained mfn, once done.

Rationale: rwlock will prevent races between lookups and async p2m
modifications by paging, sharing, or feature X. The lookup routine
will be protected from races and able to atomically get_page on the
obtained mfn. The lookup caller will be able to work on this mfn
knowing it won't disappear underneath (as in the case currently
brought forward by Zhen)

I'm somewhat wary of having all callers required to put_page, but I
don't think it's a big deal because it's perfectly reasonable.

I'm more wary that turning p2m locking into read/write will result in
code deadlocking itself: taking a read lock first and a write lock
later. Possibly the current rwlock implementation could be improved to
keep a cpumask of read-lockers, and provide an atomic "promote from
read to write" atomic operation (something along the lines of wait
until you're the only reader in the cpumask, and then cmpxchg(lock,
-1, WRITE_BIAS))

Hope that made sense. Thoughts?

Andres

> Date: Mon, 10 Oct 2011 00:40:32 +0800
> From: zhen shi <bickys1986@gmail.com>
> Subject: Re: [Xen-devel] Re: mapping problems in xenpaging
> To: Tim Deegan <tim@xen.org>, Olaf Hering <olaf@aepfle.de>,     Adin
>        Scannell <adin@gridcentric.ca>
> Cc: xen-devel@lists.xensource.com
> Message-ID:
>        <CACavRyA+Djzr3AVwgaZQu1-doPiMkAZ-NpdVR1nXjiiW_74PqQ@mail.gmail.com>
> Content-Type: text/plain; charset="iso-8859-1"
>
> 2011/10/6 Tim Deegan <tim@xen.org>
>
>> At 16:56 +0200 on 03 Oct (1317660976), Olaf Hering wrote:
>> > On Fri, Sep 30, Adin Scannell wrote:
>> >
>> > > >>  When we analyze and test xenpaging,we found there are some
>> problems between
>> > > >> mapping and xenpaging.
>> > > >>  1) When mapping firstly, then do xenpaging,and the code paths have
>> resolved
>> > > >> the problems.It's OK.
>> > > >>  2) The problems exists if we do address mapping firstly then go to
>> > > >> xenpaging,and our confusions are as followings:
>> > > >>    a) If the domU's memory is directly mapped to dom0,such as the
>> hypercall
>> > > >> from pv driver,then it will build a related page-table in dom0,which
>> will not
>> > > >> change p2m-type.
>> > > >>       and then do the xenpaging to page out the domU's memory pages
>> whose gfn
>> > > >> address have been already mapped to dom0;So it will cause some
>> problems when
>> > > >> dom0
>> > > >>       accesses these pages.Because these pages are paged-out,and
>> dom0 cannot
>> > > >> tell the p2mt before access the pages.
>> > > >
>> > > > I'm not entirely sure what you do. xenpaging runs in dom0 and is able
>> to
>> > > > map paged-out pages. It uses that to trigger a page-in, see
>> > > > tools/xenpaging/pagein.c in xen-unstable.hg
>> > >
>> > > Here's my take...
>> > >
>> > > Xenpaging doesn't allow selection of pages that have been mapped by
>> > > other domains (as in p2m.c):
>> > >
>> > >  669 int p2m_mem_paging_nominate(struct domain *d, unsigned long gfn)
>> > > ....
>> > >  693     /* Check page count and type */
>> > >  694     page = mfn_to_page(mfn);
>> > >  695     if ( (page->count_info & (PGC_count_mask | PGC_allocated)) !=
>> > >  696          (1 | PGC_allocated) )
>> > >  697         goto out;
>>
>>     I wonder if pages have been mapped by other domains,then the
> page->count_info will be added.I have analyzed  xc_map_foreign_pages()
> function,and have not figured out how to add the page->count_info
> by xc_map_foreign_pages().and the page->count_info decreases in munmap().
>
>
>> > > *However*, I think that the problem Zhen is describing still exists:
>> > > 1) xenpaging nominates a page, it is successful.
>> > > 2) dom0 maps the same page (a process other than xenpaging, which will
>> > > also map it).
>> > > 3) xenpaging evicts the page, successfully.
>> > >
>> > > I've experienced a few nasty crashes, and I think this could account
>> > > for a couple (but certainly not all)... I think that the solution may
>> > > be to repeat the refcount check in paging_evict, and roll back the
>> > > nomination gracefully if the race is detected. Thoughts?
>>
>
>
>> > Are there really code paths that touch a mfn without going through the
>> > p2m functions? If so I will copy the check and update xenpaging.
>>
>> >No, but there are race conditions where CPU A could to the p2m lookup,
>> >then CPU B nominates the page and changes its p2m entry, then CPU A
>> >completes the mapping.  In the extreme case, detecting this in the
>> >eviction code is also subject to the same race; some sort of atomic
>> >lookup-and-get-reference operation seems like a better fix.
>>
>
>  Tim , Olaf and Adin, do you have any good ideas about the above
> situation.
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: http://lists.xensource.com/archives/html/xen-devel/attachments/20111010/55486330/attachment.html
>
> ------------------------------
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
> End of Xen-devel Digest, Vol 80, Issue 104
> ******************************************
>

Re: Re: mapping problems in xenpaging

[Tim Deegan]

At 21:20 -0400 on 09 Oct (1318195224), Andres Lagar Cavilla wrote:
> I have a proposal. I'd like to hear from the list what they think.
> 
> - 1. change p2m lock to a read/write lock
> - 2. Make lookups (gfn_to_mfn_* family) take a read lock. All current
> callers of p2m_lock will become write lockers.
> - 3. Change the gfn_to_mfn_* family to get_page on the mfn obtained,
> while holding the read lock.
> - 4. Have all lookup callers put_page on the obtained mfn, once done.

This seems like a step in the right direction, but if we're going to
make this big an interface change there might be better interfaces to
end up with.

A few issues I can see with it:
 - p2m lookups are on some very performance-sensitive paths
   (e.g. multiple times in any pagetable walk or instruction emulation
   in a HVM guest) so adding the rwlock might have a noticeable impact.

 - This fixes one class of races (page gets freed-to-xen underfoot) but
   leaves another one (gfn -> mfn map changes underfoot) untouched.  In
   particular it doesn't solve the race where a foreign mapper 
   gets a r/w map of what's meant to be a read-only frame.

I think that to fix things properly we need to have some refcount
associated with the p2m mapping itself.  That would be taken by all
lookups (or at least some - we could have a flag to the p2m lookup) and
released as you suggest, but more importantly it would block all p2m changes
while the count was raised.  (I think that a least in the common case we
could encode such a refcount using the existing typecount).

One problem then is how to make all the callers of the p2m update
functions handle failure, either by waiting (new deadlock risk?) or
returning EAGAIN at the hypercall interface.  Paths where the update
isn't caused by an explicit request (like log-dirty and the mem-event
rx-to-rw conversion) would be particularly tricky.  

More seriously, it introduces another round of the sort of priority
inversion we already get with the existing refcounts - a foreign
mapping, caused by a user-space program in another VM, could arbitrarily
delay a p2m update (and so prevent a VCPU from making progress), without
any mechanism to even request that the mapping be removed.

Any ideas how to avoid that?  Potentially with some extra bookkeeping on
foreign mappings we could revoke or redirect them when the p2m changes.
That would fit nicely with the abstraction in the interfaces where HVM
domains' memory is always indexed by pfn.  I can imagine it being quite
tricky though.

> I'm more wary that turning p2m locking into read/write will result in
> code deadlocking itself: taking a read lock first and a write lock
> later. Possibly the current rwlock implementation could be improved to
> keep a cpumask of read-lockers, and provide an atomic "promote from
> read to write" atomic operation (something along the lines of wait
> until you're the only reader in the cpumask, and then cmpxchg(lock,
> -1, WRITE_BIAS))

I think that would deadlock if two cpus tried it at once.

Tim.

Re: Re: mapping problems in xenpaging

[Andres Lagar Cavilla]

Hi Tim,

On Mon, Oct 10, 2011 at 5:21 AM, Tim Deegan <tim@xen.org> wrote:
> At 21:20 -0400 on 09 Oct (1318195224), Andres Lagar Cavilla wrote:
>> I have a proposal. I'd like to hear from the list what they think.
>>
>> - 1. change p2m lock to a read/write lock
>> - 2. Make lookups (gfn_to_mfn_* family) take a read lock. All current
>> callers of p2m_lock will become write lockers.
>> - 3. Change the gfn_to_mfn_* family to get_page on the mfn obtained,
>> while holding the read lock.
>> - 4. Have all lookup callers put_page on the obtained mfn, once done.
>
> This seems like a step in the right direction, but if we're going to
> make this big an interface change there might be better interfaces to
> end up with.
Agree, and hopefully we can converge towards something awesome.

> A few issues I can see with it:
>  - p2m lookups are on some very performance-sensitive paths
>   (e.g. multiple times in any pagetable walk or instruction emulation
>   in a HVM guest) so adding the rwlock might have a noticeable impact.
>
>  - This fixes one class of races (page gets freed-to-xen underfoot) but
>   leaves another one (gfn -> mfn map changes underfoot) untouched.  In
>   particular it doesn't solve the race where a foreign mapper
>   gets a r/w map of what's meant to be a read-only frame.

Can you elaborate a bit? Under what situations does the gfn->mfn map
change underfoot (other than sharing, paging in or superpage
sharding)? Wouldn't those two be taking a writer lock, and thus be
mutually ecxluded from lookups?

Also, the second problem you mention (foregin rw map of ro page) seems
to be a tad different. That fix should go into get_page_from_l1e,
right? Isn't qemu allowed to do this all the time?

> I think that to fix things properly we need to have some refcount
> associated with the p2m mapping itself.  That would be taken by all
> lookups (or at least some - we could have a flag to the p2m lookup) and
> released as you suggest, but more importantly it would block all p2m changes
> while the count was raised.  (I think that a least in the common case we
> could encode such a refcount using the existing typecount).

Assuming mapping means "entry in p2m", multiple mappings would have
their ref count collapse in the page typecount. Isn't that a problem?
Do we need per-mapping refcounts, or rather, per mapping mutual
exclusion? My feel is that page refcounts are necessary to prevent the
page from disappearing, and mappings need to have their lookups and
modifications synchronized.

> One problem then is how to make all the callers of the p2m update
> functions handle failure, either by waiting (new deadlock risk?) or
> returning EAGAIN at the hypercall interface.  Paths where the update
> isn't caused by an explicit request (like log-dirty and the mem-event
> rx-to-rw conversion) would be particularly tricky.

Callers already wait on lock_p2m. They'll wait longer :)

On failure, to cite a specific example, if paging was trying to swap
something out that got foreign-mapped by somebody else, then yeah,
there's no other option than failing that call.

How would log-dirty and x^w fail (if the refcount increases before
they get exclusive access to the mapping)? They're not trying to
change the mapping and/or make a page go away, rather they're changing
the p2m permission.

> More seriously, it introduces another round of the sort of priority
> inversion we already get with the existing refcounts - a foreign
> mapping, caused by a user-space program in another VM, could arbitrarily
> delay a p2m update (and so prevent a VCPU from making progress), without
> any mechanism to even request that the mapping be removed.

Yeah, that's tricky. I do not know if there is a fix at all.
Fundamentally, the foreign mapper (some dom0 sharing/paging/foo
utility) is completely async to the domain. Even if we were to revoke
the foreign mapping as you suggest below, that would involve an upcall
into the foreign-mapper-guest-OS to have it cleanly neuter the mapping
and drop the refcount. That's not at all trivial! Perhaps foreign
mapping vma's should be taught to patiently re-establish mappings if
they disappear under their feet? Event then, you need to keep track of
those foreign l1e's, and nothing short of a list will do.

Because this is performance rather than correctness I'm inclined to
not poke the beast.

> Any ideas how to avoid that?  Potentially with some extra bookkeeping on
> foreign mappings we could revoke or redirect them when the p2m changes.
> That would fit nicely with the abstraction in the interfaces where HVM
> domains' memory is always indexed by pfn.  I can imagine it being quite
> tricky though.
>
>> I'm more wary that turning p2m locking into read/write will result in
>> code deadlocking itself: taking a read lock first and a write lock
>> later. Possibly the current rwlock implementation could be improved to
>> keep a cpumask of read-lockers, and provide an atomic "promote from
>> read to write" atomic operation (something along the lines of wait
>> until you're the only reader in the cpumask, and then cmpxchg(lock,
>> -1, WRITE_BIAS))
>
> I think that would deadlock if two cpus tried it at once.

If you keep a cpumask of all read lockers, and only try to promote if
you're the only read locker, it wouldn't. But then you'd have to
protect the cpumask from races with another spinlock. Yuk.

Which brings me to another question: p2m_locked_by_me (and others)
check on the physical cpu (lock->holder == curent->processor,
roughly). Well, what if you lock and then the vcpu migrates? Are vcpu
migrations prevented if you hold any locks? Or is there some other
magic going on?

Thanks!
Andres
>
> Tim.
>