All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/5] selftests/arm64: MTE check_tags_inclusion cleanups
@ 2022-05-10 16:45 ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

This series contains some random cleanups and improvements that I came
up with while looking at the check_tags_inclusion test.  There's nothing
too exciting in here but the changes did seem like they might help the
next person to look at this.

Mark Brown (5):
  selftests/arm64: Log errors in verify_mte_pointer_validity()
  selftests/arm64: Allow zero tags in mte_switch_mode()
  selftests/arm64: Check failures to set tags in check_tags_inclusion
  selftests/arm64: Remove casts to/from void in check_tags_inclusion
  selftests/arm64: Use switch statements in mte_common_util.c

 .../arm64/mte/check_tags_inclusion.c          | 54 +++++++++++--------
 .../selftests/arm64/mte/mte_common_util.c     | 25 ++++++---
 2 files changed, 50 insertions(+), 29 deletions(-)


base-commit: ae60e0763e97e977b03af1ac6ba782a4a86c3a5a
-- 
2.30.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v1 0/5] selftests/arm64: MTE check_tags_inclusion cleanups
@ 2022-05-10 16:45 ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

This series contains some random cleanups and improvements that I came
up with while looking at the check_tags_inclusion test.  There's nothing
too exciting in here but the changes did seem like they might help the
next person to look at this.

Mark Brown (5):
  selftests/arm64: Log errors in verify_mte_pointer_validity()
  selftests/arm64: Allow zero tags in mte_switch_mode()
  selftests/arm64: Check failures to set tags in check_tags_inclusion
  selftests/arm64: Remove casts to/from void in check_tags_inclusion
  selftests/arm64: Use switch statements in mte_common_util.c

 .../arm64/mte/check_tags_inclusion.c          | 54 +++++++++++--------
 .../selftests/arm64/mte/mte_common_util.c     | 25 ++++++---
 2 files changed, 50 insertions(+), 29 deletions(-)


base-commit: ae60e0763e97e977b03af1ac6ba782a4a86c3a5a
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v1 1/5] selftests/arm64: Log errors in verify_mte_pointer_validity()
  2022-05-10 16:45 ` Mark Brown
@ 2022-05-10 16:45   ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

When we detect a problem in verify_mte_pointer_validity() while checking
tags we don't log what the problem was which makes debugging harder. Add
some diagnostics.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../selftests/arm64/mte/check_tags_inclusion.c       | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
index deaef1f61076..b906914997ce 100644
--- a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
+++ b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
@@ -25,8 +25,11 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
 	/* Check the validity of the tagged pointer */
 	memset((void *)ptr, '1', BUFFER_SIZE);
 	mte_wait_after_trig();
-	if (cur_mte_cxt.fault_valid)
+	if (cur_mte_cxt.fault_valid) {
+		ksft_print_msg("Unexpected fault recorded for %p-%p in mode %x\n",
+			       ptr, ptr + BUFFER_SIZE, mode);
 		return KSFT_FAIL;
+	}
 	/* Proceed further for nonzero tags */
 	if (!MT_FETCH_TAG((uintptr_t)ptr))
 		return KSFT_PASS;
@@ -34,10 +37,13 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
 	/* Check the validity outside the range */
 	ptr[BUFFER_SIZE] = '2';
 	mte_wait_after_trig();
-	if (!cur_mte_cxt.fault_valid)
+	if (!cur_mte_cxt.fault_valid) {
+		ksft_print_msg("No valid fault recorded for %p in mode %x\n",
+			       ptr, mode);
 		return KSFT_FAIL;
-	else
+	} else {
 		return KSFT_PASS;
+	}
 }
 
 static int check_single_included_tags(int mem_type, int mode)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 1/5] selftests/arm64: Log errors in verify_mte_pointer_validity()
@ 2022-05-10 16:45   ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

When we detect a problem in verify_mte_pointer_validity() while checking
tags we don't log what the problem was which makes debugging harder. Add
some diagnostics.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../selftests/arm64/mte/check_tags_inclusion.c       | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
index deaef1f61076..b906914997ce 100644
--- a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
+++ b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
@@ -25,8 +25,11 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
 	/* Check the validity of the tagged pointer */
 	memset((void *)ptr, '1', BUFFER_SIZE);
 	mte_wait_after_trig();
-	if (cur_mte_cxt.fault_valid)
+	if (cur_mte_cxt.fault_valid) {
+		ksft_print_msg("Unexpected fault recorded for %p-%p in mode %x\n",
+			       ptr, ptr + BUFFER_SIZE, mode);
 		return KSFT_FAIL;
+	}
 	/* Proceed further for nonzero tags */
 	if (!MT_FETCH_TAG((uintptr_t)ptr))
 		return KSFT_PASS;
@@ -34,10 +37,13 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
 	/* Check the validity outside the range */
 	ptr[BUFFER_SIZE] = '2';
 	mte_wait_after_trig();
-	if (!cur_mte_cxt.fault_valid)
+	if (!cur_mte_cxt.fault_valid) {
+		ksft_print_msg("No valid fault recorded for %p in mode %x\n",
+			       ptr, mode);
 		return KSFT_FAIL;
-	else
+	} else {
 		return KSFT_PASS;
+	}
 }
 
 static int check_single_included_tags(int mem_type, int mode)
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 2/5] selftests/arm64: Allow zero tags in mte_switch_mode()
  2022-05-10 16:45 ` Mark Brown
@ 2022-05-10 16:45   ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

mte_switch_mode() currently rejects attempts to set a zero tag however
there are tests such as check_tags_inclusion which attempt to cover cases
with zero tags using mte_switch_mode(). Since it is not clear why we are
rejecting zero tags change the test to accept them.

The issue has not previously been as apparent as it should be since the
return value of mte_switch_mode() was not always checked in the callers
and the tests weren't otherwise failing.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/mte/mte_common_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c
index 260206f4dce0..6ff4c4bcbff1 100644
--- a/tools/testing/selftests/arm64/mte/mte_common_util.c
+++ b/tools/testing/selftests/arm64/mte/mte_common_util.c
@@ -283,7 +283,7 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask)
 		return -EINVAL;
 	}
 
-	if (!(incl_mask <= MTE_ALLOW_NON_ZERO_TAG)) {
+	if (incl_mask & ~MT_INCLUDE_TAG_MASK) {
 		ksft_print_msg("FAIL: Invalid incl_mask %lx\n", incl_mask);
 		return -EINVAL;
 	}
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 2/5] selftests/arm64: Allow zero tags in mte_switch_mode()
@ 2022-05-10 16:45   ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

mte_switch_mode() currently rejects attempts to set a zero tag however
there are tests such as check_tags_inclusion which attempt to cover cases
with zero tags using mte_switch_mode(). Since it is not clear why we are
rejecting zero tags change the test to accept them.

The issue has not previously been as apparent as it should be since the
return value of mte_switch_mode() was not always checked in the callers
and the tests weren't otherwise failing.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/arm64/mte/mte_common_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c
index 260206f4dce0..6ff4c4bcbff1 100644
--- a/tools/testing/selftests/arm64/mte/mte_common_util.c
+++ b/tools/testing/selftests/arm64/mte/mte_common_util.c
@@ -283,7 +283,7 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask)
 		return -EINVAL;
 	}
 
-	if (!(incl_mask <= MTE_ALLOW_NON_ZERO_TAG)) {
+	if (incl_mask & ~MT_INCLUDE_TAG_MASK) {
 		ksft_print_msg("FAIL: Invalid incl_mask %lx\n", incl_mask);
 		return -EINVAL;
 	}
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 3/5] selftests/arm64: Check failures to set tags in check_tags_inclusion
  2022-05-10 16:45 ` Mark Brown
@ 2022-05-10 16:45   ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

The MTE check_tags_inclusion test uses the mte_switch_mode() helper but
ignores the return values it generates meaning we might not be testing
the things we're trying to test, fail the test if it reports an error.
The helper will log any errors it returns.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../selftests/arm64/mte/check_tags_inclusion.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
index b906914997ce..d180ba3df990 100644
--- a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
+++ b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
@@ -49,7 +49,7 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
 static int check_single_included_tags(int mem_type, int mode)
 {
 	char *ptr;
-	int tag, run, result = KSFT_PASS;
+	int tag, run, ret, result = KSFT_PASS;
 
 	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
@@ -57,7 +57,9 @@ static int check_single_included_tags(int mem_type, int mode)
 		return KSFT_FAIL;
 
 	for (tag = 0; (tag < MT_TAG_COUNT) && (result == KSFT_PASS); tag++) {
-		mte_switch_mode(mode, MT_INCLUDE_VALID_TAG(tag));
+		ret = mte_switch_mode(mode, MT_INCLUDE_VALID_TAG(tag));
+		if (ret != 0)
+			result = KSFT_FAIL;
 		/* Try to catch a excluded tag by a number of tries. */
 		for (run = 0; (run < RUNS) && (result == KSFT_PASS); run++) {
 			ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
@@ -111,14 +113,16 @@ static int check_multiple_included_tags(int mem_type, int mode)
 static int check_all_included_tags(int mem_type, int mode)
 {
 	char *ptr;
-	int run, result = KSFT_PASS;
+	int run, ret, result = KSFT_PASS;
 
 	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
 				   mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
 
-	mte_switch_mode(mode, MT_INCLUDE_TAG_MASK);
+	ret = mte_switch_mode(mode, MT_INCLUDE_TAG_MASK);
+	if (ret != 0)
+		return KSFT_FAIL;
 	/* Try to catch a excluded tag by a number of tries. */
 	for (run = 0; (run < RUNS) && (result == KSFT_PASS); run++) {
 		ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
@@ -135,13 +139,15 @@ static int check_all_included_tags(int mem_type, int mode)
 static int check_none_included_tags(int mem_type, int mode)
 {
 	char *ptr;
-	int run;
+	int run, ret;
 
 	ptr = (char *)mte_allocate_memory(BUFFER_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE, mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
 
-	mte_switch_mode(mode, MT_EXCLUDE_TAG_MASK);
+	ret = mte_switch_mode(mode, MT_EXCLUDE_TAG_MASK);
+	if (ret != 0)
+		return KSFT_FAIL;
 	/* Try to catch a excluded tag by a number of tries. */
 	for (run = 0; run < RUNS; run++) {
 		ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 3/5] selftests/arm64: Check failures to set tags in check_tags_inclusion
@ 2022-05-10 16:45   ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

The MTE check_tags_inclusion test uses the mte_switch_mode() helper but
ignores the return values it generates meaning we might not be testing
the things we're trying to test, fail the test if it reports an error.
The helper will log any errors it returns.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../selftests/arm64/mte/check_tags_inclusion.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
index b906914997ce..d180ba3df990 100644
--- a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
+++ b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
@@ -49,7 +49,7 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
 static int check_single_included_tags(int mem_type, int mode)
 {
 	char *ptr;
-	int tag, run, result = KSFT_PASS;
+	int tag, run, ret, result = KSFT_PASS;
 
 	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
@@ -57,7 +57,9 @@ static int check_single_included_tags(int mem_type, int mode)
 		return KSFT_FAIL;
 
 	for (tag = 0; (tag < MT_TAG_COUNT) && (result == KSFT_PASS); tag++) {
-		mte_switch_mode(mode, MT_INCLUDE_VALID_TAG(tag));
+		ret = mte_switch_mode(mode, MT_INCLUDE_VALID_TAG(tag));
+		if (ret != 0)
+			result = KSFT_FAIL;
 		/* Try to catch a excluded tag by a number of tries. */
 		for (run = 0; (run < RUNS) && (result == KSFT_PASS); run++) {
 			ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
@@ -111,14 +113,16 @@ static int check_multiple_included_tags(int mem_type, int mode)
 static int check_all_included_tags(int mem_type, int mode)
 {
 	char *ptr;
-	int run, result = KSFT_PASS;
+	int run, ret, result = KSFT_PASS;
 
 	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
 				   mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
 
-	mte_switch_mode(mode, MT_INCLUDE_TAG_MASK);
+	ret = mte_switch_mode(mode, MT_INCLUDE_TAG_MASK);
+	if (ret != 0)
+		return KSFT_FAIL;
 	/* Try to catch a excluded tag by a number of tries. */
 	for (run = 0; (run < RUNS) && (result == KSFT_PASS); run++) {
 		ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
@@ -135,13 +139,15 @@ static int check_all_included_tags(int mem_type, int mode)
 static int check_none_included_tags(int mem_type, int mode)
 {
 	char *ptr;
-	int run;
+	int run, ret;
 
 	ptr = (char *)mte_allocate_memory(BUFFER_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE, mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
 
-	mte_switch_mode(mode, MT_EXCLUDE_TAG_MASK);
+	ret = mte_switch_mode(mode, MT_EXCLUDE_TAG_MASK);
+	if (ret != 0)
+		return KSFT_FAIL;
 	/* Try to catch a excluded tag by a number of tries. */
 	for (run = 0; run < RUNS; run++) {
 		ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 4/5] selftests/arm64: Remove casts to/from void in check_tags_inclusion
  2022-05-10 16:45 ` Mark Brown
@ 2022-05-10 16:45   ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

Void pointers may be freely used with other pointer types in C, any casts
between void * and other pointer types serve no purpose other than to
mask potential warnings. Drop such casts from check_tags_inclusion to
help with future review of the code.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../arm64/mte/check_tags_inclusion.c          | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
index d180ba3df990..2b1425b92b69 100644
--- a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
+++ b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
@@ -23,7 +23,7 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
 {
 	mte_initialize_current_context(mode, (uintptr_t)ptr, BUFFER_SIZE);
 	/* Check the validity of the tagged pointer */
-	memset((void *)ptr, '1', BUFFER_SIZE);
+	memset(ptr, '1', BUFFER_SIZE);
 	mte_wait_after_trig();
 	if (cur_mte_cxt.fault_valid) {
 		ksft_print_msg("Unexpected fault recorded for %p-%p in mode %x\n",
@@ -51,7 +51,7 @@ static int check_single_included_tags(int mem_type, int mode)
 	char *ptr;
 	int tag, run, ret, result = KSFT_PASS;
 
-	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
+	ptr = mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
 				   mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
@@ -62,7 +62,7 @@ static int check_single_included_tags(int mem_type, int mode)
 			result = KSFT_FAIL;
 		/* Try to catch a excluded tag by a number of tries. */
 		for (run = 0; (run < RUNS) && (result == KSFT_PASS); run++) {
-			ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
+			ptr = mte_insert_tags(ptr, BUFFER_SIZE);
 			/* Check tag value */
 			if (MT_FETCH_TAG((uintptr_t)ptr) == tag) {
 				ksft_print_msg("FAIL: wrong tag = 0x%x with include mask=0x%x\n",
@@ -74,7 +74,7 @@ static int check_single_included_tags(int mem_type, int mode)
 			result = verify_mte_pointer_validity(ptr, mode);
 		}
 	}
-	mte_free_memory_tag_range((void *)ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
+	mte_free_memory_tag_range(ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
 	return result;
 }
 
@@ -84,7 +84,7 @@ static int check_multiple_included_tags(int mem_type, int mode)
 	int tag, run, result = KSFT_PASS;
 	unsigned long excl_mask = 0;
 
-	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
+	ptr = mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
 				   mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
@@ -94,7 +94,7 @@ static int check_multiple_included_tags(int mem_type, int mode)
 		mte_switch_mode(mode, MT_INCLUDE_VALID_TAGS(excl_mask));
 		/* Try to catch a excluded tag by a number of tries. */
 		for (run = 0; (run < RUNS) && (result == KSFT_PASS); run++) {
-			ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
+			ptr = mte_insert_tags(ptr, BUFFER_SIZE);
 			/* Check tag value */
 			if (MT_FETCH_TAG((uintptr_t)ptr) < tag) {
 				ksft_print_msg("FAIL: wrong tag = 0x%x with include mask=0x%x\n",
@@ -106,7 +106,7 @@ static int check_multiple_included_tags(int mem_type, int mode)
 			result = verify_mte_pointer_validity(ptr, mode);
 		}
 	}
-	mte_free_memory_tag_range((void *)ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
+	mte_free_memory_tag_range(ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
 	return result;
 }
 
@@ -115,7 +115,7 @@ static int check_all_included_tags(int mem_type, int mode)
 	char *ptr;
 	int run, ret, result = KSFT_PASS;
 
-	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
+	ptr = mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
 				   mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
@@ -132,7 +132,7 @@ static int check_all_included_tags(int mem_type, int mode)
 		 */
 		result = verify_mte_pointer_validity(ptr, mode);
 	}
-	mte_free_memory_tag_range((void *)ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
+	mte_free_memory_tag_range(ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
 	return result;
 }
 
@@ -141,7 +141,7 @@ static int check_none_included_tags(int mem_type, int mode)
 	char *ptr;
 	int run, ret;
 
-	ptr = (char *)mte_allocate_memory(BUFFER_SIZE, mem_type, 0, false);
+	ptr = mte_allocate_memory(BUFFER_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE, mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
 
@@ -159,12 +159,12 @@ static int check_none_included_tags(int mem_type, int mode)
 		}
 		mte_initialize_current_context(mode, (uintptr_t)ptr, BUFFER_SIZE);
 		/* Check the write validity of the untagged pointer */
-		memset((void *)ptr, '1', BUFFER_SIZE);
+		memset(ptr, '1', BUFFER_SIZE);
 		mte_wait_after_trig();
 		if (cur_mte_cxt.fault_valid)
 			break;
 	}
-	mte_free_memory((void *)ptr, BUFFER_SIZE, mem_type, false);
+	mte_free_memory(ptr, BUFFER_SIZE, mem_type, false);
 	if (cur_mte_cxt.fault_valid)
 		return KSFT_FAIL;
 	else
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 4/5] selftests/arm64: Remove casts to/from void in check_tags_inclusion
@ 2022-05-10 16:45   ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

Void pointers may be freely used with other pointer types in C, any casts
between void * and other pointer types serve no purpose other than to
mask potential warnings. Drop such casts from check_tags_inclusion to
help with future review of the code.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../arm64/mte/check_tags_inclusion.c          | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
index d180ba3df990..2b1425b92b69 100644
--- a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
+++ b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
@@ -23,7 +23,7 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
 {
 	mte_initialize_current_context(mode, (uintptr_t)ptr, BUFFER_SIZE);
 	/* Check the validity of the tagged pointer */
-	memset((void *)ptr, '1', BUFFER_SIZE);
+	memset(ptr, '1', BUFFER_SIZE);
 	mte_wait_after_trig();
 	if (cur_mte_cxt.fault_valid) {
 		ksft_print_msg("Unexpected fault recorded for %p-%p in mode %x\n",
@@ -51,7 +51,7 @@ static int check_single_included_tags(int mem_type, int mode)
 	char *ptr;
 	int tag, run, ret, result = KSFT_PASS;
 
-	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
+	ptr = mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
 				   mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
@@ -62,7 +62,7 @@ static int check_single_included_tags(int mem_type, int mode)
 			result = KSFT_FAIL;
 		/* Try to catch a excluded tag by a number of tries. */
 		for (run = 0; (run < RUNS) && (result == KSFT_PASS); run++) {
-			ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
+			ptr = mte_insert_tags(ptr, BUFFER_SIZE);
 			/* Check tag value */
 			if (MT_FETCH_TAG((uintptr_t)ptr) == tag) {
 				ksft_print_msg("FAIL: wrong tag = 0x%x with include mask=0x%x\n",
@@ -74,7 +74,7 @@ static int check_single_included_tags(int mem_type, int mode)
 			result = verify_mte_pointer_validity(ptr, mode);
 		}
 	}
-	mte_free_memory_tag_range((void *)ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
+	mte_free_memory_tag_range(ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
 	return result;
 }
 
@@ -84,7 +84,7 @@ static int check_multiple_included_tags(int mem_type, int mode)
 	int tag, run, result = KSFT_PASS;
 	unsigned long excl_mask = 0;
 
-	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
+	ptr = mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
 				   mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
@@ -94,7 +94,7 @@ static int check_multiple_included_tags(int mem_type, int mode)
 		mte_switch_mode(mode, MT_INCLUDE_VALID_TAGS(excl_mask));
 		/* Try to catch a excluded tag by a number of tries. */
 		for (run = 0; (run < RUNS) && (result == KSFT_PASS); run++) {
-			ptr = (char *)mte_insert_tags(ptr, BUFFER_SIZE);
+			ptr = mte_insert_tags(ptr, BUFFER_SIZE);
 			/* Check tag value */
 			if (MT_FETCH_TAG((uintptr_t)ptr) < tag) {
 				ksft_print_msg("FAIL: wrong tag = 0x%x with include mask=0x%x\n",
@@ -106,7 +106,7 @@ static int check_multiple_included_tags(int mem_type, int mode)
 			result = verify_mte_pointer_validity(ptr, mode);
 		}
 	}
-	mte_free_memory_tag_range((void *)ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
+	mte_free_memory_tag_range(ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
 	return result;
 }
 
@@ -115,7 +115,7 @@ static int check_all_included_tags(int mem_type, int mode)
 	char *ptr;
 	int run, ret, result = KSFT_PASS;
 
-	ptr = (char *)mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
+	ptr = mte_allocate_memory(BUFFER_SIZE + MT_GRANULE_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE + MT_GRANULE_SIZE,
 				   mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
@@ -132,7 +132,7 @@ static int check_all_included_tags(int mem_type, int mode)
 		 */
 		result = verify_mte_pointer_validity(ptr, mode);
 	}
-	mte_free_memory_tag_range((void *)ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
+	mte_free_memory_tag_range(ptr, BUFFER_SIZE, mem_type, 0, MT_GRANULE_SIZE);
 	return result;
 }
 
@@ -141,7 +141,7 @@ static int check_none_included_tags(int mem_type, int mode)
 	char *ptr;
 	int run, ret;
 
-	ptr = (char *)mte_allocate_memory(BUFFER_SIZE, mem_type, 0, false);
+	ptr = mte_allocate_memory(BUFFER_SIZE, mem_type, 0, false);
 	if (check_allocated_memory(ptr, BUFFER_SIZE, mem_type, false) != KSFT_PASS)
 		return KSFT_FAIL;
 
@@ -159,12 +159,12 @@ static int check_none_included_tags(int mem_type, int mode)
 		}
 		mte_initialize_current_context(mode, (uintptr_t)ptr, BUFFER_SIZE);
 		/* Check the write validity of the untagged pointer */
-		memset((void *)ptr, '1', BUFFER_SIZE);
+		memset(ptr, '1', BUFFER_SIZE);
 		mte_wait_after_trig();
 		if (cur_mte_cxt.fault_valid)
 			break;
 	}
-	mte_free_memory((void *)ptr, BUFFER_SIZE, mem_type, false);
+	mte_free_memory(ptr, BUFFER_SIZE, mem_type, false);
 	if (cur_mte_cxt.fault_valid)
 		return KSFT_FAIL;
 	else
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 5/5] selftests/arm64: Use switch statements in mte_common_util.c
  2022-05-10 16:45 ` Mark Brown
@ 2022-05-10 16:45   ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

In the MTE tests there are several places where we use chains of if
statements to open code what could be written as switch statements, move
over to switch statements to make the idiom clearer.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../selftests/arm64/mte/mte_common_util.c     | 23 +++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c
index 6ff4c4bcbff1..00ffd34c66d3 100644
--- a/tools/testing/selftests/arm64/mte/mte_common_util.c
+++ b/tools/testing/selftests/arm64/mte/mte_common_util.c
@@ -128,13 +128,16 @@ static void *__mte_allocate_memory_range(size_t size, int mem_type, int mapping,
 	int prot_flag, map_flag;
 	size_t entire_size = size + range_before + range_after;
 
-	if (mem_type != USE_MALLOC && mem_type != USE_MMAP &&
-	    mem_type != USE_MPROTECT) {
+	switch (mem_type) {
+	case USE_MALLOC:
+		return malloc(entire_size) + range_before;
+	case USE_MMAP:
+	case USE_MPROTECT:
+		break;
+	default:
 		ksft_print_msg("FAIL: Invalid allocate request\n");
 		return NULL;
 	}
-	if (mem_type == USE_MALLOC)
-		return malloc(entire_size) + range_before;
 
 	prot_flag = PROT_READ | PROT_WRITE;
 	if (mem_type == USE_MMAP)
@@ -287,13 +290,19 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask)
 		ksft_print_msg("FAIL: Invalid incl_mask %lx\n", incl_mask);
 		return -EINVAL;
 	}
+
 	en = PR_TAGGED_ADDR_ENABLE;
-	if (mte_option == MTE_SYNC_ERR)
+	switch (mte_option) {
+	case MTE_SYNC_ERR:
 		en |= PR_MTE_TCF_SYNC;
-	else if (mte_option == MTE_ASYNC_ERR)
+		break;
+	case MTE_ASYNC_ERR:
 		en |= PR_MTE_TCF_ASYNC;
-	else if (mte_option == MTE_NONE_ERR)
+		break;
+	case MTE_NONE_ERR:
 		en |= PR_MTE_TCF_NONE;
+		break;
+	}
 
 	en |= (incl_mask << PR_MTE_TAG_SHIFT);
 	/* Enable address tagging ABI, mte error reporting mode and tag inclusion mask. */
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 5/5] selftests/arm64: Use switch statements in mte_common_util.c
@ 2022-05-10 16:45   ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2022-05-10 16:45 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Mark Brown

In the MTE tests there are several places where we use chains of if
statements to open code what could be written as switch statements, move
over to switch statements to make the idiom clearer.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 .../selftests/arm64/mte/mte_common_util.c     | 23 +++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c
index 6ff4c4bcbff1..00ffd34c66d3 100644
--- a/tools/testing/selftests/arm64/mte/mte_common_util.c
+++ b/tools/testing/selftests/arm64/mte/mte_common_util.c
@@ -128,13 +128,16 @@ static void *__mte_allocate_memory_range(size_t size, int mem_type, int mapping,
 	int prot_flag, map_flag;
 	size_t entire_size = size + range_before + range_after;
 
-	if (mem_type != USE_MALLOC && mem_type != USE_MMAP &&
-	    mem_type != USE_MPROTECT) {
+	switch (mem_type) {
+	case USE_MALLOC:
+		return malloc(entire_size) + range_before;
+	case USE_MMAP:
+	case USE_MPROTECT:
+		break;
+	default:
 		ksft_print_msg("FAIL: Invalid allocate request\n");
 		return NULL;
 	}
-	if (mem_type == USE_MALLOC)
-		return malloc(entire_size) + range_before;
 
 	prot_flag = PROT_READ | PROT_WRITE;
 	if (mem_type == USE_MMAP)
@@ -287,13 +290,19 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask)
 		ksft_print_msg("FAIL: Invalid incl_mask %lx\n", incl_mask);
 		return -EINVAL;
 	}
+
 	en = PR_TAGGED_ADDR_ENABLE;
-	if (mte_option == MTE_SYNC_ERR)
+	switch (mte_option) {
+	case MTE_SYNC_ERR:
 		en |= PR_MTE_TCF_SYNC;
-	else if (mte_option == MTE_ASYNC_ERR)
+		break;
+	case MTE_ASYNC_ERR:
 		en |= PR_MTE_TCF_ASYNC;
-	else if (mte_option == MTE_NONE_ERR)
+		break;
+	case MTE_NONE_ERR:
 		en |= PR_MTE_TCF_NONE;
+		break;
+	}
 
 	en |= (incl_mask << PR_MTE_TAG_SHIFT);
 	/* Enable address tagging ABI, mte error reporting mode and tag inclusion mask. */
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 1/5] selftests/arm64: Log errors in verify_mte_pointer_validity()
  2022-05-10 16:45   ` Mark Brown
@ 2022-05-10 17:15     ` Shuah Khan
  -1 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2022-05-10 17:15 UTC (permalink / raw)
  To: Mark Brown, Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Shuah Khan

On 5/10/22 10:45 AM, Mark Brown wrote:
> When we detect a problem in verify_mte_pointer_validity() while checking
> tags we don't log what the problem was which makes debugging harder. Add
> some diagnostics.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   .../selftests/arm64/mte/check_tags_inclusion.c       | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
> index deaef1f61076..b906914997ce 100644
> --- a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
> +++ b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
> @@ -25,8 +25,11 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
>   	/* Check the validity of the tagged pointer */
>   	memset((void *)ptr, '1', BUFFER_SIZE);
>   	mte_wait_after_trig();
> -	if (cur_mte_cxt.fault_valid)
> +	if (cur_mte_cxt.fault_valid) {
> +		ksft_print_msg("Unexpected fault recorded for %p-%p in mode %x\n",
> +			       ptr, ptr + BUFFER_SIZE, mode);
>   		return KSFT_FAIL;
> +	}
>   	/* Proceed further for nonzero tags */
>   	if (!MT_FETCH_TAG((uintptr_t)ptr))
>   		return KSFT_PASS;
> @@ -34,10 +37,13 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
>   	/* Check the validity outside the range */
>   	ptr[BUFFER_SIZE] = '2';
>   	mte_wait_after_trig();
> -	if (!cur_mte_cxt.fault_valid)
> +	if (!cur_mte_cxt.fault_valid) {
> +		ksft_print_msg("No valid fault recorded for %p in mode %x\n",
> +			       ptr, mode);
>   		return KSFT_FAIL;
> -	else
> +	} else {
>   		return KSFT_PASS;
> +	}
>   }
>   
>   static int check_single_included_tags(int mem_type, int mode)
> 

Nice. Thanks for the patch.

It would be a nice addition to print mode names as strings to make
it easy to understand. Could done in a future patch. e,g: MTE_NONE_ERR etc.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 1/5] selftests/arm64: Log errors in verify_mte_pointer_validity()
@ 2022-05-10 17:15     ` Shuah Khan
  0 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2022-05-10 17:15 UTC (permalink / raw)
  To: Mark Brown, Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Shuah Khan

On 5/10/22 10:45 AM, Mark Brown wrote:
> When we detect a problem in verify_mte_pointer_validity() while checking
> tags we don't log what the problem was which makes debugging harder. Add
> some diagnostics.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   .../selftests/arm64/mte/check_tags_inclusion.c       | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
> index deaef1f61076..b906914997ce 100644
> --- a/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
> +++ b/tools/testing/selftests/arm64/mte/check_tags_inclusion.c
> @@ -25,8 +25,11 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
>   	/* Check the validity of the tagged pointer */
>   	memset((void *)ptr, '1', BUFFER_SIZE);
>   	mte_wait_after_trig();
> -	if (cur_mte_cxt.fault_valid)
> +	if (cur_mte_cxt.fault_valid) {
> +		ksft_print_msg("Unexpected fault recorded for %p-%p in mode %x\n",
> +			       ptr, ptr + BUFFER_SIZE, mode);
>   		return KSFT_FAIL;
> +	}
>   	/* Proceed further for nonzero tags */
>   	if (!MT_FETCH_TAG((uintptr_t)ptr))
>   		return KSFT_PASS;
> @@ -34,10 +37,13 @@ static int verify_mte_pointer_validity(char *ptr, int mode)
>   	/* Check the validity outside the range */
>   	ptr[BUFFER_SIZE] = '2';
>   	mte_wait_after_trig();
> -	if (!cur_mte_cxt.fault_valid)
> +	if (!cur_mte_cxt.fault_valid) {
> +		ksft_print_msg("No valid fault recorded for %p in mode %x\n",
> +			       ptr, mode);
>   		return KSFT_FAIL;
> -	else
> +	} else {
>   		return KSFT_PASS;
> +	}
>   }
>   
>   static int check_single_included_tags(int mem_type, int mode)
> 

Nice. Thanks for the patch.

It would be a nice addition to print mode names as strings to make
it easy to understand. Could done in a future patch. e,g: MTE_NONE_ERR etc.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 2/5] selftests/arm64: Allow zero tags in mte_switch_mode()
  2022-05-10 16:45   ` Mark Brown
@ 2022-05-10 17:17     ` Shuah Khan
  -1 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2022-05-10 17:17 UTC (permalink / raw)
  To: Mark Brown, Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Shuah Khan

On 5/10/22 10:45 AM, Mark Brown wrote:
> mte_switch_mode() currently rejects attempts to set a zero tag however
> there are tests such as check_tags_inclusion which attempt to cover cases
> with zero tags using mte_switch_mode(). Since it is not clear why we are
> rejecting zero tags change the test to accept them.
> 
> The issue has not previously been as apparent as it should be since the
> return value of mte_switch_mode() was not always checked in the callers
> and the tests weren't otherwise failing.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   tools/testing/selftests/arm64/mte/mte_common_util.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c
> index 260206f4dce0..6ff4c4bcbff1 100644
> --- a/tools/testing/selftests/arm64/mte/mte_common_util.c
> +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c
> @@ -283,7 +283,7 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask)
>   		return -EINVAL;
>   	}
>   
> -	if (!(incl_mask <= MTE_ALLOW_NON_ZERO_TAG)) {
> +	if (incl_mask & ~MT_INCLUDE_TAG_MASK) {
>   		ksft_print_msg("FAIL: Invalid incl_mask %lx\n", incl_mask);
>   		return -EINVAL;
>   	}
> 

Looks good to me.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 2/5] selftests/arm64: Allow zero tags in mte_switch_mode()
@ 2022-05-10 17:17     ` Shuah Khan
  0 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2022-05-10 17:17 UTC (permalink / raw)
  To: Mark Brown, Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Shuah Khan

On 5/10/22 10:45 AM, Mark Brown wrote:
> mte_switch_mode() currently rejects attempts to set a zero tag however
> there are tests such as check_tags_inclusion which attempt to cover cases
> with zero tags using mte_switch_mode(). Since it is not clear why we are
> rejecting zero tags change the test to accept them.
> 
> The issue has not previously been as apparent as it should be since the
> return value of mte_switch_mode() was not always checked in the callers
> and the tests weren't otherwise failing.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   tools/testing/selftests/arm64/mte/mte_common_util.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c
> index 260206f4dce0..6ff4c4bcbff1 100644
> --- a/tools/testing/selftests/arm64/mte/mte_common_util.c
> +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c
> @@ -283,7 +283,7 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask)
>   		return -EINVAL;
>   	}
>   
> -	if (!(incl_mask <= MTE_ALLOW_NON_ZERO_TAG)) {
> +	if (incl_mask & ~MT_INCLUDE_TAG_MASK) {
>   		ksft_print_msg("FAIL: Invalid incl_mask %lx\n", incl_mask);
>   		return -EINVAL;
>   	}
> 

Looks good to me.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 3/5] selftests/arm64: Check failures to set tags in check_tags_inclusion
  2022-05-10 16:45   ` Mark Brown
@ 2022-05-10 17:19     ` Shuah Khan
  -1 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2022-05-10 17:19 UTC (permalink / raw)
  To: Mark Brown, Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Shuah Khan

On 5/10/22 10:45 AM, Mark Brown wrote:
> The MTE check_tags_inclusion test uses the mte_switch_mode() helper but
> ignores the return values it generates meaning we might not be testing
> the things we're trying to test, fail the test if it reports an error.
> The helper will log any errors it returns.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   .../selftests/arm64/mte/check_tags_inclusion.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 

Thank you. Looks good to me.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 3/5] selftests/arm64: Check failures to set tags in check_tags_inclusion
@ 2022-05-10 17:19     ` Shuah Khan
  0 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2022-05-10 17:19 UTC (permalink / raw)
  To: Mark Brown, Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Shuah Khan

On 5/10/22 10:45 AM, Mark Brown wrote:
> The MTE check_tags_inclusion test uses the mte_switch_mode() helper but
> ignores the return values it generates meaning we might not be testing
> the things we're trying to test, fail the test if it reports an error.
> The helper will log any errors it returns.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   .../selftests/arm64/mte/check_tags_inclusion.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 

Thank you. Looks good to me.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 4/5] selftests/arm64: Remove casts to/from void in check_tags_inclusion
  2022-05-10 16:45   ` Mark Brown
@ 2022-05-10 17:23     ` Shuah Khan
  -1 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2022-05-10 17:23 UTC (permalink / raw)
  To: Mark Brown, Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Shuah Khan

On 5/10/22 10:45 AM, Mark Brown wrote:
> Void pointers may be freely used with other pointer types in C, any casts
> between void * and other pointer types serve no purpose other than to
> mask potential warnings. Drop such casts from check_tags_inclusion to
> help with future review of the code.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   .../arm64/mte/check_tags_inclusion.c          | 24 +++++++++----------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 

Looks good to me.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 4/5] selftests/arm64: Remove casts to/from void in check_tags_inclusion
@ 2022-05-10 17:23     ` Shuah Khan
  0 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2022-05-10 17:23 UTC (permalink / raw)
  To: Mark Brown, Catalin Marinas, Will Deacon, Shuah Khan
  Cc: Joey Gouly, linux-kselftest, linux-arm-kernel, Shuah Khan

On 5/10/22 10:45 AM, Mark Brown wrote:
> Void pointers may be freely used with other pointer types in C, any casts
> between void * and other pointer types serve no purpose other than to
> mask potential warnings. Drop such casts from check_tags_inclusion to
> help with future review of the code.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   .../arm64/mte/check_tags_inclusion.c          | 24 +++++++++----------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 

Looks good to me.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 0/5] selftests/arm64: MTE check_tags_inclusion cleanups
  2022-05-10 16:45 ` Mark Brown
@ 2022-05-15 10:45   ` Catalin Marinas
  -1 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2022-05-15 10:45 UTC (permalink / raw)
  To: Will Deacon, Mark Brown, Shuah Khan
  Cc: linux-kselftest, linux-arm-kernel, Joey Gouly

On Tue, 10 May 2022 17:45:15 +0100, Mark Brown wrote:
> This series contains some random cleanups and improvements that I came
> up with while looking at the check_tags_inclusion test.  There's nothing
> too exciting in here but the changes did seem like they might help the
> next person to look at this.
> 
> Mark Brown (5):
>   selftests/arm64: Log errors in verify_mte_pointer_validity()
>   selftests/arm64: Allow zero tags in mte_switch_mode()
>   selftests/arm64: Check failures to set tags in check_tags_inclusion
>   selftests/arm64: Remove casts to/from void in check_tags_inclusion
>   selftests/arm64: Use switch statements in mte_common_util.c
> 
> [...]

Applied to arm64 (for-next/kselftest), thanks!

[1/5] selftests/arm64: Log errors in verify_mte_pointer_validity()
      https://git.kernel.org/arm64/c/9a5681710740
[2/5] selftests/arm64: Allow zero tags in mte_switch_mode()
      https://git.kernel.org/arm64/c/ffc8274c2193
[3/5] selftests/arm64: Check failures to set tags in check_tags_inclusion
      https://git.kernel.org/arm64/c/72d6771cb173
[4/5] selftests/arm64: Remove casts to/from void in check_tags_inclusion
      https://git.kernel.org/arm64/c/541235dee011
[5/5] selftests/arm64: Use switch statements in mte_common_util.c
      https://git.kernel.org/arm64/c/0639e02254e6

-- 
Catalin


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 0/5] selftests/arm64: MTE check_tags_inclusion cleanups
@ 2022-05-15 10:45   ` Catalin Marinas
  0 siblings, 0 replies; 22+ messages in thread
From: Catalin Marinas @ 2022-05-15 10:45 UTC (permalink / raw)
  To: Will Deacon, Mark Brown, Shuah Khan
  Cc: linux-kselftest, linux-arm-kernel, Joey Gouly

On Tue, 10 May 2022 17:45:15 +0100, Mark Brown wrote:
> This series contains some random cleanups and improvements that I came
> up with while looking at the check_tags_inclusion test.  There's nothing
> too exciting in here but the changes did seem like they might help the
> next person to look at this.
> 
> Mark Brown (5):
>   selftests/arm64: Log errors in verify_mte_pointer_validity()
>   selftests/arm64: Allow zero tags in mte_switch_mode()
>   selftests/arm64: Check failures to set tags in check_tags_inclusion
>   selftests/arm64: Remove casts to/from void in check_tags_inclusion
>   selftests/arm64: Use switch statements in mte_common_util.c
> 
> [...]

Applied to arm64 (for-next/kselftest), thanks!

[1/5] selftests/arm64: Log errors in verify_mte_pointer_validity()
      https://git.kernel.org/arm64/c/9a5681710740
[2/5] selftests/arm64: Allow zero tags in mte_switch_mode()
      https://git.kernel.org/arm64/c/ffc8274c2193
[3/5] selftests/arm64: Check failures to set tags in check_tags_inclusion
      https://git.kernel.org/arm64/c/72d6771cb173
[4/5] selftests/arm64: Remove casts to/from void in check_tags_inclusion
      https://git.kernel.org/arm64/c/541235dee011
[5/5] selftests/arm64: Use switch statements in mte_common_util.c
      https://git.kernel.org/arm64/c/0639e02254e6

-- 
Catalin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2022-05-15 10:47 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-10 16:45 [PATCH v1 0/5] selftests/arm64: MTE check_tags_inclusion cleanups Mark Brown
2022-05-10 16:45 ` Mark Brown
2022-05-10 16:45 ` [PATCH v1 1/5] selftests/arm64: Log errors in verify_mte_pointer_validity() Mark Brown
2022-05-10 16:45   ` Mark Brown
2022-05-10 17:15   ` Shuah Khan
2022-05-10 17:15     ` Shuah Khan
2022-05-10 16:45 ` [PATCH v1 2/5] selftests/arm64: Allow zero tags in mte_switch_mode() Mark Brown
2022-05-10 16:45   ` Mark Brown
2022-05-10 17:17   ` Shuah Khan
2022-05-10 17:17     ` Shuah Khan
2022-05-10 16:45 ` [PATCH v1 3/5] selftests/arm64: Check failures to set tags in check_tags_inclusion Mark Brown
2022-05-10 16:45   ` Mark Brown
2022-05-10 17:19   ` Shuah Khan
2022-05-10 17:19     ` Shuah Khan
2022-05-10 16:45 ` [PATCH v1 4/5] selftests/arm64: Remove casts to/from void " Mark Brown
2022-05-10 16:45   ` Mark Brown
2022-05-10 17:23   ` Shuah Khan
2022-05-10 17:23     ` Shuah Khan
2022-05-10 16:45 ` [PATCH v1 5/5] selftests/arm64: Use switch statements in mte_common_util.c Mark Brown
2022-05-10 16:45   ` Mark Brown
2022-05-15 10:45 ` [PATCH v1 0/5] selftests/arm64: MTE check_tags_inclusion cleanups Catalin Marinas
2022-05-15 10:45   ` Catalin Marinas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.