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

[Andy Lutomirski]

On Tue, Jun 6, 2017 at 3:49 PM, Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
>
>
> 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.

Why?  Shouldn't calling init_cache_modes() be sufficient?

--Andy