[Accel-config] Re: [PATCH 1/1] accel-config/test: Remove pasid_enabled restriction from test script

Zhu, Tony <tony.zhu at intel.com> Fri, 08 Jul 2022 01:19:19 +0000
Newsgroups dev.linux.lists.accel-config
Message-ID <BN9PR11MB5433CBDA72E6D805862B89918A829@BN9PR11MB5433.namprd11.prod.outlook.com>
--===============6156182543911352962==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable

Dave,

   Intel 5.18-rc6 and 5.19-rc3 have this issue, 5.19 -rc4 could not reprodu=
ce this issue. =



Tony(zhu, xinzhan)
Cube:SHZ1-3W-279
iNet:8821-5077

-----Original Message-----
From: Jiang, Dave <dave.jiang(a)intel.com> =

Sent: Friday, July 8, 2022 7:10 AM
To: Yu, Fenghua <fenghua.yu(a)intel.com>; Shen, Xiaochen <xiaochen.shen(a)i=
ntel.com>; Zhu, Tony <tony.zhu(a)intel.com>
Cc: accel-config(a)lists.01.org; Thomas, Ramesh <ramesh.thomas(a)intel.com>
Subject: Re: [Accel-config] Re: [PATCH 1/1] accel-config/test: Remove pasid=
_enabled restriction from test script

Ok hmm.... anyhow, I think the point here is instead of changing the script=
 to accommodate kernel behavior, probably should push harder to make sure i=
t's not a regression bug first. :)

On 7/7/2022 3:08 PM, Yu, Fenghua wrote:
> Yes, it's upstream 5.19-rc4. Both Tony and I tested it on SPR and found n=
o issue.
>
> Tony found the issue on an earlier kernel. @Zhu, Tony, which kernel versi=
on did you find the issue?
>
> Thanks.
>
> -Fenghua
>
>> -----Original Message-----
>> From: Jiang, Dave <dave.jiang(a)intel.com>
>> Sent: Thursday, July 07, 2022 3:04 PM
>> To: Yu, Fenghua <fenghua.yu(a)intel.com>; Shen, Xiaochen =

>> <xiaochen.shen(a)intel.com>; Zhu, Tony <tony.zhu(a)intel.com>
>> Cc: accel-config(a)lists.01.org; Thomas, Ramesh =

>> <ramesh.thomas(a)intel.com>
>> Subject: Re: [Accel-config] Re: [PATCH 1/1] accel-config/test: Remove =

>> pasid_enabled restriction from test script
>>
>> Against upstream? I'm surprised. There's no DMA pasid support and =

>> kernel pasid would get 0.
>>
>> On 7/7/2022 3:01 PM, Yu, Fenghua wrote:
>>> Hi, Dave,
>>>
>>> 5.19-rc4 doesn't have the issue. On 5.19-rc4, pasid_enabled=3D1 on =

>>> sm_on and 0
>> on sm_off. So the value is expected. Tony said the issue exists in a ear=
lier kernel.
>> But I don't know which patch fixes the issue.
>>> So seems there is no fix here.
>>>
>>> Thanks.
>>>
>>> -Fenghua
>>>
>>>> -----Original Message-----
>>>> From: Jiang, Dave <dave.jiang(a)intel.com>
>>>> Sent: Thursday, July 07, 2022 2:57 PM
>>>> To: Shen, Xiaochen <xiaochen.shen(a)intel.com>; Zhu, Tony =

>>>> <tony.zhu(a)intel.com>; Yu, Fenghua <fenghua.yu(a)intel.com>
>>>> Cc: accel-config(a)lists.01.org; Thomas, Ramesh =

>>>> <ramesh.thomas(a)intel.com>
>>>> Subject: Re: [Accel-config] Re: [PATCH 1/1] accel-config/test: =

>>>> Remove pasid_enabled restriction from test script
>>>>
>>>> After discussion with Fenghua, I think the conclusion is that we =

>>>> leave pasid_en tied to the user pasid enable. Reason being:
>>>>
>>>> 1. preserve legacy usage
>>>>
>>>> 2. no current kernel user care about the sysfs attribute, so until =

>>>> a customer really wants to know about the kernel side, no need to =

>>>> introduce a kernel_pasid_en attribute.
>>>>
>>>> so I would change the patch to:
>>>>
>>>> sysfs_emit(buf, "%u\n", device_user_pasid_enabled(idxd));
>>>>
>>>> Otherwise if user pasid is disabled and DMA pasid is enabled, your =

>>>> script will still fail.
>>>>
>>>> On 6/28/2022 8:00 PM, Shen, Xiaochen wrote:
>>>>> Hi Tony and Fenghua,
>>>>>
>>>>> This issue may be impacted by this 5.19 upstream patch:
>>>>> 42a1b73852c4a176d233a192422b5e1d0ba67cbf dmaengine: idxd: Separate =

>>>>> user and kernel pasid enabling
>>>>>
>>>>> The sysfs interface "pasid_enabled" doesn't reflect the newly added f=
lag:
>>>>>            IDXD_FLAG_PASID_ENABLED,
>>>>> +       IDXD_FLAG_USER_PASID_ENABLED,
>>>>>
>>>>>
>>>>> This patch may fix this issue:
>>>>>
>>>>> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c =

>>>>> index
>>>>> dfd549685c46..53e34a1d62d9 100644
>>>>> --- a/drivers/dma/idxd/sysfs.c
>>>>> +++ b/drivers/dma/idxd/sysfs.c
>>>>> @@ -1223,7 +1223,8 @@ static ssize_t pasid_enabled_show(struct =

>>>>> device
>>>> *dev,
>>>>>     {
>>>>>            struct idxd_device *idxd =3D confdev_to_idxd(dev);
>>>>>
>>>>> -       return sysfs_emit(buf, "%u\n", device_pasid_enabled(idxd));
>>>>> +       return sysfs_emit(buf, "%u\n",
>>>>> +                         device_pasid_enabled(idxd) || =

>>>>> + device_user_pasid_enabled(idxd));
>>>>>     }
>>>>>     static DEVICE_ATTR_RO(pasid_enabled);
>>>>>
>>>>>
>>>>> Best regards,
>>>>> Xiaochen
>>>>>
>>>>> -----Original Message-----
>>>>> From: Zhu, Tony <tony.zhu(a)intel.com>
>>>>> Sent: Wednesday, June 29, 2022 10:05
>>>>> To: Yu, Fenghua <fenghua.yu(a)intel.com>
>>>>> Cc: accel-config(a)lists.01.org; Thomas, Ramesh =

>>>>> <ramesh.thomas(a)intel.com>
>>>>> Subject: [Accel-config] Re: [PATCH 1/1] accel-config/test: Remove =

>>>>> pasid_enabled restriction from test script
>>>>>
>>>>> Fenghua,
>>>>>
>>>>>      pasid_enabled is 1 when it is scalable mode for old kernel such =
as spr-bkc.
>>>> For kernel code 5.18, pasid_enabled is 0. Though pasid_enable is 0, =

>>>> but the passid is still assigned.
>>>>> I didn't know which commit bring this change. But from the test =

>>>>> result, I could
>>>> see pasid table entry in dmesg log.
>>>>> Tony(zhu, xinzhan)
>>>>> Cube:SHZ1-3W-279
>>>>> iNet:8821-5077
>>>>>
>>>>> -----Original Message-----
>>>>> From: Yu, Fenghua <fenghua.yu(a)intel.com>
>>>>> Sent: Tuesday, June 28, 2022 11:38 PM
>>>>> To: Zhu, Tony <tony.zhu(a)intel.com>
>>>>> Cc: accel-config(a)lists.01.org; Thomas, Ramesh =

>>>>> <ramesh.thomas(a)intel.com>
>>>>> Subject: Re: [PATCH 1/1] accel-config/test: Remove pasid_enabled =

>>>>> restriction from test script
>>>>>
>>>>> Hi, Tony,
>>>>>
>>>>> On Tue, Jun 28, 2022 at 02:40:24PM +0800, Tony Zhu wrote:
>>>>>> Kernel removed the restriction because it broken accel-config.
>>>>>> Failure
>>>>>                                               s/broken/breaks/
>>>>>> will happen during wq enable. Remove the checking from test script t=
oo.
>>>>>>
>>>>>> Signed-off-by: Tony Zhu <tony.zhu(a)intel.com>
>>>>>> ---
>>>>>>     test/dsa_user_test_runner.sh | 5 -----
>>>>>>     1 file changed, 5 deletions(-)
>>>>>>
>>>>>> diff --git a/test/dsa_user_test_runner.sh =

>>>>>> b/test/dsa_user_test_runner.sh index dfaa930..ae9010d 100755
>>>>>> --- a/test/dsa_user_test_runner.sh
>>>>>> +++ b/test/dsa_user_test_runner.sh
>>>>>> @@ -19,11 +19,6 @@ check_min_kver "5.6" || do_skip "kernel does =

>>>>>> not
>>>> support idxd"
>>>>>>     # skip if no pasid support as dsa_test does not support =

>>>>>> operation w/o pasid
>>>> yet.
>>>>>>     [ ! -f "/sys/bus/dsa/devices/$DSA/pasid_enabled" ] && echo =

>>>>>> "No SVM
>>>> support" && exit "$EXIT_SKIP"
>>>>>> -pasid_en=3D$(cat /sys/bus/dsa/devices/$DSA/pasid_enabled)
>>>>>> -if [ "$pasid_en" -ne 1 ]; then
>>>>>> -exit "$EXIT_SKIP"
>>>>>> -fi
>>>>>> -
>>>>> Without pasid, what sub-tests can run? Seems to me, without PASID =

>>>>> enabled,
>>>> the test cannot continue.
>>>>>>     start_dsa()
>>>>>>     {
>>>>>>     configurable=3D$(cat /sys/bus/dsa/devices/$DSA/configurable)
>>>>>> --
>>>>>> 2.27.0
>>>>>>
>>>>> Thanks.
>>>>>
>>>>> -Fenghua
>>>>> _______________________________________________
>>>>> Accel-config mailing list -- accel-config(a)lists.01.org To =

>>>>> unsubscribe send an email to accel-config-leave(a)lists.01.org =

>>>>> _______________________________________________
>>>>> Accel-config mailing list -- accel-config(a)lists.01.org To =

>>>>> unsubscribe send an email to accel-config-leave(a)lists.01.org

--===============6156182543911352962==--