Re: [PATCH v2] X86: don't report PAT on CPUs that don't support it

[Mikulas Patocka]

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@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();
>  
>