hw/riscv, target/riscv: initial e-trace support
Subsystems (2)
v4 cover hw/riscv, target/riscv: initial e-trace support
Hi,
It's time for a refresher of the hwtrace/e-trace support for RISC-V. It has been awhile since v3 and we have Konstantin posting patches to implement N-trace on top of it.
For this new version a couple of design changes were made, most notably the FDT functions are now hosted in fdt-common.c and we're using an EXT_TB_FLAG to enable/disable the trace session instead of using a TCG global.
A new patch was also added to describe what we're implementing and some design decisions made w.r.t TCG instrumentation and encoder parameters.
This emulation is being used to support the development of the Linux RISC-V e-trace driver framework. Newest version can be found here:
("[PATCH v6 00/14] Linux RISC-V trace framework and drivers")
https://lists.infradead.org/pipermail/linux-riscv/2026-September/100456.html
Patches based on alistair/riscv-to-apply.next.
Changes from v3:
- patch 5:
- move FDT creation code to fdt-common.c
- patch 7:
- move trace_helper.c to tcg subdir
- use "CPU_TRACE_RUNNING" EXT_TB_FLAG and ctx->cpu_trace_running for insn instrumentation instead of using a TCG global
- patch 14:
- use ctx->cpu_trace_running in gen_trace_branch()
- patch 16:
- use ctx->cpu_trace_running in gen_trace_branch()
- patch 18 (new):
- add documentation
- v3 link: https://lore.kernel.org/qemu-devel/[email protected]/
Daniel Henrique Barboza (18):
hw/riscv: Trace Encoder initial impl
hw/riscv: Trace RAM Sink initial impl hw/riscv/trace-encoder: add trace start/stop logic hw/riscv/virt.c: add trace encoders and trace ram sinks hw/riscv/virt.c add trace encoder and ramsink fdt nodes hw/riscv: add e-trace message helpers
target/riscv: add initial trace instrumentation hw/riscv/trace-encoder: write e-trace packets to RAM sink test/qtest: add riscv-trace-test.c hw/riscv/rv-trace-messages.c: add encoded trap message hw/riscv, target/riscv: send trace trap messages target/riscv, hw/riscv: send trace ppccd packets hw/riscv/trace: add format2 msg helper
hw/riscv, target/riscv: send resync updiscon trace packets hw/riscv/rv-trace-messages: add format 1 msgs with branch info hw/riscv/trace-encoder: send branches info hw/riscv/trace: update branch bit in sync messages docs/specs: document RISC-V hwtrace (e-trace) support
docs/specs/index.rst | 1 + docs/specs/riscv-hwtrace.rst | 96 +++ hw/riscv/Kconfig | 5 + hw/riscv/fdt-common.c | 70 ++ hw/riscv/meson.build | 3 +- hw/riscv/rv-trace-messages.c | 373 +++++++++++ hw/riscv/rv-trace-messages.h | 40 ++ hw/riscv/trace-encoder.c | 610 ++++++++++++++++++ hw/riscv/trace-encoder.h | 62 ++ hw/riscv/trace-events | 9 + hw/riscv/trace-ram-sink.c | 263 ++++++++ hw/riscv/trace-ram-sink.h | 83 +++ hw/riscv/virt.c | 128 ++++ include/hw/riscv/fdt-common.h | 4 + include/hw/riscv/virt.h | 2 + target/riscv/cpu.h | 12 + target/riscv/helper.h | 6 + target/riscv/tcg/cpu_helper.c | 26 +- .../tcg/insn_trans/trans_privileged.c.inc | 11 + target/riscv/tcg/insn_trans/trans_rvi.c.inc | 13 + target/riscv/tcg/meson.build | 6 + target/riscv/tcg/tcg-cpu.c | 9 + target/riscv/tcg/trace_helper.c | 62 ++ target/riscv/tcg/translate.c | 16 + tests/qtest/meson.build | 2 +- tests/qtest/riscv-trace-test.c | 120 ++++ 26 files changed, 2029 insertions(+), 3 deletions(-) create mode 100644 docs/specs/riscv-hwtrace.rst create mode 100644 hw/riscv/rv-trace-messages.c create mode 100644 hw/riscv/rv-trace-messages.h create mode 100644 hw/riscv/trace-encoder.c create mode 100644 hw/riscv/trace-encoder.h create mode 100644 hw/riscv/trace-ram-sink.c create mode 100644 hw/riscv/trace-ram-sink.h create mode 100644 target/riscv/tcg/trace_helper.c create mode 100644 tests/qtest/riscv-trace-test.c
Signature · 1 line
2.43.0
PATCH v4 1/18 hw/riscv: Trace Encoder initial impl
The Trace Encoder is a hardware module that interacts with a CPU, gathering execution information (a.k.a instruction delta trace), and send it downstream to other storage components such as a RAM Sink (a RAM storage). This trace info can then be read via software (e.g. perf) to reproduce the behavior of a given binary that ran in the CPU.
This implementation is based on the Efficient Trace for RISC-V [1] and RISC-V Trace Control Interface Specification [2]. It's not intended to implement all spec features: the idea is to provide a base where we can add extra features on demand.
We'll get back to the instruction delta trace and how we'll instrument TCG to produce it later. For now we'll set the minimal components to get the basic framework running.
This Trace Encoder impl has just the minimal bits specified in [2], section "Minimal Implementation". RO and RSVP bits are taken verbatim from [2] without considering what we're actually going to support. The base impl is heavily inspired by the XLNZ-ZDMA device w.r.t the usage of the RegisterInfo and register.h framework.
Discovery of the Trace Encoder will be made via fdt, a single entry per CPU. We'll connect each Trace Encoder to its CPU in the 'virt' board later.
[1] https://github.com/riscv-non-isa/riscv-trace-spec/releases/download/v2.0-20250616/riscv-trace-spec-asciidoc.pdf
[2] https://github.com/riscv-non-isa/tg-nexus-trace/releases/download/1.0_Ratified/RISC-V-Trace-Control-Interface.pdf
--- hw/riscv/Kconfig | 5 + hw/riscv/meson.build | 2 +- hw/riscv/trace-encoder.c | 216 +++++++++++++++++++++++++++++++++++++++ hw/riscv/trace-encoder.h | 42 ++++++++ hw/riscv/trace-events | 4 + 5 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 hw/riscv/trace-encoder.c create mode 100644 hw/riscv/trace-encoder.h
hw/riscv/Kconfig +5 -0
@@ -7,6 +7,10 @@ config RISCV_NUMA config IBEX bool +config RISCV_TRACE + bool + select REGISTER + # RISC-V machines in alphabetical order config MICROCHIP_PFSOC
@@ -68,6 +72,7 @@ config RISCV_VIRT select PLATFORM_BUS select ACPI select ACPI_PCI + select RISCV_TRACE config RISCV_SERVER_PLATFORM_REF bool
hw/riscv/meson.build +1 -1
@@ -22,7 +22,7 @@ riscv_ss.add(when: 'CONFIG_XIANGSHAN_KUNMINGHU', if_true: files('xiangshan_kmh.c riscv_ss.add(when: 'CONFIG_RISCV_MIPS_CPS', if_true: files('cps.c')) riscv_ss.add(when: 'CONFIG_MIPS_BOSTON_AIA', if_true: files('boston-aia.c')) riscv_ss.add(when: 'CONFIG_K230', if_true: files('k230.c')) - +riscv_ss.add(when: 'CONFIG_RISCV_TRACE', if_true: files('trace-encoder.c')) riscv_ss.add(when: 'CONFIG_SOPHGO_CV1800B', if_true: files('cv1800b.c')) riscv_ss.add(when: 'CONFIG_MILKV_DUO', if_true: files('milkv_duo.c'))
hw/riscv/trace-encoder.c new file +216 -0
216 lines not on this page · raw message
hw/riscv/trace-encoder.h new file +42 -0
42 lines not on this page · raw message
hw/riscv/trace-events +4 -0
@@ -24,3 +24,7 @@ riscv_iommu_hpm_incr_ctr(uint64_t cntr_val) "cntr_val 0x%"PRIx64 riscv_iommu_hpm_iocntinh_cy(bool prev_cy_inh) "prev_cy_inh %d" riscv_iommu_hpm_cycle_write(uint32_t ovf, uint64_t val) "ovf 0x%x val 0x%"PRIx64 riscv_iommu_hpm_evt_write(uint32_t ctr_idx, uint32_t ovf, uint64_t val) "ctr_idx 0x%x ovf 0x%x val 0x%"PRIx64 + +# trace-encoder.c +trencoder_read_error(uint64_t addr) "addr 0x%" PRIx64 +trencoder_write_error(uint64_t addr, uint64_t value) "addr 0x%" PRIx64 " value 0x%" PRIx64
Signature · 1 line
2.43.0
Re: [PATCH v4 01/18] hw/riscv: Trace Encoder initial impl
hw/riscv/trace-encoder.h line 18
Quoted 9 lines
hw/riscv/trace-encoder.h new file +7
-0+#include "hw/core/sysbus.h" +#include "hw/core/register.h" +#include "system/dma.h" +#include "qom/object.h" +#include "cpu.h" + +#define TRACE_R_MAX (0xFFF / 4)
The Trace Control Interface register map covers offsets 0x000 through 0xFFF, i.e. a 4-KiB window. Here 0xFFF / 4 evaluates to 1023,
which is the last valid index, not the number of 32-bit entries. Since TRACE_R_MAX is later used as the default reg-mem-size via TRACE_R_MAX
* 4, the default MMIO region is only 0xFFC bytes long.
It therefore covers offsets 0x000 through 0xFFB, leaving the final word at 0xFFC through 0xFFF outside the region.
Thanks
Jay Chang
Re: [PATCH v4 01/18] hw/riscv: Trace Encoder initial impl
In reply to Daniel Henrique BarbozaQuoted 30 lines · Daniel Henrique Barboza
The Trace Encoder is a hardware module that interacts with a CPU, gathering execution information (a.k.a instruction delta trace), and send it downstream to other storage components such as a RAM Sink (a RAM
storage). This trace info can then be read via software (e.g. perf) to
reproduce the behavior of a given binary that ran in the CPU.This implementation is based on the Efficient Trace for RISC-V [1] and
RISC-V Trace Control Interface Specification [2]. It's not intended to
implement all spec features: the idea is to provide a base where we can
add extra features on demand.We'll get back to the instruction delta trace and how we'll instrument
TCG to produce it later. For now we'll set the minimal components to get
the basic framework running.This Trace Encoder impl has just the minimal bits specified in [2], section "Minimal Implementation". RO and RSVP bits are taken verbatim from [2] without considering what we're actually going to support. The
base impl is heavily inspired by the XLNZ-ZDMA device w.r.t the usage of
the RegisterInfo andregister.hframework.
Quoted 7 lines
Discovery of the Trace Encoder will be made via fdt, a single entry per
CPU. We'll connect each Trace Encoder to its CPU in the 'virt' board later.
Let's make sure we use the ratified spec, otherwise:
Alistair
Quoted 349 lines
Signed-off-by: Daniel Henrique Barboza
<[email protected]>
---
hw/riscv/Kconfig | 5 +
hw/riscv/meson.build | 2 +-
hw/riscv/trace-encoder.c | 216
+++++++++++++++++++++++++++++++++++++++
hw/riscv/trace-encoder.h | 42 ++++++++
hw/riscv/trace-events | 4 +5 files changed, 268 insertions(+), 1 deletion(-)create mode 100644
hw/riscv/trace-encoder.c
create mode 100644hw/riscv/trace-encoder.hhw/riscv/Kconfig +0
-0@@ -7,6 +7,10 @@ config RISCV_NUMAconfig IBEX
boolhw/riscv/Kconfig +4
-0+config RISCV_TRACE + bool + select REGISTER +# RISC-V machines in alphabetical order
configMICROCHIP_PFSOChw/riscv/Kconfig +0
-0@@ -68,6 +72,7 @@ config RISCV_VIRTselect
PLATFORM_BUS
select ACPI
selectACPI_PCI
+ selectRISCV_TRACE
configRISCV_SERVER_PLATFORM_REF
boolhw/riscv/meson.build +0
-0@@ -22,7 +22,7 @@ riscv_ss.add(when: 'CONFIG_XIANGSHAN_KUNMINGHU',
if_true: files('xiangshan_kmh.c
riscv_ss.add(when: 'CONFIG_RISCV_MIPS_CPS',if_true: files('cps.c'))riscv_ss.add(when: 'CONFIG_MIPS_BOSTON_AIA', if_true: files('boston- aia.c'))
riscv_ss.add(when: 'CONFIG_K230',if_true: files('k230.c'))hw/riscv/meson.build +1
-1- +riscv_ss.add(when: 'CONFIG_RISCV_TRACE', if_true: files('trace-
encoder.c'))
riscv_ss.add(when: 'CONFIG_SOPHGO_CV1800B',if_true:
files('cv1800b.c'))
riscv_ss.add(when: 'CONFIG_MILKV_DUO',if_true:files('milkv_duo.c'))hw/riscv/trace-encoder.c new file +4
-0@@ -0,0 +1,216 @@ +/* +* Emulation of a RISC-V Trace Encoder +* +* Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */hw/riscv/trace-encoder.c new file +84
-0+ +#include "qemu/osdep.h" + +#include "trace-encoder.h" +#include "hw/core/irq.h" +#include "hw/core/qdev-properties.h" +#include "migration/vmstate.h" +#include "qemu/bitops.h" +#include "qemu/log.h" +#include "qemu/module.h" +#include "qapi/error.h" +#include "trace.h" +#include "system/device_tree.h" +#include "hw/core/register.h" +#include "cpu.h" + +/* +* trTeControl register fields +*/ +REG32(TR_TE_CONTROL, 0x0) + FIELD(TR_TE_CONTROL, ACTIVE, 0, 1) + FIELD(TR_TE_CONTROL, ENABLE, 1, 1) + FIELD(TR_TE_CONTROL, INST_TRACING, 2, 1) + FIELD(TR_TE_CONTROL, EMPTY, 3, 1) + FIELD(TR_TE_CONTROL, INST_MODE, 4, 3) + FIELD(TR_TE_CONTROL, INST_SYNC_MODE, 16, 2) + FIELD(TR_TE_CONTROL, FORMAT, 24, 3) + /* reserved bits */ + FIELD(TR_TE_CONTROL, RSVP1, 7, 2) + FIELD(TR_TE_CONTROL, RSVP2, 10, 1) + FIELD(TR_TE_CONTROL, RSVP3, 14, 1) + FIELD(TR_TE_CONTROL, RSVP4, 18, 2) + FIELD(TR_TE_CONTROL, RSVP5, 27, 4) + +#define R_TR_TE_CONTROL_RSVP_BITS (MAKE_64BIT_MASK(32, 32) | \ + R_TR_TE_CONTROL_RSVP1_MASK | \ + R_TR_TE_CONTROL_RSVP2_MASK | \ + R_TR_TE_CONTROL_RSVP3_MASK | \ + R_TR_TE_CONTROL_RSVP4_MASK | \ + R_TR_TE_CONTROL_RSVP5_MASK) + +/* trTeControlEmpty is the only RO field and reset value */ +#define R_TR_TE_CONTROL_RESET R_TR_TE_CONTROL_EMPTY_MASK +#define R_TR_TE_CONTROL_RO_BITS R_TR_TE_CONTROL_EMPTY_MASK + +/* +* trTeImpl register fields +*/ +REG32(TR_TE_IMPL, 0x4) + FIELD(TR_TE_IMPL, VER_MAJOR, 0, 4) + FIELD(TR_TE_IMPL, VER_MINOR, 4, 4) + FIELD(TR_TE_IMPL, COMP_TYPE, 8, 4) + FIELD(TR_TE_IMPL, PROTOCOL_MAJOR, 16, 4) + FIELD(TR_TE_IMPL, PROTOCOL_MINOR, 20, 4) + /* reserved bits */ + FIELD(TR_TE_IMPL, RSVP1, 12, 4) + FIELD(TR_TE_IMPL, RSVP2, 24, 8) + +#define R_TR_TE_IMPL_RSVP_BITS (MAKE_64BIT_MASK(32, 32) | \ + R_TR_TE_IMPL_RSVP1_MASK | \ + R_TR_TE_IMPL_RSVP2_MASK) + +#define R_TR_TE_IMPL_RO_BITS (R_TR_TE_IMPL_VER_MAJOR_MASK | \ + R_TR_TE_IMPL_VER_MINOR_MASK | \ + R_TR_TE_IMPL_COMP_TYPE_MASK | \ + R_TR_TE_IMPL_PROTOCOL_MAJOR_MASK | \ + R_TR_TE_IMPL_PROTOCOL_MINOR_MASK) + +#define R_TR_TE_IMPL_RESET (BIT(0) | BIT(8)) + +static RegisterAccessInfo trencoder_regs_info[] = { + { .name = "TR_TE_CONTROL", .addr = A_TR_TE_CONTROL, + .rsvd = R_TR_TE_CONTROL_RSVP_BITS, + .reset = R_TR_TE_CONTROL_RESET, + .ro = R_TR_TE_CONTROL_RO_BITS, + }, + { .name = "TR_TE_IMPL", .addr = A_TR_TE_IMPL, + .rsvd = R_TR_TE_IMPL_RSVP_BITS, + .reset = R_TR_TE_IMPL_RESET, + .ro = R_TR_TE_IMPL_RO_BITS, + }, +}; + +static uint64_t trencoder_read(void *opaque, hwaddr addr, unsignedsize)
hw/riscv/trace-encoder.c new file +76
-0+{ + TraceEncoder *te = TRACE_ENCODER(opaque); + RegisterInfo *r = &te->regs_info[addr / 4]; + + if (!r->data) { + trace_trencoder_read_error(addr); + return 0; + } + + return register_read(r, ~0, NULL, false); +} + +static void trencoder_write (void *opaque,hwaddr addr, + uint64_t value, unsigned size) +{ + TraceEncoder *te = TRACE_ENCODER(opaque); + RegisterInfo *r = &te->regs_info[addr / 4]; + + if (!r->data) { + trace_trencoder_write_error(addr, value); + return; + } + + register_write(r, value, ~0, NULL, false); +} + +static const MemoryRegionOps trencoder_ops = { + .read = trencoder_read, + .write = trencoder_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .valid = { + .min_access_size = 4, + .max_access_size = 4, + }, +}; + +static void trencoder_reset (DeviceState *dev) +{ + TraceEncoder *te = TRACE_ENCODER(dev); + + for (int i = 0; i < ARRAY_SIZE(te->regs_info); i++) { + register_reset(&te->regs_info[i]); + } +} + +static void trencoder_realize (DeviceState *dev,Error **errp) +{ + TraceEncoder *te = TRACE_ENCODER(dev); + + memory_region_init_io(&te->reg_mem, OBJECT(dev), + &trencoder_ops, te, + TYPE_TRACE_ENCODER, + te->reg_mem_size); + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &te->reg_mem); + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, te->baseaddr); + + /* RegisterInfo init taken from hw/dma/xlnx-zdma.c */ + for (int i = 0; i < ARRAY_SIZE(trencoder_regs_info); i++) { + uint32_t reg_idx = trencoder_regs_info[i].addr / 4; + RegisterInfo *r = &te->regs_info[reg_idx]; + + *r = (RegisterInfo) { + .data = (uint8_t *)&te->regs[reg_idx], + .data_size = sizeof(uint32_t), + .access = &trencoder_regs_info[i], + .opaque = te, + }; + } +} + +static const Property trencoder_props[] = { + /* + * We need a link to the associated CPU to + * enable/disable tracing. + */ + DEFINE_PROP_LINK("cpu", TraceEncoder, cpu, TYPE_RISCV_CPU,RISCVCPU *),
+DEFINE_PROP_UINT64("baseaddr", TraceEncoder, baseaddr, 0), +DEFINE_PROP_UINT64("dest-baseaddr", TraceEncoder, dest_baseaddr, 0),
+ DEFINE_PROP_UINT64("ramsink-ramstart", TraceEncoder, +ramsink_ramstart, 0),
+ DEFINE_PROP_UINT64("ramsink-ramlimit", TraceEncoder, +ramsink_ramlimit, 0),
+ DEFINE_PROP_UINT32("reg-mem-size", TraceEncoder,
+reg_mem_size,TRACE_R_MAX* 4),
+DEFINE_PROP_INT32("cpu-id", TraceEncoder, cpu_id, 0),hw/riscv/trace-encoder.c new file +18
-0+}; + +static const VMStateDescription vmstate_trencoder = { + .name = TYPE_TRACE_ENCODER, + .version_id = 1, + .minimum_version_id = 1, + .fields = (const VMStateField[]) { + VMSTATE_UINT32_ARRAY(regs, TraceEncoder, TRACE_R_MAX), + VMSTATE_UINT64(baseaddr, TraceEncoder), + VMSTATE_UINT64(dest_baseaddr, TraceEncoder), + VMSTATE_UINT64(ramsink_ramstart, TraceEncoder), + VMSTATE_UINT64(ramsink_ramlimit, TraceEncoder), + VMSTATE_INT32(cpu_id, TraceEncoder), + VMSTATE_END_OF_LIST(), + } +}; + +static void trencoder_class_init(ObjectClass *klass, const void*data)
hw/riscv/trace-encoder.c new file +22
-0+{ + DeviceClass *dc = DEVICE_CLASS(klass); + + device_class_set_legacy_reset(dc, trencoder_reset); + device_class_set_props(dc, trencoder_props); + dc->realize = trencoder_realize; + dc->vmsd = &vmstate_trencoder; +} + +static const TypeInfo trencoder_info = { + .name = TYPE_TRACE_ENCODER, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(TraceEncoder), + .class_init = trencoder_class_init, +}; + +static void trencoder_register_types (void ) +{ + type_register_static(&trencoder_info); +} + +type_init (trencoder_register_types)hw/riscv/trace-encoder.h new file +4
-0@@ -0,0 +1,42 @@ +/* +* Emulation of a RISC-V Trace Encoder +* +* Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */hw/riscv/trace-encoder.h new file +35
-0+ +#ifndef RISCV_TRACE_ENCODER_H +#define RISCV_TRACE_ENCODER_H + +#include "hw/core/sysbus.h" +#include "hw/core/register.h" +#include "system/dma.h" +#include "qom/object.h" +#include "cpu.h" + +#define TRACE_R_MAX (0xFFF / 4) + +struct TraceEncoder { + /*< private >*/ + SysBusDevice parent_obj; + + RISCVCPU *cpu; + int cpu_id; + + MemoryRegion reg_mem; + uint32_t reg_mem_size; + + hwaddr baseaddr; + hwaddr dest_baseaddr; + hwaddr ramsink_ramstart; + hwaddr ramsink_ramlimit; + uint32_t regs[TRACE_R_MAX]; + RegisterInfo regs_info[TRACE_R_MAX]; +}; + +#define TYPE_TRACE_ENCODER "trace-encoder" + +OBJECT_DECLARE_SIMPLE_TYPE(TraceEncoder, TRACE_ENCODER) + +#endif hw/riscv/trace-events +0
-0@@ -24,3 +24,7 @@ riscv_iommu_hpm_incr_ctr(uint64_t cntr_val)"
cntr_val0x%"PRIx64
riscv_iommu_hpm_iocntinh_cy(boolprev_cy_inh) "prev_cy_inh%d"riscv_iommu_hpm_cycle_write(uint32_t ovf, uint64_t val)"ovf 0x%x val 0x%"PRIx64
riscv_iommu_hpm_evt_write(uint32_tctr_idx,uint32_tovf,uint64_tval) "ctr_idx0x%x ovf 0x%x val 0x%"PRIx64hw/riscv/trace-events +4
-0+ +# trace-encoder.c +trencoder_read_error(uint64_t addr) "addr 0x%" PRIx64 +trencoder_write_error(uint64_t addr, uint64_t value) "addr 0x%"PRIx64 " value 0x%" PRIx64
PATCH v4 2/18 hw/riscv: Trace RAM Sink initial impl
In reply to Daniel Henrique BarbozaFollowing the effort to implement the basic support for Efficient Trace (e-trace) in QEMU we'll add a Trace RAM Sink implementation.
Similar to the Trace Encoder, this is inspired in both the Efficient Trace for RISC-V [1] and RISC-V Trace Control Interface Specification
[2] specs. It implements a minimal set of features to get started - only SMEM will be supported for now.
We'll implement the RAM sink logic in the next patches, although most of the work will be done by the trace encoder.
[1] https://github.com/riscv-non-isa/riscv-trace-spec/releases/download/v2.0-20250616/riscv-trace-spec-asciidoc.pdf
[2] https://github.com/riscv-non-isa/tg-nexus-trace/releases/download/1.0_Ratified/RISC-V-Trace-Control-Interface.pdf
--- hw/riscv/meson.build | 2 +- hw/riscv/trace-events | 4 + hw/riscv/trace-ram-sink.c | 263 ++++++++++++++++++++++++++++++++++++++ hw/riscv/trace-ram-sink.h | 83 ++++++++++++ 4 files changed, 351 insertions(+), 1 deletion(-) create mode 100644 hw/riscv/trace-ram-sink.c create mode 100644 hw/riscv/trace-ram-sink.h
hw/riscv/meson.build +1 -1
@@ -22,7 +22,7 @@ riscv_ss.add(when: 'CONFIG_XIANGSHAN_KUNMINGHU', if_true: files('xiangshan_kmh.c riscv_ss.add(when: 'CONFIG_RISCV_MIPS_CPS', if_true: files('cps.c')) riscv_ss.add(when: 'CONFIG_MIPS_BOSTON_AIA', if_true: files('boston-aia.c')) riscv_ss.add(when: 'CONFIG_K230', if_true: files('k230.c')) -riscv_ss.add(when: 'CONFIG_RISCV_TRACE', if_true: files('trace-encoder.c')) +riscv_ss.add(when: 'CONFIG_RISCV_TRACE', if_true: files('trace-encoder.c', 'trace-ram-sink.c')) riscv_ss.add(when: 'CONFIG_SOPHGO_CV1800B', if_true: files('cv1800b.c')) riscv_ss.add(when: 'CONFIG_MILKV_DUO', if_true: files('milkv_duo.c'))
hw/riscv/trace-events +4 -0
@@ -28,3 +28,7 @@ riscv_iommu_hpm_evt_write(uint32_t ctr_idx, uint32_t ovf, uint64_t val) "ctr_idx # trace-encoder.c trencoder_read_error(uint64_t addr) "addr 0x%" PRIx64 trencoder_write_error(uint64_t addr, uint64_t value) "addr 0x%" PRIx64 " value 0x%" PRIx64 + +# trace-ram-sink.c +tr_ramsink_read_error(uint64_t addr) "addr 0x%" PRIx64 +tr_ramsink_write_error(uint64_t addr, uint64_t value) "addr 0x%" PRIx64 " value 0x%" PRIx64
hw/riscv/trace-ram-sink.c new file +263 -0
263 lines not on this page · raw message
hw/riscv/trace-ram-sink.h new file +83 -0
83 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v4 02/18] hw/riscv: Trace RAM Sink initial impl
Quoted 6 lines
+static
uint64_ttr_ramsink_msgread(void *opaque, hwaddr addr, unsigned size)hw/riscv/trace-ram-sink.c new file +3
-0+{ +TraceRamSink *tram =TRACE_RAM_SINK (opaque); +
hw/riscv/trace-ram-sink.c line 120
Quoted 10 lines
hw/riscv/trace-ram-sink.c new file +10
-0+switch (size) { +case 1 : +return tram->msgs[addr]; +case 2 : +return (uint16_t )tram->msgs[addr]; +case 4 : +return (uint32_t )tram->msgs[addr]; +default : +g_assert_not_reached (); + }
Could we use little-endian load helpers for the multi-byte accesses? As written, both cases only read tram->msgs[addr]; the casts do not load the following bytes. For example:
case 2:
return lduw_le_p(&tram->msgs[addr]);
case 4:
return ldl_le_p(&tram->msgs[addr]);Thanks
Jay Chang
PATCH v4 3/18 hw/riscv/trace-encoder: add trace start/stop logic
In reply to Daniel Henrique BarbozaStarting/stopping a trace session will rely on two flags:
- trace_next_insn: this flag will be used to signal the translation that the encoder wants to know the PC of the current insn. The translation helper (to be added) will clear this flag after sending the current insn to the encoder;
- trace_running: shows that we're running a trace session and certain insns classes (traps, certain jumps and branches) need to report to the trace encoder.
These flags are controlled by the trTeInstTracing bit. This bit requires other two bits to be set beforehand (ACTIVE and ENABLE). We'll revisit these bits in the future as more features are implemented.
While we're at it, add hardwire bits to indicate which features we're (not) implementing at this time.
--- hw/riscv/trace-encoder.c | 107 +++++++++++++++++++++++++++++++++++++++ hw/riscv/trace-encoder.h | 4 ++ 2 files changed, 111 insertions(+)
hw/riscv/trace-encoder.c +107 -0
@@ -30,7 +30,11 @@ REG32(TR_TE_CONTROL, 0x0) FIELD(TR_TE_CONTROL, INST_TRACING, 2, 1) FIELD(TR_TE_CONTROL, EMPTY, 3, 1) FIELD(TR_TE_CONTROL, INST_MODE, 4, 3) + FIELD(TR_TE_CONTROL, CONTEXT, 9, 1) + FIELD(TR_TE_CONTROL, INST_STALL_ENA, 13, 1) + FIELD(TR_TE_CONTROL, INHIBIT_SRC, 15, 1) FIELD(TR_TE_CONTROL, INST_SYNC_MODE, 16, 2) + FIELD(TR_TE_CONTROL, INST_SYNC_MAX, 20, 4)FIELD (TR_TE_CONTROL ,FORMAT ,24 ,3 ) /* reserved bits */FIELD (TR_TE_CONTROL ,RSVP1 ,7 ,2 )
@@ -75,17 +79,116 @@ REG32(TR_TE_IMPL, 0x4)#define R_TR_TE_IMPL_RESET (BIT(0) | BIT(8)) +REG32(TR_TE_INST_FEATURES, 0x8) + FIELD(TR_TE_INST_FEATURES, NO_ADDR_DIFF, 0, 1) + +static uint64_t trencoder_te_ctrl_set_hardwire_vals (uint64_t input) +{ + input =FIELD_DP32 (input,TR_TE_CONTROL ,INST_MODE ,0x6 ); + input =FIELD_DP32 (input,TR_TE_CONTROL ,CONTEXT ,0 ); + input =FIELD_DP32 (input,TR_TE_CONTROL ,INST_STALL_ENA ,0 ); + input =FIELD_DP32 (input,TR_TE_CONTROL ,INHIBIT_SRC ,1 ); + input =FIELD_DP32 (input,TR_TE_CONTROL ,FORMAT ,0 ); + +/* SYNC_MODE and SYNC_MAX will be revisited */ + input =FIELD_DP32 (input,TR_TE_CONTROL ,INST_SYNC_MODE ,0 ); + input =FIELD_DP32 (input,TR_TE_CONTROL ,INST_SYNC_MAX ,0 ); + +return input; +} + +static uint64_t trencoder_te_ctrl_prew (RegisterInfo *reg,uint64_t val) +{ +TraceEncoder *te =TRACE_ENCODER (reg->opaque); +uint32_t trTeActive =ARRAY_FIELD_EX32 (te->regs,TR_TE_CONTROL ,ACTIVE ); +uint32_t trTeInstTracing =ARRAY_FIELD_EX32 (te->regs,TR_TE_CONTROL , +INST_TRACING ); +uint32_t temp; + + val =trencoder_te_ctrl_set_hardwire_vals (val); + +if (!trTeActive) { +/* +* 11.2 Reset and discovery, table 58, trTeControl = 0x1 +* means "Release from reset and set all defaults." Do +* that only if trTeActive is 0. +*/ +if (val ==0x1 ) { + val =FIELD_DP32 (val,TR_TE_CONTROL ,EMPTY ,1 ); + +return val; + } + +/* +* 11.3 Enabling and Disabling hints that the device must +* be activated first (trTeActive = 1), then enabled. +* Do not enable the device if it's not active +* beforehand. +*/ + temp =FIELD_EX32 (val,TR_TE_CONTROL ,ENABLE ); +if (temp) { + val =FIELD_DP32 (val,TR_TE_CONTROL ,ENABLE ,0 ); + } + } + +/* +* Do not allow inst tracing to start if the device isn't +* already enabled. Do not allow enabling the devince and +* and enable tracing at the same time. +*/ +if (!te->enabled && trTeInstTracing) { + val =FIELD_DP32 (val,TR_TE_CONTROL ,INST_TRACING ,0 ); + } + +return val; +} + +static void trencoder_te_ctrl_postw (RegisterInfo *reg,uint64_t val) +{ +TraceEncoder *te =TRACE_ENCODER (reg->opaque); +uint32_t trTeActive =ARRAY_FIELD_EX32 (te->regs,TR_TE_CONTROL ,ACTIVE ); +uint32_t trTeEnable =ARRAY_FIELD_EX32 (te->regs,TR_TE_CONTROL ,ENABLE ); +uint32_t trTeInstTracing =ARRAY_FIELD_EX32 (te->regs,TR_TE_CONTROL , +INST_TRACING ); + +if (!trTeActive) { + te->enabled = false; + te->trace_running = false; + te->trace_next_insn = false; +return ; + } + +if (te->enabled && !trTeEnable) { +/* TODO: this should cause a pending trace data flush. */ + } + + te->enabled = trTeEnable ? true : false; + +if (!te->trace_running && trTeInstTracing) { +/* Starting trace. Ask the CPU for the first trace insn */ + te->trace_next_insn = true; + } + + te->trace_running = trTeInstTracing ? true : false; +} +static RegisterAccessInfo trencoder_regs_info[] = { { .name ="TR_TE_CONTROL" , .addr =A_TR_TE_CONTROL , .rsvd =R_TR_TE_CONTROL_RSVP_BITS , .reset =R_TR_TE_CONTROL_RESET , .ro =R_TR_TE_CONTROL_RO_BITS , + .pre_write = &trencoder_te_ctrl_prew, + .post_write = &trencoder_te_ctrl_postw, }, { .name ="TR_TE_IMPL" , .addr =A_TR_TE_IMPL , .rsvd =R_TR_TE_IMPL_RSVP_BITS , .reset =R_TR_TE_IMPL_RESET , .ro =R_TR_TE_IMPL_RO_BITS , }, + { .name ="TR_TE_INST_FEATURES" , .addr =A_TR_TE_INST_FEATURES , + .reset =R_TR_TE_INST_FEATURES_NO_ADDR_DIFF_MASK , + .ro = ~0 , + }, };static uint64_t trencoder_read (void *opaque,hwaddr addr,unsigned size)
@@ -132,6 +235,10 @@ static void trencoder_reset(DeviceState *dev)for (int i =0 ; i <ARRAY_SIZE (te->regs_info); i++) {register_reset (&te->regs_info[i]); } + + te->enabled = false; + te->trace_running = false; + te->trace_next_insn = false; }static void trencoder_realize (DeviceState *dev,Error **errp)
hw/riscv/trace-encoder.h +4 -0
@@ -33,6 +33,10 @@ struct TraceEncoder {hwaddr ramsink_ramlimit;uint32_t regs[TRACE_R_MAX ];RegisterInfo regs_info[TRACE_R_MAX ]; + +bool enabled; +bool trace_running; +bool trace_next_insn; };#define TYPE_TRACE_ENCODER "trace-encoder"
Signature · 1 line
2.43.0
Re: [PATCH v4 03/18] hw/riscv/trace-encoder: add trace start/stop logic
Quoted 9 lines
These flags are controlled by the trTeInstTracing bit. This bit requires other two bits to be set beforehand (ACTIVE and ENABLE). We'll revisit these bits in the future as more features are implemented.
While we're at it, add hardwire bits to indicate which features we're (not) implementing at this time.
Signed-off-by Daniel Henrique Barboza <[email protected]>
Thanks
Jay Chang
Re: [PATCH v4 03/18] hw/riscv/trace-encoder: add trace start/stop logic
In reply to Daniel Henrique BarbozaQuoted 19 lines · Daniel Henrique Barboza
Starting/stopping a trace session will rely on two flags:
-
trace_next_insn: this flag will be used to signal the translation that
the encoder wants to know the PC of the current insn. The translation
helper (to be added) will clear this flag after sending the current insn to the encoder;-
trace_running: shows that we're running a trace session and certain insns classes (traps, certain jumps and branches) need to report to the trace encoder.These flags are controlled by the trTeInstTracing bit. This bit requires
other two bits to be set beforehand (ACTIVE and ENABLE). We'll revisit
these bits in the future as more features are implemented.
Quoted 5 lines
While we're at it, add hardwire bits to indicate which features we're (not) implementing at this time.
Signed-off-by: Daniel Henrique Barboza
<[email protected]>
Alistair
Quoted 173 lines
---
hw/riscv/trace-encoder.c | 107
+++++++++++++++++++++++++++++++++++++++
hw/riscv/trace-encoder.h | 4 ++2 files changed, 111 insertions(+)hw/riscv/trace-encoder.c +4
-0@@ -30,7 +30,11 @@ REG32(TR_TE_CONTROL, 0x0) FIELD(TR_TE_CONTROL, INST_TRACING, 2, 1) FIELD(TR_TE_CONTROL, EMPTY, 3, 1) FIELD(TR_TE_CONTROL, INST_MODE, 4, 3) + FIELD(TR_TE_CONTROL, CONTEXT, 9, 1) + FIELD(TR_TE_CONTROL, INST_STALL_ENA, 13, 1) + FIELD(TR_TE_CONTROL, INHIBIT_SRC, 15, 1) FIELD(TR_TE_CONTROL, INST_SYNC_MODE, 16, 2) + FIELD(TR_TE_CONTROL, INST_SYNC_MAX, 20, 4) FIELD(TR_TE_CONTROL, FORMAT, 24, 3) /* reserved bits */ FIELD(TR_TE_CONTROL, RSVP1, 7, 2)@@ -75,17 +79,116 @@ REG32(TR_TE_IMPL, 0x4)
#defineR_TR_TE_IMPL_RESET(BIT(0) | BIT(8))hw/riscv/trace-encoder.c +19
-0+REG32(TR_TE_INST_FEATURES, 0x8) + FIELD(TR_TE_INST_FEATURES, NO_ADDR_DIFF, 0, 1) + +static uint64_t trencoder_te_ctrl_set_hardwire_vals (uint64_t input) +{ + input = FIELD_DP32(input, TR_TE_CONTROL, INST_MODE, 0x6); + input = FIELD_DP32(input, TR_TE_CONTROL, CONTEXT, 0); + input = FIELD_DP32(input, TR_TE_CONTROL, INST_STALL_ENA, 0); + input = FIELD_DP32(input, TR_TE_CONTROL, INHIBIT_SRC, 1); + input = FIELD_DP32(input, TR_TE_CONTROL, FORMAT, 0); + + /* SYNC_MODE and SYNC_MAX will be revisited */ + input = FIELD_DP32(input, TR_TE_CONTROL, INST_SYNC_MODE, 0); + input = FIELD_DP32(input, TR_TE_CONTROL, INST_SYNC_MAX, 0); + + return input; +} + +static uint64_t trencoder_te_ctrl_prew(RegisterInfo *reg, uint64_tval)
+{
+ TraceEncoder *te =TRACE_ENCODER(reg->opaque);
+uint32_ttrTeActive =ARRAY_FIELD_EX32(te->regs, TR_TE_CONTROL, ACTIVE);
+uint32_ttrTeInstTracing = ARRAY_FIELD_EX32(te->regs,TR_TE_CONTROL,
+INST_TRACING); +uint32_ttemp;hw/riscv/trace-encoder.c +43
-0+ + val = trencoder_te_ctrl_set_hardwire_vals(val); + + if (!trTeActive) { + /* + * 11.2 Reset and discovery, table 58, trTeControl = 0x1 + * means "Release from reset and set all defaults." Do + * that only if trTeActive is 0. + */ + if (val == 0x1) { + val = FIELD_DP32(val, TR_TE_CONTROL, EMPTY, 1); + + return val; + } + + /* + * 11.3 Enabling and Disabling hints that the device must + * be activated first (trTeActive = 1), then enabled. + * Do not enable the device if it's not active + * beforehand. + */ + temp = FIELD_EX32(val, TR_TE_CONTROL, ENABLE); + if (temp) { + val = FIELD_DP32(val, TR_TE_CONTROL, ENABLE, 0); + } + } + + /* + * Do not allow inst tracing to start if the device isn't + * already enabled. Do not allow enabling the devince and + * and enable tracing at the same time. + */ + if (!te->enabled && trTeInstTracing) { + val = FIELD_DP32(val, TR_TE_CONTROL, INST_TRACING, 0); + } + + return val; +} + +static void trencoder_te_ctrl_postw (RegisterInfo *reg,uint64_t val) +{ + TraceEncoder *te = TRACE_ENCODER(reg->opaque); + uint32_t trTeActive = ARRAY_FIELD_EX32(te->regs, TR_TE_CONTROL,ACTIVE);
+uint32_ttrTeEnable =ARRAY_FIELD_EX32(te->regs, TR_TE_CONTROL, ENABLE);
+uint32_ttrTeInstTracing = ARRAY_FIELD_EX32(te->regs,TR_TE_CONTROL,
+INST_TRACING);hw/riscv/trace-encoder.c +22
-0+ + if (!trTeActive) { + te->enabled = false; + te->trace_running = false; + te->trace_next_insn = false; + return; + } + + if (te->enabled && !trTeEnable) { + /* TODO: this should cause a pending trace data flush. */ + } + + te->enabled = trTeEnable ? true : false; + + if (!te->trace_running && trTeInstTracing) { + /* Starting trace. Ask the CPU for the first trace insn */ + te->trace_next_insn = true; + } + + te->trace_running = trTeInstTracing ? true : false; +} +static RegisterAccessInfo trencoder_regs_info[] = {
{ .name = "TR_TE_CONTROL", .addr =A_TR_TE_CONTROL, .rsvd =R_TR_TE_CONTROL_RSVP_BITS,
.reset =R_TR_TE_CONTROL_RESET,
.ro =R_TR_TE_CONTROL_RO_BITS,
+ .pre_write =&trencoder_te_ctrl_prew,
+ .post_write =&trencoder_te_ctrl_postw,
},
{ .name = "TR_TE_IMPL", .addr =A_TR_TE_IMPL, .rsvd =R_TR_TE_IMPL_RSVP_BITS,
.reset =R_TR_TE_IMPL_RESET,
.ro =R_TR_TE_IMPL_RO_BITS,
},
+ { .name = "TR_TE_INST_FEATURES", .addr =A_TR_TE_INST_FEATURES,
+ .reset =R_TR_TE_INST_FEATURES_NO_ADDR_DIFF_MASK, + .ro = ~0,
+ },
};
staticuint64_ttrencoder_read(void *opaque, hwaddr addr, unsigned size)hw/riscv/trace-encoder.c +4
-0@@ -132,6 +235,10 @@ static void trencoder_reset(DeviceState *dev) for (int i = 0; i < ARRAY_SIZE(te->regs_info); i++) { register_reset(&te->regs_info[i]); } + + te->enabled = false; + te->trace_running = false; + te->trace_next_insn = false; }
static voidtrencoder_realize(DeviceState *dev, Error **errp)hw/riscv/trace-encoder.h +4
-0@@ -33,6 +33,10 @@ struct TraceEncoder { hwaddr ramsink_ramlimit; uint32_t regs[TRACE_R_MAX]; RegisterInfo regs_info[TRACE_R_MAX]; + + bool enabled; + bool trace_running; + bool trace_next_insn; };
#defineTYPE_TRACE_ENCODER"trace-encoder"
PATCH v4 4/18 hw/riscv/virt.c: add trace encoders and trace ram sinks
In reply to Daniel Henrique BarbozaAdd the minimal topology required for e-trace support in the virt machine, e.g. each CPU will have a single trace encoder, and each trace encoder will communicate to a single associated trace ram sink.
At this moment we're not going to support more complex topologies with trace funnels and so on.
--- hw/riscv/virt.c | 81 ++++++++++++++++++++++++++++++++++++++ include/hw/riscv/virt.h | 2 + target/riscv/cpu.h | 9 +++++ target/riscv/tcg/tcg-cpu.c | 5 +++ 4 files changed, 97 insertions(+)
hw/riscv/virt.c +81 -0
@@ -60,6 +60,8 @@#include "qapi/qapi-visit-common.h" #include "hw/virtio/virtio-iommu.h" #include "hw/uefi/var-service-api.h" +#include "hw/riscv/trace-encoder.h" +#include "hw/riscv/trace-ram-sink.h" #include "aia.h"
@@ -83,6 +85,17 @@ static bool virt_aclint_allowed(void)return tcg_enabled () ||qtest_enabled (); } +#define TR_DEV_REGMAP_SIZE 0x1000 +/* For VIRT_CPUS_MAX = 512: TRACE_DEV_REG_MAX = 0x200000 */ +#define TRACE_DEV_REG_MAX (TR_DEV_REGMAP_SIZE * VIRT_CPUS_MAX) + +/* +* 64k for the RAM Sink that includes the 4k (0x1000) +* for regs, for each possible CPU. For 512 max CPUs, +* total size = 0x2000000. +*/ +#define TRACE_RAM_SINK_SIZE (1UL << 16) +static const MemMapEntry virt_memmap[] = { [VIRT_DEBUG ] = {0x0 ,0x100 }, [VIRT_MROM ] = {0x1000 ,0xf000 },
@@ -92,7 +105,9 @@ static const MemMapEntry virt_memmap[] = { [VIRT_ACLINT_SSWI] = { 0x2F00000, 0x4000 }, [VIRT_PCIE_PIO] = { 0x3000000, 0x10000 }, [VIRT_IOMMU_SYS] = { 0x3010000, 0x1000 }, + [VIRT_TR_ENCODERS] = { 0x3020000, TRACE_DEV_REG_MAX }, [VIRT_PLATFORM_BUS] = { 0x4000000, 0x2000000 }, + [VIRT_TR_RAM_SINKS] = { 0x6000000, TRACE_RAM_SINK_SIZE * VIRT_CPUS_MAX }, [VIRT_PLIC] = { 0xc000000, VIRT_PLIC_SIZE(VIRT_CPUS_MAX * 2) }, [VIRT_APLIC_M] = { 0xc000000, APLIC_SIZE(VIRT_CPUS_MAX) }, [VIRT_APLIC_S] = { 0xd000000, APLIC_SIZE(VIRT_CPUS_MAX) },
@@ -710,6 +725,68 @@ static void virt_machine_done(Notifier *notifier, void *data) } } +/* +* Must be called after 'soc' realize since it +* uses CPU objs. +*/ +static void virt_init_socket_trace_hw (RISCVVirtState *s,int socket_num) +{ +int hartid_base = s->soc[socket_num].hartid_base; + +for (int cpu =0 ; cpu < s->soc[socket_num].num_harts; cpu++) { +RISCVCPU *cpu_ptr = &s->soc[socket_num].harts[cpu]; +DeviceState *trencoder, *ram_sink; +uint64_t trencoder_addr, ram_sink_addr, smem_addr; +uint32_t smem_size =TRACE_RAM_SINK_SIZE -TR_DEV_REGMAP_SIZE ; +int cpu_id = hartid_base + cpu; + + ram_sink =qdev_new (TYPE_TRACE_RAM_SINK ); + + ram_sink_addr = virt_memmap[VIRT_TR_RAM_SINKS ].base + +TRACE_RAM_SINK_SIZE * cpu_id; +/* smem is located right after ram sink base */ + smem_addr = ram_sink_addr +TR_DEV_REGMAP_SIZE ; + +object_property_set_uint (OBJECT (ram_sink),"baseaddr" , + ram_sink_addr, &error_fatal); +object_property_set_uint (OBJECT (ram_sink),"smemaddr" , + smem_addr, &error_fatal); +object_property_set_uint (OBJECT (ram_sink),"smemsize" , + smem_size, &error_fatal); +sysbus_realize_and_unref (SYS_BUS_DEVICE (ram_sink), &error_fatal); + +/* +* We can't do object_property_set_link() because we're +* coming after cpu.realize() (the riscv_hart obj creates +* the CPU objs in its realize() since it has no init). +* We need changes in how riscv_hart works to use +* set_link() and to not manually realize the trace +* encoder. +* +* For now do everything manually. +*/ + trencoder =qdev_new (TYPE_TRACE_ENCODER ); + cpu_ptr->trencoder =OBJECT (trencoder); + + trencoder_addr = virt_memmap[VIRT_TR_ENCODERS ].base + +TR_DEV_REGMAP_SIZE * cpu_id; + +object_property_set_link (OBJECT (trencoder),"cpu" , +OBJECT (cpu_ptr), &error_fatal); +object_property_set_int (OBJECT (trencoder),"cpu-id" , cpu_id, + &error_fatal); +object_property_set_uint (OBJECT (trencoder),"baseaddr" , + trencoder_addr, &error_fatal); +object_property_set_uint (OBJECT (trencoder),"dest-baseaddr" , + ram_sink_addr, &error_fatal); +object_property_set_uint (OBJECT (trencoder),"ramsink-ramstart" , + smem_addr, &error_fatal); +object_property_set_uint (OBJECT (trencoder),"ramsink-ramlimit" , + smem_addr + smem_size, &error_fatal); +sysbus_realize_and_unref (SYS_BUS_DEVICE (trencoder), &error_fatal); + } +} +static void virt_machine_init (MachineState *machine) {RISCVVirtState *s =RISCV_VIRT_MACHINE (machine);
@@ -765,6 +842,10 @@ static void virt_machine_init(MachineState *machine) hart_count, &error_abort);sysbus_realize (SYS_BUS_DEVICE (&s->soc[i]), &error_fatal); +if (tcg_enabled ()) { +virt_init_socket_trace_hw (s, i); + } +if (virt_aclint_allowed () && s->have_aclint) {if (s->aia_type ==VIRT_AIA_TYPE_APLIC_IMSIC ) {/* Per-socket ACLINT MTIMER */
include/hw/riscv/virt.h +2 -0
@@ -91,6 +91,8 @@ enum {VIRT_PLATFORM_BUS ,VIRT_PCIE_ECAM ,VIRT_IOMMU_SYS , +VIRT_TR_ENCODERS , + VIRT_TR_RAM_SINKS, }; enum {
target/riscv/cpu.h +9 -0
@@ -593,6 +593,15 @@ struct ArchCPU {GHashTable *misa_ext_user_opts;GHashTable *multi_ext_user_opts;const GPtrArray *decoders; + +#ifndef CONFIG_USER_ONLY +/* +* Associated Trace Encoder. It will not be NULL if +* we're running with TCG and initialized manually by +* the board. +*/ +Object *trencoder; +#endif };typedef struct RISCVCSR RISCVCSR ;
target/riscv/tcg/tcg-cpu.c +5 -0
@@ -39,6 +39,7 @@#include "exec/icount.h" #include "target/riscv/tcg/debug.h" #include "target/riscv/tcg/csr.h" +#include "hw/riscv/trace-encoder.h" #endif static GHashTable *multi_ext_implied_rules;
@@ -1366,6 +1367,10 @@ static bool riscv_tcg_cpu_realize(CPUState *cs, Error **errp)if (riscv_has_ext (env,RVH )) { env->mideleg =MIP_VSSIP |MIP_VSTIP |MIP_VSEIP |MIP_SGEIP ; } + +if (cpu->trencoder) { +qdev_realize (DEVICE (cpu->trencoder),NULL , &error_fatal); + }#endif return true;
Signature · 1 line
2.43.0
Re: [PATCH v4 04/18] hw/riscv/virt.c: add trace encoders and trace ram sinks
Quoted 10 lines
Add the minimal topology required for e-trace support in the virt machine, e.g. each CPU will have a single trace encoder, and each trace encoder will communicate to a single associated trace ram sink.
At this moment we're not going to support more complex topologies with trace funnels and so on.
Signed-off-by Daniel Henrique Barboza <[email protected]>
Thanks
Jay Chang
PATCH v4 5/18 hw/riscv/virt.c add trace encoder and ramsink fdt nodes
In reply to Daniel Henrique BarbozaThe trace encoder and trace ramsink nodes should confirm to the bindings described in "riscv,trace-component.yaml" in the Linux kernel. That way, encoder and ramsink devices get populated on the rvtrace bus and perf record is able to capture the trace data in the auxtrace section as expected.
--- hw/riscv/fdt-common.c | 70 +++++++++++++++++++++++++++++++++++ hw/riscv/virt.c | 47 +++++++++++++++++++++++ include/hw/riscv/fdt-common.h | 4 ++ 3 files changed, 121 insertions(+)
hw/riscv/fdt-common.c +70 -0
@@ -105,6 +105,12 @@ void riscv_fdt_create_cpu_socket_subnode(void *fdt,qemu_fdt_add_subnode (fdt,"/cpus/cpu-map" ); } +/* Caller must free string after use */ +static char *fdt_get_cpu_nodename (int cpu_id) +{ +return g_strdup_printf ("/cpus/cpu@%d" , cpu_id); +} + static void create_fdt_socket_cpu_internal(void *fdt, char *clust_name, RISCVCPU *cpu_ptr, int cpu, int socket_id, int socket_hartid_base,
@@ -849,3 +855,67 @@ void riscv_create_fdt_rtc(void *fdt, const MemMapEntry *rtc_mem,qemu_fdt_setprop_cells (fdt, name,"interrupts" , rtc_irq,0x4 ); } } + +/* Returns the 'out-ports/port/endpoint' nodepath. Caller +must free the string after use. */ +char *riscv_create_fdt_trace_encoder (void *fdt,hwaddr addr,hwaddr size, +int cpu_id) +{ +static const char *const tr_compat[2 ] = {"qemu,trace-component" , +"riscv,trace-component" }; + g_autofree char *cpu_nodename = fdt_get_cpu_nodename(cpu_id); + g_autofree char *trencoder_name = NULL; +char *endpoint =NULL ; + + trencoder_name =g_strdup_printf ("/soc/encoder@%" HWADDR_PRIx, addr); +qemu_fdt_add_subnode (fdt, trencoder_name); + +qemu_fdt_setprop_sized_cells (fdt, trencoder_name,"reg" , +2 , addr, +2 , size); + +qemu_fdt_setprop_string_array (fdt, trencoder_name,"compatible" , + (char **)&tr_compat, +ARRAY_SIZE (tr_compat)); + +qemu_fdt_setprop_phandle (fdt, trencoder_name,"cpus" , cpu_nodename); + + endpoint =g_strdup_printf ("%s/out-ports/port/endpoint" , trencoder_name); +qemu_fdt_add_path (fdt, endpoint); +qemu_fdt_setprop_cell (fdt, endpoint,"phandle" , +qemu_fdt_alloc_phandle (fdt)); + +return endpoint; +} + +/* Returns the 'in-ports/port/endpoint' nodepath. Caller +must free the string after use. */ +char *riscv_create_fdt_trace_ramsink (void *fdt,hwaddr addr,hwaddr size, +int cpu_id) +{ +static const char *const tr_compat[2 ] = {"qemu,trace-component" , +"riscv,trace-component" }; + g_autofree char *cpu_nodename = fdt_get_cpu_nodename(cpu_id); + g_autofree char *ramsink_name = NULL; +char *endpoint =NULL ; + + ramsink_name =g_strdup_printf ("/soc/ramsink@%" HWADDR_PRIx, addr); +qemu_fdt_add_subnode (fdt, ramsink_name); + +qemu_fdt_setprop_sized_cells (fdt, ramsink_name,"reg" , +2 , addr, +2 , size); + +qemu_fdt_setprop_string_array (fdt, ramsink_name,"compatible" , + (char **)&tr_compat, +ARRAY_SIZE (tr_compat)); + +qemu_fdt_setprop_phandle (fdt, ramsink_name,"cpus" , cpu_nodename); + + endpoint =g_strdup_printf ("%s/in-ports/port/endpoint" , ramsink_name); +qemu_fdt_add_path (fdt, endpoint); +qemu_fdt_setprop_cell (fdt, endpoint,"phandle" , +qemu_fdt_alloc_phandle (fdt)); + +return endpoint; +}
hw/riscv/virt.c +47 -0
59 lines not on this page · raw message
include/hw/riscv/fdt-common.h +4 -0
@@ -136,4 +136,8 @@ void riscv_create_fdt_uart(void *fdt, const MemMapEntry *uart_mem,void riscv_create_fdt_rtc (void *fdt,const MemMapEntry *rtc_mem,int rtc_irq,int aia_type,uint32_t irq_phandle); +char *riscv_create_fdt_trace_encoder (void *fdt,hwaddr addr,hwaddr size, +int cpu_id); +char *riscv_create_fdt_trace_ramsink (void *fdt,hwaddr addr,hwaddr size, +int cpu_id);#endif
Signature · 1 line
2.43.0
Re: [PATCH v4 05/18] hw/riscv/virt.c add trace encoder and ramsink fdt nodes
Quoted 10 lines · Daniel Henrique Barboza
The trace encoder and trace ramsink nodes should confirm to the bindings
described in "riscv,trace-component.yaml" in the Linux kernel. That way,
encoder and ramsink devices get populated on the rvtrace bus and perf record is able to capture the trace data in the auxtrace section as expected.Signed-off-by: Daniel Henrique Barboza
<[email protected]>
Alistair
Quoted 204 lines
---
hw/riscv/fdt-common.c | 70
+++++++++++++++++++++++++++++++++++
hw/riscv/virt.c | 47 +++++++++++++++++++++++include/hw/riscv/fdt-common.h| 4 ++3 files changed, 121 insertions(+)hw/riscv/fdt-common.c +0
-0@@ -105,6 +105,12 @@ void riscv_fdt_create_cpu_socket_subnode(void *fdt, qemu_fdt_add_subnode(fdt, "/cpus/cpu-map"); }hw/riscv/fdt-common.c +6
-0+/* Caller must free string after use */ +static char *fdt_get_cpu_nodename (int cpu_id) +{ + return g_strdup_printf("/cpus/cpu@%d", cpu_id); +} +static void
create_fdt_socket_cpu_internal(void *fdt, char*clust_name, RISCVCPU*cpu_ptr,
int cpu, intsocket_id, intsocket_hartid_base,hw/riscv/fdt-common.c +0
-0@@ -849,3 +855,67 @@ void riscv_create_fdt_rtc(void *fdt, constMemMapEntry *rtc_mem, qemu_fdt_setprop_cells(fdt, name, "interrupts", rtc_irq, 0x4);}
}hw/riscv/fdt-common.c +4
-0+ +/* Returns the 'out-ports/port/endpoint' nodepath. Caller +must free the string after use. */ +char *riscv_create_fdt_trace_encoder(void *fdt, hwaddr addr, hwaddrsize,
+ intcpu_id)
+{
+ static const char * const tr_compat[2] = { "qemu,trace- component",
+ "riscv,trace- component" };
+g_autofreechar*cpu_nodename=fdt_get_cpu_nodename(cpu_id); +g_autofreechar*trencoder_name= NULL;
+ char *endpoint = NULL;
+
+trencoder_name=g_strdup_printf("/soc/encoder@%"HWADDR_PRIx, addr);
+qemu_fdt_add_subnode(fdt, trencoder_name);hw/riscv/fdt-common.c +10
-0+ + qemu_fdt_setprop_sized_cells(fdt, trencoder_name, "reg", + 2, addr, + 2, size); + + qemu_fdt_setprop_string_array(fdt, trencoder_name, "compatible", + (char **)&tr_compat, + ARRAY_SIZE(tr_compat)); + + qemu_fdt_setprop_phandle(fdt, trencoder_name, "cpus",
cpu_nodename);
+
+ endpoint =g_strdup_printf("%s/out-ports/port/endpoint", trencoder_name);
+qemu_fdt_add_path(fdt, endpoint);
+ qemu_fdt_setprop_cell(fdt, endpoint, "phandle", +qemu_fdt_alloc_phandle(fdt));hw/riscv/fdt-common.c +7
-0+ + return endpoint; +} + +/* Returns the 'in-ports/port/endpoint' nodepath. Caller +must free the string after use. */ +char *riscv_create_fdt_trace_ramsink(void *fdt, hwaddr addr, hwaddrsize,
+ intcpu_id)
+{
+ static const char * const tr_compat[2] = { "qemu,trace- component",
+ "riscv,trace- component" };
+g_autofreechar*cpu_nodename=fdt_get_cpu_nodename(cpu_id); +g_autofreechar*ramsink_name= NULL;
+ char *endpoint = NULL;
+
+ramsink_name=g_strdup_printf("/soc/ramsink@%"HWADDR_PRIx, addr);
+qemu_fdt_add_subnode(fdt, ramsink_name);hw/riscv/fdt-common.c +10
-0+ + qemu_fdt_setprop_sized_cells(fdt, ramsink_name, "reg", + 2, addr, + 2, size); + + qemu_fdt_setprop_string_array(fdt, ramsink_name, "compatible", + (char **)&tr_compat, + ARRAY_SIZE(tr_compat)); + + qemu_fdt_setprop_phandle(fdt, ramsink_name, "cpus",
cpu_nodename);
+
+ endpoint =g_strdup_printf("%s/in-ports/port/endpoint", ramsink_name);
+qemu_fdt_add_path(fdt, endpoint);
+ qemu_fdt_setprop_cell(fdt, endpoint, "phandle", +qemu_fdt_alloc_phandle(fdt));hw/riscv/fdt-common.c +3
-0+ + return endpoint; +}hw/riscv/virt.c +0
-0@@ -479,6 +479,51 @@ static void create_fdt_iommu(RISCVVirtState *s, uint16_t bdf) s->pci_iommu_bdf = bdf; }hw/riscv/virt.c +19
-0+static void create_fdt_rvtrace (RISCVVirtState *s) +{ + MachineState *ms = MACHINE(s); + int socket_count = riscv_socket_count(ms); + hwaddr encoder_addr, ramsink_addr, size = 0x100; + RISCVCPU *cpu_ptr; + int cpu_id; + + for (int i = 0; i < socket_count; i++) { + for (int cpu = 0; cpu < s->soc[i].num_harts; cpu++) { + g_autofree char *in_endpoint = NULL; + g_autofree char *out_endpoint = NULL; + + cpu_ptr = &s->soc[i].harts[cpu]; + if (!cpu_ptr->trencoder) { + continue; + } + + encoder_addr = object_property_get_uint(cpu_ptr-Quoted 1 line
trencoder,
+ "baseaddr", +
&error_abort); +ramsink_addr= object_property_get_uint(cpu_ptr-Quoted 1 line
trencoder,
+ "dest-baseaddr", +
&error_abort); +cpu_id= s->soc[i].hartid_base + cpu;
+
+in_endpoint= riscv_create_fdt_trace_ramsink(ms->fdt, +ramsink_addr,
+ size,cpu_id);
+
+out_endpoint= riscv_create_fdt_trace_encoder(ms->fdt, +encoder_addr,
+ size,cpu_id);
+
+ /* The remote-endpoint property is bidirectional, i.e. the
+ in-ports endpoint must point to the out-ports endpoint
+ and vice-versa. */
+ qemu_fdt_setprop_phandle(ms->fdt,in_endpoint, + "remote-endpoint",out_endpoint);
+ qemu_fdt_setprop_phandle(ms->fdt,out_endpoint, + "remote-endpoint",in_endpoint);
+ }
+ }hw/riscv/virt.c +2
-0+} +static void
finalize_fdt(RISCVVirtState *s)
{
uint32_tphandle = 1,irq_mmio_phandle= 1,msi_pcie_phandle= 1;hw/riscv/virt.c +0
-0@@ -520,6 +565,8 @@ static void finalize_fdt(RISCVVirtState *s)
riscv_create_fdt_rtc(MACHINE(s)->fdt, &s->memmap[VIRT_RTC],RTC_IRQ,
s->aia_type,irq_mmio_phandle); +
+create_fdt_rvtrace(s);
}
static voidcreate_fdt(RISCVVirtState *s)include/hw/riscv/fdt- renamed from include/hw/riscv/fdt-common.h +0
-0
common.h
indexeb9db8278d..fd03a9e0ab100644include/hw/riscv/fdt-common.h +1
-0@@ -136,4 +136,8 @@ void riscv_create_fdt_uart(void *fdt, constMemMapEntry *uart_mem, void riscv_create_fdt_rtc(void *fdt, const MemMapEntry *rtc_mem, int rtc_irq, int aia_type, uint32_t irq_phandle); +char *riscv_create_fdt_trace_encoder(void *fdt, hwaddr addr, hwaddrsize,
+ intcpu_id);
+char *riscv_create_fdt_trace_ramsink(void *fdt, hwaddr addr, hwaddr size,
+ intcpu_id);
#endif
PATCH v4 6/18 hw/riscv: add e-trace message helpers
In reply to Daniel Henrique BarbozaBefore making the trace encoder writing into the RAM sink we need a way to encode the messages/packets. The encoding is LSB (least significant bit) based. The doc "Efficient Trace for RISC-V", Chapter 7, mentions:
"The remainder of this section describes the contents of the payload portion which should be independent of the infrastructure. In each table, the fields are listed in transmission order: first field in the table is transmitted first, and multi-bit fields are transmitted LSB first."
The "RISC-V Trace Control Interface Specification" docs, Chapter 7, states when talking about the Trace RAM Sink:
"Trace data is placed in memory in LSB order (first byte of trace packet/data is placed on LSB)."
This means that the LSB encoding must be used to write into the RAM Sink memory, which is our goal.
The design we're going for is to have all these encoder helpers, along with the message formats, in a separated file. The trace encoder will make use of these helpers to blindly write a byte array with the packet desired, and then write it as is in the RAM Sink.
We'll start by modeling the synchronisation packet first, adding more formats as we increment the Trace Encoder capabilities.
--- hw/riscv/meson.build | 3 +- hw/riscv/rv-trace-messages.c | 94 ++++++++++++++++++++++++++++++++++++ hw/riscv/rv-trace-messages.h | 25 ++++++++++ 3 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 hw/riscv/rv-trace-messages.c create mode 100644 hw/riscv/rv-trace-messages.h
hw/riscv/meson.build +2 -1
@@ -22,7 +22,8 @@ riscv_ss.add(when: 'CONFIG_XIANGSHAN_KUNMINGHU', if_true: files('xiangshan_kmh.c riscv_ss.add(when: 'CONFIG_RISCV_MIPS_CPS', if_true: files('cps.c')) riscv_ss.add(when: 'CONFIG_MIPS_BOSTON_AIA', if_true: files('boston-aia.c')) riscv_ss.add(when: 'CONFIG_K230', if_true: files('k230.c')) -riscv_ss.add(when: 'CONFIG_RISCV_TRACE', if_true: files('trace-encoder.c', 'trace-ram-sink.c')) +riscv_ss.add(when: 'CONFIG_RISCV_TRACE', if_true: files('trace-encoder.c', + 'trace-ram-sink.c', 'rv-trace-messages.c')) riscv_ss.add(when: 'CONFIG_SOPHGO_CV1800B', if_true: files('cv1800b.c')) riscv_ss.add(when: 'CONFIG_MILKV_DUO', if_true: files('milkv_duo.c'))
hw/riscv/rv-trace-messages.c new file +94 -0
94 lines not on this page · raw message
hw/riscv/rv-trace-messages.h new file +25 -0
@@ -0,0 +1,25 @@ +/* +* Helpers for RISC-V Trace Messages +* +* Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. +* +* SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#ifndef RISCV_RV_TRACE_MESSAGES_H +#define RISCV_RV_TRACE_MESSAGES_H + +typedef enum { +U =0 , +S_HS =1 , +RESERVED =2 , +M =3 , +D =4 , +VU =5 , +VS =6 , +}TracePrivLevel ; + +size_t rv_etrace_gen_encoded_sync_msg (uint8_t *buf,uint64_t pc, +TracePrivLevel priv_level); + +#endif
Signature · 1 line
2.43.0
PATCH v4 7/18 target/riscv: add initial trace instrumentation
In reply to Daniel Henrique BarbozaWe'll have to instrument some RISC-V translations, as well as the start of a translation block, communicating with the associated trace encoder in case we're running a trace session. The trace encoder needs to report the first insn of the session so we'll start with that.
Add a new EXT_TB_FLAGS called CPU_TRACE_RUNNING, along with its ctx->cpu_trace_running boolean, to track when we should call the hwtrace helpers. For now we care about just the first insn of the trace session.
--- hw/riscv/trace-encoder.c | 17 ++++++++++++++++ hw/riscv/trace-encoder.h | 4 ++++ hw/riscv/trace-events | 1 + target/riscv/cpu.h | 3 +++ target/riscv/helper.h | 4 ++++ target/riscv/tcg/meson.build | 6 ++++++ target/riscv/tcg/tcg-cpu.c | 4 ++++ target/riscv/tcg/trace_helper.c | 36 +++++++++++++++++++++++++++++++++ target/riscv/tcg/translate.c | 9 +++++++++ 9 files changed, 84 insertions(+) create mode 100644 target/riscv/tcg/trace_helper.c
hw/riscv/trace-encoder.c +17 -0
@@ -150,11 +150,15 @@ static void trencoder_te_ctrl_postw(RegisterInfo *reg, uint64_t val)uint32_t trTeEnable =ARRAY_FIELD_EX32 (te->regs,TR_TE_CONTROL ,ENABLE );uint32_t trTeInstTracing =ARRAY_FIELD_EX32 (te->regs,TR_TE_CONTROL ,INST_TRACING ); +RISCVCPU *cpu = te->cpu; +CPURISCVState *env = &cpu->env;if (!trTeActive) { te->enabled = false; te->trace_running = false; te->trace_next_insn = false; + + env->trace_running = false;return ; }
@@ -170,6 +174,7 @@ static void trencoder_te_ctrl_postw(RegisterInfo *reg, uint64_t val) } te->trace_running = trTeInstTracing ? true : false; + env->trace_running = te->trace_running; }static RegisterAccessInfo trencoder_regs_info[] = {
@@ -231,6 +236,8 @@ static const MemoryRegionOps trencoder_ops = {static void trencoder_reset (DeviceState *dev) {TraceEncoder *te =TRACE_ENCODER (dev); +RISCVCPU *cpu = te->cpu; +CPURISCVState *env = &cpu->env;for (int i =0 ; i <ARRAY_SIZE (te->regs_info); i++) {register_reset (&te->regs_info[i]);
@@ -239,6 +246,7 @@ static void trencoder_reset(DeviceState *dev) te->enabled = false; te->trace_running = false; te->trace_next_insn = false; + env->trace_running = false; }static void trencoder_realize (DeviceState *dev,Error **errp)
@@ -266,6 +274,14 @@ static void trencoder_realize(DeviceState *dev, Error **errp) } } +void trencoder_set_first_trace_insn (Object *trencoder_obj,uint64_t pc) +{ +TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj); + + trencoder->first_pc = pc; +trace_trencoder_first_trace_insn (pc); +} +static const Property trencoder_props[] = {/* * We need a link to the associated CPU to
@@ -294,6 +310,7 @@ static const VMStateDescription vmstate_trencoder = { VMSTATE_UINT64(ramsink_ramstart, TraceEncoder), VMSTATE_UINT64(ramsink_ramlimit, TraceEncoder), VMSTATE_INT32(cpu_id, TraceEncoder), + VMSTATE_UINT64(first_pc, TraceEncoder), VMSTATE_END_OF_LIST(), } };
hw/riscv/trace-encoder.h +4 -0
@@ -27,6 +27,8 @@ struct TraceEncoder {MemoryRegion reg_mem;uint32_t reg_mem_size; +uint64_t first_pc; +hwaddr baseaddr;hwaddr dest_baseaddr;hwaddr ramsink_ramstart;
@@ -43,4 +45,6 @@ struct TraceEncoder { OBJECT_DECLARE_SIMPLE_TYPE(TraceEncoder, TRACE_ENCODER) +void trencoder_set_first_trace_insn (Object *trencoder_obj,uint64_t pc); +#endif
hw/riscv/trace-events +1 -0
@@ -28,6 +28,7 @@ riscv_iommu_hpm_evt_write(uint32_t ctr_idx, uint32_t ovf, uint64_t val) "ctr_idx # trace-encoder.c trencoder_read_error(uint64_t addr) "addr 0x%" PRIx64 trencoder_write_error(uint64_t addr, uint64_t value) "addr 0x%" PRIx64 " value 0x%" PRIx64 +trencoder_first_trace_insn(uint64_t pc) "pc 0x%" PRIx64 # trace-ram-sink.c tr_ramsink_read_error(uint64_t addr) "addr 0x%" PRIx64
target/riscv/cpu.h +3 -0
@@ -551,6 +551,8 @@ struct CPUArchState {uint32_t kvm_mp_state;bool kvm_mp_state_loaded;#endif /* CONFIG_KVM */ + +bool trace_running; };/*
@@ -791,6 +793,7 @@ FIELD(EXT_TB_FLAGS, ALTFMT, 32, 1)FIELD (EXT_TB_FLAGS ,BIG_ENDIAN ,33 ,1 ) /* sdext single-step needs a TB flag to build 1-insn TBs */ FIELD(EXT_TB_FLAGS, SDEXT_STEP, 34, 1) +FIELD(EXT_TB_FLAGS, CPU_TRACE_RUNNING, 35, 1)#ifdef TARGET_RISCV32 #define riscv_cpu_mxl (env) ((void)(env), MXL_RV32)
target/riscv/helper.h +4 -0
@@ -129,6 +129,10 @@ DEF_HELPER_4(csrrw, tl, env, int, tl, tl) DEF_HELPER_2(csrr_i128, tl, env, int) DEF_HELPER_4(csrw_i128, void, env, int, tl, tl)DEF_HELPER_6 (csrrw_i128, tl, env, int, tl, tl, tl, tl) + +/* Trace helpers (should be put inside ifdef) */ +DEF_HELPER_2(trace_insn, void, env, i64) +#ifndef CONFIG_USER_ONLY DEF_HELPER_1(sret, tl, env)DEF_HELPER_1 (mret, tl, env)
target/riscv/tcg/meson.build +6 -0
@@ -10,6 +10,12 @@ riscv_ss.add(files( 'fpu_helper.c', 'm128_helper.c', 'op_helper.c', + + # In theory the RISC-V HW e-trace support is system_ss only, + # but we're instrumeting TCG code that is visible to riscv_ss + # ae well, ergo the helper needs to be visible in user-mode too. + 'trace_helper.c', + 'translate.c', 'tcg-cpu.c', 'vcrypto_helper.c',
target/riscv/tcg/tcg-cpu.c +4 -0
@@ -201,6 +201,10 @@ static TCGTBCPUState riscv_get_tb_cpu_state(CPUState *cs)if (cpu->cfg.ext_sdext && !env->debug_mode && (env->dcsr &DCSR_STEP )) { ext_flags =FIELD_DP64 (ext_flags,EXT_TB_FLAGS ,SDEXT_STEP ,1 ); } + +if (env->trace_running) { + ext_flags =FIELD_DP64 (ext_flags,EXT_TB_FLAGS ,CPU_TRACE_RUNNING ,1 ); + }#endif return (TCGTBCPUState ){
target/riscv/tcg/trace_helper.c new file +36 -0
@@ -0,0 +1,36 @@ +/* +* RISC-V Trace Support TCG helpers +* +* Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. +* +* SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "qapi/error.h" +#include "cpu.h" +#include "trace.h" +#include "exec/helper-proto.h" + +#ifndef CONFIG_USER_ONLY +#include "hw/riscv/trace-encoder.h" +#endif + +#ifndef CONFIG_USER_ONLY +void helper_trace_insn (CPURISCVState *env,uint64_t pc) +{ +RISCVCPU *cpu =env_archcpu (env); +TraceEncoder *te =TRACE_ENCODER (cpu->trencoder); + +if (te->trace_next_insn) { +trencoder_set_first_trace_insn (cpu->trencoder, pc); + te->trace_next_insn = false; + } +} +#else /* #ifndef CONFIG_USER_ONLY */ +void helper_trace_insn (CPURISCVState *env,uint64_t pc) +{ +return ; +} +#endif /* #ifndef CONFIG_USER_ONLY*/
target/riscv/tcg/translate.c +9 -0
@@ -125,6 +125,8 @@ typedef struct DisasContext {bool bcfi_enabled;/* Data endianness from MSTATUS UBE/SBE/MBE */ MemOp mo_endianness; +/* Detect if we're in the middle of a CPU trace session. */ +bool cpu_trace_running; } DisasContext;static inline bool has_ext (DisasContext *ctx,uint32_t ext)
@@ -1368,10 +1370,17 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) ctx->decoders = cpu->decoders; ctx->mo_endianness =FIELD_EX64 (ext_tb_flags,EXT_TB_FLAGS ,BIG_ENDIAN ) ?MO_BE :MO_LE ; + ctx->cpu_trace_running =FIELD_EX64 (ext_tb_flags,EXT_TB_FLAGS , +CPU_TRACE_RUNNING ); }static void riscv_tr_tb_start (DisasContextBase *db,CPUState *cpu) { +DisasContext *ctx =container_of (db, DisasContext, base); + +if (ctx->cpu_trace_running) { +gen_helper_trace_insn (tcg_env,tcg_constant_i64 (db->pc_first)); + } }static void riscv_tr_insn_start (DisasContextBase *dcbase,CPUState *cpu)
Signature · 1 line
2.43.0
PATCH v4 8/18 hw/riscv/trace-encoder: write e-trace packets to RAM sink
In reply to Daniel Henrique BarbozaWe have the needed pieces to make the trace encoder write messages (or packets, in e-trace lingo) in its associated RAM sink.
We'll start by writing a sync message that we're already able to fetch from the TCG instruction train. The trace encoder must be able to read/write TR_RAM_WP RAM sink regs to know where it is supposed to write packets, thus a handful of helpers are added to manipulate both the ram sink SMEM memory and relevant registers.
The bulk of the work is done in trencoder_send_message_smem() where the trace encoder writes each trace packet. The 'wrap' mechanism is implemented to keep writing the RAM sink memory even after a SMEM workflow. If we can't fit a full packet in the end of the ring buffer we're filling with NULLs and writing a new packet back to ramstart.
In an overflow event we'll wrap the TR_RAM_WP_LOW WRAP bit and userspace is supposed to clear it after the wrapping was handled (i.e. userspace read the whole ring buffer back to the start).
We're also allowing userspace to freely set ramsink and ramstart before the trace session begins, allowing for more freedom w.r.t where the SMEM will be written by the encoder. Note that this also means that userspace is responsible for setting at least TR_RAM_WP_LOW and TR_RAM_WP_HIGH before starting a trace session.
--- hw/riscv/trace-encoder.c | 122 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+)
hw/riscv/trace-encoder.c +122 -0
@@ -18,8 +18,37 @@#include "qapi/error.h" #include "trace.h" #include "system/device_tree.h" +#include "system/physmem.h" #include "hw/core/register.h" #include "cpu.h" +#include "hw/riscv/trace-ram-sink.h" +#include "rv-trace-messages.h" + +/* +* Size of header + payload since we're not sending +* srcID and timestamp. +*/ +#define TRACE_MSG_MAX_SIZE 32 + +static TracePrivLevel trencoder_get_curr_priv_level (TraceEncoder *te) +{ +CPURISCVState *env = &te->cpu->env; + +switch (env->priv) { +case PRV_U : +return env->virt_enabled ?VU :U ; +case PRV_S : +return env->virt_enabled ?VS :S_HS ; +case PRV_M : +return M ; + } + +/* +* Return a reserved value to signal an error. +* TODO: handle Debug (D). +*/ +return RESERVED ; +}/* * trTeControl register fields
@@ -82,6 +111,41 @@ REG32(TR_TE_IMPL, 0x4) REG32(TR_TE_INST_FEATURES, 0x8) FIELD(TR_TE_INST_FEATURES, NO_ADDR_DIFF, 0, 1) +static uint32_t trencoder_read_reg (TraceEncoder *te,uint32_t reg_addr) +{ +hwaddr addr = te->dest_baseaddr + reg_addr; +uint32_t val; + +physical_memory_read (addr, &val,sizeof (uint32_t )); +return val; +} + +static void trencoder_write_reg (TraceEncoder *te,uint32_t reg_addr, +uint32_t val) +{ +hwaddr addr = te->dest_baseaddr + reg_addr; + +physical_memory_write (addr, &val,sizeof (uint32_t )); +} + +static hwaddr trencoder_read_ramsink_writep (TraceEncoder *te) +{ +hwaddr ret =trencoder_read_reg (te,A_TR_RAM_WP_HIGH ); + ret <<=32 ; + ret +=trencoder_read_reg (te,A_TR_RAM_WP_LOW ); + +return ret; +} + +static hwaddr trencoder_read_ramsink_ramlimit (TraceEncoder *te) +{ +hwaddr ret =trencoder_read_reg (te,A_TR_RAM_LIMIT_HIGH ); + ret <<=32 ; + ret +=trencoder_read_reg (te,A_TR_RAM_LIMIT_LOW ); + +return ret; +} +static uint64_t trencoder_te_ctrl_set_hardwire_vals (uint64_t input) { input =FIELD_DP32 (input,TR_TE_CONTROL ,INST_MODE ,0x6 );
@@ -171,6 +235,9 @@ static void trencoder_te_ctrl_postw(RegisterInfo *reg, uint64_t val)if (!te->trace_running && trTeInstTracing) {/* Starting trace. Ask the CPU for the first trace insn */ te->trace_next_insn = true; + + te->ramsink_ramstart =trencoder_read_ramsink_writep (te); + te->ramsink_ramlimit =trencoder_read_ramsink_ramlimit (te); } te->trace_running = trTeInstTracing ? true : false;
@@ -274,12 +341,67 @@ static void trencoder_realize(DeviceState *dev, Error **errp) } } +static void trencoder_update_ramsink_writep (TraceEncoder *te, +hwaddr wp_val, +bool wrapped) +{ +uint32_t wp_low =trencoder_read_reg (te,A_TR_RAM_WP_LOW ); + + wp_low =FIELD_DP32 (wp_low,TR_RAM_WP_LOW ,ADDR , +extract64 (wp_val,2 ,30 )); + +if (wrapped) { + wp_low =FIELD_DP32 (wp_low,TR_RAM_WP_LOW ,WRAP ,1 ); + } + +trencoder_write_reg (te,A_TR_RAM_WP_LOW , wp_low); +trencoder_write_reg (te,A_TR_RAM_WP_HIGH ,extract64 (wp_val,32 ,32 )); +} + +static void trencoder_send_message_smem (TraceEncoder *trencoder, +uint8_t *msg,uint8_t msg_size) +{ +hwaddr dest =trencoder_read_ramsink_writep (trencoder); +bool wrapped = false; + + msg_size =QEMU_ALIGN_UP (msg_size,4 ); + +/* clear trRamWrap before writing to SMEM */ + dest =FIELD_DP64 (dest,TR_RAM_WP_LOW ,WRAP ,0 ); + +/* +* Fill with null bytes if we can't fit the packet in +* ramlimit, set wrap and write the packet in ramstart. +*/ +if (dest + msg_size > trencoder->ramsink_ramlimit) { + g_autofree uint8_t *null_packet = NULL; +uint8_t null_size = trencoder->ramsink_ramlimit - dest; + + null_packet =g_malloc0 (null_size); +physical_memory_write (dest, null_packet, null_size); + + dest = trencoder->ramsink_ramstart; + wrapped = true; + } + +physical_memory_write (dest, msg, msg_size); + dest += msg_size; + +trencoder_update_ramsink_writep (trencoder, dest, wrapped); +} +void trencoder_set_first_trace_insn (Object *trencoder_obj,uint64_t pc) {TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj); +TracePrivLevel priv =trencoder_get_curr_priv_level (trencoder); + g_autofree uint8_t *msg = g_malloc0(TRACE_MSG_MAX_SIZE); +uint8_t msg_size; trencoder->first_pc = pc;trace_trencoder_first_trace_insn (pc); + msg_size =rv_etrace_gen_encoded_sync_msg (msg, pc, priv); + +trencoder_send_message_smem (trencoder, msg, msg_size); }static const Property trencoder_props[] = {
Signature · 1 line
2.43.0
PATCH v4 9/18 test/qtest: add riscv-trace-test.c
In reply to Daniel Henrique BarbozaAdd a simple smoke test for the trace encoder/trace ram sink integration with the RISC-V 'virt' machine.
--- hw/riscv/virt.c | 2 +- tests/qtest/meson.build | 2 +- tests/qtest/riscv-trace-test.c | 120 +++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 tests/qtest/riscv-trace-test.c
hw/riscv/virt.c +1 -1
@@ -889,7 +889,7 @@ static void virt_machine_init(MachineState *machine) hart_count, &error_abort);sysbus_realize (SYS_BUS_DEVICE (&s->soc[i]), &error_fatal); -if (tcg_enabled ()) { +if (tcg_enabled () ||qtest_enabled ()) {virt_init_socket_trace_hw (s, i); }
tests/qtest/meson.build +1 -1
@@ -311,7 +311,7 @@ qtests_s390x = \ qtests_riscv32 = \ (config_all_devices.has_key('CONFIG_SIFIVE_E_AON') ? ['sifive-e-aon-watchdog-test'] : []) -qtests_riscv64 = ['riscv-csr-test'] + \ +qtests_riscv64 = ['riscv-csr-test', 'riscv-trace-test'] + \ (unpack_edk2_blobs ? ['bios-tables-test'] : []) + \ (config_all_devices.has_key('CONFIG_IOMMU_TESTDEV') and config_all_devices.has_key('CONFIG_RISCV_IOMMU') ?
tests/qtest/riscv-trace-test.c new file +120 -0
120 lines not on this page · raw message
Signature · 1 line
2.43.0
PATCH v4 10/18 hw/riscv/rv-trace-messages.c: add encoded trap message
In reply to Daniel Henrique BarbozaThe trap message consists of a Format 3 Subformat 1 e-trace message.
According to the Efficient Trace for RISC-V spec, section "Format 3 thaddr, address and privilege fields", the 'thaddr' will be zero if:
- we can't infer the EPC after an uninferable PC discontinuity (like traps). This doesn't happen in our current TCG backend - we'll always know the trap EPC in riscv_cpu_do_interrupt();
- a second interrupt/exception happens while the handler of the first trap hasn't exited. This also doesn't happen in TCG given that we do not emulate a multi-insn pipeline model, i.e. we'll only retire one insns at a time.
This means that we'll always send trap packets with 'thaddr' set to 1, thus we're hardcoding it in the message helper.
--- hw/riscv/rv-trace-messages.c | 94 ++++++++++++++++++++++++++++++++++++ hw/riscv/rv-trace-messages.h | 5 ++ 2 files changed, 99 insertions(+)
hw/riscv/rv-trace-messages.c +94 -0
@@ -35,6 +35,24 @@ typedef struct RVTraceSyncPayload { } RVTraceSyncPayload;#define SYNC_PAYLOAD_SIZE_64BITS 9 +/* +* Format 3 subformat 1 without 'time' and 'context' fields +*/ +typedef struct RVTraceTrapPayload { +uint8_t format:2 ; +uint8_t subformat:2 ; +uint8_t branch:1 ; +uint8_t privilege:3 ; +uint8_t ecause:6 ; +uint8_t interrupt:1 ; +uint8_t thaddr:1 ; +uint32_t addressLow; +uint32_t addressHigh; +uint32_t tvalLow; +uint32_t tvalHigh; +}RVTraceTrapPayload ; +#define TRAP_PAYLOAD_SIZE_64BITS 18 +static void rv_etrace_write_bits (uint8_t *bytes,uint32_t bit_pos,uint32_t num_bits,uint32_t val) {
@@ -92,3 +110,79 @@ size_t rv_etrace_gen_encoded_sync_msg(uint8_t *buf, uint64_t pc,return HEADER_SIZE +SYNC_PAYLOAD_SIZE_64BITS ; } + +/* +* Note: this function assumes thaddr = 1. +*/ +size_t rv_etrace_gen_encoded_trap_msg (uint8_t *buf,uint64_t trap_addr, +TracePrivLevel priv_level, +uint8_t ecause, +bool is_interrupt, +uint64_t tval) +{ +RVTraceTrapPayload payload = {.format =0b11 , + .subformat =0b01 , + .branch =1 , + .privilege = priv_level, + .ecause = ecause}; +RVTraceMessageHeader header = {.flow =0 , .extend =0 , + .length =TRAP_PAYLOAD_SIZE_64BITS }; +uint8_t bit_pos; + + payload.addressLow =extract64 (trap_addr,0 ,32 ); + payload.addressHigh =extract64 (trap_addr,32 ,32 ); + +/* +* When interrupt = 1 'tval' is ommited. Take 8 bytes +* from the final size. +*/ +if (is_interrupt) { + header.length =TRAP_PAYLOAD_SIZE_64BITS -8 ; + } + +rv_etrace_write_header (buf, header); + bit_pos =8 ; + +rv_etrace_write_bits (buf, bit_pos,2 , payload.format); + bit_pos +=2 ; +rv_etrace_write_bits (buf, bit_pos,2 , payload.subformat); + bit_pos +=2 ; +rv_etrace_write_bits (buf, bit_pos,1 , payload.branch); + bit_pos +=1 ; +rv_etrace_write_bits (buf, bit_pos,3 , payload.privilege); + bit_pos +=3 ; + +rv_etrace_write_bits (buf, bit_pos,6 , payload.ecause); + bit_pos +=6 ; + +if (is_interrupt) { +rv_etrace_write_bits (buf, bit_pos,1 ,1 ); + }else { +rv_etrace_write_bits (buf, bit_pos,1 ,0 ); + } + bit_pos +=1 ; + +/* thaddr is hardcoded to 1 for now */ +rv_etrace_write_bits (buf, bit_pos,1 ,1 ); + bit_pos +=1 ; + +rv_etrace_write_bits (buf, bit_pos,32 , payload.addressLow); + bit_pos +=32 ; +rv_etrace_write_bits (buf, bit_pos,32 , payload.addressHigh); + bit_pos +=32 ; + +/* Skip trap_addr if is_interrupt */ +if (is_interrupt) { + goto out; + } + + payload.tvalLow =extract64 (trap_addr,0 ,32 ); + payload.tvalHigh =extract64 (trap_addr,32 ,32 ); + +rv_etrace_write_bits (buf, bit_pos,32 , payload.tvalLow); + bit_pos +=32 ; +rv_etrace_write_bits (buf, bit_pos,32 , payload.tvalHigh); + +out: +return HEADER_SIZE + header.length; +}
hw/riscv/rv-trace-messages.h +5 -0
@@ -21,5 +21,10 @@ typedef enum {size_t rv_etrace_gen_encoded_sync_msg (uint8_t *buf,uint64_t pc,TracePrivLevel priv_level); +size_t rv_etrace_gen_encoded_trap_msg (uint8_t *buf,uint64_t trap_addr, +TracePrivLevel priv_level, +uint8_t ecause, +bool is_interrupt, +uint64_t tval);#endif
Signature · 1 line
2.43.0
Re: [PATCH v4 10/18] hw/riscv/rv-trace-messages.c: add encoded trap message
Review
Quoted 3 lines
(quoted diff) +2
-0+ payload.tvalLow = extract64(trap_addr, 0, 32); + payload.tvalHigh = extract64(trap_addr, 32, 32)
For synchronous exceptions, these fields should contain the trap value (tval), but both are currently extracted from trap_addr
Thanks
Jay Chang
PATCH v4 11/18 hw/riscv, target/riscv: send trace trap messages
In reply to Daniel Henrique BarbozaUse the rv_etrace_gen_encoded_trap_msg() helper we added in the previous patch to encode trap packets to be written in the RAM sink SMEM.
--- hw/riscv/trace-encoder.c | 17 +++++++++++++++++ hw/riscv/trace-encoder.h | 4 ++++ target/riscv/tcg/cpu_helper.c | 13 +++++++++++++ 3 files changed, 34 insertions(+)
hw/riscv/trace-encoder.c +17 -0
@@ -404,6 +404,23 @@ void trencoder_set_first_trace_insn(Object *trencoder_obj, uint64_t pc)trencoder_send_message_smem (trencoder, msg, msg_size); } +void trencoder_trace_trap_insn (Object *trencoder_obj, +uint64_t pc,uint32_t ecause, +bool is_interrupt, +uint64_t tval) +{ +TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj); +TracePrivLevel priv =trencoder_get_curr_priv_level (trencoder); + g_autofree uint8_t *msg = g_malloc0(TRACE_MSG_MAX_SIZE); +uint8_t msg_size; + + msg_size =rv_etrace_gen_encoded_trap_msg (msg, pc, priv, + ecause, is_interrupt, + tval); + +trencoder_send_message_smem (trencoder, msg, msg_size); +} +static const Property trencoder_props[] = {/* * We need a link to the associated CPU to
hw/riscv/trace-encoder.h +4 -0
@@ -46,5 +46,9 @@ struct TraceEncoder { OBJECT_DECLARE_SIMPLE_TYPE(TraceEncoder, TRACE_ENCODER)void trencoder_set_first_trace_insn (Object *trencoder_obj,uint64_t pc); +void trencoder_trace_trap_insn (Object *trencoder_obj, +uint64_t pc,uint32_t ecause, +bool is_interrupt, +uint64_t tval);#endif
target/riscv/tcg/cpu_helper.c +13 -0
@@ -40,6 +40,10 @@#include "pmp.h" #include "qemu/plugin.h" +#ifndef CONFIG_USER_ONLY +#include "hw/riscv/trace-encoder.h" +#endif +int riscv_env_mmu_index (CPURISCVState *env,bool ifetch) {#ifdef CONFIG_USER_ONLY
@@ -2300,6 +2304,15 @@ void riscv_cpu_do_interrupt(CPUState *cs) __func__, env->mhartid, async, cause, env->pc, tval, riscv_cpu_get_trap_name(cause, async)); +if (cpu->trencoder) { +TraceEncoder *te =TRACE_ENCODER (cpu->trencoder); + +if (te->trace_running) { +trencoder_trace_trap_insn (cpu->trencoder, env->pc, + cause, async, tval); + } + } + mode = env->priv <=PRV_S && cause <64 && (((deleg >> cause) &1 ) || s_injected || vs_injected) ?PRV_S :PRV_M ;
Signature · 1 line
2.43.0
PATCH v4 12/18 target/riscv, hw/riscv: send trace ppccd packets
In reply to Daniel Henrique BarbozaThe ppccd (privilege change) trace packets consists of sync packets like we already sent during trace start and resyncs.
The privilege change is being fetched via riscv_cpu_set_mode().
--- hw/riscv/trace-encoder.c | 18 ++++++++++++++++++ hw/riscv/trace-encoder.h | 1 + target/riscv/tcg/cpu_helper.c | 13 ++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-)
hw/riscv/trace-encoder.c +18 -0
@@ -390,6 +390,19 @@ static void trencoder_send_message_smem(TraceEncoder *trencoder,trencoder_update_ramsink_writep (trencoder, dest, wrapped); } +static void trencoder_send_sync_msg (Object *trencoder_obj,uint64_t pc) +{ +TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj); +TracePrivLevel priv =trencoder_get_curr_priv_level (trencoder); + g_autofree uint8_t *msg = g_malloc0(TRACE_MSG_MAX_SIZE); +uint8_t msg_size; + + trencoder->first_pc = pc; + msg_size =rv_etrace_gen_encoded_sync_msg (msg, pc, priv); + +trencoder_send_message_smem (trencoder, msg, msg_size); +} +void trencoder_set_first_trace_insn (Object *trencoder_obj,uint64_t pc) {TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj);
@@ -421,6 +434,11 @@ void trencoder_trace_trap_insn(Object *trencoder_obj,trencoder_send_message_smem (trencoder, msg, msg_size); } +void trencoder_trace_ppccd (Object *trencoder_obj,uint64_t pc) +{ +trencoder_send_sync_msg (trencoder_obj, pc); +} +static const Property trencoder_props[] = {/* * We need a link to the associated CPU to
hw/riscv/trace-encoder.h +1 -0
@@ -50,5 +50,6 @@ void trencoder_trace_trap_insn(Object *trencoder_obj, uint64_t pc, uint32_t ecause, bool is_interrupt, uint64_t tval); +void trencoder_trace_ppccd (Object *trencoder_obj,uint64_t pc);#endif
target/riscv/tcg/cpu_helper.c +12 -1
@@ -943,11 +943,22 @@ void riscv_cpu_set_mode(CPURISCVState *env, privilege_mode_t newpriv, {g_assert (newpriv <=PRV_M && newpriv !=PRV_RESERVED ); +CPUState *cs =env_cpu (env); +if (newpriv != env->priv || env->virt_enabled != virt_en) {if (icount_enabled ()) {riscv_itrigger_update_priv (env); } +if (newpriv != env->priv) { +RISCVCPU *cpu =RISCV_CPU (cs); + +if (cpu->trencoder && +TRACE_ENCODER (cpu->trencoder)->trace_running) { +trencoder_trace_ppccd (cpu->trencoder, env->pc); + } + } +riscv_pmu_update_fixed_ctrs (env, newpriv, virt_en); }
@@ -968,7 +979,7 @@ void riscv_cpu_set_mode(CPURISCVState *env, privilege_mode_t newpriv,if (riscv_has_ext (env,RVH )) {/* Flush the TLB on all virt mode changes. */ if (env->virt_enabled != virt_en) { -tlb_flush (env_cpu (env)); +tlb_flush (cs); } env->virt_enabled = virt_en;
Signature · 1 line
2.43.0
Re: [PATCH v4 12/18] target/riscv, hw/riscv: send trace ppccd packets
Quoted 14 lines
--
a/target/riscv/tcg/cpu_helper.c
+++b/target/riscv/tcg/cpu_helper.ctarget/riscv/tcg/cpu_helper.c +2
-0@@ -943,11 +943,22 @@ void riscv_cpu_set_mode(CPURISCVState *env, privilege_mode_t newpriv, {g_assert (newpriv <=PRV_M && newpriv !=PRV_RESERVED ); +CPUState *cs =env_cpu (env); +if (newpriv != env->priv || env->virt_enabled != virt_en) {if (icount_enabled ()) {riscv_itrigger_update_priv (env); }
target/riscv/tcg/cpu_helper.c void riscv_cpu_set_mode(CPURISCVState *env, · line 962
Quoted 10 lines
target/riscv/tcg/cpu_helper.c +8
-0+if (newpriv != env->priv) { +RISCVCPU *cpu =RISCV_CPU (cs); + +if (cpu->trencoder && +TRACE_ENCODER (cpu->trencoder)->trace_running) { +trencoder_trace_ppccd (cpu->trencoder, env->pc); + } + }
On the trap-entry path, env->pc has already been updated to the handler address, but env->priv still holds the previous privilege level. Emitting a PPCCD sync packet here therefore pairs the handler PC with the old privilege level.
PATCH v4 13/18 hw/riscv/trace: add format2 msg helper
In reply to Daniel Henrique BarbozaBefore handling updiscon (uninferable PC discontinuity) cases add the encoded message we're going to use, format 2. Format 1 will be added when we add the branch map.
There are design decisions worth documenting in this patch. For irreport, the e-trace-encap spec [1] in section 7.6. "Format 2 packets", says that we should make this bit !updiscon if we're either:
"following a return because its address differs from the predicted return address at the top of the implicit_return return address stack, or the last retired before an exception, interrupt, privilege change or resync because it is necessary to report the current address stack depth or nested call count."
We do not implement any form of call/return prediction in the encoder, and TCG will always retire a single insn per cycle. This means that we'll never set irreport, i.e. irreport will always be equal to updiscon. If this interpretation turns out to be wrong we'll revisit this helper.
Likewise, irdepth is also not implemented since we'll always return a package where irreport == updiscon. The size of the field is arbitrarily set to '3' to avoid adding padding in the end of the msg to complete an extra byte.
[1] https://github.com/riscv-non-isa/e-trace-encap/releases/tag/v1.0
--- hw/riscv/rv-trace-messages.c | 59 ++++++++++++++++++++++++++++++++++++ hw/riscv/rv-trace-messages.h | 2 ++ 2 files changed, 61 insertions(+)
hw/riscv/rv-trace-messages.c +59 -0
@@ -53,6 +53,17 @@ typedef struct RVTraceTrapPayload { } RVTraceTrapPayload;#define TRAP_PAYLOAD_SIZE_64BITS 18 +typedef struct RVTraceFormat2Payload { +uint8_t format:2 ; +uint32_t addressLow; +uint32_t addressHigh; +uint8_t notify:1 ; +uint8_t updiscon:1 ; +uint8_t irreport:1 ; +uint8_t irdepth:3 ; +}RVTraceFormat2Payload ; +#define FORMAT2_PAYLOAD_SIZE_64BITS 9 +static void rv_etrace_write_bits (uint8_t *bytes,uint32_t bit_pos,uint32_t num_bits,uint32_t val) {
@@ -186,3 +197,51 @@ size_t rv_etrace_gen_encoded_trap_msg(uint8_t *buf, uint64_t trap_addr, out:return HEADER_SIZE + header.length; } + +/* +* Note: irreport and irdepth is always == updiscon. +* +* return_stack_size_p + call_counter_size_p is hardcoded +* to 3 since we don't implement neither ATM. +*/ +size_t rv_etrace_gen_encoded_format2_msg (uint8_t *buf,uint64_t addr, +bool notify,bool updiscon) +{ +RVTraceFormat2Payload payload = {.format =0b11 , + .notify = notify, + .updiscon = updiscon}; +RVTraceMessageHeader header = {.flow =0 , .extend =0 , + .length =FORMAT2_PAYLOAD_SIZE_64BITS }; +uint8_t bit_pos; + + payload.addressLow =extract64 (addr,0 ,32 ); + payload.addressHigh =extract64 (addr,32 ,32 ); + + payload.irreport = updiscon; +if (updiscon) { + payload.irdepth =0b111 ; + }else { + payload.irdepth =0 ; + } + +rv_etrace_write_header (buf, header); + bit_pos =8 ; + +rv_etrace_write_bits (buf, bit_pos,2 , payload.format); + bit_pos +=2 ; + +rv_etrace_write_bits (buf, bit_pos,32 , payload.addressLow); + bit_pos +=32 ; +rv_etrace_write_bits (buf, bit_pos,32 , payload.addressHigh); + bit_pos +=32 ; + +rv_etrace_write_bits (buf, bit_pos,1 , payload.notify); + bit_pos +=1 ; +rv_etrace_write_bits (buf, bit_pos,1 , payload.updiscon); + bit_pos +=1 ; +rv_etrace_write_bits (buf, bit_pos,1 , payload.irreport); + bit_pos +=1 ; +rv_etrace_write_bits (buf, bit_pos,3 , payload.irdepth); + +return HEADER_SIZE + header.length; +}
hw/riscv/rv-trace-messages.h +2 -0
@@ -26,5 +26,7 @@ size_t rv_etrace_gen_encoded_trap_msg(uint8_t *buf, uint64_t trap_addr,uint8_t ecause, bool is_interrupt, uint64_t tval); +size_t rv_etrace_gen_encoded_format2_msg (uint8_t *buf,uint64_t addr, +bool notify,bool updiscon);#endif
Signature · 1 line
2.43.0
Re: [PATCH v4 13/18] hw/riscv/trace: add format2 msg helper
hw/riscv/rv-trace-messages.c size_t rv_etrace_gen_encoded_trap_msg(uint8_t *buf, uint64_t trap_addr, · line 214
Quoted 9 lines
hw/riscv/rv-trace-messages.c +8
-0+size_t rv_etrace_gen_encoded_format2_msg (uint8_t *buf,uint64_t addr, +bool notify,bool updiscon) +{ +RVTraceFormat2Payload payload = {.format =0b11 , + .notify = notify, + .updiscon = updiscon}; +RVTraceMessageHeader header = {.flow =0 , .extend =0 , + .length =FORMAT2_PAYLOAD_SIZE_64BITS };
Shouldn't the format field be 0b10 for Format 2
PATCH v4 14/18 hw/riscv, target/riscv: send resync updiscon trace packets
In reply to Daniel Henrique BarbozaSend updiscon packets based on the constraints already discussed in the previous patch:
- We do not implement any form of call/return prediction in the encoder, and TCG will always retire a single insn per cycle, e.g. irreport will always be equal to updiscon;
- irdepth is not implemented since we'll always return a package where irreport == updiscon.
Note that we're sending an updiscon packet if the 'updiscon_pending' flag is set when we're about the send a resync or a trap packet. The TCG helper in this case is just setting the trace encoder flags instead of actually triggering a RAM sink SMEM write.
--- hw/riscv/trace-encoder.c | 37 +++++++++++++++++++ hw/riscv/trace-encoder.h | 3 ++ target/riscv/helper.h | 1 + .../tcg/insn_trans/trans_privileged.c.inc | 11 ++++++ target/riscv/tcg/insn_trans/trans_rvi.c.inc | 2 + target/riscv/tcg/trace_helper.c | 14 +++++++ target/riscv/tcg/translate.c | 7 ++++ 7 files changed, 75 insertions(+)
hw/riscv/trace-encoder.c +37 -0
@@ -403,6 +403,22 @@ static void trencoder_send_sync_msg(Object *trencoder_obj, uint64_t pc)trencoder_send_message_smem (trencoder, msg, msg_size); } +static void trencoder_send_updiscon (TraceEncoder *trencoder,uint64_t pc) +{ + g_autofree uint8_t *format2_msg = g_malloc0(TRACE_MSG_MAX_SIZE); +uint8_t addr_msb =extract64 (pc,31 ,1 ); +bool notify = addr_msb; +bool updiscon = !notify; +uint8_t msg_size; + + msg_size =rv_etrace_gen_encoded_format2_msg (format2_msg, pc, + notify, + updiscon); +trencoder_send_message_smem (trencoder, format2_msg, msg_size); + + trencoder->updiscon_pending = false; +} +void trencoder_set_first_trace_insn (Object *trencoder_obj,uint64_t pc) {TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj);
@@ -410,6 +426,10 @@ void trencoder_set_first_trace_insn(Object *trencoder_obj, uint64_t pc) g_autofree uint8_t *msg = g_malloc0(TRACE_MSG_MAX_SIZE);uint8_t msg_size; +if (trencoder->updiscon_pending) { +trencoder_send_updiscon (trencoder, pc); + } + trencoder->first_pc = pc;trace_trencoder_first_trace_insn (pc); msg_size =rv_etrace_gen_encoded_sync_msg (msg, pc, priv);
@@ -427,6 +447,10 @@ void trencoder_trace_trap_insn(Object *trencoder_obj, g_autofree uint8_t *msg = g_malloc0(TRACE_MSG_MAX_SIZE);uint8_t msg_size; +if (trencoder->updiscon_pending) { +trencoder_send_updiscon (trencoder, pc); + } + msg_size =rv_etrace_gen_encoded_trap_msg (msg, pc, priv, ecause, is_interrupt, tval);
@@ -436,9 +460,22 @@ void trencoder_trace_trap_insn(Object *trencoder_obj,void trencoder_trace_ppccd (Object *trencoder_obj,uint64_t pc) { +TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj); + +if (trencoder->updiscon_pending) { +trencoder_send_updiscon (trencoder, pc); + } +trencoder_send_sync_msg (trencoder_obj, pc); } +void trencoder_report_updiscon (Object *trencoder_obj) +{ +TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj); + + trencoder->updiscon_pending = true; +} +static const Property trencoder_props[] = {/* * We need a link to the associated CPU to
hw/riscv/trace-encoder.h +3 -0
@@ -36,6 +36,8 @@ struct TraceEncoder {uint32_t regs[TRACE_R_MAX ];RegisterInfo regs_info[TRACE_R_MAX ]; +bool updiscon_pending; +bool enabled;bool trace_running;bool trace_next_insn;
@@ -51,5 +53,6 @@ void trencoder_trace_trap_insn(Object *trencoder_obj,bool is_interrupt, uint64_t tval);void trencoder_trace_ppccd (Object *trencoder_obj,uint64_t pc); +void trencoder_report_updiscon (Object *trencoder_obj);#endif
target/riscv/helper.h +1 -0
@@ -132,6 +132,7 @@ DEF_HELPER_6(csrrw_i128, tl, env, int, tl, tl, tl, tl)/* Trace helpers (should be put inside ifdef) */ DEF_HELPER_2(trace_insn, void, env, i64) +DEF_HELPER_1(trace_updiscon, void, env)#ifndef CONFIG_USER_ONLY DEF_HELPER_1 (sret, tl, env)
target/riscv/tcg/insn_trans/trans_privileged.c.inc +11 -0
@@ -26,6 +26,8 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a) { + gen_trace_updiscon(ctx); + /* always generates U-level ECALL, fixed in do_interrupt handler */ generate_exception(ctx, RISCV_EXCP_U_ECALL); return true;
@@ -40,6 +42,8 @@ static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a) uint32_t ebreak = 0; uint32_t post = 0; + gen_trace_updiscon(ctx); + /* * The RISC-V semihosting spec specifies the following * three-instruction sequence to flag a semihosting call:
@@ -101,6 +105,8 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a) { #ifndef CONFIG_USER_ONLY if (has_ext(ctx, RVS)) { + gen_trace_updiscon(ctx); + decode_save_opc(ctx, 0); translator_io_start(&ctx->base); gen_update_pc(ctx, 0);
@@ -119,6 +125,8 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a) static bool trans_mret(DisasContext *ctx, arg_mret *a) { #ifndef CONFIG_USER_ONLY + gen_trace_updiscon(ctx); + decode_save_opc(ctx, 0); translator_io_start(&ctx->base); gen_update_pc(ctx, 0);
@@ -153,6 +161,9 @@ static bool trans_mnret(DisasContext *ctx, arg_mnret *a) { #ifndef CONFIG_USER_ONLY REQUIRE_SMRNMI(ctx); + + gen_trace_updiscon(ctx); + decode_save_opc(ctx, 0); gen_helper_mnret(cpu_pc, tcg_env); tcg_gen_exit_tb(NULL, 0); /* no chaining */
target/riscv/tcg/insn_trans/trans_rvi.c.inc +2 -0
@@ -183,6 +183,8 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a) } } + gen_trace_updiscon(ctx); + lookup_and_goto_ptr(ctx); if (misaligned) {
target/riscv/tcg/trace_helper.c +14 -0
@@ -28,9 +28,23 @@ void helper_trace_insn(CPURISCVState *env, uint64_t pc) te->trace_next_insn = false; } } + +void helper_trace_updiscon (CPURISCVState *env) +{ +RISCVCPU *cpu =env_archcpu (env); +TraceEncoder *te =TRACE_ENCODER (cpu->trencoder); + + te->updiscon_pending = true; + te->trace_next_insn = true; +}#else /* #ifndef CONFIG_USER_ONLY */ void helper_trace_insn (CPURISCVState *env,uint64_t pc) {return ; } + +void helper_trace_updiscon (CPURISCVState *env) +{ +return ; +}#endif /* #ifndef CONFIG_USER_ONLY*/
target/riscv/tcg/translate.c +7 -0
@@ -627,6 +627,13 @@ static void gen_ctr_jal(DisasContext *ctx, int rd, target_ulong imm) }#endif +static void gen_trace_updiscon (DisasContext *ctx) +{ +if (ctx->cpu_trace_running) { +gen_helper_trace_updiscon (tcg_env); + } +} +static void gen_jal (DisasContext *ctx,int rd,target_ulong imm) {TCGv succ_pc =dest_gpr (ctx, rd);
Signature · 1 line
2.43.0
Re: [PATCH v4 14/18] hw/riscv, target/riscv: send resync updiscon trace packets
target/riscv/tcg/trace_helper.c void helper_trace_insn(CPURISCVState *env, uint64_t pc) · line 38
Quoted 9 lines
target/riscv/tcg/trace_helper.c +7
-0+void helper_trace_updiscon (CPURISCVState *env) +{ +RISCVCPU *cpu =env_archcpu (env); +TraceEncoder *te =TRACE_ENCODER (cpu->trencoder); + + te->updiscon_pending = true; + te->trace_next_insn = true;
Could we use trencoder_report_updiscon() here instead of setting updiscon_pending and trace_next_insn directly?
Thanks
Jay Chang
PATCH v4 15/18 hw/riscv/rv-trace-messages: add format 1 msgs with branch info
In reply to Daniel Henrique BarbozaAdd encoded message helpers to send branch information trace packets.
--- hw/riscv/rv-trace-messages.c | 126 +++++++++++++++++++++++++++++++++++ hw/riscv/rv-trace-messages.h | 7 ++ 2 files changed, 133 insertions(+)
hw/riscv/rv-trace-messages.c +126 -0
@@ -64,6 +64,31 @@ typedef struct RVTraceFormat2Payload { } RVTraceFormat2Payload;#define FORMAT2_PAYLOAD_SIZE_64BITS 9 +typedef struct RVTraceFormat1BasePayload { +uint8_t format:2 ; +uint8_t branches:5 ; +uint32_t branch_map:31 ; +}RVTraceFormat1BasePayload ; +#define FORMAT1_BASE_PAYLOAD_SIZE_64BITS 5 + +typedef struct RVTraceFormat1Payload { +uint8_t format:2 ; +uint8_t branches:5 ; +uint32_t branch_map; +uint32_t addressLow; +uint32_t addressHigh; +uint8_t notify:1 ; +uint8_t updiscon:1 ; +uint8_t irreport:1 ; +uint8_t irdepth:3 ; +}RVTraceFormat1Payload ; + +/* +* FORMAT2_PAYLOAD_SIZE_64BITS = 9 plus 5 bits of 'branches', +* plus minimal 3 bits of 'branch_map' = 10 bytes. +*/ +#define FORMAT1_PAYLOAD_MIN_SIZE_64BITS 10 +static void rv_etrace_write_bits (uint8_t *bytes,uint32_t bit_pos,uint32_t num_bits,uint32_t val) {
@@ -245,3 +270,104 @@ size_t rv_etrace_gen_encoded_format2_msg(uint8_t *buf, uint64_t addr,return HEADER_SIZE + header.length; } + +size_t rv_etrace_gen_encoded_format1_noaddr (uint8_t *buf, +uint8_t branches, +uint32_t branch_map) +{ +RVTraceMessageHeader header = {.flow =0 , .extend =0 , + .length =FORMAT1_BASE_PAYLOAD_SIZE_64BITS }; +RVTraceFormat1BasePayload payload = {.format =0b01 , + .branches = branches, .branch_map = branch_map}; +uint8_t bit_pos; + +rv_etrace_write_header (buf, header); + bit_pos =8 ; + +rv_etrace_write_bits (buf, bit_pos,2 , payload.format); + bit_pos +=2 ; + +rv_etrace_write_bits (buf, bit_pos,5 , payload.branches); + bit_pos +=5 ; + +rv_etrace_write_bits (buf, bit_pos,31 , payload.branch_map); + +return HEADER_SIZE + header.length; +} + +/* +* Same reservations made in the format 2 helper: +* +* - irreport and irdepth is always == updiscon; +* +* - return_stack_size_p + call_counter_size_p is hardcoded +* to 3 since we don't implement neither ATM. +*/ +size_t rv_etrace_gen_encoded_format1 (uint8_t *buf, +uint8_t branches,uint32_t branch_map, +uint64_t addr, +bool notify,bool updiscon) +{ +RVTraceMessageHeader header = {.flow =0 , .extend =0 }; +RVTraceFormat1Payload payload = {.format =0b01 , + .branches = branches, + .notify = notify, + .updiscon = updiscon}; +uint8_t payload_size =FORMAT1_PAYLOAD_MIN_SIZE_64BITS ; +uint8_t branch_map_size =0 ; +uint8_t bit_pos; + +g_assert (branches <32 ); + +if (branches <=3 ) { + branch_map_size =3 ; + }else if (branches <=7 ) { + branch_map_size =7 ; + payload_size++; + }else if (branches <=15 ) { + branch_map_size =15 ; + payload_size +=2 ; + }else { + branch_map_size =31 ; + payload_size +=4 ; + } + + header.length = payload_size; + +rv_etrace_write_header (buf, header); + bit_pos =8 ; + + payload.addressLow =extract64 (addr,0 ,32 ); + payload.addressHigh =extract64 (addr,32 ,32 ); + + payload.irreport = updiscon; +if (updiscon) { + payload.irdepth =0b111 ; + }else { + payload.irdepth =0 ; + } + +rv_etrace_write_bits (buf, bit_pos,2 , payload.format); + bit_pos +=2 ; + +rv_etrace_write_bits (buf, bit_pos,5 , payload.branches); + bit_pos +=5 ; + +rv_etrace_write_bits (buf, bit_pos, branch_map_size, payload.branch_map); + bit_pos += branch_map_size; + +rv_etrace_write_bits (buf, bit_pos,32 , payload.addressLow); + bit_pos +=32 ; +rv_etrace_write_bits (buf, bit_pos,32 , payload.addressHigh); + bit_pos +=32 ; + +rv_etrace_write_bits (buf, bit_pos,1 , payload.notify); + bit_pos +=1 ; +rv_etrace_write_bits (buf, bit_pos,1 , payload.updiscon); + bit_pos +=1 ; +rv_etrace_write_bits (buf, bit_pos,1 , payload.irreport); + bit_pos +=1 ; +rv_etrace_write_bits (buf, bit_pos,3 , payload.irdepth); + +return HEADER_SIZE + header.length; +}
hw/riscv/rv-trace-messages.h +7 -0
@@ -28,5 +28,12 @@ size_t rv_etrace_gen_encoded_trap_msg(uint8_t *buf, uint64_t trap_addr, uint64_t tval);size_t rv_etrace_gen_encoded_format2_msg (uint8_t *buf,uint64_t addr,bool notify,bool updiscon); +size_t rv_etrace_gen_encoded_format1_noaddr (uint8_t *buf, +uint8_t branches, +uint32_t branch_map); +size_t rv_etrace_gen_encoded_format1 (uint8_t *buf, +uint8_t branches,uint32_t branch_map, +uint64_t addr, +bool notify,bool updiscon);#endif
Signature · 1 line
2.43.0
Re: [PATCH v4 15/18] hw/riscv/rv-trace-messages: add format 1 msgs with branch info
hw/riscv/rv-trace-messages.c size_t rv_etrace_gen_encoded_format2_msg(uint8_t *buf, uint64_t addr, · line 315
Quoted 13 lines
(quoted diff) +2
-0+size_t rv_etrace_gen_encoded_format1(uint8_t *buf, + uint8_t branches, uint32_t
branch_map,hw/riscv/rv-trace-messages.c +8
-0+uint64_t addr, + bool notify, bool updiscon) +{ + RVTraceMessageHeader header = {.flow = 0, .extend = 0}; +RVTraceFormat1Payload payload = {.format =0b01 , + .branches = branches, + .notify = notify, + .updiscon = updiscon};
Should we also initialize .branch_map here with .branch_map = branch_map? The function accepts branch_map, but it is not included in this payload initializer.
Thanks
Jay Chang
PATCH v4 16/18 hw/riscv/trace-encoder: send branches info
In reply to Daniel Henrique BarbozaBranch info is reported via the TCG helpers, updating the trace encoder internal branch map.
Branch packets are sent in two circunstances:
- when the branch map is full;
- when an updiscon packet is about to be sent and the branch map isn't empty.
The former will trigger a Format 1 no-addr packet, the latter a Format 1 with the updiscon address.
--- hw/riscv/trace-encoder.c | 52 ++++++++++++++++++++- hw/riscv/trace-encoder.h | 4 ++ target/riscv/helper.h | 1 + target/riscv/tcg/insn_trans/trans_rvi.c.inc | 11 +++++ target/riscv/tcg/trace_helper.c | 12 +++++ 5 files changed, 79 insertions(+), 1 deletion(-)
hw/riscv/trace-encoder.c +51 -1
@@ -30,6 +30,8 @@*/ #define TRACE_MSG_MAX_SIZE 32 +#define TRACE_MAX_BRANCHES 31 +static TracePrivLevel trencoder_get_curr_priv_level (TraceEncoder *te) {CPURISCVState *env = &te->cpu->env;
@@ -314,6 +316,9 @@ static void trencoder_reset(DeviceState *dev) te->trace_running = false; te->trace_next_insn = false; env->trace_running = false; + + te->branch_map =0 ; + te->branches =0 ; }static void trencoder_realize (DeviceState *dev,Error **errp)
@@ -411,9 +416,20 @@ static void trencoder_send_updiscon(TraceEncoder *trencoder, uint64_t pc)bool updiscon = !notify;uint8_t msg_size; - msg_size =rv_etrace_gen_encoded_format2_msg (format2_msg, pc, +if (trencoder->branches >0 ) { + msg_size =rv_etrace_gen_encoded_format1 (format2_msg, + trencoder->branches, + trencoder->branch_map, + pc, notify, updiscon); + trencoder->branches =0 ; + }else { + msg_size =rv_etrace_gen_encoded_format2_msg (format2_msg, pc, + notify, + updiscon); + } +trencoder_send_message_smem (trencoder, format2_msg, msg_size); trencoder->updiscon_pending = false;
@@ -458,6 +474,40 @@ void trencoder_trace_trap_insn(Object *trencoder_obj,trencoder_send_message_smem (trencoder, msg, msg_size); } +static void trencoder_send_branch_map (Object *trencoder_obj) +{ +TraceEncoder *te =TRACE_ENCODER (trencoder_obj); + g_autofree uint8_t *msg = g_malloc0(TRACE_MSG_MAX_SIZE); +uint8_t msg_size; + + msg_size =rv_etrace_gen_encoded_format1_noaddr (msg, + te->branches, + te->branch_map); +trencoder_send_message_smem (te, msg, msg_size); +} + +void trencoder_report_branch (Object *trencoder_obj,uint64_t pc,bool taken) +{ +TraceEncoder *te =TRACE_ENCODER (trencoder_obj); + +/* +* Note: the e-trace spec determines the value '1' for a +* branch *not* taken. The helper API is using taken = 1 +* to be more intuitive when reading TCG code. +*/ +if (!taken) { + te->branch_map =deposit32 (te->branch_map, te->branches,1 ,1 ); + } + + te->last_branch_pc = pc; + te->branches++; + +if (te->branches ==TRACE_MAX_BRANCHES ) { +trencoder_send_branch_map (trencoder_obj); + te->branches =0 ; + } +} +void trencoder_trace_ppccd (Object *trencoder_obj,uint64_t pc) {TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj);
hw/riscv/trace-encoder.h +4 -0
@@ -28,6 +28,9 @@ struct TraceEncoder {uint32_t reg_mem_size;uint64_t first_pc; +uint64_t last_branch_pc; +uint32_t branch_map; +uint8_t branches;hwaddr baseaddr;hwaddr dest_baseaddr;
@@ -54,5 +57,6 @@ void trencoder_trace_trap_insn(Object *trencoder_obj, uint64_t tval);void trencoder_trace_ppccd (Object *trencoder_obj,uint64_t pc);void trencoder_report_updiscon (Object *trencoder_obj); +void trencoder_report_branch (Object *trencoder_obj,uint64_t pc,bool taken);#endif
target/riscv/helper.h +1 -0
@@ -133,6 +133,7 @@ DEF_HELPER_6(csrrw_i128, tl, env, int, tl, tl, tl, tl)/* Trace helpers (should be put inside ifdef) */ DEF_HELPER_2(trace_insn, void, env, i64) DEF_HELPER_1(trace_updiscon, void, env) +DEF_HELPER_3(trace_branch, void, env, tl, int)#ifndef CONFIG_USER_ONLY DEF_HELPER_1 (sret, tl, env)
target/riscv/tcg/insn_trans/trans_rvi.c.inc +11 -0
@@ -268,6 +268,13 @@ static void gen_setcond_i128(TCGv rl, TCGv rh, tcg_gen_movi_tl(rh, 0); } +static void gen_trace_branch(DisasContext *ctx, int taken) +{ + if (ctx->cpu_trace_running) { + gen_helper_trace_branch(tcg_env, cpu_pc, tcg_constant_i32(taken)); + } +} + static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond) { TCGLabel *l = gen_new_label();
@@ -299,11 +306,15 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond) } #endif + gen_trace_branch(ctx, 0); + gen_goto_tb(ctx, 1, ctx->cur_insn_len); ctx->pc_save = orig_pc_save; gen_set_label(l); /* branch taken */ + gen_trace_branch(ctx, 1); + if (!riscv_cpu_allow_16bit_insn(ctx->cfg_ptr, ctx->priv_ver, ctx->misa_ext) &&
target/riscv/tcg/trace_helper.c +12 -0
@@ -37,6 +37,13 @@ void helper_trace_updiscon(CPURISCVState *env) te->updiscon_pending = true; te->trace_next_insn = true; } + +void helper_trace_branch (CPURISCVState *env,target_ulong pc,int taken) +{ +RISCVCPU *cpu =env_archcpu (env); + +trencoder_report_branch (cpu->trencoder, pc, taken); +}#else /* #ifndef CONFIG_USER_ONLY */ void helper_trace_insn (CPURISCVState *env,uint64_t pc) {
@@ -47,4 +54,9 @@ void helper_trace_updiscon(CPURISCVState *env) {return ; } + +void helper_trace_branch (CPURISCVState *env,target_ulong pc,int taken) +{ +return ; +}#endif /* #ifndef CONFIG_USER_ONLY*/
Signature · 1 line
2.43.0
Re: [PATCH v4 16/18] hw/riscv/trace-encoder: send branches info
Quoted 12 lines
+void
trencoder_report_branch(Object *trencoder_obj, uint64_t pc, bool taken)hw/riscv/trace-encoder.c +8
-0+{ +TraceEncoder *te =TRACE_ENCODER (trencoder_obj); + +/* +* Note: the e-trace spec determines the value '1' for a +* branch *not* taken. The helper API is using taken = 1 +* to be more intuitive when reading TCG code. +*/
hw/riscv/trace-encoder.c void trencoder_trace_trap_insn(Object *trencoder_obj, · line 507
Quoted 10 lines
hw/riscv/trace-encoder.c +10
-0+if (!taken) { + te->branch_map =deposit32 (te->branch_map, te->branches,1 ,1 ); + } + + te->last_branch_pc = pc; + te->branches++; + +if (te->branches ==TRACE_MAX_BRANCHES ) { +trencoder_send_branch_map (trencoder_obj); + te->branches =0 ;
Should we clear branch_map after sending the packet, or explicitly write 0 for taken branches using deposit32(te->branch_map, te->branches, 1, 0)?
Thanks
Jay Chang
PATCH v4 17/18 hw/riscv/trace: update branch bit in sync messages
In reply to Daniel Henrique BarbozaNow that we have a working branch map update the branch bit in sync messages by checking if the sync address is a branch address that was taken (or not).
--- hw/riscv/rv-trace-messages.c | 6 +++--- hw/riscv/rv-trace-messages.h | 3 ++- hw/riscv/trace-encoder.c | 30 ++++++++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 6 deletions(-)
hw/riscv/rv-trace-messages.c +3 -3
@@ -115,11 +115,12 @@ static void rv_etrace_write_header(uint8_t *buf, RVTraceMessageHeader header) }size_t rv_etrace_gen_encoded_sync_msg (uint8_t *buf,uint64_t pc, -TracePrivLevel priv_level) +TracePrivLevel priv_level, +bool pc_is_branch) {RVTraceSyncPayload payload = {.format =0b11 , .subformat =0b00 , - .branch =1 , + .branch = pc_is_branch, .privilege = priv_level};RVTraceMessageHeader header = {.flow =0 , .extend =0 , .length =SYNC_PAYLOAD_SIZE_64BITS };
@@ -158,7 +159,6 @@ size_t rv_etrace_gen_encoded_trap_msg(uint8_t *buf, uint64_t trap_addr, {RVTraceTrapPayload payload = {.format =0b11 , .subformat =0b01 , - .branch =1 , .privilege = priv_level, .ecause = ecause};RVTraceMessageHeader header = {.flow =0 , .extend =0 ,
hw/riscv/rv-trace-messages.h +2 -1
@@ -20,7 +20,8 @@ typedef enum { } TracePrivLevel;size_t rv_etrace_gen_encoded_sync_msg (uint8_t *buf,uint64_t pc, -TracePrivLevel priv_level); +TracePrivLevel priv_level, +bool pc_is_branch); size_t rv_etrace_gen_encoded_trap_msg(uint8_t *buf, uint64_t trap_addr, TracePrivLevel priv_level, uint8_t ecause,
hw/riscv/trace-encoder.c +28 -2
@@ -395,15 +395,38 @@ static void trencoder_send_message_smem(TraceEncoder *trencoder,trencoder_update_ramsink_writep (trencoder, dest, wrapped); } +static bool trencoder_addr_is_branch_taken (TraceEncoder *te,uint64_t addr) +{ +uint8_t last_branch; + +if (te->branches ==0 ) { +return false; + } + +if (te->last_branch_pc == addr) { + last_branch =extract32 (te->branches, te->branches -1 ,1 ); + +/* 0: branch taken, 1: not taken*/ +if (last_branch ==0 ) { +return true; + } + } + +return false; +} +static void trencoder_send_sync_msg (Object *trencoder_obj,uint64_t pc) {TraceEncoder *trencoder =TRACE_ENCODER (trencoder_obj);TracePrivLevel priv =trencoder_get_curr_priv_level (trencoder); g_autofree uint8_t *msg = g_malloc0(TRACE_MSG_MAX_SIZE);uint8_t msg_size; +bool is_branch_taken; trencoder->first_pc = pc; - msg_size =rv_etrace_gen_encoded_sync_msg (msg, pc, priv); + is_branch_taken =trencoder_addr_is_branch_taken (trencoder, pc); + msg_size =rv_etrace_gen_encoded_sync_msg (msg, pc, priv, + is_branch_taken);trencoder_send_message_smem (trencoder, msg, msg_size); }
@@ -441,6 +464,7 @@ void trencoder_set_first_trace_insn(Object *trencoder_obj, uint64_t pc)TracePrivLevel priv =trencoder_get_curr_priv_level (trencoder); g_autofree uint8_t *msg = g_malloc0(TRACE_MSG_MAX_SIZE);uint8_t msg_size; +bool is_branch_taken;if (trencoder->updiscon_pending) {trencoder_send_updiscon (trencoder, pc);
@@ -448,7 +472,9 @@ void trencoder_set_first_trace_insn(Object *trencoder_obj, uint64_t pc) trencoder->first_pc = pc;trace_trencoder_first_trace_insn (pc); - msg_size =rv_etrace_gen_encoded_sync_msg (msg, pc, priv); + is_branch_taken =trencoder_addr_is_branch_taken (trencoder, pc); + msg_size =rv_etrace_gen_encoded_sync_msg (msg, pc, priv, + is_branch_taken);trencoder_send_message_smem (trencoder, msg, msg_size); }
Signature · 1 line
2.43.0
Re: [PATCH v4 17/18] hw/riscv/trace: update branch bit in sync messages
hw/riscv/rv-trace-messages.c static void rv_etrace_write_header(uint8_t *buf, RVTraceMessageHeader header) · line 123
Quoted 10 lines
size_trv_etrace_gen_encoded_sync_msg(uint8_t *buf,uint64_tpc,hw/riscv/rv-trace-messages.c +3
-2- TracePrivLevel priv_level) + TracePrivLevel priv_level, + bool pc_is_branch) {RVTraceSyncPayload payload = {.format =0b11 , .subformat =0b00 , - .branch =1 , + .branch = pc_is_branch,
Spec says that
"Set to 0 if the address points to a branch instruction, and the branch was taken. Set to 1 if the instruction is
not a branch or if the branch is not taken."
Thanks
Jay Chang
Re: [PATCH v4 17/18] hw/riscv/trace: update branch bit in sync messages
In reply to Daniel Henrique Barbozahw/riscv/trace-encoder.c static void trencoder_send_message_smem(TraceEncoder *trencoder, · line 407
Quoted 13 lines
+static bool
trencoder_addr_is_branch_taken(TraceEncoder *te, uint64_t addr)hw/riscv/trace-encoder.c +9
-0+{ +uint8_t last_branch; + +if (te->branches ==0 ) { +return false; + } + +if (te->last_branch_pc == addr) { + last_branch =extract32 (te->branches, te->branches -1 ,1 );
last_branch = extract32(te->branch_map, te->branches - 1, 1);
Thanks
Jay Chang
PATCH v4 18/18 docs/specs: document RISC-V hwtrace (e-trace) support
In reply to Daniel Henrique BarbozaAdd extra information and context about the e-trace support added in QEMU. Most of the information documented is already presented in the commit messages, albeit in a more pleasant way.
--- docs/specs/index.rst | 1 + docs/specs/riscv-hwtrace.rst | 96 ++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 docs/specs/riscv-hwtrace.rst
docs/specs/index.rst +1 -0
@@ -43,3 +43,4 @@ guest hardware that is specific to QEMU. aspeed-intc iommu-testdev s390x-secure-ipl + riscv-hwtrace
docs/specs/riscv-hwtrace.rst new file +96 -0
96 lines not on this page · raw message
Signature · 1 line
2.43.0
Quoted 14 lines · Daniel Henrique Barboza
Hi,
It's time for a refresher of the hwtrace/e-trace support for RISC-V. It
has been awhile since v3 and we have Konstantin posting patches to implement N-trace on top of it.For this new version a couple of design changes were made, most notably
the FDT functions are now hosted infdt-common.cand we're using anEXT_TB_FLAGto enable/disable the trace session instead of using a TCG
global.
Quoted 8 lines
A new patch was also added to describe what we're implementing and some
design decisions made w.r.t TCG instrumentation and encoder parameters.This emulation is being used to support the development of the Linux RISC-V
e-trace driver framework. Newest version can be found here:
Can you link do the documentation?
Is this based on
https://github.com/riscv-non-isa/riscv-trace-spec/releases/tag/v2.1 ?
Alistair
Quoted 80 lines
("[PATCH v6 00/14] Linux RISC-V trace framework and drivers")
https://lists.infradead.org/pipermail/linux-riscv/2026-September/100456.htmlPatches based on alistair/riscv-to-apply.next.
Changes from v3:
- patch 5:
- move FDT creation code tofdt-common.c
- patch 7:
- movetrace_helper.cto tcg subdir
- use "CPU_TRACE_RUNNING"EXT_TB_FLAGandctx->cpu_trace_runningfor insn instrumentation
instead of using a TCG global
- patch 14:
- usectx->cpu_trace_runningingen_trace_branch()
- patch 16:
- usectx->cpu_trace_runningingen_trace_branch()
- patch 18 (new):
- add documentation
- v3 link:
https://lore.kernel.org/qemu-devel/[email protected]/Daniel Henrique Barboza (18):
hw/riscv: Trace Encoder initial impl
hw/riscv: Trace RAM Sink initial impl hw/riscv/trace-encoder: add trace start/stop logichw/riscv/virt.c: add trace encoders and trace ram sinkshw/riscv/virt.cadd trace encoder and ramsink fdt nodes hw/riscv: add e-trace message helpers
target/riscv: add initial trace instrumentation hw/riscv/trace-encoder: write e-trace packets to RAM sink test/qtest: addriscv-trace-test.chw/riscv/rv-trace-messages.c: add encoded trap message hw/riscv, target/riscv: send trace trap messages target/riscv, hw/riscv: send trace ppccd packets hw/riscv/trace: add format2 msg helper
hw/riscv, target/riscv: send resync updiscon trace packets hw/riscv/rv-trace-messages: add format 1 msgs with branch info hw/riscv/trace-encoder: send branches info hw/riscv/trace: update branch bit in sync messages docs/specs: document RISC-V hwtrace (e-trace) supportdocs/specs/index.rst | 1 + docs/specs/riscv-hwtrace.rst | 96 +++ hw/riscv/Kconfig | 5 + hw/riscv/fdt-common.c | 70 ++ hw/riscv/meson.build | 3 +- hw/riscv/rv-trace-messages.c | 373 +++++++++++ hw/riscv/rv-trace-messages.h | 40 ++ hw/riscv/trace-encoder.c | 610
++++++++++++++++++
hw/riscv/trace-encoder.h | 62 ++ hw/riscv/trace-events | 9 + hw/riscv/trace-ram-sink.c | 263 ++++++++ hw/riscv/trace-ram-sink.h | 83 +++ hw/riscv/virt.c | 128 ++++ include/hw/riscv/fdt-common.h | 4 + include/hw/riscv/virt.h | 2 + target/riscv/cpu.h | 12 + target/riscv/helper.h | 6 + target/riscv/tcg/cpu_helper.c | 26 +- .../tcg/insn_trans/trans_privileged.c.inc | 11 + target/riscv/tcg/insn_trans/trans_rvi.c.inc | 13 + target/riscv/tcg/meson.build | 6 + target/riscv/tcg/tcg-cpu.c | 9 + target/riscv/tcg/trace_helper.c | 62 ++ target/riscv/tcg/translate.c | 16 + tests/qtest/meson.build | 2 +- tests/qtest/riscv-trace-test.c | 120 ++++26 files changed, 2029 insertions(+), 3 deletions(-)create mode 100644
docs/specs/riscv-hwtrace.rstcreate mode 100644hw/riscv/rv-trace-messages.ccreate mode 100644hw/riscv/rv-trace-messages.hcreate mode 100644hw/riscv/trace-encoder.ccreate mode 100644hw/riscv/trace-encoder.hcreate mode 100644hw/riscv/trace-ram-sink.ccreate mode 100644hw/riscv/trace-ram-sink.hcreate mode 100644target/riscv/tcg/trace_helper.ccreate mode 100644tests/qtest/riscv-trace-test.c