[PATCH v3 13/13] iommu/arm-smmu-v3: Enable PRI for PCI device in arm_smmu_probe_device()
Nicolin Chen
nicolinc at nvidia.com
Fri Sep 4 22:53:27 PDT 2026
On Thu, Sep 03, 2026 at 12:18:33PM -0700, Jonathan Cameron wrote:
> > SMMUv3 forbids the Stall model on PCIe streams. Refuse to enable PRI on a
> > PCIe master that came with stall_enabled, so page_response() can dispatch
> > on master state unambiguously.
>
> As earlier, we have exceptions in tree for Stall mode on PCIe streams
> (lets not reopen that fun arguement) so I'd focus this on PRI not
> making any sense if stall mode is in use as that has alternative
> handling for page faults.
Replaced with:
A stalling master resolves its faults synchronously, so it has no reason at
all to issue a page request. Refuse to enable PRI on such a master, so that
page_response() can keep dispatching on the master state unambiguously.
> > + /* Same as the eventq drain above, for the hardware priq */
> > + if (master->pri_enabled) {
> > + ret |= arm_smmu_drain_queue(smmu, &smmu->priq.q, false);
> > + /* Ensure pending requests have reached the IOPF queue */
> > + if (!ret && smmu->priq.q.irq)
>
> Similar to before, I'd factor out the if (!ret)
> Using |= on return values is never particularly nice though safe
> as used here. Still I'd use another variable to store that things
> timed out already so we are skipping these.
OK. Replaced with a "bool timed_out" (in both PATCH-4 and here):
@@ void arm_smmu_attach_release(struct arm_smmu_attach_state *state)
struct arm_smmu_master_domain *master_domain = state->old_master_domain;
struct arm_smmu_master *master = state->master;
struct arm_smmu_device *smmu = master->smmu;
- int ret = 0;
+ bool timed_out = false;
lockdep_assert_not_held(&arm_smmu_asid_lock);
iommu_group_mutex_assert(master->dev);
@@ void arm_smmu_attach_release(struct arm_smmu_attach_state *state)
* against the next domain.
*/
if (master->stall_enabled) {
- ret = arm_smmu_wait_for_queue_drained(smmu, &smmu->evtq.q, false);
+ timed_out = arm_smmu_wait_for_queue_drained(smmu, &smmu->evtq.q,
+ false);
/*
* Ensure pending events have reached the IOPF queue, unless
* the drain timed out: a stuck consumer would also block an
* unbounded wait_event() inside the synchronize_irq().
*/
- if (!ret) {
+ if (!timed_out) {
if (smmu->evtq.q.irq)
synchronize_irq(smmu->evtq.q.irq);
/* Pending events might be in the combined_irq handler */
@@ void arm_smmu_attach_release(struct arm_smmu_attach_state *state)
/* Same as the eventq drain above, for the hardware priq */
if (master->pri_enabled) {
- ret |= arm_smmu_wait_for_queue_drained(smmu, &smmu->priq.q, false);
+ timed_out |= arm_smmu_wait_for_queue_drained(
+ smmu, &smmu->priq.q, false);
/* Ensure pending requests have reached the IOPF queue */
- if (!ret) {
+ if (!timed_out) {
if (smmu->priq.q.irq)
synchronize_irq(smmu->priq.q.irq);
/* Pending requests might be in the combined_irq handler */
@@ void arm_smmu_attach_release(struct arm_smmu_attach_state *state)
* such work still references. Skip the iopf teardown and
* leak master_domain, rather than risk a UAF.
*/
- if (WARN_ON(ret)) {
+ if (WARN_ON(timed_out)) {
state->old_master_domain = NULL;
return;
}
> > + if (!pci_pri_supported(pdev))
> > + return;
> > +
> > + /* SMMUv3 forbids the Stall model on PCIe streams */
>
> Again, I'd tweak the wording given we have quite a few examples
> in tree that do stall mode on PCIe smelling streams.
Replaced with:
/* A stalling master resolves its faults without any page request */
Thanks
Nicolin
More information about the linux-arm-kernel
mailing list