Re: oops/warning report for the week of November 26, 2008

From: H. Peter Anvin
Date: Thu Nov 27 2008 - 16:44:03 EST


Arjan van de Ven wrote:

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 1159e26..0044e61 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -1567,6 +1567,8 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
* Make sure we only trim uncachable memory on machines that
* support the Intel MTRR architecture:
*/
+ if (!cpu_has_mtrr)
+ return 0;
if (!is_cpu(INTEL) || disable_mtrr_trim)
return 0;
rdmsr(MTRRdefType_MSR, def, dummy);

Okay... is_cpu() here is defined as:

#define is_cpu(vnd) (mtrr_if && mtrr_if->vendor == X86_VENDOR_##vnd)

... so an MTRR interface has been identified. Therefore testing cpu_has_mtrr is redundant.

As far as use_intel() versus is_cpu(INTEL), it looks to me as though the two are identical in the current code -- mtrr_if->vendor is never set in the generic code, and so defaults to 0 - meaning X86_VENDOR_INTEL.

All in all, it looks like the vendor ID stuff is a bad case of "works by accident" in the MTRR code, however, *given the current code* I conclude that is_cpu(INTEL) == use_intel() and that neither can be true without MTRRs enabled.

-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/