[PATCH v2] X86: don't report PAT on CPUs that don't support it
From: Mikulas Patocka
Date: Tue Jun 06 2017 - 18:49:52 EST
On Sun, 28 May 2017, Andy Lutomirski wrote:
> On Sun, May 28, 2017 at 11:18 AM, Bernhard Held <berny156@xxxxxx> wrote:
> > Hi,
> >
> > this patch breaks the boot of my kernel. The last message is "Booting
> > the kernel.".
> >
> > My setup might be unusual: I'm running a Xenon E5450 (LGA 771) in a
> > Gigbayte G33-DS3R board (LGA 775). The BIOS is patched with the
> > microcode of the E5450 and recognizes the CPU.
> >
> > Please find below the dmesg of a the latest kernel w/o the PAT-patch.
> > I'm happy to provide more information or to test patches.
>
> I think this patch is bogus. pat_enabled() sure looks like it's
> supposed to return true if PAT is *enabled*, and these days PAT is
> "enabled" even if there's no HW PAT support. Even if the patch were
> somehow correct, it should have been split up into two patches, one to
> change pat_enabled() and one to use this_cpu_has().
>
> Ingo, I'd suggest reverting the patch, cc-ing stable on the revert so
> -stable knows not to backport it, and starting over with the fix.
> >From very brief inspection, the right fix is to make sure that
> pat_init(), or at least init_cache_modes(), gets called on the
> affected CPUs.
>
> --Andy
Hi
Here I send the second version of the patch. It drops the change from
boot_cpu_has(X86_FEATURE_PAT) to this_cpu_has(X86_FEATURE_PAT) (that
caused kernel to be unbootable for some people).
Another change is that setup_arch() calls init_cache_modes() if PAT is
disabled, so that init_cache_modes() is always called.
Mikulas
From: Mikulas Patocka <mpatocka@xxxxxxxxxx>
In the file arch/x86/mm/pat.c, there's a variable __pat_enabled. The
variable is set to 1 by default and the function pat_init() sets
__pat_enabled to 0 if the CPU doesn't support PAT.
However, on AMD K6-3 CPU, the processor initialization code never calls
pat_init() and so __pat_enabled stays 1 and the function pat_enabled()
returns true, even though the K6-3 CPU doesn't support PAT.
The result of this bug is that this warning is produced when attemting to
start the Xserver and the Xserver doesn't start (fork() returns ENOMEM).
Another symptom of this bug is that the framebuffer driver doesn't set the
K6-3 MTRR registers.
This patch changes pat_enabled() so that it returns true only if pat
initialization was actually done.
As Andy Lutomirski suggested, we also need to call init_cache_modes() if
pat was not initialized, so we call it after mtrr_bp_init()
(mtrr_bp_init() would or wouldn't call pat_init()). Note that
init_cache_modes() detects if it was called more than once and does
nothing in that case.
x86/PAT: Xorg:3891 map pfn expected mapping type uncached-minus for [mem 0xe4000000-0xe5ffffff], got write-combining
------------[ cut here ]------------
WARNING: CPU: 0 PID: 3891 at arch/x86/mm/pat.c:1020 untrack_pfn+0x5c/0x9f
Modules linked in: blowfish_generic blowfish_common cbc dm_crypt dm_loop msr configfs cpufreq_userspace cpufreq_powersave cpufreq_ondemand cpufreq_conservative ext4 crc16 jbd2 mbcache hpfs nls_cp852 msdos fat matroxfb_base matroxfb_g450 matroxfb_accel cfbfillrect cfbimgblt cfbcopyarea matroxfb_DAC1064 g450_pll matroxfb_misc floppy snd_usb_audio snd_hwdep snd_usbmidi_lib snd_seq_midi snd_seq_midi_event snd_rawmidi snd_pcm snd_seq snd_seq_device snd_timer hid_generic snd usbhid hid soundcore powernow_k6 pcspkr evdev e1000 ehci_pci uhci_hcd ehci_hcd usbcore usb_common dm_mod pata_it821x unix
CPU: 0 PID: 3891 Comm: Xorg Not tainted 4.11.0-rc6-test+ #35
Hardware name: System Manufacturer Product Name/VA-503A, BIOS 4.51 PG 08/02/00
Call Trace:
? __warn+0xab/0xc0
? untrack_pfn+0x5c/0x9f
? warn_slowpath_null+0xf/0x13
? untrack_pfn+0x5c/0x9f
? unmap_single_vma+0x43/0x66
? unmap_vmas+0x24/0x30
? exit_mmap+0x3c/0xa5
? __mmput+0xf/0x76
? copy_process.part.76+0xb43/0x1129
? _do_fork+0x96/0x177
? do_int80_syscall_32+0x3e/0x4c
? entry_INT80_32+0x2a/0x2a
---[ end trace 8726f9d9fa90d702 ]---
x86/PAT: Xorg:3891 map pfn expected mapping type uncached-minus for [mem 0xe4000000-0xe5ffffff], got write-combining
Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx # v4.2+
---
arch/x86/include/asm/pat.h | 1 +
arch/x86/kernel/setup.c | 2 ++
arch/x86/mm/pat.c | 10 ++++++----
3 files changed, 9 insertions(+), 4 deletions(-)
Index: linux-stable/arch/x86/mm/pat.c
===================================================================
--- linux-stable.orig/arch/x86/mm/pat.c
+++ linux-stable/arch/x86/mm/pat.c
@@ -40,7 +40,6 @@
static bool boot_cpu_done;
static int __read_mostly __pat_enabled = IS_ENABLED(CONFIG_X86_PAT);
-static void init_cache_modes(void);
void pat_disable(const char *reason)
{
@@ -65,9 +64,11 @@ static int __init nopat(char *str)
}
early_param("nopat", nopat);
+static bool __read_mostly __pat_initialized = false;
+
bool pat_enabled(void)
{
- return !!__pat_enabled;
+ return __pat_initialized;
}
EXPORT_SYMBOL_GPL(pat_enabled);
@@ -225,6 +226,7 @@ static void pat_bsp_init(u64 pat)
}
wrmsrl(MSR_IA32_CR_PAT, pat);
+ __pat_initialized = true;
__init_cache_modes(pat);
}
@@ -242,7 +244,7 @@ static void pat_ap_init(u64 pat)
wrmsrl(MSR_IA32_CR_PAT, pat);
}
-static void init_cache_modes(void)
+void init_cache_modes(void)
{
u64 pat = 0;
static int init_cm_done;
@@ -306,7 +308,7 @@ void pat_init(void)
u64 pat;
struct cpuinfo_x86 *c = &boot_cpu_data;
- if (!pat_enabled()) {
+ if (!__pat_enabled) {
init_cache_modes();
return;
}
Index: linux-stable/arch/x86/include/asm/pat.h
===================================================================
--- linux-stable.orig/arch/x86/include/asm/pat.h
+++ linux-stable/arch/x86/include/asm/pat.h
@@ -7,6 +7,7 @@
bool pat_enabled(void);
void pat_disable(const char *reason);
extern void pat_init(void);
+extern void init_cache_modes(void);
extern int reserve_memtype(u64 start, u64 end,
enum page_cache_mode req_pcm, enum page_cache_mode *ret_pcm);
Index: linux-stable/arch/x86/kernel/setup.c
===================================================================
--- linux-stable.orig/arch/x86/kernel/setup.c
+++ linux-stable/arch/x86/kernel/setup.c
@@ -1070,6 +1070,8 @@ void __init setup_arch(char **cmdline_p)
/* update e820 for memory not covered by WB MTRRs */
mtrr_bp_init();
+ if (!pat_enabled())
+ init_cache_modes();
if (mtrr_trim_uncached_memory(max_pfn))
max_pfn = e820__end_of_ram_pfn();