Message ID | 20231220005153.3984502-3-haifeng.zhao@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-6215-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:24d3:b0:fb:cd0c:d3e with SMTP id r19csp2336055dyi; Tue, 19 Dec 2023 16:55:40 -0800 (PST) X-Google-Smtp-Source: AGHT+IHHB5TjaXSVWaaYiBcK+6AgSiKhMuUYsfcYV2aIMLUsSDy/CXDSGRagAtCKIFzk2iCVfkEq X-Received: by 2002:a05:6e02:1c2f:b0:35d:59a2:929a with SMTP id m15-20020a056e021c2f00b0035d59a2929amr22739315ilh.46.1703033635174; Tue, 19 Dec 2023 16:53:55 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1703033635; cv=none; d=google.com; s=arc-20160816; b=lrsIikDvuybZnnvScWlhBvc4NT3o3+rC5tpgo2jo0garkkGpr1fwt1IJKvQNaBFKXy tgMjDGfggfnWzsVV0+EcxdV1kLaFvLi5KVcjBsE+pP5q5zEOiGhPqHRLjvWyW+LlGMfY OHJP5koWcXZjYlvPyRI85aIB9BtMcpD0qp1T816B0Z8HYf01NAXVyq6QlIxghatHwW30 sCwoUeM6uJNshXgJFf4yxopD512ibYl85OGih6u+sOvoYuY0Ikp5OQLmt5UEiY2TkHms 2XrtfTP/Cr2jFsiQau9ZCxvSOeLhfr9jOysnhGW2TLjflAM6/mx0STcFSaaDOf3MgZ95 kNHA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :date:subject:cc:to:from:dkim-signature; bh=J7i7nV+e1J1Vs3Q8lkc/iiCbAlrYkhESOlx+krfND5I=; fh=cQTxynQ7rGExgmbyXq8tNmd/VmE1Fzgt9QW51hABj2c=; b=A3bjtWAMOCQq5i41i/7iq8IylYkueasqLXQmSyns3aiYP560ENRl9TsW8s0Rda707N EiV9D6d96F7gikTEoOEfMzgfx9J7Z78ruUG4cvSFv226x9krJncrguj18m+W5DAPp6tN RiPQmpAvcOXJd/9z400wLb2g4yGXAkAZQpNoPElecVEWUk414sSTdR/zTf3IyiU8NqFm yWR81VxW7ti6iIymWPN7iKOTU+6dxXaix6CmijkrRc44xcVzZlMt/n1AEAa/QzAlZ4+V Ji4YJhMzcFWcSVMlHrrZLqCr2r1x/Q9JzpoRLjPQ1HWDmfrmj79oKREk2LLjisuCtPWY GnDw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=S8BAewp8; spf=pass (google.com: domain of linux-kernel+bounces-6215-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-6215-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id x4-20020a654544000000b005c6ec7ede97si19906272pgr.870.2023.12.19.16.53.55 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Dec 2023 16:53:55 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-6215-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=S8BAewp8; spf=pass (google.com: domain of linux-kernel+bounces-6215-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-6215-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id E584428596A for <ouuuleilei@gmail.com>; Wed, 20 Dec 2023 00:53:54 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 9AE4DC8E8; Wed, 20 Dec 2023 00:52:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="S8BAewp8" X-Original-To: linux-kernel@vger.kernel.org Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5376846B0; Wed, 20 Dec 2023 00:52:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1703033554; x=1734569554; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=TeO26yGTeKCJ1V78ZWomV1eTGapN19AztW7PHl3ULlg=; b=S8BAewp8Yo+6T3zI0t8W9/eipw/v2Tnfzo6No22UhyRemn04y4+hh8qp ZpwYdHkrQnpt/FqnYtgIeHUJZWVEtRs2kE/zYG77bawu4OM8A6uBt0w1V TcHSnePiBAv2QknmeZIou5MC3/tIdyolUwzP8S6HYyupoa6H4vOkKniiY c7q1PFZ9NmY/PIOUSLOEnTwuTtk/ajaTAo5+FVGYlwOVJLCis9h6Ek6uj a+enHpEaS3xKfDO0DuI5PyJOegaj2zdZsIpf6vOyTSTbNjsS0FaauJMve E4pMIX+1W25QaGf00BgqDcfY2dyziytoUmaYKkOC6CeTxqQdnK8IbtIk2 A==; X-IronPort-AV: E=McAfee;i="6600,9927,10929"; a="2566104" X-IronPort-AV: E=Sophos;i="6.04,290,1695711600"; d="scan'208";a="2566104" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmvoesa105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Dec 2023 16:52:05 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10929"; a="1107551580" X-IronPort-AV: E=Sophos;i="6.04,290,1695711600"; d="scan'208";a="1107551580" Received: from ply01-vm-store.bj.intel.com ([10.238.153.201]) by fmsmga005.fm.intel.com with ESMTP; 19 Dec 2023 16:52:01 -0800 From: Ethan Zhao <haifeng.zhao@linux.intel.com> To: bhelgaas@google.com, baolu.lu@linux.intel.com, dwmw2@infradead.org, will@kernel.org, robin.murphy@arm.com, lukas@wunner.de Cc: linux-pci@vger.kernel.org, iommu@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [PATCH v4 2/2] iommu/vt-d: don's issue devTLB flush request when device is disconnected Date: Tue, 19 Dec 2023 19:51:53 -0500 Message-Id: <20231220005153.3984502-3-haifeng.zhao@linux.intel.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20231220005153.3984502-1-haifeng.zhao@linux.intel.com> References: <20231220005153.3984502-1-haifeng.zhao@linux.intel.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785760197136033082 X-GMAIL-MSGID: 1785760197136033082 |
Series |
fix vt-d hard lockup when hotplug ATS capable device
|
|
Commit Message
Ethan Zhao
Dec. 20, 2023, 12:51 a.m. UTC
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 pciehpt_ist() DLLSC interrupt sequence response, pciehp will unload the device driver and then power it off. thus cause an IOMMU devTLB flush 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) Fix it by checking the device's error_state in devtlb_invalidation_with_pasid() to avoid sending meaningless devTLB flush request to link down device that is set to pci_channel_io_perm_failure and then power off in pciehp_ist() pciehp_handle_presence_or_link_change() pciehp_disable_slot() remove_board() pciehp_unconfigure_device() For SAVE_REMOVAL unplug, link is alive when iommu releases device and issues devTLB invalidate request, wouldn't trigger such issue. This patch works for all kinds of SURPPRISE_REMOVAL unplug operation. Tested-by: Haorong Ye <yehaorong@bytedance.com> Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com> --- drivers/iommu/intel/pasid.c | 3 +++ 1 file changed, 3 insertions(+)
Comments
On Tue, Dec 19, 2023 at 07:51:53PM -0500, Ethan Zhao wrote: > 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 pciehpt_ist() DLLSC > interrupt sequence response, pciehp will unload the device driver and > then power it off. thus cause an IOMMU devTLB flush request for device to > be sent and a long time completion/timeout waiting in interrupt context. I think the problem is in the "waiting in interrupt context". Can you change qi_submit_sync() to *sleep* until the queue is done? Instead of busy-waiting in atomic context? Is the hardware capable of sending an interrupt once the queue is done? If it is not capable, would it be viable to poll with exponential backoff and sleep in-between polling once the polling delay increases beyond, say, 10 usec? Again, the proposed patch is not a proper solution. It will paper over the issue most of the time but every once in a while someone will still get a hard lockup splat and it will then be more difficult to reproduce and fix if the proposed patch is accepted. > [ 4223.822622] CPU: 144 PID: 1422 Comm: irq/57-pciehp Kdump: loaded Tainted: G S > OE kernel version xxxx I don't see any reason to hide the kernel version. This isn't Intel Confidential information. > [ 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 __dmar_remove_one_dev_info() was removed by db75c9573b08 in v6.0 one and a half years ago, so the stack trace appears to be from an older kernel version. Thanks, Lukas
On Thu, Dec 21, 2023 at 11:39:40AM +0100, Lukas Wunner wrote: > On Tue, Dec 19, 2023 at 07:51:53PM -0500, Ethan Zhao wrote: > > 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 pciehpt_ist() DLLSC > > interrupt sequence response, pciehp will unload the device driver and > > then power it off. thus cause an IOMMU devTLB flush request for device to > > be sent and a long time completion/timeout waiting in interrupt context. > > I think the problem is in the "waiting in interrupt context". I'm wondering whether Intel IOMMUs possibly have a (perhaps undocumented) capability to reduce the Invalidate Completion Timeout to a sane value? Could you check whether that's supported? Granted, the Implementation Note you've pointed to allows 1 sec + 50%, but that's not even a "must", it's a "should". So devices are free to take even longer. We have to cut off at *some* point. Thanks, Lukas
On 12/21/2023 6:39 PM, Lukas Wunner wrote: > On Tue, Dec 19, 2023 at 07:51:53PM -0500, Ethan Zhao wrote: >> 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 pciehpt_ist() DLLSC >> interrupt sequence response, pciehp will unload the device driver and >> then power it off. thus cause an IOMMU devTLB flush request for device to >> be sent and a long time completion/timeout waiting in interrupt context. > I think the problem is in the "waiting in interrupt context". > > Can you change qi_submit_sync() to *sleep* until the queue is done? > Instead of busy-waiting in atomic context? If you read that function carefully, you wouldn't say "sleep" there.... that is 'sync'ed. > > Is the hardware capable of sending an interrupt once the queue is done? > If it is not capable, would it be viable to poll with exponential backoff > and sleep in-between polling once the polling delay increases beyond, say, > 10 usec? I don't know if the polling along sleeping for completion of meanningless devTLB invalidation request blindly sent to (removed/powered down/link down) device makes sense or not. But according to PCIe spec 6.1 10.3.1 "Software ensures no invalidations are issued to a Function when its ATS capability is disabled. " > > Again, the proposed patch is not a proper solution. It will paper over > the issue most of the time but every once in a while someone will still > get a hard lockup splat and it will then be more difficult to reproduce > and fix if the proposed patch is accepted. Could you point out why is not proper ? Is there any other window the hard lockup still could happen with the ATS capable devcie supprise_removal case if we checked the connection state first ? Please help to elaberate it. > > >> [ 4223.822622] CPU: 144 PID: 1422 Comm: irq/57-pciehp Kdump: loaded Tainted: G S >> OE kernel version xxxx > I don't see any reason to hide the kernel version. > This isn't Intel Confidential information. > Yes, this is the old kernel stack trace, but customer also tried lasted 6.7rc4 (doesn't work) and the patched 6.7rc4 (fixed). Thanks, Ethan >> [ 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 > __dmar_remove_one_dev_info() was removed by db75c9573b08 in v6.0 > one and a half years ago, so the stack trace appears to be from > an older kernel version. > > Thanks, > > Lukas
On 12/21/2023 7:01 PM, Lukas Wunner wrote: > On Thu, Dec 21, 2023 at 11:39:40AM +0100, Lukas Wunner wrote: >> On Tue, Dec 19, 2023 at 07:51:53PM -0500, Ethan Zhao wrote: >>> 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 pciehpt_ist() DLLSC >>> interrupt sequence response, pciehp will unload the device driver and >>> then power it off. thus cause an IOMMU devTLB flush request for device to >>> be sent and a long time completion/timeout waiting in interrupt context. >> I think the problem is in the "waiting in interrupt context". > I'm wondering whether Intel IOMMUs possibly have a (perhaps undocumented) > capability to reduce the Invalidate Completion Timeout to a sane value? > Could you check whether that's supported? It is not about Intel vt-d's capability per my understanding, it is the third party PCIe switch's capability, they are not aware of ATS transation at all, if its downstream port endpoint device is removed/powered-off/link-down, it couldn't feedback the upstream iommu a fault/completion/timeout for ATS transaction breakage reason. While the root port could (verified). > > Granted, the Implementation Note you've pointed to allows 1 sec + 50%, 1 min (60 sec)+50% > but that's not even a "must", it's a "should". So devices are free to I could happen if blindly wait here, so we should avoid such case. Thanks, Ethan > take even longer. We have to cut off at *some* point. > > Thanks, > > Lukas
On 12/21/2023 7:01 PM, Lukas Wunner wrote: > On Thu, Dec 21, 2023 at 11:39:40AM +0100, Lukas Wunner wrote: >> On Tue, Dec 19, 2023 at 07:51:53PM -0500, Ethan Zhao wrote: >>> 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 pciehpt_ist() DLLSC >>> interrupt sequence response, pciehp will unload the device driver and >>> then power it off. thus cause an IOMMU devTLB flush request for device to >>> be sent and a long time completion/timeout waiting in interrupt context. >> I think the problem is in the "waiting in interrupt context". > I'm wondering whether Intel IOMMUs possibly have a (perhaps undocumented) > capability to reduce the Invalidate Completion Timeout to a sane value? > Could you check whether that's supported? > > Granted, the Implementation Note you've pointed to allows 1 sec + 50%, > but that's not even a "must", it's a "should". So devices are free to > take even longer. We have to cut off at *some* point. I really "expected" there is interrrupt signal to iommu hardware when the PCIe swtich downstream device 'gone', or some internal polling /heartbeating the endpoint device for ATS breaking, but so far seems there are only hotplug interrupts to downstream control. ... How to define the point "some" msec to timeout while software break out the waiting loop ? or polling if the target is gone ? Thanks, Ethan > > Thanks, > > Lukas
On Fri, Dec 22, 2023 at 09:56:39AM +0800, Ethan Zhao wrote: > I don't know if the polling along sleeping for completion of meanningless > devTLB invalidation request blindly sent to (removed/powered down/link down) > device makes sense or not. If you have a way to get to the struct pci_dev * which you're waiting for in qi_submit_sync() then I guess you could check for its presence and bail out if it's gone, instead of issuing a cpu_relax(). > > Again, the proposed patch is not a proper solution. It will paper over > > the issue most of the time but every once in a while someone will still > > get a hard lockup splat and it will then be more difficult to reproduce > > and fix if the proposed patch is accepted. > > Could you point out why is not proper ? Is there any other window > the hard lockup still could happen with the ATS capable devcie > supprise_removal case if we checked the connection state first ? > Please help to elaberate it. Even though user space may have initiated orderly removal via sysfs, the device may be yanked from the slot (surprise removed) while the orderly removal is happening. > Yes, this is the old kernel stack trace, but customer also tried lasted > 6.7rc4 If you could provide a stacktrace for a contemporary kernel, I think that would be preferred. > (doesn't work) and the patched 6.7rc4 (fixed). Why is it fixed in v6.7-rc4? Is the present patch thus unnecessary? > > Finally, it is common to adhere to terms > > used in the PCIe Base Spec in commit messages, so "ATC Invalidate Request" > > might be preferable to "devTLB flush request". > > ATS Invalidate Request ? devTLB flush request has the same meaning, > > I thought all iommu/PCIe guys could understand. I'm just pointing out the preferred way to write commit messages in the PCI subsystem (as I've perceived it over the years) so that you can reduce the number of iterations you have to go through due to maintainer feedback. I'm just trying to be helpful. > How to define the point "some" msec to timeout while software > break out the waiting loop ? or polling if the target is gone ? I'd say adhere to the 1 min + 50% number provided in the spec. If you know the device is gone before that then you can break out of the loop in qi_submit_sync() of course. The question is, does the Intel IOMMU have a timeout at all for Invalidate Requests? I guess we don't really know that because in the stack trace you've provided, the watchdog stops the machine before a timeout occurs. So it's at least 12 sec. Or there's no timeout at all. If the Intel IOMMU doesn't enforce a timeout, you should probably amend qi_submit_sync() to break out of the loop once the 1 min + 50% limit is exceeded. And you need to amend the function to sleep instead of polling in interrupt context. Can you check with hardware engineers whether there's a timeout? Thanks, Lukas
On 12/22/2023 4:14 PM, Lukas Wunner wrote: > On Fri, Dec 22, 2023 at 09:56:39AM +0800, Ethan Zhao wrote: >> I don't know if the polling along sleeping for completion of meanningless >> devTLB invalidation request blindly sent to (removed/powered down/link down) >> device makes sense or not. > If you have a way to get to the struct pci_dev * which you're waiting for > in qi_submit_sync() then I guess you could check for its presence and bail > out if it's gone, instead of issuing a cpu_relax(). One option to bail out the loop. > >>> Again, the proposed patch is not a proper solution. It will paper over >>> the issue most of the time but every once in a while someone will still >>> get a hard lockup splat and it will then be more difficult to reproduce >>> and fix if the proposed patch is accepted. >> Could you point out why is not proper ? Is there any other window >> the hard lockup still could happen with the ATS capable devcie >> supprise_removal case if we checked the connection state first ? >> Please help to elaberate it. > Even though user space may have initiated orderly removal via sysfs, > the device may be yanked from the slot (surprise removed) while the > orderly removal is happening. Yes, just after the wait descripor is submitted and before waiting in loop. the rare but worst case. > > >> Yes, this is the old kernel stack trace, but customer also tried lasted >> 6.7rc4 > If you could provide a stacktrace for a contemporary kernel, > I think that would be preferred. Customer tried, but they didn't provide me the lastest trace. > > >> (doesn't work) and the patched 6.7rc4 (fixed). > Why is it fixed in v6.7-rc4? Is the present patch thus unnecessary? Not fixed in v6.7rc4, with this patch, they said the unplug works. > >>> Finally, it is common to adhere to terms >>> used in the PCIe Base Spec in commit messages, so "ATC Invalidate Request" >>> might be preferable to "devTLB flush request". >> ATS Invalidate Request ? devTLB flush request has the same meaning, >> >> I thought all iommu/PCIe guys could understand. > I'm just pointing out the preferred way to write commit messages > in the PCI subsystem (as I've perceived it over the years) so that > you can reduce the number of iterations you have to go through > due to maintainer feedback. I'm just trying to be helpful. > Understand. >> How to define the point "some" msec to timeout while software >> break out the waiting loop ? or polling if the target is gone ? > I'd say adhere to the 1 min + 50% number provided in the spec. > > If you know the device is gone before that then you can break out > of the loop in qi_submit_sync() of course. I am trying to find a way to break it out in this qi_submit_sync(). checking the device state in this loop, but seems not good in this iommu low level code and need some interfaces to be modified. That would cost me much more hours to make the rare case work, to be perfect: 1. check the pci device state in the loop 2. modify the invalidation descriptor status in pciehp_ist()->intel_iommu_release_device() call. > > The question is, does the Intel IOMMU have a timeout at all for > Invalidate Requests? I guess we don't really know that because > in the stack trace you've provided, the watchdog stops the machine > before a timeout occurs. So it's at least 12 sec. Or there's > no timeout at all. The calltrace wouldn't tell us there is really timeout of 1min+50% or not, event there is, meanlingless. > If the Intel IOMMU doesn't enforce a timeout, you should probably amend > qi_submit_sync() to break out of the loop once the 1 min + 50% limit > is exceeded. And you need to amend the function to sleep instead of > polling in interrupt context. Too many paths to call this function, and revise it to non-sync, to much things impacted. > > Can you check with hardware engineers whether there's a timeout? Combinated with third party PCIe switch chips ? Thanks, Ethan > > Thanks, > > Lukas >
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c index 74e8e4c17e81..7dbee9931eb6 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;