Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h
Marc Zyngier
maz at kernel.org
Tue Nov 3 06:41:21 EST 2020
On 2020-11-03 10:31, Thomas Gleixner wrote:
> On Tue, Nov 03 2020 at 09:54, Marc Zyngier wrote:
>> On 2020-11-02 22:18, Thomas Gleixner wrote:
>>> So we really need some other solution and removing the warning is not
>>> an option. If MSI is enabled then we want to get a warning when a PCI
>>> device has no MSI domain associated. Explicitly expressing the PCIE
>>> brigde misfeature of not supporting MSI is way better than silently
>>> returning an error code which is swallowed anyway.
>>
>> I don't disagree here, though the PCI_MSI_ARCH_FALLBACKS mechanism
>> makes it more difficult to establish.
>
> Only for the few leftovers which implement msi_controller, i.e.
>
> drivers/pci/controller/pci-hyperv.c
> drivers/pci/controller/pci-tegra.c
> drivers/pci/controller/pcie-rcar-host.c
> drivers/pci/controller/pcie-xilinx.c
>
> The architectures which select PCI_MSI_ARCH_FALLBACKS are:
>
> arch/ia64/Kconfig: select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
> arch/mips/Kconfig: select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
> arch/powerpc/Kconfig: select PCI_MSI_ARCH_FALLBACKS if
> PCI_MSI
> arch/s390/Kconfig: select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
> arch/sparc/Kconfig: select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
>
> implement arch_setup_msi_irq() which makes it magically work :)
>
>>> Whatever the preferred way is via flags at host probe time or
>>> flagging
>>> it post probe I don't care much as long as it is consistent.
>>
>> Host probe time is going to require some changes in the core PCI api,
>> as everything that checks for a MSI domain is based on the pci_bus
>> structure, which is only allocated much later.
>
> Yeah, it's nasty. One possible solution is to add flags or a callback
> to
> pci_ops, but it's not pretty either.
>
> I think we should go with the 'mark it after pci_host_probe()' hack for
> 5.10-rc. The real fix will be larger and go into 5.11.
>
> Thoughts?
We can do that, although I worried that it isn't 100% reliable:
pci_host_probe() ends up calling pci_add_device(), and will start
probing devices if the endpoint drivers have already registered
with the core code, long before the flag gets set.
Here's what I've hacked together for a guest that doesn't have
any MSI capability:
diff --git a/drivers/pci/controller/pci-host-common.c
b/drivers/pci/controller/pci-host-common.c
index 6ce34a1deecb..7dd5145cd38d 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -55,6 +55,7 @@ int pci_host_common_probe(struct platform_device
*pdev)
struct pci_host_bridge *bridge;
struct pci_config_window *cfg;
const struct pci_ecam_ops *ops;
+ int ret;
ops = of_device_get_match_data(&pdev->dev);
if (!ops)
@@ -80,7 +81,10 @@ int pci_host_common_probe(struct platform_device
*pdev)
platform_set_drvdata(pdev, bridge);
- return pci_host_probe(bridge);
+ ret = pci_host_probe(bridge);
+ bridge->bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+
+ return ret;
}
EXPORT_SYMBOL_GPL(pci_host_common_probe);
(plus another hack to get the host controller to initialise a bit
later, though building it as a module will achieve the same thing):
[ 0.369114] 9pnet: Installing 9P2000 support
[ 0.369807] mpls_gso: MPLS GSO support
[ 0.370512] registered taskstats version 1
[ 0.371204] Loading compiled-in X.509 certificates
[ 0.371988] zswap: loaded using pool lzo/zbud
[ 0.373041] pci-host-generic 40000000.pci: host bridge /pci ranges:
[ 0.374045] pci-host-generic 40000000.pci: IO
0x0000000000..0x000000ffff -> 0x0000000000
[ 0.375458] pci-host-generic 40000000.pci: MEM
0x0041000000..0x007fffffff -> 0x0041000000
[ 0.376848] pci-host-generic 40000000.pci: ECAM at [mem
0x40000000-0x40ffffff] for [bus 00]
[ 0.378204] pci-host-generic 40000000.pci: PCI host bridge to bus
0000:00
[ 0.379316] pci_bus 0000:00: root bus resource [bus 00]
[ 0.380146] pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
[ 0.381131] pci_bus 0000:00: root bus resource [mem
0x41000000-0x7fffffff]
[ 0.382267] pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
[ 0.383369] pci 0000:00:00.0: reg 0x10: [io 0x6200-0x62ff]
[ 0.384286] pci 0000:00:00.0: reg 0x14: [mem 0x41000000-0x410000ff]
[ 0.385324] pci 0000:00:00.0: reg 0x18: [mem 0x41000200-0x410003ff]
[ 0.386680] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
[ 0.387778] pci 0000:00:01.0: reg 0x10: [io 0x6300-0x63ff]
[ 0.388696] pci 0000:00:01.0: reg 0x14: [mem 0x41000400-0x410004ff]
[ 0.389730] pci 0000:00:01.0: reg 0x18: [mem 0x41000600-0x410007ff]
[ 0.391070] pci 0000:00:02.0: [1af4:1000] type 00 class 0x020000
[ 0.392212] pci 0000:00:02.0: reg 0x10: [io 0x6400-0x64ff]
[ 0.393137] pci 0000:00:02.0: reg 0x14: [mem 0x41000800-0x410008ff]
[ 0.394163] pci 0000:00:02.0: reg 0x18: [mem 0x41000a00-0x41000bff]
[ 0.395678] pci 0000:00:00.0: BAR 2: assigned [mem
0x41000000-0x410001ff]
[ 0.396762] pci 0000:00:01.0: BAR 2: assigned [mem
0x41000200-0x410003ff]
[ 0.397851] pci 0000:00:02.0: BAR 2: assigned [mem
0x41000400-0x410005ff]
[ 0.398934] pci 0000:00:00.0: BAR 0: assigned [io 0x1000-0x10ff]
[ 0.400014] pci 0000:00:00.0: BAR 1: assigned [mem
0x41000600-0x410006ff]
[ 0.401105] pci 0000:00:01.0: BAR 0: assigned [io 0x1100-0x11ff]
[ 0.402080] pci 0000:00:01.0: BAR 1: assigned [mem
0x41000700-0x410007ff]
[ 0.403185] pci 0000:00:02.0: BAR 0: assigned [io 0x1200-0x12ff]
[ 0.404156] pci 0000:00:02.0: BAR 1: assigned [mem
0x41000800-0x410008ff]
[ 0.405344] virtio-pci 0000:00:00.0: virtio_pci: leaving for legacy
driver
[ 0.406569] ------------[ cut here ]------------
[ 0.407347] WARNING: CPU: 1 PID: 1 at include/linux/msi.h:213
__pci_enable_msix_range+0x680/0x720
[ 0.408884] Modules linked in:
[ 0.409429] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
5.10.0-rc2-dirty #2117
[ 0.410646] Hardware name: linux,dummy-virt (DT)
[ 0.411463] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
[ 0.412505] pc : __pci_enable_msix_range+0x680/0x720
[ 0.413380] lr : __pci_enable_msix_range+0x394/0x720
[ 0.414244] sp : ffffffc0119ab3d0
[ 0.414830] x29: ffffffc0119ab3d0 x28: 0000000000000002
[ 0.415766] x27: ffffff804107b0b8 x26: ffffff804107b000
[ 0.416689] x25: ffffff80410acb00 x24: 0000000000000000
[ 0.417623] x23: ffffff80410ac480 x22: ffffff804107b000
[ 0.418549] x21: ffffff804107b2e8 x20: 0000000000000014
[ 0.419482] x19: 0000000000000002 x18: 00000000fffffffc
[ 0.420408] x17: 000000002ce4d3e3 x16: 00000000a4f09d66
[ 0.421342] x15: 0000000000000020 x14: ffffffffffffffff
[ 0.422268] x13: 0000000041001000 x12: ffffffc0119cf000
[ 0.423201] x11: ffffffc010000000 x10: ffffffc0119cd000
[ 0.424127] x9 : ffffffc010628ad4 x8 : 0000000000000000
[ 0.425063] x7 : 0000000000000000 x6 : 000000000000003f
[ 0.425989] x5 : 0000000000000040 x4 : ffffff804107b2e8
[ 0.426914] x3 : 0000000000000004 x2 : 0000000000000000
[ 0.427855] x1 : 0000000000000000 x0 : 0000000000000000
[ 0.428788] Call trace:
[ 0.429226] __pci_enable_msix_range+0x680/0x720
[ 0.430033] pci_alloc_irq_vectors_affinity+0xcc/0x144
[ 0.430932] vp_find_vqs_msix+0xdc/0x414
[ 0.431631] vp_find_vqs+0x54/0x1c0
[ 0.432245] p9_virtio_probe+0xa8/0x374
This is admittedly a contrived example, but I'm not convinced it is
completely unlikely.
M.
--
Jazz is not dead. It just smells funny...
More information about the Linux-mediatek
mailing list