Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
MIME-Version: 1.0
References: <20231122221526.2750966-1-rananta@google.com> <d5cc3cf1-7b39-9ca3-adf2-224007c751fe@redhat.com>
In-Reply-To: <d5cc3cf1-7b39-9ca3-adf2-224007c751fe@redhat.com>
From:   Raghavendra Rao Ananta <rananta@google.com>
Date:   Mon, 27 Nov 2023 13:41:29 -0800
Message-ID: <CAJHc60zinMfkjFF=QS5R6YO0B8_Anmyw2cZ2f0QEnW0i=hWHcQ@mail.gmail.com>
Subject: Re: [PATCH] KVM: selftests: aarch64: Remove unused functions from
 vpmu test
To:     Shaoqin Huang <shahuang@redhat.com>
Cc:     Oliver Upton <oliver.upton@linux.dev>,
        Marc Zyngier <maz@kernel.org>,
        James Morse <james.morse@arm.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Zenghui Yu <yuzenghui@huawei.com>,
        linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Shaoqin,

On Wed, Nov 22, 2023 at 10:43=E2=80=AFPM 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.c=
om/
>
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, unsign=
ed long val)
> >       isb();
> >   }
> >
> > -static inline void enable_counter(int idx)
> > -{
> > -     uint64_t v =3D read_sysreg(pmcntenset_el0);
> > -
> > -     write_sysreg(BIT(idx) | v, pmcntenset_el0);
> > -     isb();
> > -}
> > -
> > -static inline void disable_counter(int idx)
> > -{
> > -     uint64_t v =3D read_sysreg(pmcntenset_el0);
> > -
> > -     write_sysreg(BIT(idx) | v, pmcntenclr_el0);
> > -     isb();
> > -}
> > -
> >   static void pmu_disable_reset(void)
> >   {
> >       uint64_t pmcr =3D read_sysreg(pmcr_el0);
>