[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LRH.2.02.1707030102320.1593@file01.intranet.prod.int.rdu2.redhat.com>
Date: Mon, 3 Jul 2017 01:05:52 -0400 (EDT)
From: Mikulas Patocka <mpatocka@...hat.com>
To: Ingo Molnar <mingo@...nel.org>
cc: Bernhard Held <berny156@....de>, Andy Lutomirski <luto@...nel.org>,
Toshi Kani <toshi.kani@...com>, Borislav Petkov <bp@...en8.de>,
Andrew Morton <akpm@...ux-foundation.org>,
Brian Gerst <brgerst@...il.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
"H. Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
"Luis R. Rodriguez" <mcgrof@...e.com>,
Denys Vlasenko <dvlasenk@...hat.com>,
Josh Poimboeuf <jpoimboe@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] X86: don't report PAT on CPUs that don't support it
On Tue, 6 Jun 2017, Mikulas Patocka wrote:
> 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
Is there any progress with this patch? Will you accept it or do you want
some changes to it?
> From: Mikulas Patocka <mpatocka@...hat.com>
>
> 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@...hat.com>
> Cc: stable@...r.kernel.org # 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();
>
>
Powered by blists - more mailing lists