* [PATCH 1/6] riscv: Add a wrapper to call sbi_ecall for base extension
2024-03-13 14:53 [PATCH V3 0/6] Add riscv tests to cover the base extension specs cem
@ 2024-03-13 14:53 ` cem
2024-03-13 19:24 ` Andrew Jones
2024-03-13 14:53 ` [PATCH 2/6] riscv: Add test to probe SBI Extension cem
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: cem @ 2024-03-13 14:53 UTC (permalink / raw)
To: kvm-riscv
From: Carlos Maiolino <cem@kernel.org>
All SBI extension functions accepts at most one argument, so create a
wrapper around sbi_ecall() to avoid needing to pass in arguments 1 to 5
all the time, also, the wrapper can specify SBI_EXT_BASE directly.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V3:
- Move to the beginning of the series
- Don't mark __base_sbi_ecall() as inline
V2:
- This patch was introduced in V2
riscv/sbi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index ffb07a25..48d0b06e 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -14,7 +14,13 @@ static void help(void)
puts("An environ must be provided where expected values are given.\n");
}
+static struct sbiret __base_sbi_ecall(int fid, unsigned long arg0)
+{
+ return sbi_ecall(SBI_EXT_BASE, fid, arg0, 0, 0, 0, 0, 0);
+}
+
int main(int argc, char **argv)
+
{
struct sbiret ret;
long expected;
@@ -32,7 +38,7 @@ int main(int argc, char **argv)
}
expected = strtol(getenv("MVENDORID"), NULL, 0);
- ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_MVENDORID, 0, 0, 0, 0, 0, 0);
+ ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
report(!ret.error, "mvendorid: no error");
report(ret.value == expected, "mvendorid");
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 1/6] riscv: Add a wrapper to call sbi_ecall for base extension
2024-03-13 14:53 ` [PATCH 1/6] riscv: Add a wrapper to call sbi_ecall for base extension cem
@ 2024-03-13 19:24 ` Andrew Jones
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2024-03-13 19:24 UTC (permalink / raw)
To: kvm-riscv
On Wed, Mar 13, 2024 at 03:53:24PM +0100, cem at kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
>
> All SBI extension functions accepts at most one argument, so create a
> wrapper around sbi_ecall() to avoid needing to pass in arguments 1 to 5
> all the time, also, the wrapper can specify SBI_EXT_BASE directly.
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> V3:
> - Move to the beginning of the series
> - Don't mark __base_sbi_ecall() as inline
> V2:
> - This patch was introduced in V2
>
> riscv/sbi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index ffb07a25..48d0b06e 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -14,7 +14,13 @@ static void help(void)
> puts("An environ must be provided where expected values are given.\n");
> }
>
> +static struct sbiret __base_sbi_ecall(int fid, unsigned long arg0)
> +{
> + return sbi_ecall(SBI_EXT_BASE, fid, arg0, 0, 0, 0, 0, 0);
> +}
> +
> int main(int argc, char **argv)
> +
stray blank line
> {
> struct sbiret ret;
> long expected;
> @@ -32,7 +38,7 @@ int main(int argc, char **argv)
> }
> expected = strtol(getenv("MVENDORID"), NULL, 0);
>
> - ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_MVENDORID, 0, 0, 0, 0, 0, 0);
> + ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
> report(!ret.error, "mvendorid: no error");
> report(ret.value == expected, "mvendorid");
>
> --
> 2.44.0
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/6] riscv: Add test to probe SBI Extension
2024-03-13 14:53 [PATCH V3 0/6] Add riscv tests to cover the base extension specs cem
2024-03-13 14:53 ` [PATCH 1/6] riscv: Add a wrapper to call sbi_ecall for base extension cem
@ 2024-03-13 14:53 ` cem
2024-03-13 20:02 ` Andrew Jones
2024-03-13 14:53 ` [PATCH 3/6] riscv: Factor out environment variable check and report generation cem
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: cem @ 2024-03-13 14:53 UTC (permalink / raw)
To: kvm-riscv
From: Carlos Maiolino <cem@kernel.org>
Factor out vendor id test to a new helper, and add a new test for
probing the SBI extension.
Compare the retrieved value against an environment variable, as the
implementation can return any non-zero value.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V3:
- Adapt patch to use __base_sbi_ecall
riscv/sbi.c | 44 ++++++++++++++++++++++++++++++++------------
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 48d0b06e..55bfcd42 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -19,29 +19,49 @@ static struct sbiret __base_sbi_ecall(int fid, unsigned long arg0)
return sbi_ecall(SBI_EXT_BASE, fid, arg0, 0, 0, 0, 0, 0);
}
-int main(int argc, char **argv)
-
+static void check_base(void)
{
struct sbiret ret;
long expected;
- if (argc > 1 && !strcmp(argv[1], "-h")) {
- help();
- exit(0);
- }
-
- report_prefix_push("sbi");
+ report_prefix_push("base");
if (!getenv("MVENDORID")) {
report_skip("mvendorid: missing MVENDORID environment variable");
- goto done;
+ return;
}
+
+ report_prefix_push("mvendorid");
expected = strtol(getenv("MVENDORID"), NULL, 0);
ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
- report(!ret.error, "mvendorid: no error");
- report(ret.value == expected, "mvendorid");
-done:
+ report(!ret.error, "no sbi.error");
+ report(ret.value == expected, "expected sbi.value");
+ report_prefix_pop();
+
+ report_prefix_push("probe_ext");
+ expected = getenv("PROBE_EXT") ? strtol(getenv("PROBE_EXT"), NULL, 0) : 1;
+
+ ret = __base_sbi_ecall(SBI_EXT_BASE_PROBE_EXT, 0);
+
+ report(!ret.error, "no sbi.error");
+ report(ret.value == expected, "expected sbi.value");
+ report_prefix_pop();
+
+ report_prefix_pop();
+}
+
+int main(int argc, char **argv)
+{
+
+ if (argc > 1 && !strcmp(argv[1], "-h")) {
+ help();
+ exit(0);
+ }
+
+ report_prefix_push("sbi");
+ check_base();
+
return report_summary();
}
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/6] riscv: Add test to probe SBI Extension
2024-03-13 14:53 ` [PATCH 2/6] riscv: Add test to probe SBI Extension cem
@ 2024-03-13 20:02 ` Andrew Jones
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2024-03-13 20:02 UTC (permalink / raw)
To: kvm-riscv
On Wed, Mar 13, 2024 at 03:53:25PM +0100, cem at kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
>
> Factor out vendor id test to a new helper, and add a new test for
> probing the SBI extension.
> Compare the retrieved value against an environment variable, as the
> implementation can return any non-zero value.
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> V3:
> - Adapt patch to use __base_sbi_ecall
>
> riscv/sbi.c | 44 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index 48d0b06e..55bfcd42 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -19,29 +19,49 @@ static struct sbiret __base_sbi_ecall(int fid, unsigned long arg0)
> return sbi_ecall(SBI_EXT_BASE, fid, arg0, 0, 0, 0, 0, 0);
> }
>
> -int main(int argc, char **argv)
> -
The last patch's stray blank line is removed here, so OK, but better to
not add it in the first place.
> +static void check_base(void)
> {
> struct sbiret ret;
> long expected;
>
> - if (argc > 1 && !strcmp(argv[1], "-h")) {
> - help();
> - exit(0);
> - }
> -
> - report_prefix_push("sbi");
> + report_prefix_push("base");
>
> if (!getenv("MVENDORID")) {
> report_skip("mvendorid: missing MVENDORID environment variable");
> - goto done;
> + return;
> }
> +
> + report_prefix_push("mvendorid");
> expected = strtol(getenv("MVENDORID"), NULL, 0);
>
> ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
> - report(!ret.error, "mvendorid: no error");
> - report(ret.value == expected, "mvendorid");
>
> -done:
> + report(!ret.error, "no sbi.error");
> + report(ret.value == expected, "expected sbi.value");
> + report_prefix_pop();
> +
> + report_prefix_push("probe_ext");
> + expected = getenv("PROBE_EXT") ? strtol(getenv("PROBE_EXT"), NULL, 0) : 1;
> +
> + ret = __base_sbi_ecall(SBI_EXT_BASE_PROBE_EXT, 0);
The arg shouldn't be zero for this test. Before, you probed for the base
extension itself.
> +
> + report(!ret.error, "no sbi.error");
> + report(ret.value == expected, "expected sbi.value");
> + report_prefix_pop();
> +
> + report_prefix_pop();
> +}
> +
> +int main(int argc, char **argv)
> +{
> +
> + if (argc > 1 && !strcmp(argv[1], "-h")) {
> + help();
> + exit(0);
> + }
> +
> + report_prefix_push("sbi");
> + check_base();
> +
> return report_summary();
> }
> --
> 2.44.0
>
Thanks,
drew
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/6] riscv: Factor out environment variable check and report generation
2024-03-13 14:53 [PATCH V3 0/6] Add riscv tests to cover the base extension specs cem
2024-03-13 14:53 ` [PATCH 1/6] riscv: Add a wrapper to call sbi_ecall for base extension cem
2024-03-13 14:53 ` [PATCH 2/6] riscv: Add test to probe SBI Extension cem
@ 2024-03-13 14:53 ` cem
2024-03-13 20:05 ` Andrew Jones
2024-03-13 14:53 ` [PATCH 4/6] riscv: Implement test for architecture ID register cem
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: cem @ 2024-03-13 14:53 UTC (permalink / raw)
To: kvm-riscv
From: Carlos Maiolino <cem@kernel.org>
We do check Environment variables and generate reports all the time,
so use a couple of helpers for that
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V3:
- Adapt to use __base_sbi_ecall()
riscv/sbi.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 55bfcd42..ce0be84f 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -19,6 +19,23 @@ static struct sbiret __base_sbi_ecall(int fid, unsigned long arg0)
return sbi_ecall(SBI_EXT_BASE, fid, arg0, 0, 0, 0, 0, 0);
}
+static bool env_or_skip(const char *env)
+{
+
+ if (!getenv(env)) {
+ report_skip("missing %s environment variable", env);
+ return false;
+ }
+
+ return true;
+}
+
+static void gen_report(struct sbiret *ret, long expected)
+{
+ report(!ret->error, "no sbi.error");
+ report(ret->value == expected, "expected sbi.value");
+}
+
static void check_base(void)
{
struct sbiret ret;
@@ -26,27 +43,18 @@ static void check_base(void)
report_prefix_push("base");
- if (!getenv("MVENDORID")) {
- report_skip("mvendorid: missing MVENDORID environment variable");
- return;
- }
-
report_prefix_push("mvendorid");
- expected = strtol(getenv("MVENDORID"), NULL, 0);
-
- ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
-
- report(!ret.error, "no sbi.error");
- report(ret.value == expected, "expected sbi.value");
+ if (env_or_skip("MVENDORID")) {
+ expected = strtol(getenv("MVENDORID"), NULL, 0);
+ ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
+ gen_report(&ret, expected);
+ }
report_prefix_pop();
report_prefix_push("probe_ext");
expected = getenv("PROBE_EXT") ? strtol(getenv("PROBE_EXT"), NULL, 0) : 1;
-
ret = __base_sbi_ecall(SBI_EXT_BASE_PROBE_EXT, 0);
-
- report(!ret.error, "no sbi.error");
- report(ret.value == expected, "expected sbi.value");
+ gen_report(&ret, expected);
report_prefix_pop();
report_prefix_pop();
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/6] riscv: Factor out environment variable check and report generation
2024-03-13 14:53 ` [PATCH 3/6] riscv: Factor out environment variable check and report generation cem
@ 2024-03-13 20:05 ` Andrew Jones
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2024-03-13 20:05 UTC (permalink / raw)
To: kvm-riscv
On Wed, Mar 13, 2024 at 03:53:26PM +0100, cem at kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
>
> We do check Environment variables and generate reports all the time,
> so use a couple of helpers for that
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> V3:
> - Adapt to use __base_sbi_ecall()
>
> riscv/sbi.c | 38 +++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index 55bfcd42..ce0be84f 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -19,6 +19,23 @@ static struct sbiret __base_sbi_ecall(int fid, unsigned long arg0)
> return sbi_ecall(SBI_EXT_BASE, fid, arg0, 0, 0, 0, 0, 0);
> }
>
> +static bool env_or_skip(const char *env)
> +{
> +
stray blank line here (might want to check your editor settings...)
> + if (!getenv(env)) {
> + report_skip("missing %s environment variable", env);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static void gen_report(struct sbiret *ret, long expected)
> +{
> + report(!ret->error, "no sbi.error");
> + report(ret->value == expected, "expected sbi.value");
> +}
> +
> static void check_base(void)
> {
> struct sbiret ret;
> @@ -26,27 +43,18 @@ static void check_base(void)
>
> report_prefix_push("base");
>
> - if (!getenv("MVENDORID")) {
> - report_skip("mvendorid: missing MVENDORID environment variable");
> - return;
> - }
> -
> report_prefix_push("mvendorid");
> - expected = strtol(getenv("MVENDORID"), NULL, 0);
> -
> - ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
> -
> - report(!ret.error, "no sbi.error");
> - report(ret.value == expected, "expected sbi.value");
> + if (env_or_skip("MVENDORID")) {
> + expected = strtol(getenv("MVENDORID"), NULL, 0);
> + ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
> + gen_report(&ret, expected);
> + }
> report_prefix_pop();
>
> report_prefix_push("probe_ext");
> expected = getenv("PROBE_EXT") ? strtol(getenv("PROBE_EXT"), NULL, 0) : 1;
> -
> ret = __base_sbi_ecall(SBI_EXT_BASE_PROBE_EXT, 0);
> -
> - report(!ret.error, "no sbi.error");
> - report(ret.value == expected, "expected sbi.value");
> + gen_report(&ret, expected);
> report_prefix_pop();
>
> report_prefix_pop();
> --
> 2.44.0
>
With the fixups it'll need after fixing the last patch and the removal of
the blank line,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/6] riscv: Implement test for architecture ID register
2024-03-13 14:53 [PATCH V3 0/6] Add riscv tests to cover the base extension specs cem
` (2 preceding siblings ...)
2024-03-13 14:53 ` [PATCH 3/6] riscv: Factor out environment variable check and report generation cem
@ 2024-03-13 14:53 ` cem
2024-03-13 20:06 ` Andrew Jones
2024-03-13 14:53 ` [PATCH 5/6] riscv: Enable gen_report() to print the wrong value in case of failure cem
2024-03-13 14:53 ` [PATCH 6/6] riscv: Test for specific SBI implementation ID cem
5 siblings, 1 reply; 15+ messages in thread
From: cem @ 2024-03-13 14:53 UTC (permalink / raw)
To: kvm-riscv
From: Carlos Maiolino <cem@kernel.org>
Probe the MARCHID register and compare it to the specified MARCHID
environment variable.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V3:
- Adapt to use __base_sbi_ecall (which also fixes the wrong paramenters)
V2:
- Wrap commit message around 70 chars
- Remove unneeded blank lines
riscv/sbi.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index ce0be84f..56b27765 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -57,6 +57,14 @@ static void check_base(void)
gen_report(&ret, expected);
report_prefix_pop();
+ report_prefix_push("marchid");
+ if (env_or_skip("MARCHID")) {
+ expected = strtol(getenv("MARCHID"), NULL, 0);
+ ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MARCHID, 0);
+ gen_report(&ret, expected);
+ }
+ report_prefix_pop();
+
report_prefix_pop();
}
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/6] riscv: Implement test for architecture ID register
2024-03-13 14:53 ` [PATCH 4/6] riscv: Implement test for architecture ID register cem
@ 2024-03-13 20:06 ` Andrew Jones
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2024-03-13 20:06 UTC (permalink / raw)
To: kvm-riscv
On Wed, Mar 13, 2024 at 03:53:27PM +0100, cem at kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
>
> Probe the MARCHID register and compare it to the specified MARCHID
> environment variable.
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> V3:
> - Adapt to use __base_sbi_ecall (which also fixes the wrong paramenters)
> V2:
> - Wrap commit message around 70 chars
> - Remove unneeded blank lines
>
> riscv/sbi.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index ce0be84f..56b27765 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -57,6 +57,14 @@ static void check_base(void)
> gen_report(&ret, expected);
> report_prefix_pop();
>
> + report_prefix_push("marchid");
> + if (env_or_skip("MARCHID")) {
> + expected = strtol(getenv("MARCHID"), NULL, 0);
> + ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MARCHID, 0);
> + gen_report(&ret, expected);
> + }
> + report_prefix_pop();
> +
> report_prefix_pop();
> }
>
> --
> 2.44.0
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/6] riscv: Enable gen_report() to print the wrong value in case of failure
2024-03-13 14:53 [PATCH V3 0/6] Add riscv tests to cover the base extension specs cem
` (3 preceding siblings ...)
2024-03-13 14:53 ` [PATCH 4/6] riscv: Implement test for architecture ID register cem
@ 2024-03-13 14:53 ` cem
2024-03-13 20:07 ` Andrew Jones
2024-03-13 14:53 ` [PATCH 6/6] riscv: Test for specific SBI implementation ID cem
5 siblings, 1 reply; 15+ messages in thread
From: cem @ 2024-03-13 14:53 UTC (permalink / raw)
To: kvm-riscv
From: Carlos Maiolino <cem@kernel.org>
If the test fails because the expected value doesn't match, it's
useful to know what value was actually printed.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V3:
- Include expected error check in gen_report()
- Implement Drew's suggestion on cleaning up the function a bit
- Update patch to use __base_sbi_ecall()
V2:
- Reduce subject to fit 70 chars
- Use report_info() to output expected vs received values,
leaving only necessary information for parsers on report()
riscv/sbi.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 56b27765..a4a16325 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -30,10 +30,18 @@ static bool env_or_skip(const char *env)
return true;
}
-static void gen_report(struct sbiret *ret, long expected)
+static void gen_report(struct sbiret *ret,
+ long expected_error, long expected_value)
{
- report(!ret->error, "no sbi.error");
- report(ret->value == expected, "expected sbi.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)\n",
+ expected_error, expected_value, ret->error, ret->value);
+
+ report(check_error, "expected sbi.error");
+ report(check_value, "expected sbi.value");
}
static void check_base(void)
@@ -47,21 +55,21 @@ static void check_base(void)
if (env_or_skip("MVENDORID")) {
expected = strtol(getenv("MVENDORID"), NULL, 0);
ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
- gen_report(&ret, expected);
+ gen_report(&ret, 0, expected);
}
report_prefix_pop();
report_prefix_push("probe_ext");
expected = getenv("PROBE_EXT") ? strtol(getenv("PROBE_EXT"), NULL, 0) : 1;
ret = __base_sbi_ecall(SBI_EXT_BASE_PROBE_EXT, 0);
- gen_report(&ret, expected);
+ gen_report(&ret, 0, expected);
report_prefix_pop();
report_prefix_push("marchid");
if (env_or_skip("MARCHID")) {
expected = strtol(getenv("MARCHID"), NULL, 0);
ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MARCHID, 0);
- gen_report(&ret, expected);
+ gen_report(&ret, 0, expected);
}
report_prefix_pop();
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/6] riscv: Enable gen_report() to print the wrong value in case of failure
2024-03-13 14:53 ` [PATCH 5/6] riscv: Enable gen_report() to print the wrong value in case of failure cem
@ 2024-03-13 20:07 ` Andrew Jones
0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2024-03-13 20:07 UTC (permalink / raw)
To: kvm-riscv
On Wed, Mar 13, 2024 at 03:53:28PM +0100, cem at kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
>
> If the test fails because the expected value doesn't match, it's
> useful to know what value was actually printed.
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> V3:
> - Include expected error check in gen_report()
> - Implement Drew's suggestion on cleaning up the function a bit
> - Update patch to use __base_sbi_ecall()
> V2:
> - Reduce subject to fit 70 chars
> - Use report_info() to output expected vs received values,
> leaving only necessary information for parsers on report()
>
> riscv/sbi.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index 56b27765..a4a16325 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -30,10 +30,18 @@ static bool env_or_skip(const char *env)
> return true;
> }
>
> -static void gen_report(struct sbiret *ret, long expected)
> +static void gen_report(struct sbiret *ret,
> + long expected_error, long expected_value)
> {
> - report(!ret->error, "no sbi.error");
> - report(ret->value == expected, "expected sbi.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)\n",
> + expected_error, expected_value, ret->error, ret->value);
> +
> + report(check_error, "expected sbi.error");
> + report(check_value, "expected sbi.value");
> }
>
> static void check_base(void)
> @@ -47,21 +55,21 @@ static void check_base(void)
> if (env_or_skip("MVENDORID")) {
> expected = strtol(getenv("MVENDORID"), NULL, 0);
> ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MVENDORID, 0);
> - gen_report(&ret, expected);
> + gen_report(&ret, 0, expected);
> }
> report_prefix_pop();
>
> report_prefix_push("probe_ext");
> expected = getenv("PROBE_EXT") ? strtol(getenv("PROBE_EXT"), NULL, 0) : 1;
> ret = __base_sbi_ecall(SBI_EXT_BASE_PROBE_EXT, 0);
> - gen_report(&ret, expected);
> + gen_report(&ret, 0, expected);
> report_prefix_pop();
>
> report_prefix_push("marchid");
> if (env_or_skip("MARCHID")) {
> expected = strtol(getenv("MARCHID"), NULL, 0);
> ret = __base_sbi_ecall(SBI_EXT_BASE_GET_MARCHID, 0);
> - gen_report(&ret, expected);
> + gen_report(&ret, 0, expected);
> }
> report_prefix_pop();
>
> --
> 2.44.0
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/6] riscv: Test for specific SBI implementation ID
2024-03-13 14:53 [PATCH V3 0/6] Add riscv tests to cover the base extension specs cem
` (4 preceding siblings ...)
2024-03-13 14:53 ` [PATCH 5/6] riscv: Enable gen_report() to print the wrong value in case of failure cem
@ 2024-03-13 14:53 ` cem
2024-03-13 20:08 ` Andrew Jones
5 siblings, 1 reply; 15+ messages in thread
From: cem @ 2024-03-13 14:53 UTC (permalink / raw)
To: kvm-riscv
From: Carlos Maiolino <cem@kernel.org>
Retrieve the ID from the SBI, and test it against IMPL_ID
enviroment variable.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V3:
- Update to use __base_sbi_ecall (also fixes the correct paramenters)
- Rename env var to IMPL_ID to match the other tests
V2:
- Update commit description to fit 70 chars
- Move sbi_ecall() after expected assignment to make consistent with
other tests
riscv/sbi.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index a4a16325..8a624655 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -59,6 +59,14 @@ static void check_base(void)
}
report_prefix_pop();
+ report_prefix_push("impl_id");
+ if (env_or_skip("IMPL_ID")) {
+ expected = strtol(getenv("IMPL_ID"), NULL, 0);
+ ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_IMP_ID, 0, 0, 0, 0, 0, 0);
+ gen_report(&ret, 0, expected);
+ }
+ report_prefix_pop();
+
report_prefix_push("probe_ext");
expected = getenv("PROBE_EXT") ? strtol(getenv("PROBE_EXT"), NULL, 0) : 1;
ret = __base_sbi_ecall(SBI_EXT_BASE_PROBE_EXT, 0);
--
2.44.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 6/6] riscv: Test for specific SBI implementation ID
2024-03-13 14:53 ` [PATCH 6/6] riscv: Test for specific SBI implementation ID cem
@ 2024-03-13 20:08 ` Andrew Jones
2024-03-13 21:00 ` Carlos Maiolino
0 siblings, 1 reply; 15+ messages in thread
From: Andrew Jones @ 2024-03-13 20:08 UTC (permalink / raw)
To: kvm-riscv
On Wed, Mar 13, 2024 at 03:53:29PM +0100, cem at kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
>
> Retrieve the ID from the SBI, and test it against IMPL_ID
> enviroment variable.
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> V3:
> - Update to use __base_sbi_ecall (also fixes the correct paramenters)
> - Rename env var to IMPL_ID to match the other tests
> V2:
> - Update commit description to fit 70 chars
> - Move sbi_ecall() after expected assignment to make consistent with
> other tests
>
> riscv/sbi.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index a4a16325..8a624655 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -59,6 +59,14 @@ static void check_base(void)
> }
> report_prefix_pop();
>
> + report_prefix_push("impl_id");
> + if (env_or_skip("IMPL_ID")) {
> + expected = strtol(getenv("IMPL_ID"), NULL, 0);
> + ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_IMP_ID, 0, 0, 0, 0, 0, 0);
commit message says you switched to __base_sbi_ecall(), but you didn't :-)
> + gen_report(&ret, 0, expected);
> + }
> + report_prefix_pop();
> +
> report_prefix_push("probe_ext");
> expected = getenv("PROBE_EXT") ? strtol(getenv("PROBE_EXT"), NULL, 0) : 1;
> ret = __base_sbi_ecall(SBI_EXT_BASE_PROBE_EXT, 0);
> --
> 2.44.0
>
Thanks,
drew
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 6/6] riscv: Test for specific SBI implementation ID
2024-03-13 20:08 ` Andrew Jones
@ 2024-03-13 21:00 ` Carlos Maiolino
0 siblings, 0 replies; 15+ messages in thread
From: Carlos Maiolino @ 2024-03-13 21:00 UTC (permalink / raw)
To: kvm-riscv
On Wed, Mar 13, 2024 at 09:08:54PM +0100, Andrew Jones wrote:
> On Wed, Mar 13, 2024 at 03:53:29PM +0100, cem at kernel.org wrote:
> > From: Carlos Maiolino <cem@kernel.org>
> >
> > Retrieve the ID from the SBI, and test it against IMPL_ID
> > enviroment variable.
> >
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> > V3:
> > - Update to use __base_sbi_ecall (also fixes the correct paramenters)
> > - Rename env var to IMPL_ID to match the other tests
> > V2:
> > - Update commit description to fit 70 chars
> > - Move sbi_ecall() after expected assignment to make consistent with
> > other tests
> >
> > riscv/sbi.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/riscv/sbi.c b/riscv/sbi.c
> > index a4a16325..8a624655 100644
> > --- a/riscv/sbi.c
> > +++ b/riscv/sbi.c
> > @@ -59,6 +59,14 @@ static void check_base(void)
> > }
> > report_prefix_pop();
> >
> > + report_prefix_push("impl_id");
> > + if (env_or_skip("IMPL_ID")) {
> > + expected = strtol(getenv("IMPL_ID"), NULL, 0);
> > + ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_IMP_ID, 0, 0, 0, 0, 0, 0);
>
> commit message says you switched to __base_sbi_ecall(), but you didn't :-)
Sigh... my apologies, had a short time to update these patches and overlooked it, I'll fix it and
send a decent version tonight.
Thanks for the reviews
Carlos
>
> > + gen_report(&ret, 0, expected);
> > + }
> > + report_prefix_pop();
> > +
> > report_prefix_push("probe_ext");
> > expected = getenv("PROBE_EXT") ? strtol(getenv("PROBE_EXT"), NULL, 0) : 1;
> > ret = __base_sbi_ecall(SBI_EXT_BASE_PROBE_EXT, 0);
> > --
> > 2.44.0
> >
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 15+ messages in thread