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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.09.2020 10:15, Roger Pau Monné wrote:
> 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?

I agree. And hence while moving it I think you want to also adjust
that comment.

Jan