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

[Daniel Henrique Barboza <dbarboza@ventanamicro.com>]

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
>>
>>