Re: [PATCH v2 03/33] target/riscv: Fix size of mhartid

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 10/1/25 1:28 AM, Anton Johansson wrote:
> On 01/10/25, Philippe Mathieu-Daudé wrote:
>> On 1/10/25 09:32, Anton Johansson wrote:
>>> and update formatting in log.
>>>
>>> Signed-off-by: Anton Johansson <anjo@rev.ng>
>>> ---
>>>    target/riscv/cpu.h         | 2 +-
>>>    target/riscv/cpu_helper.c  | 2 +-
>>>    target/riscv/machine.c     | 2 +-
>>>    target/riscv/tcg/tcg-cpu.c | 2 +-
>>>    4 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>>> index 736e4f6daa..3235108112 100644
>>> --- a/target/riscv/cpu.h
>>> +++ b/target/riscv/cpu.h
>>> @@ -279,7 +279,7 @@ struct CPUArchState {
>>>        target_ulong geilen;
>>>        uint64_t resetvec;
>>> -    target_ulong mhartid;
>>> +    uint64_t mhartid;
>>>        /*
>>>         * For RV32 this is 32-bit mstatus and 32-bit mstatush.
>>>         * For RV64 this is a 64-bit mstatus.
>>
>>
>>> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>>> index 99e46c3136..328fb674e1 100644
>>> --- a/target/riscv/machine.c
>>> +++ b/target/riscv/machine.c
>>> @@ -425,7 +425,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>>>            VMSTATE_UINTTL(env.priv, RISCVCPU),
>>>            VMSTATE_BOOL(env.virt_enabled, RISCVCPU),
>>>            VMSTATE_UINT64(env.resetvec, RISCVCPU),
>>> -        VMSTATE_UINTTL(env.mhartid, RISCVCPU),
>>> +        VMSTATE_UINT64(env.mhartid, RISCVCPU),
>>
>> The problem is you break the migration stream again. Maybe simpler to
>> just squash with previous patch?
>>
>> Thay said, IIUC mhartid is only set once in riscv_hart_realize().
>> I suspect it is pointless to migrate it.
> 
> Right, wrt the other vmstate changes I only bumped the version on first
> modification.  Maybe it makes more sense to group changes based on what
> vmstate they touch?  If excessive version bumping is a problem that is.

As long as it's bumped once in the series, I think it's acceptable.
No one will try to bisect that for migration.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>