Message ID | 20250316123209.100561-1-akshaybehl231@gmail.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [kvm-unit-tests] riscv: Refactor SBI FWFT lock tests | expand |
On 16/03/2025 13:32, Akshay Behl wrote: > This patch adds a generic function for lock tests for all > the sbi fwft features. It expects the feature is already > locked before being called and tests the locked feature. > > Signed-off-by: Akshay Behl <akshaybehl231@gmail.com> > --- > riscv/sbi-fwft.c | 48 ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 32 insertions(+), 16 deletions(-) > > diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c > index 581cbf6b..5c0a7f6f 100644 > --- a/riscv/sbi-fwft.c > +++ b/riscv/sbi-fwft.c > @@ -74,6 +74,33 @@ static void fwft_check_reset(uint32_t feature, unsigned long reset) > sbiret_report(&ret, SBI_SUCCESS, reset, "resets to %lu", reset); > } > > +/* Must be called after locking the feature using SBI_FWFT_SET_FLAG_LOCK */ > +static void fwft_feature_lock_test(int32_t feature, unsigned long locked_value) > +{ > + struct sbiret ret; > + unsigned long alt_value = locked_value ? 0 : 1; Hi Akshay, This will work for boolean FWFT features but might not work for PMLEN for instance. It could be good to pass the alt value (or values for PMLEN) as an argument to this function. Thanks, Clément > + > + ret = fwft_set(feature, locked_value, 0); > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > + "Set locked feature to %lu without lock", locked_value); > + > + ret = fwft_set(feature, locked_value, SBI_FWFT_SET_FLAG_LOCK); > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > + "Set locked feature to %lu with lock", locked_value); > + > + ret = fwft_set(feature, alt_value, 0); > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > + "Set locked feature to %lu without lock", alt_value); > + > + ret = fwft_set(feature, alt_value, SBI_FWFT_SET_FLAG_LOCK); > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > + "Set locked feature to %lu with lock", alt_value); > + > + ret = fwft_get(feature); > + sbiret_report(&ret, SBI_SUCCESS, locked_value, > + "Get locked feature value %lu", locked_value); > +} > + > static void fwft_check_base(void) > { > report_prefix_push("base"); > @@ -181,11 +208,9 @@ static void fwft_check_misaligned_exc_deleg(void) > /* Lock the feature */ > ret = fwft_misaligned_exc_set(0, SBI_FWFT_SET_FLAG_LOCK); > sbiret_report_error(&ret, SBI_SUCCESS, "Set misaligned deleg feature value 0 and lock"); > - ret = fwft_misaligned_exc_set(1, 0); > - sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > - "Set locked misaligned deleg feature to new value"); > - ret = fwft_misaligned_exc_get(); > - sbiret_report(&ret, SBI_SUCCESS, 0, "Get misaligned deleg locked value 0"); > + > + /* Test feature lock */ > + fwft_feature_lock_test(SBI_FWFT_MISALIGNED_EXC_DELEG, 0); > > report_prefix_pop(); > } > @@ -326,17 +351,8 @@ adue_inval_tests: > 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); > + /* Test the feature lock */ > + fwft_feature_lock_test(SBI_FWFT_PTE_AD_HW_UPDATING, enabled); > > adue_done: > install_exception_handler(EXC_LOAD_PAGE_FAULT, NULL);
On Mon, Mar 17, 2025 at 09:20:27AM +0100, Clément Léger wrote: > > > On 16/03/2025 13:32, Akshay Behl wrote: > > This patch adds a generic function for lock tests for all > > the sbi fwft features. It expects the feature is already > > locked before being called and tests the locked feature. > > > > Signed-off-by: Akshay Behl <akshaybehl231@gmail.com> > > --- > > riscv/sbi-fwft.c | 48 ++++++++++++++++++++++++++++++++---------------- > > 1 file changed, 32 insertions(+), 16 deletions(-) > > > > diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c > > index 581cbf6b..5c0a7f6f 100644 > > --- a/riscv/sbi-fwft.c > > +++ b/riscv/sbi-fwft.c > > @@ -74,6 +74,33 @@ static void fwft_check_reset(uint32_t feature, unsigned long reset) > > sbiret_report(&ret, SBI_SUCCESS, reset, "resets to %lu", reset); > > } > > > > +/* Must be called after locking the feature using SBI_FWFT_SET_FLAG_LOCK */ > > +static void fwft_feature_lock_test(int32_t feature, unsigned long locked_value) ^ ^ uint32_t > > +{ > > + struct sbiret ret; > > + unsigned long alt_value = locked_value ? 0 : 1; > > Hi Akshay, > > This will work for boolean FWFT features but might not work for PMLEN > for instance. It could be good to pass the alt value (or values for > PMLEN) as an argument to this function. Yup, static void fwft_feature_lock_test_values(uint32_t feature, size_t nr_values, unsigned long test_values[]) { for (i = 0; i < nr_values; ++i) { ... test without lock flag ... ... test with lock flag ... } ... test get ... } static void fwft_feature_lock_test(uint32_t feature) { unsigned long values[] = { 0, 1 }; fwft_feature_lock_test_values(feature, 2, values); } So we have fwft_feature_lock_test() for boolean features (likely most of them) and also fwft_feature_lock_test_values() for everything else. Thanks, drew > > Thanks, > > Clément > > > + > > + ret = fwft_set(feature, locked_value, 0); > > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > > + "Set locked feature to %lu without lock", locked_value); > > + > > + ret = fwft_set(feature, locked_value, SBI_FWFT_SET_FLAG_LOCK); > > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > > + "Set locked feature to %lu with lock", locked_value); > > + > > + ret = fwft_set(feature, alt_value, 0); > > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > > + "Set locked feature to %lu without lock", alt_value); > > + > > + ret = fwft_set(feature, alt_value, SBI_FWFT_SET_FLAG_LOCK); > > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > > + "Set locked feature to %lu with lock", alt_value); > > + > > + ret = fwft_get(feature); > > + sbiret_report(&ret, SBI_SUCCESS, locked_value, > > + "Get locked feature value %lu", locked_value); > > +} > > + > > static void fwft_check_base(void) > > { > > report_prefix_push("base"); > > @@ -181,11 +208,9 @@ static void fwft_check_misaligned_exc_deleg(void) > > /* Lock the feature */ > > ret = fwft_misaligned_exc_set(0, SBI_FWFT_SET_FLAG_LOCK); > > sbiret_report_error(&ret, SBI_SUCCESS, "Set misaligned deleg feature value 0 and lock"); > > - ret = fwft_misaligned_exc_set(1, 0); > > - sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > > - "Set locked misaligned deleg feature to new value"); > > - ret = fwft_misaligned_exc_get(); > > - sbiret_report(&ret, SBI_SUCCESS, 0, "Get misaligned deleg locked value 0"); > > + > > + /* Test feature lock */ > > + fwft_feature_lock_test(SBI_FWFT_MISALIGNED_EXC_DELEG, 0); > > > > report_prefix_pop(); > > } > > @@ -326,17 +351,8 @@ adue_inval_tests: > > 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); > > + /* Test the feature lock */ > > + fwft_feature_lock_test(SBI_FWFT_PTE_AD_HW_UPDATING, enabled); > > > > adue_done: > > install_exception_handler(EXC_LOAD_PAGE_FAULT, NULL); >
diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c index 581cbf6b..5c0a7f6f 100644 --- a/riscv/sbi-fwft.c +++ b/riscv/sbi-fwft.c @@ -74,6 +74,33 @@ static void fwft_check_reset(uint32_t feature, unsigned long reset) sbiret_report(&ret, SBI_SUCCESS, reset, "resets to %lu", reset); } +/* Must be called after locking the feature using SBI_FWFT_SET_FLAG_LOCK */ +static void fwft_feature_lock_test(int32_t feature, unsigned long locked_value) +{ + struct sbiret ret; + unsigned long alt_value = locked_value ? 0 : 1; + + ret = fwft_set(feature, locked_value, 0); + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, + "Set locked feature to %lu without lock", locked_value); + + ret = fwft_set(feature, locked_value, SBI_FWFT_SET_FLAG_LOCK); + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, + "Set locked feature to %lu with lock", locked_value); + + ret = fwft_set(feature, alt_value, 0); + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, + "Set locked feature to %lu without lock", alt_value); + + ret = fwft_set(feature, alt_value, SBI_FWFT_SET_FLAG_LOCK); + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, + "Set locked feature to %lu with lock", alt_value); + + ret = fwft_get(feature); + sbiret_report(&ret, SBI_SUCCESS, locked_value, + "Get locked feature value %lu", locked_value); +} + static void fwft_check_base(void) { report_prefix_push("base"); @@ -181,11 +208,9 @@ static void fwft_check_misaligned_exc_deleg(void) /* Lock the feature */ ret = fwft_misaligned_exc_set(0, SBI_FWFT_SET_FLAG_LOCK); sbiret_report_error(&ret, SBI_SUCCESS, "Set misaligned deleg feature value 0 and lock"); - ret = fwft_misaligned_exc_set(1, 0); - sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, - "Set locked misaligned deleg feature to new value"); - ret = fwft_misaligned_exc_get(); - sbiret_report(&ret, SBI_SUCCESS, 0, "Get misaligned deleg locked value 0"); + + /* Test feature lock */ + fwft_feature_lock_test(SBI_FWFT_MISALIGNED_EXC_DELEG, 0); report_prefix_pop(); } @@ -326,17 +351,8 @@ adue_inval_tests: 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); + /* Test the feature lock */ + fwft_feature_lock_test(SBI_FWFT_PTE_AD_HW_UPDATING, enabled); adue_done: install_exception_handler(EXC_LOAD_PAGE_FAULT, NULL);
This patch adds a generic function for lock tests for all the sbi fwft features. It expects the feature is already locked before being called and tests the locked feature. Signed-off-by: Akshay Behl <akshaybehl231@gmail.com> --- riscv/sbi-fwft.c | 48 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 16 deletions(-)