Re: [PATCH v9 02/13] PCI: Add devres helpers for iomap table [resulting in backtraces on HPPA]

Guenter Roeck
2025-11-23T16:42:43Z

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/20240605081605.18769-4-pstanner@redhat.com
> Signed-off-by: Philipp Stanner <pstanner@redhat.com>
> [bhelgaas: s/short bar/int bar/ for consistency]
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> 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;

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).

How would you suggest to fix the problem ?

Thanks,
Guenter