Using a mask instead of the number of PMU devices supports the accurate
emulation of platforms that have a discontinuous set of PMU counters.
Generate a warning if the old property changed from the default but
still go ahead and use it to generate the mask if the user has changed
it from the default
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
---
target/riscv/cpu.c | 5 +++--
target/riscv/cpu_cfg.h | 3 ++-
target/riscv/machine.c | 2 +-
target/riscv/pmu.c | 14 ++++++++++----
4 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c9d8fc12fe..4d2987e568 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1487,7 +1487,7 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, 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);
@@ -1812,7 +1812,8 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
static Property riscv_cpu_extensions[] = {
/* Defaults for standard extensions */
- DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
+ DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16), /* Deprecated */
+ DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask, MAKE_32BIT_MASK(3, 16)),
DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false),
DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 0e6a0f245c..d273487040 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -123,7 +123,8 @@ struct RISCVCPUConfig {
bool ext_xtheadsync;
bool ext_XVentanaCondOps;
- uint8_t pmu_num;
+ uint8_t pmu_num; /* Deprecated */
+ 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 360c76f63e..f2d35b4d3b 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c
@@ -18,6 +18,7 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
+#include "qemu/error-report.h"
#include "cpu.h"
#include "pmu.h"
#include "sysemu/cpu-timers.h"
@@ -182,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,7 +433,7 @@ void riscv_pmu_init(RISCVCPU *cpu, Error **errp)
{
uint8_t pmu_num = cpu->cfg.pmu_num;
- 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;
}
@@ -443,6 +444,11 @@ 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);
+ /* Check if user set it by comparing against default */
+ if (pmu_num != 16) {
+ warn_report("\"pmu-num\" property is deprecated; use \"pmu-mask\"");
+ cpu->cfg.pmu_mask = MAKE_32BIT_MASK(3, pmu_num);
+ }
+
+ cpu->pmu_avail_ctrs = cpu->cfg.pmu_mask;
}
--
2.41.0
On 2023/10/11 22:45, Rob Bradford 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.
>
> Generate a warning if the old property changed from the default but
> still go ahead and use it to generate the mask if the user has changed
> it from the default
>
> Signed-off-by: Rob Bradford<rbradford@rivosinc.com>
> ---
> target/riscv/cpu.c | 5 +++--
> target/riscv/cpu_cfg.h | 3 ++-
> target/riscv/machine.c | 2 +-
> target/riscv/pmu.c | 14 ++++++++++----
> 4 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index c9d8fc12fe..4d2987e568 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1487,7 +1487,7 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, 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);
> @@ -1812,7 +1812,8 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
>
> static Property riscv_cpu_extensions[] = {
> /* Defaults for standard extensions */
> - DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
> + DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16), /* Deprecated */
> + DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask, MAKE_32BIT_MASK(3, 16)),
> DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false),
> DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 0e6a0f245c..d273487040 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -123,7 +123,8 @@ struct RISCVCPUConfig {
> bool ext_xtheadsync;
> bool ext_XVentanaCondOps;
>
> - uint8_t pmu_num;
> + uint8_t pmu_num; /* Deprecated */
> + 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 360c76f63e..f2d35b4d3b 100644
> --- a/target/riscv/pmu.c
> +++ b/target/riscv/pmu.c
> @@ -18,6 +18,7 @@
>
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> +#include "qemu/error-report.h"
> #include "cpu.h"
> #include "pmu.h"
> #include "sysemu/cpu-timers.h"
> @@ -182,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,7 +433,7 @@ void riscv_pmu_init(RISCVCPU *cpu, Error **errp)
> {
> uint8_t pmu_num = cpu->cfg.pmu_num;
>
> - 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;
> }
> @@ -443,6 +444,11 @@ 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);
> + /* Check if user set it by comparing against default */
> + if (pmu_num != 16) {
> + warn_report("\"pmu-num\" property is deprecated; use \"pmu-mask\"");
> + cpu->cfg.pmu_mask = MAKE_32BIT_MASK(3, pmu_num);
> + }
> +
> + cpu->pmu_avail_ctrs = cpu->cfg.pmu_mask;
How to process the pmu_mask[0:2] no-zero bits? They should not included
into pmu_avail_ctrs.
Zhiwei
> }
On Thu, 2023-10-12 at 17:05 +0800, LIU Zhiwei wrote:
>
>
>
>
> On 2023/10/11 22:45, Rob Bradford 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.
> >
> > Generate a warning if the old property changed from the default but
> > still go ahead and use it to generate the mask if the user has
> > changed
> > it from the default
> >
> > Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> > ---
> > target/riscv/cpu.c | 5 +++--
> > target/riscv/cpu_cfg.h | 3 ++-
> > target/riscv/machine.c | 2 +-
> > target/riscv/pmu.c | 14 ++++++++++----
> > 4 files changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index c9d8fc12fe..4d2987e568 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -1487,7 +1487,7 @@ static void riscv_cpu_realize_tcg(DeviceState
> > *dev, 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);
> > @@ -1812,7 +1812,8 @@ static void
> > riscv_cpu_add_misa_properties(Object *cpu_obj)
> >
> > static Property riscv_cpu_extensions[] = {
> > /* Defaults for standard extensions */
> > - DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
> > + DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16), /*
> > Deprecated */
> > + DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask,
> > MAKE_32BIT_MASK(3, 16)),
> > DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf,
> > false),
> > DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> > DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> > index 0e6a0f245c..d273487040 100644
> > --- a/target/riscv/cpu_cfg.h
> > +++ b/target/riscv/cpu_cfg.h
> > @@ -123,7 +123,8 @@ struct RISCVCPUConfig {
> > bool ext_xtheadsync;
> > bool ext_XVentanaCondOps;
> >
> > - uint8_t pmu_num;
> > + uint8_t pmu_num; /* Deprecated */
> > + 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 360c76f63e..f2d35b4d3b 100644
> > --- a/target/riscv/pmu.c
> > +++ b/target/riscv/pmu.c
> > @@ -18,6 +18,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "qemu/log.h"
> > +#include "qemu/error-report.h"
> > #include "cpu.h"
> > #include "pmu.h"
> > #include "sysemu/cpu-timers.h"
> > @@ -182,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,7 +433,7 @@ void riscv_pmu_init(RISCVCPU *cpu, Error
> > **errp)
> > {
> > uint8_t pmu_num = cpu->cfg.pmu_num;
> >
> > - 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;
> > }
> > @@ -443,6 +444,11 @@ 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);
> > + /* Check if user set it by comparing against default */
> > + if (pmu_num != 16) {
> > + warn_report("\"pmu-num\" property is deprecated; use
> > \"pmu-mask\"");
> > + cpu->cfg.pmu_mask = MAKE_32BIT_MASK(3, pmu_num);
> > + }
> > +
> > + cpu->pmu_avail_ctrs = cpu->cfg.pmu_mask;
> >
>
> How to process the pmu_mask[0:2] no-zero bits? They should not
> included into pmu_avail_ctrs.
>
Good point, thanks Zhiwei. I think rather than masking those bits it is
better to add a check and error out if those bits are set.
Cheers,
Rob
> Zhiwei
>
>
> >
> > }
> >
>
On 2023/10/12 20:38, Rob Bradford wrote:
> On Thu, 2023-10-12 at 17:05 +0800, LIU Zhiwei wrote:
>>
>>
>>
>>
>> On 2023/10/11 22:45, Rob Bradford 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.
>>>
>>> Generate a warning if the old property changed from the default but
>>> still go ahead and use it to generate the mask if the user has
>>> changed
>>> it from the default
>>>
>>> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
>>> ---
>>> target/riscv/cpu.c | 5 +++--
>>> target/riscv/cpu_cfg.h | 3 ++-
>>> target/riscv/machine.c | 2 +-
>>> target/riscv/pmu.c | 14 ++++++++++----
>>> 4 files changed, 16 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> index c9d8fc12fe..4d2987e568 100644
>>> --- a/target/riscv/cpu.c
>>> +++ b/target/riscv/cpu.c
>>> @@ -1487,7 +1487,7 @@ static void riscv_cpu_realize_tcg(DeviceState
>>> *dev, 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);
>>> @@ -1812,7 +1812,8 @@ static void
>>> riscv_cpu_add_misa_properties(Object *cpu_obj)
>>>
>>> static Property riscv_cpu_extensions[] = {
>>> /* Defaults for standard extensions */
>>> - DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
>>> + DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16), /*
>>> Deprecated */
>>> + DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask,
>>> MAKE_32BIT_MASK(3, 16)),
>>> DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf,
>>> false),
>>> DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>>> DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
>>> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
>>> index 0e6a0f245c..d273487040 100644
>>> --- a/target/riscv/cpu_cfg.h
>>> +++ b/target/riscv/cpu_cfg.h
>>> @@ -123,7 +123,8 @@ struct RISCVCPUConfig {
>>> bool ext_xtheadsync;
>>> bool ext_XVentanaCondOps;
>>>
>>> - uint8_t pmu_num;
>>> + uint8_t pmu_num; /* Deprecated */
>>> + 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 360c76f63e..f2d35b4d3b 100644
>>> --- a/target/riscv/pmu.c
>>> +++ b/target/riscv/pmu.c
>>> @@ -18,6 +18,7 @@
>>>
>>> #include "qemu/osdep.h"
>>> #include "qemu/log.h"
>>> +#include "qemu/error-report.h"
>>> #include "cpu.h"
>>> #include "pmu.h"
>>> #include "sysemu/cpu-timers.h"
>>> @@ -182,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,7 +433,7 @@ void riscv_pmu_init(RISCVCPU *cpu, Error
>>> **errp)
>>> {
>>> uint8_t pmu_num = cpu->cfg.pmu_num;
>>>
>>> - 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;
>>> }
>>> @@ -443,6 +444,11 @@ 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);
>>> + /* Check if user set it by comparing against default */
>>> + if (pmu_num != 16) {
>>> + warn_report("\"pmu-num\" property is deprecated; use
>>> \"pmu-mask\"");
>>> + cpu->cfg.pmu_mask = MAKE_32BIT_MASK(3, pmu_num);
>>> + }
>>> +
>>> + cpu->pmu_avail_ctrs = cpu->cfg.pmu_mask;
>>>
>>
>> How to process the pmu_mask[0:2] no-zero bits? They should not
>> included into pmu_avail_ctrs.
>>
> Good point, thanks Zhiwei. I think rather than masking those bits it is
> better to add a check and error out if those bits are set.
Agree.
Thanks,
Zhiwei
>
> Cheers,
>
> Rob
>
>> Zhiwei
>>
>>
>>>
>>> }
>>>
>>
© 2016 - 2026 Red Hat, Inc.