Re: [PATCH v3 01/27] target: Set CPUClass::vmsd instead of DeviceClass::vmsd

[Philippe Mathieu-Daudé]

On 4/22/21 1:01 PM, Philippe Mathieu-Daudé wrote:
> On 4/22/21 12:28 PM, Peter Maydell wrote:
>> On Thu, 22 Apr 2021 at 10:55, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>> My guess is CPUState is the only device used in user emulation,
>>> so it would be a way to restrict the vmstate_dummy to CPU and
>>> not to any DeviceState?
>>>
>>> But looking at the introductory commit:
>>>
>>> commit b170fce3dd06372f7bfec9a780ebcb1fce6d57e4
>>> Author: Andreas Färber <afaerber@suse.de>
>>> Date:   Sun Jan 20 20:23:22 2013 +0100
>>>
>>>     cpu: Register VMStateDescription through CPUState
>>>
>>>     In comparison to DeviceClass::vmsd, CPU VMState is split in two,
>>>     "cpu_common" and "cpu", and uses cpu_index as instance_id instead of -1.
>>>     Therefore add a CPU-specific CPUClass::vmsd field.
>>>
>>>     Unlike the legacy CPUArchState registration, rather register CPUState.
>>>
>>> Juan, do you remember?
>>
>> Oh yes, I remember this. There are two ways to handle migration for
>> a CPU object:
>>
>>  (1) like any other device, so it has a dc->vmsd that covers
>> migration for the whole object. As usual for objects that are a
>> subclass of a parent that has state, the first entry in the
>> VMStateDescription field list is VMSTATE_CPU(), which migrates
>> the cpu_common fields, followed by whatever the CPU's own migration
>> fields are.

target/alpha/cpu.c:240:    dc->vmsd = &vmstate_alpha_cpu;
target/cris/cpu.c:296:    dc->vmsd = &vmstate_cris_cpu;
target/hppa/cpu.c:165:    dc->vmsd = &vmstate_hppa_cpu;
target/m68k/cpu.c:537:    dc->vmsd = &vmstate_m68k_cpu;
target/microblaze/cpu.c:390:    dc->vmsd = &vmstate_mb_cpu;
target/openrisc/cpu.c:207:    dc->vmsd = &vmstate_openrisc_cpu;
target/sh4/cpu.c:265:    dc->vmsd = &vmstate_sh_cpu;
target/unicore32/cpu.c:149:    dc->vmsd = &vmstate_uc32_cpu;
target/xtensa/cpu.c:221:    dc->vmsd = &vmstate_xtensa_cpu;

>>  (2) a backwards-compatible mechanism for CPUs that were
>> originally migrated using manual "write fields to the migration
>> stream structures". The on-the-wire migration format
>> for those is based on the 'env' pointer (which isn't a QOM object),
>> and the cpu_common part of the migration data is elsewhere.

target/arm/cpu.c:1985:    cc->vmsd = &vmstate_arm_cpu;
target/avr/cpu.c:216:    cc->vmsd = &vms_avr_cpu;
target/i386/cpu.c:7434:    cc->vmsd = &vmstate_x86_cpu;
target/lm32/cpu.c:244:    cc->vmsd = &vmstate_lm32_cpu;
target/mips/cpu.c:723:    cc->vmsd = &vmstate_mips_cpu;
target/moxie/cpu.c:125:    cc->vmsd = &vmstate_moxie_cpu;
target/ppc/translate_init.c.inc:10923:    cc->vmsd = &vmstate_ppc_cpu;
target/riscv/cpu.c:627:    cc->vmsd = &vmstate_riscv_cpu;
target/s390x/cpu.c:520:    cc->vmsd = &vmstate_s390_cpu;
target/sparc/cpu.c:892:    cc->vmsd = &vmstate_sparc_cpu;

>> cpu_exec_realizefn() handles both possibilities:
>>  * for type 1, dc->vmsd is set and cc->vmsd is not,
>>    so cpu_exec_realizefn() does nothing, and the standard
>>    "register dc->vmsd for a device" code does everything needed
>>  * for type 2, dc->vmsd is NULL and so we register the vmstate_cpu_common
>>    directly to handle the cpu-common fields, and the cc->vmsd to handle
>>    the per-CPU stuff
>>
>> You can't change a CPU from one type to the other without breaking
>> migration compatibility, which is why some guest architectures
>> are stuck on the cc->vmsd form. New targets should use dc->vmsd.

IOW new targets should use type 1.


Looking at type 2:

a) targets added 'recently' with the incorrect type 2:

target/avr/cpu.c:216:    cc->vmsd = &vms_avr_cpu;
target/riscv/cpu.c:627:    cc->vmsd = &vmstate_riscv_cpu;

b) targets where migration isn't really an issue:

target/lm32/cpu.c:244:    cc->vmsd = &vmstate_lm32_cpu;
target/moxie/cpu.c:125:    cc->vmsd = &vmstate_moxie_cpu;

c) targets where migration could be broken:

target/mips/cpu.c:723:    cc->vmsd = &vmstate_mips_cpu;
target/sparc/cpu.c:892:    cc->vmsd = &vmstate_sparc_cpu;

d) KVM targets ("security boundary" or Tier-1) are left:

target/arm/cpu.c:1985:    cc->vmsd = &vmstate_arm_cpu;
target/i386/cpu.c:7434:    cc->vmsd = &vmstate_x86_cpu;
target/ppc/translate_init.c.inc:10923:    cc->vmsd = &vmstate_ppc_cpu;
target/s390x/cpu.c:520:    cc->vmsd = &vmstate_s390_cpu;


Isn't "machine type" what allows us to change migration stream?
All targets in d) do support that.


How could we enforce no new type 2 targets?


Thanks,

Phil.