Re: [RFC PATCH 2/5] iommu/arm-smmu-v3: Add a helper to wait till cmdq drains

[Pranjal Shrivastava <praan@google.com>]

On Tue, Mar 25, 2025 at 09:51:52PM -0700, Daniel Mentz wrote:
> On Tue, Mar 18, 2025 at 5:43 PM Pranjal Shrivastava <praan@google.com> wrote:
> > +/*
> > + * Wait until the SMMU cmdq is empty
> > + * Must be called with the cmdq lock held in some capacity.
> > + */
> > +static int arm_smmu_cmdq_poll_until_empty(struct arm_smmu_device *smmu,
> > +                                         struct arm_smmu_cmdq *cmdq,
> > +                                         struct arm_smmu_ll_queue *llq)
> > +{
> > +       struct arm_smmu_queue_poll qp;
> > +       int ret = 0;
> > +
> > +       queue_poll_init(smmu, &qp);
> > +       llq->val = READ_ONCE(cmdq->q.llq.val);
> 
> In a later patch, I believe you're calling this function with llq
> pointing to cmdq->q.llq in which case this assignment would be
> redundant.
> 

Ack. Will fix it.

> As an alternative approach, I'm wondering if you can reuse the
> function __arm_smmu_cmdq_poll_until_consumed. Just take the current
> producer index and wait for it to get consumed.
>

Actually, that could cause some trouble because the function,
__arm_smmu_cmdq_poll_until_consumed, waits till the cons __crosses__ the
prod by relying on `queue_consumed`. This could cause trouble as, when
the queue is empty, the cons won't cross prod and hence we'll keep
waiting in suspend... 

Instead, the `arm_smmu_cmdq_poll_until_empty` relies on `queue_empty` to
confirm that the queue has been drained before suspending..

>
> 
> > +       do {
> > +               if (queue_empty(llq))
> > +                       break;
> > +
> > +               ret = queue_poll(&qp);
> > +               llq->cons = readl(cmdq->q.cons_reg);
> > +       } while (!ret);
> > +
> > +       return ret;
> > +}
> > +
> >  static void arm_smmu_cmdq_write_entries(struct arm_smmu_cmdq *cmdq, u64 *cmds,
> >                                         u32 prod, int n)
> >  {
> > --
> > 2.49.0.rc1.451.g8f38331e32-goog
> >

Thanks,
Praan