Re: [PATCH v3 4/8] x86/svm: handle BU_CFG and BU_CFG2 with cases

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

On Wed, Sep 02, 2020 at 10:02:33PM +0100, Andrew Cooper wrote:
> On 01/09/2020 11:54, Roger Pau Monne wrote:
> > @@ -1942,19 +1966,6 @@ static int svm_msr_read_intercept(unsigned int msr, 
> > uint64_t *msr_content)
> >      default:
> >          if ( rdmsr_safe(msr, *msr_content) == 0 )
> >              break;
> > -
> > -        if ( boot_cpu_data.x86 == 0xf && msr == MSR_F10_BU_CFG )
> > -        {
> > -            /* Win2k8 x64 reads this MSR on revF chips, where it
> > -             * wasn't publically available; it uses a magic constant
> > -             * in %rdi as a password, which we don't have in
> > -             * rdmsr_safe().  Since we'll ignore the later writes,
> > -             * just use a plausible value here (the reset value from
> > -             * rev10h chips) if the real CPU didn't provide one. */
> > -            *msr_content = 0x0000000010200020ull;
> > -            break;
> > -        }
> > -
> >          goto gpf;
> >      }
> >  
> > @@ -2110,6 +2121,12 @@ static int svm_msr_write_intercept(unsigned int msr, 
> > uint64_t msr_content)
> >          nsvm->ns_msr_hsavepa = msr_content;
> >          break;
> >  
> > +    case MSR_F10_BU_CFG:
> > +    case MSR_F10_BU_CFG2:
> > +        if ( rdmsr_safe(msr, msr_content) )
> > +            goto gpf;
> 
> The comment you've moved depends on this not having this behaviour, as
> you'll now hand #GP back to Win2k8 on its write.

Oh, unless I'm very confused the comment was already wrong, since
svm_msr_write_intercept previous to this patch would return a #GP when
trying to write to BU_CFG if the underlying MSR is not readable, so
this just keeps the previous behavior.

Looking at the original commit (338db98dd8d) it seems the intention
was to only handle reads and let writes return a #GP?

Maybe Win2k8 would only read the MSR?

> Honestly, given that how obsolete both Win2k8 and K10's are, I'm
> seriously tempted to suggest dropping this workaround entirely.

I'm fine to just drop the workaround, but it seems quite trivial to
just keep it as is (reads returns a know value, writes #GP). I can
adjust the comment to be clearer.

Thanks, Roger.