Re: [PATCH v2 05/12] target/microblaze: Fill in VMStateDescription for cpu

[Richard Henderson]

On 9/4/20 5:20 AM, Edgar E. Iglesias wrote:
>> +static VMStateField vmstate_mmu_fields[] = {
>> +    VMSTATE_UINT64_2DARRAY(rams, MicroBlazeMMU, 2, TLB_ENTRIES),
>> +    VMSTATE_UINT8_ARRAY(tids, MicroBlazeMMU, TLB_ENTRIES),
>> +    VMSTATE_UINT32_ARRAY(regs, MicroBlazeMMU, 3),
>> +    VMSTATE_INT32(c_mmu, MicroBlazeMMU),
>> +    VMSTATE_INT32(c_mmu_tlb_access, MicroBlazeMMU),
>> +    VMSTATE_INT32(c_mmu_zones, MicroBlazeMMU),
>> +    VMSTATE_UINT64(c_addr_mask, MicroBlazeMMU),
> 
> These last 4 entries are elaboration-time settings, i.e they will not
> change during VM runtime. I don't think we need to transfer these since
> we expect the peer to setup the same kind of microblaze?

Ah, I see.  Yes, migration is only supported between "like" systems.

Though in this case I wasn't thinking so much of migration and the other uses
to which VMState gets put, like record/replay and the follow-on patches for gdb
reverse debugging.

>> +    VMSTATE_UINT32_ARRAY(pvr.regs, CPUMBState, 13),
> 
> pvr.regs are also elaboration time setup and should be read-only during
> the VM's lifetime.

What do you think about moving all of these to cpu->cfg, so that all of the
configuration-time stuff is together?


r~


> 
> If you fix those, this looks good to me.:
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>