Re: [PATCH 2/8] x86/svm: silently drop writes to SYSCFG and related MSRs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.08.2020 16:53, Andrew Cooper wrote:
> On 17/08/2020 16:57, Roger Pau Monne wrote:
>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>> index ca3bbfcbb3..671cdcb724 100644
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -1917,6 +1917,13 @@ static int svm_msr_read_intercept(unsigned int msr, 
>> uint64_t *msr_content)
>>              goto gpf;
>>          break;
>>  
>> +    case MSR_K8_TOP_MEM1:
>> +    case MSR_K8_TOP_MEM2:
>> +    case MSR_K8_SYSCFG:
>> +        /* Return all 0s. */
>> +        *msr_content = 0;
> 
> On a Rome box, these are the normal values:
> 
> 0xc0010010 (SYSCFG) 0x740000
> 0xc001001a (MEM1) 0xb0000000
> 0xc001001d (MEM2) 0x3c50000000
> 
> The SYSCFG bits are MtrrFixDramEn[18], MtrrVarDramEn[20], MtrrTom2En[21]
> and Tom2ForceMemTypeWB[22].  In particular, bits 18 and 20 are expected
> to be set by the system firmware.
> 
> Clearly we shouldn't be leaking TOP_MEM{1,2} into guests, but Xen also
> doesn't have enough information to set these correctly yet either.
> 
> At a minimum, I think SYSCFG wants to report 0x40000 which means "the
> fixed MTRRs behave as expected",

How do you derive that meaning? The bit tells us whether in the
individual fixed range MTRR bytes bits 3 and 4 have an AMD-
specific meaning. Guests reading these will get back zero for
the bits in all cases right now, and hence the meaning would be
"MMIO" for all of them, despite there being some RAM ranges
covered as well. Imo the bit needs to be zero to be compatible
with the rest of our code.

Jan

> and the other bits being clear should mean that TOP_MEM{1,2} aren't enabled.
> 
> ~Andrew
>