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

Vasant Hegde
2023-10-18T09:03:28Z

Hi Joao,

On 10/18/2023 2:23 PM, Joao Martins wrote:
> 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

Right.

>
> 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.

IIUC this can be an independent patch and doesn't have strict dependency on this
series itself. May be you can rebase it on top of iommu/next and post it separately?

-Vasant