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

Joao Martins
2023-10-18T09:06:02Z

On 18/10/2023 10:03, Vasant Hegde wrote:
> 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?

Yeap, pretty much; I've removed this from this series.