[PATCH] KVM: selftests: aarch64: Remove unused functions from vpmu test

[PATCH] KVM: selftests: aarch64: Remove unused functions from vpmu test

[Raghavendra Rao Ananta]

vpmu_counter_access's disable_counter() carries a bug that disables
all the counters that are enabled, instead of just the requested one.
Fortunately, it's not an issue as there are no callers of it. Hence,
instead of fixing it, remove the definition entirely.

Remove enable_counter() as it's unused as well.

Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 .../selftests/kvm/aarch64/vpmu_counter_access.c  | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
index 5ea78986e665f..e2f0b720cbfcf 100644
--- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
+++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
@@ -94,22 +94,6 @@ static inline void write_sel_evtyper(int sel, unsigned long val)
 	isb();
 }
 
-static inline void enable_counter(int idx)
-{
-	uint64_t v = read_sysreg(pmcntenset_el0);
-
-	write_sysreg(BIT(idx) | v, pmcntenset_el0);
-	isb();
-}
-
-static inline void disable_counter(int idx)
-{
-	uint64_t v = read_sysreg(pmcntenset_el0);
-
-	write_sysreg(BIT(idx) | v, pmcntenclr_el0);
-	isb();
-}
-
 static void pmu_disable_reset(void)
 {
 	uint64_t pmcr = read_sysreg(pmcr_el0);
-- 
2.43.0.rc1.413.gea7ed67945-goog

Re: [PATCH] KVM: selftests: aarch64: Remove unused functions from vpmu test

[Zenghui Yu]

On 2023/11/23 6:15, Raghavendra Rao Ananta wrote:
> vpmu_counter_access's disable_counter() carries a bug that disables
> all the counters that are enabled, instead of just the requested one.
> Fortunately, it's not an issue as there are no callers of it. Hence,
> instead of fixing it, remove the definition entirely.
> 
> Remove enable_counter() as it's unused as well.
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>

Re: [PATCH] KVM: selftests: aarch64: Remove unused functions from vpmu test

[Shaoqin Huang]

Hi Raghavendra,

Those functions might be useful for other pmu tests. Recently I just 
wrote a pmu_event_filter_test[1] and use the enable_counter().

There may have more pmu tests which can use the helper functions, so I 
think we can keep it now. And in my series[1], I have moved them into 
the lib/ as the helper function.

[1]https://lore.kernel.org/all/20231123063750.2176250-1-shahuang@redhat.com/

Thanks,
Shaoqin

On 11/23/23 06:15, Raghavendra Rao Ananta wrote:
> vpmu_counter_access's disable_counter() carries a bug that disables
> all the counters that are enabled, instead of just the requested one.
> Fortunately, it's not an issue as there are no callers of it. Hence,
> instead of fixing it, remove the definition entirely.
> 
> Remove enable_counter() as it's unused as well.
> 
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> ---
>   .../selftests/kvm/aarch64/vpmu_counter_access.c  | 16 ----------------
>   1 file changed, 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
> index 5ea78986e665f..e2f0b720cbfcf 100644
> --- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
> +++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
> @@ -94,22 +94,6 @@ static inline void write_sel_evtyper(int sel, unsigned long val)
>   	isb();
>   }
>   
> -static inline void enable_counter(int idx)
> -{
> -	uint64_t v = read_sysreg(pmcntenset_el0);
> -
> -	write_sysreg(BIT(idx) | v, pmcntenset_el0);
> -	isb();
> -}
> -
> -static inline void disable_counter(int idx)
> -{
> -	uint64_t v = read_sysreg(pmcntenset_el0);
> -
> -	write_sysreg(BIT(idx) | v, pmcntenclr_el0);
> -	isb();
> -}
> -
>   static void pmu_disable_reset(void)
>   {
>   	uint64_t pmcr = read_sysreg(pmcr_el0);

Re: [PATCH] KVM: selftests: aarch64: Remove unused functions from vpmu test

[Raghavendra Rao Ananta]

Hi Shaoqin,

On Wed, Nov 22, 2023 at 10:43 PM Shaoqin Huang <shahuang@redhat.com> wrote:
>
> Hi Raghavendra,
>
> Those functions might be useful for other pmu tests. Recently I just
> wrote a pmu_event_filter_test[1] and use the enable_counter().
>
> There may have more pmu tests which can use the helper functions, so I
> think we can keep it now. And in my series[1], I have moved them into
> the lib/ as the helper function.
>
> [1]https://lore.kernel.org/all/20231123063750.2176250-1-shahuang@redhat.com/
>
Thanks for the pointer. If you are planning to use it, then we can
abandon this patch. However, disable_counter() may need fixing. I'll
comment directly on your patch.

Thank you.
Raghavendra

> Thanks,
> Shaoqin
>
> On 11/23/23 06:15, Raghavendra Rao Ananta wrote:
> > vpmu_counter_access's disable_counter() carries a bug that disables
> > all the counters that are enabled, instead of just the requested one.
> > Fortunately, it's not an issue as there are no callers of it. Hence,
> > instead of fixing it, remove the definition entirely.
> >
> > Remove enable_counter() as it's unused as well.
> >
> > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> > ---
> >   .../selftests/kvm/aarch64/vpmu_counter_access.c  | 16 ----------------
> >   1 file changed, 16 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
> > index 5ea78986e665f..e2f0b720cbfcf 100644
> > --- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
> > +++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
> > @@ -94,22 +94,6 @@ static inline void write_sel_evtyper(int sel, unsigned long val)
> >       isb();
> >   }
> >
> > -static inline void enable_counter(int idx)
> > -{
> > -     uint64_t v = read_sysreg(pmcntenset_el0);
> > -
> > -     write_sysreg(BIT(idx) | v, pmcntenset_el0);
> > -     isb();
> > -}
> > -
> > -static inline void disable_counter(int idx)
> > -{
> > -     uint64_t v = read_sysreg(pmcntenset_el0);
> > -
> > -     write_sysreg(BIT(idx) | v, pmcntenclr_el0);
> > -     isb();
> > -}
> > -
> >   static void pmu_disable_reset(void)
> >   {
> >       uint64_t pmcr = read_sysreg(pmcr_el0);
>

Re: [PATCH] KVM: selftests: aarch64: Remove unused functions from vpmu test

[Oliver Upton]

On Wed, 22 Nov 2023 22:15:26 +0000, Raghavendra Rao Ananta wrote:
> vpmu_counter_access's disable_counter() carries a bug that disables
> all the counters that are enabled, instead of just the requested one.
> Fortunately, it's not an issue as there are no callers of it. Hence,
> instead of fixing it, remove the definition entirely.
> 
> Remove enable_counter() as it's unused as well.
> 
> [...]

Applied to kvmarm/next, thanks!

[1/1] KVM: selftests: aarch64: Remove unused functions from vpmu test
      https://git.kernel.org/kvmarm/kvmarm/c/43b3bedb7cc4

--
Best,
Oliver