Re: [PATCH v3 1/3] iommu/arm-smmu-v3: Factor out CMDQ batch force-sync conditions

[Will Deacon]

On Tue, Jun 02, 2026 at 01:08:26PM -0700, Nicolin Chen wrote:
> On Tue, Jun 02, 2026 at 08:55:10PM +0100, Will Deacon wrote:
> > On Mon, Jun 01, 2026 at 10:48:43AM +0000, Ashish Mhetre wrote:
> > > From: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > > 
> > > arm_smmu_cmdq_batch_add_cmd_p() carries two distinct reasons for
> > > flushing the current batch with a CMD_SYNC before appending the
> > > new command:
> > > 
> > >   - The batch's pre-assigned cmdq does not support the new command.
> > >   - The Arm erratum 2812531 workaround (ARM_SMMU_OPT_CMDQ_FORCE_SYNC)
> > >     forces a SYNC at one entry before the batch is full.
> > > 
> > > Lift those checks into a new arm_smmu_cmdq_batch_force_sync() helper
> > > so that adding another force-sync condition becomes a one-line
> > > addition. No functional change.
> > > 
> > > Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > > Signed-off-by: Ashish Mhetre <amhetre@xxxxxxxxxx>
> > > ---
> > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 26 ++++++++++++++++-----
> > >  1 file changed, 20 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > index 9be589d14a3b..4d29bd343460 100644
> > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > @@ -847,16 +847,30 @@ static void arm_smmu_cmdq_batch_init_cmd(struct arm_smmu_device *smmu,
> > >  	cmds->cmdq = arm_smmu_get_cmdq(smmu, cmd);
> > >  }
> > >  
> > > +static bool arm_smmu_cmdq_batch_force_sync(struct arm_smmu_device *smmu,
> > > +					   struct arm_smmu_cmdq_batch *cmds,
> > > +					   struct arm_smmu_cmd *cmd)
> > > +{
> > > +	if (!cmds->num)
> > > +		return false;
> > 
> > This check seems new?
> 
> You are right. Maybe the commit message should have mentioned that
> there is a slight behavior change to the unsupported_cmd path:
> 
>  - Before: if cmds->num = 0 && unsupported_cmd, it would issue an
>            empty batch (one CMD_SYNC)
>  - After : if cmds->num = 0, no issue on the empty batch
> 
> With that, I think it's good to have?

It just shouldn't be part of the refactoring patch with "no functional
change". Do it in the patch that needs it and explain why it's needed.

Will