Re: [PATCH v2 11/11] tpm_crb: support restoring older vmstate

[Stefan Berger <stefanb@linux.ibm.com>]

On 7/14/23 15:44, Joelle van Dyne wrote:
> On Fri, Jul 14, 2023 at 12:12 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>>
>>
>>
>> On 7/14/23 14:49, Joelle van Dyne wrote:
>>> On Fri, Jul 14, 2023 at 11:41 AM Stefan Berger <stefanb@linux.ibm.com> wrote:
>>>>
>>>>
>>>>
>>>> On 7/14/23 14:22, Stefan Berger wrote:
>>>>> On 7/14/23 13:04, Joelle van Dyne wrote:
>>>>>> On Fri, Jul 14, 2023 at 7:51 AM Stefan Berger <stefanb@linux.ibm.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 7/14/23 10:05, Stefan Berger wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 7/14/23 03:09, Joelle van Dyne wrote:
>>>>>>>>> When we moved to a single mapping and modified TPM CRB's VMState, it
>>>>>>>>> broke restoring of VMs that were saved on an older version. This
>>>>>>>>> change allows those VMs to gracefully migrate to the new memory
>>>>>>>>> mapping.
>>>>>>>>
>>>>>>>> Thanks. This has to be in 4/11 though.
>>>>>>>>
>>>>>>>
>>>>>>> After applying the whole series and trying to resume state taken with current git
>>>>>>> master I cannot restore it but it leads to this error here. I would just leave it
>>>>>>> completely untouched in 4/11.
>>>>>>>
>>>>>>> 2023-07-14T14:46:34.547550Z qemu-system-x86_64: Unknown ramblock "tpm-crb-cmd", cannot accept migration
>>>>>>> 2023-07-14T14:46:34.547799Z qemu-system-x86_64: error while loading state for instance 0x0 of device 'ram'
>>>>>>> 2023-07-14T14:46:34.547835Z qemu-system-x86_64: load of migration failed: Invalid argument
>>>>>>>
>>>>>>>        Stefan
>>>>>>
>>>>>> To be clear, you are asking to back out of 4/11? That patch changes
>>>>>> how the registers are mapped so it's impossible to support the old
>>>>>> style register mapping. This patch attempts to fix that with a
>>>>>
>>>>> Why can we not keep the old style register mapping as 'secondary mapping'?
>>>>
>>>> I think the first goal should be for existing TPM CRB device not to change anything, they
>>>> keep their .read and .write behaivor as it.
>>>>
>>>> If you need different .read behavior for the sysbus device due to AARCH64 then it may want to use its own MemoryRegionOps.
>>>>
>>>> I am fairly sure that you could refactor the core of the existing tpm_crb_mmio_write() and have it work on s->regs and mmio regs.
>>>> The former would be used by existing code, the latter for CRB sysbus calling into this new function from a wrapper.
>>>>
>>>>       Stefan
>>>
>>> I agree that new QEMU should be able to read old QEMU state but vice
>>> versa is not always true. There's been many changes in the past that
>>> incremented the vmstate's version_id to indicate that the state format
>>> has changed. Also, we are not changing the .read behavior because in
>>
>> Unfortunately the CRB device is being used by x86 on some distros
>> and the expectation is that this existing device can also downgrade
>> to a previous version of QEMU I would say. I have read people migrating
>> from RHEL 9.x even to RHEL 8.x and the expectation is that this works.
> But would the migration even work due to other parts of QEMU? The only
> way you can, say, migrate from QEMU 8.1.0 to 8.0.0 is if every single
> VMstate has its version_id unchanged. Does QEMU provide that
> guarantee? I'm fine with changing it but just want to make sure
> expectations are set correctly. Have you tested a downgrade and found
> that no other device impeded the process?

No I have not done this. The best we can do is that CRB at least is not the
reason that is causing such a failure and since we are introducing a new
device it need not be the reason, either.

   Stefan