From: Peter Maydell <peter.maydell@linaro.org>
To: Alistair Francis <alistair23@gmail.com>
Cc: qemu-devel@nongnu.org, Rob Bradford <rbradford@rivosinc.com>,
LIU Zhiwei <zhiwei_liu@linux.alibaba.com>,
Alistair Francis <alistair.francis@wdc.com>,
Atish Patra <atishp@rivosinc.com>
Subject: Re: [PULL 46/49] target/riscv: Don't assume PMU counters are continuous
Date: Thu, 9 Nov 2023 15:24:06 +0000 [thread overview]
Message-ID: <CAFEAcA_DrOBcQytM+UkMNEFFgEhOYqbKnnSREhRY+UsEwD+RbQ@mail.gmail.com> (raw)
In-Reply-To: <20231107022946.1055027-47-alistair.francis@wdc.com>
On Tue, 7 Nov 2023 at 02:36, Alistair Francis <alistair23@gmail.com> wrote:
>
> From: Rob Bradford <rbradford@rivosinc.com>
>
> Check the PMU available bitmask when checking if a counter is valid
> rather than comparing the index against the number of PMUs.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
> Message-ID: <20231031154000.18134-3-rbradford@rivosinc.com>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> target/riscv/csr.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index fc26b52c88..fde7ce1a53 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -188,7 +188,8 @@ static RISCVException zcmt(CPURISCVState *env, int csrno)
> #if !defined(CONFIG_USER_ONLY)
> static RISCVException mctr(CPURISCVState *env, int csrno)
> {
> - int pmu_num = riscv_cpu_cfg(env)->pmu_num;
> + RISCVCPU *cpu = env_archcpu(env);
> + uint32_t pmu_avail_ctrs = cpu->pmu_avail_ctrs;
> int ctr_index;
> int base_csrno = CSR_MHPMCOUNTER3;
>
> @@ -197,7 +198,7 @@ static RISCVException mctr(CPURISCVState *env, int csrno)
> base_csrno += 0x80;
> }
> ctr_index = csrno - base_csrno;
> - if (!pmu_num || ctr_index >= pmu_num) {
> + if ((BIT(ctr_index) & pmu_avail_ctrs >> 3) == 0) {
> /* The PMU is not enabled or counter is out of range */
> return RISCV_EXCP_ILLEGAL_INST;
> }
Hi; Coverity is not convinced that ctr_index is necessarily
guaranteed to be within the valid range to be an argument
to BIT() (eg that it won't be negative). Looking at the
code as a human I'm pretty unsure too. Could somebody have
a look at this and maybe improve the readability / add an
assertion / fix a bug if any ? (CID 1523910)
More generally there are about half a dozen other riscv
issues in Coverity at the moment, so if somebody who knows
the riscv code could have a look at them that would be great.
thanks
-- PMM
next prev parent reply other threads:[~2023-11-09 15:24 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-07 2:28 [PULL 00/49] riscv-to-apply queue Alistair Francis
2023-11-07 2:28 ` [PULL 01/49] target/riscv: rename ext_ifencei to ext_zifencei Alistair Francis
2023-11-07 2:28 ` [PULL 02/49] target/riscv: rename ext_icsr to ext_zicsr Alistair Francis
2023-11-07 2:28 ` [PULL 03/49] target/riscv: rename ext_icbom to ext_zicbom Alistair Francis
2023-11-07 2:29 ` [PULL 04/49] target/riscv: rename ext_icboz to ext_zicboz Alistair Francis
2023-11-07 2:29 ` [PULL 05/49] target/riscv: Without H-mode mask all HS mode inturrupts in mie Alistair Francis
2023-11-07 2:29 ` [PULL 06/49] target/riscv: Check for async flag in case of RISCV_EXCP_SEMIHOST Alistair Francis
2023-11-07 2:29 ` [PULL 07/49] target/riscv: Set VS* bits to one in mideleg when H-Ext is enabled Alistair Francis
2023-11-07 2:29 ` [PULL 08/49] target/riscv: Split interrupt logic from riscv_cpu_update_mip Alistair Francis
2023-11-07 2:29 ` [PULL 09/49] target/riscv: Add M-mode virtual interrupt and IRQ filtering support Alistair Francis
2023-11-07 2:29 ` [PULL 10/49] target/riscv: Add HS-mode " Alistair Francis
2023-11-07 2:29 ` [PULL 11/49] linux-user/riscv: change default cpu to 'max' Alistair Francis
2023-11-07 2:29 ` [PULL 12/49] docs/system/riscv: update 'virt' machine core limit Alistair Francis
2023-11-07 2:29 ` [PULL 13/49] target/riscv/kvm/kvm-cpu.c: add missing property getters() Alistair Francis
2023-11-07 2:29 ` [PULL 14/49] qapi,risc-v: add query-cpu-model-expansion Alistair Francis
2023-11-07 2:29 ` [PULL 15/49] target/riscv/tcg: add tcg_cpu_finalize_features() Alistair Francis
2023-11-07 2:29 ` [PULL 16/49] target/riscv: handle custom props in qmp_query_cpu_model_expansion Alistair Francis
2023-11-07 2:29 ` [PULL 17/49] target/riscv: add riscv_cpu_accelerator_compatible() Alistair Francis
2023-11-07 2:29 ` [PULL 18/49] target/riscv/riscv-qmp-cmds.c: check CPU accel in query-cpu-model-expansion Alistair Francis
2023-11-07 2:29 ` [PULL 19/49] Add epmp to extensions list and rename it to smepmp Alistair Francis
2023-11-07 2:29 ` [PULL 20/49] target/riscv: pmp: Clear pmp/smepmp bits on reset Alistair Francis
2023-11-07 2:29 ` [PULL 21/49] target/riscv: pmp: Ignore writes when RW=01 Alistair Francis
2023-11-07 2:29 ` [PULL 22/49] target/riscv: add zicntr extension flag for TCG Alistair Francis
2023-11-07 2:29 ` [PULL 23/49] target/riscv/kvm: add zicntr reg Alistair Francis
2023-11-07 2:29 ` [PULL 24/49] target/riscv: add zihpm extension flag for TCG Alistair Francis
2023-11-07 2:29 ` [PULL 25/49] target/riscv/kvm: add zihpm reg Alistair Francis
2023-11-07 2:29 ` [PULL 26/49] target/riscv/kvm: add zicsr, zifencei, zba, zbs, svnapot Alistair Francis
2023-11-07 2:29 ` [PULL 27/49] target/riscv: correct csr_ops[CSR_MSECCFG] Alistair Francis
2023-11-07 2:29 ` [PULL 28/49] MAINTAINERS: update mail address for Weiwei Li Alistair Francis
2023-11-07 2:29 ` [PULL 29/49] target/riscv: Add cfg property for Zvkt extension Alistair Francis
2023-11-07 2:29 ` [PULL 30/49] target/riscv: Expose Zvkt extension property Alistair Francis
2023-11-07 2:29 ` [PULL 31/49] target/riscv: Add cfg property for Zvkb extension Alistair Francis
2023-11-07 2:29 ` [PULL 32/49] target/riscv: Replace Zvbb checking by Zvkb Alistair Francis
2023-11-07 2:29 ` [PULL 33/49] target/riscv: Expose Zvkb extension property Alistair Francis
2023-11-07 2:29 ` [PULL 34/49] target/riscv: Add cfg properties for Zvkn[c|g] extensions Alistair Francis
2023-11-07 2:29 ` [PULL 35/49] target/riscv: Expose Zvkn[c|g] extnesion properties Alistair Francis
2023-11-07 2:29 ` [PULL 36/49] target/riscv: Add cfg properties for Zvks[c|g] extensions Alistair Francis
2023-11-07 2:29 ` [PULL 37/49] target/riscv: Expose Zvks[c|g] extnesion properties Alistair Francis
2023-11-07 2:29 ` [PULL 38/49] target/riscv: Move vector crypto extensions to riscv_cpu_extensions Alistair Francis
2023-11-07 2:29 ` [PULL 39/49] disas/riscv: Add rv_fmt_vd_vs2_uimm format Alistair Francis
2023-11-07 2:29 ` [PULL 40/49] disas/riscv: Add rv_codec_vror_vi for vror.vi Alistair Francis
2023-11-07 2:29 ` [PULL 41/49] disas/riscv: Add support for vector crypto extensions Alistair Francis
2023-11-07 2:29 ` [PULL 42/49] disas/riscv: Replace TABs with space Alistair Francis
2023-11-07 2:29 ` [PULL 43/49] hw/ssi: ibex_spi_host: Clear the interrupt even if disabled Alistair Francis
2023-11-07 2:29 ` [PULL 44/49] target/riscv: cpu: Set the OpenTitan priv to 1.12.0 Alistair Francis
2023-11-07 2:29 ` [PULL 45/49] target/riscv: Propagate error from PMU setup Alistair Francis
2023-11-07 2:29 ` [PULL 46/49] target/riscv: Don't assume PMU counters are continuous Alistair Francis
2023-11-09 15:24 ` Peter Maydell [this message]
2023-11-22 5:52 ` Alistair Francis
2023-11-07 2:29 ` [PULL 47/49] target/riscv: Use existing PMU counter mask in FDT generation Alistair Francis
2023-11-07 2:29 ` [PULL 48/49] target/riscv: Add "pmu-mask" property to replace "pmu-num" Alistair Francis
2023-11-07 2:29 ` [PULL 49/49] docs/about/deprecated: Document RISC-V "pmu-num" deprecation Alistair Francis
2023-11-07 4:59 ` [PULL 00/49] riscv-to-apply queue Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFEAcA_DrOBcQytM+UkMNEFFgEhOYqbKnnSREhRY+UsEwD+RbQ@mail.gmail.com \
--to=peter.maydell@linaro.org \
--cc=alistair.francis@wdc.com \
--cc=alistair23@gmail.com \
--cc=atishp@rivosinc.com \
--cc=qemu-devel@nongnu.org \
--cc=rbradford@rivosinc.com \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).