Re: [PATCH v9 02/12] iommu/arm-smmu-v3: Add a helper to drain cmd queues
From: Pranjal Shrivastava <praan@google.com>
Date: 2026-08-25 17:38:04
Also in:
linux-iommu
On Tue, Aug 25, 2026 at 01:36:11PM -0300, Jason Gunthorpe wrote:
quoted
[ ... 36 lines skipped ... ] + if (queue_empty(llq))[Severity: High] Does polling for an empty CMDQ actually guarantee that commands have completed? queue_empty() relies on the SMMU consumer register equaling the producer pointer. However, for CMD_SYNC, the SMMU advances the consumer register immediately upon consumption, not upon completion. Suspending the SMMU while commands like ATC_INV are still in-flight could abort them, leaving stale translations in the device ATC and causing submitters to time out. This seems broadly wrong, but it doesn't seem explained very well, maybe put a comment above the function something like The caller must have ensured that no concurrent command submission can happen. Thus the entry at the PROD must be a CMD_SYNC due to how all submitters generate commands. Waiting for PROD==CONS guarentees completion of the sync and all prior work and thus idle HW. I don't think the other substantive remarks are valid. The Werror thing is right, every patch should compile alone without warnings. Sometimes people add __maybe_unused to accomplish this..
Ack. I plan to add __maybe_unused here.
quoted
[ ... 19 lines skipped ... ][Severity: High] Does this logic miss commands routed to secondary queues? The driver supports routing commands, specifically ATC_INV, to secondary queues (like the Tegra241 CMDQV virtual queues) via get_secondary_cmdq(). By strictly hardcoding the poll to smmu->cmdq.q, we ignore secondary queues. If the SMMU suspends while ATC_INV commands are processing there, they could be dropped. Yes, I think the ordering is wrong, to keep them as different patches iommu/tegra241-cmdqv: Add a helper to drain VCMDQs Should come first, adding the op callback, then this patch would have the hunk completing the function so the newly introduced function works completely.
Ack, I'll re-order the patches.
Otherwise the approach looks OK to me Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Thanks, Praan