Re: [PATCH 2/2] Suppress false "mtrr all empty" warning message when running as VMware guest

Previous thread: Re: parport_pc disables parallel port on unload. by Bodo Eggert on Wednesday, September 24, 2008 - 5:47 am. (2 messages)

Next thread: [PATCH] x86 gart: remove unnecessary initialization by FUJITA Tomonori on Wednesday, September 24, 2008 - 6:41 am. (3 messages)
From: Yan Li
Date: Wednesday, September 24, 2008 - 5:24 am

Since the mtrr empty was detected very early before we can use DMI or
PCI to check whether we are running as a VMware guest or not, we now
only print an info there. Warning will only be issued later when we
are sure that we are not running as a VMware guest.

mtrr_trim_uncached_memory() is modified to return meaningful codes for
later warning decision.

Signed-off-by: Yan Li <elliot.li.tech@gmail.com>
---
 arch/x86/kernel/cpu/mtrr/main.c |   16 +++++++++++++++-
 arch/x86/kernel/setup.c         |   27 ++++++++++++++++++++++++++-
 include/asm-x86/mtrr.h          |    2 ++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index b117d7f..95d1cc0 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -1453,6 +1453,11 @@ static u64 __init real_trim_memory(unsigned long start_pfn,
  * all of the memory the kernel is intending to use. If not, it'll trim any
  * memory off the end by adjusting end_pfn, removing it from the kernel's
  * allocation pools, warning the user with an obnoxious message.
+ *
+ * Return code:
+ * EMTRR_ALL_BLANK (-1):  not trimmed due to CPU MTRRs all blank
+ *                    0:  not trimmed due to other reasons
+ *                    1:  trimmed successfully
  */
 int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
 {
@@ -1494,11 +1499,20 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
 			highest_pfn = base + size;
 	}
 
-	/* kvm/qemu doesn't have mtrr set right, don't trim them all */
+	/* kvm/qemu/VMware doesn't have mtrr set right, don't trim them all */
 	if (!highest_pfn) {
+#ifdef CONFIG_VMWARE_GUEST_DETECT
+		/* the "mtrr all blank" warning will be deferred until
+		 * after DMI scanning and we know the machine is not a
+		 * VMware guest
+		 */
+		printk(KERN_INFO "CPU MTRRs all blank\n");
+		return EMTRR_ALL_BLANK;
+#else
 		WARN(!kvm_para_available(), KERN_WARNING
 				"WARNING: strange, CPU MTRRs ...
From: Laurent Vivier
Date: Wednesday, September 24, 2008 - 5:54 am

perhaps something like:

#ifdef CONFIG_VMWARE_GUEST_DETECT
 		WARN(!kvm_para_available() && !is_vmware_guest(), KERN_WARNING
				"WARNING: strange, CPU MTRRs all blank?\n");
#else
 		WARN(!kvm_para_available(), KERN_WARNING
  				"WARNING: strange, CPU MTRRs all blank?\n");
...

???

Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

--

From: Yan Li
Date: Wednesday, September 24, 2008 - 6:10 am

As stated in the comments: the warning has to be deferred. The reason
is that "is_vmware_guest()" replies on DMI and can only be used after
dmi_scan().  These mtrr codes here are run very early during
setup_arch() when the DMI is not available. So when I've detected all
MTRRs are blank, I returned a status code to setup_arch(), who will
call "is_vmware_guest()" later, after dmi_scan(), to decide whether to
issue a warning or not.


-- 
Li, Yan

"Everything that is really great and inspiring is created by the
individual who can labor in freedom."
              - Albert Einstein, in Out of My Later Years (1950)
--

From: Joerg Roedel
Date: Wednesday, September 24, 2008 - 7:16 am

A lot of code if #ifdef'ed to vmware. Can you hide this into a header

Can we move the WARN after the DMI scanning for all cases? This will

-- 
           |           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

--

From: Yan Li
Date: Wednesday, September 24, 2008 - 7:51 pm

Sounds good. Thanks!


-- 
Li, Yan
--

From: Yinghai Lu
Date: Wednesday, September 24, 2008 - 9:39 am

we have moved dmi_scan_machine() much early...

so please check with latest tip/master to use
dmi_check_system with that.

BTW:
those warning should be good, because VMware is the root cause.

YH
--

From: Yan Li
Date: Wednesday, September 24, 2008 - 7:52 pm

Sorry, do you mean we also need a warning for this in VMware?

-- 
Li, Yan
--

From: Yan Li
Date: Thursday, September 25, 2008 - 7:18 am

I checked your patch 1c6e5503 that moved dmi_scan_machine() earlier,
but it's still behind mtrr_trim_uncached_memory(), so we still can't
use DMI to check for VMware there.

-- 
Li, Yan
--

From: Yinghai Lu
Date: Thursday, September 25, 2008 - 10:13 am

you must be kidding!

...

        dmi_scan_machine();

        dmi_check_system(bad_bios_dmi_table);

#ifdef CONFIG_X86_32
        probe_roms();
#endif

        /* after parse_early_param, so could debug it */
        insert_resource(&iomem_resource, &code_resource);
        insert_resource(&iomem_resource, &data_resource);
        insert_resource(&iomem_resource, &bss_resource);

        if (efi_enabled)
                efi_init();

#ifdef CONFIG_X86_32
        if (ppro_with_ram_bug()) {
                e820_update_range(0x70000000ULL, 0x40000ULL, E820_RAM,
                                  E820_RESERVED);
                sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
                printk(KERN_INFO "fixed physical RAM map:\n");
                e820_print_map("bad_ppro");
        }
#else
        early_gart_iommu_check();
#endif

        /*
         * partially used pages are not usable - thus
         * we are rounding upwards:
         */
        max_pfn = e820_end_of_ram_pfn();

        /* preallocate 4k for mptable mpc */
        early_reserve_e820_mpc_new();
        /* update e820 for memory not covered by WB MTRRs */
        mtrr_bp_init();
        if (mtrr_trim_uncached_memory(max_pfn))
                max_pfn = e820_end_of_ram_pfn();
--

From: Yan Li
Date: Friday, September 26, 2008 - 2:50 am

Ooops. Sorry, my fault.  I checked the wrong tree. I'm looking into
this.

Thanks!

-- 
Li, Yan
--

From: Pavel Machek
Date: Wednesday, October 1, 2008 - 1:03 pm

Hi!

Subject says: Suppress false "mtrr all empty"  .... would you explain
why is the message false? I believe it is very true, and points out
fact that VMWare virtualization fails to simulate MTRR-s.

IMO VMWare should just fix their emulator.

								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--

From: Yan Li
Date: Sunday, October 5, 2008 - 5:05 am

You are right, to some degree. Actually I have no strong evidence to
support this either.  Since KVM doesn't set MTRR, by hunch I thought
VMware might having similar shortages. Also nothing bad can be
observed after the system booted, so I guessed that warning was
useless. Moreover, none other OSes (include old Linux kernels) is
complaining about this in VMware so users of VMware might think 2.6.27
is buggy on this.

Hope someone from VMware could clear this up.  I think they are
working on this problem followed this thread. I'd be happy to help.

-- 
Li, Yan
--

Previous thread: Re: parport_pc disables parallel port on unload. by Bodo Eggert on Wednesday, September 24, 2008 - 5:47 am. (2 messages)

Next thread: [PATCH] x86 gart: remove unnecessary initialization by FUJITA Tomonori on Wednesday, September 24, 2008 - 6:41 am. (3 messages)