All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
To: TANG Tiancheng <lyndra@linux.alibaba.com>, qemu-devel@nongnu.org
Cc: "Zephyr Li" <fritchleybohrer@gmail.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Alistair Francis" <alistair.francis@wdc.com>,
	"Weiwei Li" <liwei1518@gmail.com>,
	"Liu Zhiwei" <zhiwei_liu@linux.alibaba.com>,
	"Chao Liu" <chao.liu@processmission.com>,
	qemu-riscv@nongnu.org,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@oss.qualcomm.com>
Subject: Re: [PATCH v2 10/14] target/riscv: Apply minstret exception accounting to HPM counters
Date: Fri, 25 Sep 2026 13:31:15 -0300	[thread overview]
Message-ID: <cb85596b-485b-4380-9f67-2a8348a57eb5@oss.qualcomm.com> (raw)
In-Reply-To: <20260910-riscv-pmu-correctness-v2-10-5da5159a0c64@linux.alibaba.com>



On 9/10/2026 11:39 AM, TANG Tiancheng wrote:
> With icount, helper_raise_exception() excludes faulting instructions from
> minstret but not HPM counters selecting HW_INSTRUCTIONS.
> 
> Adjust the baseline of every running instruction counter that counts the
> current privilege mode. Do not read icount here: the helper can run inside
> a TB. Keep the existing timer, since excluding an instruction can only
> postpone overflow and expiry checks for an actual wrap.
> 
> Extend the ECALL regression to compare both counters and add an LPAD
> fault test covering an exception inside a multi-instruction TB.
> 
> Fixes: 14664483457b ("target/riscv: Add sscofpmf extension support")
> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/tcg/pmu.c                  | 35 ++++++++++++----
>   tests/tcg/riscv64/pmu-lpad.S            | 72 +++++++++++++++++++++++++++++++++
>   tests/tcg/riscv64/system/meson.build    |  7 ++++
>   tests/tcg/riscv64/test-minstret-ecall.S | 26 ++++++++++++
>   4 files changed, 133 insertions(+), 7 deletions(-)
> 
> diff --git a/target/riscv/tcg/pmu.c b/target/riscv/tcg/pmu.c
> index f88f6ae671d877ed874d22c9d4b840edb6f433a5..08298010b6d06f5792fa14ff81fe2f7a28c6476f 100644
> --- a/target/riscv/tcg/pmu.c
> +++ b/target/riscv/tcg/pmu.c
> @@ -387,18 +387,39 @@ void riscv_pmu_write_inhibit(CPURISCVState *env, uint32_t value)
>   
>   void riscv_pmu_decr_instret(CPURISCVState *env)
>   {
> -    if (!icount_enabled() ||
> -        (env->mcountinhibit & COUNTEREN_IR) ||
> -        riscv_pmu_counter_filtered(env, env->minstretcfg)) {
> +    RISCVCPU *cpu = env_archcpu(env);
> +    uint32_t ctr_mask;
> +
> +    if (!icount_enabled()) {
>           return;
>       }
>   
>       /*
> -     * minstret is derived from icount, which includes the current
> -     * instruction.  Move the baseline forward to exclude an instruction
> -     * that raises an exception and therefore does not retire.
> +     * Fixed instruction events are derived from icount, which includes the
> +     * current instruction.  Move the baseline of each running
> +     * instruction-source counter that counts the current privilege mode to
> +     * exclude an instruction that raises an exception and does not retire.
> +     *
> +     * Do not read icount here: this helper can run in the middle of a TB.
> +     * Excluding an instruction only postpones overflow, so keep the current
> +     * timer deadline. The expiry handler checks for an actual counter wrap.
>        */
> -    env->pmu_ctrs[2].mhpmcounter_prev++;
> +    ctr_mask = COUNTEREN_IR |
> +               riscv_pmu_event_counter_mask(
> +                   cpu, RISCV_PMU_EVENT_HW_INSTRUCTIONS);
> +    while (ctr_mask) {
> +        uint32_t ctr_idx = ctz32(ctr_mask);
> +        uint64_t cfg = ctr_idx == 2 ? env->minstretcfg :
> +                                      env->mhpmevent_val[ctr_idx];
> +
> +        ctr_mask &= ~BIT(ctr_idx);
> +        if (!riscv_pmu_fixed_ctr_running(env, ctr_idx) ||
> +            riscv_pmu_counter_filtered(env, cfg)) {
> +            continue;
> +        }
> +
> +        env->pmu_ctrs[ctr_idx].mhpmcounter_prev++;
> +    }
>   }
>   
>   int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx)
> diff --git a/tests/tcg/riscv64/pmu-lpad.S b/tests/tcg/riscv64/pmu-lpad.S
> new file mode 100644
> index 0000000000000000000000000000000000000000..40a6b34b7c2b71dd42e78b6873d72df4d508289d
> --- /dev/null
> +++ b/tests/tcg/riscv64/pmu-lpad.S
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +/* CSR number for older assemblers. */
> +#define CSR_MSECCFG          0x747
> +
> +/* An LPAD fault must be deliverable with HPM instruction counting enabled. */
> +
> +	.option	norvc
> +	.option	norelax
> +
> +	.text
> +	.global	_start
> +_start:
> +	lla	t0, trap
> +	csrw	mtvec, t0
> +	li	t0, 2		/* HW_INSTRUCTIONS */
> +	csrw	mhpmevent3, t0
> +	li	t0, 1 << 10	/* mseccfg.MLPE */
> +	csrs	CSR_MSECCFG, t0
> +
> +	/* x7[31:12] = 0 does not match the nonzero LPAD label. */
> +	li	t2, 0
> +	lla	a0, target
> +	jalr	ra, a0, 0
> +	j	fail
> +
> +	.balign	4
> +target:
> +	.word	0x00001017	/* lpad 1 */
> +	/*
> +	 * Keep the LPAD check inside a multi-instruction TB. Its exception
> +	 * helper must not read icount before leaving generated code.
> +	 */
> +	.rept	16
> +	nop
> +	.endr
> +	j	fail
> +
> +trap:
> +	csrr	t0, mcause
> +	li	t1, 18		/* Software-check exception */
> +	bne	t0, t1, fail
> +	csrr	t0, mtval
> +	li	t1, 2		/* Landing-pad fault */
> +	bne	t0, t1, fail
> +	csrr	t0, mepc
> +	lla	t1, target
> +	bne	t0, t1, fail
> +	li	a0, 0
> +	j	exit
> +
> +fail:
> +	li	a0, 1
> +
> +exit:
> +	lla	a1, semiargs
> +	li	t0, 0x20026	/* ADP_Stopped_ApplicationExit */
> +	sd	t0, 0(a1)
> +	sd	a0, 8(a1)
> +	li	a0, 0x20	/* TARGET_SYS_EXIT_EXTENDED */
> +
> +	/* Semihosting call sequence. */
> +	.balign	16
> +	slli	zero, zero, 0x1f
> +	ebreak
> +	srai	zero, zero, 0x7
> +	j	.
> +
> +	.data
> +	.balign	16
> +semiargs:
> +	.space	16
> diff --git a/tests/tcg/riscv64/system/meson.build b/tests/tcg/riscv64/system/meson.build
> index 13ee7954ef4f09559a02a3730fb017ccf2cfd882..d2355090121ba7a7d0046c1fc1824d6a89c1267f 100644
> --- a/tests/tcg/riscv64/system/meson.build
> +++ b/tests/tcg/riscv64/system/meson.build
> @@ -61,6 +61,13 @@ tests += {
>     }
>   }
>   
> +tests += {
> +  'pmu-lpad.S': {
> +    'cflags': cflags,
> +    'qemu_args': ['-cpu', 'max', '-icount', 'shift=0', qemu_args],
> +  },
> +}
> +
>   # Exercise RV32 CSRs with the RV64 emulator's 64-bit target_ulong.
>   tests += {
>     '../riscv32/smcdeleg-rv32.S': {
> diff --git a/tests/tcg/riscv64/test-minstret-ecall.S b/tests/tcg/riscv64/test-minstret-ecall.S
> index ab268f7f22985820f19bde353215385608643f3a..b1857543d488df984b923ad1c135edb35cb948c7 100644
> --- a/tests/tcg/riscv64/test-minstret-ecall.S
> +++ b/tests/tcg/riscv64/test-minstret-ecall.S
> @@ -18,11 +18,37 @@ _start:
>   	li	t1, 1
>   	bne	t0, t1, fail
>   
> +	/*
> +	 * minstret and a counter selecting HW_INSTRUCTIONS must both exclude
> +	 * ECALL, so they must contain the same number of retired instructions.
> +	 */
> +	li	t0, 12		/* mcountinhibit.IR | mcountinhibit.HPM3 */
> +	csrs	mcountinhibit, t0
> +	csrw	minstret, zero
> +	csrw	mhpmcounter3, zero
> +	li	t0, 2		/* RISCV_PMU_EVENT_HW_INSTRUCTIONS */
> +	csrw	mhpmevent3, t0
> +	lla	t0, trap_hpm
> +	csrw	mtvec, t0
> +	li	t0, 12
> +	csrc	mcountinhibit, t0
> +	ecall
> +	bne	s3, s4, fail
> +
>   	li	a0, 0
>   	j	_exit
>   
>   trap:
>   	csrr	s1, minstret
> +	j	trap_check
> +
> +trap_hpm:
> +	li	t0, 12
> +	csrs	mcountinhibit, t0
> +	csrr	s3, minstret
> +	csrr	s4, mhpmcounter3
> +
> +trap_check:
>   	csrr	t0, mcause
>   	li	t1, 11		/* Environment call from M-mode */
>   	bne	t0, t1, fail
> 



  reply	other threads:[~2026-09-25 16:31 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-10 14:39 [PATCH v2 00/14] RISC-V TCG PMU correctness fixes TANG Tiancheng
2026-09-10 14:39 ` [PATCH v2 01/14] target/riscv: Preserve PMU state across event selector writes TANG Tiancheng
2026-09-11  3:47   ` Chao Liu
2026-09-10 14:39 ` [PATCH v2 02/14] target/riscv: Support multiple counters per PMU event TANG Tiancheng
2026-09-11  3:54   ` Chao Liu
2026-09-10 14:39 ` [PATCH v2 03/14] target/riscv: Use VM-elapsed sources for fixed PMU events TANG Tiancheng
2026-09-11  3:55   ` Chao Liu
2026-09-10 14:39 ` [PATCH v2 04/14] target/riscv: Preserve MINH on delegated config reads TANG Tiancheng
2026-09-11  3:56   ` Chao Liu
2026-09-10 14:39 ` [PATCH v2 05/14] target/riscv: Preserve minstretcfgh on RV32 minstretcfg writes TANG Tiancheng
2026-09-11  4:43   ` Chao Liu
2026-09-10 14:39 ` [PATCH v2 06/14] target/riscv: Fix RV32 accesses to delegated PMU registers TANG Tiancheng
2026-09-25 15:44   ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 07/14] target/riscv: Preserve fixed counters across PMU state changes TANG Tiancheng
2026-09-25 15:47   ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 08/14] target/riscv: Require Sscofpmf for non-fixed event overflow TANG Tiancheng
2026-09-25 16:08   ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 09/14] target/riscv: Rebuild fixed-event PMU overflow deadlines TANG Tiancheng
2026-09-25 16:21   ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 10/14] target/riscv: Apply minstret exception accounting to HPM counters TANG Tiancheng
2026-09-25 16:31   ` Daniel Henrique Barboza [this message]
2026-09-10 14:39 ` [PATCH v2 11/14] target/riscv: Process PMU timer expiry on the owner vCPU TANG Tiancheng
2026-09-25 16:34   ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 12/14] target/riscv: Migrate fixed PMU counter state TANG Tiancheng
2026-09-25 16:40   ` Daniel Henrique Barboza
2026-09-10 14:39 ` [PATCH v2 13/14] target/riscv: Clear virtualization mode on reset TANG Tiancheng
2026-09-25 16:45   ` Daniel Henrique Barboza
2026-09-28  1:31     ` TianCheng TANG
2026-09-10 14:39 ` [PATCH v2 14/14] target/riscv: Preserve fixed PMU state across reset TANG Tiancheng
2026-09-25 16:45   ` Daniel Henrique Barboza

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=cb85596b-485b-4380-9f67-2a8348a57eb5@oss.qualcomm.com \
    --to=daniel.barboza@oss.qualcomm.com \
    --cc=alistair.francis@wdc.com \
    --cc=chao.liu@processmission.com \
    --cc=fritchleybohrer@gmail.com \
    --cc=liwei1518@gmail.com \
    --cc=lyndra@linux.alibaba.com \
    --cc=palmer@dabbelt.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@oss.qualcomm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.