* [kvm-unit-tests PATCH v2 1/3] riscv: sbi: Improve gen_report
2025-02-18 18:54 [kvm-unit-tests PATCH v2 0/3] riscv: sbi: Provide sbiret_report/check Andrew Jones
@ 2025-02-18 18:54 ` Andrew Jones
2025-02-18 18:54 ` [kvm-unit-tests PATCH v2 2/3] riscv: sbi: Export sbiret_report/check Andrew Jones
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Andrew Jones @ 2025-02-18 18:54 UTC (permalink / raw)
To: kvm-riscv; +Cc: cleger, atishp
Make several improvements to gen_report(), starting by renaming it
to something less generic (sbiret_report) and then, instead of
relying on report prefix to link the report_info with the unexpected
return values to the failed test, use the test string itself, by
taking it as a parameter. And, reporting sbiret.error and
sbiret.value separately was more verbose than necessary since the
report_info can be used to see which one failed, so combine them.
Finally, return the pass/fail result of the test in case a caller
wants to use it.
Tested-by: Clément Léger <cleger@rivosinc.com>
Reviewed-by: Clément Léger <cleger@rivosinc.com>
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi.c | 45 ++++++++++++++++++++++++---------------------
1 file changed, 24 insertions(+), 21 deletions(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 6f4ddaf13df3..9f591f8ff76a 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -156,19 +156,22 @@ static bool get_invalid_addr(phys_addr_t *paddr, bool allow_default)
return false;
}
-static void gen_report(struct sbiret *ret,
- long expected_error, long expected_value)
-{
- bool check_error = ret->error == expected_error;
- bool check_value = ret->value == expected_value;
-
- if (!check_error || !check_value)
- report_info("expected (error: %ld, value: %ld), received: (error: %ld, value %ld)",
- expected_error, expected_value, ret->error, ret->value);
-
- report(check_error, "expected sbi.error");
- report(check_value, "expected sbi.value");
-}
+#define sbiret_report(ret, expected_error, expected_value, fmt, ...) ({ \
+ long ex_err = expected_error; \
+ long ex_val = expected_value; \
+ bool ch_err = (ret)->error == ex_err; \
+ bool ch_val = (ret)->value == ex_val; \
+ bool pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \
+ \
+ if (!pass) \
+ report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)", \
+ ##__VA_ARGS__, ex_err, ex_val, (ret)->error, (ret)->value); \
+ \
+ pass; \
+})
+
+#define sbiret_check(ret, expected_error, expected_value) \
+ sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
static void check_base(void)
{
@@ -184,7 +187,7 @@ static void check_base(void)
expected = (long)strtoul(getenv("SBI_SPEC_VERSION"), NULL, 0);
assert_msg(!(expected & BIT(31)), "SBI spec version bit 31 must be zero");
assert_msg(__riscv_xlen == 32 || !(expected >> 32), "SBI spec version bits greater than 31 must be zero");
- gen_report(&ret, 0, expected);
+ sbiret_check(&ret, 0, expected);
}
report_prefix_pop();
@@ -199,7 +202,7 @@ static void check_base(void)
if (env_or_skip("SBI_IMPL_ID")) {
expected = (long)strtoul(getenv("SBI_IMPL_ID"), NULL, 0);
ret = sbi_base(SBI_EXT_BASE_GET_IMP_ID, 0);
- gen_report(&ret, 0, expected);
+ sbiret_check(&ret, 0, expected);
}
report_prefix_pop();
@@ -207,17 +210,17 @@ static void check_base(void)
if (env_or_skip("SBI_IMPL_VERSION")) {
expected = (long)strtoul(getenv("SBI_IMPL_VERSION"), NULL, 0);
ret = sbi_base(SBI_EXT_BASE_GET_IMP_VERSION, 0);
- gen_report(&ret, 0, expected);
+ sbiret_check(&ret, 0, expected);
}
report_prefix_pop();
report_prefix_push("probe_ext");
expected = getenv("SBI_PROBE_EXT") ? (long)strtoul(getenv("SBI_PROBE_EXT"), NULL, 0) : 1;
ret = sbi_base(SBI_EXT_BASE_PROBE_EXT, SBI_EXT_BASE);
- gen_report(&ret, 0, expected);
+ sbiret_check(&ret, 0, expected);
report_prefix_push("unavailable");
ret = sbi_base(SBI_EXT_BASE_PROBE_EXT, 0xb000000);
- gen_report(&ret, 0, 0);
+ sbiret_check(&ret, 0, 0);
report_prefix_popn(2);
report_prefix_push("mvendorid");
@@ -225,7 +228,7 @@ static void check_base(void)
expected = (long)strtoul(getenv("MVENDORID"), NULL, 0);
assert(__riscv_xlen == 32 || !(expected >> 32));
ret = sbi_base(SBI_EXT_BASE_GET_MVENDORID, 0);
- gen_report(&ret, 0, expected);
+ sbiret_check(&ret, 0, expected);
}
report_prefix_pop();
@@ -233,7 +236,7 @@ static void check_base(void)
if (env_or_skip("MARCHID")) {
expected = (long)strtoul(getenv("MARCHID"), NULL, 0);
ret = sbi_base(SBI_EXT_BASE_GET_MARCHID, 0);
- gen_report(&ret, 0, expected);
+ sbiret_check(&ret, 0, expected);
}
report_prefix_pop();
@@ -241,7 +244,7 @@ static void check_base(void)
if (env_or_skip("MIMPID")) {
expected = (long)strtoul(getenv("MIMPID"), NULL, 0);
ret = sbi_base(SBI_EXT_BASE_GET_MIMPID, 0);
- gen_report(&ret, 0, expected);
+ sbiret_check(&ret, 0, expected);
}
report_prefix_popn(2);
}
--
2.48.1
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply related [flat|nested] 6+ messages in thread* [kvm-unit-tests PATCH v2 2/3] riscv: sbi: Export sbiret_report/check
2025-02-18 18:54 [kvm-unit-tests PATCH v2 0/3] riscv: sbi: Provide sbiret_report/check Andrew Jones
2025-02-18 18:54 ` [kvm-unit-tests PATCH v2 1/3] riscv: sbi: Improve gen_report Andrew Jones
@ 2025-02-18 18:54 ` Andrew Jones
2025-02-18 18:54 ` [kvm-unit-tests PATCH v2 3/3] riscv: sbi: Add sbiret_report_error Andrew Jones
2025-03-04 9:32 ` [kvm-unit-tests PATCH v2 0/3] riscv: sbi: Provide sbiret_report/check Andrew Jones
3 siblings, 0 replies; 6+ messages in thread
From: Andrew Jones @ 2025-02-18 18:54 UTC (permalink / raw)
To: kvm-riscv; +Cc: cleger, atishp
As more SBI tests are written not all will be in riscv/sbi.c. Export
sbiret reporting functions to be used in other SBI test files.
Reviewed-by: Clément Léger <cleger@rivosinc.com>
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi-tests.h | 21 +++++++++++++++++++++
riscv/sbi.c | 17 -----------------
2 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
index ce129968fe99..2bdc9440d82d 100644
--- a/riscv/sbi-tests.h
+++ b/riscv/sbi-tests.h
@@ -33,4 +33,25 @@
#define SBI_SUSP_TEST_HARTID (1 << 2)
#define SBI_SUSP_TEST_MASK 7
+#ifndef __ASSEMBLY__
+#include <asm/sbi.h>
+
+#define sbiret_report(ret, expected_error, expected_value, fmt, ...) ({ \
+ long ex_err = expected_error; \
+ long ex_val = expected_value; \
+ bool ch_err = (ret)->error == ex_err; \
+ bool ch_val = (ret)->value == ex_val; \
+ bool pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \
+ \
+ if (!pass) \
+ report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)", \
+ ##__VA_ARGS__, ex_err, ex_val, (ret)->error, (ret)->value); \
+ \
+ pass; \
+})
+
+#define sbiret_check(ret, expected_error, expected_value) \
+ sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
+
+#endif /* __ASSEMBLY__ */
#endif /* _RISCV_SBI_TESTS_H_ */
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 9f591f8ff76a..3eca8c7e35ed 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -156,23 +156,6 @@ static bool get_invalid_addr(phys_addr_t *paddr, bool allow_default)
return false;
}
-#define sbiret_report(ret, expected_error, expected_value, fmt, ...) ({ \
- long ex_err = expected_error; \
- long ex_val = expected_value; \
- bool ch_err = (ret)->error == ex_err; \
- bool ch_val = (ret)->value == ex_val; \
- bool pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \
- \
- if (!pass) \
- report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)", \
- ##__VA_ARGS__, ex_err, ex_val, (ret)->error, (ret)->value); \
- \
- pass; \
-})
-
-#define sbiret_check(ret, expected_error, expected_value) \
- sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
-
static void check_base(void)
{
struct sbiret ret;
--
2.48.1
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply related [flat|nested] 6+ messages in thread* [kvm-unit-tests PATCH v2 3/3] riscv: sbi: Add sbiret_report_error
2025-02-18 18:54 [kvm-unit-tests PATCH v2 0/3] riscv: sbi: Provide sbiret_report/check Andrew Jones
2025-02-18 18:54 ` [kvm-unit-tests PATCH v2 1/3] riscv: sbi: Improve gen_report Andrew Jones
2025-02-18 18:54 ` [kvm-unit-tests PATCH v2 2/3] riscv: sbi: Export sbiret_report/check Andrew Jones
@ 2025-02-18 18:54 ` Andrew Jones
2025-02-25 10:10 ` Clément Léger
2025-03-04 9:32 ` [kvm-unit-tests PATCH v2 0/3] riscv: sbi: Provide sbiret_report/check Andrew Jones
3 siblings, 1 reply; 6+ messages in thread
From: Andrew Jones @ 2025-02-18 18:54 UTC (permalink / raw)
To: kvm-riscv; +Cc: cleger, atishp
An sbiret value may be unspecified in the case of errors, and even in
the case of success when the function doesn't return anything.
Provide an sbiret report function that only checks sbiret.error.
sbiret_report_error() has a different PASS format than
sbiret_report(). The former will always output the stringized
expected error so the format string doesn't need to contain it, e.g.
PASS: sbi: test foo: SBI_SUCCESS
PASS: sbi: test event id 0x0: SBI_INVALID_PARAM
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi-tests.h | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
index 2bdc9440d82d..7a24e083a7b8 100644
--- a/riscv/sbi-tests.h
+++ b/riscv/sbi-tests.h
@@ -36,20 +36,35 @@
#ifndef __ASSEMBLY__
#include <asm/sbi.h>
-#define sbiret_report(ret, expected_error, expected_value, fmt, ...) ({ \
+#define __sbiret_report(ret, expected_error, expected_value, has_value, expected_error_name, fmt, ...) ({ \
long ex_err = expected_error; \
long ex_val = expected_value; \
+ bool has_val = !!(has_value); \
bool ch_err = (ret)->error == ex_err; \
bool ch_val = (ret)->value == ex_val; \
- bool pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \
+ bool pass; \
\
- if (!pass) \
+ if (has_val) \
+ pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \
+ else \
+ pass = report(ch_err, fmt ": %s", ##__VA_ARGS__, expected_error_name); \
+ \
+ if (!pass && has_val) \
report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)", \
##__VA_ARGS__, ex_err, ex_val, (ret)->error, (ret)->value); \
+ else if (!pass) \
+ report_info(fmt ": %s (%ld): received error %ld", \
+ ##__VA_ARGS__, expected_error_name, ex_err, (ret)->error); \
\
pass; \
})
+#define sbiret_report(ret, expected_error, expected_value, ...) \
+ __sbiret_report(ret, expected_error, expected_value, true, #expected_error, __VA_ARGS__)
+
+#define sbiret_report_error(ret, expected_error, ...) \
+ __sbiret_report(ret, expected_error, 0, false, #expected_error, __VA_ARGS__)
+
#define sbiret_check(ret, expected_error, expected_value) \
sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
--
2.48.1
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [kvm-unit-tests PATCH v2 3/3] riscv: sbi: Add sbiret_report_error
2025-02-18 18:54 ` [kvm-unit-tests PATCH v2 3/3] riscv: sbi: Add sbiret_report_error Andrew Jones
@ 2025-02-25 10:10 ` Clément Léger
0 siblings, 0 replies; 6+ messages in thread
From: Clément Léger @ 2025-02-25 10:10 UTC (permalink / raw)
To: Andrew Jones, kvm-riscv; +Cc: atishp
On 18/02/2025 19:54, Andrew Jones wrote:
> An sbiret value may be unspecified in the case of errors, and even in
> the case of success when the function doesn't return anything.
> Provide an sbiret report function that only checks sbiret.error.
>
> sbiret_report_error() has a different PASS format than
> sbiret_report(). The former will always output the stringized
> expected error so the format string doesn't need to contain it, e.g.
>
> PASS: sbi: test foo: SBI_SUCCESS
> PASS: sbi: test event id 0x0: SBI_INVALID_PARAM
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
> riscv/sbi-tests.h | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
> index 2bdc9440d82d..7a24e083a7b8 100644
> --- a/riscv/sbi-tests.h
> +++ b/riscv/sbi-tests.h
> @@ -36,20 +36,35 @@
> #ifndef __ASSEMBLY__
> #include <asm/sbi.h>
>
> -#define sbiret_report(ret, expected_error, expected_value, fmt, ...) ({ \
> +#define __sbiret_report(ret, expected_error, expected_value, has_value, expected_error_name, fmt, ...) ({ \
> long ex_err = expected_error; \
> long ex_val = expected_value; \
> + bool has_val = !!(has_value); \
> bool ch_err = (ret)->error == ex_err; \
> bool ch_val = (ret)->value == ex_val; \
> - bool pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \
> + bool pass; \
> \
> - if (!pass) \
> + if (has_val) \
> + pass = report(ch_err && ch_val, fmt, ##__VA_ARGS__); \
> + else \
> + pass = report(ch_err, fmt ": %s", ##__VA_ARGS__, expected_error_name); \
> + \
> + if (!pass && has_val) \
> report_info(fmt ": expected (error: %ld, value: %ld), received: (error: %ld, value %ld)", \
> ##__VA_ARGS__, ex_err, ex_val, (ret)->error, (ret)->value); \
> + else if (!pass) \
> + report_info(fmt ": %s (%ld): received error %ld", \
> + ##__VA_ARGS__, expected_error_name, ex_err, (ret)->error); \
> \
> pass; \
> })
>
> +#define sbiret_report(ret, expected_error, expected_value, ...) \
> + __sbiret_report(ret, expected_error, expected_value, true, #expected_error, __VA_ARGS__)
> +
> +#define sbiret_report_error(ret, expected_error, ...) \
> + __sbiret_report(ret, expected_error, 0, false, #expected_error, __VA_ARGS__)
> +
> #define sbiret_check(ret, expected_error, expected_value) \
> sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
>
Hey Andrew,
Tested-by: Clément Léger <cleger@rivosinc.com>
Reviewed-by: Clément Léger <cleger@rivosinc.com>
Thanks,
Clément
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v2 0/3] riscv: sbi: Provide sbiret_report/check
2025-02-18 18:54 [kvm-unit-tests PATCH v2 0/3] riscv: sbi: Provide sbiret_report/check Andrew Jones
` (2 preceding siblings ...)
2025-02-18 18:54 ` [kvm-unit-tests PATCH v2 3/3] riscv: sbi: Add sbiret_report_error Andrew Jones
@ 2025-03-04 9:32 ` Andrew Jones
3 siblings, 0 replies; 6+ messages in thread
From: Andrew Jones @ 2025-03-04 9:32 UTC (permalink / raw)
To: kvm-riscv; +Cc: cleger, atishp
On Tue, Feb 18, 2025 at 07:54:01PM +0100, Andrew Jones wrote:
> It's commom for SBI tests to expect a particular error return (or no
> error, which case it expects the returned error to be SBI_SUCCESS).
> When we don't get the expected error it'd be nice to output what we
> did get. gen_report() in the SBI tests were doing that for the BASE
> tests, improve it and export it to be used by all SBI tests.
>
> v2:
> - Output expected error string with sbiret_report_error() [Clément]
> - Picked up Clément's tags except for patch3 since it changed a
> decent amount.
>
> Andrew Jones (3):
> riscv: sbi: Improve gen_report
> riscv: sbi: Export sbiret_report/check
> riscv: sbi: Add sbiret_report_error
>
> riscv/sbi-tests.h | 36 ++++++++++++++++++++++++++++++++++++
> riscv/sbi.c | 30 ++++++++----------------------
> 2 files changed, 44 insertions(+), 22 deletions(-)
>
> --
> 2.48.1
>
Merged.
Thanks,
drew
--
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread