lkml.org 
[lkml]   [2017]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] X86: don't report PAT on CPUs that don't support it


    On Tue, 6 Jun 2017, Andy Lutomirski wrote:

    > On Tue, Jun 6, 2017 at 3:49 PM, Mikulas Patocka <mpatocka@redhat.com> wrote:
    > >
    > >
    > > On Sun, 28 May 2017, Andy Lutomirski wrote:
    > >
    > >> On Sun, May 28, 2017 at 11:18 AM, Bernhard Held <berny156@gmx.de> 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@redhat.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.
    >
    > Why? Shouldn't calling init_cache_modes() be sufficient?
    >
    > --Andy

    See the function arch_phys_wc_add():

    if (pat_enabled() || !mtrr_enabled())
    return 0; /* Success! (We don't need to do anything.) */
    ret = mtrr_add(base, size, MTRR_TYPE_WRCOMB, true);

    - if pat_enabled() returns true, that function doesn't set MTRRs.
    pat_enabled() must return false on systems without PAT, so that MTRRs are
    set.

    Mikulas

    \
     
     \ /
      Last update: 2017-06-12 00:56    [W:2.385 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site