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.
