Re: [PATCH 2/3] target/riscv/kvm: use env->sie to read/write 'sie' CSR

[Daniel Henrique Barboza]

On 2/21/25 5:37 AM, Andrew Jones wrote:

On Thu, Feb 20, 2025 at 01:13:12PM -0300, Daniel Henrique Barboza wrote:

Using env->sie is clearer than using env->mie.

Maybe? Just as sstatus is a subset of mstatus, sip and sie can be
subsets of mip and mie. However, the AIA can change sip/sie so they
no longer alias mip/mie, which is why we have 'mvip' an 'sie' members
in CPURISCVState. In the end, for KVM, it doesn't really matter since
this is just s/r storage. I'd probably just drop this patch and keep
using mie. Otherwise, what about mip?

We don't have env->sip so it would fall on the same case as using env->mstatus
to get/put sstatus.

But yeah this change isn't that big of a deal and can be replaced with a comment
in reset_vcpu like you said in v3. Or maybe we could put those assignments in a
kvm_riscv_reset_regs_csr(), and the name of the function itself is an indication
that we follow the same order as kvm_riscv_(get/put)_regs_csr().

I think I'll use patch 2 to do that and patch 3 to add the missing KVM CSRs
(scounteren and senvcfg).


Thanks,

Daniel

Thanks,
drew

Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
  target/riscv/kvm/kvm-cpu.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 484b6afe7c..fea03f3657 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -610,7 +610,7 @@ static int kvm_riscv_get_regs_csr(CPUState *cs)
      CPURISCVState *env = &RISCV_CPU(cs)->env;

KVM_RISCV_GET_CSR(cs, env, sstatus, env->mstatus);

-    KVM_RISCV_GET_CSR(cs, env, sie, env->mie);
+    KVM_RISCV_GET_CSR(cs, env, sie, env->sie);
      KVM_RISCV_GET_CSR(cs, env, stvec, env->stvec);
      KVM_RISCV_GET_CSR(cs, env, sscratch, env->sscratch);
      KVM_RISCV_GET_CSR(cs, env, sepc, env->sepc);
@@ -627,7 +627,7 @@ static int kvm_riscv_put_regs_csr(CPUState *cs)
      CPURISCVState *env = &RISCV_CPU(cs)->env;

KVM_RISCV_SET_CSR(cs, env, sstatus, env->mstatus);

-    KVM_RISCV_SET_CSR(cs, env, sie, env->mie);
+    KVM_RISCV_SET_CSR(cs, env, sie, env->sie);
      KVM_RISCV_SET_CSR(cs, env, stvec, env->stvec);
      KVM_RISCV_SET_CSR(cs, env, sscratch, env->sscratch);
      KVM_RISCV_SET_CSR(cs, env, sepc, env->sepc);
--
2.48.1