From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6D41BC32772 for ; Tue, 23 Aug 2022 19:22:02 +0000 (UTC) Received: from localhost ([::1]:36052 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oQZTN-0001qx-8F for qemu-devel@archiver.kernel.org; Tue, 23 Aug 2022 15:22:01 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:34508) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oQZRc-0000pG-8E for qemu-devel@nongnu.org; Tue, 23 Aug 2022 15:20:13 -0400 Received: from mail-pf1-x429.google.com ([2607:f8b0:4864:20::429]:44003) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oQZRU-0003E4-V6 for qemu-devel@nongnu.org; Tue, 23 Aug 2022 15:20:10 -0400 Received: by mail-pf1-x429.google.com with SMTP id w138so11743063pfc.10 for ; Tue, 23 Aug 2022 12:19:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20210112.gappssmtp.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=TQKGZ/a3XQa12CnmwVBsi1elK++BC8O561g6lJ6kILE=; b=jXAcFGU0mPOo7CLQEyAYyWMWPH20PJLOQblVSt6c4T1zA5N8l2gLG50L0pv4tdaSwi rFL27tzoaO7hVWVyF/fqBIiF2GNbzvqAd5VPVBS7Pt4Oc8cY87SxMl648oSjXkNVbjJH NNHvOBD29V0meUH2Q/Dp0vuVzi/2dKfua/rQQiAFZ5YXTo1/ydlqwPrJLsUZKbXapT5I Cp9AHiOpSp7SQXKV+SToHyUUqBSWPCZEQGUNavdpropY5cSRyeJcpHFyQEngIYOCwgvP ahQ1IVFArAx6na1LzYr2pZF+CconwFxEUG1pcdAzJvndj7x89ZrSYKcUpl8mm668q1L2 WfaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=TQKGZ/a3XQa12CnmwVBsi1elK++BC8O561g6lJ6kILE=; b=31MEBRRLdp5pJlXqyp6UdkkTG0On6SyLabmRpsP109ZHIkv0cDZZI5BobgLslmI3oG OGo+2Drt+PbqKeao+LKiTkYej7ElBv4/uhLRvXpQrAueDaOv+do27GlVKbQGOFIpkXVn nM2OoM19IsAa11lFALaH3X6c/NzI08izZ9ykH5aKylw77W0R8IBIkN00WZKdxDWifutE cULGRaaQa2hmsNosEAq8vGs9gDLC6inN5U/TAOpB4JXgoHeMKeVrzbTdU1xw6XRn78NJ kouXwYpA2DjZOlpXn01WODIqlNYzUnLj9cUMT34sysIKnMdO4kgAq9p+WhUm2z1C6k/X HFIQ== X-Gm-Message-State: ACgBeo3Wd3CKzVvcC5rcfaANqz9a7a5sukunSnl740ba5loefV4iowl3 5/LNTihMlIKm52lLuS4hqq8JRj8t+oOrQANvG+oXdw== X-Google-Smtp-Source: AA6agR5YyDzYaHNf+Ly/Fge8xQMSKYt0FBCOfGpkLGM5+XWDhXi2HsCplvMU1pLHn9RRHGG+i7x9GMcnmJLPmbfslUM= X-Received: by 2002:aa7:93a8:0:b0:536:8b59:c3bc with SMTP id x8-20020aa793a8000000b005368b59c3bcmr11886952pff.15.1661282396200; Tue, 23 Aug 2022 12:19:56 -0700 (PDT) MIME-Version: 1.0 References: <20220816232321.558250-1-atishp@rivosinc.com> <20220816232321.558250-4-atishp@rivosinc.com> In-Reply-To: From: Atish Kumar Patra Date: Tue, 23 Aug 2022 12:19:45 -0700 Message-ID: Subject: Re: [PATCH v13 3/6] target/riscv: Add few cache related PMU events To: Alistair Francis Cc: "qemu-devel@nongnu.org Developers" , Atish Patra , Alistair Francis , Heiko Stuebner , Bin Meng , Palmer Dabbelt , "open list:RISC-V" Content-Type: multipart/alternative; boundary="00000000000018f79b05e6ed7303" Received-SPF: pass client-ip=2607:f8b0:4864:20::429; envelope-from=atishp@rivosinc.com; helo=mail-pf1-x429.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000018f79b05e6ed7303 Content-Type: text/plain; charset="UTF-8" On Mon, Aug 22, 2022 at 5:38 PM Alistair Francis wrote: > On Wed, Aug 17, 2022 at 9:24 AM Atish Patra wrote: > > > > From: Atish Patra > > > > Qemu can monitor the following cache related PMU events through > > tlb_fill functions. > > > > 1. DTLB load/store miss > > 3. ITLB prefetch miss > > > > Increment the PMU counter in tlb_fill function. > > > > Reviewed-by: Alistair Francis > > Tested-by: Heiko Stuebner > > Signed-off-by: Atish Patra > > Signed-off-by: Atish Patra > > This patch causes a number of test failures. > > On some boots I see lots of these errors printed: > > qemu-system-riscv64: GLib: g_hash_table_lookup: assertion 'hash_table > != NULL' failed > > and I'm unable to boot Linux. > > The command line is: > > qemu-system-riscv64 \ > -machine sifive_u \ > -serial mon:stdio -serial null -nographic \ > -append "root=/dev/ram rw highres=off console=ttySIF0 ip=dhcp > earlycon=sbi" \ > -smp 5 \ > -d guest_errors \ > -kernel ./images/qemuriscv64/Image \ > -initrd ./images/qemuriscv64/buildroot/rootfs.cpio \ > -bios default -m 256M > > I see that faiulre with the 5.19-rc7 kernel and OpenSBI 1.1. > > I also see the same messages on other machines when running baremetal > code. I'm going to drop these patches from my tree > > Sorry for the breakage. This should fix the issue. We just need to add few sanity checks for the platforms that don't support these events. diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c index ad33b81b2ea0..0a473010cadd 100644 --- a/target/riscv/pmu.c +++ b/target/riscv/pmu.c @@ -187,6 +187,9 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx) CPURISCVState *env = &cpu->env; gpointer value; + if (!cpu->cfg.pmu_num) + return 0; + value = g_hash_table_lookup(cpu->pmu_event_ctr_map, GUINT_TO_POINTER(event_idx)); if (!value) { @@ -221,6 +224,9 @@ bool riscv_pmu_ctr_monitor_instructions(CPURISCVState *env, } cpu = RISCV_CPU(env_cpu(env)); + if (!cpu->pmu_event_ctr_map) + return false; + event_idx = RISCV_PMU_EVENT_HW_INSTRUCTIONS; ctr_idx = GPOINTER_TO_UINT(g_hash_table_lookup(cpu->pmu_event_ctr_map, GUINT_TO_POINTER(event_idx))); @@ -243,6 +249,9 @@ bool riscv_pmu_ctr_monitor_cycles(CPURISCVState *env, uint32_t target_ctr) } cpu = RISCV_CPU(env_cpu(env)); + if (!cpu->pmu_event_ctr_map) + return false; + event_idx = RISCV_PMU_EVENT_HW_CPU_CYCLES; ctr_idx = GPOINTER_TO_UINT(g_hash_table_lookup(cpu->pmu_event_ctr_map, GUINT_TO_POINTER(event_idx))); @@ -280,7 +289,7 @@ int riscv_pmu_update_event_map(CPURISCVState *env, uint64_t value, uint32_t event_idx; RISCVCPU *cpu = RISCV_CPU(env_cpu(env)); - if (!riscv_pmu_counter_valid(cpu, ctr_idx)) { + if (!riscv_pmu_counter_valid(cpu, ctr_idx) || !cpu->pmu_event_ctr_map) { return -1; } Should I respin the series or send this as a fix ? Alistair > > > --- > > target/riscv/cpu_helper.c | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > > index 1e4faa84e839..81948b37dd9a 100644 > > --- a/target/riscv/cpu_helper.c > > +++ b/target/riscv/cpu_helper.c > > @@ -21,11 +21,13 @@ > > #include "qemu/log.h" > > #include "qemu/main-loop.h" > > #include "cpu.h" > > +#include "pmu.h" > > #include "exec/exec-all.h" > > #include "instmap.h" > > #include "tcg/tcg-op.h" > > #include "trace.h" > > #include "semihosting/common-semi.h" > > +#include "cpu_bits.h" > > > > int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch) > > { > > @@ -1188,6 +1190,28 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, > vaddr addr, > > cpu_loop_exit_restore(cs, retaddr); > > } > > > > + > > +static void pmu_tlb_fill_incr_ctr(RISCVCPU *cpu, MMUAccessType > access_type) > > +{ > > + enum riscv_pmu_event_idx pmu_event_type; > > + > > + switch (access_type) { > > + case MMU_INST_FETCH: > > + pmu_event_type = RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS; > > + break; > > + case MMU_DATA_LOAD: > > + pmu_event_type = RISCV_PMU_EVENT_CACHE_DTLB_READ_MISS; > > + break; > > + case MMU_DATA_STORE: > > + pmu_event_type = RISCV_PMU_EVENT_CACHE_DTLB_WRITE_MISS; > > + break; > > + default: > > + return; > > + } > > + > > + riscv_pmu_incr_ctr(cpu, pmu_event_type); > > +} > > + > > bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, > > MMUAccessType access_type, int mmu_idx, > > bool probe, uintptr_t retaddr) > > @@ -1286,6 +1310,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr > address, int size, > > } > > } > > } else { > > + pmu_tlb_fill_incr_ctr(cpu, access_type); > > /* Single stage lookup */ > > ret = get_physical_address(env, &pa, &prot, address, NULL, > > access_type, mmu_idx, true, false, > false); > > -- > > 2.25.1 > > > > > --00000000000018f79b05e6ed7303 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Mon, Aug 22, 2022 at 5:38 PM Alist= air Francis <alistair23@gmail.co= m> wrote:
On Wed, Aug 17, 2022 at 9:24 AM Atish Patra <atishp@rivosinc.com> wrote:
>
> From: Atish Patra <atish.patra@wdc.com>
>
> Qemu can monitor the following cache related PMU events through
> tlb_fill functions.
>
> 1. DTLB load/store miss
> 3. ITLB prefetch miss
>
> Increment the PMU counter in tlb_fill function.
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>

This patch causes a number of test failures.

On some boots I see lots of these errors printed:

qemu-system-riscv64: GLib: g_hash_table_lookup: assertion 'hash_table !=3D NULL' failed

and I'm unable to boot Linux.

The command line is:

qemu-system-riscv64 \
=C2=A0 =C2=A0 -machine sifive_u \
=C2=A0 =C2=A0 -serial mon:stdio -serial null -nographic \
=C2=A0 =C2=A0 -append "root=3D/dev/ram rw highres=3Doff console=3DttyS= IF0 ip=3Ddhcp
earlycon=3Dsbi" \
=C2=A0 =C2=A0 -smp 5 \
=C2=A0 =C2=A0 -d guest_errors \
=C2=A0 =C2=A0 -kernel ./images/qemuriscv64/Image \
=C2=A0 =C2=A0 -initrd ./images/qemuriscv64/buildroot/rootfs.cpio \
=C2=A0 =C2=A0 -bios default -m 256M

I see that faiulre with the 5.19-rc7 kernel and OpenSBI 1.1.

I also see the same messages on other machines when running baremetal
code. I'm going to drop these patches from my tree


Sorry for the breakage.=C2=A0 This sho= uld fix the issue. We just need to add few sanity checks
for the = platforms that don't support these events.

dif= f --git a/target/riscv/pmu.c b/target/riscv/pmu.c
index ad33b81b2ea0..0a= 473010cadd 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c@@ -187,6 +187,9 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_= event_idx event_idx)
=C2=A0 =C2=A0 =C2=A0CPURISCVState *env =3D &cpu= ->env;
=C2=A0 =C2=A0 =C2=A0gpointer value;
=C2=A0
+ =C2=A0 =C2= =A0if (!cpu->cfg.pmu_num)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
+=
=C2=A0 =C2=A0 =C2=A0value =3D g_hash_table_lookup(cpu->pmu_event_ctr= _map,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0GUINT_TO_POINTER(event_= idx));
=C2=A0 =C2=A0 =C2=A0if (!value) {
@@ -221,6 +224,9 @@ bool ris= cv_pmu_ctr_monitor_instructions(CPURISCVState *env,
=C2=A0 =C2=A0 =C2=A0= }
=C2=A0
=C2=A0 =C2=A0 =C2=A0cpu =3D RISCV_CPU(env_cpu(env));
+ = =C2=A0 =C2=A0if (!cpu->pmu_event_ctr_map)
+ =C2=A0 =C2=A0 =C2=A0 =C2= =A0return false;
+
=C2=A0 =C2=A0 =C2=A0event_idx =3D RISCV_PMU_EVENT_= HW_INSTRUCTIONS;
=C2=A0 =C2=A0 =C2=A0ctr_idx =3D GPOINTER_TO_UINT(g_hash= _table_lookup(cpu->pmu_event_ctr_map,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 GUINT_TO_POINTER(event_idx)));
@@ -243,6 +249,9 @@ bool riscv_pmu= _ctr_monitor_cycles(CPURISCVState *env, uint32_t target_ctr)
=C2=A0 =C2= =A0 =C2=A0}
=C2=A0
=C2=A0 =C2=A0 =C2=A0cpu =3D RISCV_CPU(env_cpu(env)= );
+ =C2=A0 =C2=A0if (!cpu->pmu_event_ctr_map)
+ =C2=A0 =C2=A0 =C2= =A0 =C2=A0return false;
+
=C2=A0 =C2=A0 =C2=A0event_idx =3D RISCV_PMU= _EVENT_HW_CPU_CYCLES;
=C2=A0 =C2=A0 =C2=A0ctr_idx =3D GPOINTER_TO_UINT(g= _hash_table_lookup(cpu->pmu_event_ctr_map,
=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 GUINT_TO_POINTER(event_idx)));
@@ -280,7 +289,7 @@ int ris= cv_pmu_update_event_map(CPURISCVState *env, uint64_t value,
=C2=A0 =C2= =A0 =C2=A0uint32_t event_idx;
=C2=A0 =C2=A0 =C2=A0RISCVCPU *cpu =3D RISC= V_CPU(env_cpu(env));
=C2=A0
- =C2=A0 =C2=A0if (!riscv_pmu_counter_val= id(cpu, ctr_idx)) {
+ =C2=A0 =C2=A0if (!riscv_pmu_counter_valid(cpu, ctr= _idx) || !cpu->pmu_event_ctr_map) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0return -1;
=C2=A0 =C2=A0 =C2=A0}
=C2=A0
Shoul= d I respin the series or send this as a fix ?

Alistair

> ---
>=C2=A0 target/riscv/cpu_helper.c | 25 +++++++++++++++++++++++++
>=C2=A0 1 file changed, 25 insertions(+)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 1e4faa84e839..81948b37dd9a 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -21,11 +21,13 @@
>=C2=A0 #include "qemu/log.h"
>=C2=A0 #include "qemu/main-loop.h"
>=C2=A0 #include "cpu.h"
> +#include "pmu.h"
>=C2=A0 #include "exec/exec-all.h"
>=C2=A0 #include "instmap.h"
>=C2=A0 #include "tcg/tcg-op.h"
>=C2=A0 #include "trace.h"
>=C2=A0 #include "semihosting/common-semi.h"
> +#include "cpu_bits.h"
>
>=C2=A0 int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
>=C2=A0 {
> @@ -1188,6 +1190,28 @@ void riscv_cpu_do_unaligned_access(CPUState *cs= , vaddr addr,
>=C2=A0 =C2=A0 =C2=A0 cpu_loop_exit_restore(cs, retaddr);
>=C2=A0 }
>
> +
> +static void pmu_tlb_fill_incr_ctr(RISCVCPU *cpu, MMUAccessType access= _type)
> +{
> +=C2=A0 =C2=A0 enum riscv_pmu_event_idx pmu_event_type;
> +
> +=C2=A0 =C2=A0 switch (access_type) {
> +=C2=A0 =C2=A0 case MMU_INST_FETCH:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 pmu_event_type =3D RISCV_PMU_EVENT_CACHE_= ITLB_PREFETCH_MISS;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case MMU_DATA_LOAD:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 pmu_event_type =3D RISCV_PMU_EVENT_CACHE_= DTLB_READ_MISS;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case MMU_DATA_STORE:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 pmu_event_type =3D RISCV_PMU_EVENT_CACHE_= DTLB_WRITE_MISS;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 riscv_pmu_incr_ctr(cpu, pmu_event_type);
> +}
> +
>=C2=A0 bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 MMUAccessType access_type, int mmu_idx,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 bool probe, uintptr_t retaddr)
> @@ -1286,6 +1310,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr addr= ess, int size,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0 } else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 pmu_tlb_fill_incr_ctr(cpu, access_type);<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Single stage lookup */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ret =3D get_physical_address(env, &a= mp;pa, &prot, address, NULL,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0access_type, = mmu_idx, true, false, false);
> --
> 2.25.1
>
>
--00000000000018f79b05e6ed7303--