Re: [PATCH v7 08/11] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops

[Pranjal Shrivastava <praan@google.com>]

On Thu, May 28, 2026 at 03:13:48PM -0700, Nicolin Chen wrote:
> On Thu, May 28, 2026 at 09:21:22PM +0000, Pranjal Shrivastava wrote:
> > On Thu, May 28, 2026 at 12:39:46PM -0700, Nicolin Chen wrote:
> > > On Wed, May 27, 2026 at 10:14:04PM +0000, Pranjal Shrivastava wrote:
> > > > @@ -789,7 +822,8 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
> > > >  		/* b. Stop gathering work by clearing the owned flag */
> > > >  		prod = atomic_fetch_andnot_relaxed(CMDQ_PROD_OWNED_FLAG,
> > > >  						   &cmdq->q.llq.atomic.prod);
> > > > -		prod &= ~CMDQ_PROD_OWNED_FLAG;
> > > > +		/* Strip all metadata flags */
> > > > +		prod &= CMDQ_PROD_IDX_MASK;
> > > 
> > > Should its prior atomic_fetch_andnot_relaxed() call do something
> > > about the CMDQ_PROD_STOP_FLAG as well?
> > 
> > Umm.. No, the atomic_fetch_andnot_relaxed() call must leave the STOP_FLAG.
> > This block is the owner-publish phase, which occurs *after* the Point of
> > Commitment. If a submission successfully reserved its indices before the
> > gate closed, it shall be allowed to finish.
> > 
> > If the owner thread cleared the STOP_FLAG here in the global memory, it
> > would prematurely re-open the gate, allowing new racing submissions to 
> > leak in during the suspend sequence. 
> > 
> > The algorithm works mainly because the RPM callbacks are the only ones
> > that are allowed to manipulate this flag.
> 
> That reads plausible to me. It would be nicer to leave a brief note
> inline.

Ack.

> 
> > > Should it handle a secondary_cmdq?
> > 
> > No, I don't think we need to check secondary queues here. The STOP_FLAG
> > being set on the primary CMDQ during the suspend should suffice to 
> > indicate the entire SMMU's power state. That's why the function takes in
> > the smmu ptr and not the cmdq.
> 
> But if we don't set STOP flag to the secondary_cmdq, wouldn't the
> drain() timeout on the secondary_cmdq that does not elide?

I believe you're right..

I was banking on the fact that the ellision would work based on the
primary queue alone as we check the call-sites *before* the cmdq is
chosen. However, I see the need for setting the STOP_FLAG to secondary
CMDQs too (especially when we remove the check from the call-sites).

I'll add setting the FLAG on secondary queues too!

Thanks,
Praan