Re: [PATCH v3 5/5] pci: Suspend iommu function prior to resetting a device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/20/2025 5:59 AM, Nicolin Chen wrote:
On Tue, Aug 19, 2025 at 10:12:41PM +0800, Ethan Zhao wrote:
On 8/12/2025 6:59 AM, Nicolin Chen wrote:
@@ -4529,13 +4530,26 @@ EXPORT_SYMBOL(pci_wait_for_pending_transaction);
    */
   int pcie_flr(struct pci_dev *dev)
   {
+	int ret = 0;
+
   	if (!pci_wait_for_pending_transaction(dev))
   		pci_err(dev, "timed out waiting for pending transaction; performing function level reset anyway\n");
+	/*
+	 * Per PCIe r6.3, sec 10.3.1 IMPLEMENTATION NOTE, software disables ATS
+	 * before initiating a reset. Notify the iommu driver that enabled ATS.
+	 * Have to call it after waiting for pending DMA transaction.
+	 */
+	ret = iommu_dev_reset_prepare(&dev->dev);

If we dont' consider the association between IOMMU and devices in FLR(),
it can be understood that more complex processing logic resides outside
this function. However, if the FLR() function already synchironizes and
handles the association with IOMMU like this (disabling ATS by attaching
device to blocking domain), then how would the following scenarios
behave ?

That's a good point. The iommu-level reset is per struct device.
So, basically it'll match with the FLR per pci_dev. Yet, the RID
isolation between siblings might be a concern:

1. Reset one of PCIe alias devices.

IIRC, an alias device might have:

  a) one pci_dev; multiple RIDs

     In this case, neither FLR nor IOMMU isolates between RIDs.
     So, both FLR and IOMMU blocking will reset all RIDs. There
     should be no issue resulted from the IOMMU blocking.

  b) multiple pci_devs; single RID

     In this case, FLR only resets one device, while the IOMMU-
     level reset will block the entire RID (i.e. all devices),
     since they share the single translation tunnel. This could
     break the siblings, if they aren't also being reset along.
Yup, such alias devices might not have ATS cap. because of they
are PCI devices or they share the RID(BDF), so checking ATS cap
condition might be useful here to skip the prepare()/done() .>
2. Reset PF when its VFs are actvie.

  c) multiple pci_devs with their own RIDs

     In this case, either FLR or IOMMU only resets the PF. That
     being said, VFs might be affected since PF is resetting?
     If there is an issue, I don't see it coming from the IOMMU-
     level reset..
Each of the PF and its VFs has it owns RID(BDF), but the VFs' life
depends on the living of PF, resetting PF, means all its VFs are
lost.

There is no processing logic about PF and its VFs in FLR() yet.
my understanding the upper layer callers should consider the
complexity of such case.

While we introducing the connection of IOMMU & device in FLR(),
seems we brought some of the logic from the outside to the inside
part.

One method might we don't handle PF either by explicit checking its
VF configuration existing to skip prepare()/done() ? till we have
much clearer handling logic about it.

Thanks,
Ethan
  > d
Thus, case b might be breaking. So, perhaps we should add a few
conditions when calling iommu_dev_reset_prepare/done():
  + Make sure that the pci_dev has ATS capability
  + Make sure no sibling pci_dev(s) sharing the same RID
  + Any others?

Thanks
Nicolin





[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux