From 197e0da1f1a3445b9b266f83d5d037b4709dae2e Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Fri, 24 Nov 2023 11:09:13 +0200 Subject: x86/pci: Use PCI_HEADER_TYPE_* instead of literals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace 0x7f and 0x80 literals with PCI_HEADER_TYPE_* defines. Link: https://lore.kernel.org/r/20231124090919.23687-1-ilpo.jarvinen@linux.intel.com Signed-off-by: Ilpo Järvinen Signed-off-by: Bjorn Helgaas --- arch/x86/kernel/aperture_64.c | 3 +-- arch/x86/kernel/early-quirks.c | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c index 4feaa670d578..89c0c8a3fc7e 100644 --- a/arch/x86/kernel/aperture_64.c +++ b/arch/x86/kernel/aperture_64.c @@ -259,10 +259,9 @@ static u32 __init search_agp_bridge(u32 *order, int *valid_agp) order); } - /* No multi-function device? */ type = read_pci_config_byte(bus, slot, func, PCI_HEADER_TYPE); - if (!(type & 0x80)) + if (!(type & PCI_HEADER_TYPE_MFD)) break; } } diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index a6c1867fc7aa..59f4aefc6bc1 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -779,13 +779,13 @@ static int __init check_dev_quirk(int num, int slot, int func) type = read_pci_config_byte(num, slot, func, PCI_HEADER_TYPE); - if ((type & 0x7f) == PCI_HEADER_TYPE_BRIDGE) { + if ((type & PCI_HEADER_TYPE_MASK) == PCI_HEADER_TYPE_BRIDGE) { sec = read_pci_config_byte(num, slot, func, PCI_SECONDARY_BUS); if (sec > num) early_pci_scan_bus(sec); } - if (!(type & 0x80)) + if (!(type & PCI_HEADER_TYPE_MFD)) return -1; return 0; -- cgit v1.2.3 From 0d481ff35c9a85d775e5544bb2e331e7d5eb6c3c Mon Sep 17 00:00:00 2001 From: Ilpo Järvinen Date: Fri, 24 Nov 2023 10:59:24 +0200 Subject: x86/pci: Clean up open-coded PCIBIOS return code mangling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per PCI Firmware spec r3.3, sec 2.5.2, 2.6.2, and 2.7, the return code for these PCI BIOS interfaces is in 8 bits of the EAX register. Previously it was extracted by open-coded masks and shifting. Name the return code bits with a #define and add pcibios_get_return_code() to extract the return code to improve code readability. In addition, replace zero test with PCIBIOS_SUCCESSFUL. No functional changes intended. Link: https://lore.kernel.org/r/20231124085924.13830-1-ilpo.jarvinen@linux.intel.com Signed-off-by: Ilpo Järvinen Signed-off-by: Bjorn Helgaas --- arch/x86/pci/pcbios.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/pci/pcbios.c b/arch/x86/pci/pcbios.c index 4f15280732ed..244c643bb0b5 100644 --- a/arch/x86/pci/pcbios.c +++ b/arch/x86/pci/pcbios.c @@ -3,6 +3,8 @@ * BIOS32 and PCI BIOS handling. */ +#include +#include #include #include #include @@ -29,8 +31,19 @@ #define PCIBIOS_HW_TYPE1_SPEC 0x10 #define PCIBIOS_HW_TYPE2_SPEC 0x20 +/* + * Returned in EAX: + * - AH: return code + */ +#define PCIBIOS_RETURN_CODE GENMASK(15, 8) + int pcibios_enabled; +static u8 pcibios_get_return_code(u32 eax) +{ + return FIELD_GET(PCIBIOS_RETURN_CODE, eax); +} + /* According to the BIOS specification at: * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could * restrict the x zone to some pages and make it ro. But this may be @@ -154,7 +167,7 @@ static int __init check_pcibios(void) : "memory"); local_irq_restore(flags); - status = (eax >> 8) & 0xff; + status = pcibios_get_return_code(eax); hw_mech = eax & 0xff; major_ver = (ebx >> 8) & 0xff; minor_ver = ebx & 0xff; @@ -227,7 +240,7 @@ static int pci_bios_read(unsigned int seg, unsigned int bus, raw_spin_unlock_irqrestore(&pci_config_lock, flags); - return (int)((result & 0xff00) >> 8); + return pcibios_get_return_code(result); } static int pci_bios_write(unsigned int seg, unsigned int bus, @@ -269,7 +282,7 @@ static int pci_bios_write(unsigned int seg, unsigned int bus, raw_spin_unlock_irqrestore(&pci_config_lock, flags); - return (int)((result & 0xff00) >> 8); + return pcibios_get_return_code(result); } @@ -385,9 +398,10 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void) "m" (opt) : "memory"); DBG("OK ret=%d, size=%d, map=%x\n", ret, opt.size, map); - if (ret & 0xff00) - printk(KERN_ERR "PCI: Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff); - else if (opt.size) { + ret = pcibios_get_return_code(ret); + if (ret) { + printk(KERN_ERR "PCI: Error %02x when fetching IRQ routing table.\n", ret); + } else if (opt.size) { rt = kmalloc(sizeof(struct irq_routing_table) + opt.size, GFP_KERNEL); if (rt) { memset(rt, 0, sizeof(struct irq_routing_table)); @@ -415,7 +429,7 @@ int pcibios_set_irq_routing(struct pci_dev *dev, int pin, int irq) "b" ((dev->bus->number << 8) | dev->devfn), "c" ((irq << 8) | (pin + 10)), "S" (&pci_indirect)); - return !(ret & 0xff00); + return pcibios_get_return_code(ret) == PCIBIOS_SUCCESSFUL; } EXPORT_SYMBOL(pcibios_set_irq_routing); -- cgit v1.2.3 From 070909e56a7d65fd0b4aad6e808966b7c634befe Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 21 Nov 2023 12:36:35 -0600 Subject: x86/pci: Reserve ECAM if BIOS didn't include it in PNP0C02 _CRS Tomasz, Sebastian, and some Proxmox users reported problems initializing ixgbe NICs. I think the problem is that ECAM space described in the ACPI MCFG table is not reserved via a PNP0C02 _CRS method as required by the PCI Firmware spec (r3.3, sec 4.1.2), but it *is* included in the PNP0A03 host bridge _CRS as part of the MMIO aperture. If we allocate space for a PCI BAR, we're likely to allocate it from that ECAM space, which obviously cannot work. This could happen for any device, but in the ixgbe case it happens because it's an SR-IOV device and the BIOS didn't allocate space for the VF BARs, so Linux reallocated the bridge window leading to ixgbe and put it on top of the ECAM space. From Tomasz' system: PCI: MMCONFIG for domain 0000 [bus 00-ff] at [mem 0x80000000-0x8fffffff] (base 0x80000000) PCI: MMCONFIG at [mem 0x80000000-0x8fffffff] not reserved in ACPI motherboard resources pci_bus 0000:00: root bus resource [mem 0x80000000-0xfbffffff window] pci 0000:00:01.1: PCI bridge to [bus 02-03] pci 0000:00:01.1: bridge window [mem 0xfb900000-0xfbbfffff] pci 0000:02:00.0: [8086:10fb] type 00 class 0x020000 # ixgbe pci 0000:02:00.0: reg 0x10: [mem 0xfba80000-0xfbafffff 64bit] pci 0000:02:00.0: VF(n) BAR0 space: [mem 0x00000000-0x000fffff 64bit] (contains BAR0 for 64 VFs) pci 0000:02:00.0: BAR 7: no space for [mem size 0x00100000 64bit] # VF BAR 0 pci_bus 0000:00: No. 2 try to assign unassigned res pci 0000:00:01.1: resource 14 [mem 0xfb900000-0xfbbfffff] released pci 0000:00:01.1: BAR 14: assigned [mem 0x80000000-0x806fffff] pci 0000:02:00.0: BAR 0: assigned [mem 0x80000000-0x8007ffff 64bit] pci 0000:02:00.0: BAR 7: assigned [mem 0x80204000-0x80303fff 64bit] # VF BAR 0 Fixes: 07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map") Fixes: fd3a8cff4d4a ("x86/pci: Treat EfiMemoryMappedIO as reservation of ECAM space") Reported-by: Tomasz Pala Link: https://bugzilla.kernel.org/show_bug.cgi?id=218050 Reported-by: Sebastian Manciulea Link: https://bugzilla.kernel.org/show_bug.cgi?id=218107 Link: https://forum.proxmox.com/threads/proxmox-8-kernel-6-2-16-4-pve-ixgbe-driver-fails-to-load-due-to-pci-device-probing-failure.131203/ Link: https://lore.kernel.org/r/20231121183643.249006-2-helgaas@kernel.org Tested-by: Tomasz Pala Signed-off-by: Bjorn Helgaas Cc: stable@vger.kernel.org # v6.2+ --- arch/x86/pci/mmconfig-shared.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'arch/x86') diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index 4b3efaa82ab7..e9497ee0f854 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -525,6 +525,8 @@ static bool __ref is_mmconf_reserved(check_reserved_t is_reserved, static bool __ref pci_mmcfg_check_reserved(struct device *dev, struct pci_mmcfg_region *cfg, int early) { + struct resource *conflict; + if (!early && !acpi_disabled) { if (is_mmconf_reserved(is_acpi_reserved, cfg, dev, "ACPI motherboard resource")) @@ -542,8 +544,17 @@ pci_mmcfg_check_reserved(struct device *dev, struct pci_mmcfg_region *cfg, int e &cfg->res); if (is_mmconf_reserved(is_efi_mmio, cfg, dev, - "EfiMemoryMappedIO")) + "EfiMemoryMappedIO")) { + conflict = insert_resource_conflict(&iomem_resource, + &cfg->res); + if (conflict) + pr_warn("MMCONFIG %pR conflicts with %s %pR\n", + &cfg->res, conflict->name, conflict); + else + pr_info("MMCONFIG %pR reserved to work around lack of ACPI motherboard _CRS\n", + &cfg->res); return true; + } } /* -- cgit v1.2.3 From e1fad9dd25ea36ae3655c5f4d505d57ab6d4bcc3 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 21 Nov 2023 12:36:36 -0600 Subject: x86/pci: Reword ECAM EfiMemoryMappedIO logging to avoid 'reserved' fd3a8cff4d4a ("x86/pci: Treat EfiMemoryMappedIO as reservation of ECAM space") added the concept of using the EFI memory map to help decide whether ECAM space mentioned in the MCFG table is valid. Unfortunately it described that EfiMemoryMappedIO space as "reserved", but it is actually not *reserved* by the EFI memory map. EfiMemoryMappedIO only means the firmware requested that the OS map this space for use by firmware runtime services. Change the dmesg logging to describe it as simply "EfiMemoryMappedIO", not as "reserved as EfiMemoryMappedIO". A previous commit actually *does* reserve the space if ACPI PNP0C01/02 devices haven't done so: - PCI: ECAM at [mem 0xe0000000-0xefffffff] reserved as EfiMemoryMappedIO + PCI: ECAM at [mem 0xe0000000-0xefffffff] is EfiMemoryMappedIO; assuming valid PCI: ECAM [mem 0xe0000000-0xefffffff] reserved to work around lack of ACPI motherboard _CRS Link: https://lore.kernel.org/r/20231121183643.249006-3-helgaas@kernel.org Tested-by: Tomasz Pala Signed-off-by: Bjorn Helgaas --- arch/x86/pci/mmconfig-shared.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index e9497ee0f854..64c39a23d37a 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -443,9 +443,11 @@ static bool is_acpi_reserved(u64 start, u64 end, enum e820_type not_used) return mcfg_res.flags; } -static bool is_efi_mmio(u64 start, u64 end, enum e820_type not_used) +static bool is_efi_mmio(struct resource *res) { #ifdef CONFIG_EFI + u64 start = res->start; + u64 end = res->start + resource_size(res); efi_memory_desc_t *md; u64 size, mmio_start, mmio_end; @@ -455,11 +457,6 @@ static bool is_efi_mmio(u64 start, u64 end, enum e820_type not_used) mmio_start = md->phys_addr; mmio_end = mmio_start + size; - /* - * N.B. Caller supplies (start, start + size), - * so to match, mmio_end is the first address - * *past* the EFI_MEMORY_MAPPED_IO area. - */ if (mmio_start <= start && end <= mmio_end) return true; } @@ -543,8 +540,9 @@ pci_mmcfg_check_reserved(struct device *dev, struct pci_mmcfg_region *cfg, int e "ACPI motherboard resources\n", &cfg->res); - if (is_mmconf_reserved(is_efi_mmio, cfg, dev, - "EfiMemoryMappedIO")) { + if (is_efi_mmio(&cfg->res)) { + pr_info("ECAM %pR is EfiMemoryMappedIO; assuming valid\n", + &cfg->res); conflict = insert_resource_conflict(&iomem_resource, &cfg->res); if (conflict) -- cgit v1.2.3 From 286ae88c9e40b261d7860b367c36346434ffeaa3 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 21 Nov 2023 12:36:37 -0600 Subject: x86/pci: Add MCFG debug logging MCFG handling is a frequent source of problems. Add more logging to aid in debugging. Enable the logging with CONFIG_DYNAMIC_DEBUG=y and the kernel boot parameter 'dyndbg="file arch/x86/pci +p"'. Link: https://lore.kernel.org/r/20231121183643.249006-4-helgaas@kernel.org Tested-by: Tomasz Pala Signed-off-by: Bjorn Helgaas --- arch/x86/pci/acpi.c | 3 +++ arch/x86/pci/mmconfig-shared.c | 23 ++++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c index ea2eb2ec90e2..55c4b07ec1f6 100644 --- a/arch/x86/pci/acpi.c +++ b/arch/x86/pci/acpi.c @@ -283,6 +283,9 @@ static int setup_mcfg_map(struct acpi_pci_root_info *ci) info->mcfg_added = false; seg = info->sd.domain; + dev_dbg(dev, "%s(%04x %pR ECAM %pa)\n", __func__, seg, + &root->secondary, &root->mcfg_addr); + /* return success if MMCFG is not in use */ if (raw_pci_ext_ops && raw_pci_ext_ops != &pci_mmcfg) return 0; diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index 64c39a23d37a..bc1312d920da 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -579,7 +579,8 @@ static void __init pci_mmcfg_reject_broken(int early) list_for_each_entry(cfg, &pci_mmcfg_list, list) { if (pci_mmcfg_check_reserved(NULL, cfg, early) == 0) { - pr_info(PREFIX "not using MMCONFIG\n"); + pr_info(PREFIX "not using MMCONFIG (%pR not reserved)\n", + &cfg->res); free_all_mmcfg(); return; } @@ -676,6 +677,8 @@ static int pci_mmcfg_for_each_region(int (*func)(__u64 start, __u64 size, static void __init __pci_mmcfg_init(int early) { + pr_debug(PREFIX "%s(%s)\n", __func__, early ? "early" : "late"); + pci_mmcfg_reject_broken(early); if (list_empty(&pci_mmcfg_list)) return; @@ -702,6 +705,8 @@ static int __initdata known_bridge; void __init pci_mmcfg_early_init(void) { + pr_debug(PREFIX "%s() pci_probe %#x\n", __func__, pci_probe); + if (pci_probe & PCI_PROBE_MMCONF) { if (pci_mmcfg_check_hostbridge()) known_bridge = 1; @@ -715,6 +720,8 @@ void __init pci_mmcfg_early_init(void) void __init pci_mmcfg_late_init(void) { + pr_debug(PREFIX "%s() pci_probe %#x\n", __func__, pci_probe); + /* MMCONFIG disabled */ if ((pci_probe & PCI_PROBE_MMCONF) == 0) return; @@ -735,6 +742,8 @@ static int __init pci_mmcfg_late_insert_resources(void) pci_mmcfg_running_state = true; + pr_debug(PREFIX "%s() pci_probe %#x\n", __func__, pci_probe); + /* If we are not using MMCONFIG, don't insert the resources. */ if ((pci_probe & PCI_PROBE_MMCONF) == 0) return 1; @@ -744,9 +753,12 @@ static int __init pci_mmcfg_late_insert_resources(void) * marked so it won't cause request errors when __request_region is * called. */ - list_for_each_entry(cfg, &pci_mmcfg_list, list) - if (!cfg->res.parent) + list_for_each_entry(cfg, &pci_mmcfg_list, list) { + if (!cfg->res.parent) { + pr_debug(PREFIX "%s() insert %pR\n", __func__, &cfg->res); insert_resource(&iomem_resource, &cfg->res); + } + } return 0; } @@ -766,6 +778,8 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, struct resource *tmp = NULL; struct pci_mmcfg_region *cfg; + dev_dbg(dev, "%s(%04x [bus %02x-%02x])\n", __func__, seg, start, end); + if (!(pci_probe & PCI_PROBE_MMCONF) || pci_mmcfg_arch_init_failed) return -ENODEV; @@ -810,8 +824,7 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, "%s %pR\n", &cfg->res, tmp->name, tmp); } else if (pci_mmcfg_arch_map(cfg)) { - dev_warn(dev, "fail to map MMCONFIG %pR.\n", - &cfg->res); + dev_warn(dev, "fail to map MMCONFIG %pR\n", &cfg->res); } else { list_add_sorted(cfg); dev_info(dev, "MMCONFIG at %pR (base %#lx)\n", -- cgit v1.2.3 From 704891033b9714f4c9813bf9ffd888fc69ae3948 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 21 Nov 2023 12:36:38 -0600 Subject: x86/pci: Rename 'MMCONFIG' to 'ECAM', use pr_fmt The "MMCONFIG" term is not used in PCI/PCIe specs. Replace it with "ECAM", the term used in PCIe r6.0, sec 7.2.2. Define pr_fmt() instead of repeating PREFIX in every log message. Link: https://lore.kernel.org/r/20231121183643.249006-5-helgaas@kernel.org Tested-by: Tomasz Pala Signed-off-by: Bjorn Helgaas --- arch/x86/pci/mmconfig-shared.c | 111 ++++++++++++++++++----------------------- arch/x86/pci/mmconfig_32.c | 2 +- arch/x86/pci/mmconfig_64.c | 6 +-- 3 files changed, 52 insertions(+), 67 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index bc1312d920da..896cc11013bd 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* - * mmconfig-shared.c - Low-level direct PCI config space access via - * MMCONFIG - common code between i386 and x86-64. + * Low-level direct PCI config space access via ECAM - common code between + * i386 and x86-64. * * This code does: * - known chipset handling @@ -11,6 +11,8 @@ * themselves. */ +#define pr_fmt(fmt) "PCI: " fmt + #include #include #include @@ -24,9 +26,7 @@ #include #include -#define PREFIX "PCI: " - -/* Indicate if the mmcfg resources have been placed into the resource table. */ +/* Indicate if the ECAM resources have been placed into the resource table */ static bool pci_mmcfg_running_state; static bool pci_mmcfg_arch_init_failed; static DEFINE_MUTEX(pci_mmcfg_lock); @@ -90,7 +90,7 @@ static struct pci_mmcfg_region *pci_mmconfig_alloc(int segment, int start, res->end = addr + PCI_MMCFG_BUS_OFFSET(end + 1) - 1; res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; snprintf(new->name, PCI_MMCFG_RESOURCE_NAME_LEN, - "PCI MMCONFIG %04x [bus %02x-%02x]", segment, start, end); + "PCI ECAM %04x [bus %02x-%02x]", segment, start, end); res->name = new->name; return new; @@ -107,10 +107,8 @@ struct pci_mmcfg_region *__init pci_mmconfig_add(int segment, int start, list_add_sorted(new); mutex_unlock(&pci_mmcfg_lock); - pr_info(PREFIX - "MMCONFIG for domain %04x [bus %02x-%02x] at %pR " - "(base %#lx)\n", - segment, start, end, &new->res, (unsigned long)addr); + pr_info("ECAM %pR (base %#lx) for domain %04x [bus %02x-%02x]\n", + &new->res, (unsigned long)addr, segment, start, end); } return new; @@ -205,7 +203,7 @@ static const char *__init pci_mmcfg_amd_fam10h(void) msr <<= 32; msr |= low; - /* mmconfig is not enable */ + /* ECAM is not enabled */ if (!(msr & FAM10H_MMIO_CONF_ENABLE)) return NULL; @@ -367,7 +365,7 @@ static int __init pci_mmcfg_check_hostbridge(void) name = pci_mmcfg_probes[i].probe(); if (name) - pr_info(PREFIX "%s with MMCONFIG support\n", name); + pr_info("%s with ECAM support\n", name); } /* some end_bus_number is crazy, fix it */ @@ -487,11 +485,10 @@ static bool __ref is_mmconf_reserved(check_reserved_t is_reserved, return false; if (dev) - dev_info(dev, "MMCONFIG at %pR reserved as %s\n", + dev_info(dev, "ECAM %pR reserved as %s\n", &cfg->res, method); else - pr_info(PREFIX "MMCONFIG at %pR reserved as %s\n", - &cfg->res, method); + pr_info("ECAM %pR reserved as %s\n", &cfg->res, method); if (old_size != size) { /* update end_bus */ @@ -500,20 +497,16 @@ static bool __ref is_mmconf_reserved(check_reserved_t is_reserved, cfg->res.end = cfg->res.start + PCI_MMCFG_BUS_OFFSET(num_buses) - 1; snprintf(cfg->name, PCI_MMCFG_RESOURCE_NAME_LEN, - "PCI MMCONFIG %04x [bus %02x-%02x]", + "PCI ECAM %04x [bus %02x-%02x]", cfg->segment, cfg->start_bus, cfg->end_bus); if (dev) - dev_info(dev, - "MMCONFIG " - "at %pR (base %#lx) (size reduced!)\n", - &cfg->res, (unsigned long) cfg->address); + dev_info(dev, "ECAM %pR (base %#lx) (size reduced!)\n", + &cfg->res, (unsigned long) cfg->address); else - pr_info(PREFIX - "MMCONFIG for %04x [bus%02x-%02x] " - "at %pR (base %#lx) (size reduced!)\n", - cfg->segment, cfg->start_bus, cfg->end_bus, - &cfg->res, (unsigned long) cfg->address); + pr_info("ECAM %pR (base %#lx) for %04x [bus%02x-%02x] (size reduced!)\n", + &cfg->res, (unsigned long) cfg->address, + cfg->segment, cfg->start_bus, cfg->end_bus); } return true; @@ -530,15 +523,11 @@ pci_mmcfg_check_reserved(struct device *dev, struct pci_mmcfg_region *cfg, int e return true; if (dev) - dev_info(dev, FW_INFO - "MMCONFIG at %pR not reserved in " - "ACPI motherboard resources\n", + dev_info(dev, FW_INFO "ECAM %pR not reserved in ACPI motherboard resources\n", &cfg->res); else - pr_info(FW_INFO PREFIX - "MMCONFIG at %pR not reserved in " - "ACPI motherboard resources\n", - &cfg->res); + pr_info(FW_INFO "ECAM %pR not reserved in ACPI motherboard resources\n", + &cfg->res); if (is_efi_mmio(&cfg->res)) { pr_info("ECAM %pR is EfiMemoryMappedIO; assuming valid\n", @@ -546,10 +535,10 @@ pci_mmcfg_check_reserved(struct device *dev, struct pci_mmcfg_region *cfg, int e conflict = insert_resource_conflict(&iomem_resource, &cfg->res); if (conflict) - pr_warn("MMCONFIG %pR conflicts with %s %pR\n", + pr_warn("ECAM %pR conflicts with %s %pR\n", &cfg->res, conflict->name, conflict); else - pr_info("MMCONFIG %pR reserved to work around lack of ACPI motherboard _CRS\n", + pr_info("ECAM %pR reserved to work around lack of ACPI motherboard _CRS\n", &cfg->res); return true; } @@ -579,7 +568,7 @@ static void __init pci_mmcfg_reject_broken(int early) list_for_each_entry(cfg, &pci_mmcfg_list, list) { if (pci_mmcfg_check_reserved(NULL, cfg, early) == 0) { - pr_info(PREFIX "not using MMCONFIG (%pR not reserved)\n", + pr_info("not using ECAM (%pR not reserved)\n", &cfg->res); free_all_mmcfg(); return; @@ -599,9 +588,9 @@ static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg, if ((mcfg->header.revision >= 1) && (dmi_get_bios_year() >= 2010)) return 0; - pr_err(PREFIX "MCFG region for %04x [bus %02x-%02x] at %#llx " - "is above 4GB, ignored\n", cfg->pci_segment, - cfg->start_bus_number, cfg->end_bus_number, cfg->address); + pr_err("ECAM at %#llx for %04x [bus %02x-%02x] is above 4GB, ignored\n", + cfg->address, cfg->pci_segment, cfg->start_bus_number, + cfg->end_bus_number); return -EINVAL; } @@ -626,7 +615,7 @@ static int __init pci_parse_mcfg(struct acpi_table_header *header) i -= sizeof(struct acpi_mcfg_allocation); } if (entries == 0) { - pr_err(PREFIX "MMCONFIG has no entries\n"); + pr_err("MCFG has no entries\n"); return -ENODEV; } @@ -640,7 +629,7 @@ static int __init pci_parse_mcfg(struct acpi_table_header *header) if (pci_mmconfig_add(cfg->pci_segment, cfg->start_bus_number, cfg->end_bus_number, cfg->address) == NULL) { - pr_warn(PREFIX "no memory for MCFG entries\n"); + pr_warn("no memory for MCFG entries\n"); free_all_mmcfg(); return -ENOMEM; } @@ -677,7 +666,7 @@ static int pci_mmcfg_for_each_region(int (*func)(__u64 start, __u64 size, static void __init __pci_mmcfg_init(int early) { - pr_debug(PREFIX "%s(%s)\n", __func__, early ? "early" : "late"); + pr_debug("%s(%s)\n", __func__, early ? "early" : "late"); pci_mmcfg_reject_broken(early); if (list_empty(&pci_mmcfg_list)) @@ -705,7 +694,7 @@ static int __initdata known_bridge; void __init pci_mmcfg_early_init(void) { - pr_debug(PREFIX "%s() pci_probe %#x\n", __func__, pci_probe); + pr_debug("%s() pci_probe %#x\n", __func__, pci_probe); if (pci_probe & PCI_PROBE_MMCONF) { if (pci_mmcfg_check_hostbridge()) @@ -720,16 +709,16 @@ void __init pci_mmcfg_early_init(void) void __init pci_mmcfg_late_init(void) { - pr_debug(PREFIX "%s() pci_probe %#x\n", __func__, pci_probe); + pr_debug("%s() pci_probe %#x\n", __func__, pci_probe); - /* MMCONFIG disabled */ + /* ECAM disabled */ if ((pci_probe & PCI_PROBE_MMCONF) == 0) return; if (known_bridge) return; - /* MMCONFIG hasn't been enabled yet, try again */ + /* ECAM hasn't been enabled yet, try again */ if (pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF) { acpi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg); __pci_mmcfg_init(0); @@ -742,9 +731,9 @@ static int __init pci_mmcfg_late_insert_resources(void) pci_mmcfg_running_state = true; - pr_debug(PREFIX "%s() pci_probe %#x\n", __func__, pci_probe); + pr_debug("%s() pci_probe %#x\n", __func__, pci_probe); - /* If we are not using MMCONFIG, don't insert the resources. */ + /* If we are not using ECAM, don't insert the resources. */ if ((pci_probe & PCI_PROBE_MMCONF) == 0) return 1; @@ -755,7 +744,7 @@ static int __init pci_mmcfg_late_insert_resources(void) */ list_for_each_entry(cfg, &pci_mmcfg_list, list) { if (!cfg->res.parent) { - pr_debug(PREFIX "%s() insert %pR\n", __func__, &cfg->res); + pr_debug("%s() insert %pR\n", __func__, &cfg->res); insert_resource(&iomem_resource, &cfg->res); } } @@ -764,13 +753,13 @@ static int __init pci_mmcfg_late_insert_resources(void) } /* - * Perform MMCONFIG resource insertion after PCI initialization to allow for + * Perform ECAM resource insertion after PCI initialization to allow for * misprogrammed MCFG tables that state larger sizes but actually conflict * with other system resources. */ late_initcall(pci_mmcfg_late_insert_resources); -/* Add MMCFG information for host bridges */ +/* Add ECAM information for host bridges */ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, phys_addr_t addr) { @@ -790,11 +779,9 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, cfg = pci_mmconfig_lookup(seg, start); if (cfg) { if (cfg->end_bus < end) - dev_info(dev, FW_INFO - "MMCONFIG for " - "domain %04x [bus %02x-%02x] " - "only partially covers this bridge\n", - cfg->segment, cfg->start_bus, cfg->end_bus); + dev_info(dev, FW_INFO "ECAM %pR for domain %04x [bus %02x-%02x] only partially covers this bridge\n", + &cfg->res, cfg->segment, cfg->start_bus, + cfg->end_bus); mutex_unlock(&pci_mmcfg_lock); return -EEXIST; } @@ -807,10 +794,10 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, rc = -EBUSY; cfg = pci_mmconfig_alloc(seg, start, end, addr); if (cfg == NULL) { - dev_warn(dev, "fail to add MMCONFIG (out of memory)\n"); + dev_warn(dev, "fail to add ECAM (out of memory)\n"); rc = -ENOMEM; } else if (!pci_mmcfg_check_reserved(dev, cfg, 0)) { - dev_warn(dev, FW_BUG "MMCONFIG %pR isn't reserved\n", + dev_warn(dev, FW_BUG "ECAM %pR isn't reserved\n", &cfg->res); } else { /* Insert resource if it's not in boot stage */ @@ -819,15 +806,13 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, &cfg->res); if (tmp) { - dev_warn(dev, - "MMCONFIG %pR conflicts with " - "%s %pR\n", + dev_warn(dev, "ECAM %pR conflicts with %s %pR\n", &cfg->res, tmp->name, tmp); } else if (pci_mmcfg_arch_map(cfg)) { - dev_warn(dev, "fail to map MMCONFIG %pR\n", &cfg->res); + dev_warn(dev, "fail to map ECAM %pR\n", &cfg->res); } else { list_add_sorted(cfg); - dev_info(dev, "MMCONFIG at %pR (base %#lx)\n", + dev_info(dev, "ECAM %pR (base %#lx)\n", &cfg->res, (unsigned long)addr); cfg = NULL; rc = 0; @@ -845,7 +830,7 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, return rc; } -/* Delete MMCFG information for host bridges */ +/* Delete ECAM information for host bridges */ int pci_mmconfig_delete(u16 seg, u8 start, u8 end) { struct pci_mmcfg_region *cfg; diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c index bfa789875322..f9ef97c593cf 100644 --- a/arch/x86/pci/mmconfig_32.c +++ b/arch/x86/pci/mmconfig_32.c @@ -131,7 +131,7 @@ const struct pci_raw_ops pci_mmcfg = { int __init pci_mmcfg_arch_init(void) { - printk(KERN_INFO "PCI: Using MMCONFIG for extended config space\n"); + printk(KERN_INFO "PCI: Using ECAM for extended config space\n"); raw_pci_ext_ops = &pci_mmcfg; return 1; } diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c index 0c7b6e66c644..dfdeac0a7571 100644 --- a/arch/x86/pci/mmconfig_64.c +++ b/arch/x86/pci/mmconfig_64.c @@ -6,6 +6,8 @@ * space mapped. This allows lockless config space operation. */ +#define pr_fmt(fmt) "PCI: " fmt + #include #include #include @@ -14,8 +16,6 @@ #include #include -#define PREFIX "PCI: " - static char __iomem *pci_dev_base(unsigned int seg, unsigned int bus, unsigned int devfn) { struct pci_mmcfg_region *cfg = pci_mmconfig_lookup(seg, bus); @@ -138,7 +138,7 @@ int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg) { cfg->virt = mcfg_ioremap(cfg); if (!cfg->virt) { - pr_err(PREFIX "can't map MMCONFIG at %pR\n", &cfg->res); + pr_err("can't map ECAM at %pR\n", &cfg->res); return -ENOMEM; } -- cgit v1.2.3 From 9ad67912d0d019791f0ed6bbf46374fce8a3656e Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 21 Nov 2023 12:36:39 -0600 Subject: x86/pci: Rename acpi_mcfg_check_entry() to acpi_mcfg_valid_entry() "acpi_mcfg_check_entry()" doesn't give a hint about what the return value means. Rename it to "acpi_mcfg_valid_entry()", convert the return value to bool, and update the return values and callers to match so testing "if (acpi_mcfg_valid_entry())" makes sense. Link: https://lore.kernel.org/r/20231121183643.249006-6-helgaas@kernel.org Tested-by: Tomasz Pala Signed-off-by: Bjorn Helgaas --- arch/x86/pci/mmconfig-shared.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index 896cc11013bd..91fd7921d221 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -576,22 +576,22 @@ static void __init pci_mmcfg_reject_broken(int early) } } -static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg, - struct acpi_mcfg_allocation *cfg) +static bool __init acpi_mcfg_valid_entry(struct acpi_table_mcfg *mcfg, + struct acpi_mcfg_allocation *cfg) { if (cfg->address < 0xFFFFFFFF) - return 0; + return true; if (!strncmp(mcfg->header.oem_id, "SGI", 3)) - return 0; + return true; if ((mcfg->header.revision >= 1) && (dmi_get_bios_year() >= 2010)) - return 0; + return true; pr_err("ECAM at %#llx for %04x [bus %02x-%02x] is above 4GB, ignored\n", cfg->address, cfg->pci_segment, cfg->start_bus_number, cfg->end_bus_number); - return -EINVAL; + return false; } static int __init pci_parse_mcfg(struct acpi_table_header *header) @@ -622,7 +622,7 @@ static int __init pci_parse_mcfg(struct acpi_table_header *header) cfg_table = (struct acpi_mcfg_allocation *) &mcfg[1]; for (i = 0; i < entries; i++) { cfg = &cfg_table[i]; - if (acpi_mcfg_check_entry(mcfg, cfg)) { + if (!acpi_mcfg_valid_entry(mcfg, cfg)) { free_all_mmcfg(); return -ENODEV; } -- cgit v1.2.3 From d26e7fc3d907db19f7e25126476cb416f0527592 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 21 Nov 2023 12:36:40 -0600 Subject: x86/pci: Rename pci_mmcfg_check_reserved() to pci_mmcfg_reserved() "pci_mmcfg_check_reserved()" doesn't give a hint about what the boolean return value means. Rename it to pci_mmcfg_reserved() so testing "if (pci_mmcfg_reserved())" makes sense. Update callers to treat the return value as boolean instead of comparing with 0. Link: https://lore.kernel.org/r/20231121183643.249006-7-helgaas@kernel.org Tested-by: Tomasz Pala Signed-off-by: Bjorn Helgaas --- arch/x86/pci/mmconfig-shared.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index 91fd7921d221..b36c10e86505 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -512,8 +512,8 @@ static bool __ref is_mmconf_reserved(check_reserved_t is_reserved, return true; } -static bool __ref -pci_mmcfg_check_reserved(struct device *dev, struct pci_mmcfg_region *cfg, int early) +static bool __ref pci_mmcfg_reserved(struct device *dev, + struct pci_mmcfg_region *cfg, int early) { struct resource *conflict; @@ -567,7 +567,7 @@ static void __init pci_mmcfg_reject_broken(int early) struct pci_mmcfg_region *cfg; list_for_each_entry(cfg, &pci_mmcfg_list, list) { - if (pci_mmcfg_check_reserved(NULL, cfg, early) == 0) { + if (!pci_mmcfg_reserved(NULL, cfg, early)) { pr_info("not using ECAM (%pR not reserved)\n", &cfg->res); free_all_mmcfg(); @@ -796,7 +796,7 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, if (cfg == NULL) { dev_warn(dev, "fail to add ECAM (out of memory)\n"); rc = -ENOMEM; - } else if (!pci_mmcfg_check_reserved(dev, cfg, 0)) { + } else if (!pci_mmcfg_reserved(dev, cfg, 0)) { dev_warn(dev, FW_BUG "ECAM %pR isn't reserved\n", &cfg->res); } else { -- cgit v1.2.3 From f284dff47b6d00efe1f774d25e9d74874e78c600 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 21 Nov 2023 12:36:41 -0600 Subject: x86/pci: Comment pci_mmconfig_insert() obscure MCFG dependency In pci_mmconfig_insert(), there's no reference to "addr" between locking pci_mmcfg_lock and testing "addr", so it *looks* like we should move the test before the lock. But 07f9b61c3915 ("x86/PCI: MMCONFIG: Check earlier for MMCONFIG region at address zero") did that, which broke things by returning -EINVAL when "addr" is zero instead of -EEXIST. So 07f9b61c3915 was reverted by 67d470e0e171 ("Revert "x86/PCI: MMCONFIG: Check earlier for MMCONFIG region at address zero""). Add a comment about this issue to prevent it from happening again. Link: https://lore.kernel.org/r/20231121183643.249006-8-helgaas@kernel.org Tested-by: Tomasz Pala Signed-off-by: Bjorn Helgaas --- arch/x86/pci/mmconfig-shared.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'arch/x86') diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index b36c10e86505..459e95782bb1 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -786,6 +786,10 @@ int pci_mmconfig_insert(struct device *dev, u16 seg, u8 start, u8 end, return -EEXIST; } + /* + * Don't move earlier; we must return -EEXIST, not -EINVAL, if + * pci_mmconfig_lookup() finds something + */ if (!addr) { mutex_unlock(&pci_mmcfg_lock); return -EINVAL; -- cgit v1.2.3 From f12659832612dac746bd57e20956aabb373a00e7 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 21 Nov 2023 12:36:42 -0600 Subject: x86/pci: Return pci_mmconfig_add() failure early If pci_mmconfig_alloc() fails, return the failure early so it's obvious that the failure is the exception, and the success is the normal case. No functional change intended. Link: https://lore.kernel.org/r/20231121183643.249006-9-helgaas@kernel.org Tested-by: Tomasz Pala Signed-off-by: Bjorn Helgaas --- arch/x86/pci/mmconfig-shared.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index 459e95782bb1..0cc9520666ef 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -102,14 +102,15 @@ struct pci_mmcfg_region *__init pci_mmconfig_add(int segment, int start, struct pci_mmcfg_region *new; new = pci_mmconfig_alloc(segment, start, end, addr); - if (new) { - mutex_lock(&pci_mmcfg_lock); - list_add_sorted(new); - mutex_unlock(&pci_mmcfg_lock); + if (!new) + return NULL; - pr_info("ECAM %pR (base %#lx) for domain %04x [bus %02x-%02x]\n", - &new->res, (unsigned long)addr, segment, start, end); - } + mutex_lock(&pci_mmcfg_lock); + list_add_sorted(new); + mutex_unlock(&pci_mmcfg_lock); + + pr_info("ECAM %pR (base %#lx) for domain %04x [bus %02x-%02x]\n", + &new->res, (unsigned long)addr, segment, start, end); return new; } -- cgit v1.2.3 From 1dfc86af06131d65be5b6ffb749c5dbceafb6d00 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Tue, 21 Nov 2023 12:36:43 -0600 Subject: x86/pci: Reorder pci_mmcfg_arch_map() definition before calls The typical style is to define functions before calling them. Move pci_mmcfg_arch_map() and pci_mmcfg_arch_unmap() earlier so they're defined before they're called. No functional change intended. Link: https://lore.kernel.org/r/20231121183643.249006-10-helgaas@kernel.org Tested-by: Tomasz Pala Signed-off-by: Bjorn Helgaas --- arch/x86/pci/mmconfig_64.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c index dfdeac0a7571..cb5aa79a759e 100644 --- a/arch/x86/pci/mmconfig_64.c +++ b/arch/x86/pci/mmconfig_64.c @@ -111,6 +111,25 @@ static void __iomem *mcfg_ioremap(struct pci_mmcfg_region *cfg) return addr; } +int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg) +{ + cfg->virt = mcfg_ioremap(cfg); + if (!cfg->virt) { + pr_err("can't map ECAM at %pR\n", &cfg->res); + return -ENOMEM; + } + + return 0; +} + +void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg) +{ + if (cfg && cfg->virt) { + iounmap(cfg->virt + PCI_MMCFG_BUS_OFFSET(cfg->start_bus)); + cfg->virt = NULL; + } +} + int __init pci_mmcfg_arch_init(void) { struct pci_mmcfg_region *cfg; @@ -133,22 +152,3 @@ void __init pci_mmcfg_arch_free(void) list_for_each_entry(cfg, &pci_mmcfg_list, list) pci_mmcfg_arch_unmap(cfg); } - -int pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg) -{ - cfg->virt = mcfg_ioremap(cfg); - if (!cfg->virt) { - pr_err("can't map ECAM at %pR\n", &cfg->res); - return -ENOMEM; - } - - return 0; -} - -void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg) -{ - if (cfg && cfg->virt) { - iounmap(cfg->virt + PCI_MMCFG_BUS_OFFSET(cfg->start_bus)); - cfg->virt = NULL; - } -} -- cgit v1.2.3