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

[xen stable-4.13] x86/svm: do not try to handle recalc NPT faults immediately



commit 11ea967c99b90011f371e64ec063bb6e38bd2b6c
Author:     Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
AuthorDate: Wed Jun 24 16:45:59 2020 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Jun 24 16:45:59 2020 +0200

    x86/svm: do not try to handle recalc NPT faults immediately
    
    A recalculation NPT fault doesn't always require additional handling
    in hvm_hap_nested_page_fault(), moreover in general case if there is no
    explicit handling done there - the fault is wrongly considered fatal.
    
    This covers a specific case of migration with vGPU assigned which
    uses direct MMIO mappings made by XEN_DOMCTL_memory_mapping hypercall:
    at a moment log-dirty is enabled globally, recalculation is requested
    for the whole guest memory including those mapped MMIO regions
    which causes a page fault being raised at the first access to them;
    but due to MMIO P2M type not having any explicit handling in
    hvm_hap_nested_page_fault() a domain is erroneously crashed with unhandled
    SVM violation.
    
    Instead of trying to be opportunistic - use safer approach and handle
    P2M recalculation in a separate NPT fault by attempting to retry after
    making the necessary adjustments. This is aligned with Intel behavior
    where there are separate VMEXITs for recalculation and EPT violations
    (faults) and only faults are handled in hvm_hap_nested_page_fault().
    Do it by also unifying do_recalc return code with Intel implementation
    where returning 1 means P2M was actually changed.
    
    Since there was no case previously where p2m_pt_handle_deferred_changes()
    could return a positive value - it's safe to replace ">= 0" with just "== 0"
    in VMEXIT_NPF handler. finish_type_change() is also not affected by the
    change as being able to deal with >0 return value of p2m->recalc from
    EPT implementation.
    
    Signed-off-by: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
    Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    master commit: 51ca66c37371b10b378513af126646de22eddb17
    master date: 2020-06-05 17:12:11 +0200
---
 xen/arch/x86/hvm/svm/svm.c | 5 +++--
 xen/arch/x86/mm/p2m-pt.c   | 7 ++++++-
 xen/arch/x86/mm/p2m.c      | 2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 0cec511b3a..8317ef8fb0 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2947,9 +2947,10 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
             v->arch.hvm.svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
         rc = vmcb->exitinfo1 & PFEC_page_present
              ? p2m_pt_handle_deferred_changes(vmcb->exitinfo2) : 0;
-        if ( rc >= 0 )
+        if ( rc == 0 )
+            /* If no recal adjustments were being made - handle this fault */
             svm_do_nested_pgfault(v, regs, vmcb->exitinfo1, vmcb->exitinfo2);
-        else
+        else if ( rc < 0 )
         {
             printk(XENLOG_G_ERR
                    "%pv: Error %d handling NPF (gpa=%08lx ec=%04lx)\n",
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index eb66077496..2b74726e39 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -341,6 +341,7 @@ static int do_recalc(struct p2m_domain *p2m, unsigned long 
gfn)
     unsigned int level = 4;
     l1_pgentry_t *pent;
     int err = 0;
+    bool recalc_done = false;
 
     table = map_domain_page(pagetable_get_mfn(p2m_get_pagetable(p2m)));
     while ( --level )
@@ -402,6 +403,8 @@ static int do_recalc(struct p2m_domain *p2m, unsigned long 
gfn)
                 clear_recalc(l1, e);
                 err = p2m->write_p2m_entry(p2m, gfn, pent, e, level + 1);
                 ASSERT(!err);
+
+                recalc_done = true;
             }
         }
         unmap_domain_page((void *)((unsigned long)pent & PAGE_MASK));
@@ -448,12 +451,14 @@ static int do_recalc(struct p2m_domain *p2m, unsigned 
long gfn)
             clear_recalc(l1, e);
         err = p2m->write_p2m_entry(p2m, gfn, pent, e, level + 1);
         ASSERT(!err);
+
+        recalc_done = true;
     }
 
  out:
     unmap_domain_page(table);
 
-    return err;
+    return err ?: recalc_done;
 }
 
 int p2m_pt_handle_deferred_changes(uint64_t gpa)
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index ba126f790a..4039630761 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1194,7 +1194,7 @@ static int finish_type_change(struct p2m_domain *p2m,
         rc = p2m->recalc(p2m, gfn);
         /*
          * ept->recalc could return 0/1/-ENOMEM. pt->recalc could return
-         * 0/-ENOMEM/-ENOENT, -ENOENT isn't an error as we are looping
+         * 0/1/-ENOMEM/-ENOENT, -ENOENT isn't an error as we are looping
          * gfn here. If rc is 1 we need to have it 0 for success.
          */
         if ( rc == -ENOENT || rc > 0 )
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.13



 


Rackspace

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