Re: [PATCH 00/33] hw/cpu/arm: Remove one use of qemu_get_cpu() in A7/A15 MPCore priv

[Fabiano Rosas]

Cédric Le Goater <clg@kaod.org> writes:

> On 1/9/24 18:40, Fabiano Rosas wrote:
>> Cédric Le Goater <clg@kaod.org> writes:
>> 
>>> On 1/3/24 20:53, Fabiano Rosas wrote:
>>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>>
>>>>> +Peter/Fabiano
>>>>>
>>>>> On 2/1/24 17:41, Cédric Le Goater wrote:
>>>>>> On 1/2/24 17:15, Philippe Mathieu-Daudé wrote:
>>>>>>> Hi Cédric,
>>>>>>>
>>>>>>> On 2/1/24 15:55, Cédric Le Goater wrote:
>>>>>>>> On 12/12/23 17:29, Philippe Mathieu-Daudé wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> When a MPCore cluster is used, the Cortex-A cores belong the the
>>>>>>>>> cluster container, not to the board/soc layer. This series move
>>>>>>>>> the creation of vCPUs to the MPCore private container.
>>>>>>>>>
>>>>>>>>> Doing so we consolidate the QOM model, moving common code in a
>>>>>>>>> central place (abstract MPCore parent).
>>>>>>>>
>>>>>>>> Changing the QOM hierarchy has an impact on the state of the machine
>>>>>>>> and some fixups are then required to maintain migration compatibility.
>>>>>>>> This can become a real headache for KVM machines like virt for which
>>>>>>>> migration compatibility is a feature, less for emulated ones.
>>>>>>>
>>>>>>> All changes are either moving properties (which are not migrated)
>>>>>>> or moving non-migrated QOM members (i.e. pointers of ARMCPU, which
>>>>>>> is still migrated elsewhere). So I don't see any obvious migration
>>>>>>> problem, but I might be missing something, so I Cc'ed Juan :>
>>>>
>>>> FWIW, I didn't spot anything problematic either.
>>>>
>>>> I've ran this through my migration compatibility series [1] and it
>>>> doesn't regress aarch64 migration from/to 8.2. The tests use '-M
>>>> virt -cpu max', so the cortex-a7 and cortex-a15 are not covered. I don't
>>>> think we even support migration of anything non-KVM on arm.
>>>
>>> it happens we do.
>>>
>> 
>> Oh, sorry, I didn't mean TCG here. Probably meant to say something like
>> non-KVM-capable cpus, as in 32-bit. Nevermind.
>
> Theoretically, we should be able to migrate to a TCG guest. Well, this
> worked in the past for PPC. When I was doing more KVM related changes,
> this was very useful for dev. Also, some machines are partially emulated.
> Anyhow I agree this is not a strong requirement and we often break it.
> Let's focus on KVM only.
>
>>>> 1- https://gitlab.com/farosas/qemu/-/jobs/5853599533
>>>
>>> yes it depends on the QOM hierarchy and virt seems immune to the changes.
>>> Good.
>>>
>>> However, changing the QOM topology clearly breaks migration compat,
>> 
>> Well, "clearly" is relative =) You've mentioned pseries and aspeed
>> already, do you have a pointer to one of those cases were we broke
>> migration 
>
> Regarding pseries, migration compat broke because of 5bc8d26de20c
> ("spapr: allocate the ICPState object from under sPAPRCPUCore") which
> is similar to the changes proposed by this series, it impacts the QOM
> hierarchy. Here is the workaround/fix from Greg : 46f7afa37096
> ("spapr: fix migration of ICPState objects from/to older QEMU") which
> is quite an headache and this turned out to raise another problem some
> months ago ... :/ That's why I sent [1] to prepare removal of old
> machines and workarounds becoming a burden.

This feels like something that could be handled by the vmstate code
somehow. The state is there, just under a different path. No one wants
to be policing QOM hierarchy changes in every single series that shows
up on the list.

Anyway, thanks for the pointers. I'll study that code a bit more, maybe
I can come up with some way to handle these cases.

Hopefully between the analyze-migration test and the compat tests we'll
catch the next bug of this kind before it gets merged.