On Mon, Mar 28, 2022 at 02:54:42PM +0200, Hans de Goede wrote:
> Hi,
>
> On 3/24/22 23:19, Mark Brown wrote:
> > On Thu, Mar 24, 2022 at 09:34:30PM +0100, Hans de Goede wrote:
> >
> >> Mark, if one of use writes a test patch, can you get that Asus machine to boot a
> >> kernel build from next + the test patch ?
> >
> > I can't directly unfortunately as the board is in Collabora's lab but
> > Guillaume (who's already CCed) ought to be able to, and I can generally
> > prod and try to get that done.
>
> Ok, Guillaume, can you try a kernel with commit 5949965ec9340cfc0e65f7d8a576b660b26e2535
> ("x86/PCI: Preserve host bridge windows completely covered by E820") + the
> attached patch added on top a try on the asus-C523NA-A20057-coral machine please
> and see if that makes it boot again ?
>
> Regards,
>
> Hans
> From b8080a6d2d889847900e1408f71d0c01c73f5c94 Mon Sep 17 00:00:00 2001
> From: Hans de Goede <hdegoede@xxxxxxxxxx>
> Date: Mon, 28 Mar 2022 14:47:41 +0200
> Subject: [PATCH] x86/PCI: Limit "e820 entry fully covers window" check to non
> ISA MMIO addresses
>
> Commit FIXME ("x86/PCI: Preserve host bridge windows completely
> covered by E820") added a check to skip e820 table entries which
> fully cover a PCI bride's memory window when clipping PCI bridge
> memory windows.
>
> This check also caused ISA MMIO windows to not get clipped when
> fully covered, which is causing some coreboot based Chromebooks
> to not boot.
>
> Modify the fully covered check to not apply to ISA MMIO windows.
I'd like to include URLs to the kernelci results unless they are
ephemeral. There's a lot of valuable information in these:
Asus C523NA-A20057-coral with the last good commit:
https://lava.collabora.co.uk/scheduler/job/5937945
https://storage.kernelci.org/next/master/next-20220310/x86_64/x86_64_defconfig+x86-chromebook/gcc-10/lab-collabora/baseline-asus-C523NA-A20057-coral.html
https://storage.kernelci.org/next/master/next-20220310/x86_64/x86_64_defconfig+x86-chromebook/gcc-10/lab-collabora/baseline-hp-x360-12b-n4000-octopus.html
> Fixes: FIXME ("x86/PCI: Preserve host bridge windows completely covered by E820")
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> arch/x86/kernel/resource.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/resource.c b/arch/x86/kernel/resource.c
> index 6be82e16e5f4..d9ec913619c3 100644
> --- a/arch/x86/kernel/resource.c
> +++ b/arch/x86/kernel/resource.c
> @@ -46,8 +46,12 @@ void remove_e820_regions(struct device *dev, struct resource *avail)
> * 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.
> + * Note this *entire* resource covering check is only
> + * intended for 32 bit memory resources for the 16 bit
> + * isa window we always apply the e820 entries.
> */
> - if (e820_start <= avail->start && avail->end <= e820_end) {
> + if (avail->start >= ISA_END_ADDRESS &&
What is the justification for needing to check ISA_END_ADDRESS here?
The commit log basically says "this makes it work", which isn't very
satisfying.
The Asus log of the last good commit shows:
PCI: 00:0d.0 [8086/5a92] enabled
constrain_resources: PCI: 00:0d.0 10 base d0000000 limit d0ffffff mem (fixed)
...
BIOS-e820: [mem 0x000000007b000000-0x000000007fffffff] reserved
BIOS-e820: [mem 0x00000000d0000000-0x00000000d0ffffff] reserved
BIOS-e820: [mem 0x00000000e0000000-0x00000000efffffff] reserved
...
acpi PNP0A08:00: clipped [mem 0x000a0000-0x000bffff window] to [mem 0x00100000-0x000bffff window] for e820 entry [mem 0x000a0000-0x000fffff]
acpi PNP0A08:00: clipped [mem 0x7b800000-0x7fffffff window] to [mem 0x80000000-0x7fffffff window] for e820 entry [mem 0x7b000000-0x7fffffff]
acpi PNP0A08:00: clipped [mem 0x80000000-0xe0000000 window] to [mem 0x80000000-0xcfffffff window] for e820 entry [mem 0xd0000000-0xd0ffffff]
acpi PNP0A08:00: ignoring host bridge window [mem 0x00100000-0x000bffff window] (conflicts with PCI mem [mem 0x00000000-0x7fffffffff])
acpi PNP0A08:00: ignoring host bridge window [mem 0x80000000-0x7fffffff window] (conflicts with PCI mem [mem 0x00000000-0x7fffffffff])
It looks like _CRS gave us [mem 0x80000000-0xe0000000 window], which
is one byte too big (it should end at 0xdfffffff).
