Re: [PATCH v7 6/6] iommu/tegra241-cmdqv: Limit CMDs for guest owned VINTF

[Jason Gunthorpe]

On Sun, May 12, 2024 at 03:09:25PM -0700, Nicolin Chen wrote:
> > > -static struct arm_smmu_cmdq *arm_smmu_get_cmdq(struct arm_smmu_device *smmu)
> > > +static struct arm_smmu_cmdq *
> > > +arm_smmu_get_cmdq(struct arm_smmu_device *smmu, u8 opcode)
> > >  {
> > >  	if (arm_smmu_has_tegra241_cmdqv(smmu))
> > > -		return tegra241_cmdqv_get_cmdq(smmu);
> > > +		return tegra241_cmdqv_get_cmdq(smmu, opcode);
> > 
> > It is worth a comment descrbing opcode here, I think.. At least the
> > nesting invalidation will send mixed batches.
> 
> Right, this makes the "opcode" look bad, though we know that the
> "opcode" in the nesting invalidation doesn't matter because VCMDQ
> in that case supports all commands with HYP_OWN=1.

Yeah, it isn't a real problem, it just looks a little messy and
should have a small comment someplace at least..
 
> A CMD_SYNC, on the other hand, is outside the batch struct. And
> here is another assumption that CMD_SYNC is always supported by a
> VCMDQ..
> 
> I could clarify the "opcode" here with these assumptions. Or maybe
> we should think think of a better alternative?

I don't think it really needs to be more complex, but we should
document that invalidation is going to be special and doesn't quite
follow this rule.

Jason