Re: [PATCH v4 03/16] iommu/amd: Introduce get_amd_iommu_from_dev()

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Dec 12, 2023 at 08:52:11AM +0000, Vasant Hegde wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> And replace rlookup_amd_iommu() with the new helper function where
> applicable to avoid unnecessary loop to look up struct amd_iommu from
> struct device.
> 
> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu.h | 11 +++++++++
>  drivers/iommu/amd/iommu.c     | 43 ++++++++++-------------------------
>  include/linux/iommu.h         | 13 +++++++++++
>  3 files changed, 36 insertions(+), 31 deletions(-)

This is basically fine

But can you improve the core helper a bit so everyone else can easially use it too:

static inline struct iommu_device *__iommu_get_iommu_dev(struct device *dev)
{
	return dev->iommu->iommu_dev;
}

#define iommu_get_iommu_dev(dev, type, member) \
	container_of(__iommu_get_iommu_dev(dev), type, member)

static inline struct amd_iommu *get_amd_iommu_from_dev(struct device *dev)
{
	return iommu_get_iommu_dev(dev, struct amd_iommu, iommu);
}

static inline struct amd_iommu *get_amd_iommu_from_dev_data(struct iommu_dev_data *dev_data)
{
	return iommu_get_iommu_dev(dev_data->dev, struct amd_iommu, iommu);
}

> @@ -1813,9 +1809,7 @@ static void do_attach(struct iommu_dev_data *dev_data,
>  {
>  	struct amd_iommu *iommu;
>  
> -	iommu = rlookup_amd_iommu(dev_data->dev);
> -	if (!iommu)
> -		return;
> +	iommu = get_amd_iommu_from_dev(dev_data->dev);

And I suggest to stylistically keep with the common pattern of
initializing the casts at the top of the function

	     struct amd_iommu *iommu = get_amd_iommu_from_dev(dev_data->dev);
	     struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);

Jason