From: Hans de Goede <hdegoede@redhat.com>
To: Bjorn Helgaas <helgaas@kernel.org>,
"Rafael J . Wysocki" <rjw@rjwysocki.net>,
Borislav Petkov <bp@alien8.de>, "H . Peter Anvin" <hpa@zytor.com>,
Ingo Molnar <mingo@redhat.com>
Cc: "Mika Westerberg" <mika.westerberg@linux.intel.com>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Myron Stowe" <myron.stowe@redhat.com>,
"Juha-Pekka Heikkila" <juhapekka.heikkila@gmail.com>,
"Benoit Grégoire" <benoitg@coeus.ca>,
"Hui Wang" <hui.wang@canonical.com>,
"Kai-Heng Feng" <kai.heng.feng@canonical.com>,
linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org,
"Bjorn Helgaas" <bhelgaas@google.com>,
wse@tuxedocomputers.com
Subject: Re: [PATCH 3/3] x86/PCI: Preserve host bridge windows completely covered by E820
Date: Fri, 4 Mar 2022 15:16:42 +0100 [thread overview]
Message-ID: <c7233c9a-8aa9-edb2-f3a7-1bcaa5a880d2@redhat.com> (raw)
In-Reply-To: <20220304035110.988712-4-helgaas@kernel.org>
Hi Bjorn,
On 3/4/22 04:51, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Many folks have reported PCI devices not working. It could affect any
> device, but most reports are for Thunderbolt controllers on Lenovo Yoga and
> Clevo Barebone laptops and the touchpad on Lenovo IdeaPads.
>
> In every report, a region in the E820 table entirely encloses a PCI host
> bridge window from _CRS, and because of 4dc2287c1805 ("x86: avoid E820
> regions when allocating address space"), we ignore the entire window,
> preventing us from assigning space to PCI devices.
>
> For example, the dmesg log [2] from bug report [1] shows:
>
> BIOS-e820: [mem 0x000000004bc50000-0x00000000cfffffff] reserved
> pci_bus 0000:00: root bus resource [mem 0x65400000-0xbfffffff window]
> pci 0000:00:15.0: BAR 0: no space for [mem size 0x00001000 64bit]
>
> The efi=debug dmesg log [3] from the same report shows the EFI memory map
> entries that created the E820 map:
>
> efi: mem47: [Reserved | |WB|WT|WC|UC] range=[0x4bc50000-0x5fffffff]
> efi: mem48: [Reserved | |WB| | |UC] range=[0x60000000-0x60ffffff]
> efi: mem49: [Reserved | | | | | ] range=[0x61000000-0x653fffff]
> efi: mem50: [MMIO |RUN| | | |UC] range=[0x65400000-0xcfffffff]
>
> 4dc2287c1805 ("x86: avoid E820 regions when allocating address space")
> works around issues where _CRS contains non-window address space that can't
> be used for PCI devices. It does this by removing E820 regions from host
> bridge windows. But in these reports, the E820 region covers the entire
> window, so 4dc2287c1805 makes it completely unusable.
>
> Per UEFI v2.8, sec 7.2, the EfiMemoryMappedIO type means:
>
> Used by system firmware to request that a memory-mapped IO region be
> mapped by the OS to a virtual address so it can be accessed by EFI
> runtime services.
>
> A host bridge window is definitely a memory-mapped IO region, and EFI
> runtime services may need to access it, so I don't think we can argue that
> this is a firmware defect.
>
> Instead, change the 4dc2287c1805 strategy so it only removes E820 regions
> when they overlap *part* of a host bridge window on the assumption that a
> partial overlap is really register space, not part of the window proper.
>
> If an E820 region covers the entire window from _CRS, assume the _CRS
> window is correct and do nothing.
>
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1868899
> [2] https://bugzilla.redhat.com/attachment.cgi?id=1711424
> [3] https://bugzilla.redhat.com/attachment.cgi?id=1861407
>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=206459
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=214259
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1868899
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1871793
> BugLink: https://bugs.launchpad.net/bugs/1878279
> BugLink: https://bugs.launchpad.net/bugs/1931715
> BugLink: https://bugs.launchpad.net/bugs/1932069
> BugLink: https://bugs.launchpad.net/bugs/1921649
> Fixes: 4dc2287c1805 ("x86: avoid E820 regions when allocating address space")
> Link: https://lore.kernel.org/r/20220228105259.230903-1-hdegoede@redhat.com
> Based-on-patch-by: Hans de Goede <hdegoede@redhat.com>
> Reported-by: Benoit Grégoire <benoitg@coeus.ca> # BZ 206459
> Reported-by: wse@tuxedocomputers.com # BZ 214259
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> arch/x86/kernel/resource.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/x86/kernel/resource.c b/arch/x86/kernel/resource.c
> index 7378ea146976..405f0af53e3d 100644
> --- a/arch/x86/kernel/resource.c
> +++ b/arch/x86/kernel/resource.c
> @@ -39,6 +39,17 @@ void remove_e820_regions(struct device *dev, struct resource *avail)
> e820_start = entry->addr;
> e820_end = entry->addr + entry->size - 1;
>
> + /*
> + * If an E820 entry covers just part of the resource, we
> + * assume E820 is telling us about something like host
> + * bridge register space that is unavailable for PCI
> + * devices. But if it covers the *entire* resource, it's
> + * more likely just telling us that this is MMIO space, and
> + * that doesn't need to be removed.
> + */
> + if (e820_start <= avail->start && avail->end <= e820_end)
> + continue;
> +
IMHO it would be good to add some logging here, since hitting this is
somewhat of a special case. For the Fedora test kernels I did I changed
this to:
if (e820_start <= avail->start && avail->end <= e820_end) {
dev_info(dev, "resource %pR fully covered by e820 entry [mem %#010Lx-%#010Lx]\n",
avail, e820_start, e820_end);
continue;
}
And I expect/hope to see this new info message on the ideapad with the
touchpad issue.
Regards,
Hans
> resource_clip(avail, e820_start, e820_end);
> if (orig.start != avail->start || orig.end != avail->end) {
> dev_info(dev, "clipped %pR to %pR for e820 entry [mem %#010Lx-%#010Lx]\n",
next prev parent reply other threads:[~2022-03-04 14:16 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-04 3:51 [PATCH 0/3] x86/PCI: Clip only partial E820 overlaps Bjorn Helgaas
2022-03-04 3:51 ` [PATCH 1/3] x86/PCI: Eliminate remove_e820_regions() common subexpressions Bjorn Helgaas
2022-03-04 3:51 ` [PATCH 2/3] x86/PCI: Log host bridge window clipping for E820 regions Bjorn Helgaas
2022-03-04 3:51 ` [PATCH 3/3] x86/PCI: Preserve host bridge windows completely covered by E820 Bjorn Helgaas
2022-03-04 14:16 ` Hans de Goede [this message]
2022-03-04 15:32 ` Bjorn Helgaas
2022-03-04 15:46 ` Hans de Goede
2022-03-04 18:34 ` Bjorn Helgaas
2022-03-05 10:37 ` Hans de Goede
2022-03-07 10:02 ` Hans de Goede
2022-03-08 14:52 ` Rafael J. Wysocki
2022-03-09 18:15 ` Bjorn Helgaas
2022-03-10 12:28 ` Hans de Goede
2022-03-11 7:52 ` Hans de Goede
2022-03-11 16:24 ` Bjorn Helgaas
2022-03-11 15:13 ` Hans de Goede
2022-03-04 14:15 ` [PATCH 0/3] x86/PCI: Clip only partial E820 overlaps Hans de Goede
2022-03-04 15:21 ` Mika Westerberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c7233c9a-8aa9-edb2-f3a7-1bcaa5a880d2@redhat.com \
--to=hdegoede@redhat.com \
--cc=benoitg@coeus.ca \
--cc=bhelgaas@google.com \
--cc=bp@alien8.de \
--cc=helgaas@kernel.org \
--cc=hpa@zytor.com \
--cc=hui.wang@canonical.com \
--cc=juhapekka.heikkila@gmail.com \
--cc=kai.heng.feng@canonical.com \
--cc=kw@linux.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--cc=mingo@redhat.com \
--cc=myron.stowe@redhat.com \
--cc=rjw@rjwysocki.net \
--cc=wse@tuxedocomputers.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).