[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Re: Possible regression in "x86-64: reduce range spanned by 1:1 mapping and frame table indexes"



Hi,

I seem to have missed this thread earlier; sorry about that. 

At 01:39 +0000 on 11 Dec (1260495591), Simon Horman wrote:
> * The memory access is to a MMIO region - a control register for the NIC.
>   The access is made by the e1000 gPXE driver.
> 
> * Interestingly, if there is a write to the page (or perhaps even anywhere
>   in the entire 20-page MMIO region?) before a read, the problem doesn't
>   occur. So a simple work-around in the gPXE driver is to just write to a
>   register before reading any.
> 
> * In sh_page_fault() the call to gfn_to_mfn_guest_dbg(d, gfn, &p2mt) sets
>   p2mt to p2m_mmio_direct, which seems to be correct. I'm a bit
>   stuck in working out what goes wrong from there.

So from the trace below it looks like the shadow fault handler is trying
to emulate an access to a passthrough MMIO address.  That's surprising. 
And if it only happens when the first access is a read, that's a bit 
suspect too. 

> 106706 sh: sh_page_fault__guest_2(): d:v=1:0 va=0xf30000d8 err=2, rip=6f6

Oh, but this is a write, according to the error code. 

> 106707 sh: sh_page_fault__guest_2(): 2954: A
> 106708 sh: sh_page_fault__guest_2(): 2998: B
> 106709 sh: sh_page_fault__guest_2(): 3077: page_fault_slow_path:
> 106710 sh: sh_page_fault__guest_2(): 3081: C
> 106711 sh: sh_page_fault__guest_2(): 3095: rewalk:
> 106712 sh: sh_page_fault__guest_2(): 3097: D
> 106713 sh: sh_page_fault__guest_2(): 3106: E
> 106714 sh: sh_page_fault__guest_2(): 3122: F
> (XEN) _gfn_to_mfn_type_dbg: current
> 106715 sh: sh_page_fault__guest_2(): 3141: gfn_to_mfn_guest_dbg: p2mt=5
> 106716 sh: sh_page_fault__guest_2(): 3143: G
> 106717 sh: sh_page_fault__guest_2(): 3181: H
> 106718 sh: sh_page_fault__guest_2(): 3193: I
> 106719 sh: sh_page_fault__guest_2(): 3204: J
> 106720 sh: sh_page_fault__guest_2(): 3216: K
> 106721 shdebug: make_fl1_shadow(): (f3000)=>118644

FL1 shadow means we're building a shadow L1 that's equivalent to a
single PSE guest entry, pointing a guest frames f3000 to f31ff.

> 106722 sh: set_fl1_shadow_status(): gfn=f3000, type=00000002, smfn=118644
> 106723 shdebug: _sh_propagate(): demand write level 2 guest f30000e7 shadow 
> 0000000118644067
> 106724 sh: sh_page_fault__guest_2(): 3241: L
> 106725 shdebug: _sh_propagate(): demand write level 1 guest f3000067 shadow 
> 00000000f0500037
> 106726 sh: sh_page_fault__guest_2(): 3263: M
> 106727 shdebug: _sh_propagate(): prefetch level 1 guest f3001067 shadow 
> 00000000f0501037
> 106728 shdebug: _sh_propagate(): prefetch level 1 guest f3002067 shadow 
> 00000000f0502037

[...] and the shadow PTE flags look OK: A, PCD, U/S, R/W, P.

> 106758 sh: sh_page_fault__guest_2(): 3285: N
> 106759 sh: sh_page_fault__guest_2(): 3310: O
> 106760 sh: sh_page_fault__guest_2(): 3319: P
> 106761 sh: sh_page_fault__guest_2(): 3332: Q
> 106762 sh: sh_page_fault__guest_2(): 3343: goto emulate;

That's the interesting one.  I guess your line numbers are different
because of the extra printout but it must be this:

    if ( is_hvm_domain(d) 
         && unlikely(!hvm_wp_enabled(v)) 
         && regs->error_code == (PFEC_write_access|PFEC_page_present) )
    {
        perfc_incr(shadow_fault_emulate_wp);
        goto emulate;
    }

Hah.  That explains why we're emulating.  The guest has CR0.WP clear,
and this was a write fault that wouldn't have happened on real hardware,
so we need to emulate it because we can't actually disable CR0.WP or the
shadow pagetables stop working altogether.

In fact in this case we don't need to emulate it because retrying would
be good enough, but in the general case we might.

> 106763 sh: sh_page_fault__guest_2(): 3361: emulate:
> 106764 sh: sh_page_fault__guest_2(): 3367: R
> 106765 sh: sh_page_fault__guest_2(): 3390: emulate_readonly:
> 106766 sh: sh_page_fault__guest_2(): 3403: early_emulation:
> 106767 sh: sh_page_fault__guest_2(): 3405: S
> 106768 sh: sh_page_fault__guest_2(): emulate: eip=0x6f6 esp=0x3d264
> 106769 sh: sh_page_fault__guest_2(): 3446: T
> 106770 sh: sh_page_fault__guest_2(): emulator failure, unshadowing mfn 0xf0500

The emulation fails (because the emulator can't/won't map MMIO space)
and we're about to bail out, try unshadowing everything and retry (at
which point everything will just work).  Except:

> 106771 sh: sh_remove_shadows(): d=1, v=0, gmfn=f0500

sh_remove_shadows isn't ready to be called with an MMIO MFN.  We used to
get away with this before the m2p was made sparse, but now MMIO holes
mean m2p holes. 

Two fixes suggest themselves.  The first is to gate the CR0.WP emulation
path on mfn_valid().  The emulator won't handle it so it only leads to
confusion and delay if we try:

--- a/xen/arch/x86/mm/shadow/multi.c    Fri Dec 11 09:17:09 2009 +0000
+++ b/xen/arch/x86/mm/shadow/multi.c    Fri Dec 11 10:06:39 2009 +0000
@@ -3305,7 +3305,8 @@
      * fault was a non-user write to a present page.  */
     if ( is_hvm_domain(d) 
          && unlikely(!hvm_wp_enabled(v)) 
-         && regs->error_code == (PFEC_write_access|PFEC_page_present) )
+         && regs->error_code == (PFEC_write_access|PFEC_page_present) 
+         && mfn_valid(gmfn) )
     {
         perfc_incr(shadow_fault_emulate_wp);
         goto emulate;


The second is to make sh_remove_shadows() safe against being called with
a bogus MFN.  I'm not quite decided whether the right answer is to put 
"if (!mfn_valid(gmfn)) return;" at the top of it (which would stop a
certain class of crashes) or just "ASSERT(gmfn_valid(gmfn));" to make it
clearer what's gone wrong.  

If neither of those works we may have to make the emulator handle actual
MMIO but that scares me.

I'm away for the next week, but if this is still broken after that I'll
have another go at it.

Cheers,

Tim.

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

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.