[Xen-changelog] [xen stable-4.9] x86/mm: drop bogus paging mode assertion

[patchbot@xxxxxxx]

commit c09e166b687b4753c47e6b486b008355315c9031
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Wed Dec 20 15:57:59 2017 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Dec 20 15:57:59 2017 +0100

    x86/mm: drop bogus paging mode assertion
    
    Olaf has observed this assertion to trigger after an aborted migration
    of a PV guest:
    
    (XEN) Xen call trace:
    (XEN)    [<ffff82d0802a85dc>] do_page_fault+0x39f/0x55c
    (XEN)    [<ffff82d08036b7d8>] 
x86_64/entry.S#handle_exception_saved+0x66/0xa4
    (XEN)    [<ffff82d0802a9274>] __copy_to_user_ll+0x22/0x30
    (XEN)    [<ffff82d0802772d4>] update_runstate_area+0x19c/0x228
    (XEN)    [<ffff82d080277371>] domain.c#_update_runstate_area+0x11/0x39
    (XEN)    [<ffff82d080277596>] context_switch+0x1fd/0xf25
    (XEN)    [<ffff82d0802395c5>] schedule.c#schedule+0x303/0x6a8
    (XEN)    [<ffff82d08023d067>] softirq.c#__do_softirq+0x6c/0x95
    (XEN)    [<ffff82d08023d0da>] do_softirq+0x13/0x15
    (XEN)    [<ffff82d08036b2f1>] x86_64/entry.S#process_softirqs+0x21/0x30
    
    Release builds work fine, which is a first indication that the assertion
    isn't really needed.
    
    What's worse though - there appears to be a timing window where the
    guest runs in shadow mode, but not in log-dirty mode, and that is what
    triggers the assertion (the same could, afaict, be achieved by test-
    enabling shadow mode on a PV guest). This is because turing off log-
    dirty mode is being performed in two steps: First the log-dirty bit gets
    cleared (paging_log_dirty_disable() [having paused the domain] ->
    sh_disable_log_dirty() -> shadow_one_bit_disable()), followed by
    unpausing the domain and only then clearing shadow mode (via
    shadow_test_disable(), which pauses the domain a second time).
    
    Hence besides removing the ASSERT() here (or optionally replacing it by
    explicit translate and refcounts mode checks, but this seems rather
    pointless now that the three are tied together) I wonder whether either
    shadow_one_bit_disable() should turn off shadow mode if no other bit
    besides PG_SH_enable remains set (just like shadow_one_bit_enable()
    enables it if not already set), or the domain pausing scope should be
    extended so that both steps occur without the domain getting a chance to
    run in between.
    
    Reported-by: Olaf Hering <olaf@xxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Tim Deegan <tim@xxxxxxx>
    Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    master commit: b95f7be32d668fa4b09300892ebe19636ecebe36
    master date: 2017-12-12 16:56:15 +0100
---
 xen/arch/x86/traps.c         | 6 +-----
 xen/include/asm-x86/paging.h | 3 ---
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e861f7a..5e187b1 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1486,12 +1486,8 @@ static int fixup_page_fault(unsigned long addr, struct 
cpu_user_regs *regs)
      */
     if ( paging_mode_enabled(d) && !paging_mode_external(d) )
     {
-        int ret;
+        int ret = paging_fault(addr, regs);
 
-        /* Logdirty mode is the only expected paging mode for PV guests. */
-        ASSERT(paging_mode_only_log_dirty(d));
-
-        ret = paging_fault(addr, regs);
         if ( ret == EXCRET_fault_fixed )
             trace_trap_two_addr(TRC_PV_PAGING_FIXUP, regs->rip, addr);
         return ret;
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index f262c9e..5bce5e5 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -69,9 +69,6 @@
 #define paging_mode_translate(_d) (!!((_d)->arch.paging.mode & PG_translate))
 #define paging_mode_external(_d)  (!!((_d)->arch.paging.mode & PG_external))
 
-#define paging_mode_only_log_dirty(_d)                  \
-    (((_d)->arch.paging.mode & PG_MASK) == PG_log_dirty)
-
 /* flags used for paging debug */
 #define PAGING_DEBUG_LOGDIRTY 0
 
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.9

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog