[+cc Myron, just FYI] On Thu, Jun 24, 2021 at 05:53:24PM +0800, Hui Wang wrote: > The touchpad can't work on many Lenovo Ideapad S145/BS145 laptops. > But it works well under Windows. > > On those machines, the touchpad is an I2C device, while the I2C host > controllers fail to initialize as below shown: > pci 0000:00:15.0: BAR 0: no space for [mem size 0x00001000 64bit] > pci 0000:00:15.0: BAR 0: failed to assign [mem size 0x00001000 64bit] > pci 0000:00:15.1: BAR 0: no space for [mem size 0x00001000 64bit] > pci 0000:00:15.1: BAR 0: failed to assign [mem size 0x00001000 64bit] > > The BIOS assigns iomem space to host bridge and I2C host controller > like this: > pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000bffff window] > pci_bus 0000:00: root bus resource [mem 0x65400000-0xbfffffff window] > > pci 0000:00:15.0: [8086:34e8] type 00 class 0x0c8000 > pci 0000:00:15.0: reg 0x10: [mem 0x00000000-0x00000fff 64bit] > pci 0000:00:15.1: [8086:34e9] type 00 class 0x0c8000 > pci 0000:00:15.1: reg 0x10: [mem 0x00000000-0x00000fff 64bit] > > The I2C host controllers need to allocate iomem space from root > bridge, but all iomem window of the root bridge are overlapped with > BIOS-e820 mapped region, that makes the allocate_resource() fail: > BIOS-e820: [mem 0x0000000000000000-0x000000000009efff] usable > BIOS-e820: [mem 0x000000000009f000-0x00000000000fffff] reserved > ... > BIOS-e820: [mem 0x000000004bc50000-0x00000000cfffffff] reserved > > We could add "pci=nocrs" to make the touchpad work, but users expect > the touchpad to work out-of-box under Linux distro as under Windows. > > Here design a generic solution for x86 machines, if host bridge uses > crs, we will check if all root bridge iomem windows are overlapped > with BIOS-e820 mapped region, if yes, we try to build a non-e820 > mapped window according to the biggest gap in the e820 mapped region, > and we need to clip this window with MMCONFIG region if this region > is not mapped by BIOS-e820, then insert this window to the tail of > the root bridge. > > After this change, the I2C host controller could allocate the iomem > region from host bridge successfully and the touchpad could work > out-of-box. I see from https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1931715 that a fix has been committed, I assume to the Ubuntu kernel, but I assume this is still a problem for upstream and probably other distro kernels. The 0-day bot found some issue, which I haven't looked at: https://lore.kernel.org/r/20210627143859.GD17986@xsang-OptiPlex-9020 I think this patch might be the wrong approach. We learned from _CRS that [mem 0x65400000-0xbfffffff window] is assigned to the PNP0A08:00 host bridge. The e820 "[mem 0x000000004bc50000-0x00000000cfffffff] reserved" region certainly includes all of that PCI aperture, and this entry should prevent that space from being allocated to devices that haven't already been assigned space. But I don't think that e820 entry should prevent the PCI core from managing that PCI aperture. The platform told us that PNP0A08:00 owns the [mem 0x65400000-0xbfffffff window], and the PNP0A08:00 driver (pci_root.c) should be able to manage it as needed. > BugLink: http://bugs.launchpad.net/bugs/1931715 > BugLink: http://bugs.launchpad.net/bugs/1932069 > BugLink: http://bugs.launchpad.net/bugs/1921649 > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > Cc: Yinghai Lu <yinghai@xxxxxxxxxx> > Cc: Ingo Molnar <mingo@xxxxxxxxxx> > Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > Cc: linux-acpi@xxxxxxxxxxxxxxx > Cc: linux-pci@xxxxxxxxxxxxxxx > Signed-off-by: Hui Wang <hui.wang@xxxxxxxxxxxxx> > --- > arch/x86/include/asm/e820/api.h | 1 + > arch/x86/include/asm/pci.h | 2 ++ > arch/x86/include/asm/pci_x86.h | 1 + > arch/x86/kernel/e820.c | 2 ++ > arch/x86/pci/acpi.c | 46 ++++++++++++++++++++++++++++++++- > arch/x86/pci/mmconfig-shared.c | 28 ++++++++++++++++++++ > 6 files changed, 79 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/e820/api.h b/arch/x86/include/asm/e820/api.h > index e8f58ddd06d9..46c76201dc28 100644 > --- a/arch/x86/include/asm/e820/api.h > +++ b/arch/x86/include/asm/e820/api.h > @@ -9,6 +9,7 @@ extern struct e820_table *e820_table_kexec; > extern struct e820_table *e820_table_firmware; > > extern unsigned long pci_mem_start; > +extern unsigned long pci_mem_gap_size; > > extern bool e820__mapped_raw_any(u64 start, u64 end, enum e820_type type); > extern bool e820__mapped_any(u64 start, u64 end, enum e820_type type); > diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h > index d2c76c8d8cfd..a940a792ef7b 100644 > --- a/arch/x86/include/asm/pci.h > +++ b/arch/x86/include/asm/pci.h > @@ -81,6 +81,8 @@ static inline int pcibios_assign_all_busses(void) { return 0; } > #endif > > extern unsigned long pci_mem_start; > +extern unsigned long pci_mem_gap_size; > + > #define PCIBIOS_MIN_IO 0x1000 > #define PCIBIOS_MIN_MEM (pci_mem_start) > > diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h > index 490411dba438..3ea30aed9100 100644 > --- a/arch/x86/include/asm/pci_x86.h > +++ b/arch/x86/include/asm/pci_x86.h > @@ -171,6 +171,7 @@ extern int pci_mmconfig_delete(u16 seg, u8 start, u8 end); > extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus); > extern struct pci_mmcfg_region *__init pci_mmconfig_add(int segment, int start, > int end, u64 addr); > +extern void pci_mmconfig_clip_resource(struct resource *res); > > extern struct list_head pci_mmcfg_list; > > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c > index bc0657f0deed..2933745b19b7 100644 > --- a/arch/x86/kernel/e820.c > +++ b/arch/x86/kernel/e820.c > @@ -66,6 +66,7 @@ struct e820_table *e820_table_firmware __refdata = &e820_table_firmware_init; > > /* For PCI or other memory-mapped resources */ > unsigned long pci_mem_start = 0xaeedbabe; > +unsigned long pci_mem_gap_size; > #ifdef CONFIG_PCI > EXPORT_SYMBOL(pci_mem_start); > #endif > @@ -677,6 +678,7 @@ __init void e820__setup_pci_gap(void) > * e820__reserve_resources_late() protects stolen RAM already: > */ > pci_mem_start = gapstart; > + pci_mem_gap_size = gapsize; > > pr_info("[mem %#010lx-%#010lx] available for PCI devices\n", > gapstart, gapstart + gapsize - 1); > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c > index 948656069cdd..e3a691024683 100644 > --- a/arch/x86/pci/acpi.c > +++ b/arch/x86/pci/acpi.c > @@ -231,6 +231,11 @@ static void teardown_mcfg_map(struct acpi_pci_root_info *ci) > info->mcfg_added = false; > } > } > + > +static void clip_resource_from_mmcfg(struct resource *res) > +{ > + return pci_mmconfig_clip_resource(res); > +} > #else > static int setup_mcfg_map(struct acpi_pci_root_info *ci) > { > @@ -240,6 +245,10 @@ static int setup_mcfg_map(struct acpi_pci_root_info *ci) > static void teardown_mcfg_map(struct acpi_pci_root_info *ci) > { > } > + > +static void clip_resource_from_mmcfg(struct resource *res) > +{ > +} > #endif > > static int pci_acpi_root_get_node(struct acpi_pci_root *root) > @@ -296,13 +305,48 @@ static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci) > struct acpi_device *device = ci->bridge; > int busnum = ci->root->secondary.start; > struct resource_entry *entry, *tmp; > + bool has_non_e820_region = false; > int status; > > status = acpi_pci_probe_root_resources(ci); > if (pci_use_crs) { > - resource_list_for_each_entry_safe(entry, tmp, &ci->resources) > + resource_list_for_each_entry_safe(entry, tmp, &ci->resources) { > + struct resource avail = *entry->res; > + > if (resource_is_pcicfg_ioport(entry->res)) > resource_list_destroy_entry(entry); > + > + if (avail.flags & IORESOURCE_MEM) { > + arch_remove_reservations(&avail); > + if (avail.end > avail.start) > + has_non_e820_region = true; > + } > + } > + > + /* all bridge windows are in the BIOS-e820 mapped region, this > + * will make allocate_resource() fail when PCI devices request > + * iomem address from bridge. To fix it, we try to build a non > + * e820 mapped iomem resource and clip it with MMCONFIG region, > + * then add it to the bridge window list. > + */ > + if (!has_non_e820_region) { > + struct resource_entry *rentry; > + struct resource avail; > + > + avail.start = pci_mem_start; > + avail.end = pci_mem_start + pci_mem_gap_size - 1; > + avail.flags = IORESOURCE_MEM | IORESOURCE_WINDOW; > + avail.name = ci->name; > + clip_resource_from_mmcfg(&avail); > + if (avail.end > avail.start) { > + rentry = resource_list_create_entry(NULL, 0); > + if (rentry) { > + *rentry->res = avail; > + resource_list_add_tail(rentry, &ci->resources); > + } > + } > + } > + > return status; > } > > diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c > index de6bf0e7e8f8..1f3ffb1bcdc8 100644 > --- a/arch/x86/pci/mmconfig-shared.c > +++ b/arch/x86/pci/mmconfig-shared.c > @@ -813,3 +813,31 @@ int pci_mmconfig_delete(u16 seg, u8 start, u8 end) > > return -ENOENT; > } > + > +void pci_mmconfig_clip_resource(struct resource *res) > +{ > + struct pci_mmcfg_region *cfg; > + resource_size_t start, end; > + resource_size_t low = 0, high = 0; > + > + /* Refers to the resource_clip() in the x86/kernel/resource.c */ > + list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) { > + start = cfg->res.start; > + end = cfg->res.end; > + > + if (res->end < start || res->start > end) > + continue; /* no conflict */ > + > + if (res->start < start) > + low = start - res->start; > + > + if (res->end > end) > + high = res->end - end; > + > + /* Keep the area above or below the conflict, whichever is larger */ > + if (low > high) > + res->end = start - 1; > + else > + res->start = end + 1; > + } > +} > -- > 2.25.1 >
