2022-06-10 09:13:51

by Kalpana Shetty

[permalink] [raw]
Subject: [PATCH V2] selftests/vm: Add protection_keys tests to run_vmtests

Adding "protected_keys" tests to "run_vmtests.sh" would help out to run all VM related tests
from a single shell script.

Signed-off-by: Kalpana Shetty <[email protected]>
---
Changes in V2:
* Added patch description.

tools/testing/selftests/vm/run_vmtests.sh | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/vm/run_vmtests.sh
index 41fce8bea929..54a0c28f810c 100755
--- a/tools/testing/selftests/vm/run_vmtests.sh
+++ b/tools/testing/selftests/vm/run_vmtests.sh
@@ -179,4 +179,11 @@ run_test ./ksm_tests -N -m 1
# KSM test with 2 NUMA nodes and merge_across_nodes = 0
run_test ./ksm_tests -N -m 0

+# protection_keys tests
+if [ $VADDR64 -eq 0 ]; then
+ run_test ./protection_keys_32
+else
+ run_test ./protection_keys_64
+fi
+
exit $exitcode
--
2.25.1


2022-06-13 22:07:15

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH V2] selftests/vm: Add protection_keys tests to run_vmtests

On 6/10/22 3:07 AM, Kalpana Shetty wrote:
> Adding "protected_keys" tests to "run_vmtests.sh" would help out to run all VM related tests
> from a single shell script.
>

Makes sense - can you explain why you can't just run
protection_keys_32 without checks? Why are you checking
for VADDR64? All of this information helps us review the
patch and give you feedback and suggest a different approach.

> Signed-off-by: Kalpana Shetty <[email protected]>
> ---
> Changes in V2:
> * Added patch description.
>

thanks,
-- Shuah

2022-06-14 12:30:11

by Kalpana Shetty

[permalink] [raw]
Subject: Re: [PATCH V2] selftests/vm: Add protection_keys tests to run_vmtests


On 6/14/2022 3:14 AM, Shuah Khan wrote:
> On 6/10/22 3:07 AM, Kalpana Shetty wrote:
>> Adding "protected_keys" tests to "run_vmtests.sh" would help out to
>> run all VM related tests
>> from a single shell script.
>>
>
> Makes sense - can you explain why you can't just run
> protection_keys_32 without checks?

Yes; we can run protection_keys_32 without check.


> Why are you checking for VADDR64?

The check is added to ensure if the system is in 64-bit mode before
executing 64-bit binary.


> All of this information helps us review the
> patch and give you feedback and suggest a different approach.


Thanks,

Kalpana

>
>> Signed-off-by: Kalpana Shetty <[email protected]>
>> ---
>>   Changes in V2:
>>    * Added patch description.
>>
>
> thanks,
> -- Shuah

2022-06-14 18:10:19

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH V2] selftests/vm: Add protection_keys tests to run_vmtests

On 6/14/22 6:15 AM, Shetty, Kalpana wrote:
>
> On 6/14/2022 3:14 AM, Shuah Khan wrote:
>> On 6/10/22 3:07 AM, Kalpana Shetty wrote:
>>> Adding "protected_keys" tests to "run_vmtests.sh" would help out to run all VM related tests
>>> from a single shell script.
>>>
>>
>> Makes sense - can you explain why you can't just run
>> protection_keys_32 without checks?
>
> Yes; we can run protection_keys_32 without check.
>
>
>> Why are you checking for VADDR64?
>
> The check is added to ensure if the system is in 64-bit mode before executing 64-bit binary.
>
>

Okay. protection_keys_32 will only be built on 32-bit system and.
protection_keys_64 on 64-bit system.

Won't it be better to check if binary exists and run either _32 or
_64 instead of checking for VADDR64?

thanks,
-- Shuah

2022-06-15 12:16:40

by Kalpana Shetty

[permalink] [raw]
Subject: Re: [PATCH V2] selftests/vm: Add protection_keys tests to run_vmtests


On 6/14/2022 10:50 PM, Shuah Khan wrote:
> On 6/14/22 6:15 AM, Shetty, Kalpana wrote:
>>
>> On 6/14/2022 3:14 AM, Shuah Khan wrote:
>>> On 6/10/22 3:07 AM, Kalpana Shetty wrote:
>>>> Adding "protected_keys" tests to "run_vmtests.sh" would help out to
>>>> run all VM related tests
>>>> from a single shell script.
>>>>
>>>
>>> Makes sense - can you explain why you can't just run
>>> protection_keys_32 without checks?
>>
>> Yes; we can run protection_keys_32 without check.
>>
>>
>>> Why are you checking for VADDR64?
>>
>> The check is added to ensure if the system is in 64-bit mode before
>> executing 64-bit binary.
>>
>>
>
> Okay. protection_keys_32 will only be built on 32-bit system and.
> protection_keys_64 on 64-bit system.

On 64-bit system, we get both 32-bit and 64-bit binary.


>
> Won't it be better to check if binary exists and run either _32 or
> _64 instead of checking for VADDR64?

makes sense;

In this case on 64-bit platform we would run both _32 and _64 and this
should be fine.


> thanks,
> -- Shuah

Thanks,

Kalpana

2022-06-16 20:06:18

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH V2] selftests/vm: Add protection_keys tests to run_vmtests

On 6/15/22 6:04 AM, Shetty, Kalpana wrote:
>
> On 6/14/2022 10:50 PM, Shuah Khan wrote:
>> On 6/14/22 6:15 AM, Shetty, Kalpana wrote:
>>>
>>> On 6/14/2022 3:14 AM, Shuah Khan wrote:
>>>> On 6/10/22 3:07 AM, Kalpana Shetty wrote:
>>>>> Adding "protected_keys" tests to "run_vmtests.sh" would help out to run all VM related tests
>>>>> from a single shell script.
>>>>>
>>>>
>>>> Makes sense - can you explain why you can't just run
>>>> protection_keys_32 without checks?
>>>
>>> Yes; we can run protection_keys_32 without check.
>>>
>>>
>>>> Why are you checking for VADDR64?
>>>
>>> The check is added to ensure if the system is in 64-bit mode before executing 64-bit binary.
>>>
>>>
>>
>> Okay. protection_keys_32 will only be built on 32-bit system and.
>> protection_keys_64 on 64-bit system.
>
> On 64-bit system, we get both 32-bit and 64-bit binary.
>
>
>>
>> Won't it be better to check if binary exists and run either _32 or
>> _64 instead of checking for VADDR64?
>
> makes sense;
>
> In this case on 64-bit platform we would run both _32 and _64 and this should be fine.
>
>

Okay - send v3 with the change.

thanks,
-- Shuah

2022-06-17 12:01:00

by Kalpana Shetty

[permalink] [raw]
Subject: Re: [PATCH V2] selftests/vm: Add protection_keys tests to run_vmtests


On 6/17/2022 1:23 AM, Shuah Khan wrote:
> On 6/15/22 6:04 AM, Shetty, Kalpana wrote:
>>
>> On 6/14/2022 10:50 PM, Shuah Khan wrote:
>>> On 6/14/22 6:15 AM, Shetty, Kalpana wrote:
>>>>
>>>> On 6/14/2022 3:14 AM, Shuah Khan wrote:
>>>>> On 6/10/22 3:07 AM, Kalpana Shetty wrote:
>>>>>> Adding "protected_keys" tests to "run_vmtests.sh" would help out
>>>>>> to run all VM related tests
>>>>>> from a single shell script.
>>>>>>
>>>>>
>>>>> Makes sense - can you explain why you can't just run
>>>>> protection_keys_32 without checks?
>>>>
>>>> Yes; we can run protection_keys_32 without check.
>>>>
>>>>
>>>>> Why are you checking for VADDR64?
>>>>
>>>> The check is added to ensure if the system is in 64-bit mode before
>>>> executing 64-bit binary.
>>>>
>>>>
>>>
>>> Okay. protection_keys_32 will only be built on 32-bit system and.
>>> protection_keys_64 on 64-bit system.
>>
>> On 64-bit system, we get both 32-bit and 64-bit binary.
>>
>>
>>>
>>> Won't it be better to check if binary exists and run either _32 or
>>> _64 instead of checking for VADDR64?
>>
>> makes sense;
>>
>> In this case on 64-bit platform we would run both _32 and _64 and
>> this should be fine.
>>
>>
>
> Okay - send v3 with the change.
Done; thanks for your input/review comments.
>
> thanks,
> -- Shuah

Thanks,

Kalpana