diff mbox series

[3/3] target/riscv/kvm: reset all available KVM CSRs in kvm_reset()

Message ID 20250220161313.127376-4-dbarboza@ventanamicro.com (mailing list archive)
State New
Headers show
Series target/riscv/kvm: reset time changes | expand

Commit Message

Daniel Henrique Barboza Feb. 20, 2025, 4:13 p.m. UTC 
Explictly reset env->mstatus and env->sie. Add a comment about env->mip
being read/written into KVM 'sip' CSR.

We're also not read/writing 'scounteren' which is available in the KVM
UAPI. Add it in kvm_reset() and get/put_regs_csr().

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

Comments

Andrew Jones Feb. 21, 2025, 8:45 a.m. UTC | #1 
On Thu, Feb 20, 2025 at 01:13:13PM -0300, Daniel Henrique Barboza wrote:
> Explictly reset env->mstatus and env->sie.

mie was already getting set to zero, so that should have just been renamed
in the last patch, but I still think we should drop the last patch.

> Add a comment about env->mip
> being read/written into KVM 'sip' CSR.
> 
> We're also not read/writing 'scounteren' which is available in the KVM
> UAPI. Add it in kvm_reset() and get/put_regs_csr().
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/kvm/kvm-cpu.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index fea03f3657..ee7a9295b4 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -618,6 +618,7 @@ static int kvm_riscv_get_regs_csr(CPUState *cs)
>      KVM_RISCV_GET_CSR(cs, env, stval, env->stval);
>      KVM_RISCV_GET_CSR(cs, env, sip, env->mip);
>      KVM_RISCV_GET_CSR(cs, env, satp, env->satp);
> +    KVM_RISCV_GET_CSR(cs, env, scounteren, env->scounteren);

senvcfg is also missing.

>  
>      return 0;
>  }
> @@ -635,6 +636,7 @@ static int kvm_riscv_put_regs_csr(CPUState *cs)
>      KVM_RISCV_SET_CSR(cs, env, stval, env->stval);
>      KVM_RISCV_SET_CSR(cs, env, sip, env->mip);
>      KVM_RISCV_SET_CSR(cs, env, satp, env->satp);
> +    KVM_RISCV_SET_CSR(cs, env, scounteren, env->scounteren);
>  
>      return 0;
>  }
> @@ -1609,6 +1611,10 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
>      env->pc = cpu->env.kernel_addr;
>      env->gpr[10] = kvm_arch_vcpu_id(CPU(cpu)); /* a0 */
>      env->gpr[11] = cpu->env.fdt_addr;          /* a1 */
> +
> +    /* sstatus is read/written into mstatus */

How about just a single comment above this function stating that we
reset all registers that we will s/r with csr get/put. Interested
parties can go look at get or put to see the mappings.

> +    env->mstatus = 0;
> +    env->sie = 0;
>      env->satp = 0;
>      env->mie = 0;
>      env->stvec = 0;
> @@ -1616,7 +1622,9 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
>      env->sepc = 0;
>      env->scause = 0;
>      env->stval = 0;
> +    /* sip is read/written into mip */
>      env->mip = 0;
> +    env->scounteren = 0;
>  }
>  
>  void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)
> -- 
> 2.48.1
> 
>

Thanks,
drew
Andrew Jones Feb. 21, 2025, 9:01 a.m. UTC | #2 
On Fri, Feb 21, 2025 at 09:45:35AM +0100, Andrew Jones wrote:
> On Thu, Feb 20, 2025 at 01:13:13PM -0300, Daniel Henrique Barboza wrote:
> > Explictly reset env->mstatus and env->sie.
> 
> mie was already getting set to zero, so that should have just been renamed
> in the last patch, but I still think we should drop the last patch.
> 
> > Add a comment about env->mip
> > being read/written into KVM 'sip' CSR.
> > 
> > We're also not read/writing 'scounteren' which is available in the KVM
> > UAPI. Add it in kvm_reset() and get/put_regs_csr().
> > 
> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > ---
> >  target/riscv/kvm/kvm-cpu.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> > index fea03f3657..ee7a9295b4 100644
> > --- a/target/riscv/kvm/kvm-cpu.c
> > +++ b/target/riscv/kvm/kvm-cpu.c
> > @@ -618,6 +618,7 @@ static int kvm_riscv_get_regs_csr(CPUState *cs)
> >      KVM_RISCV_GET_CSR(cs, env, stval, env->stval);
> >      KVM_RISCV_GET_CSR(cs, env, sip, env->mip);
> >      KVM_RISCV_GET_CSR(cs, env, satp, env->satp);
> > +    KVM_RISCV_GET_CSR(cs, env, scounteren, env->scounteren);
> 
> senvcfg is also missing.
> 
> >  
> >      return 0;
> >  }
> > @@ -635,6 +636,7 @@ static int kvm_riscv_put_regs_csr(CPUState *cs)
> >      KVM_RISCV_SET_CSR(cs, env, stval, env->stval);
> >      KVM_RISCV_SET_CSR(cs, env, sip, env->mip);
> >      KVM_RISCV_SET_CSR(cs, env, satp, env->satp);
> > +    KVM_RISCV_SET_CSR(cs, env, scounteren, env->scounteren);
> >  
> >      return 0;
> >  }
> > @@ -1609,6 +1611,10 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
> >      env->pc = cpu->env.kernel_addr;
> >      env->gpr[10] = kvm_arch_vcpu_id(CPU(cpu)); /* a0 */
> >      env->gpr[11] = cpu->env.fdt_addr;          /* a1 */
> > +
> > +    /* sstatus is read/written into mstatus */
> 
> How about just a single comment above this function stating that we
> reset all registers that we will s/r with csr get/put. Interested
> parties can go look at get or put to see the mappings.
> 
> > +    env->mstatus = 0;
> > +    env->sie = 0;
> >      env->satp = 0;
> >      env->mie = 0;
> >      env->stvec = 0;
> > @@ -1616,7 +1622,9 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
> >      env->sepc = 0;
> >      env->scause = 0;
> >      env->stval = 0;
> > +    /* sip is read/written into mip */
> >      env->mip = 0;
> > +    env->scounteren = 0;

It'd be nice to put all the above register assignments in the order of
struct kvm_riscv_csr, like get/put do.

Thanks,
drew


> >  }
> >  
> >  void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)
> > -- 
> > 2.48.1
> > 
> >
> 
> Thanks,
> drew
diff mbox series

Patch

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index fea03f3657..ee7a9295b4 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -618,6 +618,7 @@  static int kvm_riscv_get_regs_csr(CPUState *cs)
     KVM_RISCV_GET_CSR(cs, env, stval, env->stval);
     KVM_RISCV_GET_CSR(cs, env, sip, env->mip);
     KVM_RISCV_GET_CSR(cs, env, satp, env->satp);
+    KVM_RISCV_GET_CSR(cs, env, scounteren, env->scounteren);
 
     return 0;
 }
@@ -635,6 +636,7 @@  static int kvm_riscv_put_regs_csr(CPUState *cs)
     KVM_RISCV_SET_CSR(cs, env, stval, env->stval);
     KVM_RISCV_SET_CSR(cs, env, sip, env->mip);
     KVM_RISCV_SET_CSR(cs, env, satp, env->satp);
+    KVM_RISCV_SET_CSR(cs, env, scounteren, env->scounteren);
 
     return 0;
 }
@@ -1609,6 +1611,10 @@  void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
     env->pc = cpu->env.kernel_addr;
     env->gpr[10] = kvm_arch_vcpu_id(CPU(cpu)); /* a0 */
     env->gpr[11] = cpu->env.fdt_addr;          /* a1 */
+
+    /* sstatus is read/written into mstatus */
+    env->mstatus = 0;
+    env->sie = 0;
     env->satp = 0;
     env->mie = 0;
     env->stvec = 0;
@@ -1616,7 +1622,9 @@  void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
     env->sepc = 0;
     env->scause = 0;
     env->stval = 0;
+    /* sip is read/written into mip */
     env->mip = 0;
+    env->scounteren = 0;
 }
 
 void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)