All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
To: Andrew Jones <ajones@ventanamicro.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
	alistair.francis@wdc.com, bmeng@tinylab.org,
	liweiwei@iscas.ac.cn, zhiwei_liu@linux.alibaba.com,
	palmer@rivosinc.com, philmd@linaro.org
Subject: Re: [PATCH v6 11/20] target/riscv/cpu: add misa_ext_info_arr[]
Date: Thu, 29 Jun 2023 08:40:29 -0300	[thread overview]
Message-ID: <301b09a1-11dd-3413-15d3-c7bdd1f81dde@ventanamicro.com> (raw)
In-Reply-To: <20230629-6a932f8d5f71fefc9afbd974@orel>



On 6/29/23 05:59, Andrew Jones wrote:
> On Wed, Jun 28, 2023 at 06:30:24PM -0300, 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 <ajones@ventanamicro.com>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@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}
> 
> We don't have to give up on passing RV* to this macro. Directly
> using __builtin_ctz() with a constant should work, i.e.
> 
>   #define MISA_EXT_INFO(_bit, _propname, _descr) \
>       [__builtin_ctz(_bit)] = {.name = _propname, .description = _descr}
> 
> and then
> 
>   MISA_EXT_INFO(RVA, "a", "Atomic instructions"),
>   MISA_EXT_INFO(RVD, "d", "Double-precision float point"),
>   ...
> 
> (We don't need the ctz32() wrapper since we know we'll never input zero to
> __builtin_ctz().)

I'll give it a try. I'll spare us from having to assign name and descr during
runtime (at least for this file).

Daniel

> 
>> +
>> +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)
>> +{
>> +    return misa_ext_info_arr[ctz32(bit)].name;
>> +}
>> +
>> +const char *riscv_get_misa_ext_descr(uint32_t bit)
> 
> nit: I'd prefer riscv_get_misa_ext_description() for the name.
> 
>> +{
>> +    return misa_ext_info_arr[ctz32(bit)].description;
>> +}
>> +
>> +#define MISA_CFG(_bit, _enabled) \
>> +    {.misa_bit = _bit, .enabled = _enabled}
>> +
>> +static RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
>> +    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];
>> +
>> +        misa_cfg->name = riscv_get_misa_ext_name(misa_cfg->misa_bit);
>> +        misa_cfg->description = riscv_get_misa_ext_descr(misa_cfg->misa_bit);
> 
> This might be necessary for the KVM side, but we should be able to avoid
> this runtime setting here where the compiler can be certain everything is
> a constant. We just need the old MISA_CFG() back, slightly tweaked to use
> __builtin_ctz().
> 
>>   
>>           object_property_add(cpu_obj, misa_cfg->name, "bool",
>>                               cpu_get_misa_ext_cfg,
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index cc20ee25a7..acae118b9b 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -41,7 +41,10 @@
>>   
>>   #define RV(x) ((target_ulong)1 << (x - 'A'))
>>   
>> -/* Consider updating misa_ext_cfgs[] when adding new MISA bits here */
>> +/*
>> + * Consider updating misa_ext_info_arr[] and misa_ext_cfgs[]
>> + * when adding new MISA bits here.
>> + */
>>   #define RVI RV('I')
>>   #define RVE RV('E') /* E and I are mutually exclusive */
>>   #define RVM RV('M')
>> @@ -56,6 +59,8 @@
>>   #define RVJ RV('J')
>>   #define RVG RV('G')
>>   
>> +const char *riscv_get_misa_ext_name(uint32_t bit);
>> +const char *riscv_get_misa_ext_descr(uint32_t bit);
>>   
>>   /* Privileged specification version */
>>   enum {
>> -- 
>> 2.41.0
>>
> 
> Thanks,
> drew


  reply	other threads:[~2023-06-29 11:41 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-28 21:30 [PATCH v6 00/20] target/riscv, KVM: fixes and enhancements Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 01/20] target/riscv: skip features setup for KVM CPUs Daniel Henrique Barboza
2023-06-29  8:24   ` Andrew Jones
2023-06-28 21:30 ` [PATCH v6 02/20] hw/riscv/virt.c: skip 'mmu-type' FDT if satp mode not set Daniel Henrique Barboza
2023-06-30  7:36   ` Michael Tokarev
2023-06-30  7:46     ` Daniel Henrique Barboza
2023-06-30  7:51       ` Michael Tokarev
2023-06-28 21:30 ` [PATCH v6 03/20] target/riscv/cpu.c: restrict 'mvendorid' value Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 04/20] target/riscv/cpu.c: restrict 'mimpid' value Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 05/20] target/riscv/cpu.c: restrict 'marchid' value Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 06/20] target/riscv: use KVM scratch CPUs to init KVM properties Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 07/20] target/riscv: read marchid/mimpid in kvm_riscv_init_machine_ids() Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 08/20] target/riscv: handle mvendorid/marchid/mimpid for KVM CPUs Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 09/20] linux-headers: Update to v6.4-rc1 Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 10/20] target/riscv/kvm.c: init 'misa_ext_mask' with scratch CPU Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 11/20] target/riscv/cpu: add misa_ext_info_arr[] Daniel Henrique Barboza
2023-06-29  7:26   ` Philippe Mathieu-Daudé
2023-06-29 11:36     ` Daniel Henrique Barboza
2023-06-29 11:43       ` Daniel Henrique Barboza
2023-06-29  8:59   ` Andrew Jones
2023-06-29 11:40     ` Daniel Henrique Barboza [this message]
2023-06-29 22:04     ` Daniel Henrique Barboza
2023-06-30  7:21       ` Andrew Jones
2023-06-28 21:30 ` [PATCH v6 12/20] target/riscv: add KVM specific MISA properties Daniel Henrique Barboza
2023-06-29  9:12   ` Andrew Jones
2023-06-29 11:50     ` Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 13/20] target/riscv/kvm.c: update KVM MISA bits Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 14/20] target/riscv/kvm.c: add multi-letter extension KVM properties Daniel Henrique Barboza
2023-06-29  9:14   ` Andrew Jones
2023-06-28 21:30 ` [PATCH v6 15/20] target/riscv/cpu.c: add satp_mode properties earlier Daniel Henrique Barboza
2023-06-29  7:30   ` Philippe Mathieu-Daudé
2023-06-29  9:15   ` Andrew Jones
2023-06-28 21:30 ` [PATCH v6 16/20] target/riscv/cpu.c: remove priv_ver check from riscv_isa_string_ext() Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 17/20] target/riscv/cpu.c: create KVM mock properties Daniel Henrique Barboza
2023-06-29  9:17   ` Andrew Jones
2023-06-28 21:30 ` [PATCH v6 18/20] target/riscv: update multi-letter extension KVM properties Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 19/20] target/riscv/kvm.c: add kvmconfig_get_cfg_addr() helper Daniel Henrique Barboza
2023-06-28 21:30 ` [PATCH v6 20/20] target/riscv/kvm.c: read/write (cbom|cboz)_blocksize in KVM Daniel Henrique Barboza

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=301b09a1-11dd-3413-15d3-c7bdd1f81dde@ventanamicro.com \
    --to=dbarboza@ventanamicro.com \
    --cc=ajones@ventanamicro.com \
    --cc=alistair.francis@wdc.com \
    --cc=bmeng@tinylab.org \
    --cc=liweiwei@iscas.ac.cn \
    --cc=palmer@rivosinc.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=zhiwei_liu@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.