[Qemu-devel] [PATCH] MTRR support on x86, part 2

[Qemu-devel] [PATCH] MTRR support on x86, part 2

[Carl-Daniel Hailfinger]

Load and save MTRR state together with machine state.

Add support for the MTRRcap MSR which is used by the latest Bochs BIOS
and some operating systems.

Fix a typo in ext2_feature_name.

With this patch, MTRR emulation should be good enough to not trigger any
sanity checks in well behaved BIOS/kernel code.
Some corner cases for BIOS/firmware usage remain to be implemented, but
that can be deferred to another patch.
Also, MTRR accesses on hardware not supporting MTRRs should cause #GP.
That can be enforced by another patch as well.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

Index: target-i386/helper.c
===================================================================
--- target-i386/helper.c	(Revision 6461)
+++ target-i386/helper.c	(Arbeitskopie)
@@ -55,7 +55,7 @@
     };
     static const char *ext2_feature_name[] = {
        "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
-       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mttr", "pge", "mca", "cmov",
+       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr", "pge", "mca", "cmov",
        "pat", "pse36", NULL, NULL /* Linux mp */, "nx" /* Intel xd */, NULL, "mmxext", "mmx",
        "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp", NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
     };
Index: target-i386/machine.c
===================================================================
--- target-i386/machine.c	(Revision 6461)
+++ target-i386/machine.c	(Arbeitskopie)
@@ -134,6 +134,15 @@
     qemu_put_be16s(f, &env->intercept_dr_write);
     qemu_put_be32s(f, &env->intercept_exceptions);
     qemu_put_8s(f, &env->v_tpr);
+
+    /* MTRRs */
+    for(i = 0; i < 11; i++)
+        qemu_put_be64s(f, &env->mtrr_fixed[i]);
+    qemu_put_be64s(f, &env->mtrr_deftype);
+    for(i = 0; i < 8; i++) {
+        qemu_put_be64s(f, &env->mtrr_var[i].base);
+        qemu_put_be64s(f, &env->mtrr_var[i].mask);
+    }
 }
 
 #ifdef USE_X86LDOUBLE
@@ -169,7 +178,7 @@
     int32_t a20_mask;
 
     if (version_id != 3 && version_id != 4 && version_id != 5
-        && version_id != 6 && version_id != 7)
+        && version_id != 6 && version_id != 7 && version_id != 8)
         return -EINVAL;
     for(i = 0; i < CPU_NB_REGS; i++)
         qemu_get_betls(f, &env->regs[i]);
@@ -302,6 +311,18 @@
         qemu_get_be32s(f, &env->intercept_exceptions);
         qemu_get_8s(f, &env->v_tpr);
     }
+
+    if (version_id >= 8) {
+        /* MTRRs */
+        for(i = 0; i < 11; i++)
+            qemu_get_be64s(f, &env->mtrr_fixed[i]);
+        qemu_get_be64s(f, &env->mtrr_deftype);
+        for(i = 0; i < 8; i++) {
+            qemu_get_be64s(f, &env->mtrr_var[i].base);
+            qemu_get_be64s(f, &env->mtrr_var[i].mask);
+        }
+    }
+
     /* XXX: ensure compatiblity for halted bit ? */
     /* XXX: compute redundant hflags bits */
     env->hflags = hflags;
Index: target-i386/cpu.h
===================================================================
--- target-i386/cpu.h	(Revision 6461)
+++ target-i386/cpu.h	(Arbeitskopie)
@@ -251,6 +251,11 @@
 #define MSR_IA32_APICBASE_ENABLE        (1<<11)
 #define MSR_IA32_APICBASE_BASE          (0xfffff<<12)
 
+#define MSR_MTRRcap			0xfe
+#define MSR_MTRRcap_VCNT		8
+#define MSR_MTRRcap_FIXRANGE_SUPPORT	(1 << 8)
+#define MSR_MTRRcap_WC_SUPPORTED	(1 << 10)
+
 #define MSR_IA32_SYSENTER_CS            0x174
 #define MSR_IA32_SYSENTER_ESP           0x175
 #define MSR_IA32_SYSENTER_EIP           0x176
Index: target-i386/op_helper.c
===================================================================
--- target-i386/op_helper.c	(Revision 6461)
+++ target-i386/op_helper.c	(Arbeitskopie)
@@ -3215,6 +3215,13 @@
     case MSR_MTRRdefType:
         val = env->mtrr_deftype;
         break;
+    case MSR_MTRRcap:
+        if (env->cpuid_features & CPUID_MTRR)
+            val = MSR_MTRRcap_VCNT | MSR_MTRRcap_FIXRANGE_SUPPORT | MSR_MTRRcap_WC_SUPPORTED;
+        else
+            /* XXX: exception ? */
+            val = 0;
+        break;
     default:
         /* XXX: exception ? */
         val = 0;


-- 
http://www.hailfinger.org/

Re: [Qemu-devel] [PATCH] MTRR support on x86, part 2

[Anthony Liguori]

Carl-Daniel Hailfinger wrote:
> Load and save MTRR state together with machine state.
>
> Add support for the MTRRcap MSR which is used by the latest Bochs BIOS
> and some operating systems.
>
> Fix a typo in ext2_feature_name.
>
> With this patch, MTRR emulation should be good enough to not trigger any
> sanity checks in well behaved BIOS/kernel code.
> Some corner cases for BIOS/firmware usage remain to be implemented, but
> that can be deferred to another patch.
> Also, MTRR accesses on hardware not supporting MTRRs should cause #GP.
> That can be enforced by another patch as well.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>
> Index: target-i386/helper.c
> ===================================================================
> --- target-i386/helper.c	(Revision 6461)
> +++ target-i386/helper.c	(Arbeitskopie)
> @@ -55,7 +55,7 @@
>      };
>      static const char *ext2_feature_name[] = {
>         "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
> -       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mttr", "pge", "mca", "cmov",
> +       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr", "pge", "mca", "cmov",
>   

As best as I can tell, there is no change here.  I removed it from the 
diff and applied the rest.

Thanks.

Regards,

Anthony Liguori
>         "pat"

Re: [Qemu-devel] [PATCH] MTRR support on x86, part 2

[Kevin Wolf]

Anthony Liguori schrieb:
> Carl-Daniel Hailfinger wrote:
>> Load and save MTRR state together with machine state.
>>
>> Add support for the MTRRcap MSR which is used by the latest Bochs BIOS
>> and some operating systems.
>>
>> Fix a typo in ext2_feature_name.
>>
>> With this patch, MTRR emulation should be good enough to not trigger any
>> sanity checks in well behaved BIOS/kernel code.
>> Some corner cases for BIOS/firmware usage remain to be implemented, but
>> that can be deferred to another patch.
>> Also, MTRR accesses on hardware not supporting MTRRs should cause #GP.
>> That can be enforced by another patch as well.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>>
>> Index: target-i386/helper.c
>> ===================================================================
>> --- target-i386/helper.c    (Revision 6461)
>> +++ target-i386/helper.c    (Arbeitskopie)
>> @@ -55,7 +55,7 @@
>>      };
>>      static const char *ext2_feature_name[] = {
>>         "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
>> -       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mttr",
>> "pge", "mca", "cmov",
>> +       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr",
>> "pge", "mca", "cmov",
>>   
> 
> As best as I can tell, there is no change here.  I removed it from the
> diff and applied the rest.

The change seems to be s/mttr/mtrr/

Kevin

Re: [Qemu-devel] [PATCH] MTRR support on x86, part 2

[Jung-uk Kim]

On Thursday 29 January 2009 12:02 pm, Anthony Liguori wrote:
> As best as I can tell, there is no change here.  I removed it from
> the diff and applied the rest.

It's funny I had the same thought when the patch showed up.  Then, I 
looked closer and found something different: "mttr" -> "mtrr". ;-)

Jung-uk Kim

Re: [Qemu-devel] [PATCH] MTRR support on x86, part 2

[Carl-Daniel Hailfinger]

On 29.01.2009 18:02, Anthony Liguori wrote:
> Carl-Daniel Hailfinger wrote:
>> Load and save MTRR state together with machine state.
>>
>> Add support for the MTRRcap MSR which is used by the latest Bochs BIOS
>> and some operating systems.
>>
>> Fix a typo in ext2_feature_name.
>>
>> With this patch, MTRR emulation should be good enough to not trigger any
>> sanity checks in well behaved BIOS/kernel code.
>> Some corner cases for BIOS/firmware usage remain to be implemented, but
>> that can be deferred to another patch.
>> Also, MTRR accesses on hardware not supporting MTRRs should cause #GP.
>> That can be enforced by another patch as well.
>>
>> Signed-off-by: Carl-Daniel Hailfinger
>> <c-d.hailfinger.devel.2006@gmx.net>

Thanks for applying the patch!

>> Index: target-i386/helper.c
>> ===================================================================
>> --- target-i386/helper.c    (Revision 6461)
>> +++ target-i386/helper.c    (Arbeitskopie)
>> @@ -55,7 +55,7 @@
>>      };
>>      static const char *ext2_feature_name[] = {
>>         "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
>> -       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mttr",
>> "pge", "mca", "cmov",
>> +       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr",
>> "pge", "mca", "cmov",
>>   
>
> As best as I can tell, there is no change here.  I removed it from the
> diff and applied the rest.

It was mtTr, but it should have been mtRr (emphasis mine).

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/

Re: [Qemu-devel] [PATCH] MTRR support on x86, part 2

[Anthony Liguori]

Jung-uk Kim wrote:
> On Thursday 29 January 2009 12:02 pm, Anthony Liguori wrote:
>   
>> As best as I can tell, there is no change here.  I removed it from
>> the diff and applied the rest.
>>     
>
> It's funny I had the same thought when the patch showed up.  Then, I 
> looked closer and found something different: "mttr" -> "mtrr". ;-)
>   

Yeah, I guess I missed that.  I'll fix it.

Regards,

Anthony Liguori

> Jung-uk Kim
>   

Re: [Qemu-devel] [PATCH] MTRR support on x86, part 2

[Anthony Liguori]

Carl-Daniel Hailfinger wrote:
> On 29.01.2009 18:02, Anthony Liguori wrote:
>   
>> Carl-Daniel Hailfinger wrote:
>>     
>>> Load and save MTRR state together with machine state.
>>>
>>> Add support for the MTRRcap MSR which is used by the latest Bochs BIOS
>>> and some operating systems.
>>>
>>> Fix a typo in ext2_feature_name.
>>>
>>> With this patch, MTRR emulation should be good enough to not trigger any
>>> sanity checks in well behaved BIOS/kernel code.
>>> Some corner cases for BIOS/firmware usage remain to be implemented, but
>>> that can be deferred to another patch.
>>> Also, MTRR accesses on hardware not supporting MTRRs should cause #GP.
>>> That can be enforced by another patch as well.
>>>
>>> Signed-off-by: Carl-Daniel Hailfinger
>>> <c-d.hailfinger.devel.2006@gmx.net>
>>>       
>
> Thanks for applying the patch!
>
>   
>>> Index: target-i386/helper.c
>>> ===================================================================
>>> --- target-i386/helper.c    (Revision 6461)
>>> +++ target-i386/helper.c    (Arbeitskopie)
>>> @@ -55,7 +55,7 @@
>>>      };
>>>      static const char *ext2_feature_name[] = {
>>>         "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
>>> -       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mttr",
>>> "pge", "mca", "cmov",
>>> +       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr",
>>> "pge", "mca", "cmov",
>>>   
>>>       
>> As best as I can tell, there is no change here.  I removed it from the
>> diff and applied the rest.
>>     
>
> It was mtTr, but it should have been mtRr (emphasis mine).
>   

Yeah, sorry.  In general, it's better to split multiple fixes into 
multiple patches to make it clear.  I should have bounced the patch 
instead of removing that chunk though. Then my mistake would have been 
more obvious.  Sorry again for that.

Regards,

Anthony Liguori

> Regards,
> Carl-Daniel
>
>