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

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

On Tue, Aug 18, 2020 at 03:53:16PM +0100, 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", and the other bits being clear should
> mean that TOP_MEM{1,2} aren't enabled.

I didn't enable MtrrFixDramEn because AFAICT the emulated MTRR
implementation doesn't support the usage of the Extended type-field
format, and hence those bits will be 0. I'm fine with returning having
it set, as long as we don't allow setting MtrrFixDramModEn[19].

Thanks, Roger.