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

[Pranjal Shrivastava <praan@google.com>]

On Sun, Jun 07, 2026 at 02:53:24PM -0700, Daniel Mentz wrote:
> On Mon, Jun 1, 2026 at 2:59 PM Pranjal Shrivastava <praan@google.com> wrote:
> > @@ -4898,6 +4939,21 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
> >         if (is_kdump_kernel())
> >                 enables &= ~(CR0_EVTQEN | CR0_PRIQEN);
> >
> > +       /*
> > +        * While the SMMU was suspended, concurrent CPU threads may have
> > +        * updated in-memory structures (such as STEs, CDs, and PTEs).
> > +        * Any invalidations corresponding to those updates were safely
> > +        * elided because the command queue was stopped (STOP_FLAG == 1).
> > +        *
> > +        * Since the reset invalidate-all commands above have fully cleared
> > +        * the HW TLBs and config caches, the SMMU will fetch these descriptors
> > +        * directly from RAM as soon as translation is enabled.
> > +        *
> > +        * Add a memory barrier to collect all prior RAM writes to ensure the
> > +        * SMMU sees a consistent view of memory before translation is enabled.
> > +        */
> > +       smp_mb();
> 
> I'm not convinced that this is necessary. I understand that the write
> to smmu->cmdq.q.llq.atomic.prod needs to be ordered before setting
> CR0_SMMUEN in ARM_SMMU_CR0. However, this ordering requirement appears
> to already be met by the dma_wmb() in arm_smmu_cmdq_issue_cmdlist.
> Could you provide an example of a scenario that might fail if this
> smp_mb() were removed?

Agreed. The first dma_wmb() in the issue_cmdlist will handle this. We
don't need this smp_mb(); I'll add a note as specified in [1]

Thanks,
Praan

[1] https://lore.kernel.org/all/aiflaI4svEJvZbsC@google.com/