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
