[PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[Joerg Roedel]

Inside a KVM virtual machine the MTRRs are usually blank. This confuses Linux
and causes a warning message at boot. This patch removes that warning message
when running Linux as a KVM guest.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/cpu/mtrr/main.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index b6e136f..47e624c 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -43,6 +43,7 @@
 #include <asm/uaccess.h>
 #include <asm/processor.h>
 #include <asm/msr.h>
+#include <asm/kvm_para.h>
 #include "mtrr.h"
 
 u32 num_var_ranges = 0;
@@ -687,12 +688,14 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
 	}
 
 	/* kvm/qemu doesn't have mtrr set right, don't trim them all */
-	if (!highest_pfn) {
+	if (!highest_pfn && !kvm_para_available()) {
 		printk(KERN_WARNING "WARNING: strange, CPU MTRRs all blank?\n");
 		WARN_ON(1);
-		return 0;
 	}
 
+	if (!highest_pfn)
+		return 0;
+
 	if (highest_pfn < end_pfn) {
 		printk(KERN_WARNING "WARNING: BIOS bug: CPU MTRRs don't cover"
 			" all of memory, losing %luMB of RAM.\n",
-- 
1.5.3.7

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[Yinghai Lu]

On Thu, Feb 21, 2008 at 6:50 AM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> Inside a KVM virtual machine the MTRRs are usually blank. This confuses Linux
>  and causes a warning message at boot. This patch removes that warning message
>  when running Linux as a KVM guest.
>
>  Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
>  ---
>   arch/x86/kernel/cpu/mtrr/main.c |    7 +++++--
>   1 files changed, 5 insertions(+), 2 deletions(-)
>
>  diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
>  index b6e136f..47e624c 100644
>  --- a/arch/x86/kernel/cpu/mtrr/main.c
>  +++ b/arch/x86/kernel/cpu/mtrr/main.c
>  @@ -43,6 +43,7 @@
>   #include <asm/uaccess.h>
>   #include <asm/processor.h>
>   #include <asm/msr.h>
>  +#include <asm/kvm_para.h>
>   #include "mtrr.h"
>
>   u32 num_var_ranges = 0;
>  @@ -687,12 +688,14 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
>         }
>
>         /* kvm/qemu doesn't have mtrr set right, don't trim them all */
>  -       if (!highest_pfn) {
>  +       if (!highest_pfn && !kvm_para_available()) {
>                 printk(KERN_WARNING "WARNING: strange, CPU MTRRs all blank?\n");
>                 WARN_ON(1);
>  -               return 0;
>         }
>
>  +       if (!highest_pfn)
>  +               return 0;
>  +
>         if (highest_pfn < end_pfn) {
>                 printk(KERN_WARNING "WARNING: BIOS bug: CPU MTRRs don't cover"
>                         " all of memory, losing %luMB of RAM.\n",
>  --
>  1.5.3.7
>

Doest that WARN_ON(1) hurt?

or you may do some patch in qemu too.

so
        rdmsr(MTRRdefType_MSR, def, dummy);
        def &= 0xff;
        if (def != MTRR_TYPE_UNCACHABLE)
                return 0;

is happy out.

or set those MTRR entries in qemu...

YH

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[Joerg Roedel]

On Thu, Feb 21, 2008 at 11:54:00AM -0800, Yinghai Lu wrote:
> On Thu, Feb 21, 2008 at 6:50 AM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> > Inside a KVM virtual machine the MTRRs are usually blank. This confuses Linux
> >  and causes a warning message at boot. This patch removes that warning message
> >  when running Linux as a KVM guest.
> >
> >  Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> >  ---
> >   arch/x86/kernel/cpu/mtrr/main.c |    7 +++++--
> >   1 files changed, 5 insertions(+), 2 deletions(-)
> >
> >  diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
> >  index b6e136f..47e624c 100644
> >  --- a/arch/x86/kernel/cpu/mtrr/main.c
> >  +++ b/arch/x86/kernel/cpu/mtrr/main.c
> >  @@ -43,6 +43,7 @@
> >   #include <asm/uaccess.h>
> >   #include <asm/processor.h>
> >   #include <asm/msr.h>
> >  +#include <asm/kvm_para.h>
> >   #include "mtrr.h"
> >
> >   u32 num_var_ranges = 0;
> >  @@ -687,12 +688,14 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
> >         }
> >
> >         /* kvm/qemu doesn't have mtrr set right, don't trim them all */
> >  -       if (!highest_pfn) {
> >  +       if (!highest_pfn && !kvm_para_available()) {
> >                 printk(KERN_WARNING "WARNING: strange, CPU MTRRs all blank?\n");
> >                 WARN_ON(1);
> >  -               return 0;
> >         }
> >
> >  +       if (!highest_pfn)
> >  +               return 0;
> >  +
> >         if (highest_pfn < end_pfn) {
> >                 printk(KERN_WARNING "WARNING: BIOS bug: CPU MTRRs don't cover"
> >                         " all of memory, losing %luMB of RAM.\n",
> >  --
> >  1.5.3.7
> >
> 
> Doest that WARN_ON(1) hurt?

Yes. It inidicates a problem where there isn't one. In a virtual machine
MTRRs make absolutly no sense and so they should all be blank there.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[Ingo Molnar]

* Joerg Roedel <joerg.roedel@amd.com> wrote:

> -	if (!highest_pfn) {
> +	if (!highest_pfn && !kvm_para_available()) {
>  		printk(KERN_WARNING "WARNING: strange, CPU MTRRs all blank?\n");
>  		WARN_ON(1);
> -		return 0;
>  	}
>  
> +	if (!highest_pfn)
> +		return 0;

hm, why not have a single test for !highest_pfn:

	if (!highest_pfn) {
		if (!kvm_para_available()) {
  			printk(KERN_WARNING
			       "WARNING: strange, CPU MTRRs all blank?\n");
	  		WARN_ON(1);
		}
		return 0;
	}

? But yeah, your patch looks good otherwise.

I'm not sure how we could detect pure Qemu instances - perhaps it should 
define some special MSR or something?

	Ingo

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[H. Peter Anvin]

Ingo Molnar wrote:
> 
> I'm not sure how we could detect pure Qemu instances - perhaps it should 
> define some special MSR or something?
> 

Perhaps they should fix the Qemu BIOS to actually simulate working MSRs; 
if nothing else, they should set up the default MTRR to be cachable.

	-hpa

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

>> I'm not sure how we could detect pure Qemu instances - perhaps it 
>> should define some special MSR or something?
>
> Perhaps they should fix the Qemu BIOS to actually simulate working 
> MSRs; if nothing else, they should set up the default MTRR to be 
> cachable.

btw., should KVM do the same?

	Ingo

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[H. Peter Anvin]

Ingo Molnar wrote:
> * H. Peter Anvin <hpa@zytor.com> wrote:
> 
>>> I'm not sure how we could detect pure Qemu instances - perhaps it 
>>> should define some special MSR or something?
>> Perhaps they should fix the Qemu BIOS to actually simulate working 
>> MSRs; if nothing else, they should set up the default MTRR to be 
>> cachable.
> 
> btw., should KVM do the same?
> 

Yes.  Especially since they share a BIOS.

	-hpa

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> Ingo Molnar wrote:
>> * H. Peter Anvin <hpa@zytor.com> wrote:
>>
>>>> I'm not sure how we could detect pure Qemu instances - perhaps it should 
>>>> define some special MSR or something?
>>> Perhaps they should fix the Qemu BIOS to actually simulate working MSRs; 
>>> if nothing else, they should set up the default MTRR to be cachable.
>>
>> btw., should KVM do the same?
>
> Yes.  Especially since they share a BIOS.

ok. Then i guess we should just leave the warning and the backtrace in 
place until they get a fix done?

	Ingo

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[H. Peter Anvin]

Ingo Molnar wrote:
> * H. Peter Anvin <hpa@zytor.com> wrote:
> 
>> Ingo Molnar wrote:
>>> * H. Peter Anvin <hpa@zytor.com> wrote:
>>>
>>>>> I'm not sure how we could detect pure Qemu instances - perhaps it should 
>>>>> define some special MSR or something?
>>>> Perhaps they should fix the Qemu BIOS to actually simulate working MSRs; 
>>>> if nothing else, they should set up the default MTRR to be cachable.
>>> btw., should KVM do the same?
>> Yes.  Especially since they share a BIOS.
> 
> ok. Then i guess we should just leave the warning and the backtrace in 
> place until they get a fix done?
> 

That would be my opinion.

Last I checked, they didn't even clear the cache disable bits in %cr0.

	-hpa

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[Joerg Roedel]

On Fri, Feb 22, 2008 at 11:00:44AM +0100, Ingo Molnar wrote:
> 
> * H. Peter Anvin <hpa@zytor.com> wrote:
> 
> > Ingo Molnar wrote:
> >> * H. Peter Anvin <hpa@zytor.com> wrote:
> >>
> >>>> I'm not sure how we could detect pure Qemu instances - perhaps it should 
> >>>> define some special MSR or something?
> >>> Perhaps they should fix the Qemu BIOS to actually simulate working MSRs; 
> >>> if nothing else, they should set up the default MTRR to be cachable.
> >>
> >> btw., should KVM do the same?
> >
> > Yes.  Especially since they share a BIOS.
> 
> ok. Then i guess we should just leave the warning and the backtrace in 
> place until they get a fix done?

No. I don't agree. The MTRRs are set up by the BIOS because it knows the
hardware best (I know this is only true in theory). The OS should
basically trust the BIOS MTRR settings. If the OS can't trust the BIOS,
like on real hardware, the check and the warning is necessary. If the OS
can trust the BIOS, like running in a KVM or QEMU virtual machine, it
should just ignore the case where all MTRRs are blank.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH] X86: don't print a warning when MTRR are blank and running in KVM

[Ingo Molnar]

* Joerg Roedel <joerg.roedel@amd.com> wrote:

> > ok. Then i guess we should just leave the warning and the backtrace 
> > in place until they get a fix done?
> 
> No. I don't agree. The MTRRs are set up by the BIOS because it knows 
> the hardware best (I know this is only true in theory). The OS should 
> basically trust the BIOS MTRR settings. If the OS can't trust the 
> BIOS, like on real hardware, the check and the warning is necessary. 
> If the OS can trust the BIOS, like running in a KVM or QEMU virtual 
> machine, it should just ignore the case where all MTRRs are blank.

well, i can agree with the warning being pointless on KVM paravirt, but 
it's not pointless in the Qemu context.

we fundamentall _dont_ trust the BIOS's MTRR settings, just check the 
very function that you modified: mtrr_trim_uncached_memory() - that 
function handles _exactly_ a case where a BIOS messes up MTRR's. This 
problem has caused real user-side breakage and just not emitting a 
warning because Qemu is lazy to emulate a proper PC is not a strong 
enough argument.

so i've applied the KVM bits but the warning stays for Qemu.

	Ingo