dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* amd-iommu / agpgart-amd64 problem: Resources present before probing
@ 2025-06-17 19:47 Fedor Pchelkin
  2025-06-18  5:27 ` Lukas Wunner
  0 siblings, 1 reply; 2+ messages in thread
From: Fedor Pchelkin @ 2025-06-17 19:47 UTC (permalink / raw)
  To: Lukas Wunner, Ben Hutchings
  Cc: Bjorn Helgaas, Joerg Roedel, Suravee Suthikulpanit, linux-pci,
	iommu, dri-devel, David Airlie, lvc-project

Hello,

there is a 

  [    0.579232] pci 0000:00:00.2: Resources present before probing

error message observed after

  commit 3be5fa236649da6404f1bca1491bf02d4b0d5cce
  Author: Lukas Wunner <lukas@wunner.de>
  Date:   Fri Apr 25 11:24:21 2025 +0200
  
      Revert "iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices"


After tracking this down I've found that it's agpgart-amd64 driver trying
to bind to the IOMMU device:

  00:00.2 IOMMU: Advanced Micro Devices, Inc. [AMD] Family 17h-19h IOMMU
          Subsystem: Lenovo Device 3803
          Flags: bus master, fast devsel, latency 0, IRQ 25
          Capabilities: <access denied>

IOMMU device itself has no pci_driver attached to it but has a pci_dev,
and its struct device already has some devres associated with it.

agpgart-amd64 driver booting with 'agp_try_unsupported=1' (turns out it's
a default behavior) traverses the devices on the PCI bus and tries to
attach:


static const struct pci_device_id agp_amd64_pci_promisc_table[] = {
	{ PCI_DEVICE_CLASS(0, 0) },
	{ }
};

...

int __init agp_amd64_init(void)
{
...
		/* Look for any AGP bridge */
		agp_amd64_pci_driver.id_table = agp_amd64_pci_promisc_table;
		err = driver_attach(&agp_amd64_pci_driver.driver);
		if (err == 0 && agp_bridges_found == 0) {
			pci_unregister_driver(&agp_amd64_pci_driver);
			err = -ENODEV;
		}


IOMMU device is busy at the moment but, to my mind, lack of pci_driver
associated with it leads driver core trying to bind it, too.

But registering a pci_driver for IOMMU device is no good.

Initial commit cbbc00be2ce3 ("iommu/amd: Prevent binding other PCI drivers
to IOMMU PCI devices") was added in 2015, not sure whether the problem
manifested before that. At least the commit message doesn't state that
it tried to fix such kind of a bug.

The problem on itself is no harm at the end as driver core handles the
error and skips the device. But it still indicates a logical bug.


The partial revert of the revert does work, obviously. Though it badly
contradicts the intention to hide priv_flags manipulation in the PCI core.

So I wonder whether agpgart-amd64 should be somehow fixed instead... to
skip IOMMU device from its wildcard promiscuous PCI ID table? Or drop this
'try_unsupported' feature entirely? 

Would be glad to hear your thoughts on this.

Found by Linux Verification Center (linuxtesting.org).

--
Thanks,
Fedor

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: amd-iommu / agpgart-amd64 problem: Resources present before probing
  2025-06-17 19:47 amd-iommu / agpgart-amd64 problem: Resources present before probing Fedor Pchelkin
@ 2025-06-18  5:27 ` Lukas Wunner
  0 siblings, 0 replies; 2+ messages in thread
From: Lukas Wunner @ 2025-06-18  5:27 UTC (permalink / raw)
  To: linux-pci
  Cc: Ben Hutchings, Bjorn Helgaas, Joerg Roedel, Suravee Suthikulpanit,
	iommu, dri-devel, David Airlie

On Tue, Jun 17, 2025 at 10:47:48PM +0300, Fedor Pchelkin wrote:
> Hello,
> 
> there is a 
> 
>   [    0.579232] pci 0000:00:00.2: Resources present before probing
> 
> error message observed after
> 
>   commit 3be5fa236649da6404f1bca1491bf02d4b0d5cce
>   Author: Lukas Wunner <lukas@wunner.de>
>   Date:   Fri Apr 25 11:24:21 2025 +0200
>   
>       Revert "iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices"

For the record, the reporter of the above-quoted issue appears to be
working for an OFAC sanctioned entity:

https://sanctionssearch.ofac.treas.gov/Details.aspx?id=50890

This prohibits me from two-way engagement with the reporter:

https://www.linuxfoundation.org/blog/navigating-global-regulations-and-open-source-us-ofac-sanctions

   "Reviewing an unsolicited patch from a contributor in a sanctioned
    region should generally be fine, but actively engaging them to
    better understand their issue, diagnose the problem, or help
    improve a patch or modify code would likely cross the line.
    If the contributor is linked to a sanctioned entity or region,
    in general, it is best to keep communications strictly one-way.
    If a patch is received and you improve it and submit it upstream,
    that should be fine, but going back and forth in communications
    with the SDN developer likely would not."

Hence I am removing the reporter and the lvc-project@linuxtesting.org
address (hosted by ispras.ru) from the To: and Cc: headers.

I note that prior to 6fd024893911, the amd64-agp.c driver was only bound
to devices with a PCI_CAP_ID_AGP capability.

agp_amd64_probe() does check for presence of the capability, but that's
too late to avoid the error message emitted by really_probe().

What we could do however is to first check for presence of a device with
PCI_CAP_ID_AGP, and only if one is found would we try to bind to any
device.  That should avoid the message on any halfway modern system.

Thoughts?

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-06-18  5:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-17 19:47 amd-iommu / agpgart-amd64 problem: Resources present before probing Fedor Pchelkin
2025-06-18  5:27 ` Lukas Wunner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).