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

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

[Kalpana Shetty]

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 <kalpana.shetty@amd.com>
---
 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

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

[Shuah Khan]

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 <kalpana.shetty@amd.com>
> ---
>   Changes in V2:
>    * Added patch description.
> 

thanks,
-- Shuah

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

[Shetty, Kalpana]

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 <kalpana.shetty@amd.com>
>> ---
>>   Changes in V2:
>>    * Added patch description.
>>
>
> thanks,
> -- Shuah

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

[Shuah Khan]

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

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

[Shetty, Kalpana]

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

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

[Shuah Khan]

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

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

[Shetty, Kalpana]

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