Re: [PATCH v4 08/14] iommu/amd: Consolidate feature detection and reporting logic

[Vasant Hegde <vasant.hegde@amd.com>]

Jason,

On 8/16/2023 12:01 AM, Jason Gunthorpe wrote:
> On Tue, Aug 15, 2023 at 10:21:56AM +0000, Vasant Hegde wrote:
> 
>> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
>> index 6bf55de9fbeb..242e25ba2f93 100644
>> --- a/drivers/iommu/amd/amd_iommu.h
>> +++ b/drivers/iommu/amd/amd_iommu.h
>> @@ -90,9 +90,19 @@ static inline bool is_rd890_iommu(struct pci_dev *pdev)
>>  	       (pdev->device == PCI_DEVICE_ID_RD890_IOMMU);
>>  }
>>  
>> -static inline bool iommu_feature(struct amd_iommu *iommu, u64 mask)
>> +static inline bool check_feature(u64 mask)
>>  {
>> -	return !!(iommu->features & mask);
>> +	return !!(amd_iommu_efr & mask);
>> +}
>> +
>> +static inline bool check_feature2(u64 mask)
>> +{
>> +	return !!(amd_iommu_efr2 & mask);
>> +}
> 
> You don't need the !! when returning a bool

Sure. Will fix it.

> 
>> @@ -2012,8 +2002,8 @@ static ssize_t amd_iommu_show_features(struct device *dev,
>>  				       struct device_attribute *attr,
>>  				       char *buf)
>>  {
>> -	struct amd_iommu *iommu = dev_to_amd_iommu(dev);
>> -	return sysfs_emit(buf, "%llx:%llx\n", iommu->features2, iommu->features);
>> +	return sysfs_emit(buf, "EFR:%llx, EFR2:%llx\n",
>> +			  amd_iommu_efr, amd_iommu_efr2);
>>  }
>>  static DEVICE_ATTR(features, S_IRUGO, amd_iommu_show_features, NULL);
> 
> Can't do this, sysfs is ABI (and it should have not have returned two
> values in the first place, Greg would reject it)

Correct. Its existing ABI. Will fix it.

-Vasant