On Tue, May 07, 2024 at 10:56:54PM -0700, Nicolin Chen wrote: > When VCMDQs are assigned to a VINTF owned by a guest (HYP_OWN bit unset), > only TLB and ATC invalidation commands are supported by the VCMDQ HW. So, > add a new helper to scan the input cmd to make sure it is supported when > selecting a queue. > > Note that the guest VM shouldn't have HYP_OWN bit being set regardless of > guest kernel driver writing it or not, i.e. the hypervisor running in the > host OS should wire this bit to zero when trapping a write access to this > VINTF_CONFIG register from a guest kernel. > > Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 20 ++++++----- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 +-- > .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 36 ++++++++++++++++++- > 3 files changed, 49 insertions(+), 12 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 d1098991d64e..baf20e9976d3 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -332,10 +332,11 @@ 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) > { > 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. opcode is sort of a handle for a group of related commands. But what is the group? Minimally it is opcode + SYNC, right? The caller must only send opcode + SYNC commands to this queue. The opcodes XX,YY,ZZ are interchangable and can be sent together. ? > -struct arm_smmu_cmdq *tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu) > +static bool tegra241_vintf_support_cmd(struct tegra241_vintf *vintf, u8 opcode) > +{ > + /* Hypervisor-owned VINTF can execute any command in its VCMDQs */ > + if (READ_ONCE(vintf->hyp_own)) > + return true; > + > + /* Guest-owned VINTF must Check against the list of supported CMDs */ > + switch (opcode) { > + case CMDQ_OP_TLBI_NH_ASID: > + case CMDQ_OP_TLBI_NH_VA: > + case CMDQ_OP_ATC_INV: > + return true; > + default: > + return false; > + } When I look at the nesting patch it also includes SYNC, NH_VAA, and NH_ALL. Are they supported here? VAA is not supported in the HW at all right? What about NH_ALL? It looks fine otherwise Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx> Jason