This patchset is used to fix vt-d hard lockup reported when surprise
unplug ATS capable endpoint device connects to system via PCIe switch
as following topology.
+-[0000:15]-+-00.0 Intel Corporation Ice Lake Memory Map/VT-d
| +-00.1 Intel Corporation Ice Lake Mesh 2 PCIe
| +-00.2 Intel Corporation Ice Lake RAS
| +-00.4 Intel Corporation Device 0b23
| \-01.0-[16-1b]----00.0-[17-1b]--+-00.0-[18]----00.0
NVIDIA Corporation Device 2324
| +-01.0-[19]----00.0
Mellanox Technologies MT2910 Family [ConnectX-7]
User brought endpoint device 19:00.0's link down by flapping it's hotplug
capable slot 17:01.0 link control register, as sequence DLLSC response,
pciehp_ist() will unload device driver and power it off, durning device
driver is unloading an iommu device-TLB invalidation (Intel vt-d spec, or
'ATS invalidation' in PCIe spec) request issued to that link down device,
thus a long time completion/timeout waiting in interrupt context causes
continuous hard lockup warnning and system hang.
Other detail, see every patch commit log.
patch [3&4] were tested by [email protected] on stable v6.7-rc4.
patch [1&2] only passed compiling on stable v6.7-rc6.
change log:
v8:
- add a patch to break the loop for timeout device-TLB invalidation, as
Bjorn said there is possibility device just no reponse but not gone.
v7:
- reorder patches and revise commit log per Bjorn's guide.
- other code and commit log revise per Lukas' suggestion.
- rebased to stable v6.7-rc6.
v6:
- add two patches to break out device-TLB invalidation if device is gone.
v5:
- add a patch try to fix the rare case (surprise remove a device in
safe removal process). not work because surprise removal handling can't
re-enter when another safe removal is in process.
v4:
- move the PCI device state checking after ATS per Baolu's suggestion.
v3:
- fix commit description typo.
v2:
- revise commit[1] description part according to Lukas' suggestion.
- revise commit[2] description to clarify the issue's impact.
v1:
- https://lore.kernel.org/lkml/20231213034637.2603013-1-haifeng.zhao@
linux.intel.com/T/
Thanks,
Ethan
Ethan Zhao (5):
iommu/vt-d: add flush_target_dev member to struct intel_iommu and pass
device info to all ATS invalidation functions
iommu/vt-d: break out device-TLB invalidation if target device is gone
PCI: make pci_dev_is_disconnected() helper public for other drivers
iommu/vt-d: don't issue device-TLB invalidate request when device is
disconnected
iommu/vt-d: don't loop for timeout device-TLB invalidation request
forever
drivers/iommu/intel/dmar.c | 14 +++++++++++++-
drivers/iommu/intel/iommu.c | 1 +
drivers/iommu/intel/iommu.h | 2 ++
drivers/iommu/intel/pasid.c | 4 ++++
drivers/iommu/intel/svm.c | 1 +
drivers/pci/pci.h | 5 -----
include/linux/pci.h | 5 +++++
7 files changed, 26 insertions(+), 6 deletions(-)
--
2.31.1
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.
That would cause following continuous hard lockup warning and system hang
[ 4211.433662] pcieport 0000:17:01.0: pciehp: Slot(108): Link Down
[ 4211.433664] pcieport 0000:17:01.0: pciehp: Slot(108): Card not present
[ 4223.822591] NMI watchdog: Watchdog detected hard LOCKUP on cpu 144
[ 4223.822622] CPU: 144 PID: 1422 Comm: irq/57-pciehp Kdump: loaded Tainted: G S
OE kernel version xxxx
[ 4223.822623] Hardware name: vendorname xxxx 666-106,
BIOS 01.01.02.03.01 05/15/2023
[ 4223.822623] RIP: 0010:qi_submit_sync+0x2c0/0x490
[ 4223.822624] Code: 48 be 00 00 00 00 00 08 00 00 49 85 74 24 20 0f 95 c1 48 8b
57 10 83 c1 04 83 3c 1a 03 0f 84 a2 01 00 00 49 8b 04 24 8b 70 34 <40> f6 c6 1
0 74 17 49 8b 04 24 8b 80 80 00 00 00 89 c2 d3 fa 41 39
[ 4223.822624] RSP: 0018:ffffc4f074f0bbb8 EFLAGS: 00000093
[ 4223.822625] RAX: ffffc4f040059000 RBX: 0000000000000014 RCX: 0000000000000005
[ 4223.822625] RDX: ffff9f3841315800 RSI: 0000000000000000 RDI: ffff9f38401a8340
[ 4223.822625] RBP: ffff9f38401a8340 R08: ffffc4f074f0bc00 R09: 0000000000000000
[ 4223.822626] R10: 0000000000000010 R11: 0000000000000018 R12: ffff9f384005e200
[ 4223.822626] R13: 0000000000000004 R14: 0000000000000046 R15: 0000000000000004
[ 4223.822626] FS: 0000000000000000(0000) GS:ffffa237ae400000(0000)
knlGS:0000000000000000
[ 4223.822627] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4223.822627] CR2: 00007ffe86515d80 CR3: 000002fd3000a001 CR4: 0000000000770ee0
[ 4223.822627] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 4223.822628] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
[ 4223.822628] PKRU: 55555554
[ 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
[ 4223.822631] pciehp_disable_slot+0x6b/0x100
[ 4223.822631] pciehp_handle_presence_or_link_change+0xd8/0x320
[ 4223.822631] pciehp_ist+0x176/0x180
[ 4223.822631] ? irq_finalize_oneshot.part.50+0x110/0x110
[ 4223.822632] irq_thread_fn+0x19/0x50
[ 4223.822632] irq_thread+0x104/0x190
[ 4223.822632] ? irq_forced_thread_fn+0x90/0x90
[ 4223.822632] ? irq_thread_check_affinity+0xe0/0xe0
[ 4223.822633] kthread+0x114/0x130
[ 4223.822633] ? __kthread_cancel_work+0x40/0x40
[ 4223.822633] ret_from_fork+0x1f/0x30
[ 4223.822633] Kernel panic - not syncing: Hard LOCKUP
[ 4223.822634] CPU: 144 PID: 1422 Comm: irq/57-pciehp Kdump: loaded Tainted: G S
OE kernel version xxxx
[ 4223.822634] Hardware name: vendorname xxxx 666-106,
BIOS 01.01.02.03.01 05/15/2023
[ 4223.822634] Call Trace:
[ 4223.822634] <NMI>
[ 4223.822635] dump_stack+0x6d/0x88
[ 4223.822635] panic+0x101/0x2d0
[ 4223.822635] ? ret_from_fork+0x11/0x30
[ 4223.822635] nmi_panic.cold.14+0xc/0xc
[ 4223.822636] watchdog_overflow_callback.cold.8+0x6d/0x81
[ 4223.822636] __perf_event_overflow+0x4f/0xf0
[ 4223.822636] handle_pmi_common+0x1ef/0x290
[ 4223.822636] ? __set_pte_vaddr+0x28/0x40
[ 4223.822637] ? flush_tlb_one_kernel+0xa/0x20
[ 4223.822637] ? __native_set_fixmap+0x24/0x30
[ 4223.822637] ? ghes_copy_tofrom_phys+0x70/0x100
[ 4223.822637] ? __ghes_peek_estatus.isra.16+0x49/0xa0
[ 4223.822637] intel_pmu_handle_irq+0xba/0x2b0
[ 4223.822638] perf_event_nmi_handler+0x24/0x40
[ 4223.822638] nmi_handle+0x4d/0xf0
[ 4223.822638] default_do_nmi+0x49/0x100
[ 4223.822638] exc_nmi+0x134/0x180
[ 4223.822639] end_repeat_nmi+0x16/0x67
[ 4223.822639] RIP: 0010:qi_submit_sync+0x2c0/0x490
[ 4223.822639] Code: 48 be 00 00 00 00 00 08 00 00 49 85 74 24 20 0f 95 c1 48 8b
57 10 83 c1 04 83 3c 1a 03 0f 84 a2 01 00 00 49 8b 04 24 8b 70 34 <40> f6 c6 10
74 17 49 8b 04 24 8b 80 80 00 00 00 89 c2 d3 fa 41 39
[ 4223.822640] RSP: 0018:ffffc4f074f0bbb8 EFLAGS: 00000093
[ 4223.822640] RAX: ffffc4f040059000 RBX: 0000000000000014 RCX: 0000000000000005
[ 4223.822640] RDX: ffff9f3841315800 RSI: 0000000000000000 RDI: ffff9f38401a8340
[ 4223.822641] RBP: ffff9f38401a8340 R08: ffffc4f074f0bc00 R09: 0000000000000000
[ 4223.822641] R10: 0000000000000010 R11: 0000000000000018 R12: ffff9f384005e200
[ 4223.822641] R13: 0000000000000004 R14: 0000000000000046 R15: 0000000000000004
[ 4223.822641] ? qi_submit_sync+0x2c0/0x490
[ 4223.822642] ? qi_submit_sync+0x2c0/0x490
[ 4223.822642] </NMI>
[ 4223.822642] qi_flush_dev_iotlb+0xb1/0xd0
[ 4223.822642] __dmar_remove_one_dev_info+0x224/0x250
[ 4223.822643] dmar_remove_one_dev_info+0x3e/0x50
[ 4223.822643] intel_iommu_release_device+0x1f/0x30
[ 4223.822643] iommu_release_device+0x33/0x60
[ 4223.822643] iommu_bus_notifier+0x7f/0x90
[ 4223.822644] blocking_notifier_call_chain+0x60/0x90
[ 4223.822644] device_del+0x2e5/0x420
[ 4223.822644] pci_remove_bus_device+0x70/0x110
[ 4223.822644] pciehp_unconfigure_device+0x7c/0x130
[ 4223.822644] pciehp_disable_slot+0x6b/0x100
[ 4223.822645] pciehp_handle_presence_or_link_change+0xd8/0x320
[ 4223.822645] pciehp_ist+0x176/0x180
[ 4223.822645] ? irq_finalize_oneshot.part.50+0x110/0x110
[ 4223.822645] irq_thread_fn+0x19/0x50
[ 4223.822646] irq_thread+0x104/0x190
[ 4223.822646] ? irq_forced_thread_fn+0x90/0x90
[ 4223.822646] ? irq_thread_check_affinity+0xe0/0xe0
[ 4223.822646] kthread+0x114/0x130
[ 4223.822647] ? __kthread_cancel_work+0x40/0x40
[ 4223.822647] ret_from_fork+0x1f/0x30
[ 4223.822647] Kernel Offset: 0x6400000 from 0xffffffff81000000 (relocation
range: 0xffffffff80000000-0xffffffffbfffffff)
Furthermore even an in-process safe removal unplugged device could be
surprise removed anytime, thus need to check the ATS invalidation target
device state to see if it is gone, and don't wait for the completion/
timeout blindly, thus avoid the up to 1min+50% (see Implementation Note
in PCIe spec r6.1 sec 10.3.1) waiting and cause hard lockup or system
hang.
Signed-off-by: Ethan Zhao <[email protected]>
---
drivers/iommu/intel/dmar.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 23cb80d62a9a..76903a8bf963 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1347,6 +1347,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
unsigned int count, unsigned long options)
{
struct q_inval *qi = iommu->qi;
+ struct pci_dev *pdev = NULL;
s64 devtlb_start_ktime = 0;
s64 iotlb_start_ktime = 0;
s64 iec_start_ktime = 0;
@@ -1360,6 +1361,9 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
if (!qi)
return 0;
+ if (iommu->flush_target_dev && dev_is_pci(iommu->flush_target_dev))
+ pdev = to_pci_dev(iommu->flush_target_dev);
+
type = desc->qw0 & GENMASK_ULL(3, 0);
if ((type == QI_IOTLB_TYPE || type == QI_EIOTLB_TYPE) &&
@@ -1423,6 +1427,14 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
writel(qi->free_head << shift, iommu->reg + DMAR_IQT_REG);
while (qi->desc_status[wait_index] != QI_DONE) {
+ /*
+ * 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;
/*
* We will leave the interrupts disabled, to prevent interrupt
* context to queue another cmd while a cmd is already submitted
--
2.31.1
As iommu is a pointer member of device_domain_info, so can't play
trick like container_of() to get the info and device instance for
qi_submit_sync() low level function to check device status, add a
flush_target_dev member to struct inte_iommu and pass dev info to
all device-TLB invalidation (a.k.a ATS invalidation) functions.
Signed-off-by: Ethan Zhao <[email protected]>
---
drivers/iommu/intel/iommu.c | 1 +
drivers/iommu/intel/iommu.h | 2 ++
drivers/iommu/intel/pasid.c | 1 +
drivers/iommu/intel/svm.c | 1 +
4 files changed, 5 insertions(+)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 897159dba47d..c3724f1d86dc 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1461,6 +1461,7 @@ static void __iommu_flush_dev_iotlb(struct device_domain_info *info,
sid = info->bus << 8 | info->devfn;
qdep = info->ats_qdep;
+ info->iommu->flush_target_dev = info->dev;
qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
qdep, addr, mask);
quirk_extra_dev_tlb_flush(info, addr, mask, IOMMU_NO_PASID, qdep);
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index ce030c5b5772..e892c5c7560a 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -731,6 +731,8 @@ struct intel_iommu {
void *perf_statistic;
struct iommu_pmu *pmu;
+
+ struct device *flush_target_dev; /* the target device TLB to be invalidated. */
};
/* PCI domain-device relationship */
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 74e8e4c17e81..1c87fb1b1039 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -485,6 +485,7 @@ devtlb_invalidation_with_pasid(struct intel_iommu *iommu,
qdep = info->ats_qdep;
pfsid = info->pfsid;
+ info->iommu->flush_target_dev = info->dev;
/*
* When PASID 0 is used, it indicates RID2PASID(DMA request w/o PASID),
* devTLB flush w/o PASID should be used. For non-zero PASID under
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index ac12f76c1212..d42a99801cdf 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -181,6 +181,7 @@ static void __flush_svm_range_dev(struct intel_svm *svm,
qi_flush_piotlb(sdev->iommu, sdev->did, svm->pasid, address, pages, ih);
if (info->ats_enabled) {
+ info->iommu->flush_target_dev = info->dev;
qi_flush_dev_iotlb_pasid(sdev->iommu, sdev->sid, info->pfsid,
svm->pasid, sdev->qdep, address,
order_base_2(pages));
--
2.31.1
Make pci_dev_is_disconnected() public so that it can be called from
Intel vt-d driver to quickly fix/workaround the surprise removal
unplug hang issue for those ATS capable devices on PCIe switch downstream
hotplug capable ports.
Beside pci_device_is_present() function, this one has no config space
space access, so is light enough to optimize the normal pure surprise
removal and safe removal flow.
Tested-by: Haorong Ye <[email protected]>
Signed-off-by: Ethan Zhao <[email protected]>
---
drivers/pci/pci.h | 5 -----
include/linux/pci.h | 5 +++++
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 5ecbcf041179..75fa2084492f 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -366,11 +366,6 @@ static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
return 0;
}
-static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
-{
- return dev->error_state == pci_channel_io_perm_failure;
-}
-
/* pci_dev priv_flags */
#define PCI_DEV_ADDED 0
#define PCI_DPC_RECOVERED 1
diff --git a/include/linux/pci.h b/include/linux/pci.h
index dea043bc1e38..4779eec8b267 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2506,6 +2506,11 @@ static inline struct pci_dev *pcie_find_root_port(struct pci_dev *dev)
return NULL;
}
+static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
+{
+ return dev->error_state == pci_channel_io_perm_failure;
+}
+
void pci_request_acs(void);
bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
bool pci_acs_path_enabled(struct pci_dev *start,
--
2.31.1
When the device-TLB invalidation (ATS invalidation) timeout happens, the
qi_submit_sync() will restart and loop for the invalidation request
forever till it is done, it will block another invalidation thread such
as the fq_timer to issue invalidation request, cause the system lockup as
following
[exception RIP: native_queued_spin_lock_slowpath+92]
RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002
RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0
RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000
R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000
R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
--- ---
(the left part of exception see the hotplug case of ATS capable device)
If one endpoint device just no response to the device-TLB invalidation
request, but is not gone, it will bring down the whole system, to avoid
such case, don't try the timeout device-TLB request forever.
and as synchronous program model of current qi_submit_sync() implementation
we couldn't wait for the enough time as PCIe spec said 1min+50%, just break
it in current sync model. (PCIe spec r6.1, sec 10.3.1)
Signed-off-by: Ethan Zhao <[email protected]>
---
drivers/iommu/intel/dmar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 76903a8bf963..206ab0b7294f 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1457,7 +1457,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
reclaim_free_desc(qi);
raw_spin_unlock_irqrestore(&qi->q_lock, flags);
- if (rc == -EAGAIN)
+ if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != QI_DEIOTLB_TYPE)
goto restart;
if (iotlb_start_ktime)
--
2.31.1
Except those aggressive hotplug cases - surprise remove a hotplug device
while its safe removal is requested and handled in process by:
1. pull it out directly.
2. turn off its power.
3. bring the link down.
4. just died there that moment.
etc, in a word, 'gone' or 'disconnected'.
Mostly are regular normal safe removal and surprise removal unplug.
these hot unplug handling process could be optimized for fix the ATS
invalidation hang issue by calling pci_dev_is_disconnected() in function
devtlb_invalidation_with_pasid() to check target device state to avoid
sending meaningless ATS invalidation request to iommu when device is gone.
(see IMPLEMENTATION NOTE in PCIe spec r6.1 section 10.3.1)
For safe removal, device wouldn't be removed untill the whole software
handling process is done, it wouldn't trigger the hard lock up issue
caused by too long ATS invalidation timeout wait. in safe removal path,
device state isn't set to pci_channel_io_perm_failure in
pciehp_unconfigure_device() by checking 'presence' parameter, calling
pci_dev_is_disconnected() in devtlb_invalidation_with_pasid() will return
false there, wouldn't break the function.
For surprise removal, device state is set to pci_channel_io_perm_failure in
pciehp_unconfigure_device(), means device is already gone (disconnected)
call pci_dev_is_disconnected() in devtlb_invalidation_with_pasid() will
return true to break the function not to send ATS invalidation request to
the disconnected device blindly, thus avoid the further long time waiting
triggers the hard lockup.
safe removal & surprise removal
pciehp_ist()
pciehp_handle_presence_or_link_change()
pciehp_disable_slot()
remove_board()
pciehp_unconfigure_device(presence)
Tested-by: Haorong Ye <[email protected]>
Signed-off-by: Ethan Zhao <[email protected]>
---
drivers/iommu/intel/pasid.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 1c87fb1b1039..a08bdbec90eb 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -481,6 +481,9 @@ devtlb_invalidation_with_pasid(struct intel_iommu *iommu,
if (!info || !info->ats_enabled)
return;
+ if (pci_dev_is_disconnected(to_pci_dev(dev)))
+ return;
+
sid = info->bus << 8 | info->devfn;
qdep = info->ats_qdep;
pfsid = info->pfsid;
--
2.31.1
On 12/27/2023 10:59 AM, Ethan Zhao wrote:
> When the device-TLB invalidation (ATS invalidation) timeout happens, the
> qi_submit_sync() will restart and loop for the invalidation request
> forever till it is done, it will block another invalidation thread such
> as the fq_timer to issue invalidation request, cause the system lockup as
> following
>
> [exception RIP: native_queued_spin_lock_slowpath+92]
>
> RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002
>
> RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000
>
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0
>
> RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000
>
> R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000
>
> R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980
>
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
--- ---
#12 [ffffb202f268cdc8] native_queued_spin_lock_slowpath at ffffffffa9d1025c
#13 [ffffb202f268cdc8] do_raw_spin_lock at ffffffffa9d121f1
#14 [ffffb202f268cdd8] _raw_spin_lock_irqsave at ffffffffaa51795b
#15 [ffffb202f268cdf8] iommu_flush_dev_iotlb at ffffffffaa20df48
#16 [ffffb202f268ce28] iommu_flush_iova at ffffffffaa20e182
#17 [ffffb202f268ce60] iova_domain_flush at ffffffffaa220e27
#18 [ffffb202f268ce70] fq_flush_timeout at ffffffffaa221c9d
#19 [ffffb202f268cea8] call_timer_fn at ffffffffa9d46661
#20 [ffffb202f268cf08] run_timer_softirq at ffffffffa9d47933
#21 [ffffb202f268cf98] __softirqentry_text_start at ffffffffaa8000e0
#22 [ffffb202f268cff0] asm_call_sysvec_on_stack at ffffffffaa60114f
> --- ---
>
> (the left part of exception see the hotplug case of ATS capable device)
>
> If one endpoint device just no response to the device-TLB invalidation
> request, but is not gone, it will bring down the whole system, to avoid
> such case, don't try the timeout device-TLB request forever.
>
> and as synchronous program model of current qi_submit_sync() implementation
> we couldn't wait for the enough time as PCIe spec said 1min+50%, just break
> it in current sync model. (PCIe spec r6.1, sec 10.3.1)
>
> Signed-off-by: Ethan Zhao <[email protected]>
> ---
> drivers/iommu/intel/dmar.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 76903a8bf963..206ab0b7294f 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1457,7 +1457,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
> reclaim_free_desc(qi);
> raw_spin_unlock_irqrestore(&qi->q_lock, flags);
>
> - if (rc == -EAGAIN)
> + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != QI_DEIOTLB_TYPE)
> goto restart;
>
> if (iotlb_start_ktime)
On 12/27/2023 10:59 AM, Ethan Zhao wrote:
> This patchset is used to fix vt-d hard lockup reported when surprise
> unplug ATS capable endpoint device connects to system via PCIe switch
> as following topology.
>
> +-[0000:15]-+-00.0 Intel Corporation Ice Lake Memory Map/VT-d
> | +-00.1 Intel Corporation Ice Lake Mesh 2 PCIe
> | +-00.2 Intel Corporation Ice Lake RAS
> | +-00.4 Intel Corporation Device 0b23
> | \-01.0-[16-1b]----00.0-[17-1b]--+-00.0-[18]----00.0
> NVIDIA Corporation Device 2324
> | +-01.0-[19]----00.0
> Mellanox Technologies MT2910 Family [ConnectX-7]
>
> User brought endpoint device 19:00.0's link down by flapping it's hotplug
> capable slot 17:01.0 link control register, as sequence DLLSC response,
> pciehp_ist() will unload device driver and power it off, durning device
> driver is unloading an iommu device-TLB invalidation (Intel vt-d spec, or
> 'ATS invalidation' in PCIe spec) request issued to that link down device,
> thus a long time completion/timeout waiting in interrupt context causes
> continuous hard lockup warnning and system hang.
>
> Other detail, see every patch commit log.
>
> patch [3&4] were tested by [email protected] on stable v6.7-rc4.
> patch [1&2] only passed compiling on stable v6.7-rc6.
>
>
> change log:
> v8:
> - add a patch to break the loop for timeout device-TLB invalidation, as
> Bjorn said there is possibility device just no reponse but not gone.
> v7:
> - reorder patches and revise commit log per Bjorn's guide.
> - other code and commit log revise per Lukas' suggestion.
> - rebased to stable v6.7-rc6.
> v6:
> - add two patches to break out device-TLB invalidation if device is gone.
> v5:
> - add a patch try to fix the rare case (surprise remove a device in
> safe removal process). not work because surprise removal handling can't
> re-enter when another safe removal is in process.
> v4:
> - move the PCI device state checking after ATS per Baolu's suggestion.
> v3:
> - fix commit description typo.
> v2:
> - revise commit[1] description part according to Lukas' suggestion.
> - revise commit[2] description to clarify the issue's impact.
> v1:
> - https://lore.kernel.org/lkml/20231213034637.2603013-1-haifeng.zhao@
> linux.intel.com/T/
>
>
> Thanks,
> Ethan
>
> Ethan Zhao (5):
> iommu/vt-d: add flush_target_dev member to struct intel_iommu and pass
> device info to all ATS invalidation functions
> iommu/vt-d: break out device-TLB invalidation if target device is gone
> PCI: make pci_dev_is_disconnected() helper public for other drivers
> iommu/vt-d: don't issue device-TLB invalidate request when device is
> disconnected
> iommu/vt-d: don't loop for timeout device-TLB invalidation request
> forever
>
> drivers/iommu/intel/dmar.c | 14 +++++++++++++-
> drivers/iommu/intel/iommu.c | 1 +
> drivers/iommu/intel/iommu.h | 2 ++
> drivers/iommu/intel/pasid.c | 4 ++++
> drivers/iommu/intel/svm.c | 1 +
> drivers/pci/pci.h | 5 -----
> include/linux/pci.h | 5 +++++
> 7 files changed, 26 insertions(+), 6 deletions(-)
Sorry, post the wrong call stack, will re-send.
>
On 12/27/2023 10:59 AM, Ethan Zhao wrote:
> When the device-TLB invalidation (ATS invalidation) timeout happens, the
> qi_submit_sync() will restart and loop for the invalidation request
> forever till it is done, it will block another invalidation thread such
> as the fq_timer to issue invalidation request, cause the system lockup as
> following
>
> [exception RIP: native_queued_spin_lock_slowpath+92]
>
> RIP: ffffffffa9d1025c RSP: ffffb202f268cdc8 RFLAGS: 00000002
>
> RAX: 0000000000000101 RBX: ffffffffab36c2a0 RCX: 0000000000000000
>
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffffab36c2a0
>
> RBP: ffffffffab36c2a0 R8: 0000000000000001 R9: 0000000000000000
>
> R10: 0000000000000010 R11: 0000000000000018 R12: 0000000000000000
>
> R13: 0000000000000004 R14: ffff9e10d71b1c88 R15: ffff9e10d71b1980
>
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
Wrong past, please ignore this series.
Thanks,
Ethan
>
> --- ---
>
> (the left part of exception see the hotplug case of ATS capable device)
>
> If one endpoint device just no response to the device-TLB invalidation
> request, but is not gone, it will bring down the whole system, to avoid
> such case, don't try the timeout device-TLB request forever.
>
> and as synchronous program model of current qi_submit_sync() implementation
> we couldn't wait for the enough time as PCIe spec said 1min+50%, just break
> it in current sync model. (PCIe spec r6.1, sec 10.3.1)
>
> Signed-off-by: Ethan Zhao <[email protected]>
> ---
> drivers/iommu/intel/dmar.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 76903a8bf963..206ab0b7294f 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1457,7 +1457,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
> reclaim_free_desc(qi);
> raw_spin_unlock_irqrestore(&qi->q_lock, flags);
>
> - if (rc == -EAGAIN)
> + if (rc == -EAGAIN && type !=QI_DIOTLB_TYPE && type != QI_DEIOTLB_TYPE)
> goto restart;
>
> if (iotlb_start_ktime)
I suggest using "ATS Invalidate Request" in the subject as well.
Otherwise we have to figure out whether "device-TLB invalidate
request" is the same as "ATS Invalidate Request".
If they are the same, just use the same words.
On Tue, Dec 26, 2023 at 09:59:22PM -0500, Ethan Zhao wrote:
> Except those aggressive hotplug cases - surprise remove a hotplug device
> while its safe removal is requested and handled in process by:
>
> 1. pull it out directly.
> 2. turn off its power.
> 3. bring the link down.
> 4. just died there that moment.
>
> etc, in a word, 'gone' or 'disconnected'.
>
> Mostly are regular normal safe removal and surprise removal unplug.
> these hot unplug handling process could be optimized for fix the ATS
> invalidation hang issue by calling pci_dev_is_disconnected() in function
> devtlb_invalidation_with_pasid() to check target device state to avoid
> sending meaningless ATS invalidation request to iommu when device is gone.
> (see IMPLEMENTATION NOTE in PCIe spec r6.1 section 10.3.1)
Suggest "ATS Invalidate Request", capitalized exactly that way so we
know it's a specific name of something defined in the PCIe spec.
> For safe removal, device wouldn't be removed untill the whole software
> handling process is done, it wouldn't trigger the hard lock up issue
> caused by too long ATS invalidation timeout wait. in safe removal path,
Ditto.
Capitalize "In the safe removal ..." since it starts a new sentence.
> device state isn't set to pci_channel_io_perm_failure in
> pciehp_unconfigure_device() by checking 'presence' parameter, calling
> pci_dev_is_disconnected() in devtlb_invalidation_with_pasid() will return
> false there, wouldn't break the function.
>
> For surprise removal, device state is set to pci_channel_io_perm_failure in
> pciehp_unconfigure_device(), means device is already gone (disconnected)
> call pci_dev_is_disconnected() in devtlb_invalidation_with_pasid() will
> return true to break the function not to send ATS invalidation request to
Ditto.
> the disconnected device blindly, thus avoid the further long time waiting
> triggers the hard lockup.
>
> safe removal & surprise removal
>
> pciehp_ist()
> pciehp_handle_presence_or_link_change()
> pciehp_disable_slot()
> remove_board()
> pciehp_unconfigure_device(presence)
On 12/27/2023 9:11 PM, Bjorn Helgaas wrote:
> I suggest using "ATS Invalidate Request" in the subject as well.
> Otherwise we have to figure out whether "device-TLB invalidate
> request" is the same as "ATS Invalidate Request".
>
> If they are the same, just use the same words.
>
> On Tue, Dec 26, 2023 at 09:59:22PM -0500, Ethan Zhao wrote:
>> Except those aggressive hotplug cases - surprise remove a hotplug device
>> while its safe removal is requested and handled in process by:
>>
>> 1. pull it out directly.
>> 2. turn off its power.
>> 3. bring the link down.
>> 4. just died there that moment.
>>
>> etc, in a word, 'gone' or 'disconnected'.
>>
>> Mostly are regular normal safe removal and surprise removal unplug.
>> these hot unplug handling process could be optimized for fix the ATS
>> invalidation hang issue by calling pci_dev_is_disconnected() in function
>> devtlb_invalidation_with_pasid() to check target device state to avoid
>> sending meaningless ATS invalidation request to iommu when device is gone.
>> (see IMPLEMENTATION NOTE in PCIe spec r6.1 section 10.3.1)
> Suggest "ATS Invalidate Request", capitalized exactly that way so we
> know it's a specific name of something defined in the PCIe spec.
>
>> For safe removal, device wouldn't be removed untill the whole software
>> handling process is done, it wouldn't trigger the hard lock up issue
>> caused by too long ATS invalidation timeout wait. in safe removal path,
> Ditto.
>
> Capitalize "In the safe removal ..." since it starts a new sentence.
>
>> device state isn't set to pci_channel_io_perm_failure in
>> pciehp_unconfigure_device() by checking 'presence' parameter, calling
>> pci_dev_is_disconnected() in devtlb_invalidation_with_pasid() will return
>> false there, wouldn't break the function.
>>
>> For surprise removal, device state is set to pci_channel_io_perm_failure in
>> pciehp_unconfigure_device(), means device is already gone (disconnected)
>> call pci_dev_is_disconnected() in devtlb_invalidation_with_pasid() will
>> return true to break the function not to send ATS invalidation request to
> Ditto.
Okay.
Thanks,
Ethan
>> the disconnected device blindly, thus avoid the further long time waiting
>> triggers the hard lockup.
>>
>> safe removal & surprise removal
>>
>> pciehp_ist()
>> pciehp_handle_presence_or_link_change()
>> pciehp_disable_slot()
>> remove_board()
>> pciehp_unconfigure_device(presence)