Xen devel - Mar 2012 - [PATCH] xen: Fake out the Bus Unit Config MSR on revF AMD CPUs

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.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1516,6 +1516,18 @@ static int svm_msr_read_intercept(unsign
         if ( rdmsr_safe(msr, *msr_content) == 0 )
             break;
 
+        if ( 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;
     }
 
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -211,6 +211,9 @@
 #define MSR_F10_MC4_MISC2		0xc0000409
 #define MSR_F10_MC4_MISC3		0xc000040A
 
+/* AMD Family10h MMU control MSRs */
+#define MSR_F10_BU_CFG                  0xc0011023
+
 /* Other AMD Fam10h MSRs */
 #define MSR_FAM10H_MMIO_CONF_BASE	0xc0010058
 #define FAM10H_MMIO_CONF_ENABLE         (1<<0)

>>> On 07.03.12 at 18:59, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> 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.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1516,6 +1516,18 @@ static int svm_msr_read_intercept(unsign
>          if ( rdmsr_safe(msr, *msr_content) == 0 )
>              break;
>  
> +        if ( msr == MSR_F10_BU_CFG )
As you''re aiming at revF only, shouldn''t you check at least
the CPU
family here? With the MSR being model specific, you otherwise risk
to return (bogus) data for something completely different on future
CPU families.

Jan
> +        {
> +            /* 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;
>      }
>  
> diff --git a/xen/include/asm-x86/msr-index.h
b/xen/include/asm-x86/msr-index.h
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -211,6 +211,9 @@
>  #define MSR_F10_MC4_MISC2		0xc0000409
>  #define MSR_F10_MC4_MISC3		0xc000040A
>  
> +/* AMD Family10h MMU control MSRs */
> +#define MSR_F10_BU_CFG                  0xc0011023
> +
>  /* Other AMD Fam10h MSRs */
>  #define MSR_FAM10H_MMIO_CONF_BASE	0xc0010058
>  #define FAM10H_MMIO_CONF_ENABLE         (1<<0)
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel

On Thu, 2012-03-08 at 10:14 +0000, Jan Beulich wrote:
> >>> On 07.03.12 at 18:59, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> > 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.
> > 
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> > 
> > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -1516,6 +1516,18 @@ static int svm_msr_read_intercept(unsign
> >          if ( rdmsr_safe(msr, *msr_content) == 0 )
> >              break;
> >  
> > +        if ( msr == MSR_F10_BU_CFG )
> 
> As you''re aiming at revF only, shouldn''t you check at
least the CPU
> family here? With the MSR being model specific, you otherwise risk
> to return (bogus) data for something completely different on future
> CPU families.
I think that makes sense, but a quick glance through arch/x86/cpu/amd.c
doesn''t suggest to me the most concise way of making that test... could
you give me a suggestion?

Thanks,
 -George

>>> On 08.03.12 at 12:13, George Dunlap
<george.dunlap@citrix.com> wrote:
> On Thu, 2012-03-08 at 10:14 +0000, Jan Beulich wrote:
>> >>> On 07.03.12 at 18:59, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
>> > 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.
>> > 
>> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> > 
>> > diff --git a/xen/arch/x86/hvm/svm/svm.c
b/xen/arch/x86/hvm/svm/svm.c
>> > --- a/xen/arch/x86/hvm/svm/svm.c
>> > +++ b/xen/arch/x86/hvm/svm/svm.c
>> > @@ -1516,6 +1516,18 @@ static int svm_msr_read_intercept(unsign
>> >          if ( rdmsr_safe(msr, *msr_content) == 0 )
>> >              break;
>> >  
>> > +        if ( msr == MSR_F10_BU_CFG )
>> 
>> As you''re aiming at revF only, shouldn''t you check at
least the CPU
>> family here? With the MSR being model specific, you otherwise risk
>> to return (bogus) data for something completely different on future
>> CPU families.
> 
> I think that makes sense, but a quick glance through arch/x86/cpu/amd.c
> doesn''t suggest to me the most concise way of making that test...
could
> you give me a suggestion?
Just check boot_cpu_data.x86 == 0xf - I don''t think we (or they) are
prepared to handle mixed family systems (mixed model already being
questionable in some places).

Jan