| Message ID | 20230714154101.184057-2-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,v1,1/1] hw/tpm: TIS on sysbus: Remove unsupport ppi command line option | expand |
Hi Stefan, On 14/7/23 17:41, Stefan Berger 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> > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Message-id: 20230713171955.149236-1-stefanb@linux.ibm.com > --- > hw/tpm/tpm_tis_sysbus.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c > index 45e63efd63..6724b3d4f6 100644 > --- a/hw/tpm/tpm_tis_sysbus.c > +++ b/hw/tpm/tpm_tis_sysbus.c > @@ -93,7 +93,6 @@ static void tpm_tis_sysbus_reset(DeviceState *dev) > static Property tpm_tis_sysbus_properties[] = { > DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ), > DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver), > - DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false), Since properties are user-facing, shouldn't we deprecate their removal? I'm not sure so I ask :) Otherwise we could register the property with object_class_property_add_bool() and have the setter display a warning. Anyhow I suppose now setting "ppi" triggers some error, which is better than a abort. > DEFINE_PROP_END_OF_LIST(), > }; >
On 7/14/23 13:58, Philippe Mathieu-Daudé wrote: > Hi Stefan, > > On 14/7/23 17:41, Stefan Berger 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> >> Reviewed-by: Eric Auger <eric.auger@redhat.com> >> Message-id: 20230713171955.149236-1-stefanb@linux.ibm.com >> --- >> hw/tpm/tpm_tis_sysbus.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c >> index 45e63efd63..6724b3d4f6 100644 >> --- a/hw/tpm/tpm_tis_sysbus.c >> +++ b/hw/tpm/tpm_tis_sysbus.c >> @@ -93,7 +93,6 @@ static void tpm_tis_sysbus_reset(DeviceState *dev) >> static Property tpm_tis_sysbus_properties[] = { >> DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ), >> DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver), >> - DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false), > > Since properties are user-facing, shouldn't we deprecate their > removal? I'm not sure so I ask :) Otherwise we could register > the property with object_class_property_add_bool() and have > the setter display a warning. Anyhow I suppose now setting > "ppi" triggers some error, which is better than a abort. ppi=on crashed it, now it doesn't crash it. On the next level, ppi=on may come with the expectation that ppi is working on aarch64 and I am not sure about this. > >> DEFINE_PROP_END_OF_LIST(), >> }; >
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c index 45e63efd63..6724b3d4f6 100644 --- a/hw/tpm/tpm_tis_sysbus.c +++ b/hw/tpm/tpm_tis_sysbus.c @@ -93,7 +93,6 @@ static void tpm_tis_sysbus_reset(DeviceState *dev) static Property tpm_tis_sysbus_properties[] = { DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num, TPM_TIS_IRQ), DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver), - DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false), DEFINE_PROP_END_OF_LIST(), };