| Message ID | 20220203143620.30629-3-joey.gouly@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add more tests to MTE kselftests | expand |
On 2/3/22 7:36 AM, Joey Gouly wrote: > This makes it easier to have multiple iterations in the test. Please add more information here. Based on the code instead of pagesz, this test allows other sizes. This is a good change and bit more info. helps describe the value of this change. > > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Mark Brown <broonie@kernel.org> > Cc: Shuah Khan <shuah@kernel.org> > --- > .../selftests/arm64/mte/check_user_mem.c | 27 ++++++++++++------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c > index 5a5a7e1f5789..e5bfeaeb64bb 100644 > --- a/tools/testing/selftests/arm64/mte/check_user_mem.c > +++ b/tools/testing/selftests/arm64/mte/check_user_mem.c > @@ -27,7 +27,7 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping, > size_t len, read_len; > void *ptr, *ptr_next; > > - err = KSFT_FAIL; > + err = KSFT_PASS; > len = 2 * page_sz; > mte_switch_mode(mode, MTE_ALLOW_NON_ZERO_TAG); > fd = create_temp_file(); > @@ -46,14 +46,18 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping, > /* Copy from file into buffer with valid tag */ > read_len = read(fd, ptr, len); > mte_wait_after_trig(); > - if (cur_mte_cxt.fault_valid || read_len < len) > + if (cur_mte_cxt.fault_valid || read_len < len) { > + err = KSFT_FAIL; Why not set err in usermem_acc_err - this way you can make the error paths common and simpler. > goto usermem_acc_err; > + } > /* Verify same pattern is read */ > for (i = 0; i < len; i++) > if (*(char *)(ptr + i) != val) > break; > - if (i < len) > + if (i < len) { > + err = KSFT_FAIL; The above assignment can go way if you add this to usermem_acc_err path > goto usermem_acc_err; > + } > > if (!tag_len) > tag_len = len - tag_offset; > @@ -71,12 +75,17 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping, > * mode without fault but may not fail in async mode as per the > * implemented MTE userspace support in Arm64 kernel. > */ > - if (mode == MTE_SYNC_ERR && > - !cur_mte_cxt.fault_valid && read_len < len) { > - err = KSFT_PASS; > - } else if (mode == MTE_ASYNC_ERR && > - !cur_mte_cxt.fault_valid && read_len == len) { > - err = KSFT_PASS; > + if (cur_mte_cxt.fault_valid) { > + err = KSFT_FAIL; The above assignment can go way if you add this to usermem_acc_err path > + goto usermem_acc_err; > + } > + if (mode == MTE_SYNC_ERR && read_len < len) { > + /* test passed */ > + } else if (mode == MTE_ASYNC_ERR && read_len == len) { > + /* test passed */ > + } else { > + err = KSFT_FAIL; The above assignment can go way if you add this to usermem_acc_err path > + goto usermem_acc_err; > } > usermem_acc_err: > mte_free_memory((void *)ptr, len, mem_type, true); > With the above requested changes: Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> thanks, -- Shuah
diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c index 5a5a7e1f5789..e5bfeaeb64bb 100644 --- a/tools/testing/selftests/arm64/mte/check_user_mem.c +++ b/tools/testing/selftests/arm64/mte/check_user_mem.c @@ -27,7 +27,7 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping, size_t len, read_len; void *ptr, *ptr_next; - err = KSFT_FAIL; + err = KSFT_PASS; len = 2 * page_sz; mte_switch_mode(mode, MTE_ALLOW_NON_ZERO_TAG); fd = create_temp_file(); @@ -46,14 +46,18 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping, /* Copy from file into buffer with valid tag */ read_len = read(fd, ptr, len); mte_wait_after_trig(); - if (cur_mte_cxt.fault_valid || read_len < len) + if (cur_mte_cxt.fault_valid || read_len < len) { + err = KSFT_FAIL; goto usermem_acc_err; + } /* Verify same pattern is read */ for (i = 0; i < len; i++) if (*(char *)(ptr + i) != val) break; - if (i < len) + if (i < len) { + err = KSFT_FAIL; goto usermem_acc_err; + } if (!tag_len) tag_len = len - tag_offset; @@ -71,12 +75,17 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping, * mode without fault but may not fail in async mode as per the * implemented MTE userspace support in Arm64 kernel. */ - if (mode == MTE_SYNC_ERR && - !cur_mte_cxt.fault_valid && read_len < len) { - err = KSFT_PASS; - } else if (mode == MTE_ASYNC_ERR && - !cur_mte_cxt.fault_valid && read_len == len) { - err = KSFT_PASS; + if (cur_mte_cxt.fault_valid) { + err = KSFT_FAIL; + goto usermem_acc_err; + } + if (mode == MTE_SYNC_ERR && read_len < len) { + /* test passed */ + } else if (mode == MTE_ASYNC_ERR && read_len == len) { + /* test passed */ + } else { + err = KSFT_FAIL; + goto usermem_acc_err; } usermem_acc_err: mte_free_memory((void *)ptr, len, mem_type, true);
This makes it easier to have multiple iterations in the test. Signed-off-by: Joey Gouly <joey.gouly@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Mark Brown <broonie@kernel.org> Cc: Shuah Khan <shuah@kernel.org> --- .../selftests/arm64/mte/check_user_mem.c | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-)