Re: [PATCH v6 24/24] KVM: riscv: selftests: Add commandline option for SBI PMU test

[Atish Patra]

On 4/15/24 06:43, Andrew Jones wrote:
> On Wed, Apr 10, 2024 at 05:07:52PM -0700, Atish Patra wrote:
> > SBI PMU test comprises of multiple tests and user may want to run
> > only a subset depending on the platform. The most common case would
> > be to run all to validate all the tests. However, some platform may
> > not support all events or all ISA extensions.
> >
> > The commandline option allows user to disable particular test if they
> > want to.
> >
> > Suggested-by: Andrew Jones <ajones@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Atish Patra <atishp@xxxxxxxxxxxx>
> > ---
> >   .../selftests/kvm/riscv/sbi_pmu_test.c        | 77 ++++++++++++++++---
> >   1 file changed, 68 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c
> > index 0fd9b76ae838..57025b07a403 100644
> > --- a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c
> > +++ b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c
> > @@ -33,6 +33,16 @@ static unsigned long counter_mask_available;
> >   
> >   static bool illegal_handler_invoked;
> >   
> > +enum sbi_pmu_test_id {
> > +	SBI_PMU_TEST_BASIC = 0,
> > +	SBI_PMU_TEST_EVENTS,
> > +	SBI_PMU_TEST_SNAPSHOT,
> > +	SBI_PMU_TEST_OVERFLOW,
> > +	SBI_PMU_TEST_MAX,
> > +};
> > +
> > +static int disabled_test_id = SBI_PMU_TEST_MAX;
>
> I think we should allow specifying '-d' multiple times in case a user
> wants to disable more than one type of test. So we should set a bit
> in this variable instead of assigning it. We could also flip it
> around. If the user uses '-e' to enable test type then only type
> will be run, but then we'd probably want to allow multiple '-e'
> too so it doesn't gain anything.

Sure. Added multiple disable option.

> > +
> >   unsigned long pmu_csr_read_num(int csr_num)
> >   {
> >   #define switchcase_csr_read(__csr_num, __val)		{\
> > @@ -608,19 +618,68 @@ static void test_vm_events_overflow(void *guest_code)
> >   	test_vm_destroy(vm);
> >   }
> >   
> > -int main(void)
> > +static void test_print_help(char *name)
> >   {
> > -	test_vm_basic_test(test_pmu_basic_sanity);
> > -	pr_info("SBI PMU basic test : PASS\n");
> > +	pr_info("Usage: %s [-h] [-d <test name>]\n", name);
> > +	pr_info("\t-d: Test to disable. Available tests are 'basic', 'events', 'snapshot', 'overflow'\n");
> > +	pr_info("\t-h: print this help screen\n");
> > +}
> >   
> > -	test_vm_events_test(test_pmu_events);
> > -	pr_info("SBI PMU event verification test : PASS\n");
> > +static bool parse_args(int argc, char *argv[])
> > +{
> > +	int opt;
> > +
> > +	while ((opt = getopt(argc, argv, "hd:")) != -1) {
> > +		switch (opt) {
> > +		case 'd':
> > +			if (!strncmp("basic", optarg, 5))
> > +				disabled_test_id = SBI_PMU_TEST_BASIC;
> > +			else if (!strncmp("events", optarg, 6))
> > +				disabled_test_id = SBI_PMU_TEST_EVENTS;
> > +			else if (!strncmp("snapshot", optarg, 8))
> > +				disabled_test_id = SBI_PMU_TEST_SNAPSHOT;
> > +			else if (!strncmp("overflow", optarg, 8))
> > +				disabled_test_id = SBI_PMU_TEST_OVERFLOW;
> > +			else
> > +				goto done;
> > +			break;
> > +		break;
>
> Extra 'break'

Fixed. Thanks for catching this.

> > +		case 'h':
> > +		default:
> > +			goto done;
> > +		}
> > +	}
> >   
> > -	test_vm_events_snapshot_test(test_pmu_events_snaphost);
> > -	pr_info("SBI PMU event verification with snapshot test : PASS\n");
> > +	return true;
> > +done:
> > +	test_print_help(argv[0]);
> > +	return false;
> > +}
> >   
> > -	test_vm_events_overflow(test_pmu_events_overflow);
> > -	pr_info("SBI PMU event verification with overflow test : PASS\n");
> > +int main(int argc, char *argv[])
> > +{
> > +	if (!parse_args(argc, argv))
> > +		exit(KSFT_SKIP);
> > +
> > +	if (disabled_test_id != SBI_PMU_TEST_BASIC) {
> > +		test_vm_basic_test(test_pmu_basic_sanity);
> > +		pr_info("SBI PMU basic test : PASS\n");
> > +	}
> > +
> > +	if (disabled_test_id != SBI_PMU_TEST_EVENTS) {
> > +		test_vm_events_test(test_pmu_events);
> > +		pr_info("SBI PMU event verification test : PASS\n");
> > +	}
> > +
> > +	if (disabled_test_id != SBI_PMU_TEST_SNAPSHOT) {
> > +		test_vm_events_snapshot_test(test_pmu_events_snaphost);
> > +		pr_info("SBI PMU event verification with snapshot test : PASS\n");
> > +	}
> > +
> > +	if (disabled_test_id != SBI_PMU_TEST_OVERFLOW) {
> > +		test_vm_events_overflow(test_pmu_events_overflow);
> > +		pr_info("SBI PMU event verification with overflow test : PASS\n");
> > +	}
> >   
> >   	return 0;
> >   }
> > --
> > 2.34.1
>
> Thanks,
> drew
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv