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

--===============8240426197762839889==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable

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 it'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.c=
om>
>> 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_o=
n 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
--===============8240426197762839889==--