Re: [Xen-devel] [PATCH 2/7] x86/viridian: fix xen-hvmcrash when vp_assist page is present

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 20 March 2017 11:36
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/7] x86/viridian: fix xen-hvmcrash when vp_assist page
> is present
> 
> >>> On 17.03.17 at 10:57, <paul.durrant@xxxxxxxxxx> wrote:
> > @@ -288,6 +304,14 @@ static void initialize_vp_assist(struct vcpu *v)
> >       * enlightenment.
> >       */
> >
> > +    if ( v->arch.hvm_vcpu.viridian.vp_assist.va )
> > +    {
> > +        if ( v->arch.hvm_vcpu.viridian.vp_assist.gmfn == gmfn )
> > +            return;
> 
> Is this shortcut valid? I.e. is it not valid for the guest to expect the
> VP assist state to be fully reset if it calls this more than once on a
> vCPU, yet possibly with the same GFN? (It also looks like this isn't
> really part of the corrections you want to make here, according to
> the description.)

Hmm. The spec is not clear. The problem is that doing a save-context followed 
by restore-context is going through this path. Maybe it's best to leave the 
teardown in the MSR right and special-case a restore when vp_assist_va is set.

> 
> > --- a/xen/include/asm-x86/hvm/viridian.h
> > +++ b/xen/include/asm-x86/hvm/viridian.h
> > @@ -23,6 +23,7 @@ struct viridian_vcpu
> >  {
> >      struct {
> >          union viridian_vp_assist msr;
> > +        unsigned long gmfn;
> 
> gfn_t ?
> 

Yes, you're right. I should probably precede this with a patch fixing up the 
gmfn stack variables in viridian.c to use gfn_t for consistency though.

  Paul

> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel