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

[Stefan Berger]

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