shadow OOS and fast path are incompatible

shadow OOS and fast path are incompatible

[Frank van der Linden]

We recently observed a problem with Solaris HVM domains. The bug was 
seen was seen with a higher number of VCPUs (3 or more), and always had 
the same pattern: some memory was allocated in the guest, but the first 
reference caused it to crash with a fatal pagefault. However, on 
inspection of the page tables, the guests' view of the pagetables was 
consistent: the page was present.

Disabling the out-of-sync optimization made this problem go away.

Eventually, I tracked it down to the fault fast path and the OOS code in 
sh_page_fault(). Here's what happens:

* CPU 0 has a page fault for a PTE in an OOS page that hasn't been 
synched yet
* CPU 1 has the same page fault (or at least one involving the same L1 page)
* CPU 1 enters the fast path
* CPU 0 finds the L1 page OOS and starts a resync
* CPU 1 finds it's a "special" entry (mmio or gnp)
* CPU 0 finishes resync, clears OOS flag for the L1 page
* CPU 1 finds it's not an OOS L1 page
* CPU 1 finds that the shadow L1 entry is GNP
* CPU 1 bounces fault to guest (sh_page_fault returns 0)
* guest sees an unexpected page fault

There are certainly ways to rearrange the code to avoid this particular 
scenario, but it points to a bigger issue: the fast fault path and OOS 
pages are inherently incompatible. Since the fast path works outside of 
the shadow lock, there is nothing that prevents another CPU coming in 
and changing the OOS status, re-syncing the page, etc, right under your 
nose.

Optimized operations without OOS (i.e. on a single L1 PTE) are safe in 
the fast path outside of the lock, since the guest will have the 
appropriate locking around the PTE writes. But with OOS, you're dealing 
with an entire L1 page.

I haven't checked the fast emulation path, but similar problems might be 
lurking there in combination with OOS.

I can think of some ways to fix this, but they involve locking, which 
mostly defeats the purpose of the fast fault path.

Ideas/suggestions?

- Frank

Re: shadow OOS and fast path are incompatible

[Gianluca Guida]

Hi,

On Thu, Jul 2, 2009 at 10:30 PM, Frank van der
Linden<Frank.Vanderlinden@sun.com> wrote:
> We recently observed a problem with Solaris HVM domains. The bug was seen
> was seen with a higher number of VCPUs (3 or more), and always had the same
> pattern: some memory was allocated in the guest, but the first reference
> caused it to crash with a fatal pagefault. However, on inspection of the
> page tables, the guests' view of the pagetables was consistent: the page was
> present.
>
> Disabling the out-of-sync optimization made this problem go away.
>
> Eventually, I tracked it down to the fault fast path and the OOS code in
> sh_page_fault(). Here's what happens:
>
> * CPU 0 has a page fault for a PTE in an OOS page that hasn't been synched
> yet
> * CPU 1 has the same page fault (or at least one involving the same L1 page)
> * CPU 1 enters the fast path
> * CPU 0 finds the L1 page OOS and starts a resync

CPU0 doesn't resync a whole L1 page because it's accessing it. There
are other reasons for a resync here (especially if the guest is 64
bit), but most probably the resync happen because CPU0 is unsyncing
another page. Anyway yes, it's highly unlikely but this race can
definitely happen. I think I never saw it.

> * CPU 1 finds it's a "special" entry (mmio or gnp)
> * CPU 0 finishes resync, clears OOS flag for the L1 page
> * CPU 1 finds it's not an OOS L1 page
> * CPU 1 finds that the shadow L1 entry is GNP
> * CPU 1 bounces fault to guest (sh_page_fault returns 0)
> * guest sees an unexpected page fault
>
> There are certainly ways to rearrange the code to avoid this particular
> scenario, but it points to a bigger issue: the fast fault path and OOS pages
> are inherently incompatible. Since the fast path works outside of the shadow
> lock, there is nothing that prevents another CPU coming in and changing the
> OOS status, re-syncing the page, etc, right under your nose.

You're right about this, the gnp fast path has always brought me some
doubt and I already intended to remove it because it's also useless as
an optimization with OOS (can actually slow down things in a classical
demand-paging scheme), but I never came to see what you just pointed
out.

> I haven't checked the fast emulation path, but similar problems might be
> lurking there in combination with OOS.

I think that it should be safe enough, but yes another look at it
should be worth it.

> I can think of some ways to fix this, but they involve locking, which mostly
> defeats the purpose of the fast fault path.
>
> Ideas/suggestions?

Removing the fast GNP and leaving only fast mmio (it's safe and useful
for performances). I have this trivial patch somewhere, I'll post it
ASAP.

Thanks for tracking this down!
Gianluca


-- 
It was a type of people I did not know, I found them very strange and
they did not inspire confidence at all. Later I learned that I had been
introduced to electronic engineers.
                                                  E. W. Dijkstra

Re: shadow OOS and fast path are incompatible

[Tim Deegan]

Hi, 

At 22:42 +0100 on 02 Jul (1246574577), Gianluca Guida wrote:
> CPU0 doesn't resync a whole L1 page because it's accessing it. There
> are other reasons for a resync here (especially if the guest is 64
> bit), but most probably the resync happen because CPU0 is unsyncing
> another page. Anyway yes, it's highly unlikely but this race can
> definitely happen. I think I never saw it.

Yes.  The fast-not-present path is safe without OOS because the changing
L1 would have to be as a result of the guest writing it, which is a race
on real hardware.   The fast-MMIO path is OK even with OOS because it
only caches present entries, but for proper safety we might want to
avoid using fast-path MMIO if the guest maps MMIO space read-only (does
anyone actually do that?)

> > I haven't checked the fast emulation path, but similar problems might be
> > lurking there in combination with OOS.
> 
> I think that it should be safe enough, but yes another look at it
> should be worth it.

I think you might as well kill the fast emulation path too, while you're
in there. :)  The OOS optimization makes it mostly redundant, and it
just adds complexity now.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

Re: shadow OOS and fast path are incompatible

[Gianluca Guida]

Hi,

On Fri, Jul 3, 2009 at 10:50 AM, Tim Deegan<Tim.Deegan@citrix.com> wrote:
> At 22:42 +0100 on 02 Jul (1246574577), Gianluca Guida wrote:
>> CPU0 doesn't resync a whole L1 page because it's accessing it. There
>> are other reasons for a resync here (especially if the guest is 64
>> bit), but most probably the resync happen because CPU0 is unsyncing
>> another page. Anyway yes, it's highly unlikely but this race can
>> definitely happen. I think I never saw it.
>
> Yes.  The fast-not-present path is safe without OOS because the changing
> L1 would have to be as a result of the guest writing it, which is a race
> on real hardware.   The fast-MMIO path is OK even with OOS because it
> only caches present entries, but for proper safety we might want to
> avoid using fast-path MMIO if the guest maps MMIO space read-only (does
> anyone actually do that?)

Yes, the subject of this thread is incorrect, OOS is incompatible only
with *current* fast gnp code. This bug is easily fixable by checking
if the page is out of sync (and go to the fast path) *before* checking
for the l1 being a special entry. This will remove the race and make
the fast gnp correct again with OOS -- I guess, but I perhaps
shouldn't write about shadow SMP races before breakfast.

But, as I said, there's really no point in keeping the fast gnp path
there already, so I'd be for removing.

>> > I haven't checked the fast emulation path, but similar problems might be
>> > lurking there in combination with OOS.
>>
>> I think that it should be safe enough, but yes another look at it
>> should be worth it.
>
> I think you might as well kill the fast emulation path too, while you're
> in there. :)  The OOS optimization makes it mostly redundant, and it
> just adds complexity now.

Yes, that was on the list as well. Yet, on a correctness point of
view, I think it's safe -- again, no-caffeine warning. The fast
emulation is very simple, and activate in a very particular case: two
emulation in a row, without any other pagefault in between.

Thanks,
Gianluca


-- 
It was a type of people I did not know, I found them very strange and
they did not inspire confidence at all. Later I learned that I had been
introduced to electronic engineers.
                                                  E. W. Dijkstra

Re: shadow OOS and fast path are incompatible

[Gianluca Guida]

About the read-only MMIO,

On Fri, Jul 3, 2009 at 10:50 AM, Tim Deegan<Tim.Deegan@citrix.com> wrote:
> Yes.  The fast-not-present path is safe without OOS because the changing
> L1 would have to be as a result of the guest writing it, which is a race
> on real hardware.   The fast-MMIO path is OK even with OOS because it
> only caches present entries, but for proper safety we might want to
> avoid using fast-path MMIO if the guest maps MMIO space read-only (does
> anyone actually do that?)

That is still correct. The fast path gets only the gfn + offset and
pass it to the mmio handling code, that will check the permission by
using proper locks.

Gianluca

-- 
It was a type of people I did not know, I found them very strange and
they did not inspire confidence at all. Later I learned that I had been
introduced to electronic engineers.
                                                  E. W. Dijkstra