public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH] arm: pmu: Actually use counter 0 in test_event_counter_config()
@ 2025-02-03 18:10 Oliver Upton
  2025-02-04  7:44 ` Eric Auger
  2025-02-04 13:20 ` Andrew Jones
  0 siblings, 2 replies; 3+ messages in thread
From: Oliver Upton @ 2025-02-03 18:10 UTC (permalink / raw)
  To: kvmarm; +Cc: kvm, Andrew Jones, Alexandru Elisei, Oliver Upton, Eric Auger

test_event_counter_config() checks that there is at least one event
counter but mistakenly uses counter 1 for part of the test.

Most implementations have more than a single event counter which is
probably why this went unnoticed. However, due to limitations of the
underlying hardware, KVM's PMUv3 emulation on Apple silicon can only
provide 1 event counter.

Consistenly use counter 0 throughout the test, matching the precondition
and allowing the test to pass on Apple parts.

Cc: Eric Auger <eric.auger@redhat.com>
Fixes: 4ce2a804 ("arm: pmu: Basic event counter Tests")
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arm/pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arm/pmu.c b/arm/pmu.c
index 9ff7a301..2dc0822b 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -396,13 +396,13 @@ static void test_event_counter_config(void)
 	 * Test setting through PMESELR/PMXEVTYPER and PMEVTYPERn read,
 	 * select counter 0
 	 */
-	write_sysreg(1, PMSELR_EL0);
+	write_sysreg(0, PMSELR_EL0);
 	/* program this counter to count unsupported event */
 	write_sysreg(0xEA, PMXEVTYPER_EL0);
 	write_sysreg(0xdeadbeef, PMXEVCNTR_EL0);
-	report((read_regn_el0(pmevtyper, 1) & 0xFFF) == 0xEA,
+	report((read_regn_el0(pmevtyper, 0) & 0xFFF) == 0xEA,
 		"PMESELR/PMXEVTYPER/PMEVTYPERn");
-	report((read_regn_el0(pmevcntr, 1) == 0xdeadbeef),
+	report((read_regn_el0(pmevcntr, 0) == 0xdeadbeef),
 		"PMESELR/PMXEVCNTR/PMEVCNTRn");
 
 	/* try to configure an unsupported event within the range [0x0, 0x3F] */

base-commit: 1f08a91a41402b0e032ecce8ed1b5952cbfca0ea
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [kvm-unit-tests PATCH] arm: pmu: Actually use counter 0 in test_event_counter_config()
  2025-02-03 18:10 [kvm-unit-tests PATCH] arm: pmu: Actually use counter 0 in test_event_counter_config() Oliver Upton
@ 2025-02-04  7:44 ` Eric Auger
  2025-02-04 13:20 ` Andrew Jones
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Auger @ 2025-02-04  7:44 UTC (permalink / raw)
  To: Oliver Upton, kvmarm; +Cc: kvm, Andrew Jones, Alexandru Elisei

Hi Oliver,


On 2/3/25 7:10 PM, Oliver Upton wrote:
> test_event_counter_config() checks that there is at least one event
> counter but mistakenly uses counter 1 for part of the test.
>
> Most implementations have more than a single event counter which is
> probably why this went unnoticed. However, due to limitations of the
> underlying hardware, KVM's PMUv3 emulation on Apple silicon can only
> provide 1 event counter.
>
> Consistenly use counter 0 throughout the test, matching the precondition
> and allowing the test to pass on Apple parts.
>
> Cc: Eric Auger <eric.auger@redhat.com>
> Fixes: 4ce2a804 ("arm: pmu: Basic event counter Tests")
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arm/pmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 9ff7a301..2dc0822b 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -396,13 +396,13 @@ static void test_event_counter_config(void)
>  	 * Test setting through PMESELR/PMXEVTYPER and PMEVTYPERn read,
>  	 * select counter 0
>  	 */
> -	write_sysreg(1, PMSELR_EL0);
> +	write_sysreg(0, PMSELR_EL0);
>  	/* program this counter to count unsupported event */
>  	write_sysreg(0xEA, PMXEVTYPER_EL0);
>  	write_sysreg(0xdeadbeef, PMXEVCNTR_EL0);
> -	report((read_regn_el0(pmevtyper, 1) & 0xFFF) == 0xEA,
> +	report((read_regn_el0(pmevtyper, 0) & 0xFFF) == 0xEA,
>  		"PMESELR/PMXEVTYPER/PMEVTYPERn");
> -	report((read_regn_el0(pmevcntr, 1) == 0xdeadbeef),
> +	report((read_regn_el0(pmevcntr, 0) == 0xdeadbeef),
>  		"PMESELR/PMXEVCNTR/PMEVCNTRn");
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thank you for noticing and fixing that

Eric
>  
>  	/* try to configure an unsupported event within the range [0x0, 0x3F] */
>
> base-commit: 1f08a91a41402b0e032ecce8ed1b5952cbfca0ea


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [kvm-unit-tests PATCH] arm: pmu: Actually use counter 0 in test_event_counter_config()
  2025-02-03 18:10 [kvm-unit-tests PATCH] arm: pmu: Actually use counter 0 in test_event_counter_config() Oliver Upton
  2025-02-04  7:44 ` Eric Auger
@ 2025-02-04 13:20 ` Andrew Jones
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Jones @ 2025-02-04 13:20 UTC (permalink / raw)
  To: Oliver Upton; +Cc: kvmarm, kvm, Alexandru Elisei, Eric Auger

On Mon, Feb 03, 2025 at 10:10:26AM -0800, Oliver Upton wrote:
> test_event_counter_config() checks that there is at least one event
> counter but mistakenly uses counter 1 for part of the test.
> 
> Most implementations have more than a single event counter which is
> probably why this went unnoticed. However, due to limitations of the
> underlying hardware, KVM's PMUv3 emulation on Apple silicon can only
> provide 1 event counter.
> 
> Consistenly use counter 0 throughout the test, matching the precondition
> and allowing the test to pass on Apple parts.
> 
> Cc: Eric Auger <eric.auger@redhat.com>
> Fixes: 4ce2a804 ("arm: pmu: Basic event counter Tests")
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arm/pmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 9ff7a301..2dc0822b 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -396,13 +396,13 @@ static void test_event_counter_config(void)
>  	 * Test setting through PMESELR/PMXEVTYPER and PMEVTYPERn read,
>  	 * select counter 0
>  	 */
> -	write_sysreg(1, PMSELR_EL0);
> +	write_sysreg(0, PMSELR_EL0);
>  	/* program this counter to count unsupported event */
>  	write_sysreg(0xEA, PMXEVTYPER_EL0);
>  	write_sysreg(0xdeadbeef, PMXEVCNTR_EL0);
> -	report((read_regn_el0(pmevtyper, 1) & 0xFFF) == 0xEA,
> +	report((read_regn_el0(pmevtyper, 0) & 0xFFF) == 0xEA,
>  		"PMESELR/PMXEVTYPER/PMEVTYPERn");
> -	report((read_regn_el0(pmevcntr, 1) == 0xdeadbeef),
> +	report((read_regn_el0(pmevcntr, 0) == 0xdeadbeef),
>  		"PMESELR/PMXEVCNTR/PMEVCNTRn");
>  
>  	/* try to configure an unsupported event within the range [0x0, 0x3F] */
> 
> base-commit: 1f08a91a41402b0e032ecce8ed1b5952cbfca0ea
> -- 
> 2.39.5
>

Merged.

Thanks,
drew

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-02-04 13:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-03 18:10 [kvm-unit-tests PATCH] arm: pmu: Actually use counter 0 in test_event_counter_config() Oliver Upton
2025-02-04  7:44 ` Eric Auger
2025-02-04 13:20 ` Andrew Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox