[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6a48f023-2701-4f2f-8077-14fe348794dd@linux.intel.com>
Date: Wed, 31 Jan 2024 14:21:20 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: Jason Gunthorpe <jgg@...pe.ca>, Ethan Zhao <haifeng.zhao@...ux.intel.com>
Cc: baolu.lu@...ux.intel.com, "Tian, Kevin" <kevin.tian@...el.com>,
"Liu, Yi L" <yi.l.liu@...el.com>, "bhelgaas@...gle.com"
<bhelgaas@...gle.com>, "robin.murphy@....com" <robin.murphy@....com>,
"dwmw2@...radead.org" <dwmw2@...radead.org>,
"will@...nel.org" <will@...nel.org>, "lukas@...ner.de" <lukas@...ner.de>,
"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
Subject: Re: [PATCH v12 5/5] iommu/vt-d: improve ITE fault handling if target
device isn't present
On 2024/1/31 0:29, Jason Gunthorpe wrote:
> On Tue, Jan 30, 2024 at 04:15:33PM +0800, Ethan Zhao wrote:
>> Some tricky situations:
>>
>> 1. The ATS invalidation request is issued from driver driver, while it is
>> in handling, device is removed. this momment, the device instance still
>> exists in the bus list. yes, if searching it by BDF, could get it.
>>
>> 2. The ATS invalidation request is issued from iommu_bus_notifier()
>> for surprise removal reason, as shown in above calltrace, device was
>> already removed from bus list. if searching it by BDF, return NULL.
>>
>> 3. The ATS invlidation request is issued from iommu_bus_notifier()
>> for safe removal, when is in handling, device is removed or link
>> is down. also as #2, device was already removed from bus list.
>> if searching it by BDF. got NULL.
>> ...
>>
>> so, searching device by BDF, only works for the ATS invalidation
>> request is from device driver.
> In the good path, where the hot removal is expected and this is about
> coordinating, the IOMMU driver should do an orderly shutdown of the
> ATS mechanism:
>
> 1 Write to PCI config space to disable the ATS
> 2 Make the IOMMU respond to ATS requests with UR and set it to BLOCKED
> 3 Issue a flush of the ATC
> 4 Wait for all outstanding ATC flushes to complete
>
> If it is a bad/surprise path where the device is already gone then:
>
> 1 should automatically not do anything, possibly timing out
> 2 must succeed
> 3 should time out
> 4 should "complete" in that the ATC flushes are all timed out
>
> IMHO all you need to do is not crash/lockup while processing the ATC
> timeouts. If this is a surprise path then the ATC timeout might
> already happened before the iommu driver remove notifier event happens.
>
> If the driver needs to translate from the IOMMU device table index
> into a struct device it is probably best to do that inside the driver.
>
> eg ARM maintains a rbtree in the iommu dev data. (see
> arm_smmu_insert_master)
An rbtree for IOMMU device data for the VT-d driver would be beneficial.
It also benefits other paths of fault handling, such as the I/O page
fault handling path, where it currently still relies on the PCI
subsystem to convert a RID value into a pci_device structure.
Given that such an rbtree would be helpful for multiple individual
drivers that handle PCI devices, it seems valuable to implement it in
the core?
Best regards,
baolu
Powered by blists - more mailing lists