* [kvm-unit-tests PATCH v2 01/11] riscv: sbi: Drop fwft upper bits test
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
@ 2025-02-27 14:19 ` Andrew Jones
2025-02-27 14:21 ` Clément Léger
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 02/11] riscv: sbi: Add fwft pte_hw_ad_updating test Andrew Jones
` (10 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:19 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
The test should be checking for SBI_SUCCESS and that the behavior of
feature = BIT(32) is the same as feature = 0 (MISALIGNED_EXC_DELEG).
However, enabling MISALIGNED_EXC_DELEG doesn't always lead to traps
in S-mode (the platform may support misaligned accesses and not
trap at all). Drop this upper bits of feature ID test for now.
We'll add it back with another feature that has a deterministic
behavior (such as with the ADUE feature).
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi-fwft.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c
index f73ae52e7397..f3408d8de081 100644
--- a/riscv/sbi-fwft.c
+++ b/riscv/sbi-fwft.c
@@ -57,21 +57,6 @@ static void fwft_check_base(void)
fwft_check_reserved(SBI_FWFT_GLOBAL_RESERVED_START);
fwft_check_reserved(SBI_FWFT_GLOBAL_RESERVED_END);
-#if __riscv_xlen > 32
- /* Check id > 32 bits */
- {
- struct sbiret ret;
-
- ret = fwft_get_raw(BIT(32));
- sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM,
- "get feature with bit 32 set error");
-
- ret = fwft_set_raw(BIT(32), 0, 0);
- sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM,
- "set feature with bit 32 set error");
- }
-#endif
-
report_prefix_pop();
}
--
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] 17+ messages in thread* Re: [kvm-unit-tests PATCH v2 01/11] riscv: sbi: Drop fwft upper bits test
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 01/11] riscv: sbi: Drop fwft upper bits test Andrew Jones
@ 2025-02-27 14:21 ` Clément Léger
0 siblings, 0 replies; 17+ messages in thread
From: Clément Léger @ 2025-02-27 14:21 UTC (permalink / raw)
To: Andrew Jones, kvm-riscv; +Cc: atishp, jamestiotio
On 27/02/2025 15:19, Andrew Jones wrote:
> The test should be checking for SBI_SUCCESS and that the behavior of
> feature = BIT(32) is the same as feature = 0 (MISALIGNED_EXC_DELEG).
> However, enabling MISALIGNED_EXC_DELEG doesn't always lead to traps
> in S-mode (the platform may support misaligned accesses and not
> trap at all). Drop this upper bits of feature ID test for now.
> We'll add it back with another feature that has a deterministic
> behavior (such as with the ADUE feature).
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
> riscv/sbi-fwft.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c
> index f73ae52e7397..f3408d8de081 100644
> --- a/riscv/sbi-fwft.c
> +++ b/riscv/sbi-fwft.c
> @@ -57,21 +57,6 @@ static void fwft_check_base(void)
> fwft_check_reserved(SBI_FWFT_GLOBAL_RESERVED_START);
> fwft_check_reserved(SBI_FWFT_GLOBAL_RESERVED_END);
>
> -#if __riscv_xlen > 32
> - /* Check id > 32 bits */
> - {
> - struct sbiret ret;
> -
> - ret = fwft_get_raw(BIT(32));
> - sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM,
> - "get feature with bit 32 set error");
> -
> - ret = fwft_set_raw(BIT(32), 0, 0);
> - sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM,
> - "set feature with bit 32 set error");
> - }
> -#endif
> -
> report_prefix_pop();
> }
>
Hi Andrew,
Looks good to me.
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] 17+ messages in thread
* [kvm-unit-tests PATCH v2 02/11] riscv: sbi: Add fwft pte_hw_ad_updating test
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 01/11] riscv: sbi: Drop fwft upper bits test Andrew Jones
@ 2025-02-27 14:19 ` Andrew Jones
2025-02-27 14:30 ` Clément Léger
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 03/11] riscv: sbi: Ensure we have IPIs enabled for HSM suspend tests Andrew Jones
` (9 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:19 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
Add SBI FWFT tests for the PTE_HW_AD_UPDATING feature.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi-fwft.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 170 insertions(+)
diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c
index f3408d8de081..ca4d49b5db85 100644
--- a/riscv/sbi-fwft.c
+++ b/riscv/sbi-fwft.c
@@ -5,9 +5,12 @@
* Copyright (C) 2024, Rivos Inc., Clément Léger <cleger@rivosinc.com>
*/
#include <libcflat.h>
+#include <alloc.h>
#include <stdlib.h>
#include <asm/csr.h>
+#include <asm/io.h>
+#include <asm/mmu.h>
#include <asm/processor.h>
#include <asm/ptrace.h>
#include <asm/sbi.h>
@@ -37,6 +40,21 @@ static struct sbiret fwft_get(uint32_t feature)
return fwft_get_raw(feature);
}
+static struct sbiret fwft_set_and_check_raw(const char *str, unsigned long feature,
+ unsigned long value, unsigned long flags)
+{
+ struct sbiret ret;
+
+ ret = fwft_set_raw(feature, value, flags);
+ if (!sbiret_report_error(&ret, SBI_SUCCESS, "set to %ld%s", value, str))
+ return ret;
+
+ ret = fwft_get_raw(feature);
+ sbiret_report(&ret, SBI_SUCCESS, value, "get %ld after set%s", value, str);
+
+ return ret;
+}
+
static void fwft_check_reserved(unsigned long id)
{
struct sbiret ret;
@@ -160,6 +178,157 @@ static void fwft_check_misaligned_exc_deleg(void)
report_prefix_pop();
}
+static bool adue_triggered_read, adue_triggered_write;
+
+static void adue_set_ad(unsigned long addr, pteval_t prot)
+{
+ pte_t *ptep = get_pte(current_pgtable(), addr);
+ *ptep = __pte(pte_val(*ptep) | prot);
+ local_flush_tlb_page(addr);
+}
+
+static void adue_read_handler(struct pt_regs *regs)
+{
+ adue_triggered_read = true;
+ adue_set_ad(regs->badaddr, _PAGE_ACCESSED);
+}
+
+static void adue_write_handler(struct pt_regs *regs)
+{
+ adue_triggered_write = true;
+ adue_set_ad(regs->badaddr, _PAGE_ACCESSED | _PAGE_DIRTY);
+}
+
+static bool adue_check_pte(pteval_t pte, bool write)
+{
+ return (pte & (_PAGE_ACCESSED | _PAGE_DIRTY)) == (_PAGE_ACCESSED | (write ? _PAGE_DIRTY : 0));
+}
+
+static void adue_check(bool hw_updating_enabled, bool write)
+{
+ unsigned long *ptr = malloc(sizeof(unsigned long));
+ pte_t *ptep = get_pte(current_pgtable(), (uintptr_t)ptr);
+ bool *triggered;
+ const char *op;
+
+ WRITE_ONCE(adue_triggered_read, false);
+ WRITE_ONCE(adue_triggered_write, false);
+
+ *ptep = __pte(pte_val(*ptep) & ~(_PAGE_ACCESSED | _PAGE_DIRTY));
+ local_flush_tlb_page((uintptr_t)ptr);
+
+ if (write) {
+ op = "write";
+ triggered = &adue_triggered_write;
+ writel(0xdeadbeef, ptr);
+ } else {
+ op = "read";
+ triggered = &adue_triggered_read;
+ readl(ptr);
+ }
+
+ report(hw_updating_enabled != *triggered &&
+ adue_check_pte(pte_val(*ptep), write), "hw updating %s %s",
+ hw_updating_enabled ? "enabled" : "disabled", op);
+
+ free(ptr);
+}
+
+static bool adue_toggle_and_check_raw(const char *str, unsigned long feature, unsigned long value,
+ unsigned long flags)
+{
+ struct sbiret ret = fwft_set_and_check_raw(str, feature, value, flags);
+
+ if (!ret.error) {
+ adue_check(value, false);
+ adue_check(value, true);
+ return true;
+ }
+
+ return false;
+}
+
+static bool adue_toggle_and_check(const char *str, unsigned long value, unsigned long flags)
+{
+ return adue_toggle_and_check_raw(str, SBI_FWFT_PTE_AD_HW_UPDATING, value, flags);
+}
+
+static void fwft_check_pte_ad_hw_updating(void)
+{
+ struct sbiret ret;
+ bool enabled;
+
+ report_prefix_push("pte_ad_hw_updating");
+
+ ret = fwft_get(SBI_FWFT_PTE_AD_HW_UPDATING);
+ if (ret.error == SBI_ERR_NOT_SUPPORTED) {
+ report_skip("not supported by platform");
+ return;
+ } else if (!sbiret_report_error(&ret, SBI_SUCCESS, "get")) {
+ /* Not much we can do without a working get... */
+ return;
+ }
+
+ report(ret.value == 0 || ret.value == 1, "first get value is 0/1");
+
+ enabled = ret.value;
+ report_kfail(true, !enabled, "resets to 0");
+
+ install_exception_handler(EXC_LOAD_PAGE_FAULT, adue_read_handler);
+ install_exception_handler(EXC_STORE_PAGE_FAULT, adue_write_handler);
+
+ adue_check(enabled, false);
+ adue_check(enabled, true);
+
+ if (!adue_toggle_and_check("", !enabled, 0))
+ goto adue_inval_tests;
+ else
+ enabled = !enabled;
+
+ if (!adue_toggle_and_check(" again", !enabled, 0))
+ goto adue_inval_tests;
+ else
+ enabled = !enabled;
+
+#if __riscv_xlen > 32
+ if (!adue_toggle_and_check_raw(" with high feature bits set",
+ BIT(32) | SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 0))
+ goto adue_inval_tests;
+ else
+ enabled = !enabled;
+#endif
+
+adue_inval_tests:
+ ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, 2, 0);
+ sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM, "set to 2");
+
+ ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 2);
+ sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM, "set to %d with flags=2", !enabled);
+
+ if (!adue_toggle_and_check(" with lock", !enabled, 1))
+ goto adue_done;
+ else
+ enabled = !enabled;
+
+ ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 0);
+ sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d without lock", !enabled);
+
+ ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 1);
+ sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d with lock", !enabled);
+
+ ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, enabled, 0);
+ sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d without lock", enabled);
+
+ ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, enabled, 1);
+ sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d with lock", enabled);
+
+adue_done:
+ install_exception_handler(EXC_LOAD_PAGE_FAULT, NULL);
+ install_exception_handler(EXC_STORE_PAGE_FAULT, NULL);
+
+ report_prefix_pop();
+}
+
void check_fwft(void)
{
report_prefix_push("fwft");
@@ -172,6 +341,7 @@ void check_fwft(void)
fwft_check_base();
fwft_check_misaligned_exc_deleg();
+ fwft_check_pte_ad_hw_updating();
report_prefix_pop();
}
--
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] 17+ messages in thread* Re: [kvm-unit-tests PATCH v2 02/11] riscv: sbi: Add fwft pte_hw_ad_updating test
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 02/11] riscv: sbi: Add fwft pte_hw_ad_updating test Andrew Jones
@ 2025-02-27 14:30 ` Clément Léger
0 siblings, 0 replies; 17+ messages in thread
From: Clément Léger @ 2025-02-27 14:30 UTC (permalink / raw)
To: Andrew Jones, kvm-riscv; +Cc: atishp, jamestiotio
On 27/02/2025 15:19, Andrew Jones wrote:
> Add SBI FWFT tests for the PTE_HW_AD_UPDATING feature.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
> riscv/sbi-fwft.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 170 insertions(+)
>
> diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c
> index f3408d8de081..ca4d49b5db85 100644
> --- a/riscv/sbi-fwft.c
> +++ b/riscv/sbi-fwft.c
> @@ -5,9 +5,12 @@
> * Copyright (C) 2024, Rivos Inc., Clément Léger <cleger@rivosinc.com>
> */
> #include <libcflat.h>
> +#include <alloc.h>
> #include <stdlib.h>
>
> #include <asm/csr.h>
> +#include <asm/io.h>
> +#include <asm/mmu.h>
> #include <asm/processor.h>
> #include <asm/ptrace.h>
> #include <asm/sbi.h>
> @@ -37,6 +40,21 @@ static struct sbiret fwft_get(uint32_t feature)
> return fwft_get_raw(feature);
> }
>
> +static struct sbiret fwft_set_and_check_raw(const char *str, unsigned long feature,
> + unsigned long value, unsigned long flags)
> +{
> + struct sbiret ret;
> +
> + ret = fwft_set_raw(feature, value, flags);
> + if (!sbiret_report_error(&ret, SBI_SUCCESS, "set to %ld%s", value, str))
> + return ret;
> +
> + ret = fwft_get_raw(feature);
> + sbiret_report(&ret, SBI_SUCCESS, value, "get %ld after set%s", value, str);
> +
> + return ret;
> +}
> +
> static void fwft_check_reserved(unsigned long id)
> {
> struct sbiret ret;
> @@ -160,6 +178,157 @@ static void fwft_check_misaligned_exc_deleg(void)
> report_prefix_pop();
> }
>
> +static bool adue_triggered_read, adue_triggered_write;
> +
> +static void adue_set_ad(unsigned long addr, pteval_t prot)
> +{
> + pte_t *ptep = get_pte(current_pgtable(), addr);
> + *ptep = __pte(pte_val(*ptep) | prot);
> + local_flush_tlb_page(addr);
> +}
> +
> +static void adue_read_handler(struct pt_regs *regs)
> +{
> + adue_triggered_read = true;
> + adue_set_ad(regs->badaddr, _PAGE_ACCESSED);
> +}
> +
> +static void adue_write_handler(struct pt_regs *regs)
> +{
> + adue_triggered_write = true;
> + adue_set_ad(regs->badaddr, _PAGE_ACCESSED | _PAGE_DIRTY);
> +}
> +
> +static bool adue_check_pte(pteval_t pte, bool write)
> +{
> + return (pte & (_PAGE_ACCESSED | _PAGE_DIRTY)) == (_PAGE_ACCESSED | (write ? _PAGE_DIRTY : 0));
> +}
> +
> +static void adue_check(bool hw_updating_enabled, bool write)
> +{
> + unsigned long *ptr = malloc(sizeof(unsigned long));
> + pte_t *ptep = get_pte(current_pgtable(), (uintptr_t)ptr);
> + bool *triggered;
> + const char *op;
> +
> + WRITE_ONCE(adue_triggered_read, false);
> + WRITE_ONCE(adue_triggered_write, false);
> +
> + *ptep = __pte(pte_val(*ptep) & ~(_PAGE_ACCESSED | _PAGE_DIRTY));
> + local_flush_tlb_page((uintptr_t)ptr);
> +
> + if (write) {
> + op = "write";
> + triggered = &adue_triggered_write;
> + writel(0xdeadbeef, ptr);
> + } else {
> + op = "read";
> + triggered = &adue_triggered_read;
> + readl(ptr);
> + }
> +
> + report(hw_updating_enabled != *triggered &&
> + adue_check_pte(pte_val(*ptep), write), "hw updating %s %s",
> + hw_updating_enabled ? "enabled" : "disabled", op);
> +
> + free(ptr);
> +}
> +
> +static bool adue_toggle_and_check_raw(const char *str, unsigned long feature, unsigned long value,
> + unsigned long flags)
> +{
> + struct sbiret ret = fwft_set_and_check_raw(str, feature, value, flags);
> +
> + if (!ret.error) {
> + adue_check(value, false);
> + adue_check(value, true);
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static bool adue_toggle_and_check(const char *str, unsigned long value, unsigned long flags)
> +{
> + return adue_toggle_and_check_raw(str, SBI_FWFT_PTE_AD_HW_UPDATING, value, flags);
> +}
> +
> +static void fwft_check_pte_ad_hw_updating(void)
> +{
> + struct sbiret ret;
> + bool enabled;
> +
> + report_prefix_push("pte_ad_hw_updating");
> +
> + ret = fwft_get(SBI_FWFT_PTE_AD_HW_UPDATING);
> + if (ret.error == SBI_ERR_NOT_SUPPORTED) {
> + report_skip("not supported by platform");
> + return;
> + } else if (!sbiret_report_error(&ret, SBI_SUCCESS, "get")) {
> + /* Not much we can do without a working get... */
> + return;
> + }
> +
> + report(ret.value == 0 || ret.value == 1, "first get value is 0/1");
> +
> + enabled = ret.value;
> + report_kfail(true, !enabled, "resets to 0");
> +
> + install_exception_handler(EXC_LOAD_PAGE_FAULT, adue_read_handler);
> + install_exception_handler(EXC_STORE_PAGE_FAULT, adue_write_handler);
> +
> + adue_check(enabled, false);
> + adue_check(enabled, true);
> +
> + if (!adue_toggle_and_check("", !enabled, 0))
> + goto adue_inval_tests;
> + else
> + enabled = !enabled;
> +
> + if (!adue_toggle_and_check(" again", !enabled, 0))
> + goto adue_inval_tests;
> + else
> + enabled = !enabled;
> +
> +#if __riscv_xlen > 32
> + if (!adue_toggle_and_check_raw(" with high feature bits set",
> + BIT(32) | SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 0))
> + goto adue_inval_tests;
> + else
> + enabled = !enabled;
> +#endif
> +
> +adue_inval_tests:
> + ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, 2, 0);
> + sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM, "set to 2");
> +
> + ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 2);
> + sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM, "set to %d with flags=2", !enabled);
> +
> + if (!adue_toggle_and_check(" with lock", !enabled, 1))
> + goto adue_done;
> + else
> + enabled = !enabled;
> +
> + ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 0);
> + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d without lock", !enabled);
> +
> + ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 1);
> + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d with lock", !enabled);> +
> + ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, enabled, 0);
> + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d without lock", enabled);
> +
> + ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, enabled, 1);
> + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d with lock", enabled);
Hi Andrew,
As I said, this could probably be factorized in some function common to
all features since it will (probably) be done by other tests as well.
But that can be done at a later time as well.
> +
> +adue_done:
> + install_exception_handler(EXC_LOAD_PAGE_FAULT, NULL);
> + install_exception_handler(EXC_STORE_PAGE_FAULT, NULL);
> +
> + report_prefix_pop();
> +}
> +
> void check_fwft(void)
> {
> report_prefix_push("fwft");
> @@ -172,6 +341,7 @@ void check_fwft(void)
>
> fwft_check_base();
> fwft_check_misaligned_exc_deleg();
> + fwft_check_pte_ad_hw_updating();
>
> report_prefix_pop();
> }
With or without my previous comment fixed:
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] 17+ messages in thread
* [kvm-unit-tests PATCH v2 03/11] riscv: sbi: Ensure we have IPIs enabled for HSM suspend tests
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 01/11] riscv: sbi: Drop fwft upper bits test Andrew Jones
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 02/11] riscv: sbi: Add fwft pte_hw_ad_updating test Andrew Jones
@ 2025-02-27 14:19 ` Andrew Jones
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 04/11] riscv: sbi: Ensure SUSP test gets an interrupt Andrew Jones
` (8 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:19 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
We'll start cleaning up after each test, so the HSM tests need to
ensure they have IPIs setup themselves since they plan to use them.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi.c | 62 ++++++++++++++++++++++++++++++++---------------------
1 file changed, 37 insertions(+), 25 deletions(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 7c7a2d2ddddb..d7bf758e23fb 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -62,7 +62,7 @@ static struct sbiret sbi_dbcn_write_byte(uint8_t byte)
return sbi_ecall(SBI_EXT_DBCN, SBI_EXT_DBCN_CONSOLE_WRITE_BYTE, byte, 0, 0, 0, 0, 0);
}
-static struct sbiret sbi_hart_suspend(uint32_t suspend_type, unsigned long resume_addr, unsigned long opaque)
+static struct sbiret sbi_hart_suspend_raw(unsigned long suspend_type, unsigned long resume_addr, unsigned long opaque)
{
return sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND, suspend_type, resume_addr, opaque, 0, 0, 0);
}
@@ -568,59 +568,71 @@ static void hart_start_invalid_hartid(void *data)
sbi_hsm_invalid_hartid_check = true;
}
-static void hart_retentive_suspend(void *data)
+static void ipi_nop(struct pt_regs *regs)
+{
+ ipi_ack();
+}
+
+static void hart_suspend_and_wait_ipi(unsigned long suspend_type, unsigned long resume_addr,
+ unsigned long opaque, bool returns, const char *typestr)
{
unsigned long hartid = current_thread_info()->hartid;
- struct sbiret ret = sbi_hart_suspend(SBI_EXT_HSM_HART_SUSPEND_RETENTIVE, 0, 0);
+ struct sbiret ret;
+ install_irq_handler(IRQ_S_SOFT, ipi_nop);
+ local_ipi_enable();
+ local_irq_enable();
+
+ ret = sbi_hart_suspend_raw(suspend_type, resume_addr, opaque);
if (ret.error)
- report_fail("failed to retentive suspend cpu%d (hartid = %lx) (error=%ld)",
- smp_processor_id(), hartid, ret.error);
+ report_fail("failed to %s cpu%d (hartid = %lx) (error=%ld)",
+ typestr, smp_processor_id(), hartid, ret.error);
+ else if (!returns)
+ report_fail("failed to %s cpu%d (hartid = %lx) (call should not return)",
+ typestr, smp_processor_id(), hartid);
+
+ local_irq_disable();
+ local_ipi_disable();
+ install_irq_handler(IRQ_S_SOFT, NULL);
+}
+
+static void hart_retentive_suspend(void *data)
+{
+ hart_suspend_and_wait_ipi(SBI_EXT_HSM_HART_SUSPEND_RETENTIVE, 0, 0, true, "retentive suspend");
}
static void hart_non_retentive_suspend(void *data)
{
- unsigned long hartid = current_thread_info()->hartid;
unsigned long params[] = {
[SBI_HSM_MAGIC_IDX] = SBI_HSM_MAGIC,
- [SBI_HSM_HARTID_IDX] = hartid,
+ [SBI_HSM_HARTID_IDX] = current_thread_info()->hartid,
};
- struct sbiret ret = sbi_hart_suspend(SBI_EXT_HSM_HART_SUSPEND_NON_RETENTIVE,
- virt_to_phys(&sbi_hsm_check_non_retentive_suspend),
- virt_to_phys(params));
- report_fail("failed to non-retentive suspend cpu%d (hartid = %lx) (error=%ld)",
- smp_processor_id(), hartid, ret.error);
+ hart_suspend_and_wait_ipi(SBI_EXT_HSM_HART_SUSPEND_NON_RETENTIVE,
+ virt_to_phys(&sbi_hsm_check_non_retentive_suspend), virt_to_phys(params),
+ false, "non-retentive suspend");
}
/* This test function is only being run on RV64 to verify that upper bits of suspend_type are ignored */
static void hart_retentive_suspend_with_msb_set(void *data)
{
- unsigned long hartid = current_thread_info()->hartid;
unsigned long suspend_type = SBI_EXT_HSM_HART_SUSPEND_RETENTIVE | (_AC(1, UL) << (__riscv_xlen - 1));
- struct sbiret ret = sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND, suspend_type, 0, 0, 0, 0, 0);
- if (ret.error)
- report_fail("failed to retentive suspend cpu%d (hartid = %lx) with MSB set (error=%ld)",
- smp_processor_id(), hartid, ret.error);
+ hart_suspend_and_wait_ipi(suspend_type, 0, 0, true, "retentive suspend with MSB set");
}
/* This test function is only being run on RV64 to verify that upper bits of suspend_type are ignored */
static void hart_non_retentive_suspend_with_msb_set(void *data)
{
- unsigned long hartid = current_thread_info()->hartid;
unsigned long suspend_type = SBI_EXT_HSM_HART_SUSPEND_NON_RETENTIVE | (_AC(1, UL) << (__riscv_xlen - 1));
unsigned long params[] = {
[SBI_HSM_MAGIC_IDX] = SBI_HSM_MAGIC,
- [SBI_HSM_HARTID_IDX] = hartid,
+ [SBI_HSM_HARTID_IDX] = current_thread_info()->hartid,
};
- struct sbiret ret = sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND, suspend_type,
- virt_to_phys(&sbi_hsm_check_non_retentive_suspend), virt_to_phys(params),
- 0, 0, 0);
-
- report_fail("failed to non-retentive suspend cpu%d (hartid = %lx) with MSB set (error=%ld)",
- smp_processor_id(), hartid, ret.error);
+ hart_suspend_and_wait_ipi(suspend_type,
+ virt_to_phys(&sbi_hsm_check_non_retentive_suspend), virt_to_phys(params),
+ false, "non-retentive suspend with MSB set");
}
static bool hart_wait_on_status(unsigned long hartid, enum sbi_ext_hsm_sid status, unsigned long duration)
--
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] 17+ messages in thread* [kvm-unit-tests PATCH v2 04/11] riscv: sbi: Ensure SUSP test gets an interrupt
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
` (2 preceding siblings ...)
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 03/11] riscv: sbi: Ensure we have IPIs enabled for HSM suspend tests Andrew Jones
@ 2025-02-27 14:19 ` Andrew Jones
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 05/11] riscv: sbi: Improve susp expected error output Andrew Jones
` (7 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:19 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
A system suspend may call WFI. Set a periodic timer to ensure we get
an interrupt and don't hang forever.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi-tests.h | 2 ++
riscv/sbi.c | 70 ++++++++++++++++++++++++++++++-----------------
2 files changed, 47 insertions(+), 25 deletions(-)
diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
index 7a24e083a7b8..7c7fe30541e4 100644
--- a/riscv/sbi-tests.h
+++ b/riscv/sbi-tests.h
@@ -19,12 +19,14 @@
#define SBI_SUSP_HARTID_IDX 2
#define SBI_SUSP_TESTNUM_IDX 3
#define SBI_SUSP_RESULTS_IDX 4
+#define SBI_SUSP_NR_IDX 5
#define SBI_CSR_SSTATUS_IDX 0
#define SBI_CSR_SIE_IDX 1
#define SBI_CSR_STVEC_IDX 2
#define SBI_CSR_SSCRATCH_IDX 3
#define SBI_CSR_SATP_IDX 4
+#define SBI_CSR_NR_IDX 5
#define SBI_SUSP_MAGIC 0x505b
diff --git a/riscv/sbi.c b/riscv/sbi.c
index d7bf758e23fb..dd5cb6fa26bf 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -158,6 +158,19 @@ static bool get_invalid_addr(phys_addr_t *paddr, bool allow_default)
return false;
}
+static void timer_setup(void (*handler)(struct pt_regs *))
+{
+ install_irq_handler(IRQ_S_TIMER, handler);
+ timer_irq_enable();
+}
+
+static void timer_teardown(void)
+{
+ timer_irq_disable();
+ timer_stop();
+ install_irq_handler(IRQ_S_TIMER, NULL);
+}
+
static void check_base(void)
{
struct sbiret ret;
@@ -534,18 +547,6 @@ static void hsm_timer_irq_handler(struct pt_regs *regs)
sbi_hsm_timer_fired = true;
}
-static void hsm_timer_setup(void)
-{
- install_irq_handler(IRQ_S_TIMER, hsm_timer_irq_handler);
- timer_irq_enable();
-}
-
-static void hsm_timer_teardown(void)
-{
- timer_irq_disable();
- install_irq_handler(IRQ_S_TIMER, NULL);
-}
-
static void hart_check_already_started(void *data)
{
struct sbiret ret;
@@ -753,7 +754,7 @@ static void check_hsm(void)
cpumask_copy(&secondary_cpus_mask, &cpu_present_mask);
cpumask_clear_cpu(me, &secondary_cpus_mask);
- hsm_timer_setup();
+ timer_setup(hsm_timer_irq_handler);
local_irq_enable();
/* Assume that previous tests have not cleaned up and stopped the secondary harts */
@@ -975,7 +976,7 @@ sbi_hsm_hart_stop_tests:
if (__riscv_xlen == 32 || ipi_unavailable) {
local_irq_disable();
- hsm_timer_teardown();
+ timer_teardown();
report_prefix_pop();
return;
}
@@ -1084,7 +1085,7 @@ sbi_hsm_hart_stop_tests:
report(count, "secondary hart stopped after suspension tests with MSB set");
local_irq_disable();
- hsm_timer_teardown();
+ timer_teardown();
report_prefix_popn(2);
}
@@ -1215,6 +1216,12 @@ static void check_dbcn(void)
void sbi_susp_resume(unsigned long hartid, unsigned long opaque);
jmp_buf sbi_susp_jmp;
+#define SBI_SUSP_TIMER_DURATION_US 500000
+static void susp_timer(struct pt_regs *regs)
+{
+ timer_start(SBI_SUSP_TIMER_DURATION_US);
+}
+
struct susp_params {
unsigned long sleep_type;
unsigned long resume_addr;
@@ -1226,9 +1233,17 @@ struct susp_params {
static bool susp_basic_prep(unsigned long ctx[], struct susp_params *params)
{
int cpu, me = smp_processor_id();
+ unsigned long *csrs;
struct sbiret ret;
cpumask_t mask;
+ csrs = (unsigned long *)ctx[SBI_SUSP_CSRS_IDX];
+ csrs[SBI_CSR_SSTATUS_IDX] = csr_read(CSR_SSTATUS);
+ csrs[SBI_CSR_SIE_IDX] = csr_read(CSR_SIE);
+ csrs[SBI_CSR_STVEC_IDX] = csr_read(CSR_STVEC);
+ csrs[SBI_CSR_SSCRATCH_IDX] = csr_read(CSR_SSCRATCH);
+ csrs[SBI_CSR_SATP_IDX] = csr_read(CSR_SATP);
+
memset(params, 0, sizeof(*params));
params->sleep_type = 0; /* suspend-to-ram */
params->resume_addr = virt_to_phys(sbi_susp_resume);
@@ -1352,19 +1367,11 @@ static bool susp_one_prep(unsigned long ctx[], struct susp_params *params)
static void check_susp(void)
{
- unsigned long csrs[] = {
- [SBI_CSR_SSTATUS_IDX] = csr_read(CSR_SSTATUS),
- [SBI_CSR_SIE_IDX] = csr_read(CSR_SIE),
- [SBI_CSR_STVEC_IDX] = csr_read(CSR_STVEC),
- [SBI_CSR_SSCRATCH_IDX] = csr_read(CSR_SSCRATCH),
- [SBI_CSR_SATP_IDX] = csr_read(CSR_SATP),
- };
- unsigned long ctx[] = {
+ unsigned long csrs[SBI_CSR_NR_IDX];
+ unsigned long ctx[SBI_SUSP_NR_IDX] = {
[SBI_SUSP_MAGIC_IDX] = SBI_SUSP_MAGIC,
[SBI_SUSP_CSRS_IDX] = (unsigned long)csrs,
[SBI_SUSP_HARTID_IDX] = current_thread_info()->hartid,
- [SBI_SUSP_TESTNUM_IDX] = 0,
- [SBI_SUSP_RESULTS_IDX] = 0,
};
enum {
#define SUSP_FIRST_TESTNUM 1
@@ -1393,6 +1400,10 @@ static void check_susp(void)
report_prefix_push("susp");
+ timer_setup(susp_timer);
+ local_irq_enable();
+ timer_start(SBI_SUSP_TIMER_DURATION_US);
+
ret = sbi_ecall(SBI_EXT_SUSP, 1, 0, 0, 0, 0, 0, 0);
report(ret.error == SBI_ERR_NOT_SUPPORTED, "funcid != 0 not supported");
@@ -1402,6 +1413,8 @@ static void check_susp(void)
ctx[SBI_SUSP_TESTNUM_IDX] = i;
ctx[SBI_SUSP_RESULTS_IDX] = 0;
+ local_irq_disable();
+
assert(susp_tests[i].prep);
if (!susp_tests[i].prep(ctx, ¶ms)) {
report_prefix_pop();
@@ -1411,6 +1424,8 @@ static void check_susp(void)
if ((testnum = setjmp(sbi_susp_jmp)) == 0) {
ret = sbi_system_suspend(params.sleep_type, params.resume_addr, params.opaque);
+ local_irq_enable();
+
if (!params.returns && ret.error == SBI_ERR_NOT_SUPPORTED) {
report_skip("SUSP not supported?");
report_prefix_popn(2);
@@ -1428,12 +1443,17 @@ static void check_susp(void)
}
assert(testnum == i);
+ local_irq_enable();
+
if (susp_tests[i].check)
susp_tests[i].check(ctx, ¶ms);
report_prefix_pop();
}
+ local_irq_disable();
+ timer_teardown();
+
report_prefix_pop();
}
--
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] 17+ messages in thread* [kvm-unit-tests PATCH v2 05/11] riscv: sbi: Improve susp expected error output
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
` (3 preceding siblings ...)
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 04/11] riscv: sbi: Ensure SUSP test gets an interrupt Andrew Jones
@ 2025-02-27 14:19 ` Andrew Jones
2025-02-27 14:38 ` Clément Léger
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 06/11] riscv: sbi: Improve interrupt handling cleanup Andrew Jones
` (6 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:19 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
Improve the output of expected error tests. We don't use
sbiret_report_error() because we're not passing in an SBI_ERR_*
name to stringify.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index dd5cb6fa26bf..ceeb0d8d2779 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -1433,9 +1433,8 @@ static void check_susp(void)
} else if (!params.returns) {
report_fail("unexpected return with error: %ld, value: %ld", ret.error, ret.value);
} else {
- report(ret.error == params.ret.error, "expected sbi.error");
- if (ret.error != params.ret.error)
- report_info("expected error %ld, received %ld", params.ret.error, ret.error);
+ if (!report(ret.error == params.ret.error, "got expected sbi.error (%ld)", params.ret.error))
+ report_info("expected sbi.error %ld, received %ld", params.ret.error, ret.error);
}
report_prefix_pop();
--
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] 17+ messages in thread* Re: [kvm-unit-tests PATCH v2 05/11] riscv: sbi: Improve susp expected error output
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 05/11] riscv: sbi: Improve susp expected error output Andrew Jones
@ 2025-02-27 14:38 ` Clément Léger
0 siblings, 0 replies; 17+ messages in thread
From: Clément Léger @ 2025-02-27 14:38 UTC (permalink / raw)
To: Andrew Jones, kvm-riscv; +Cc: atishp, jamestiotio
On 27/02/2025 15:19, Andrew Jones wrote:
> Improve the output of expected error tests. We don't use
> sbiret_report_error() because we're not passing in an SBI_ERR_*
> name to stringify.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
> riscv/sbi.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index dd5cb6fa26bf..ceeb0d8d2779 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -1433,9 +1433,8 @@ static void check_susp(void)
> } else if (!params.returns) {
> report_fail("unexpected return with error: %ld, value: %ld", ret.error, ret.value);
> } else {
> - report(ret.error == params.ret.error, "expected sbi.error");
> - if (ret.error != params.ret.error)
> - report_info("expected error %ld, received %ld", params.ret.error, ret.error);
> + if (!report(ret.error == params.ret.error, "got expected sbi.error (%ld)", params.ret.error))
> + report_info("expected sbi.error %ld, received %ld", params.ret.error, ret.error);
> }
>
> report_prefix_pop();
Hi Andrew,
LGTM,
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] 17+ messages in thread
* [kvm-unit-tests PATCH v2 06/11] riscv: sbi: Improve interrupt handling cleanup
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
` (4 preceding siblings ...)
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 05/11] riscv: sbi: Improve susp expected error output Andrew Jones
@ 2025-02-27 14:19 ` Andrew Jones
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 07/11] lib/cpumask: Add some operators Andrew Jones
` (5 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:19 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
Each test should clean up after itself so following tests don't
need to worry about the current state and can just do its own
prep.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index ceeb0d8d2779..7bffaac07283 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -394,6 +394,8 @@ static void ipi_hart_wait(void *data)
timer_stop();
local_ipi_disable();
timer_irq_disable();
+ install_irq_handler(IRQ_S_SOFT, NULL);
+ install_irq_handler(IRQ_S_TIMER, NULL);
cpumask_set_cpu(me, &ipi_done);
}
@@ -1395,9 +1397,6 @@ static void check_susp(void)
struct sbiret ret;
int testnum, i;
- local_irq_disable();
- timer_stop();
-
report_prefix_push("susp");
timer_setup(susp_timer);
@@ -1428,8 +1427,8 @@ static void check_susp(void)
if (!params.returns && ret.error == SBI_ERR_NOT_SUPPORTED) {
report_skip("SUSP not supported?");
- report_prefix_popn(2);
- return;
+ report_prefix_pop();
+ goto out;
} else if (!params.returns) {
report_fail("unexpected return with error: %ld, value: %ld", ret.error, ret.value);
} else {
@@ -1450,6 +1449,7 @@ static void check_susp(void)
report_prefix_pop();
}
+out:
local_irq_disable();
timer_teardown();
--
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] 17+ messages in thread* [kvm-unit-tests PATCH v2 07/11] lib/cpumask: Add some operators
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
` (5 preceding siblings ...)
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 06/11] riscv: sbi: Improve interrupt handling cleanup Andrew Jones
@ 2025-02-27 14:19 ` Andrew Jones
2025-02-27 14:22 ` [kvm-unit-tests PATCH v2 08/11] riscv: sbi: HSM suspend may not be supported Andrew Jones
` (4 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:19 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
We have a need for cpumask_andnot(). Add that and a few friends.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
lib/cpumask.h | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/lib/cpumask.h b/lib/cpumask.h
index b4cd83a66a56..3f03d3370f85 100644
--- a/lib/cpumask.h
+++ b/lib/cpumask.h
@@ -72,6 +72,62 @@ static inline bool cpumask_subset(const struct cpumask *src1, const struct cpuma
return !lastmask || !((cpumask_bits(src1)[i] & ~cpumask_bits(src2)[i]) & lastmask);
}
+/* false if dst is empty */
+static inline bool cpumask_and(cpumask_t *dst, const cpumask_t *src1, const cpumask_t *src2)
+{
+ unsigned long lastmask = BIT_MASK(nr_cpus) - 1;
+ unsigned long ret = 0;
+ int i;
+
+ for (i = 0; i < BIT_WORD(nr_cpus); ++i) {
+ cpumask_bits(dst)[i] = cpumask_bits(src1)[i] & cpumask_bits(src2)[i];
+ ret |= cpumask_bits(dst)[i];
+ }
+
+ cpumask_bits(dst)[i] = (cpumask_bits(src1)[i] & cpumask_bits(src2)[i]) & lastmask;
+
+ return ret | cpumask_bits(dst)[i];
+}
+
+static inline void cpumask_or(cpumask_t *dst, const cpumask_t *src1, const cpumask_t *src2)
+{
+ unsigned long lastmask = BIT_MASK(nr_cpus) - 1;
+ int i;
+
+ for (i = 0; i < BIT_WORD(nr_cpus); ++i)
+ cpumask_bits(dst)[i] = cpumask_bits(src1)[i] | cpumask_bits(src2)[i];
+
+ cpumask_bits(dst)[i] = (cpumask_bits(src1)[i] | cpumask_bits(src2)[i]) & lastmask;
+}
+
+static inline void cpumask_xor(cpumask_t *dst, const cpumask_t *src1, const cpumask_t *src2)
+{
+ unsigned long lastmask = BIT_MASK(nr_cpus) - 1;
+ int i;
+
+ for (i = 0; i < BIT_WORD(nr_cpus); ++i)
+ cpumask_bits(dst)[i] = cpumask_bits(src1)[i] ^ cpumask_bits(src2)[i];
+
+ cpumask_bits(dst)[i] = (cpumask_bits(src1)[i] ^ cpumask_bits(src2)[i]) & lastmask;
+}
+
+/* false if dst is empty */
+static inline bool cpumask_andnot(cpumask_t *dst, const cpumask_t *src1, const cpumask_t *src2)
+{
+ unsigned long lastmask = BIT_MASK(nr_cpus) - 1;
+ unsigned long ret = 0;
+ int i;
+
+ for (i = 0; i < BIT_WORD(nr_cpus); ++i) {
+ cpumask_bits(dst)[i] = cpumask_bits(src1)[i] & ~cpumask_bits(src2)[i];
+ ret |= cpumask_bits(dst)[i];
+ }
+
+ cpumask_bits(dst)[i] = (cpumask_bits(src1)[i] & ~cpumask_bits(src2)[i]) & lastmask;
+
+ return ret | cpumask_bits(dst)[i];
+}
+
static inline bool cpumask_equal(const struct cpumask *src1, const struct cpumask *src2)
{
unsigned long lastmask = BIT_MASK(nr_cpus) - 1;
--
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] 17+ messages in thread* [kvm-unit-tests PATCH v2 08/11] riscv: sbi: HSM suspend may not be supported
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
` (6 preceding siblings ...)
2025-02-27 14:19 ` [kvm-unit-tests PATCH v2 07/11] lib/cpumask: Add some operators Andrew Jones
@ 2025-02-27 14:22 ` Andrew Jones
2025-02-27 14:22 ` [kvm-unit-tests PATCH v2 09/11] riscv: sbi: Probe/skip SUSP Andrew Jones
` (3 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:22 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
The spec doesn't require that any suspend types are supported and
indeed KVM doesn't support non-retentive suspend. Tolerate
SBI_ERR_NOT_SUPPORTED.
TODO: The HSM tests have lots of duplicated code and this patch
adds even more. We need to refactor them and we should break them
out into their own riscv/sbi-hsm.c file too.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi.c | 87 ++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 70 insertions(+), 17 deletions(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 7bffaac07283..a0a8331b04bd 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -571,6 +571,8 @@ static void hart_start_invalid_hartid(void *data)
sbi_hsm_invalid_hartid_check = true;
}
+static cpumask_t hsm_suspend_not_supported;
+
static void ipi_nop(struct pt_regs *regs)
{
ipi_ack();
@@ -587,7 +589,9 @@ static void hart_suspend_and_wait_ipi(unsigned long suspend_type, unsigned long
local_irq_enable();
ret = sbi_hart_suspend_raw(suspend_type, resume_addr, opaque);
- if (ret.error)
+ if (ret.error == SBI_ERR_NOT_SUPPORTED)
+ cpumask_set_cpu(smp_processor_id(), &hsm_suspend_not_supported);
+ else if (ret.error)
report_fail("failed to %s cpu%d (hartid = %lx) (error=%ld)",
typestr, smp_processor_id(), hartid, ret.error);
else if (!returns)
@@ -707,7 +711,7 @@ static void check_hsm(void)
{
struct sbiret ret;
unsigned long hartid;
- cpumask_t secondary_cpus_mask, mask;
+ cpumask_t secondary_cpus_mask, mask, resume_mask;
struct hart_state_transition_info transition_states;
bool ipi_unavailable = false;
int cpu, me = smp_processor_id();
@@ -715,7 +719,7 @@ static void check_hsm(void)
unsigned long hsm_timer_duration = getenv("SBI_HSM_TIMER_DURATION")
? strtol(getenv("SBI_HSM_TIMER_DURATION"), NULL, 0) : 200000;
unsigned long sbi_hsm_hart_start_params[NR_CPUS * SBI_HSM_NUM_OF_PARAMS];
- int count, check;
+ int count, check, expected_count, resume_count;
max_cpus = MIN(MIN(max_cpus, nr_cpus), cpumask_weight(&cpu_present_mask));
@@ -879,6 +883,7 @@ static void check_hsm(void)
goto sbi_hsm_hart_stop_tests;
}
+ cpumask_clear(&hsm_suspend_not_supported);
on_cpumask_async(&secondary_cpus_mask, hart_retentive_suspend, NULL);
transition_states = (struct hart_state_transition_info) {
@@ -888,22 +893,36 @@ static void check_hsm(void)
};
count = hart_wait_state_transition(&secondary_cpus_mask, hsm_timer_duration, &transition_states);
- report(count == max_cpus - 1, "all secondary harts retentive suspended");
+ expected_count = max_cpus - 1 - cpumask_weight(&hsm_suspend_not_supported);
+
+ if (expected_count != 0) {
+ if (expected_count != max_cpus - 1)
+ report_info("not all harts support retentive suspend");
+ report(count == expected_count, "supporting secondary harts retentive suspended");
+ } else {
+ report_skip("retentive suspend not supported by any harts");
+ goto nonret_suspend_tests;
+ }
+
+ cpumask_andnot(&resume_mask, &secondary_cpus_mask, &hsm_suspend_not_supported);
+ resume_count = cpumask_weight(&resume_mask);
/* Ignore the return value since we check the status of each hart anyway */
- sbi_send_ipi_cpumask(&secondary_cpus_mask);
+ sbi_send_ipi_cpumask(&resume_mask);
transition_states = (struct hart_state_transition_info) {
.initial_state = SBI_EXT_HSM_SUSPENDED,
.intermediate_state = SBI_EXT_HSM_RESUME_PENDING,
.final_state = SBI_EXT_HSM_STARTED,
};
- count = hart_wait_state_transition(&secondary_cpus_mask, hsm_timer_duration, &transition_states);
+ count = hart_wait_state_transition(&resume_mask, hsm_timer_duration, &transition_states);
- report(count == max_cpus - 1, "all secondary harts retentive resumed");
+ report(count == resume_count, "supporting secondary harts retentive resumed");
+nonret_suspend_tests:
hart_wait_until_idle(&secondary_cpus_mask, hsm_timer_duration);
+ cpumask_clear(&hsm_suspend_not_supported);
on_cpumask_async(&secondary_cpus_mask, hart_non_retentive_suspend, NULL);
transition_states = (struct hart_state_transition_info) {
@@ -913,20 +932,32 @@ static void check_hsm(void)
};
count = hart_wait_state_transition(&secondary_cpus_mask, hsm_timer_duration, &transition_states);
- report(count == max_cpus - 1, "all secondary harts non-retentive suspended");
+ expected_count = max_cpus - 1 - cpumask_weight(&hsm_suspend_not_supported);
+
+ if (expected_count != 0) {
+ if (expected_count != max_cpus - 1)
+ report_info("not all harts support non-retentive suspend");
+ report(count == expected_count, "supporting secondary harts non-retentive suspended");
+ } else {
+ report_skip("non-retentive suspend not supported by any harts");
+ goto hsm_suspend_tests_done;
+ }
+
+ cpumask_andnot(&resume_mask, &secondary_cpus_mask, &hsm_suspend_not_supported);
+ resume_count = cpumask_weight(&resume_mask);
/* Ignore the return value since we check the status of each hart anyway */
- sbi_send_ipi_cpumask(&secondary_cpus_mask);
+ sbi_send_ipi_cpumask(&resume_mask);
transition_states = (struct hart_state_transition_info) {
.initial_state = SBI_EXT_HSM_SUSPENDED,
.intermediate_state = SBI_EXT_HSM_RESUME_PENDING,
.final_state = SBI_EXT_HSM_STARTED,
};
- count = hart_wait_state_transition(&secondary_cpus_mask, hsm_timer_duration, &transition_states);
+ count = hart_wait_state_transition(&resume_mask, hsm_timer_duration, &transition_states);
check = 0;
- for_each_cpu(cpu, &secondary_cpus_mask) {
+ for_each_cpu(cpu, &resume_mask) {
sbi_hsm_timer_fired = false;
timer_start(hsm_timer_duration);
@@ -952,15 +983,16 @@ static void check_hsm(void)
check++;
}
- report(count == max_cpus - 1, "all secondary harts non-retentive resumed");
- report(check == max_cpus - 1, "all secondary harts have expected register values after non-retentive resume");
+ report(count == resume_count, "supporting secondary harts non-retentive resumed");
+ report(check == resume_count, "supporting secondary harts have expected register values after non-retentive resume");
+hsm_suspend_tests_done:
report_prefix_pop();
sbi_hsm_hart_stop_tests:
report_prefix_push("hart_stop");
- if (ipi_unavailable)
+ if (ipi_unavailable || expected_count == 0)
on_cpumask_async(&secondary_cpus_mask, stop_cpu, NULL);
else
memset(sbi_hsm_stop_hart, 1, sizeof(sbi_hsm_stop_hart));
@@ -994,6 +1026,7 @@ sbi_hsm_hart_stop_tests:
/* Boot up the secondary cpu and let it proceed to the idle loop */
on_cpu(cpu, start_cpu, NULL);
+ cpumask_clear(&hsm_suspend_not_supported);
on_cpu_async(cpu, hart_retentive_suspend_with_msb_set, NULL);
transition_states = (struct hart_state_transition_info) {
@@ -1003,7 +1036,14 @@ sbi_hsm_hart_stop_tests:
};
count = hart_wait_state_transition(&mask, hsm_timer_duration, &transition_states);
- report(count, "secondary hart retentive suspended with MSB set");
+ expected_count = 1 - cpumask_weight(&hsm_suspend_not_supported);
+
+ if (expected_count) {
+ report(count == expected_count, "retentive suspend with MSB set");
+ } else {
+ report_skip("retentive suspend not supported by cpu%d", cpu);
+ goto nonret_suspend_with_msb;
+ }
/* Ignore the return value since we manually validate the status of the hart anyway */
sbi_send_ipi_cpu(cpu);
@@ -1017,10 +1057,12 @@ sbi_hsm_hart_stop_tests:
report(count, "secondary hart retentive resumed with MSB set");
+nonret_suspend_with_msb:
/* Reset these flags so that we can reuse them for the non-retentive suspension test */
sbi_hsm_stop_hart[cpu] = 0;
sbi_hsm_non_retentive_hart_suspend_checks[cpu] = 0;
+ cpumask_clear(&hsm_suspend_not_supported);
on_cpu_async(cpu, hart_non_retentive_suspend_with_msb_set, NULL);
transition_states = (struct hart_state_transition_info) {
@@ -1030,7 +1072,14 @@ sbi_hsm_hart_stop_tests:
};
count = hart_wait_state_transition(&mask, hsm_timer_duration, &transition_states);
- report(count, "secondary hart non-retentive suspended with MSB set");
+ expected_count = 1 - cpumask_weight(&hsm_suspend_not_supported);
+
+ if (expected_count) {
+ report(count == expected_count, "non-retentive suspend with MSB set");
+ } else {
+ report_skip("non-retentive suspend not supported by cpu%d", cpu);
+ goto hsm_hart_stop_test;
+ }
/* Ignore the return value since we manually validate the status of the hart anyway */
sbi_send_ipi_cpu(cpu);
@@ -1071,11 +1120,15 @@ sbi_hsm_hart_stop_tests:
report(count, "secondary hart non-retentive resumed with MSB set");
report(check, "secondary hart has expected register values after non-retentive resume with MSB set");
+hsm_hart_stop_test:
report_prefix_pop();
report_prefix_push("hart_stop");
- sbi_hsm_stop_hart[cpu] = 1;
+ if (expected_count == 0)
+ on_cpu_async(cpu, stop_cpu, NULL);
+ else
+ sbi_hsm_stop_hart[cpu] = 1;
transition_states = (struct hart_state_transition_info) {
.initial_state = SBI_EXT_HSM_STARTED,
--
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] 17+ messages in thread* [kvm-unit-tests PATCH v2 09/11] riscv: sbi: Probe/skip SUSP
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
` (7 preceding siblings ...)
2025-02-27 14:22 ` [kvm-unit-tests PATCH v2 08/11] riscv: sbi: HSM suspend may not be supported Andrew Jones
@ 2025-02-27 14:22 ` Andrew Jones
2025-02-27 14:22 ` [kvm-unit-tests PATCH v2 10/11] riscv: sbi: susp: Check upper bits of sleep_type are ignored Andrew Jones
` (2 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:22 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
Cleanly skip testing suspend when the SUSP extension isn't available.
Reviewed-by: Clément Léger <cleger@rivosinc.com>
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index a0a8331b04bd..e5f6e1c72ae6 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -1452,6 +1452,12 @@ static void check_susp(void)
report_prefix_push("susp");
+ if (!sbi_probe(SBI_EXT_SUSP)) {
+ report_skip("SUSP extension not available");
+ report_prefix_pop();
+ return;
+ }
+
timer_setup(susp_timer);
local_irq_enable();
timer_start(SBI_SUSP_TIMER_DURATION_US);
@@ -1479,7 +1485,7 @@ static void check_susp(void)
local_irq_enable();
if (!params.returns && ret.error == SBI_ERR_NOT_SUPPORTED) {
- report_skip("SUSP not supported?");
+ report_fail("probing claims support, but it's not?");
report_prefix_pop();
goto out;
} else if (!params.returns) {
--
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] 17+ messages in thread* [kvm-unit-tests PATCH v2 10/11] riscv: sbi: susp: Check upper bits of sleep_type are ignored
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
` (8 preceding siblings ...)
2025-02-27 14:22 ` [kvm-unit-tests PATCH v2 09/11] riscv: sbi: Probe/skip SUSP Andrew Jones
@ 2025-02-27 14:22 ` Andrew Jones
2025-02-27 14:22 ` [kvm-unit-tests PATCH v2 11/11] riscv: sbi: Add bad fid tests Andrew Jones
2025-03-04 9:31 ` [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
11 siblings, 0 replies; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:22 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
When an SBI function parameter has a specific bit width which is
less than that of a long, then the upper bits should be ignored
by the SBI implementation. I.e. since sleep_type is a uint32_t,
then on rv64 both 0 and 0x1_0000_0000 are valid and mean suspend
to ram.
Reviewed-by: Clément Léger <cleger@rivosinc.com>
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/riscv/sbi.c b/riscv/sbi.c
index e5f6e1c72ae6..1d9d1871a28b 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -67,7 +67,7 @@ static struct sbiret sbi_hart_suspend_raw(unsigned long suspend_type, unsigned l
return sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND, suspend_type, resume_addr, opaque, 0, 0, 0);
}
-static struct sbiret sbi_system_suspend(uint32_t sleep_type, unsigned long resume_addr, unsigned long opaque)
+static struct sbiret sbi_system_suspend_raw(unsigned long sleep_type, unsigned long resume_addr, unsigned long opaque)
{
return sbi_ecall(SBI_EXT_SUSP, 0, sleep_type, resume_addr, opaque, 0, 0, 0);
}
@@ -1367,6 +1367,19 @@ static bool susp_type_prep(unsigned long ctx[], struct susp_params *params)
return true;
}
+#if __riscv_xlen != 32
+static bool susp_type_prep2(unsigned long ctx[], struct susp_params *params)
+{
+ bool r;
+
+ r = susp_basic_prep(ctx, params);
+ assert(r);
+ params->sleep_type = BIT(32);
+
+ return true;
+}
+#endif
+
static bool susp_badaddr_prep(unsigned long ctx[], struct susp_params *params)
{
phys_addr_t badaddr;
@@ -1432,6 +1445,7 @@ static void check_susp(void)
#define SUSP_FIRST_TESTNUM 1
SUSP_BASIC = SUSP_FIRST_TESTNUM,
SUSP_TYPE,
+ SUSP_TYPE2,
SUSP_BAD_ADDR,
SUSP_ONE_ONLINE,
NR_SUSP_TESTS,
@@ -1441,10 +1455,13 @@ static void check_susp(void)
bool (*prep)(unsigned long ctx[], struct susp_params *params);
void (*check)(unsigned long ctx[], struct susp_params *params);
} susp_tests[] = {
- [SUSP_BASIC] = { "basic", susp_basic_prep, susp_basic_check, },
- [SUSP_TYPE] = { "sleep_type", susp_type_prep, },
- [SUSP_BAD_ADDR] = { "bad addr", susp_badaddr_prep, },
- [SUSP_ONE_ONLINE] = { "one cpu online", susp_one_prep, },
+ [SUSP_BASIC] = { "basic", susp_basic_prep, susp_basic_check, },
+ [SUSP_TYPE] = { "sleep_type", susp_type_prep, },
+#if __riscv_xlen != 32
+ [SUSP_TYPE2] = { "sleep_type upper bits", susp_type_prep2, susp_basic_check },
+#endif
+ [SUSP_BAD_ADDR] = { "bad addr", susp_badaddr_prep, },
+ [SUSP_ONE_ONLINE] = { "one cpu online", susp_one_prep, },
};
struct susp_params params;
struct sbiret ret;
@@ -1466,6 +1483,9 @@ static void check_susp(void)
report(ret.error == SBI_ERR_NOT_SUPPORTED, "funcid != 0 not supported");
for (i = SUSP_FIRST_TESTNUM; i < NR_SUSP_TESTS; i++) {
+ if (!susp_tests[i].name)
+ continue;
+
report_prefix_push(susp_tests[i].name);
ctx[SBI_SUSP_TESTNUM_IDX] = i;
@@ -1480,7 +1500,7 @@ static void check_susp(void)
}
if ((testnum = setjmp(sbi_susp_jmp)) == 0) {
- ret = sbi_system_suspend(params.sleep_type, params.resume_addr, params.opaque);
+ ret = sbi_system_suspend_raw(params.sleep_type, params.resume_addr, params.opaque);
local_irq_enable();
--
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] 17+ messages in thread* [kvm-unit-tests PATCH v2 11/11] riscv: sbi: Add bad fid tests
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
` (9 preceding siblings ...)
2025-02-27 14:22 ` [kvm-unit-tests PATCH v2 10/11] riscv: sbi: susp: Check upper bits of sleep_type are ignored Andrew Jones
@ 2025-02-27 14:22 ` Andrew Jones
2025-02-27 14:33 ` Clément Léger
2025-03-04 9:31 ` [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
11 siblings, 1 reply; 17+ messages in thread
From: Andrew Jones @ 2025-02-27 14:22 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
Ensure we get the correct error code when attempting to use an
invalid FID.
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
riscv/sbi-fwft.c | 2 ++
riscv/sbi-tests.h | 2 ++
riscv/sbi.c | 18 ++++++++++++++++++
3 files changed, 22 insertions(+)
diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c
index ca4d49b5db85..ac2e34869dfa 100644
--- a/riscv/sbi-fwft.c
+++ b/riscv/sbi-fwft.c
@@ -339,6 +339,8 @@ void check_fwft(void)
return;
}
+ sbi_bad_fid(SBI_EXT_FWFT);
+
fwft_check_base();
fwft_check_misaligned_exc_deleg();
fwft_check_pte_ad_hw_updating();
diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
index 7c7fe30541e4..f8ce3c269d48 100644
--- a/riscv/sbi-tests.h
+++ b/riscv/sbi-tests.h
@@ -70,5 +70,7 @@
#define sbiret_check(ret, expected_error, expected_value) \
sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
+void sbi_bad_fid(int ext);
+
#endif /* __ASSEMBLY__ */
#endif /* _RISCV_SBI_TESTS_H_ */
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 1d9d1871a28b..0404bb81317d 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -72,6 +72,12 @@ static struct sbiret sbi_system_suspend_raw(unsigned long sleep_type, unsigned l
return sbi_ecall(SBI_EXT_SUSP, 0, sleep_type, resume_addr, opaque, 0, 0, 0);
}
+void sbi_bad_fid(int ext)
+{
+ struct sbiret ret = sbi_ecall(ext, 0xbad, 0, 0, 0, 0, 0, 0);
+ sbiret_report_error(&ret, SBI_ERR_NOT_SUPPORTED, "Bad FID");
+}
+
static void start_cpu(void *data)
{
/* nothing to do */
@@ -178,6 +184,8 @@ static void check_base(void)
report_prefix_push("base");
+ sbi_bad_fid(SBI_EXT_BASE);
+
ret = sbi_base(SBI_EXT_BASE_GET_SPEC_VERSION, 0);
report_prefix_push("spec_version");
@@ -331,6 +339,8 @@ static void check_time(void)
return;
}
+ sbi_bad_fid(SBI_EXT_TIME);
+
report_prefix_push("set_timer");
install_irq_handler(IRQ_S_TIMER, timer_irq_handler);
@@ -438,6 +448,8 @@ static void check_ipi(void)
return;
}
+ sbi_bad_fid(SBI_EXT_IPI);
+
if (nr_cpus_present < 2) {
report_skip("At least 2 cpus required");
report_prefix_pop();
@@ -731,6 +743,8 @@ static void check_hsm(void)
return;
}
+ sbi_bad_fid(SBI_EXT_HSM);
+
report_prefix_push("hart_get_status");
hartid = current_thread_info()->hartid;
@@ -1210,6 +1224,8 @@ static void check_dbcn(void)
return;
}
+ sbi_bad_fid(SBI_EXT_DBCN);
+
report_prefix_push("write");
dbcn_write_test(DBCN_WRITE_TEST_STRING, num_bytes, false);
@@ -1475,6 +1491,8 @@ static void check_susp(void)
return;
}
+ sbi_bad_fid(SBI_EXT_SUSP);
+
timer_setup(susp_timer);
local_irq_enable();
timer_start(SBI_SUSP_TIMER_DURATION_US);
--
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] 17+ messages in thread* Re: [kvm-unit-tests PATCH v2 11/11] riscv: sbi: Add bad fid tests
2025-02-27 14:22 ` [kvm-unit-tests PATCH v2 11/11] riscv: sbi: Add bad fid tests Andrew Jones
@ 2025-02-27 14:33 ` Clément Léger
0 siblings, 0 replies; 17+ messages in thread
From: Clément Léger @ 2025-02-27 14:33 UTC (permalink / raw)
To: Andrew Jones, kvm-riscv; +Cc: atishp, jamestiotio
On 27/02/2025 15:22, Andrew Jones wrote:
> Ensure we get the correct error code when attempting to use an
> invalid FID.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
> riscv/sbi-fwft.c | 2 ++
> riscv/sbi-tests.h | 2 ++
> riscv/sbi.c | 18 ++++++++++++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c
> index ca4d49b5db85..ac2e34869dfa 100644
> --- a/riscv/sbi-fwft.c
> +++ b/riscv/sbi-fwft.c
> @@ -339,6 +339,8 @@ void check_fwft(void)
> return;
> }
>
> + sbi_bad_fid(SBI_EXT_FWFT);
> +
> fwft_check_base();
> fwft_check_misaligned_exc_deleg();
> fwft_check_pte_ad_hw_updating();
> diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
> index 7c7fe30541e4..f8ce3c269d48 100644
> --- a/riscv/sbi-tests.h
> +++ b/riscv/sbi-tests.h
> @@ -70,5 +70,7 @@
> #define sbiret_check(ret, expected_error, expected_value) \
> sbiret_report(ret, expected_error, expected_value, "check sbi.error and sbi.value")
>
> +void sbi_bad_fid(int ext);
> +
> #endif /* __ASSEMBLY__ */
> #endif /* _RISCV_SBI_TESTS_H_ */
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index 1d9d1871a28b..0404bb81317d 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -72,6 +72,12 @@ static struct sbiret sbi_system_suspend_raw(unsigned long sleep_type, unsigned l
> return sbi_ecall(SBI_EXT_SUSP, 0, sleep_type, resume_addr, opaque, 0, 0, 0);
> }
>
> +void sbi_bad_fid(int ext)
> +{
> + struct sbiret ret = sbi_ecall(ext, 0xbad, 0, 0, 0, 0, 0, 0);
> + sbiret_report_error(&ret, SBI_ERR_NOT_SUPPORTED, "Bad FID");
> +}
> +
> static void start_cpu(void *data)
> {
> /* nothing to do */
> @@ -178,6 +184,8 @@ static void check_base(void)
>
> report_prefix_push("base");
>
> + sbi_bad_fid(SBI_EXT_BASE);
> +
> ret = sbi_base(SBI_EXT_BASE_GET_SPEC_VERSION, 0);
>
> report_prefix_push("spec_version");
> @@ -331,6 +339,8 @@ static void check_time(void)
> return;
> }
>
> + sbi_bad_fid(SBI_EXT_TIME);
> +
> report_prefix_push("set_timer");
>
> install_irq_handler(IRQ_S_TIMER, timer_irq_handler);
> @@ -438,6 +448,8 @@ static void check_ipi(void)
> return;
> }
>
> + sbi_bad_fid(SBI_EXT_IPI);
> +
> if (nr_cpus_present < 2) {
> report_skip("At least 2 cpus required");
> report_prefix_pop();
> @@ -731,6 +743,8 @@ static void check_hsm(void)
> return;
> }
>
> + sbi_bad_fid(SBI_EXT_HSM);
> +
> report_prefix_push("hart_get_status");
>
> hartid = current_thread_info()->hartid;
> @@ -1210,6 +1224,8 @@ static void check_dbcn(void)
> return;
> }
>
> + sbi_bad_fid(SBI_EXT_DBCN);
> +
> report_prefix_push("write");
>
> dbcn_write_test(DBCN_WRITE_TEST_STRING, num_bytes, false);
> @@ -1475,6 +1491,8 @@ static void check_susp(void)
> return;
> }
>
> + sbi_bad_fid(SBI_EXT_SUSP);
> +
> timer_setup(susp_timer);
> local_irq_enable();
> timer_start(SBI_SUSP_TIMER_DURATION_US);
Hi Andrew,
Even though it does not validate *all* invalid fid (which would be of
course too long), at least that's something.
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] 17+ messages in thread
* Re: [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new
2025-02-27 14:19 [kvm-unit-tests PATCH v2 00/11] riscv: sbi: Test improvements and a couple new Andrew Jones
` (10 preceding siblings ...)
2025-02-27 14:22 ` [kvm-unit-tests PATCH v2 11/11] riscv: sbi: Add bad fid tests Andrew Jones
@ 2025-03-04 9:31 ` Andrew Jones
11 siblings, 0 replies; 17+ messages in thread
From: Andrew Jones @ 2025-03-04 9:31 UTC (permalink / raw)
To: kvm-riscv; +Cc: atishp, cleger, jamestiotio
On Thu, Feb 27, 2025 at 03:19:24PM +0100, Andrew Jones wrote:
> Improvements:
> - Ensure system suspend test won't hang
> - Ensure HSM suspend tests won't hang
> - Ensure state is cleaned up at the end of one extension's test before
> starting another
> - Probe SUSP and skip cleanly
> - Minor SUSP test output improvement
> - Dropped upper bits fwft test for misaligned_exc_deleg
>
> New tests:
> - Check bad FIDs for all extensions
> - Check SUSP sleep_type upper bits are ignored on RV64
> - FWFT pte-hw-ad-updating tests
>
> v2:
> - Dropped upper bits fwft test for misaligned_exc_deleg rather than
> change to kfail
> - Added FWFT pte-hw-ad-updating tests
> - Added a couple Clément tags
>
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] 17+ messages in thread