Re: [PATCH 3/9] iommu/arm-smmu-v3: Use the HW arm_smmu_cmd in cmdq submission functions

[Pranjal Shrivastava <praan@google.com>]

On Fri, May 08, 2026 at 01:00:41PM -0300, Jason Gunthorpe wrote:
> On Fri, May 08, 2026 at 08:27:26AM +0000, Pranjal Shrivastava wrote:
> > >  /* Should be installed after arm_smmu_install_ste_for_dev() */
> > > @@ -4823,7 +4826,8 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
> > >  {
> > >  	int ret;
> > >  	u32 reg, enables;
> > > -	struct arm_smmu_cmdq_ent cmd;
> > > +	struct arm_smmu_cmdq_ent ent;
> > 
> > This shouldn't be uninitialized, we only seem to be setting ent.opcode
> > later in the function. 
> 
> Yes, that's how the existing code is.
> 
>         struct arm_smmu_cmdq_ent cmd;
> 
>         cmd.opcode = CMDQ_OP_CFGI_ALL;
>         arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
> 
>                 cmd.opcode = CMDQ_OP_TLBI_EL2_ALL;
>                 arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
> 
>         cmd.opcode = CMDQ_OP_TLBI_NSNH_ALL;
>         arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
> 
> > Since arm_smmu_cmdq_build_cmd reads other fields
> > of ent to build the cmd, we are potentially sending stack garbage in ent
> 
> Ah, it is tricky, it doesn't:
> 
> static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
> {
>         memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);
>         cmd[0] |= FIELD_PREP(CMDQ_0_OP, ent->opcode);
> 
>         switch (ent->opcode) {

Right!

>         case CMDQ_OP_TLBI_EL2_ALL:
>         case CMDQ_OP_TLBI_NSNH_ALL:
>                 break;
> [..]
>         case CMDQ_OP_CFGI_ALL:
>                 /* Cover the entire SID range */
>                 cmd[1] |= FIELD_PREP(CMDQ_CFGI_1_RANGE, 31);
>                 break;
> 
> Only opcode is used, so it's "fine"
> 
> Later patches remove ent and this trickyness so let's just leave it:

Ack. Sure. No strong feelings here. 

Praan