[v1] riscv-to-apply queue

[PULL 47/56] target/riscv/kvm: add scounteren CSR

Posted by alistair23@gmail.com 8 months, 3 weeks ago

From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Add support for the scounteren KVM CSR. Note that env->scounteren is a
32 bit and all KVM CSRs are target_ulong, so scounteren will be capped
to 32 bits read/writes.

Reported-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-ID: <20250429124421.223883-10-dbarboza@ventanamicro.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/kvm/kvm-cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index ca171d5457..82f9728636 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -251,6 +251,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
     KVM_CSR_CFG("stval",      stval,      RISCV_CSR_REG(stval)),
     KVM_CSR_CFG("sip",        mip,        RISCV_CSR_REG(sip)),
     KVM_CSR_CFG("satp",       satp,       RISCV_CSR_REG(satp)),
+    KVM_CSR_CFG("scounteren", scounteren, RISCV_CSR_REG(scounteren)),
     KVM_CSR_CFG("senvcfg",    senvcfg,    RISCV_CSR_REG(senvcfg)),
 };
 
@@ -701,6 +702,7 @@ static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
     env->stval = 0;
     env->mip = 0;
     env->satp = 0;
+    env->scounteren = 0;
     env->senvcfg = 0;
 }
 
-- 
2.49.0

Re: [PULL 47/56] target/riscv/kvm: add scounteren CSR

Posted by Peter Maydell 3 months, 2 weeks ago

On Mon, 19 May 2025 at 05:25, <alistair23@gmail.com> wrote:
>
> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>
> Add support for the scounteren KVM CSR. Note that env->scounteren is a
> 32 bit and all KVM CSRs are target_ulong, so scounteren will be capped
> to 32 bits read/writes.
>
> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Message-ID: <20250429124421.223883-10-dbarboza@ventanamicro.com>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/kvm/kvm-cpu.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index ca171d5457..82f9728636 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -251,6 +251,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
>      KVM_CSR_CFG("stval",      stval,      RISCV_CSR_REG(stval)),
>      KVM_CSR_CFG("sip",        mip,        RISCV_CSR_REG(sip)),
>      KVM_CSR_CFG("satp",       satp,       RISCV_CSR_REG(satp)),
> +    KVM_CSR_CFG("scounteren", scounteren, RISCV_CSR_REG(scounteren)),
>      KVM_CSR_CFG("senvcfg",    senvcfg,    RISCV_CSR_REG(senvcfg)),
>  };
>
> @@ -701,6 +702,7 @@ static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
>      env->stval = 0;
>      env->mip = 0;
>      env->satp = 0;
> +    env->scounteren = 0;
>      env->senvcfg = 0;
>  }

Hi -- this came up in a conversation on IRC. Does this new
CPU state field need migration support adding in machine.c ?

thanks
-- PMM

Re: [PULL 47/56] target/riscv/kvm: add scounteren CSR

Posted by Daniel Henrique Barboza 3 months, 2 weeks ago


On 10/24/25 10:43 AM, Peter Maydell wrote:
> On Mon, 19 May 2025 at 05:25, <alistair23@gmail.com> wrote:
>>
>> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>
>> Add support for the scounteren KVM CSR. Note that env->scounteren is a
>> 32 bit and all KVM CSRs are target_ulong, so scounteren will be capped
>> to 32 bits read/writes.
>>
>> Reported-by: Andrew Jones <ajones@ventanamicro.com>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> Message-ID: <20250429124421.223883-10-dbarboza@ventanamicro.com>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>   target/riscv/kvm/kvm-cpu.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
>> index ca171d5457..82f9728636 100644
>> --- a/target/riscv/kvm/kvm-cpu.c
>> +++ b/target/riscv/kvm/kvm-cpu.c
>> @@ -251,6 +251,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
>>       KVM_CSR_CFG("stval",      stval,      RISCV_CSR_REG(stval)),
>>       KVM_CSR_CFG("sip",        mip,        RISCV_CSR_REG(sip)),
>>       KVM_CSR_CFG("satp",       satp,       RISCV_CSR_REG(satp)),
>> +    KVM_CSR_CFG("scounteren", scounteren, RISCV_CSR_REG(scounteren)),
>>       KVM_CSR_CFG("senvcfg",    senvcfg,    RISCV_CSR_REG(senvcfg)),
>>   };
>>
>> @@ -701,6 +702,7 @@ static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
>>       env->stval = 0;
>>       env->mip = 0;
>>       env->satp = 0;
>> +    env->scounteren = 0;
>>       env->senvcfg = 0;
>>   }
> 
> Hi -- this came up in a conversation on IRC. Does this new
> CPU state field need migration support adding in machine.c ?


Hmm, I believe it already has, doesn't it?

target/riscv/machine.c:


const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
     .version_id = 10,
     .minimum_version_id = 10,
     .post_load = riscv_cpu_post_load,
     .fields = (const VMStateField[]) {
         VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
(...)
        VMSTATE_UINT32(env.scounteren, RISCVCPU),  <-------


Or are you referring to something else like post_load callbacks and so on? Thanks,

Daniel


> 
> thanks
> -- PMM

Re: [PULL 47/56] target/riscv/kvm: add scounteren CSR

Posted by Michael Tokarev 3 months, 2 weeks ago

On 10/24/25 19:17, Daniel Henrique Barboza wrote:
> 
> 
> On 10/24/25 10:43 AM, Peter Maydell wrote:
>> On Mon, 19 May 2025 at 05:25, <alistair23@gmail.com> wrote:
>>>
>>> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>>
>>> Add support for the scounteren KVM CSR. Note that env->scounteren is a
>>> 32 bit and all KVM CSRs are target_ulong, so scounteren will be capped
>>> to 32 bits read/writes.
>>>
>>> Reported-by: Andrew Jones <ajones@ventanamicro.com>
>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>> Message-ID: <20250429124421.223883-10-dbarboza@ventanamicro.com>
>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>>   target/riscv/kvm/kvm-cpu.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
>>> index ca171d5457..82f9728636 100644
>>> --- a/target/riscv/kvm/kvm-cpu.c
>>> +++ b/target/riscv/kvm/kvm-cpu.c
>>> @@ -251,6 +251,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
>>>       KVM_CSR_CFG("stval",      stval,      RISCV_CSR_REG(stval)),
>>>       KVM_CSR_CFG("sip",        mip,        RISCV_CSR_REG(sip)),
>>>       KVM_CSR_CFG("satp",       satp,       RISCV_CSR_REG(satp)),
>>> +    KVM_CSR_CFG("scounteren", scounteren, RISCV_CSR_REG(scounteren)),
>>>       KVM_CSR_CFG("senvcfg",    senvcfg,    RISCV_CSR_REG(senvcfg)),
>>>   };
>>>
>>> @@ -701,6 +702,7 @@ static void 
>>> kvm_riscv_reset_regs_csr(CPURISCVState *env)
>>>       env->stval = 0;
>>>       env->mip = 0;
>>>       env->satp = 0;
>>> +    env->scounteren = 0;
>>>       env->senvcfg = 0;
>>>   }
>>
>> Hi -- this came up in a conversation on IRC. Does this new
>> CPU state field need migration support adding in machine.c ?
> 
> 
> Hmm, I believe it already has, doesn't it?
> 
> target/riscv/machine.c:
> 
> 
> const VMStateDescription vmstate_riscv_cpu = {
>      .name = "cpu",
>      .version_id = 10,
>      .minimum_version_id = 10,
>      .post_load = riscv_cpu_post_load,
>      .fields = (const VMStateField[]) {
>          VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
> (...)
>         VMSTATE_UINT32(env.scounteren, RISCVCPU),  <-------
> 
> 
> Or are you referring to something else like post_load callbacks and so 
> on? Thanks,

In this case, can or should this change be picked up for
qemu-stable (in this case, 10.0.x)?

Not that it's hugely important, but some subsequent patches
in this area would apply cleanly if I'll pick this one and
also senvcfg one (86b8c3821496).

Thanks,

/mjt

Re: [PULL 47/56] target/riscv/kvm: add scounteren CSR

Posted by Daniel Henrique Barboza 3 months, 2 weeks ago


On 10/25/25 1:45 PM, Michael Tokarev wrote:
> On 10/24/25 19:17, Daniel Henrique Barboza wrote:
>>
>>
>> On 10/24/25 10:43 AM, Peter Maydell wrote:
>>> On Mon, 19 May 2025 at 05:25, <alistair23@gmail.com> wrote:
>>>>
>>>> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>>>
>>>> Add support for the scounteren KVM CSR. Note that env->scounteren is a
>>>> 32 bit and all KVM CSRs are target_ulong, so scounteren will be capped
>>>> to 32 bits read/writes.
>>>>
>>>> Reported-by: Andrew Jones <ajones@ventanamicro.com>
>>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>> Message-ID: <20250429124421.223883-10-dbarboza@ventanamicro.com>
>>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>>> ---
>>>>   target/riscv/kvm/kvm-cpu.c | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
>>>> index ca171d5457..82f9728636 100644
>>>> --- a/target/riscv/kvm/kvm-cpu.c
>>>> +++ b/target/riscv/kvm/kvm-cpu.c
>>>> @@ -251,6 +251,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
>>>>       KVM_CSR_CFG("stval",      stval,      RISCV_CSR_REG(stval)),
>>>>       KVM_CSR_CFG("sip",        mip,        RISCV_CSR_REG(sip)),
>>>>       KVM_CSR_CFG("satp",       satp,       RISCV_CSR_REG(satp)),
>>>> +    KVM_CSR_CFG("scounteren", scounteren, RISCV_CSR_REG(scounteren)),
>>>>       KVM_CSR_CFG("senvcfg",    senvcfg,    RISCV_CSR_REG(senvcfg)),
>>>>   };
>>>>
>>>> @@ -701,6 +702,7 @@ static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
>>>>       env->stval = 0;
>>>>       env->mip = 0;
>>>>       env->satp = 0;
>>>> +    env->scounteren = 0;
>>>>       env->senvcfg = 0;
>>>>   }
>>>
>>> Hi -- this came up in a conversation on IRC. Does this new
>>> CPU state field need migration support adding in machine.c ?
>>
>>
>> Hmm, I believe it already has, doesn't it?
>>
>> target/riscv/machine.c:
>>
>>
>> const VMStateDescription vmstate_riscv_cpu = {
>>      .name = "cpu",
>>      .version_id = 10,
>>      .minimum_version_id = 10,
>>      .post_load = riscv_cpu_post_load,
>>      .fields = (const VMStateField[]) {
>>          VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
>> (...)
>>         VMSTATE_UINT32(env.scounteren, RISCVCPU),  <-------
>>
>>
>> Or are you referring to something else like post_load callbacks and so on? Thanks,
> 
> In this case, can or should this change be picked up for
> qemu-stable (in this case, 10.0.x)?
> 
> Not that it's hugely important, but some subsequent patches
> in this area would apply cleanly if I'll pick this one and
> also senvcfg one (86b8c3821496).

Go ahead, but to pick those you'll need to also pick these:

[PULL 44/56] target/riscv/kvm: do not read unavailable CSRs

[PULL 46/56] target/riscv/kvm: read/write KVM regs via env size

Otherwise you might introduce bugs. Thanks,


Daniel


> 
> Thanks,
> 
> /mjt

Re: [PULL 47/56] target/riscv/kvm: add scounteren CSR

Posted by Michael Tokarev 3 months, 2 weeks ago

On 10/26/25 03:36, Daniel Henrique Barboza wrote:
> On 10/25/25 1:45 PM, Michael Tokarev wrote:
..>> In this case, can or should this change be picked up for
>> qemu-stable (in this case, 10.0.x)?
>>
>> Not that it's hugely important, but some subsequent patches
>> in this area would apply cleanly if I'll pick this one and
>> also senvcfg one (86b8c3821496).
> 
> Go ahead, but to pick those you'll need to also pick these:
> 
> [PULL 44/56] target/riscv/kvm: do not read unavailable CSRs
> 
> [PULL 46/56] target/riscv/kvm: read/write KVM regs via env size
> 
> Otherwise you might introduce bugs. Thanks,

"do not read unavailable CSRs" is already in the first 10.0.1,
it's been Cc'ed to qemu-stable initially.

I also picked up "read/write KVM regs via env size" now.

That patchset had quite a number of for-stable fixes.

Please note: 10.0.x series is supposed to be long-term.

Thank you!

/mjt

Re: [PULL 47/56] target/riscv/kvm: add scounteren CSR

Posted by Peter Maydell 3 months, 2 weeks ago

On Fri, 24 Oct 2025 at 17:17, Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 10/24/25 10:43 AM, Peter Maydell wrote:
> > On Mon, 19 May 2025 at 05:25, <alistair23@gmail.com> wrote:
> >>
> >> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> >>
> >> Add support for the scounteren KVM CSR. Note that env->scounteren is a
> >> 32 bit and all KVM CSRs are target_ulong, so scounteren will be capped
> >> to 32 bits read/writes.
> >>
> >> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> >> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> >> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >> Message-ID: <20250429124421.223883-10-dbarboza@ventanamicro.com>
> >> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> >> ---
> >>   target/riscv/kvm/kvm-cpu.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> >> index ca171d5457..82f9728636 100644
> >> --- a/target/riscv/kvm/kvm-cpu.c
> >> +++ b/target/riscv/kvm/kvm-cpu.c
> >> @@ -251,6 +251,7 @@ static KVMCPUConfig kvm_csr_cfgs[] = {
> >>       KVM_CSR_CFG("stval",      stval,      RISCV_CSR_REG(stval)),
> >>       KVM_CSR_CFG("sip",        mip,        RISCV_CSR_REG(sip)),
> >>       KVM_CSR_CFG("satp",       satp,       RISCV_CSR_REG(satp)),
> >> +    KVM_CSR_CFG("scounteren", scounteren, RISCV_CSR_REG(scounteren)),
> >>       KVM_CSR_CFG("senvcfg",    senvcfg,    RISCV_CSR_REG(senvcfg)),
> >>   };
> >>
> >> @@ -701,6 +702,7 @@ static void kvm_riscv_reset_regs_csr(CPURISCVState *env)
> >>       env->stval = 0;
> >>       env->mip = 0;
> >>       env->satp = 0;
> >> +    env->scounteren = 0;
> >>       env->senvcfg = 0;
> >>   }
> >
> > Hi -- this came up in a conversation on IRC. Does this new
> > CPU state field need migration support adding in machine.c ?
>
>
> Hmm, I believe it already has, doesn't it?
>
> target/riscv/machine.c:

>         VMSTATE_UINT32(env.scounteren, RISCVCPU),  <-------

So it does. I'm not sure how I missed that in my grep.
Sorry for the noise.

-- PMM