From: Ethan Zhao <haifeng.zhao@xxxxxxxxxxxxxxx>then it's better to make it clear from beginning that this is about surprise
Sent: Thursday, December 28, 2023 9:03 PM
On 12/28/2023 4:30 PM, Tian, Kevin wrote:
bring the link down is a kind of surprise removal for hotplug capableFrom: Ethan Zhao <haifeng.zhao@xxxxxxxxxxxxxxx>is above describing the behavior of safe removal or surprise removal?
Sent: Thursday, December 28, 2023 8:17 AM
For those endpoint devices connect to system via hotplug capable ports,
users could request a warm reset to the device by flapping device's link
through setting the slot's link control register, as pciehp_ist() DLLSC
interrupt sequence response, pciehp will unload the device driver and
then power it off. thus cause an IOMMU device-TLB invalidation (Intel
VT-d spec, or ATS Invalidation in PCIe spec r6.1) request for device to
be sent and a long time completion/timeout waiting in interrupt context.
device.
removal in which device is removed and cannot respond to on-going
ATS invalidation request incurred in the removal process.
safe removal should be immune from this problem as the device is still
responsive in the whole removal process.
I'm curious why this doesn't occur earlier when the device is[ 4223.822628] Call Trace:
[ 4223.822628] qi_flush_dev_iotlb+0xb1/0xd0
[ 4223.822628] __dmar_remove_one_dev_info+0x224/0x250
[ 4223.822629] dmar_remove_one_dev_info+0x3e/0x50
[ 4223.822629] intel_iommu_release_device+0x1f/0x30
[ 4223.822629] iommu_release_device+0x33/0x60
[ 4223.822629] iommu_bus_notifier+0x7f/0x90
[ 4223.822630] blocking_notifier_call_chain+0x60/0x90
[ 4223.822630] device_del+0x2e5/0x420
[ 4223.822630] pci_remove_bus_device+0x70/0x110
[ 4223.822630] pciehp_unconfigure_device+0x7c/0x130
detached from the driver. At that point presumably the device
should be detached from the DMA domain which involves
ATS invalidation too.
one problem - the caller could pass multiple descriptors while typeHere is to handle such case, the invalidation request is sent, but thewhile (qi->desc_status[wait_index] != QI_DONE) {I'm not sure it's the right thing to do. Such check should be put in the
+ /*
+ * if the device-TLB invalidation target device is gone, don't
+ * wait anymore, it might take up to 1min+50%, causes
system
+ * hang. (see Implementation Note in PCIe spec r6.1 sec
10.3.1)
+ */
+ if ((type == QI_DIOTLB_TYPE || type == QI_DEIOTLB_TYPE)
&& pdev)
+ if (!pci_device_is_present(pdev))
+ break;
caller which has the device pointer and can already know it's absent
to not call those cache invalidation helpers.
device is just pulled out at that moment.
only refers to the 1st descriptor.
btw is it an Intel specific problem? A quick glance at smmu driver
suggests the same problem too:
arm_smmu_atc_inv_domain()
arm_smmu_cmdq_batch_submit()
arm_smmu_cmdq_issue_cmdlist()
arm_smmu_cmdq_poll_until_sync()
__arm_smmu_cmdq_poll_until_consumed()
/*
* Wait until the SMMU cons index passes llq->prod.
* Must be called with the cmdq lock held in some capacity.
*/
static int __arm_smmu_cmdq_poll_until_consumed(struct arm_smmu_device *smmu,
struct arm_smmu_ll_queue *llq)
is there a more general way to solve it?