linux-kernel - Re: [PATCH v12 5/5] iommu/vt-d: improve ITE fault handling if target device isn't present

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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