Re: [PATCH v9 09/12] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops

[Jason Gunthorpe <jgg@nvidia.com>]

> [ ... 80 lines skipped ... ]
> @@ -730,10 +770,58 @@ int __arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
>  
>  		/*
>  		 * If the SMMU is suspended/suspending, any new CMDs are elided.
> -		 * This loop is the Point of Commitment. If we haven't cmpxchg'd
> -		 * our new indices yet, we can safely bail. Once the indices are
> -		 * committed, we MUST write valid commands to those slots to
> -		 * avoid indefinite polling in the drain function.
> +		 *
> +		 * This loop acts as the Point of Commitment.
> +		 * The CMDQ_PROD_STOP_FLAG ensures that no new commands are
> +		 * committed once the SMMU begins to suspend. The synchronization
> +		 * relies on the following observability invariants:
> +		 *
> +		 * 1. Other CPUs observe the STOP_FLAG only *after* the SMMU is
> +		 *    disabled. This is enforced in arm_smmu_runtime_suspend()

 [Severity: Critical]
 If an ATC invalidation (CMDQ_OP_ATC_INV) is issued (e.g., during iommu_unmap
 from a background thread) while the SMMU is suspended, the command appears
 to be silently dropped here.

 Since ATC caches inside PCIe endpoints might not be globally invalidated
 during resume, could the endpoint retain stale ATC entries upon wake-up?
 Would this allow the endpoint to DMA into freed memory?

I agree.. I think there are only two options?
 1) After GBPA=Abort ATS requests are blocked, so you could full
    invalidate all the device ATC's and now it is safe to ignore
    ATC_INV
 2) Just don't perform suspend once ATS is activated

-- 
Jason