Re: [PATCH v2 3/9] target/riscv/kvm: turn u32/u64 reg functions in macros

[Andrew Jones <ajones@ventanamicro.com>]

sed s/in/into/ <<<$SUBJECT

On Fri, Apr 25, 2025 at 08:36:59AM -0300, Daniel Henrique Barboza wrote:
> This change is motivated by a future change w.r.t CSRs management. We
> want to handle them the same way as KVM extensions, i.e. a static array
> with KVMCPUConfig objs that will be read/write during init and so on.
> But to do that properly we must be able to declare a static array that
> hold KVM regs.
> 
> C does not allow to init static arrays and use functions as
> initializers, e.g. we can't do:
> 
> .kvm_reg_id = kvm_riscv_reg_id_ulong(...)
> 
> When instantiating the array. We can do that with macros though, so our
> goal is turn kvm_riscv_reg_ulong() in a macro. It is cleaner to turn
> every other reg_id_*() function in macros, and ulong will end up using
> the macros for u32 and u64, so we'll start with them.
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/kvm/kvm-cpu.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 6ba122f360..c91ecdfe59 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -58,6 +58,12 @@ void riscv_kvm_aplic_request(void *opaque, int irq, int level)
>  
>  static bool cap_has_mp_state;
>  
> +#define KVM_RISCV_REG_ID_U32(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U32 | \
> +                                         type | idx)
> +
> +#define KVM_RISCV_REG_ID_U64(type, idx) (KVM_REG_RISCV | KVM_REG_SIZE_U64 | \
> +                                         type | idx)
> +
>  static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
>                                   uint64_t idx)
>  {
> @@ -76,16 +82,6 @@ static uint64_t kvm_riscv_reg_id_ulong(CPURISCVState *env, uint64_t type,
>      return id;
>  }
>  
> -static uint64_t kvm_riscv_reg_id_u32(uint64_t type, uint64_t idx)
> -{
> -    return KVM_REG_RISCV | KVM_REG_SIZE_U32 | type | idx;
> -}
> -
> -static uint64_t kvm_riscv_reg_id_u64(uint64_t type, uint64_t idx)
> -{
> -    return KVM_REG_RISCV | KVM_REG_SIZE_U64 | type | idx;
> -}
> -
>  static uint64_t kvm_encode_reg_size_id(uint64_t id, size_t size_b)
>  {
>      uint64_t size_ctz = __builtin_ctz(size_b);
> @@ -119,12 +115,12 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
>      kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG, \
>                             KVM_REG_RISCV_CONFIG_REG(name))
>  
> -#define RISCV_TIMER_REG(name)  kvm_riscv_reg_id_u64(KVM_REG_RISCV_TIMER, \
> +#define RISCV_TIMER_REG(name)  KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_TIMER, \
>                   KVM_REG_RISCV_TIMER_REG(name))
>  
> -#define RISCV_FP_F_REG(idx)  kvm_riscv_reg_id_u32(KVM_REG_RISCV_FP_F, idx)
> +#define RISCV_FP_F_REG(idx)  KVM_RISCV_REG_ID_U32(KVM_REG_RISCV_FP_F, idx)
>  
> -#define RISCV_FP_D_REG(idx)  kvm_riscv_reg_id_u64(KVM_REG_RISCV_FP_D, idx)
> +#define RISCV_FP_D_REG(idx)  KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_FP_D, idx)
>  
>  #define RISCV_VECTOR_CSR_REG(env, name) \
>      kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_VECTOR, \
> -- 
> 2.49.0
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>