[PULL v1 0/1] Merge tpm 2023/07/14 v1

[PULL v1 0/1] Merge tpm 2023/07/14 v1

[Stefan Berger]

Hello!

  This PR removes the 'ppi' boolean property from the tpm tis sysbus
device. It could never be activated since it was leading to a segfault
immediatley.

    Stefan

The following changes since commit 3dd9e54703e6ae4f9ab3767f5cecc99edf066668:

  Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2023-07-12 20:46:10 +0100)

are available in the Git repository at:

  https://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2023-07-14-1

for you to fetch changes up to 4c46fe2ed492f35f411632c8b5a8442f322bc3f0:

  hw/tpm: TIS on sysbus: Remove unsupport ppi command line option (2023-07-14 11:31:54 -0400)


Stefan Berger (1):
  hw/tpm: TIS on sysbus: Remove unsupport ppi command line option

 hw/tpm/tpm_tis_sysbus.c | 1 -
 1 file changed, 1 deletion(-)

-- 
2.41.0

[PULL v1 1/1] hw/tpm: TIS on sysbus: Remove unsupport ppi command line option

[Stefan Berger]

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),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.41.0

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

[Philippe Mathieu-Daudé]

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(),
>   };
>   

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

[Stefan Berger]

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(),
>>   };
> 

Re: [PULL v1 0/1] Merge tpm 2023/07/14 v1

[Richard Henderson]

On 7/14/23 16:40, Stefan Berger wrote:
> Hello!
> 
>    This PR removes the 'ppi' boolean property from the tpm tis sysbus
> device. It could never be activated since it was leading to a segfault
> immediatley.
> 
>      Stefan
> 
> The following changes since commit 3dd9e54703e6ae4f9ab3767f5cecc99edf066668:
> 
>    Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2023-07-12 20:46:10 +0100)
> 
> are available in the Git repository at:
> 
>    https://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2023-07-14-1
> 
> for you to fetch changes up to 4c46fe2ed492f35f411632c8b5a8442f322bc3f0:
> 
>    hw/tpm: TIS on sysbus: Remove unsupport ppi command line option (2023-07-14 11:31:54 -0400)
> 
> 
> Stefan Berger (1):
>    hw/tpm: TIS on sysbus: Remove unsupport ppi command line option
> 
>   hw/tpm/tpm_tis_sysbus.c | 1 -
>   1 file changed, 1 deletion(-)
> 

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate.


r~