Repository navigation
[26.04_linux-nvidia] fix CMDQV teardown on unbind, probe failure, and shutdown/kexec - #689
Closed
NathanChenNVIDIA wants to merge 3 commits into
Closed
NathanChenNVIDIA wants to merge 3 commits into
NathanChenNVIDIA wants to merge 3 commits into
Conversation
arm_smmu_device_remove() manually frees the IOPF queue, destroys the vmid_map and disables the device, while the IRQs and queues are devm managed. devm unwinds only after remove() returns, so the cleanup runs in the wrong order. The IOPF queue is freed before the event-queue IRQ whose handler uses it. Manage all of it with devm so the unwind order is correct. Free the IOPF queue and vmid_map via devm actions, and disable the device from one registered after arm_smmu_device_reset(). This is also a prerequisite for fixing a Tegra241 CMDQV CMD_SYNC use-after-free in the subsequent patch. Cc: stable@vger.kernel.org Suggested-by: Jason Gunthorpe <jgg@ziepe.ca> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Will Deacon <will@kernel.org> (cherry picked from commit 2bd22a0) Signed-off-by: Nathan Chen <nathanc@nvidia.com>
arm_smmu_impl_remove() is registered as a devres action in arm_smmu_impl_probe(), before arm_smmu_init_queues() allocates smmu->cmdq.q.base. On a devres unwind, whether a failed probe or an unbind, the queue is freed first and arm_smmu_impl_remove() then runs tegra241_cmdqv_remove_vintf(), whose VINTF deinit issues a CMD_SYNC on the freed memory. Observed during testing with a QEMU hack that makes the VCMDQ fail to enable, so the impl reset fails and probe aborts into the devres unwind: platform NVDA200C:00: tegra241_cmdqv: VINTF0: VCMDQ0/LVCMDQ0: failed to enable, STATUS=0x00000000 platform NVDA200C:00: tegra241_cmdqv: VINTF0: VCMDQ0/LVCMDQ0: GERRORN=0x0, GERROR=0x4, CONS=0x0 platform NVDA200C:00: tegra241_cmdqv: VINTF0: VCMDQ0/LVCMDQ0: uncleared error detected, resetting arm-smmu-v3 arm-smmu-v3.0.auto: failed to reset impl arm-smmu-v3 arm-smmu-v3.0.auto: probe with driver arm-smmu-v3 failed with error -110 Unable to handle kernel paging request at virtual address ffff8000891e0098 ... Internal error: Oops: 0000000096000047 [NVIDIA#1] SMP ... Call trace: arm_smmu_cmdq_issue_cmdlist+0x320/0x6fc (P) tegra241_vcmdq_hw_deinit+0x98/0x168 tegra241_vintf_hw_deinit+0x5c/0x1b0 tegra241_cmdqv_remove_vintf+0x34/0xec tegra241_cmdqv_remove+0x40/0x9c arm_smmu_impl_remove+0x20/0x30 devm_action_release+0x14/0x20 devres_release_all+0xa8/0x110 device_unbind_cleanup+0x18/0x84 really_probe+0x1f0/0x29c Drop the VINTF deinit from tegra241_cmdqv_remove_vintf() so the unwind no longer touches the freed queue. Quiesce the VINTFs earlier instead. Add a device_disable() impl op and run it from arm_smmu_disable_action() while the CMDQ is still up. That handles a live unbind. A failed reset is already handled because tegra241_vintf_hw_init() deinits the VINTF on its own error path. tegra241_cmdqv_remove_vintf() is also used by the iommufd viommu destroy path, so quiesce there too. Fixes: 4dc0d12 ("iommu/tegra241-cmdqv: Add user-space use support") Cc: stable@vger.kernel.org Reviewed-by: Nicolin Chen <nicolinc@nvidia.com> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Will Deacon <will@kernel.org> (cherry picked from commit 9ff145a) Signed-off-by: Nathan Chen <nathanc@nvidia.com>
…down arm_smmu_device_shutdown() only calls arm_smmu_device_disable(), and never the impl_ops->device_disable() hook. On tegra241-cmdqv, that hook quiesces every VINTF through tegra241_vintf_hw_deinit(), which issues a CMD_SYNC on the shared CMDQ while it is still enabled. arm_smmu_disable_action() already runs this hook before disabling the SMMU, to cover an unbind or a probe failure, and no devres release ever follows a reboot or a kexec to run it again. Have arm_smmu_device_shutdown() call it directly, instead of reimplementing part of what it already does. Fixes: 9ff145a ("iommu/tegra241-cmdqv: Fix CMD_SYNC use-after-free on teardown") Cc: stable@vger.kernel.org Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com> Assisted-by: LLM Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Will Deacon <will@kernel.org> (cherry picked from commit ab0dbf4) Signed-off-by: Nathan Chen <nathanc@nvidia.com>
Collaborator
BaseOS Kernel ReviewCaution
|
Contributor
PR Validation ReportPatchscan ✅ No Missing FixesAll cherry-picked commits checked — no missing upstream fixes found. PR Lint
|
Collaborator
|
Codex found a potential non-blocking race in the new shutdown path:
|
Collaborator
|
I saw the same as Jamie. Will ack too. |
nirmoy
added
has_2_acks
pending_review_comment
and removed
help wanted
Collaborator
|
LGTM!
|
Collaborator
|
Applied to Canonical
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR backports three upstream fixes for tegra241-cmdqv teardown. They move SMMU teardown to devm, stop VINTF deinit from issuing a CMD_SYNC on a freed command queue during probe-failure unwind and vIOMMU destroy, and quiesce the VINTFs on shutdown and kexec.
Commits
7d8c870b84bbiommu/arm-smmu-v3: Manage teardown with devm (upstream2bd22a0d4050)3f303b85dd4diommu/tegra241-cmdqv: Fix CMD_SYNC use-after-free on teardown (upstream9ff145a25c5c)a19853cb9e2fiommu/arm-smmu-v3: Disable the impl before disabling the SMMU on shutdown (upstreamab0dbf411ace)Testing
Tested with the equivalent backport on the 26.04_linux-nvidia-bos tree on a Vera system with CMDQV enabled.
iommu.passthrough=0) modes.arm-smmuv3,accel=on,cmdqv=onand GPU passthrough. After both a clean poweroff and akill -9, the host showed no errors and the VM restarted cleanly.probe ... failed with error -110and no oops.LP: https://bugs.launchpad.net/ubuntu/+source/linux-nvidia/+bug/2173553