On Tue, May 14, 2024 at 12:15:13PM -0300, Jason Gunthorpe wrote: > 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. Yea. I just added this: @@ -333,10 +333,22 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent) return 0; } -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) { + /* + * TEGRA241 CMDQV has two modes to execute commands: host and guest. + * The host mode supports all the opcodes, while the guest mode only + * supports a few invalidation ones (check tegra241_vintf_support_cmd) + * and also a CMD_SYNC added by arm_smmu_cmdq_issue_cmdlist(..., true). + * + * Here pass in the representing opcode for either a single command or + * an arm_smmu_cmdq_batch, assuming that this SMMU driver will only add + * same type of commands into a batch as it does today or it will only + * mix supported invalidation commands in a batch. + */ if (arm_smmu_has_tegra241_cmdqv(smmu)) - return tegra241_cmdqv_get_cmdq(smmu); + return tegra241_cmdqv_get_cmdq(smmu, opcode); return &smmu->cmdq; } Thanks Nicolin