On Sun, 2025-11-23 at 08:42 -0800, Guenter Roeck wrote: > Hi, > > On Thu, Jun 13, 2024 at 01:50:15PM +0200, Philipp Stanner wrote: > > The pcim_iomap_devres.table administrated by pcim_iomap_table() has its > > entries set and unset at several places throughout devres.c using manual > > iterations which are effectively code duplications. > > > > Add pcim_add_mapping_to_legacy_table() and > > pcim_remove_mapping_from_legacy_table() helper functions and use them where > > possible. > > > > Link: https://lore.kernel.org/r/[email protected] > > Signed-off-by: Philipp Stanner <[email protected]> > > [bhelgaas: s/short bar/int bar/ for consistency] > > Signed-off-by: Bjorn Helgaas <[email protected]> > > --- > > drivers/pci/devres.c | 77 +++++++++++++++++++++++++++++++++----------- > > 1 file changed, 58 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > > index f13edd4a3873..845d6fab0ce7 100644 > > --- a/drivers/pci/devres.c > > +++ b/drivers/pci/devres.c > > @@ -297,6 +297,52 @@ void __iomem * const *pcim_iomap_table(struct pci_dev > > *pdev) > > } > > EXPORT_SYMBOL(pcim_iomap_table); > > > > +/* > > + * Fill the legacy mapping-table, so that drivers using the old API can > > + * still get a BAR's mapping address through pcim_iomap_table(). > > + */ > > +static int pcim_add_mapping_to_legacy_table(struct pci_dev *pdev, > > + void __iomem *mapping, int bar) > > +{ > > + void __iomem **legacy_iomap_table; > > + > > + if (bar >= PCI_STD_NUM_BARS) > > + return -EINVAL; > > + > > + legacy_iomap_table = (void __iomem **)pcim_iomap_table(pdev); > > + if (!legacy_iomap_table) > > + return -ENOMEM; > > + > > + /* The legacy mechanism doesn't allow for duplicate mappings. */ > > + WARN_ON(legacy_iomap_table[bar]); > > + > > Ever since this patch has been applied, I see this warning on all hppa > (parisc) systems. > > [ 0.978177] WARNING: CPU: 0 PID: 1 at drivers/pci/devres.c:473 > pcim_add_mapping_to_legacy_table.part.0+0x54/0x80 > [ 0.978850] Modules linked in: > [ 0.979277] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted > 6.18.0-rc6-64bit+ #1 NONE > [ 0.979519] Hardware name: 9000/785/C3700 > [ 0.979715] > [ 0.979768] YZrvWESTHLNXBCVMcbcbcbcbOGFRQPDI > [ 0.979886] PSW: 00001000000001000000000000001111 Not tainted > [ 0.980006] r00-03 000000000804000f 00000000414e10a0 0000000040acb300 > 00000000434b1440 > [ 0.980167] r04-07 00000000414a78a0 0000000000029000 0000000000000000 > 0000000043522000 > [ 0.980314] r08-11 0000000000000000 0000000000000008 0000000000000000 > 00000000434b0de8 > [ 0.980461] r12-15 00000000434b11b0 000000004156a8a0 0000000043c655a0 > 0000000000000000 > [ 0.980608] r16-19 000000004016e080 000000004019e7d8 0000000000000030 > 0000000043549780 > [ 0.981106] r20-23 0000000020000000 0000000000000000 000000000800000e > 0000000000000000 > [ 0.981317] r24-27 0000000000000000 000000000800000f 0000000043522260 > 00000000414a78a0 > [ 0.981480] r28-31 00000000436af480 00000000434b1680 00000000434b14d0 > 0000000000027000 > [ 0.981641] sr00-03 0000000000000000 0000000000000000 0000000000000000 > 0000000000000000 > [ 0.981805] sr04-07 0000000000000000 0000000000000000 0000000000000000 > 0000000000000000 > [ 0.981972] > [ 0.982024] IASQ: 0000000000000000 0000000000000000 IAOQ: 0000000040acb31c > 0000000040acb320 > [ 0.982185] IIR: 03ffe01f ISR: 0000000000000000 IOR: 00000000436af410 > [ 0.982322] CPU: 0 CR30: 0000000043549780 CR31: 0000000000000000 > [ 0.982458] ORIG_R28: 00000000434b16b0 > [ 0.982548] IAOQ[0]: pcim_add_mapping_to_legacy_table.part.0+0x54/0x80 > [ 0.982733] IAOQ[1]: pcim_add_mapping_to_legacy_table.part.0+0x58/0x80 > [ 0.982871] RP(r2): pcim_add_mapping_to_legacy_table.part.0+0x38/0x80 > [ 0.983100] Backtrace: > [ 0.983439] [<0000000040acba1c>] pcim_iomap+0xc4/0x170 > [ 0.983577] [<0000000040ba3e4c>] serial8250_pci_setup_port+0x8c/0x168 > [ 0.983725] [<0000000040ba7588>] setup_port+0x38/0x50 > [ 0.983837] [<0000000040ba7d94>] pci_hp_diva_setup+0x8c/0xd8 > [ 0.983957] [<0000000040baa47c>] pciserial_init_ports+0x2c4/0x358 > [ 0.984088] [<0000000040baa8bc>] pciserial_init_one+0x31c/0x330 > [ 0.984214] [<0000000040abfab4>] pci_device_probe+0x194/0x270 > > Looking into serial8250_pci_setup_port(): > > if (pci_resource_flags(dev, bar) & IORESOURCE_MEM) { > if (!pcim_iomap(dev, bar, 0) && !pcim_iomap_table(dev)) > return -ENOMEM;
Strange. From the code I see here the WARN_ON in pcim_add_mapping_to_legacy_table() should not fire. I suspect that it's actually triggered somewhere else. > > This suggests that the failure is expected. I can see that pcim_iomap_table() > is deprecated, and that one is supposed to use pcim_iomap() instead. However, > pcim_iomap() _is_ alrady used, and I don't see a function which lets the > caller replicate what is done above (attach multiple serial ports to the > same PCI bar). Is serial8250_pci_setup_port() invoked in a loop somewhere? Where does the "attach multiple" happen? > > How would you suggest to fix the problem ? I suggest you try to remove the `&& pcim_iomap_table(dev)` from above to see if that's really the cause. pcim_iomap() already creates the table, and if it succeeds the table has been created with absolute certainty. The entries will also be present. So the table-check is surplus. Report back please if that helps, I'd want to understand what's happening here. Regards P.
