diff mbox series

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

Message ID 20240411000752.955910-25-atishp@rivosinc.com (mailing list archive)
State Superseded
Headers show
Series RISC-V SBI v2.0 PMU improvements and Perf sampling in KVM guest | expand

Checks

Context Check Description
conchuod/vmtest-fixes-PR fail merge-conflict
conchuod/vmtest-for-next-PR success PR summary
conchuod/patch-24-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh
conchuod/patch-24-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh
conchuod/patch-24-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh
conchuod/patch-24-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh
conchuod/patch-24-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh
conchuod/patch-24-test-6 success .github/scripts/patches/tests/checkpatch.sh
conchuod/patch-24-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh
conchuod/patch-24-test-8 success .github/scripts/patches/tests/header_inline.sh
conchuod/patch-24-test-9 success .github/scripts/patches/tests/kdoc.sh
conchuod/patch-24-test-10 success .github/scripts/patches/tests/module_param.sh
conchuod/patch-24-test-11 success .github/scripts/patches/tests/verify_fixes.sh
conchuod/patch-24-test-12 success .github/scripts/patches/tests/verify_signedoff.sh

Commit Message

Atish Kumar Patra April 11, 2024, 12:07 a.m. UTC
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@ventanamicro.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 .../selftests/kvm/riscv/sbi_pmu_test.c        | 77 ++++++++++++++++---
 1 file changed, 68 insertions(+), 9 deletions(-)

Comments

Andrew Jones April 15, 2024, 1:43 p.m. UTC | #1
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@ventanamicro.com>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  .../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.

> +
>  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'

> +		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
Atish Kumar Patra April 16, 2024, 8:49 a.m. UTC | #2
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@ventanamicro.com>
>> Signed-off-by: Atish Patra <atishp@rivosinc.com>
>> ---
>>   .../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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
diff mbox series

Patch

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;
+
 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;
+		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;
 }