Re: [PATCH v3 18/19] iommu/amd: Print access/dirty bits if supported

Joao Martins
2023-10-18T08:54:43Z

On 18/10/2023 09:32, Vasant Hegde wrote:
> Joao,
>
> On 9/23/2023 6:55 AM, Joao Martins wrote:
>> Print the feature, much like other kernel-supported features.
>>
>> One can still probe its actual hw support via sysfs, regardless
>> of what the kernel does.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> drivers/iommu/amd/init.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
>> index 45efb7e5d725..b091a3d10819 100644
>> --- a/drivers/iommu/amd/init.c
>> +++ b/drivers/iommu/amd/init.c
>> @@ -2208,6 +2208,10 @@ static void print_iommu_info(void)
>>
>> if (iommu->features & FEATURE_GAM_VAPIC)
>> pr_cont(" GA_vAPIC");
>> + if (iommu->features & FEATURE_HASUP)
>> + pr_cont(" HASup");
>> + if (iommu->features & FEATURE_HDSUP)
>> + pr_cont(" HDSup");
>
> Note that this has a conflict with iommu/next branch. But it should be fairly
> straight to fix it. Otherwise patch looks good to me.
>
I guess it's this patch, thanks for reminding:

https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/commit/drivers/iommu/amd/init.c?h=next&id=7b7563a93437ef945c829538da28f0095f1603ec

But then it's the same problem as the previous patch. The loop above is enterily
reworked, so the code above won't work, and the "iommu->features &" conditionals
needs to be replaced with a check_feature(FEATURE_HDSUP) and
check_feature(FEATURE_HASUP). And depending on the order of pull requests this
is problematic. The previous patch can get away with direct usage of
amd_iommu_efr, but this one sadly no.

I can skip this patch in particular for v4 and re-submit after -rc1 when
everything is aligned. It is only for user experience about console printing two
strings. Real feature probe is not affected users still have the old sysfs
interface, and these days IOMMUFD GET_HW_INFO which userspace/VMM will rely on.

> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
>
Thanks!