From: Andrew Jones <ajones@ventanamicro.com>
To: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
alistair.francis@wdc.com, liwei1518@gmail.com,
zhiwei_liu@linux.alibaba.com, palmer@rivosinc.com
Subject: Re: [PATCH v2 4/9] target/riscv/kvm: turn kvm_riscv_reg_id_ulong() in a macro
Date: Fri, 25 Apr 2025 13:48:08 +0200 [thread overview]
Message-ID: <20250425-d623af137e71843fa8d571d5@orel> (raw)
In-Reply-To: <20250425113705.2741457-5-dbarboza@ventanamicro.com>
s/in/into/ <<<$SUBJECT
On Fri, Apr 25, 2025 at 08:37:00AM -0300, Daniel Henrique Barboza wrote:
> We need the reg_id_ulong() helper to be a macro to be able to create a
> static array of KVMCPUConfig that will hold CSR information.
>
> Despite the amount of changes all of them are tedious/trivial:
>
> - replace instances of "kvm_riscv_reg_id_ulong" with
> "KVM_RISCV_REG_ID_ULONG";
>
> - RISCV_CORE_REG(), RISCV_CSR_REG(), RISCV_CONFIG_REG() and
> RISCV_VECTOR_CSR_REG() only receives one 'name' arg. Remove unneeded
> 'env' variables when applicable.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/kvm/kvm-cpu.c | 99 ++++++++++++++++----------------------
> 1 file changed, 41 insertions(+), 58 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index c91ecdfe59..fd66bc1759 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -64,23 +64,11 @@ static bool cap_has_mp_state;
> #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)
> -{
> - uint64_t id = KVM_REG_RISCV | type | idx;
> -
> - switch (riscv_cpu_mxl(env)) {
> - case MXL_RV32:
> - id |= KVM_REG_SIZE_U32;
> - break;
> - case MXL_RV64:
> - id |= KVM_REG_SIZE_U64;
> - break;
> - default:
> - g_assert_not_reached();
> - }
> - return id;
> -}
> +#if defined(TARGET_RISCV64)
> +#define KVM_RISCV_REG_ID_ULONG(type, idx) KVM_RISCV_REG_ID_U64(type, idx)
> +#else
> +#define KVM_RISCV_REG_ID_ULONG(type, idx) KVM_RISCV_REG_ID_U32(type, idx)
> +#endif
>
> static uint64_t kvm_encode_reg_size_id(uint64_t id, size_t size_b)
> {
> @@ -103,16 +91,16 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
> return kvm_encode_reg_size_id(id, size_b);
> }
>
> -#define RISCV_CORE_REG(env, name) \
> - kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, \
> +#define RISCV_CORE_REG(name) \
> + KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, \
> KVM_REG_RISCV_CORE_REG(name))
>
> -#define RISCV_CSR_REG(env, name) \
> - kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CSR, \
> +#define RISCV_CSR_REG(name) \
> + KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CSR, \
> KVM_REG_RISCV_CSR_REG(name))
>
> -#define RISCV_CONFIG_REG(env, name) \
> - kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG, \
> +#define RISCV_CONFIG_REG(name) \
> + KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG, \
> KVM_REG_RISCV_CONFIG_REG(name))
>
> #define RISCV_TIMER_REG(name) KVM_RISCV_REG_ID_U64(KVM_REG_RISCV_TIMER, \
> @@ -122,13 +110,13 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
>
> #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, \
> +#define RISCV_VECTOR_CSR_REG(name) \
> + KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_VECTOR, \
> KVM_REG_RISCV_VECTOR_CSR_REG(name))
>
> #define KVM_RISCV_GET_CSR(cs, env, csr, reg) \
> do { \
> - int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, csr), ®); \
> + int _ret = kvm_get_one_reg(cs, RISCV_CSR_REG(csr), ®); \
> if (_ret) { \
> return _ret; \
> } \
> @@ -136,7 +124,7 @@ static uint64_t kvm_riscv_vector_reg_id(RISCVCPU *cpu,
>
> #define KVM_RISCV_SET_CSR(cs, env, csr, reg) \
> do { \
> - int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, csr), ®); \
> + int _ret = kvm_set_one_reg(cs, RISCV_CSR_REG(csr), ®); \
> if (_ret) { \
> return _ret; \
> } \
> @@ -244,7 +232,7 @@ static void kvm_riscv_update_cpu_misa_ext(RISCVCPU *cpu, CPUState *cs)
>
> /* If we're here we're going to disable the MISA bit */
> reg = 0;
> - id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
> + id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
> misa_cfg->kvm_reg_id);
> ret = kvm_set_one_reg(cs, id, ®);
> if (ret != 0) {
> @@ -430,7 +418,6 @@ static KVMCPUConfig kvm_sbi_dbcn = {
>
> static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
> {
> - CPURISCVState *env = &cpu->env;
> uint64_t id, reg;
> int i, ret;
>
> @@ -441,7 +428,7 @@ static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
> continue;
> }
>
> - id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
> + id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
> multi_ext_cfg->kvm_reg_id);
> reg = kvm_cpu_cfg_get(cpu, multi_ext_cfg);
> ret = kvm_set_one_reg(cs, id, ®);
> @@ -566,14 +553,14 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
> target_ulong reg;
> CPURISCVState *env = &RISCV_CPU(cs)->env;
>
> - ret = kvm_get_one_reg(cs, RISCV_CORE_REG(env, regs.pc), ®);
> + ret = kvm_get_one_reg(cs, RISCV_CORE_REG(regs.pc), ®);
> if (ret) {
> return ret;
> }
> env->pc = reg;
>
> for (i = 1; i < 32; i++) {
> - uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> + uint64_t id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, i);
> ret = kvm_get_one_reg(cs, id, ®);
> if (ret) {
> return ret;
> @@ -592,13 +579,13 @@ static int kvm_riscv_put_regs_core(CPUState *cs)
> CPURISCVState *env = &RISCV_CPU(cs)->env;
>
> reg = env->pc;
> - ret = kvm_set_one_reg(cs, RISCV_CORE_REG(env, regs.pc), ®);
> + ret = kvm_set_one_reg(cs, RISCV_CORE_REG(regs.pc), ®);
> if (ret) {
> return ret;
> }
>
> for (i = 1; i < 32; i++) {
> - uint64_t id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CORE, i);
> + uint64_t id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CORE, i);
> reg = env->gpr[i];
> ret = kvm_set_one_reg(cs, id, ®);
> if (ret) {
> @@ -796,26 +783,26 @@ static int kvm_riscv_get_regs_vector(CPUState *cs)
> return 0;
> }
>
> - ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vstart), ®);
> + ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vstart), ®);
> if (ret) {
> return ret;
> }
> env->vstart = reg;
>
> - ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vl), ®);
> + ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vl), ®);
> if (ret) {
> return ret;
> }
> env->vl = reg;
>
> - ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vtype), ®);
> + ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vtype), ®);
> if (ret) {
> return ret;
> }
> env->vtype = reg;
>
> if (kvm_v_vlenb.supported) {
> - ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vlenb), ®);
> + ret = kvm_get_one_reg(cs, RISCV_VECTOR_CSR_REG(vlenb), ®);
> if (ret) {
> return ret;
> }
> @@ -853,26 +840,26 @@ static int kvm_riscv_put_regs_vector(CPUState *cs)
> }
>
> reg = env->vstart;
> - ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vstart), ®);
> + ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vstart), ®);
> if (ret) {
> return ret;
> }
>
> reg = env->vl;
> - ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vl), ®);
> + ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vl), ®);
> if (ret) {
> return ret;
> }
>
> reg = env->vtype;
> - ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vtype), ®);
> + ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vtype), ®);
> if (ret) {
> return ret;
> }
>
> if (kvm_v_vlenb.supported) {
> reg = cpu->cfg.vlenb;
> - ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(env, vlenb), ®);
> + ret = kvm_set_one_reg(cs, RISCV_VECTOR_CSR_REG(vlenb), ®);
>
> for (int i = 0; i < 32; i++) {
> /*
> @@ -951,25 +938,24 @@ static void kvm_riscv_destroy_scratch_vcpu(KVMScratchCPU *scratch)
>
> static void kvm_riscv_init_machine_ids(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
> {
> - CPURISCVState *env = &cpu->env;
> struct kvm_one_reg reg;
> int ret;
>
> - reg.id = RISCV_CONFIG_REG(env, mvendorid);
> + reg.id = RISCV_CONFIG_REG(mvendorid);
> reg.addr = (uint64_t)&cpu->cfg.mvendorid;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> if (ret != 0) {
> error_report("Unable to retrieve mvendorid from host, error %d", ret);
> }
>
> - reg.id = RISCV_CONFIG_REG(env, marchid);
> + reg.id = RISCV_CONFIG_REG(marchid);
> reg.addr = (uint64_t)&cpu->cfg.marchid;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> if (ret != 0) {
> error_report("Unable to retrieve marchid from host, error %d", ret);
> }
>
> - reg.id = RISCV_CONFIG_REG(env, mimpid);
> + reg.id = RISCV_CONFIG_REG(mimpid);
> reg.addr = (uint64_t)&cpu->cfg.mimpid;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> if (ret != 0) {
> @@ -984,7 +970,7 @@ static void kvm_riscv_init_misa_ext_mask(RISCVCPU *cpu,
> struct kvm_one_reg reg;
> int ret;
>
> - reg.id = RISCV_CONFIG_REG(env, isa);
> + reg.id = RISCV_CONFIG_REG(isa);
> reg.addr = (uint64_t)&env->misa_ext_mask;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
>
> @@ -1001,11 +987,10 @@ static void kvm_riscv_init_misa_ext_mask(RISCVCPU *cpu,
> static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
> KVMCPUConfig *cbomz_cfg)
> {
> - CPURISCVState *env = &cpu->env;
> struct kvm_one_reg reg;
> int ret;
>
> - reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
> + reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG,
> cbomz_cfg->kvm_reg_id);
> reg.addr = (uint64_t)kvmconfig_get_cfg_addr(cpu, cbomz_cfg);
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> @@ -1019,7 +1004,6 @@ static void kvm_riscv_read_cbomz_blksize(RISCVCPU *cpu, KVMScratchCPU *kvmcpu,
> static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
> KVMScratchCPU *kvmcpu)
> {
> - CPURISCVState *env = &cpu->env;
> uint64_t val;
> int i, ret;
>
> @@ -1027,7 +1011,7 @@ static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
> KVMCPUConfig *multi_ext_cfg = &kvm_multi_ext_cfgs[i];
> struct kvm_one_reg reg;
>
> - reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_ISA_EXT,
> + reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
> multi_ext_cfg->kvm_reg_id);
> reg.addr = (uint64_t)&val;
> ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, ®);
> @@ -1159,7 +1143,7 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, KVMScratchCPU *kvmcpu)
>
> for (i = 0; i < ARRAY_SIZE(kvm_multi_ext_cfgs); i++) {
> multi_ext_cfg = &kvm_multi_ext_cfgs[i];
> - reg_id = kvm_riscv_reg_id_ulong(&cpu->env, KVM_REG_RISCV_ISA_EXT,
> + reg_id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_ISA_EXT,
> multi_ext_cfg->kvm_reg_id);
> reg_search = bsearch(®_id, reglist->reg, reglist->n,
> sizeof(uint64_t), uint64_cmp);
> @@ -1338,12 +1322,11 @@ void kvm_arch_init_irq_routing(KVMState *s)
>
> static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
> {
> - CPURISCVState *env = &cpu->env;
> target_ulong reg;
> uint64_t id;
> int ret;
>
> - id = RISCV_CONFIG_REG(env, mvendorid);
> + id = RISCV_CONFIG_REG(mvendorid);
> /*
> * cfg.mvendorid is an uint32 but a target_ulong will
> * be written. Assign it to a target_ulong var to avoid
> @@ -1355,13 +1338,13 @@ static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
> return ret;
> }
>
> - id = RISCV_CONFIG_REG(env, marchid);
> + id = RISCV_CONFIG_REG(marchid);
> ret = kvm_set_one_reg(cs, id, &cpu->cfg.marchid);
> if (ret != 0) {
> return ret;
> }
>
> - id = RISCV_CONFIG_REG(env, mimpid);
> + id = RISCV_CONFIG_REG(mimpid);
> ret = kvm_set_one_reg(cs, id, &cpu->cfg.mimpid);
>
> return ret;
> @@ -1911,7 +1894,7 @@ void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> if (cpu->cfg.ext_zicbom &&
> riscv_cpu_option_set(kvm_cbom_blocksize.name)) {
>
> - reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
> + reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG,
> kvm_cbom_blocksize.kvm_reg_id);
> reg.addr = (uint64_t)&val;
> ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, ®);
> @@ -1930,7 +1913,7 @@ void riscv_kvm_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
> if (cpu->cfg.ext_zicboz &&
> riscv_cpu_option_set(kvm_cboz_blocksize.name)) {
>
> - reg.id = kvm_riscv_reg_id_ulong(env, KVM_REG_RISCV_CONFIG,
> + reg.id = KVM_RISCV_REG_ID_ULONG(KVM_REG_RISCV_CONFIG,
> kvm_cboz_blocksize.kvm_reg_id);
> reg.addr = (uint64_t)&val;
> ret = ioctl(kvmcpu.cpufd, KVM_GET_ONE_REG, ®);
> --
> 2.49.0
>
Otherwise,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
next prev parent reply other threads:[~2025-04-25 11:48 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-25 11:36 [PATCH v2 0/9] target/riscv/kvm: CSR related fixes Daniel Henrique Barboza
2025-04-25 11:36 ` [PATCH v2 1/9] target/riscv/kvm: minor fixes/tweaks Daniel Henrique Barboza
2025-04-25 11:43 ` Andrew Jones
2025-04-25 11:36 ` [PATCH v2 2/9] target/riscv/kvm: fix leak in kvm_riscv_init_multiext_cfg() Daniel Henrique Barboza
2025-04-25 11:44 ` Andrew Jones
2025-04-25 11:36 ` [PATCH v2 3/9] target/riscv/kvm: turn u32/u64 reg functions in macros Daniel Henrique Barboza
2025-04-25 11:45 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 4/9] target/riscv/kvm: turn kvm_riscv_reg_id_ulong() in a macro Daniel Henrique Barboza
2025-04-25 11:48 ` Andrew Jones [this message]
2025-04-25 11:37 ` [PATCH v2 5/9] target/riscv/kvm: add kvm_csr_cfgs[] Daniel Henrique Barboza
2025-04-25 11:58 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 6/9] target/riscv/kvm: do not read unavailable CSRs Daniel Henrique Barboza
2025-04-25 12:02 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 7/9] target/riscv/kvm: add senvcfg CSR Daniel Henrique Barboza
2025-04-25 12:03 ` Andrew Jones
2025-04-25 11:37 ` [PATCH v2 8/9] target/riscv: widen (m|s)counteren to target_ulong Daniel Henrique Barboza
2025-04-25 12:11 ` Andrew Jones
2025-04-25 13:10 ` Daniel Henrique Barboza
2025-04-25 11:37 ` [PATCH v2 9/9] target/riscv/kvm: add scounteren CSR Daniel Henrique Barboza
2025-04-25 12:12 ` Andrew Jones
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=20250425-d623af137e71843fa8d571d5@orel \
--to=ajones@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=dbarboza@ventanamicro.com \
--cc=liwei1518@gmail.com \
--cc=palmer@rivosinc.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).