Re: [5.17 regression] "x86/PCI: Ignore E820 reservations for bridge windows on newer systems" breaks suspend/resume

[Hans de Goede]

Hi,

On 2/8/22 16:25, Hans de Goede wrote:
> Hi All,
> 
> Unfortunately I've just learned that commit 7f7b4236f204 ("x86/PCI:
> Ignore E820 reservations for bridge windows on newer systems"):
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7f7b4236f2040d19df1ddaf30047128b41e78de7
> 
> breaks suspend/resume on at least one laptop model, the Lenovo ThinkPad
> X1 gen 2, see:
> https://bugzilla.redhat.com/show_bug.cgi?id=2029207
> 
> This regression was actually caught be Fedora already carrying this
> patch for a while now and as such it has been reproduced with 5.15
> with an older version of the patch which still allowed turning the
> new behavior of by adding "pci=use_e820". Dmesg output with and
> without the option has just been attached to the bug, I've not
> analyzed this any further yet.
> 
> I guess that for now this means that we need to revert commit
> 7f7b4236f204. Rafael, I'll send you a revert with a commit msg
> explaining why this needs to be reverted tomorrow.
> 
> More interesting IMHO is finding out another solution. Both the touchpad
> problem which got me looking into this:
> https://bugzilla.redhat.com/show_bug.cgi?id=1868899
> 
> As well as the thunderbolt hotplug issue Mika was looking at:
> https://bugzilla.kernel.org/show_bug.cgi?id=206459
> 
> both are cases where we fail to find a memory-window for a
> BAR which has not been setup yet.
> 
> So I see a couple of options here:
> 
> 1. Detect that the e820 reservations fully cover (one of)
> the PCI bridge main 32 bit memory windows and if that happens
> ignore them. This actually was my first plan when I started
> working on this. In the end I choose the other option
> because Bjorn indicated that in hindsight honoring the e820
> reservations might have been a mistake and maybe we should
> get rid of honoring them all together.
> 
> 2. Have a flag which, when we fail to alloc a 32 bit
> (or 64 bit) memory PCI BAR, is set if not already set
> and then retry the alloc. And make the e820 reservation
> carve-out get skipped in this case.
> 
> 3. When booting with pci=nocrs as a workaround for
> the touchpad case a 64 but memory window ends up getting
> used. There already is some special handling for some
> AMD bridges where if there are no 64 bit memory Windows
> in the _CRS for the bridge, one gets added. Maybe we need
> to do the same for Intel bridges ?

4. It seems that all devices which have issues with allocating
a PCI bar are Ice Lake based; and the model where the ignoring
of e820 reservations has been reported to cause issues is somewhat
old. It is a Haswell, but still getting BIOS updates causing
the BIOS date check to enable the new behavior. So another
solution might be to only ignore e820 reservations on machines
with Intel Ice Lake (and presumably also Tiger Lake) CPUs.


5. It also seems that the troublesome e820 entry on all devices
ends at 0xcfffffff and starts well below 0x8000000 :

Yoga C940:
[    0.000000] BIOS-e820: [mem 0x000000004bc50000-0x00000000cfffffff] reserved

IdeaPad 3 15IIL05:
[    0.000000] BIOS-e820: [mem 0x000000004bc50000-0x00000000cfffffff] reserved

Lenovo IdeaPad 5 14IIL05:
[    0.000000] BIOS-e820: [mem 0x000000005bc50000-0x00000000cfffffff] reserved


Actually even the lower addresses seem to follow a pattern.

So I guess that we could do a really narrow fix here where we only
ignore the *this* e820 reservation using the properties that:

a) It is "huge"
b) Always ends at 0x00000000cfffffff

To identify it.



I was at first tempted to choose 1 as fix, which basically is also
based on this huge reservation covering the entire 32 bit mem window of
the pci-bridge(s) in the system. But that seems to boil down to the same
thing as 5. Where as 5 can be a really narrowly tailored fix, requiring
only changes to arch/x86/kernel/resource.c . So atm I'm leaning towards
trying 5.  As already said, plase let me know what you think.

Regards,

Hans