On Tue, 18 Apr 2017, H. Peter Anvin wrote:
> On 04/18/17 12:07, Mikulas Patocka wrote: > > 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. > > > > Also, I changed boot_cpu_has(X86_FEATURE_PAT) to > > this_cpu_has(X86_FEATURE_PAT) in pat_ap_init, so that we check the PAT > > feature on the processor that is being initialized. > > > > I'm thinking it would be better to replace __pat_enabled with > static_cpu_has(X86_FEATURE_PAT) and disable the feature bit if the user > has disabled it on the command line, just as we do with other features. > > -hpa If MTRR initialization fails for whatever reason, then pat_init() won't be called and the kernel would mistakenly believe that PAT is working (because there would be no one to clear X86_FEATURE_PAT). I think that pat should be reported only if pat_init() is called and succeeds. Another strange thing: pat_disable() calls init_cache_modes() - but since pat_disable() may not be called at all, it is possible that init_cache_modes() is also not called at all. It doesn't produce any visible misbehavior on my machine, but it doesn't seem right - we should not call init_cache_modes() from pat_disable() and do the initialization elsewhere, where it is always called. Mikulas
