Re: [PATCH 1/2] Revert "iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices"

[Lukas Wunner <lukas@wunner.de>]

Dear AMD IOMMU maintainers,

Just a gentle reminder:

Please consider ack'ing the patch below, for merging through the pci tree.

The patch removes code which prevents driver binding to the pci_dev
exposed by an AMD IOMMU.  No other IOMMU driver does that or needs that.

The code was added to work around breakage introduced by 991de2e59090:
With that commit, resume from system sleep was broken when a driver was
bound to the AMD IOMMU (e.g. vfio-pci).  The commit has since been
reverted, so there is no apparent reason to continue preventing
driver binding to the AMD IOMMU.

Random drivers are not supposed to fiddle with the match_driver flag
in struct pci_dev and having the AMD IOMMU driver do that is a
maintance burden.

So unless there's a reason to keep this code, please ack the patch below.

Thank you!

Lukas

On Fri, Apr 25, 2025 at 11:24:21AM +0200, Lukas Wunner wrote:
> Commit 991de2e59090 ("PCI, x86: Implement pcibios_alloc_irq() and
> pcibios_free_irq()") changed IRQ handling on PCI driver probing.
> It inadvertently broke resume from system sleep on AMD platforms:
> 
>   https://lore.kernel.org/r/20150926164651.GA3640@pd.tnic/
> 
> This was fixed by two independent commits:
> 
> * 8affb487d4a4 ("x86/PCI: Don't alloc pcibios-irq when MSI is enabled")
> * cbbc00be2ce3 ("iommu/amd: Prevent binding other PCI drivers to IOMMU
>                  PCI devices")
> 
> The breaking change and one of these two fixes were subsequently reverted:
> 
> * fe25d078874f ("Revert "x86/PCI: Don't alloc pcibios-irq when MSI is
>                  enabled"")
> * 6c777e8799a9 ("Revert "PCI, x86: Implement pcibios_alloc_irq() and
>                  pcibios_free_irq()"")
> 
> This rendered the second fix unnecessary, so revert it as well.  It used
> the match_driver flag in struct pci_dev, which is internal to the PCI core
> and not supposed to be touched by arbitrary drivers.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> I would have cc'ed Jiang Liu (author of the commit reverted here)
> but his Intel e-mail address appears to no longer be working.
> Someone with the same name has recently started to contribute
> using an Alibaba e-mail address, but I'm not sure it's the same
> person.
> 
>  drivers/iommu/amd/init.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index dd9e26b..33b6e12 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -2030,9 +2030,6 @@ static int __init iommu_init_pci(struct amd_iommu *iommu)
>  	if (!iommu->dev)
>  		return -ENODEV;
>  
> -	/* Prevent binding other PCI device drivers to IOMMU devices */
> -	iommu->dev->match_driver = false;
> -
>  	/* ACPI _PRT won't have an IRQ for IOMMU */
>  	iommu->dev->irq_managed = 1;
>  
> -- 
> 2.47.2