Re: [PATCH v1 15/16] iommu/arm-smmu-v3: Add viommu cache invalidation support

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Aug 07, 2024 at 01:10:56PM -0700, Nicolin Chen wrote:
> Add an arm_smmu_viommu_cache_invalidate() function for user space to issue
> cache invalidation commands via viommu.
> 
> The viommu invalidation takes the same native format of a 128-bit command,
> as the hwpt invalidation. Thus, reuse the same driver data structure, but
> make it wider to accept CMDQ_OP_ATC_INV and CMDQ_OP_CFGI_CD{_ALL}.
> 
> Scan the commands against the supported ist and fix the VMIDs and SIDs.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 54 +++++++++++++++++++--
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
>  include/uapi/linux/iommufd.h                | 20 ++++++++
>  3 files changed, 70 insertions(+), 5 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 ec76377d505c..be4f849f1a48 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -3219,15 +3219,32 @@ static void arm_smmu_domain_nested_free(struct iommu_domain *domain)
>  	kfree(container_of(domain, struct arm_smmu_nested_domain, domain));
>  }
>  
> +static int arm_smmu_convert_viommu_vdev_id(struct iommufd_viommu *viommu,
> +					   u32 vdev_id, u32 *sid)
> +{
> +	struct arm_smmu_master *master;
> +	struct device *dev;
> +
> +	dev = iommufd_viommu_find_device(viommu, vdev_id);
> +	if (!dev)
> +		return -EIO;
> +	master = dev_iommu_priv_get(dev);
> +
> +	if (sid)
> +		*sid = master->streams[0].id;

See this is the thing that needs to be locked right

xa_lock()
dev = xa_load()
master = dev_iommu_priv_get(dev);
*sid =  master->streams[0].id;
xa_unlock()

Then no risk of dev going away under us.

> @@ -3249,6 +3266,19 @@ arm_smmu_convert_user_cmd(struct arm_smmu_domain *s2_parent,
>  		cmd->cmd[0] &= ~CMDQ_TLBI_0_VMID;
>  		cmd->cmd[0] |= FIELD_PREP(CMDQ_TLBI_0_VMID, vmid);
>  		break;
> +	case CMDQ_OP_ATC_INV:
> +	case CMDQ_OP_CFGI_CD:
> +	case CMDQ_OP_CFGI_CD_ALL:

Oh, I didn't catch on that CD was needing this too.. :\

That makes the other op much more useless than I expected. I really
wanted to break these two series apart.

Maybe we need to drop the hwpt invalidation from the other series and
aim to merge this all together through the iommufd tree.

Jason