Re: [PATCH v2 3/7] ipack: tpci200: don't keep pci_device_id

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] MMIO registers are accessed before the PCI device is enabled.
--

commit 30f524f5c4f44d8d3fe4dc97ba1c44684c0fe834
Author: Gary Guo <[email protected]>

ipack: tpci200: don't keep pci_device_id

This commit removes the unused pci_device_id table from the tpci200_infos
structure. The stored ID is unnecessary as dynamic IDs may not outlive
the probe function.

> diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
> index 05dcb6675cd66..1cf51f7632932 100644
> --- a/drivers/ipack/carriers/tpci200.c
> +++ b/drivers/ipack/carriers/tpci200.c
> @@ -562,7 +562,6 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
>  
>  	/* Save struct pci_dev pointer */

[Severity: High]
This is a pre-existing issue, but does this probe function access
memory-mapped configuration registers before the PCI device is enabled?

Looking at tpci200_pci_probe() in drivers/ipack/carriers/tpci200.c,
we can see MMIO accesses happening just before this block:

drivers/ipack/carriers/tpci200.c:tpci200_pci_probe() {
	...
	reg32 = ioread32(tpci200->info->cfg_regs + LAS1_DESC);
	reg32 |= 1 << LAS_BIT_BIGENDIAN;
	iowrite32(reg32, tpci200->info->cfg_regs + LAS1_DESC);
	...
}

Since pci_enable_device() is not called until later down the call chain
in tpci200_register() (called via tpci200_install()), PCI_COMMAND_MEMORY
may not be set.

This can result in ignored writes, reading junk data (all 1s), and on
many architectures, it triggers a machine check exception, SError, or
kernel panic due to an Unsupported Request (UR) on the PCIe bus.

>  	tpci200->info->pdev = pdev;
> -	tpci200->info->id_table = (struct pci_device_id *)id;
>  
>  	/* register the device and initialize it */
>  	ret = tpci200_install(tpci200);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3