From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E8D9E107BCC6 for ; Fri, 13 Mar 2026 16:20:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=/M+GnmDCtsfBn1UUijMGSd4Iwqm7U1rMqM48OOCC7vA=; b=z6aoReA/V2UxWU SwNFNtUu6iUHamxjpj/rYYDNWgAgFkQCm+JCjNfaIpxxWGJ5hMv0EmmiQA7K4ugVQdN/DKpkAHmf7 fe0fJKkXv1V47XxQv+jPdxhVIEG/qY7F3wn3UGug8ywss2VHqfVlrYZfs5HgSBIdas8ZH29t6m6W2 ncDYu4CPsjRVsA9R9QaabszRzLQtXgSmDPJ7mD+PVHKN/y7t1w5CiC2l/GLhnq3DV5hKomXc+dz/S 0cN2pKqR+QdYEN9WcnGbEbvrbLO+Rd+n0Fpx1rKga8Q/HlqbL3aCxC415YRX3GTgA88jPrETYs00b m6Z4RM/awFtivgcgS8VA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1w15F7-00000000dCS-29YM; Fri, 13 Mar 2026 16:20:05 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1w15F5-00000000dBx-0CnA for linux-mediatek@lists.infradead.org; Fri, 13 Mar 2026 16:20:04 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id DF17E6012B; Fri, 13 Mar 2026 16:20:01 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5A11DC19421; Fri, 13 Mar 2026 16:20:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1773418801; bh=qAx+Oi9Zys6AfZVuERSj3778VmVSGb+4dt2WbzWNnQM=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=aJJdKmObhu0oGQFhPmZibqPGYMFcDKPzl/9mB3J36PzGzxkoMjSRfSXBX5MI0LL8Y pt0ITmWwZ0jIFLyCA7aoy9lBG3RymPxRzsJzBJqj9vWiPGBDsMvxyCk1l4os2eRDQw td/eczCuJoDYZVCoWeUpHqTcASeAbGh03dx5BnecNOqSl5p8et+K9XAW9thTzVqO3b s/I705px3CQoKAPV7BVcBJhNeqLLFdX+8LQVO4gyx7Ajt5S0G3iXsKebM7Hq9TDP32 7yBAcD9E5By8FtKu6VFIJzSwH6k/XctO3Gii1WV2rs0k324wL/AIFYIKMEq1Ns9reR WsWhPuulyJLwQ== Date: Fri, 13 Mar 2026 11:20:00 -0500 From: Bjorn Helgaas To: Caleb James DeLisle Cc: linux-pci@vger.kernel.org, linux-mips@vger.kernel.org, naseefkm@gmail.com, ryder.lee@mediatek.com, bhelgaas@google.com, lpieralisi@kernel.org, kwilczynski@kernel.org, mani@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com, ansuelsmth@gmail.com, linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] PCI: Skip bridge window reads when window is not supported Message-ID: <20260313162000.GA1385506@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260312165332.569772-4-cjd@cjdns.fr> X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Thu, Mar 12, 2026 at 04:53:32PM +0000, Caleb James DeLisle wrote: > pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window > registers unconditionally. If the registers are hardwired to zero > (not implemented), both base and limit will be 0. Since (0 <= 0) is > true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff] > gets created. > > pci_read_bridge_windows() already detects unsupported windows by > testing register writability and sets io_window/pref_window flags > accordingly. Check these flags at the start of pci_read_bridge_io() > and pci_read_bridge_mmio_pref() to skip reading registers when the > window is not supported. > > Suggested-by: Bjorn Helgaas > Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/ > Signed-off-by: Ahmed Naseef > Signed-off-by: Caleb James DeLisle I'd like to apply this, but I don't understand what's going on here. I instrumented a kernel to return zero for all reads of Root Port prefetchable window registers, and it worked correctly: It didn't create a bogus [mem 0x00000000-0x000fffff] window (see below for a possible explanation), didn't claim to have assigned a prefetchable window, and the 64-bit prefetchable BAR of the downstream device was correctly placed in the non-prefetchable window. I'm booting this in qemu, and the current behavior with a functional prefetchable window is this: pci 0000:00:1c.3: PCI bridge to [bus 04] pci 0000:00:1c.3: bridge window [io 0x4000-0x4fff] pci 0000:00:1c.3: bridge window [mem 0xfe200000-0xfe3fffff] pci 0000:00:1c.3: bridge window [mem 0xfd000000-0xfd1fffff 64bit pref] pci 0000:04:00.0: [1033:0194] type 00 class 0x0c0330 PCIe Endpoint pci 0000:04:00.0: BAR 0 [mem 0xfe200000-0xfe203fff 64bit] With the instrumented kernel where the prefetchable window registers are effectively hardwired to zero, I see this: pci 0000:00:1c.3: PCI bridge to [bus 04] pci 0000:00:1c.3: bridge window [io 0x4000-0x4fff] pci 0000:00:1c.3: bridge window [mem 0xfe200000-0xfe3fffff] pci 0000:04:00.0: [1033:0194] type 00 class 0x0c0330 PCIe Endpoint pci 0000:04:00.0: BAR 0 [mem 0xfe200000-0xfe203fff 64bit] So the 04:00.0 BAR 0 was put in the non-prefetchable window as we would expect when there's no prefetchable window. Either there's something else going on with EcoNet EN7528 or I don't understand the explanation. Based on the link above, Root Port 1 doesn't implement a prefetchable window, but Linux thinks it does and puts a mt7615e WiFi device BAR in the non-existent window, so it doesn't work. I assume RP 1 is 0001:00:01.0, and the mt7615e WiFi is 0001:01:00.0, and the mt7615e doesn't work: pci 0001:00:01.0: [14c3:0811] type 01 class 0x060400 PCIe Root Port pci 0001:00:01.0: PCI bridge to [bus 00] pci 0001:00:01.0: bridge window [mem 0x00000000-0x000fffff] pci 0001:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring pci 0001:00:01.0: PCI bridge to [bus 01-ff] pci 0001:00:01.0: bridge window [mem 0x28000000-0x280fffff]: assigned pci 0001:00:01.0: bridge window [mem 0x28100000-0x282fffff pref]: assigned pci 0001:01:00.0: BAR 0 [mem 0x28100000-0x281fffff 64bit pref]: assigned WARNING: CPU: 3 PID: 1777 at target-mipsel_24kc_musl/linux-econet_en7528/mt76-2025.11.06~eb567bc7/mt7615/eeprom.c:31 mt7615_eeprom_init+0x484/0x548 [mt7615_common] mt7615e 0001:01:00.0: Firmware is not ready for download The [mem 0x00000000-0x000fffff] window mentioned above is not a hardware problem. This is the non-prefetchable window, which is required for all bridges, and if it hasn't been initialized and contains the power-up values, this is the window. So all this is telling us is that there was no firmware that programmed the window before Linux. FWIW, I'll attach my instrumentation patches below. Maybe you can similarly instrument your kernel to see the actual config accesses to the RP in question. Also, minor nit: Caleb appears to be the author, but the first sign-off is from Ahmed, so the Author vs Signed-off-by order is wrong; see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.19#n449 Bjorn commit 012e94d89d10 ("PCI: Log config accesses") Author: Bjorn Helgaas Date: Sat Jan 24 12:46:02 2026 -0600 PCI: Log config accesses diff --git a/drivers/pci/access.c b/drivers/pci/access.c index b123da16b63b..8b03af748238 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -32,6 +32,49 @@ DEFINE_RAW_SPINLOCK(pci_lock); # define pci_unlock_config(f) raw_spin_unlock_irqrestore(&pci_lock, f) #endif +/* domain, bus, dev, fn */ +static u64 pci_log_cfg_dev = (0LL << 32) | PCI_DEVID(0, PCI_DEVFN(0x1c, 3)); + +#define PCI_CFG_FMT "pci %04x:%02x:%02x.%d: %s %#06x " +#define PCI_CFG_FMT_1 PCI_CFG_FMT "%#04x\n" +#define PCI_CFG_FMT_2 PCI_CFG_FMT "%#06x\n" +#define PCI_CFG_FMT_4 PCI_CFG_FMT "%#010x\n" + +static void pci_log_cfg(struct pci_bus *bus, unsigned int devfn, char *op, + int pos, int len, u32 val, int res) +{ + u64 dev_match = ((u64) pci_domain_nr(bus) << 32) | devfn; + u32 domain = pci_log_cfg_dev >> 32; + + if (pci_log_cfg_dev != dev_match) + return; + + if (res) { + pr_info(PCI_CFG_FMT "failed (%d)\n", + domain, bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), + op, pos, res); + return; + } + + switch (len) { + case 1: + pr_info(PCI_CFG_FMT_1, + domain, bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), + op, pos, val); + break; + case 2: + pr_info(PCI_CFG_FMT_2, + domain, bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), + op, pos, val); + break; + default: + pr_info(PCI_CFG_FMT_4, + domain, bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), + op, pos, val); + break; + } +} + #define PCI_OP_READ(size, type, len) \ int noinline pci_bus_read_config_##size \ (struct pci_bus *bus, unsigned int devfn, int pos, type *value) \ @@ -50,6 +93,7 @@ int noinline pci_bus_read_config_##size \ else \ *value = (type)data; \ pci_unlock_config(flags); \ + pci_log_cfg(bus, devfn, "rd", pos, len, (u32) *value, res); \ \ return res; \ } @@ -67,6 +111,7 @@ int noinline pci_bus_write_config_##size \ pci_lock_config(flags); \ res = bus->ops->write(bus, devfn, pos, len, value); \ pci_unlock_config(flags); \ + pci_log_cfg(bus, devfn, "wr", pos, len, value, res); \ \ return res; \ } commit 744b78273da8 ("PCI: Override pref window reads to return 0") Author: Bjorn Helgaas Date: Fri Mar 13 10:39:27 2026 -0500 PCI: Override pref window reads to return 0 diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 8b03af748238..675926bddc54 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -40,6 +40,43 @@ static u64 pci_log_cfg_dev = (0LL << 32) | PCI_DEVID(0, PCI_DEVFN(0x1c, 3)); #define PCI_CFG_FMT_2 PCI_CFG_FMT "%#06x\n" #define PCI_CFG_FMT_4 PCI_CFG_FMT "%#010x\n" +static int pci_override_cfg_rd(struct pci_bus *bus, unsigned int devfn, + int pos, int len, u32 *value) +{ + u64 dev_match = ((u64) pci_domain_nr(bus) << 32) | devfn; + + if (pci_log_cfg_dev != dev_match) + return -ENODEV; + + if (!(pos == 0x24 || pos == 0x26 || pos == 0x28 || pos == 0x2c)) + return -ENODEV; + + *value = 0; + +#if 0 + u32 domain = pci_log_cfg_dev >> 32; + switch (len) { + case 1: + pr_info("override " PCI_CFG_FMT_1, + domain, bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), + "rd", pos, *value); + break; + case 2: + pr_info("override " PCI_CFG_FMT_2, + domain, bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), + "rd", pos, *value); + break; + default: + pr_info("override " PCI_CFG_FMT_4, + domain, bus->number, PCI_SLOT(devfn), PCI_FUNC(devfn), + "rd", pos, *value); + break; + } +#endif + + return 0; +} + static void pci_log_cfg(struct pci_bus *bus, unsigned int devfn, char *op, int pos, int len, u32 val, int res) { @@ -87,7 +124,9 @@ int noinline pci_bus_read_config_##size \ return PCIBIOS_BAD_REGISTER_NUMBER; \ \ pci_lock_config(flags); \ - res = bus->ops->read(bus, devfn, pos, len, &data); \ + res = pci_override_cfg_rd(bus, devfn, pos, len, &data); \ + if (res) \ + res = bus->ops->read(bus, devfn, pos, len, &data); \ if (res) \ PCI_SET_ERROR_RESPONSE(value); \ else \