| Message ID | 20250220161313.127376-3-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv/kvm: reset time changes | expand |
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? Thanks, drew > > Signed-off-by: Daniel Henrique Barboza <dbarboza@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 > >
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 <dbarboza@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 >> >>
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);
Using env->sie is clearer than using env->mie. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/kvm/kvm-cpu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)