On 6/29/23 08:36, Daniel Henrique Barboza wrote:


On 6/29/23 04:26, Philippe Mathieu-Daudé wrote:
On 28/6/23 23:30, Daniel Henrique Barboza wrote:
Next patch will add KVM specific user properties for both MISA and
multi-letter extensions. For MISA extensions we want to make use of what
is already available in misa_ext_cfgs[] to avoid code repetition.

misa_ext_info_arr[] array will hold name and description for each MISA
extension that misa_ext_cfgs[] is declaring. We'll then use this new
array in KVM code to avoid duplicating strings.

There's nothing holding us back from doing the same with multi-letter
extensions. For now doing just with MISA extensions is enough.

Suggested-by: Andrew Jones <ajo...@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
  target/riscv/cpu.c | 83 ++++++++++++++++++++++++++++++----------------
  target/riscv/cpu.h |  7 +++-
  2 files changed, 61 insertions(+), 29 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 2485e820f8..90dd2078ae 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1558,33 +1558,57 @@ static void cpu_get_misa_ext_cfg(Object *obj, Visitor 
*v, const char *name,
      visit_type_bool(v, name, &value, errp);
  }
-static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
-    {.name = "a", .description = "Atomic instructions",
-     .misa_bit = RVA, .enabled = true},
-    {.name = "c", .description = "Compressed instructions",
-     .misa_bit = RVC, .enabled = true},
-    {.name = "d", .description = "Double-precision float point",
-     .misa_bit = RVD, .enabled = true},
-    {.name = "f", .description = "Single-precision float point",
-     .misa_bit = RVF, .enabled = true},
-    {.name = "i", .description = "Base integer instruction set",
-     .misa_bit = RVI, .enabled = true},
-    {.name = "e", .description = "Base integer instruction set (embedded)",
-     .misa_bit = RVE, .enabled = false},
-    {.name = "m", .description = "Integer multiplication and division",
-     .misa_bit = RVM, .enabled = true},
-    {.name = "s", .description = "Supervisor-level instructions",
-     .misa_bit = RVS, .enabled = true},
-    {.name = "u", .description = "User-level instructions",
-     .misa_bit = RVU, .enabled = true},
-    {.name = "h", .description = "Hypervisor",
-     .misa_bit = RVH, .enabled = true},
-    {.name = "x-j", .description = "Dynamic translated languages",
-     .misa_bit = RVJ, .enabled = false},
-    {.name = "v", .description = "Vector operations",
-     .misa_bit = RVV, .enabled = false},
-    {.name = "g", .description = "General purpose (IMAFD_Zicsr_Zifencei)",
-     .misa_bit = RVG, .enabled = false},
+typedef struct misa_ext_info {
+    const char *name;
+    const char *description;
+} MISAExtInfo;
+
+#define MISA_EXT_INFO(_idx, _propname, _descr) \
+    [(_idx - 'A')] = {.name = _propname, .description = _descr}
+
+static const MISAExtInfo misa_ext_info_arr[] = {
+    MISA_EXT_INFO('A', "a", "Atomic instructions"),
+    MISA_EXT_INFO('C', "c", "Compressed instructions"),
+    MISA_EXT_INFO('D', "d", "Double-precision float point"),
+    MISA_EXT_INFO('F', "f", "Single-precision float point"),
+    MISA_EXT_INFO('I', "i", "Base integer instruction set"),
+    MISA_EXT_INFO('E', "e", "Base integer instruction set (embedded)"),
+    MISA_EXT_INFO('M', "m", "Integer multiplication and division"),
+    MISA_EXT_INFO('S', "s", "Supervisor-level instructions"),
+    MISA_EXT_INFO('U', "u", "User-level instructions"),
+    MISA_EXT_INFO('H', "h", "Hypervisor"),
+    MISA_EXT_INFO('J', "x-j", "Dynamic translated languages"),
+    MISA_EXT_INFO('V', "v", "Vector operations"),
+    MISA_EXT_INFO('G', "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
+};
+
+const char *riscv_get_misa_ext_name(uint32_t bit)
+{

Is that OK to return NULL, or should we assert for
unimplemented bit/feature?

It's easier to assert out if name or description is NULL (meaning that we don't
implement the bit).



+    return misa_ext_info_arr[ctz32(bit)].name;
+}
+
+const char *riscv_get_misa_ext_descr(uint32_t bit)
+{
+    return misa_ext_info_arr[ctz32(bit)].description;

Ditto.

+}
+
+#define MISA_CFG(_bit, _enabled) \
+    {.misa_bit = _bit, .enabled = _enabled}
+
+static RISCVCPUMisaExtConfig misa_ext_cfgs[] = {

'const'

Not sure why I got rid of 'const' here. I'll reintroduce it.

Just remembered why. 'name' and 'description' are being initialized during 
runtime, so
the array can't be 'const'.

If we managed to init everything in the macro like Drew suggested we can keep 
it 'const'.


Daniel



Daniel


+    MISA_CFG(RVA, true),
+    MISA_CFG(RVC, true),
+    MISA_CFG(RVD, true),
+    MISA_CFG(RVF, true),
+    MISA_CFG(RVI, true),
+    MISA_CFG(RVE, false),
+    MISA_CFG(RVM, true),
+    MISA_CFG(RVS, true),
+    MISA_CFG(RVU, true),
+    MISA_CFG(RVH, true),
+    MISA_CFG(RVJ, false),
+    MISA_CFG(RVV, false),
+    MISA_CFG(RVG, false),
  };
  static void riscv_cpu_add_misa_properties(Object *cpu_obj)
@@ -1592,7 +1616,10 @@ static void riscv_cpu_add_misa_properties(Object 
*cpu_obj)
      int i;
      for (i = 0; i < ARRAY_SIZE(misa_ext_cfgs); i++) {
-        const RISCVCPUMisaExtConfig *misa_cfg = &misa_ext_cfgs[i];
+        RISCVCPUMisaExtConfig *misa_cfg = &misa_ext_cfgs[i];

const

+
+        misa_cfg->name = riscv_get_misa_ext_name(misa_cfg->misa_bit);
+        misa_cfg->description = riscv_get_misa_ext_descr(misa_cfg->misa_bit);

Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>


Reply via email to