Re: [Xen-devel] [PATCH v3 1/4] x86: stop handling MSR_IA32_BNDCFGS save/restore in implementation code

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 14 March 2019 13:39
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Roger Pau Monne 
> <roger.pau@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Kevin Tian 
> <kevin.tian@xxxxxxxxx>; xen-
> devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v3 1/4] x86: stop handling MSR_IA32_BNDCFGS save/restore 
> in implementation code
> 
> >>> On 14.03.19 at 14:01, <paul.durrant@xxxxxxxxxx> wrote:
> > @@ -1215,10 +1196,15 @@ static bool vmx_set_guest_bndcfgs(struct vcpu *v, 
> > u64 val)
> >      return true;
> >  }
> >
> > -static bool vmx_get_guest_bndcfgs(struct vcpu *v, u64 *val)
> > +static bool vmx_get_guest_bndcfgs(const struct vcpu *cv, u64 *val)
> >  {
> > +    struct vcpu *v;
> > +
> >      ASSERT(cpu_has_mpx && cpu_has_vmx_mpx);
> >
> > +    /* Get a non-const pointer for vmx_vmcs_enter() */
> > +    v = cv->domain->vcpu[cv->vcpu_id];
> 
> Any chance this could be made the initializer of the variable?

I thought it looked slightly odd having a comment above the declaration, which 
is why I did it this way, but I can move it.

> 
> > --- a/xen/arch/x86/msr.c
> > +++ b/xen/arch/x86/msr.c
> > @@ -160,6 +160,12 @@ int guest_rdmsr(const struct vcpu *v, uint32_t msr, 
> > uint64_t *val)
> >          ret = guest_rdmsr_x2apic(v, msr, val);
> >          break;
> >
> > +    case MSR_IA32_BNDCFGS:
> > +        if ( !cp->feat.mpx || !hvm_get_guest_bndcfgs(v, val) )
> > +            goto gp_fault;
> 
> Didn't we settle on ASSERT(is_hvm_*)?
> 
> > @@ -323,6 +329,12 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t 
> > val)
> >          ret = guest_wrmsr_x2apic(v, msr, val);
> >          break;
> >
> > +    case MSR_IA32_BNDCFGS:
> > +        if ( !cp->feat.mpx || !hvm_set_guest_bndcfgs(v, val) )
> > +            goto gp_fault;
> 
> Same here? Or did Andrew tell you otherwise?

Andrew was ok with me just dropping it, but I can put in the ASSERT if you 
prefer.

  Paul

> 
> Jan
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel