Re: Ping: [PATCH] x86/PAT: have pat_enabled() properly reflect state when running on e.g. Xen

[Chuck Zmudzinski <brchuckz@xxxxxxxxxxxx>]

On 7/14/2022 1:17 PM, Chuck Zmudzinski wrote:
> On 7/5/22 6:57 AM, Thorsten Leemhuis wrote:
> > [CCing tglx, mingo, Boris and Juergen]
> >
> > On 04.07.22 14:26, Jan Beulich wrote:
> > > On 04.07.2022 13:58, Thorsten Leemhuis wrote:
> > >> On 25.05.22 10:55, Jan Beulich wrote:
> > >>> On 28.04.2022 16:50, Jan Beulich wrote:
> > >>>> The latest with commit bdd8b6c98239 ("drm/i915: replace X86_FEATURE_PAT
> > >>>> with pat_enabled()") pat_enabled() returning false (because of PAT
> > >>>> initialization being suppressed in the absence of MTRRs being announced
> > >>>> to be available) has become a problem: The i915 driver now fails to
> > >>>> initialize when running PV on Xen (i915_gem_object_pin_map() is where I
> > >>>> located the induced failure), and its error handling is flaky enough to
> > >>>> (at least sometimes) result in a hung system.
> > >>>>
> > >>>> Yet even beyond that problem the keying of the use of WC mappings to
> > >>>> pat_enabled() (see arch_can_pci_mmap_wc()) means that in particular
> > >>>> graphics frame buffer accesses would have been quite a bit less
> > >>>> performant than possible.
> > >>>>
> > >>>> Arrange for the function to return true in such environments, without
> > >>>> undermining the rest of PAT MSR management logic considering PAT to be
> > >>>> disabled: Specifically, no writes to the PAT MSR should occur.
> > >>>>
> > >>>> For the new boolean to live in .init.data, init_cache_modes() also 
> > >>>> needs
> > >>>> moving to .init.text (where it could/should have lived already before).
> > >>>>
> > >>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > >>>
> > >>> The Linux kernel regression tracker is pestering me because things are
> > >>> taking so long (effectively quoting him), and alternative proposals
> > >>> made so far look to have more severe downsides.
> > >>
> > >> Has any progress been made with this patch? It afaics is meant to fix
> > >> this regression, which ideally should have been fixed weeks ago (btw:
> > >> adding a "Link:" tag pointing to it would be good):
> > >> https://lore.kernel.org/regressions/YnHK1Z3o99eMXsVK@mail-itl/
> > >>
> > >> According to Juergen it's still needed:
> > >> https://lore.kernel.org/lkml/c5515533-29a9-9e91-5a36-45f00f25b37b@xxxxxxxx/
> > >>
> > >> Or was a different solution found to fix that regression?
> > > 
> > > No progress and no alternatives I'm aware of.
> >
> > Getting closer to the point where I need to bring this to Linus
> > attention. I hope this mail can help avoiding this.
> >
> > Jan, I didn't follow this closely, but do you have any idea why Dave,
> > Luto, and Peter are ignoring this? Is reverting bdd8b6c98239 a option to
> > get the regression fixed? Would a repost maybe help getting this rolling
> > again?
>
> Hi, Thorsten,
>
> Here is a link to the hardware probe of my system which exhibits
> a system hang before fully booting with bdd8b6c98239. Without
> bdd8b6c98239, the problem is gone:
>
> https://linux-hardware.org/?probe=32e615b538
>
> Keep in mind this problem is not seen with bdd8b6c98239
> on the bare metal, but only when running as a traditional Dom0
> PV type guest on Xen. I don't know see the problem on Xen HVM
> DomU, and I have not tested it on Xen PVH DomU, Xen PV DomU,
> or the experimental Xen PVH Dom0.

Update: On affected hardware, you do not need to run in a
Xen PV Dom0 to see the regression caused by bdd8b6c98239.

All you need to do is run, on the bare metal, on the affected
hardware, with the Linux kernel nopat boot option.

Jan mentions in his commit message the function in the i915
driver that was touched by bdd8b6c98239 and that causes this
regression. That is, any Intel IGD that needs to execute the
function that Jan mentions in the commit message of his
proposed patch when the i915 driver is setting up the graphics
engine will most likely be hardware that is affected. My Intel
IGD was marketed as HD Graphics 4600, I think.

So find an a system with these hardware characteristics, and
try running, with the nopat option, the Linux kernel, with
and without bdd8b6c98239. You will see the regression I
am experiencing, I predict.

Chuck

>
> You can probably verify yourself that reverting bdd8b6c98239
> fixes the regression if you try to reproduce the regression with
> any Linux version that has bdd8b6c98239 or its equivalent on
> the stable branches with a hardware profile similar to the link
> to the profile of my machine which exhibits the problem. Mine
> is a Haswell core-i5 4590S CPU and ASRock B85M-Pro4
> motherboard.
>
> Also, other notes:
>
> 1. Yes, AFAICT, Marek at Qubes OS is the first to report the problem.
> 2. Juergen Gross' work to try to fix this has been helpful, but none
> of his posted patches has fixed the regression on my system.
> 3. Jan's patch fixes it also, and so do the two patches I posted to lkml
> earlier this week to the appropriate maintainers.
> 4. On the pkg-xen-devel mailing list, which is public, this issue was
> briefly discussed where I first reported it. Someone there said they
> did not see the issue with Broadwell Xeons. Mine is a Haswell core i5,
> which is one generation older than Broadwell, so you are most likely
> to be able to reproduce the problem with a Haswell core i5 desktop
> system like my ASRock system, which was my own private build
> which has been working fine for eight years until Linux 5.17.y.
>
> Hope this helps.
>
> Chuck
>
> > BTW, for anyone new to this, Jan's patch afaics is supposed to fix the
> > regression reported here:
> > https://lore.kernel.org/all/YnHK1Z3o99eMXsVK@mail-itl/
> >
> > Side note: Juergen Gross recently posted related patches in this code
> > area to fix some other problems (regressions?), but his efforts look
> > stalled, too:
> > https://lore.kernel.org/all/ddb0cc0d-cefc-4f33-23f8-3a94c7c51a49@xxxxxxxx/
> >
> > And he recently stated this Jan's patch is still needed, even if his
> > changes make it in.
> > https://lore.kernel.org/all/c5515533-29a9-9e91-5a36-45f00f25b37b@xxxxxxxx/
> >
> > This from my point all looks a bit... unsatisfying. :-/
> >
> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> >
> > P.S.: As the Linux kernel's regression tracker I deal with a lot of
> > reports and sometimes miss something important when writing mails like
> > this. If that's the case here, don't hesitate to tell me in a public
> > reply, it's in everyone's interest to set the public record straight.
>