Re: [PATCH 01/11] iommu/amd: Rename helper function rlookup_amd_iommu()

["Suthikulpanit, Suravee" <suravee.suthikulpanit@amd.com>]

On 8/8/2023 8:28 AM, Jason Gunthorpe wrote:
> On Tue, Aug 08, 2023 at 10:02:22AM +0000, Vasant Hegde wrote:
>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>
>> Rename and make it available to other code.
>>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>>   drivers/iommu/amd/amd_iommu.h |  1 +
>>   drivers/iommu/amd/iommu.c     | 30 +++++++++++++++---------------
>>   2 files changed, 16 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
>> index 56996dc2868d..1471ab2dddd1 100644
>> --- a/drivers/iommu/amd/amd_iommu.h
>> +++ b/drivers/iommu/amd/amd_iommu.h
>> @@ -24,6 +24,7 @@ int amd_iommu_init_devices(void);
>>   void amd_iommu_uninit_devices(void);
>>   void amd_iommu_init_notifier(void);
>>   void amd_iommu_set_rlookup_table(struct amd_iommu *iommu, u16 devid);
>> +struct amd_iommu *amd_iommu_rlookup_iommu(struct device *dev);
>>   
>>   #ifdef CONFIG_AMD_IOMMU_DEBUGFS
>>   void amd_iommu_debugfs_setup(struct amd_iommu *iommu);
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index 26e509ffe77a..77b913211c35 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -164,7 +164,7 @@ static struct amd_iommu *__rlookup_amd_iommu(u16 seg, u16 devid)
>>   	return NULL;
>>   }
>>   
>> -static struct amd_iommu *rlookup_amd_iommu(struct device *dev)
>> +struct amd_iommu *amd_iommu_rlookup_iommu(struct device *dev)
>>   {
> 
> Er, wah? This is a really confused function. It should just be:
> 
> static inline struct amd_iommu *amd_device_to_iommu(struct device *dev)
> {
> 	return container_of(dev->iommu->iommu_dev, struct amd_iommu, iommu);
> }
> 
> Right? At least for many of the call sites.
> 
> The core code maintains the association of the iommu_device with each
> probe'd device automatically. The drivers don't need tables and
> searching.

The rlookup table is setup from the information in the IVRS table, which 
contains list of devices belong to an IOMMU instance. This information 
is used to for setting dev->iommu during probing (See 
drivers/iommu/amd/iommu.c: amd_iommu_probe_device()).

Subsequently, it should be able to use the amd_device_to_iommu() you 
provided as long as the device has not been released.

So, we can add the helper function above, and make sure to check that 
dev->iommu is not NULL before calling the container_of().

Thanks,
Suravee