Re: [PATCH 0/3] Assorted fixes for PMU

[Alistair Francis]

On Tue, May 14, 2024 at 5:15 PM Atish Kumar Patra <atishp@rivosinc.com> wrote:
>
> On Mon, May 13, 2024 at 11:29 PM Alistair Francis <alistair23@gmail.com> 
> wrote:
> >
> > On Tue, Apr 30, 2024 at 5:29 AM Atish Patra <atishp@rivosinc.com> wrote:
> > >
> > > This series contains few miscallenous fixes related to hpmcounters
> > > and related code. The first patch fixes an issue with cycle/instret
> > > counters overcouting while the remaining two are more for specification
> > > compliance.
> > >
> > > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > > ---
> > > Atish Patra (3):
> > >       target/riscv: Save counter values during countinhibit update
> > >       target/riscv: Enforce WARL behavior for scounteren/hcounteren
> > >       target/riscv: Fix the predicate functions for mhpmeventhX CSRs
> >
> > Thanks!
> >
> > Applied to riscv-to-apply.next
> >
>
> Hi Alistair,
> Thanks for your review. But the patch 1 had some comments about
> vmstate which needs updating.

Ah, I did read the comments but forgot that you were going to bump the version.

> We also found a few more fixes that I was planning to include in v2.

I found that patch `target/riscv: Save counter values during
countinhibit update` gives me this error as well

../target/riscv/csr.c: In function ‘write_mcountinhibit’:
../target/riscv/csr.c:1981:44: error: too few arguments to function ‘get_ticks’
1981 |                 counter->mhpmcounter_val = get_ticks(false) -
     |                                            ^~~~~~~~~
../target/riscv/csr.c:765:21: note: declared here
 765 | static target_ulong get_ticks(bool shift, bool instructions)
     |                     ^~~~~~~~~
../target/riscv/csr.c:1985:49: error: too few arguments to function ‘get_ticks’
1985 |                     counter->mhpmcounterh_val = get_ticks(false) -
     |                                                 ^~~~~~~~~
../target/riscv/csr.c:765:21: note: declared here
 765 | static target_ulong get_ticks(bool shift, bool instructions)
     |                     ^~~~~~~~~



>
> I can send a separate fixes series on top riscv-to-apply.next or this
> series can be dropped for the time being.

I'm going to drop it due to the build error above

Alistair

> You can queue it v2 later. Let me know what you prefer.
>
>
> > Alistair
> >
> > >
> > >  target/riscv/cpu.h     |   1 -
> > >  target/riscv/csr.c     | 111 
> > > ++++++++++++++++++++++++++++++-------------------
> > >  target/riscv/machine.c |   1 -
> > >  3 files changed, 68 insertions(+), 45 deletions(-)
> > > ---
> > > base-commit: 1642f979a71a5667a05070be2df82f48bd43ad7a
> > > change-id: 20240428-countinhibit_fix-c6a1c11f4375
> > > --
> > > Regards,
> > > Atish patra
> > >
> > >