Re: [PATCH v2 for-4.15] x86/msr: introduce an option for HVM relaxed rdmsr behavior

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Mar 05, 2021 at 11:56:33AM +0100, Jan Beulich wrote:
> On 04.03.2021 15:47, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -1795,6 +1795,7 @@ static int svm_msr_read_intercept(unsigned int msr, 
> > uint64_t *msr_content)
> >      const struct domain *d = v->domain;
> >      struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
> >      const struct nestedsvm *nsvm = &vcpu_nestedsvm(v);
> > +    uint64_t tmp;
> >  
> >      switch ( msr )
> >      {
> > @@ -1965,6 +1966,11 @@ static int svm_msr_read_intercept(unsigned int msr, 
> > uint64_t *msr_content)
> >          break;
> >  
> >      default:
> > +        if ( d->arch.hvm.rdmsr_relaxed && !rdmsr_safe(msr, tmp) )
> > +        {
> > +            *msr_content = 0;
> > +            break;
> > +        }
> 
> You don't really need "tmp" here, do you? You could as well read
> into *msr_content, as you're zapping the value afterwards anyway.

I also thought about doing this, but felt unease. I fear the code
might be changed in the future and maybe msr_content is not zapped
anymore, thus leaking the content. I feel it's safer to use a
temporary variable that will never be returned to the guest. Maybe
I'm just too paranoid.

Thanks, Roger.