diff mbox

[v3,1/6] x86/viridian: fix xen-hvmcrash when vp_assist page is present

Message ID 1490120242-3587-2-git-send-email-paul.durrant@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Durrant March 21, 2017, 6:17 p.m. UTC 
Currently use of xen-hvmcrash will cause an immediate domain_crash() in
initialize_vp_assist() because it is called from viridian_load_vcpu_ctxt()
without having first cleared any previous mapping.

This patch addes a check into viridian_load_vcpu_ctxt() to avoid re-
initialization and turned the domain_crash() in initialize_vp_assist()
into an ASSERT() since neither codepath into that function should allow
it to be hit.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v3:
 - Slightly too much simplification in v2... missing hunk re-instated

v2:
 - Patch significantly simplified
---
 xen/arch/x86/hvm/viridian.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

Comments

Jan Beulich March 22, 2017, 9:57 a.m. UTC | #1 
>>> On 21.03.17 at 19:17, <paul.durrant@citrix.com> wrote:
> Currently use of xen-hvmcrash will cause an immediate domain_crash() in
> initialize_vp_assist() because it is called from viridian_load_vcpu_ctxt()
> without having first cleared any previous mapping.
> 
> This patch addes a check into viridian_load_vcpu_ctxt() to avoid re-
> initialization and turned the domain_crash() in initialize_vp_assist()
> into an ASSERT() since neither codepath into that function should allow
> it to be hit.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index f2c9613..a71f928 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -283,6 +283,8 @@  static void initialize_vp_assist(struct vcpu *v)
     struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
     void *va;
 
+    ASSERT(!v->arch.hvm_vcpu.viridian.vp_assist.va);
+
     /*
      * See section 7.8.7 of the specification for details of this
      * enlightenment.
@@ -306,14 +308,6 @@  static void initialize_vp_assist(struct vcpu *v)
 
     clear_page(va);
 
-    /*
-     * If we overwrite an existing address here then something has
-     * gone wrong and a domain page will leak. Instead crash the
-     * domain to make the problem obvious.
-     */
-    if ( v->arch.hvm_vcpu.viridian.vp_assist.va )
-        domain_crash(d);
-
     v->arch.hvm_vcpu.viridian.vp_assist.va = va;
     return;
 
@@ -904,7 +898,8 @@  static int viridian_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
         return -EINVAL;
 
     v->arch.hvm_vcpu.viridian.vp_assist.msr.raw = ctxt.vp_assist_msr;
-    if ( v->arch.hvm_vcpu.viridian.vp_assist.msr.fields.enabled )
+    if ( v->arch.hvm_vcpu.viridian.vp_assist.msr.fields.enabled &&
+         !v->arch.hvm_vcpu.viridian.vp_assist.va )
         initialize_vp_assist(v);
 
     v->arch.hvm_vcpu.viridian.vp_assist.vector = ctxt.vp_assist_vector;