Re: [PATCH v3 02/12] iommu/amd: Do not override PASID entry in GCR3 table

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Nov 07, 2023 at 11:56:26AM +0530, Vasant Hegde wrote:

> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 691c040216e3..558e0c196172 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -1879,6 +1879,9 @@ static int __set_gcr3(struct iommu_dev_data *dev_data,
>  	if (pte == NULL)
>  		return -ENOMEM;
> 
> +	if (*pte & GCR3_VALID)
> +		return -EBUSY;
> +
>  	*pte = (gcr3 & PAGE_MASK) | GCR3_VALID;
>  	amd_iommu_dev_flush_pasid_all(dev_data, pasid);
> 
> 
> 
> I believe as a sane design it should first remove existing mapping (that's why
> we have remove_dev_pasid() interface) before adding new mapping. That's why I
> introduced this patch.

Again, no, all these ops are replace. The ideal driver should simply
write the new GCR3 entry with the new value in a single step. No
detach/attach cycle.

Jason