[PATCH] KVM: x86: Increase the number of fixed MTRR regs to 10

[PATCH] KVM: x86: Increase the number of fixed MTRR regs to 10

[Nadav Amit]

Recent Intel CPUs have 10 variable range MTRRs. Since operating systems
sometime make assumptions on CPUs while they ignore capability MSRs, it is
better for KVM to be consistent with recent CPUs. Reporting more MTRRs than
actually supported has no functional implications.

Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
---
 arch/x86/include/asm/kvm_host.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4931415..0bab29d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -95,7 +95,7 @@ static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)
 #define KVM_REFILL_PAGES 25
 #define KVM_MAX_CPUID_ENTRIES 80
 #define KVM_NR_FIXED_MTRR_REGION 88
-#define KVM_NR_VAR_MTRR 8
+#define KVM_NR_VAR_MTRR 10
 
 #define ASYNC_PF_PER_VCPU 64
 
-- 
1.9.1

Re: [PATCH] KVM: x86: Increase the number of fixed MTRR regs to 10

[Wanpeng Li]

Hi Nadav,
On Wed, Jun 18, 2014 at 05:21:19PM +0300, Nadav Amit wrote:
>Recent Intel CPUs have 10 variable range MTRRs. Since operating systems
>sometime make assumptions on CPUs while they ignore capability MSRs, it is
>better for KVM to be consistent with recent CPUs. Reporting more MTRRs than
>actually supported has no functional implications.
>
>Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>---
> arch/x86/include/asm/kvm_host.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>index 4931415..0bab29d 100644
>--- a/arch/x86/include/asm/kvm_host.h
>+++ b/arch/x86/include/asm/kvm_host.h
>@@ -95,7 +95,7 @@ static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)
> #define KVM_REFILL_PAGES 25
> #define KVM_MAX_CPUID_ENTRIES 80
> #define KVM_NR_FIXED_MTRR_REGION 88
>-#define KVM_NR_VAR_MTRR 8
>+#define KVM_NR_VAR_MTRR 10
> 

We observed that there is obvious regression caused by this commit, 32bit 
win7 guest show blue screen during boot.

Regards,
Wanpeng Li 

> #define ASYNC_PF_PER_VCPU 64
> 
>-- 
>1.9.1
>
>--
>To unsubscribe from this list: send the line "unsubscribe kvm" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: x86: Increase the number of fixed MTRR regs to 10

[Nadav Amit]

This should have been a benign patch. I'll try to get windows 7 installation disk and check ASAP.

Nadav

> On 18 Aug 2014, at 05:17, Wanpeng Li <wanpeng.li@linux.intel.com> wrote:
> 
> Hi Nadav,
>> On Wed, Jun 18, 2014 at 05:21:19PM +0300, Nadav Amit wrote:
>> Recent Intel CPUs have 10 variable range MTRRs. Since operating systems
>> sometime make assumptions on CPUs while they ignore capability MSRs, it is
>> better for KVM to be consistent with recent CPUs. Reporting more MTRRs than
>> actually supported has no functional implications.
>> 
>> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>> ---
>> arch/x86/include/asm/kvm_host.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 4931415..0bab29d 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -95,7 +95,7 @@ static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)
>> #define KVM_REFILL_PAGES 25
>> #define KVM_MAX_CPUID_ENTRIES 80
>> #define KVM_NR_FIXED_MTRR_REGION 88
>> -#define KVM_NR_VAR_MTRR 8
>> +#define KVM_NR_VAR_MTRR 10
> 
> We observed that there is obvious regression caused by this commit, 32bit 
> win7 guest show blue screen during boot.
> 
> Regards,
> Wanpeng Li 
> 
>> #define ASYNC_PF_PER_VCPU 64
>> 
>> -- 
>> 1.9.1
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: x86: Increase the number of fixed MTRR regs to 10

[Wanpeng Li]

On Mon, Aug 18, 2014 at 09:39:39AM +0300, Nadav Amit wrote:
>This should have been a benign patch. I'll try to get windows 7 installation disk and check ASAP.
>

In addition, it just can be reproduced on 32bit win7 w/ MP enabled, in
case UP can't be reproduced.

Regards,
Wanpeng Li 

>Nadav
>
>> On 18 Aug 2014, at 05:17, Wanpeng Li <wanpeng.li@linux.intel.com> wrote:
>> 
>> Hi Nadav,
>>> On Wed, Jun 18, 2014 at 05:21:19PM +0300, Nadav Amit wrote:
>>> Recent Intel CPUs have 10 variable range MTRRs. Since operating systems
>>> sometime make assumptions on CPUs while they ignore capability MSRs, it is
>>> better for KVM to be consistent with recent CPUs. Reporting more MTRRs than
>>> actually supported has no functional implications.
>>> 
>>> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>>> ---
>>> arch/x86/include/asm/kvm_host.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>>> index 4931415..0bab29d 100644
>>> --- a/arch/x86/include/asm/kvm_host.h
>>> +++ b/arch/x86/include/asm/kvm_host.h
>>> @@ -95,7 +95,7 @@ static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)
>>> #define KVM_REFILL_PAGES 25
>>> #define KVM_MAX_CPUID_ENTRIES 80
>>> #define KVM_NR_FIXED_MTRR_REGION 88
>>> -#define KVM_NR_VAR_MTRR 8
>>> +#define KVM_NR_VAR_MTRR 10
>> 
>> We observed that there is obvious regression caused by this commit, 32bit 
>> win7 guest show blue screen during boot.
>> 
>> Regards,
>> Wanpeng Li 
>> 
>>> #define ASYNC_PF_PER_VCPU 64
>>> 
>>> -- 
>>> 1.9.1
>>> 
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: x86: Increase the number of fixed MTRR regs to 10

[Nadav Amit]

I reproduced the blue-screen. Let me to to figure it out.

Nadav

On Aug 18, 2014, at 11:11 AM, Wanpeng Li <wanpeng.li@linux.intel.com> wrote:

> On Mon, Aug 18, 2014 at 09:39:39AM +0300, Nadav Amit wrote:
>> This should have been a benign patch. I'll try to get windows 7 installation disk and check ASAP.
>> 
> 
> In addition, it just can be reproduced on 32bit win7 w/ MP enabled, in
> case UP can't be reproduced.
> 
> Regards,
> Wanpeng Li 
> 
>> Nadav
>> 
>>> On 18 Aug 2014, at 05:17, Wanpeng Li <wanpeng.li@linux.intel.com> wrote:
>>> 
>>> Hi Nadav,
>>>> On Wed, Jun 18, 2014 at 05:21:19PM +0300, Nadav Amit wrote:
>>>> Recent Intel CPUs have 10 variable range MTRRs. Since operating systems
>>>> sometime make assumptions on CPUs while they ignore capability MSRs, it is
>>>> better for KVM to be consistent with recent CPUs. Reporting more MTRRs than
>>>> actually supported has no functional implications.
>>>> 
>>>> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>>>> ---
>>>> arch/x86/include/asm/kvm_host.h | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> 
>>>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>>>> index 4931415..0bab29d 100644
>>>> --- a/arch/x86/include/asm/kvm_host.h
>>>> +++ b/arch/x86/include/asm/kvm_host.h
>>>> @@ -95,7 +95,7 @@ static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)
>>>> #define KVM_REFILL_PAGES 25
>>>> #define KVM_MAX_CPUID_ENTRIES 80
>>>> #define KVM_NR_FIXED_MTRR_REGION 88
>>>> -#define KVM_NR_VAR_MTRR 8
>>>> +#define KVM_NR_VAR_MTRR 10
>>> 
>>> We observed that there is obvious regression caused by this commit, 32bit 
>>> win7 guest show blue screen during boot.
>>> 
>>> Regards,
>>> Wanpeng Li 
>>> 
>>>> #define ASYNC_PF_PER_VCPU 64
>>>> 
>>>> -- 
>>>> 1.9.1
>>>> 
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: x86: Increase the number of fixed MTRR regs to 10

[Nadav Amit]

The cause for the blue-screen appears to be seabios, which leaves only 0x20 slots for “smp_mtrr”s.
Apparently, the increase in the variable range MTRR count caused it to exhaust the available slots.
As a result, some MSRs are not initialised by the BIOS (specifically, 3.5-4GB are not marked as UC), and cause Windows to panic.

Once we increase the size of the array smp_mtrr in seabios, Windows boots.

Paolo, you may wish to revert the patch. Please note that it was applied to some stable branches.

Nadav

On Aug 18, 2014, at 12:39 PM, Nadav Amit <nadav.amit@gmail.com> wrote:

> I reproduced the blue-screen. Let me to to figure it out.
> 
> Nadav
> 
> On Aug 18, 2014, at 11:11 AM, Wanpeng Li <wanpeng.li@linux.intel.com> wrote:
> 
>> On Mon, Aug 18, 2014 at 09:39:39AM +0300, Nadav Amit wrote:
>>> This should have been a benign patch. I'll try to get windows 7 installation disk and check ASAP.
>>> 
>> 
>> In addition, it just can be reproduced on 32bit win7 w/ MP enabled, in
>> case UP can't be reproduced.
>> 
>> Regards,
>> Wanpeng Li 
>> 
>>> Nadav
>>> 
>>>> On 18 Aug 2014, at 05:17, Wanpeng Li <wanpeng.li@linux.intel.com> wrote:
>>>> 
>>>> Hi Nadav,
>>>>> On Wed, Jun 18, 2014 at 05:21:19PM +0300, Nadav Amit wrote:
>>>>> Recent Intel CPUs have 10 variable range MTRRs. Since operating systems
>>>>> sometime make assumptions on CPUs while they ignore capability MSRs, it is
>>>>> better for KVM to be consistent with recent CPUs. Reporting more MTRRs than
>>>>> actually supported has no functional implications.
>>>>> 
>>>>> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>>>>> ---
>>>>> arch/x86/include/asm/kvm_host.h | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> 
>>>>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>>>>> index 4931415..0bab29d 100644
>>>>> --- a/arch/x86/include/asm/kvm_host.h
>>>>> +++ b/arch/x86/include/asm/kvm_host.h
>>>>> @@ -95,7 +95,7 @@ static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)
>>>>> #define KVM_REFILL_PAGES 25
>>>>> #define KVM_MAX_CPUID_ENTRIES 80
>>>>> #define KVM_NR_FIXED_MTRR_REGION 88
>>>>> -#define KVM_NR_VAR_MTRR 8
>>>>> +#define KVM_NR_VAR_MTRR 10
>>>> 
>>>> We observed that there is obvious regression caused by this commit, 32bit 
>>>> win7 guest show blue screen during boot.
>>>> 
>>>> Regards,
>>>> Wanpeng Li 
>>>> 
>>>>> #define ASYNC_PF_PER_VCPU 64
>>>>> 
>>>>> -- 
>>>>> 1.9.1
>>>>> 
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] KVM: x86: Increase the number of fixed MTRR regs to 10

[Paolo Bonzini]

Il 18/08/2014 16:31, Nadav Amit ha scritto:
> The cause for the blue-screen appears to be seabios, which leaves only 0x20 slots for “smp_mtrr”s.
> Apparently, the increase in the variable range MTRR count caused it to exhaust the available slots.
> As a result, some MSRs are not initialised by the BIOS (specifically, 3.5-4GB are not marked as UC), and cause Windows to panic.
> 
> Once we increase the size of the array smp_mtrr in seabios, Windows boots.
> 
> Paolo, you may wish to revert the patch. Please note that it was applied to some stable branches.

Thanks.  I'll post a patch to SeaBIOS too, since this should happen on
bare metal too.

Paolo