| Message ID | 1490029701-4311-2-git-send-email-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 20.03.17 at 18:08, <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. The patch looks fine to me, but not in line with the description: There's no domain_crash() being eliminated anymore. Jan
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 21 March 2017 15:27 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; xen- > devel@lists.xenproject.org > Subject: Re: [PATCH v2 1/6] x86/viridian: fix xen-hvmcrash when vp_assist > page is present > > >>> On 20.03.17 at 18:08, <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. > > The patch looks fine to me, but not in line with the description: > There's no domain_crash() being eliminated anymore. Oh. That hunk must have been dropped for some reason. It should be there. I'll send v3. Paul > > Jan
diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c index f2c9613..3a2611b 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. @@ -904,7 +906,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;
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> v2: - Patch significantly simplified --- xen/arch/x86/hvm/viridian.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)