| Message ID | 20250318174349.178646-1-akshaybehl231@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [kvm-unit-tests,v2] riscv: Refactor SBI FWFT lock tests | expand |
On Tue, Mar 18, 2025 at 11:13:49PM +0530, 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> > --- > v2: > - Made changes to handel non boolean feature tests. > riscv/sbi-fwft.c | 49 ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 33 insertions(+), 16 deletions(-) > > diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c > index 581cbf6b..7d9735d7 100644 > --- a/riscv/sbi-fwft.c > +++ b/riscv/sbi-fwft.c > @@ -74,6 +74,34 @@ 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_values(uint32_t feature, size_t nr_values, > + unsigned long test_values[], unsigned long locked_value) What happened to the indentation here? Please proofread your patches. > +{ > + struct sbiret ret; We could push a "locked" prefix here and then... > + > + for (int i = 0; i < nr_values; ++i) { > + ret = fwft_set(feature, test_values[i], 0); > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > + "Set locked feature to %lu without lock", test_values[i]); ^ flag > + > + ret = fwft_set(feature, test_values[i], SBI_FWFT_SET_FLAG_LOCK); > + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, > + "Set locked feature to %lu with lock", test_values[i]); ^ flag > + } > + > + ret = fwft_get(feature); > + sbiret_report(&ret, SBI_SUCCESS, locked_value, > + "Get locked feature value %lu", locked_value); ...drop 'locked feature' from all the reports above. Then pop the prefix here. > +} > + > +static void fwft_feature_lock_test(uint32_t feature, unsigned long locked_value) > +{ > + unsigned long values[] = {0, 1}; > + > + fwft_feature_lock_test_values(feature, 2 , values, locked_value); ^ extra space > +} > + > static void fwft_check_base(void) > { > report_prefix_push("base"); > @@ -181,11 +209,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 */ Can drop this comment since the function name says the same thing. > + fwft_feature_lock_test(SBI_FWFT_MISALIGNED_EXC_DELEG, 0); > > report_prefix_pop(); > } > @@ -326,17 +352,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 */ same comment > + fwft_feature_lock_test(SBI_FWFT_PTE_AD_HW_UPDATING, enabled); > > adue_done: > install_exception_handler(EXC_LOAD_PAGE_FAULT, NULL); > -- > 2.34.1 > Thanks, drew
diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c index 581cbf6b..7d9735d7 100644 --- a/riscv/sbi-fwft.c +++ b/riscv/sbi-fwft.c @@ -74,6 +74,34 @@ 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_values(uint32_t feature, size_t nr_values, + unsigned long test_values[], unsigned long locked_value) +{ + struct sbiret ret; + + for (int i = 0; i < nr_values; ++i) { + ret = fwft_set(feature, test_values[i], 0); + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, + "Set locked feature to %lu without lock", test_values[i]); + + ret = fwft_set(feature, test_values[i], SBI_FWFT_SET_FLAG_LOCK); + sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, + "Set locked feature to %lu with lock", test_values[i]); + } + + ret = fwft_get(feature); + sbiret_report(&ret, SBI_SUCCESS, locked_value, + "Get locked feature value %lu", locked_value); +} + +static void fwft_feature_lock_test(uint32_t feature, unsigned long locked_value) +{ + unsigned long values[] = {0, 1}; + + fwft_feature_lock_test_values(feature, 2 , values, locked_value); +} + static void fwft_check_base(void) { report_prefix_push("base"); @@ -181,11 +209,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 +352,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> --- v2: - Made changes to handel non boolean feature tests. riscv/sbi-fwft.c | 49 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-)