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

[Atish Patra]

On Tue, May 14, 2024 at 3:18 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> 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)
>      |                     ^~~~~~~~~
>

Yeah. Clement's patch got in. I will rebase and update the series
based on the riscv-to-apply.next.

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


-- 
Regards,
Atish