[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==--