Re: [PATCH] hw/tpm: TIS on sysbus: Remove unsupport ppi command line option

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

On 7/14/23 09:51, Eric Auger wrote:
> Hi Stefan,
> On 7/14/23 13:51, Stefan Berger wrote:
>>
>>
>> On 7/14/23 02:07, Joelle van Dyne wrote:
>>> On Thu, Jul 13, 2023 at 10:20 AM Stefan Berger
>>> <stefanb@linux.ibm.com> wrote:
>>>>
>>>> The ppi command line option for the TIS device on sysbus never worked
>>>> and caused an immediate segfault. Remove support for it since it also
>>>> needs support in the firmware and needs testing inside the VM.
>>>>
>>>> Reproducer with the ppi=on option passed:
>>>>
>>>> qemu-system-aarch64 \
>>>>      -machine virt,gic-version=3 \
>>>>      -m 4G  \
>>>>      -nographic -no-acpi \
>>>>      -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
>>>>      -tpmdev emulator,id=tpm0,chardev=chrtpm \
>>>>      -device tpm-tis-device,tpmdev=tpm0,ppi=on
>>>> [...]
>>>> Segmentation fault (core dumped)
>>>>
>>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>>>
>>> Do you need to add a VMSTATE_UNUSED_TEST in case a future QEMU version
>>> introduces a new field in the same position which will cause an issue
>>> when restoring from an older version?
>>
>> Hm, you got a point there. We will have to error-out in case someone
>> sets ppi=on instead since the expectation that PPI would work is
>> simply not there. v2 coming soon.
> as Joelle pointed it out ppi_enabled is not part of
> vmstate_tpm_tis_sysbus fields. And since it has never worked I suspect
> we cannot have any existing VM enabling it. So I don't get the issue
> with this 1st version?

You are right. I repeated my test with restoring state of a VM taken before the removal of this field and it restored it. So that other patch is good and I am withdrawing this patch here.

     Stefan

> 
> Thanks
> 
> Eric
>>
>>      Stefan
>>
>