> -----Original Message----- > From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of > Paul Durrant > Sent: 20 March 2017 11:50 > To: 'Jan Beulich' <jbeul...@suse.com> > Cc: Andrew Cooper <andrew.coop...@citrix.com>; xen- > de...@lists.xenproject.org > Subject: Re: [Xen-devel] [PATCH 2/7] x86/viridian: fix xen-hvmcrash when > vp_assist page is present > > > -----Original Message----- > > From: Jan Beulich [mailto:jbeul...@suse.com] > > Sent: 20 March 2017 11:36 > > To: Paul Durrant <paul.durr...@citrix.com> > > Cc: Andrew Cooper <andrew.coop...@citrix.com>; xen- > > de...@lists.xenproject.org > > Subject: Re: [PATCH 2/7] x86/viridian: fix xen-hvmcrash when vp_assist > page > > is present > > > > >>> On 17.03.17 at 10:57, <paul.durr...@citrix.com> 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.
Actually, looking at this again, I'm not sure there's any point in making this a gfn_t. It's only stored for the purposes of an identity match and the thing it matches with is an unsigned long extracted from an MSR bit-field. Paul > > Paul > > > Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
