* [PATCH v5 0/5] Support discontinuous PMU counters
@ 2023-10-31 15:37 Rob Bradford
2023-10-31 15:37 ` [PATCH v5 1/5] target/riscv: Propagate error from PMU setup Rob Bradford
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Rob Bradford @ 2023-10-31 15:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford
Currently the available PMU counters start at HPM3 and run through to
the number specified by the "pmu-num" property. There is no
requirement in the specification that the available counters be
continously numbered. This series add suppport for specifying a
discountinuous range of counters though a "pmu-mask" property.
v5:
* Added more R-B tags
* Make "pmu-num" property directly update "pmu-mask" removing the need
to store that value.
v4:
* Added more R-B tags (just missing from 4 & 5)
* Added details on how to calculate mask
* Use custom property for "pmu-num" in order to give deprecation warning
* Special case a zero value for "pmu-num"
v3:
* Use env_archcpu() in csr.c
* Re-added check to enforce deprectated "pmu-num" below limit
* Check that standard counters are not included in mask
* Remove use of MAKE_32BIT_MASK()
v2:
* Use cfg.pmu_mask wherever cfg.pmu_num was used previously
* Deprecate pmu_num property (warning, comment & updated documentation)
* Override default pmu_mask value iff pmu_num changed from default
Rob Bradford (5):
target/riscv: Propagate error from PMU setup
target/riscv: Don't assume PMU counters are continuous
target/riscv: Use existing PMU counter mask in FDT generation
target/riscv: Add "pmu-mask" property to replace "pmu-num"
docs/about/deprecated: Document RISC-V "pmu-num" deprecation
docs/about/deprecated.rst | 12 ++++++++++++
hw/riscv/virt.c | 2 +-
target/riscv/cpu.c | 40 +++++++++++++++++++++++++++++++++++++-
target/riscv/cpu_cfg.h | 2 +-
target/riscv/csr.c | 5 +++--
target/riscv/machine.c | 2 +-
target/riscv/pmu.c | 34 ++++++++++++++------------------
target/riscv/pmu.h | 5 +++--
target/riscv/tcg/tcg-cpu.c | 10 ++++++++--
9 files changed, 83 insertions(+), 29 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/5] target/riscv: Propagate error from PMU setup
2023-10-31 15:37 [PATCH v5 0/5] Support discontinuous PMU counters Rob Bradford
@ 2023-10-31 15:37 ` Rob Bradford
2023-10-31 15:37 ` [PATCH v5 2/5] target/riscv: Don't assume PMU counters are continuous Rob Bradford
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Rob Bradford @ 2023-10-31 15:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford, Weiwei Li
More closely follow the QEMU style by returning an Error and propagating
it there is an error relating to the PMU setup.
Further simplify the function by removing the num_counters parameter as
this is available from the passed in cpu pointer.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
target/riscv/pmu.c | 19 +++++++++----------
target/riscv/pmu.h | 3 ++-
target/riscv/tcg/tcg-cpu.c | 8 +++++++-
3 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
index 36f6307d28..13801ccb78 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c
@@ -434,22 +434,21 @@ int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, uint32_t ctr_idx)
}
-int riscv_pmu_init(RISCVCPU *cpu, int num_counters)
+void riscv_pmu_init(RISCVCPU *cpu, Error **errp)
{
- if (num_counters > (RV_MAX_MHPMCOUNTERS - 3)) {
- return -1;
+ uint8_t pmu_num = cpu->cfg.pmu_num;
+
+ if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) {
+ error_setg(errp, "Number of counters exceeds maximum available");
+ return;
}
cpu->pmu_event_ctr_map = g_hash_table_new(g_direct_hash, g_direct_equal);
if (!cpu->pmu_event_ctr_map) {
- /* PMU support can not be enabled */
- qemu_log_mask(LOG_UNIMP, "PMU events can't be supported\n");
- cpu->cfg.pmu_num = 0;
- return -1;
+ error_setg(errp, "Unable to allocate PMU event hash table");
+ return;
}
/* Create a bitmask of available programmable counters */
- cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, num_counters);
-
- return 0;
+ cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, pmu_num);
}
diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
index 2bfb71ba87..88e0713296 100644
--- a/target/riscv/pmu.h
+++ b/target/riscv/pmu.h
@@ -17,13 +17,14 @@
*/
#include "cpu.h"
+#include "qapi/error.h"
bool riscv_pmu_ctr_monitor_instructions(CPURISCVState *env,
uint32_t target_ctr);
bool riscv_pmu_ctr_monitor_cycles(CPURISCVState *env,
uint32_t target_ctr);
void riscv_pmu_timer_cb(void *priv);
-int riscv_pmu_init(RISCVCPU *cpu, int num_counters);
+void riscv_pmu_init(RISCVCPU *cpu, Error **errp);
int riscv_pmu_update_event_map(CPURISCVState *env, uint64_t value,
uint32_t ctr_idx);
int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx);
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index a28918ab30..ed3eb991c0 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -614,7 +614,13 @@ static bool tcg_cpu_realize(CPUState *cs, Error **errp)
}
if (cpu->cfg.pmu_num) {
- if (!riscv_pmu_init(cpu, cpu->cfg.pmu_num) && cpu->cfg.ext_sscofpmf) {
+ riscv_pmu_init(cpu, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ return false;
+ }
+
+ if (cpu->cfg.ext_sscofpmf) {
cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
riscv_pmu_timer_cb, cpu);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 2/5] target/riscv: Don't assume PMU counters are continuous
2023-10-31 15:37 [PATCH v5 0/5] Support discontinuous PMU counters Rob Bradford
2023-10-31 15:37 ` [PATCH v5 1/5] target/riscv: Propagate error from PMU setup Rob Bradford
@ 2023-10-31 15:37 ` Rob Bradford
2023-10-31 15:37 ` [PATCH v5 3/5] target/riscv: Use existing PMU counter mask in FDT generation Rob Bradford
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Rob Bradford @ 2023-10-31 15:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford, Weiwei Li
Check the PMU available bitmask when checking if a counter is valid
rather than comparing the index against the number of PMUs.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
target/riscv/csr.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 4b4ab56c40..a6ea38e0ba 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -183,7 +183,8 @@ static RISCVException zcmt(CPURISCVState *env, int csrno)
#if !defined(CONFIG_USER_ONLY)
static RISCVException mctr(CPURISCVState *env, int csrno)
{
- int pmu_num = riscv_cpu_cfg(env)->pmu_num;
+ RISCVCPU *cpu = env_archcpu(env);
+ uint32_t pmu_avail_ctrs = cpu->pmu_avail_ctrs;
int ctr_index;
int base_csrno = CSR_MHPMCOUNTER3;
@@ -192,7 +193,7 @@ static RISCVException mctr(CPURISCVState *env, int csrno)
base_csrno += 0x80;
}
ctr_index = csrno - base_csrno;
- if (!pmu_num || ctr_index >= pmu_num) {
+ if ((BIT(ctr_index) & pmu_avail_ctrs >> 3) == 0) {
/* The PMU is not enabled or counter is out of range */
return RISCV_EXCP_ILLEGAL_INST;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 3/5] target/riscv: Use existing PMU counter mask in FDT generation
2023-10-31 15:37 [PATCH v5 0/5] Support discontinuous PMU counters Rob Bradford
2023-10-31 15:37 ` [PATCH v5 1/5] target/riscv: Propagate error from PMU setup Rob Bradford
2023-10-31 15:37 ` [PATCH v5 2/5] target/riscv: Don't assume PMU counters are continuous Rob Bradford
@ 2023-10-31 15:37 ` Rob Bradford
2023-10-31 15:37 ` [PATCH v5 4/5] target/riscv: Add "pmu-mask" property to replace "pmu-num" Rob Bradford
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Rob Bradford @ 2023-10-31 15:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford, Weiwei Li
During the FDT generation use the existing mask containing the enabled
counters rather then generating a new one. Using the existing mask will
support the use of discontinuous counters.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
hw/riscv/virt.c | 2 +-
target/riscv/pmu.c | 6 +-----
target/riscv/pmu.h | 2 +-
3 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 9de578c756..241681f98d 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -722,7 +722,7 @@ static void create_fdt_pmu(RISCVVirtState *s)
pmu_name = g_strdup_printf("/pmu");
qemu_fdt_add_subnode(ms->fdt, pmu_name);
qemu_fdt_setprop_string(ms->fdt, pmu_name, "compatible", "riscv,pmu");
- riscv_pmu_generate_fdt_node(ms->fdt, hart.cfg.pmu_num, pmu_name);
+ riscv_pmu_generate_fdt_node(ms->fdt, hart.pmu_avail_ctrs, pmu_name);
g_free(pmu_name);
}
diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
index 13801ccb78..7ddf4977b1 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c
@@ -34,13 +34,9 @@
* to provide the correct value as well. Heterogeneous PMU per hart is not
* supported yet. Thus, number of counters are same across all harts.
*/
-void riscv_pmu_generate_fdt_node(void *fdt, int num_ctrs, char *pmu_name)
+void riscv_pmu_generate_fdt_node(void *fdt, uint32_t cmask, char *pmu_name)
{
uint32_t fdt_event_ctr_map[15] = {};
- uint32_t cmask;
-
- /* All the programmable counters can map to any event */
- cmask = MAKE_32BIT_MASK(3, num_ctrs);
/*
* The event encoding is specified in the SBI specification
diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
index 88e0713296..505fc850d3 100644
--- a/target/riscv/pmu.h
+++ b/target/riscv/pmu.h
@@ -28,6 +28,6 @@ void riscv_pmu_init(RISCVCPU *cpu, Error **errp);
int riscv_pmu_update_event_map(CPURISCVState *env, uint64_t value,
uint32_t ctr_idx);
int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx);
-void riscv_pmu_generate_fdt_node(void *fdt, int num_counters, char *pmu_name);
+void riscv_pmu_generate_fdt_node(void *fdt, uint32_t cmask, char *pmu_name);
int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value,
uint32_t ctr_idx);
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 4/5] target/riscv: Add "pmu-mask" property to replace "pmu-num"
2023-10-31 15:37 [PATCH v5 0/5] Support discontinuous PMU counters Rob Bradford
` (2 preceding siblings ...)
2023-10-31 15:37 ` [PATCH v5 3/5] target/riscv: Use existing PMU counter mask in FDT generation Rob Bradford
@ 2023-10-31 15:37 ` Rob Bradford
2023-11-02 0:39 ` Alistair Francis
2023-10-31 15:37 ` [PATCH v5 5/5] docs/about/deprecated: Document RISC-V "pmu-num" deprecation Rob Bradford
2023-11-06 1:15 ` [PATCH v5 0/5] Support discontinuous PMU counters Alistair Francis
5 siblings, 1 reply; 8+ messages in thread
From: Rob Bradford @ 2023-10-31 15:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford, Weiwei Li
Using a mask instead of the number of PMU devices supports the accurate
emulation of platforms that have a discontinuous set of PMU counters.
The "pmu-num" property now generates a warning when used by the user on
the command line.
Rather than storing the value for "pmu-num" convert it directly to the
mask if it is specified (overwriting the default "pmu-mask" value)
likewise the value is calculated from the mask if the property value is
obtained.
In the unusual situation that both "pmu-mask" and "pmu-num" are provided
then then the order on the command line determines which takes
precedence (later overwriting earlier.)
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
---
target/riscv/cpu.c | 40 +++++++++++++++++++++++++++++++++++++-
target/riscv/cpu_cfg.h | 2 +-
target/riscv/machine.c | 2 +-
target/riscv/pmu.c | 15 +++++++-------
target/riscv/tcg/tcg-cpu.c | 2 +-
5 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ac4a6c7eec..51accdba5f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1362,8 +1362,46 @@ const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void prop_pmu_num_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ RISCVCPU *cpu = RISCV_CPU(obj);
+ uint8_t pmu_num;
+
+ visit_type_uint8(v, name, &pmu_num, errp);
+
+ if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) {
+ error_setg(errp, "Number of counters exceeds maximum available");
+ return;
+ }
+
+ if (pmu_num == 0) {
+ cpu->cfg.pmu_mask = 0;
+ } else {
+ cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, pmu_num);
+ }
+
+ warn_report("\"pmu-num\" property is deprecated; use \"pmu-mask\"");
+}
+
+static void prop_pmu_num_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ RISCVCPU *cpu = RISCV_CPU(obj);
+ uint8_t pmu_num = ctpop32(cpu->cfg.pmu_mask);
+
+ visit_type_uint8(v, name, &pmu_num, errp);
+}
+
+const PropertyInfo prop_pmu_num = {
+ .name = "pmu-num",
+ .get = prop_pmu_num_get,
+ .set = prop_pmu_num_set,
+};
+
Property riscv_cpu_options[] = {
- DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
+ DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask, MAKE_64BIT_MASK(3, 16)),
+ {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 0e6a0f245c..f4e6f273fc 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -123,7 +123,7 @@ struct RISCVCPUConfig {
bool ext_xtheadsync;
bool ext_XVentanaCondOps;
- uint8_t pmu_num;
+ uint32_t pmu_mask;
char *priv_spec;
char *user_spec;
char *bext_spec;
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index c7c862cdd3..9f6e3f7a6d 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -313,7 +313,7 @@ static bool pmu_needed(void *opaque)
{
RISCVCPU *cpu = opaque;
- return cpu->cfg.pmu_num;
+ return (cpu->cfg.pmu_mask > 0);
}
static const VMStateDescription vmstate_pmu_ctr_state = {
diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
index 7ddf4977b1..0e7d58b8a5 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c
@@ -18,14 +18,13 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
+#include "qemu/error-report.h"
#include "cpu.h"
#include "pmu.h"
#include "sysemu/cpu-timers.h"
#include "sysemu/device_tree.h"
#define RISCV_TIMEBASE_FREQ 1000000000 /* 1Ghz */
-#define MAKE_32BIT_MASK(shift, length) \
- (((uint32_t)(~0UL) >> (32 - (length))) << (shift))
/*
* To keep it simple, any event can be mapped to any programmable counters in
@@ -184,7 +183,7 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx)
CPURISCVState *env = &cpu->env;
gpointer value;
- if (!cpu->cfg.pmu_num) {
+ if (!cpu->cfg.pmu_mask) {
return 0;
}
value = g_hash_table_lookup(cpu->pmu_event_ctr_map,
@@ -432,9 +431,12 @@ int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, uint32_t ctr_idx)
void riscv_pmu_init(RISCVCPU *cpu, Error **errp)
{
- uint8_t pmu_num = cpu->cfg.pmu_num;
+ if (cpu->cfg.pmu_mask & (COUNTEREN_CY | COUNTEREN_TM | COUNTEREN_IR)) {
+ error_setg(errp, "\"pmu-mask\" contains invalid bits (0-2) set");
+ return;
+ }
- if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) {
+ if (ctpop32(cpu->cfg.pmu_mask) > (RV_MAX_MHPMCOUNTERS - 3)) {
error_setg(errp, "Number of counters exceeds maximum available");
return;
}
@@ -445,6 +447,5 @@ void riscv_pmu_init(RISCVCPU *cpu, Error **errp)
return;
}
- /* Create a bitmask of available programmable counters */
- cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, pmu_num);
+ cpu->pmu_avail_ctrs = cpu->cfg.pmu_mask;
}
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index ed3eb991c0..53c52389b9 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -613,7 +613,7 @@ static bool tcg_cpu_realize(CPUState *cs, Error **errp)
riscv_timer_init(cpu);
}
- if (cpu->cfg.pmu_num) {
+ if (cpu->cfg.pmu_mask) {
riscv_pmu_init(cpu, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 5/5] docs/about/deprecated: Document RISC-V "pmu-num" deprecation
2023-10-31 15:37 [PATCH v5 0/5] Support discontinuous PMU counters Rob Bradford
` (3 preceding siblings ...)
2023-10-31 15:37 ` [PATCH v5 4/5] target/riscv: Add "pmu-mask" property to replace "pmu-num" Rob Bradford
@ 2023-10-31 15:37 ` Rob Bradford
2023-11-06 1:15 ` [PATCH v5 0/5] Support discontinuous PMU counters Alistair Francis
5 siblings, 0 replies; 8+ messages in thread
From: Rob Bradford @ 2023-10-31 15:37 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford, reviewer:Incompatible changes
This has been replaced by a "pmu-mask" property that provides much more
flexibility.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Acked-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
docs/about/deprecated.rst | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 4e0eb2fe02..60c26bc410 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -413,6 +413,18 @@ Specifying the iSCSI password in plain text on the command line using the
used instead, to refer to a ``--object secret...`` instance that provides
a password via a file, or encrypted.
+CPU device properties
+'''''''''''''''''''''
+
+``pmu-num=n`` on RISC-V CPUs (since 8.2)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+In order to support more flexible counter configurations this has been replaced
+by a ``pmu-mask`` property. If set of counters is continuous then the mask can
+be calculated with ``((2 ^ n) - 1) << 3``. The least significant three bits
+must be left clear.
+
+
Backwards compatibility
-----------------------
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 4/5] target/riscv: Add "pmu-mask" property to replace "pmu-num"
2023-10-31 15:37 ` [PATCH v5 4/5] target/riscv: Add "pmu-mask" property to replace "pmu-num" Rob Bradford
@ 2023-11-02 0:39 ` Alistair Francis
0 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2023-11-02 0:39 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu, Weiwei Li
On Wed, Nov 1, 2023 at 1:41 AM Rob Bradford <rbradford@rivosinc.com> wrote:
>
> Using a mask instead of the number of PMU devices supports the accurate
> emulation of platforms that have a discontinuous set of PMU counters.
>
> The "pmu-num" property now generates a warning when used by the user on
> the command line.
>
> Rather than storing the value for "pmu-num" convert it directly to the
> mask if it is specified (overwriting the default "pmu-mask" value)
> likewise the value is calculated from the mask if the property value is
> obtained.
>
> In the unusual situation that both "pmu-mask" and "pmu-num" are provided
> then then the order on the command line determines which takes
> precedence (later overwriting earlier.)
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 40 +++++++++++++++++++++++++++++++++++++-
> target/riscv/cpu_cfg.h | 2 +-
> target/riscv/machine.c | 2 +-
> target/riscv/pmu.c | 15 +++++++-------
> target/riscv/tcg/tcg-cpu.c | 2 +-
> 5 files changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ac4a6c7eec..51accdba5f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1362,8 +1362,46 @@ const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +static void prop_pmu_num_set(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + RISCVCPU *cpu = RISCV_CPU(obj);
> + uint8_t pmu_num;
> +
> + visit_type_uint8(v, name, &pmu_num, errp);
> +
> + if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) {
> + error_setg(errp, "Number of counters exceeds maximum available");
> + return;
> + }
> +
> + if (pmu_num == 0) {
> + cpu->cfg.pmu_mask = 0;
> + } else {
> + cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, pmu_num);
> + }
> +
> + warn_report("\"pmu-num\" property is deprecated; use \"pmu-mask\"");
> +}
> +
> +static void prop_pmu_num_get(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + RISCVCPU *cpu = RISCV_CPU(obj);
> + uint8_t pmu_num = ctpop32(cpu->cfg.pmu_mask);
> +
> + visit_type_uint8(v, name, &pmu_num, errp);
> +}
> +
> +const PropertyInfo prop_pmu_num = {
> + .name = "pmu-num",
> + .get = prop_pmu_num_get,
> + .set = prop_pmu_num_set,
> +};
> +
> Property riscv_cpu_options[] = {
> - DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
> + DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask, MAKE_64BIT_MASK(3, 16)),
> + {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
>
> DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
> DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 0e6a0f245c..f4e6f273fc 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -123,7 +123,7 @@ struct RISCVCPUConfig {
> bool ext_xtheadsync;
> bool ext_XVentanaCondOps;
>
> - uint8_t pmu_num;
> + uint32_t pmu_mask;
> char *priv_spec;
> char *user_spec;
> char *bext_spec;
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index c7c862cdd3..9f6e3f7a6d 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -313,7 +313,7 @@ static bool pmu_needed(void *opaque)
> {
> RISCVCPU *cpu = opaque;
>
> - return cpu->cfg.pmu_num;
> + return (cpu->cfg.pmu_mask > 0);
> }
>
> static const VMStateDescription vmstate_pmu_ctr_state = {
> diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
> index 7ddf4977b1..0e7d58b8a5 100644
> --- a/target/riscv/pmu.c
> +++ b/target/riscv/pmu.c
> @@ -18,14 +18,13 @@
>
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> +#include "qemu/error-report.h"
> #include "cpu.h"
> #include "pmu.h"
> #include "sysemu/cpu-timers.h"
> #include "sysemu/device_tree.h"
>
> #define RISCV_TIMEBASE_FREQ 1000000000 /* 1Ghz */
> -#define MAKE_32BIT_MASK(shift, length) \
> - (((uint32_t)(~0UL) >> (32 - (length))) << (shift))
>
> /*
> * To keep it simple, any event can be mapped to any programmable counters in
> @@ -184,7 +183,7 @@ int riscv_pmu_incr_ctr(RISCVCPU *cpu, enum riscv_pmu_event_idx event_idx)
> CPURISCVState *env = &cpu->env;
> gpointer value;
>
> - if (!cpu->cfg.pmu_num) {
> + if (!cpu->cfg.pmu_mask) {
> return 0;
> }
> value = g_hash_table_lookup(cpu->pmu_event_ctr_map,
> @@ -432,9 +431,12 @@ int riscv_pmu_setup_timer(CPURISCVState *env, uint64_t value, uint32_t ctr_idx)
>
> void riscv_pmu_init(RISCVCPU *cpu, Error **errp)
> {
> - uint8_t pmu_num = cpu->cfg.pmu_num;
> + if (cpu->cfg.pmu_mask & (COUNTEREN_CY | COUNTEREN_TM | COUNTEREN_IR)) {
> + error_setg(errp, "\"pmu-mask\" contains invalid bits (0-2) set");
> + return;
> + }
>
> - if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) {
> + if (ctpop32(cpu->cfg.pmu_mask) > (RV_MAX_MHPMCOUNTERS - 3)) {
> error_setg(errp, "Number of counters exceeds maximum available");
> return;
> }
> @@ -445,6 +447,5 @@ void riscv_pmu_init(RISCVCPU *cpu, Error **errp)
> return;
> }
>
> - /* Create a bitmask of available programmable counters */
> - cpu->pmu_avail_ctrs = MAKE_32BIT_MASK(3, pmu_num);
> + cpu->pmu_avail_ctrs = cpu->cfg.pmu_mask;
> }
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index ed3eb991c0..53c52389b9 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -613,7 +613,7 @@ static bool tcg_cpu_realize(CPUState *cs, Error **errp)
> riscv_timer_init(cpu);
> }
>
> - if (cpu->cfg.pmu_num) {
> + if (cpu->cfg.pmu_mask) {
> riscv_pmu_init(cpu, &local_err);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 0/5] Support discontinuous PMU counters
2023-10-31 15:37 [PATCH v5 0/5] Support discontinuous PMU counters Rob Bradford
` (4 preceding siblings ...)
2023-10-31 15:37 ` [PATCH v5 5/5] docs/about/deprecated: Document RISC-V "pmu-num" deprecation Rob Bradford
@ 2023-11-06 1:15 ` Alistair Francis
5 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2023-11-06 1:15 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu
On Wed, Nov 1, 2023 at 1:41 AM Rob Bradford <rbradford@rivosinc.com> wrote:
>
> Currently the available PMU counters start at HPM3 and run through to
> the number specified by the "pmu-num" property. There is no
> requirement in the specification that the available counters be
> continously numbered. This series add suppport for specifying a
> discountinuous range of counters though a "pmu-mask" property.
>
> v5:
>
> * Added more R-B tags
> * Make "pmu-num" property directly update "pmu-mask" removing the need
> to store that value.
>
> v4:
>
> * Added more R-B tags (just missing from 4 & 5)
> * Added details on how to calculate mask
> * Use custom property for "pmu-num" in order to give deprecation warning
> * Special case a zero value for "pmu-num"
>
> v3:
>
> * Use env_archcpu() in csr.c
> * Re-added check to enforce deprectated "pmu-num" below limit
> * Check that standard counters are not included in mask
> * Remove use of MAKE_32BIT_MASK()
>
> v2:
>
> * Use cfg.pmu_mask wherever cfg.pmu_num was used previously
> * Deprecate pmu_num property (warning, comment & updated documentation)
> * Override default pmu_mask value iff pmu_num changed from default
>
> Rob Bradford (5):
> target/riscv: Propagate error from PMU setup
> target/riscv: Don't assume PMU counters are continuous
> target/riscv: Use existing PMU counter mask in FDT generation
> target/riscv: Add "pmu-mask" property to replace "pmu-num"
> docs/about/deprecated: Document RISC-V "pmu-num" deprecation
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> docs/about/deprecated.rst | 12 ++++++++++++
> hw/riscv/virt.c | 2 +-
> target/riscv/cpu.c | 40 +++++++++++++++++++++++++++++++++++++-
> target/riscv/cpu_cfg.h | 2 +-
> target/riscv/csr.c | 5 +++--
> target/riscv/machine.c | 2 +-
> target/riscv/pmu.c | 34 ++++++++++++++------------------
> target/riscv/pmu.h | 5 +++--
> target/riscv/tcg/tcg-cpu.c | 10 ++++++++--
> 9 files changed, 83 insertions(+), 29 deletions(-)
>
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-11-06 1:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-31 15:37 [PATCH v5 0/5] Support discontinuous PMU counters Rob Bradford
2023-10-31 15:37 ` [PATCH v5 1/5] target/riscv: Propagate error from PMU setup Rob Bradford
2023-10-31 15:37 ` [PATCH v5 2/5] target/riscv: Don't assume PMU counters are continuous Rob Bradford
2023-10-31 15:37 ` [PATCH v5 3/5] target/riscv: Use existing PMU counter mask in FDT generation Rob Bradford
2023-10-31 15:37 ` [PATCH v5 4/5] target/riscv: Add "pmu-mask" property to replace "pmu-num" Rob Bradford
2023-11-02 0:39 ` Alistair Francis
2023-10-31 15:37 ` [PATCH v5 5/5] docs/about/deprecated: Document RISC-V "pmu-num" deprecation Rob Bradford
2023-11-06 1:15 ` [PATCH v5 0/5] Support discontinuous PMU counters Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).