diff mbox series

[v3,03/16] target/riscv: move 'pmu-mask' and 'pmu-num' to riscv_cpu_properties[]

Message ID 20240103174013.147279-4-dbarboza@ventanamicro.com (mailing list archive)
State New, archived
Headers show
Series target/riscv: deprecate riscv_cpu_options[] | expand

Commit Message

Daniel Henrique Barboza Jan. 3, 2024, 5:40 p.m. UTC 
Every property in riscv_cpu_options[] will be migrated to
riscv_cpu_properties[]. This will make their default values init
earlier, allowing cpu_init() functions to overwrite them. We'll also
implement common getters and setters that both accelerators will use,
allowing them to share validations that TCG is doing.

At the same time, some options (namely 'vlen', 'elen' and the cache
blocksizes) need a way of tracking if the user set a value for them.
This is benign for TCG since the cost of always validating these values
are small, but for KVM we need syscalls to read the host values to make
the validations, thus knowing whether the user didn't touch the values
makes a difference.

We'll track user setting for these properties using a hash, like we do
in the TCG driver. All riscv cpu options will update this hash in case
the user sets it. The KVM driver will use this hash to minimize the
amount of syscalls done.

For now, both 'pmu-mask' and 'pmu-num' shouldn't be changed for vendor
CPUs. The existing setter for 'pmu-num' is changed to add this
restriction. New getters and setters are required for 'pmu-mask'

While we're at it, add a 'static' modifier to 'prop_pmu_num' since we're
not exporting it.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/cpu.c | 96 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 89 insertions(+), 7 deletions(-)

Comments

Alistair Francis Jan. 5, 2024, 3:55 a.m. UTC | #1 
On Thu, Jan 4, 2024 at 5:05 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Every property in riscv_cpu_options[] will be migrated to
> riscv_cpu_properties[]. This will make their default values init
> earlier, allowing cpu_init() functions to overwrite them. We'll also
> implement common getters and setters that both accelerators will use,
> allowing them to share validations that TCG is doing.
>
> At the same time, some options (namely 'vlen', 'elen' and the cache
> blocksizes) need a way of tracking if the user set a value for them.
> This is benign for TCG since the cost of always validating these values
> are small, but for KVM we need syscalls to read the host values to make
> the validations, thus knowing whether the user didn't touch the values
> makes a difference.
>
> We'll track user setting for these properties using a hash, like we do
> in the TCG driver. All riscv cpu options will update this hash in case
> the user sets it. The KVM driver will use this hash to minimize the
> amount of syscalls done.
>
> For now, both 'pmu-mask' and 'pmu-num' shouldn't be changed for vendor
> CPUs. The existing setter for 'pmu-num' is changed to add this
> restriction. New getters and setters are required for 'pmu-mask'
>
> While we're at it, add a 'static' modifier to 'prop_pmu_num' since we're
> not exporting it.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 96 ++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 89 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 65cfa6c740..e90b70c0a7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -53,6 +53,15 @@ const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
>  #define BYTE(x)   (x)
>  #endif
>
> +/* Hash that stores general user set numeric options */
> +static GHashTable *general_user_opts;
> +
> +static void cpu_option_add_user_setting(const char *optname, uint32_t value)
> +{
> +    g_hash_table_insert(general_user_opts, (gpointer)optname,
> +                        GUINT_TO_POINTER(value));
> +}
> +
>  #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \
>      {#_name, _min_ver, CPU_CFG_OFFSET(_prop)}
>
> @@ -1218,11 +1227,15 @@ static void riscv_cpu_post_init(Object *obj)
>
>  static void riscv_cpu_init(Object *obj)
>  {
> +    RISCVCPU *cpu = RISCV_CPU(obj);
> +
>  #ifndef CONFIG_USER_ONLY
>      qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq,
>                        IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
>  #endif /* CONFIG_USER_ONLY */
>
> +    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
> +
>      /*
>       * The timer and performance counters extensions were supported
>       * in QEMU before they were added as discrete extensions in the
> @@ -1232,6 +1245,9 @@ static void riscv_cpu_init(Object *obj)
>       */
>      RISCV_CPU(obj)->cfg.ext_zicntr = true;
>      RISCV_CPU(obj)->cfg.ext_zihpm = true;
> +
> +    /* Default values for non-bool cpu properties */
> +    cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, 16);
>  }
>
>  typedef struct misa_ext_info {
> @@ -1431,26 +1447,51 @@ const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> +static bool riscv_cpu_is_vendor(Object *obj)
> +{
> +    return !riscv_cpu_is_generic(obj);
> +}
> +
> +static void cpu_set_prop_err(RISCVCPU *cpu, const char *propname,
> +                             Error **errp)
> +{
> +    g_autofree char *cpuname = riscv_cpu_get_name(cpu);
> +    error_setg(errp, "CPU '%s' does not allow changing the value of '%s'",
> +               cpuname, propname);
> +}
> +
>  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;
> +    uint8_t pmu_num, curr_pmu_num;
> +    uint32_t pmu_mask;
>
>      visit_type_uint8(v, name, &pmu_num, errp);
>
> +    curr_pmu_num = ctpop32(cpu->cfg.pmu_mask);
> +
> +    if (pmu_num != curr_pmu_num && riscv_cpu_is_vendor(obj)) {
> +        cpu_set_prop_err(cpu, name, errp);
> +        error_append_hint(errp, "Current '%s' val: %u\n",
> +                          name, curr_pmu_num);
> +        return;
> +    }
> +
>      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;
> +        pmu_mask = 0;
>      } else {
> -        cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, pmu_num);
> +        pmu_mask = MAKE_64BIT_MASK(3, pmu_num);
>      }
>
>      warn_report("\"pmu-num\" property is deprecated; use \"pmu-mask\"");
> +    cpu->cfg.pmu_mask = pmu_mask;
> +    cpu_option_add_user_setting("pmu-mask", pmu_mask);
>  }
>
>  static void prop_pmu_num_get(Object *obj, Visitor *v, const char *name,
> @@ -1462,16 +1503,54 @@ static void prop_pmu_num_get(Object *obj, Visitor *v, const char *name,
>      visit_type_uint8(v, name, &pmu_num, errp);
>  }
>
> -const PropertyInfo prop_pmu_num = {
> +static const PropertyInfo prop_pmu_num = {
>      .name = "pmu-num",
>      .get = prop_pmu_num_get,
>      .set = prop_pmu_num_set,
>  };
>
> -Property riscv_cpu_options[] = {
> -    DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask, MAKE_64BIT_MASK(3, 16)),
> -    {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
> +static void prop_pmu_mask_set(Object *obj, Visitor *v, const char *name,
> +                             void *opaque, Error **errp)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(obj);
> +    uint32_t value;
> +    uint8_t pmu_num;
> +
> +    visit_type_uint32(v, name, &value, errp);
> +
> +    if (value != cpu->cfg.pmu_mask && riscv_cpu_is_vendor(obj)) {
> +        cpu_set_prop_err(cpu, name, errp);
> +        error_append_hint(errp, "Current '%s' val: %x\n",
> +                          name, cpu->cfg.pmu_mask);
> +        return;
> +    }
> +
> +    pmu_num = ctpop32(value);
> +
> +    if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) {
> +        error_setg(errp, "Number of counters exceeds maximum available");
> +        return;
> +    }
>
> +    cpu_option_add_user_setting(name, value);
> +    cpu->cfg.pmu_mask = value;
> +}
> +
> +static void prop_pmu_mask_get(Object *obj, Visitor *v, const char *name,
> +                             void *opaque, Error **errp)
> +{
> +    uint8_t pmu_mask = RISCV_CPU(obj)->cfg.pmu_mask;
> +
> +    visit_type_uint8(v, name, &pmu_mask, errp);
> +}
> +
> +static const PropertyInfo prop_pmu_mask = {
> +    .name = "pmu-mask",
> +    .get = prop_pmu_mask_get,
> +    .set = prop_pmu_mask_set,
> +};
> +
> +Property riscv_cpu_options[] = {
>      DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
>      DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
>
> @@ -1490,6 +1569,9 @@ Property riscv_cpu_options[] = {
>  static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
>
> +    {.name = "pmu-mask", .info = &prop_pmu_mask},
> +    {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
> +
>  #ifndef CONFIG_USER_ONLY
>      DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
>  #endif
> --
> 2.43.0
>
>
diff mbox series

Patch

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 65cfa6c740..e90b70c0a7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -53,6 +53,15 @@  const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
 #define BYTE(x)   (x)
 #endif
 
+/* Hash that stores general user set numeric options */
+static GHashTable *general_user_opts;
+
+static void cpu_option_add_user_setting(const char *optname, uint32_t value)
+{
+    g_hash_table_insert(general_user_opts, (gpointer)optname,
+                        GUINT_TO_POINTER(value));
+}
+
 #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \
     {#_name, _min_ver, CPU_CFG_OFFSET(_prop)}
 
@@ -1218,11 +1227,15 @@  static void riscv_cpu_post_init(Object *obj)
 
 static void riscv_cpu_init(Object *obj)
 {
+    RISCVCPU *cpu = RISCV_CPU(obj);
+
 #ifndef CONFIG_USER_ONLY
     qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq,
                       IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
 #endif /* CONFIG_USER_ONLY */
 
+    general_user_opts = g_hash_table_new(g_str_hash, g_str_equal);
+
     /*
      * The timer and performance counters extensions were supported
      * in QEMU before they were added as discrete extensions in the
@@ -1232,6 +1245,9 @@  static void riscv_cpu_init(Object *obj)
      */
     RISCV_CPU(obj)->cfg.ext_zicntr = true;
     RISCV_CPU(obj)->cfg.ext_zihpm = true;
+
+    /* Default values for non-bool cpu properties */
+    cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, 16);
 }
 
 typedef struct misa_ext_info {
@@ -1431,26 +1447,51 @@  const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static bool riscv_cpu_is_vendor(Object *obj)
+{
+    return !riscv_cpu_is_generic(obj);
+}
+
+static void cpu_set_prop_err(RISCVCPU *cpu, const char *propname,
+                             Error **errp)
+{
+    g_autofree char *cpuname = riscv_cpu_get_name(cpu);
+    error_setg(errp, "CPU '%s' does not allow changing the value of '%s'",
+               cpuname, propname);
+}
+
 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;
+    uint8_t pmu_num, curr_pmu_num;
+    uint32_t pmu_mask;
 
     visit_type_uint8(v, name, &pmu_num, errp);
 
+    curr_pmu_num = ctpop32(cpu->cfg.pmu_mask);
+
+    if (pmu_num != curr_pmu_num && riscv_cpu_is_vendor(obj)) {
+        cpu_set_prop_err(cpu, name, errp);
+        error_append_hint(errp, "Current '%s' val: %u\n",
+                          name, curr_pmu_num);
+        return;
+    }
+
     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;
+        pmu_mask = 0;
     } else {
-        cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, pmu_num);
+        pmu_mask = MAKE_64BIT_MASK(3, pmu_num);
     }
 
     warn_report("\"pmu-num\" property is deprecated; use \"pmu-mask\"");
+    cpu->cfg.pmu_mask = pmu_mask;
+    cpu_option_add_user_setting("pmu-mask", pmu_mask);
 }
 
 static void prop_pmu_num_get(Object *obj, Visitor *v, const char *name,
@@ -1462,16 +1503,54 @@  static void prop_pmu_num_get(Object *obj, Visitor *v, const char *name,
     visit_type_uint8(v, name, &pmu_num, errp);
 }
 
-const PropertyInfo prop_pmu_num = {
+static const PropertyInfo prop_pmu_num = {
     .name = "pmu-num",
     .get = prop_pmu_num_get,
     .set = prop_pmu_num_set,
 };
 
-Property riscv_cpu_options[] = {
-    DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask, MAKE_64BIT_MASK(3, 16)),
-    {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
+static void prop_pmu_mask_set(Object *obj, Visitor *v, const char *name,
+                             void *opaque, Error **errp)
+{
+    RISCVCPU *cpu = RISCV_CPU(obj);
+    uint32_t value;
+    uint8_t pmu_num;
+
+    visit_type_uint32(v, name, &value, errp);
+
+    if (value != cpu->cfg.pmu_mask && riscv_cpu_is_vendor(obj)) {
+        cpu_set_prop_err(cpu, name, errp);
+        error_append_hint(errp, "Current '%s' val: %x\n",
+                          name, cpu->cfg.pmu_mask);
+        return;
+    }
+
+    pmu_num = ctpop32(value);
+
+    if (pmu_num > (RV_MAX_MHPMCOUNTERS - 3)) {
+        error_setg(errp, "Number of counters exceeds maximum available");
+        return;
+    }
 
+    cpu_option_add_user_setting(name, value);
+    cpu->cfg.pmu_mask = value;
+}
+
+static void prop_pmu_mask_get(Object *obj, Visitor *v, const char *name,
+                             void *opaque, Error **errp)
+{
+    uint8_t pmu_mask = RISCV_CPU(obj)->cfg.pmu_mask;
+
+    visit_type_uint8(v, name, &pmu_mask, errp);
+}
+
+static const PropertyInfo prop_pmu_mask = {
+    .name = "pmu-mask",
+    .get = prop_pmu_mask_get,
+    .set = prop_pmu_mask_set,
+};
+
+Property riscv_cpu_options[] = {
     DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
 
@@ -1490,6 +1569,9 @@  Property riscv_cpu_options[] = {
 static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
 
+    {.name = "pmu-mask", .info = &prop_pmu_mask},
+    {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
+
 #ifndef CONFIG_USER_ONLY
     DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
 #endif