If 'ppi' property is set, then `tpm_ppi_reset` is called on reset
which SEGFAULTs because `tpmppi->buf` is not allocated.

Signed-off-by: Joelle van Dyne <j@getutm.app>
---
 hw/tpm/tpm_tis_sysbus.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index 45e63efd63..1014d5d993 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -124,6 +124,10 @@ static void tpm_tis_sysbus_realizefn(DeviceState *dev, Error **errp)
         error_setg(errp, "'tpmdev' property is required");
         return;
     }
+
+    if (s->ppi_enabled) {
+        sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->ppi.ram);
+    }
 }
 
 static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
-- 
2.39.2 (Apple Git-143)

On 7/12/23 23:51, Joelle van Dyne wrote:
> If 'ppi' property is set, then `tpm_ppi_reset` is called on reset
> which SEGFAULTs because `tpmppi->buf` is not allocated.
> 
> Signed-off-by: Joelle van Dyne <j@getutm.app>
> ---
>   hw/tpm/tpm_tis_sysbus.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index 45e63efd63..1014d5d993 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -124,6 +124,10 @@ static void tpm_tis_sysbus_realizefn(DeviceState *dev, Error **errp)
>           error_setg(errp, "'tpmdev' property is required");
>           return;
>       }
> +
> +    if (s->ppi_enabled) {
> +        sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->ppi.ram);
> +    }
>   }
> 

The tpm-tis-device doesn't work for x86_64 but for aarch64.


We have this here in this file:

     DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false),

I don't know whether ppi would work on aarch64. It needs firmware support like in edk2.
I think the best solution is to remove this DEFINE_PROP_BOOL() and if someone wants
to enable it they would have to add firmware support and test it before re-enabling it.

    Stefan

>   static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)

On Thu, Jul 13, 2023 at 9:49 AM Stefan Berger <stefanb@linux.ibm.com> wrote:
>
>
> The tpm-tis-device doesn't work for x86_64 but for aarch64.
>
>
> We have this here in this file:
>
>      DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false),
>
> I don't know whether ppi would work on aarch64. It needs firmware support like in edk2.
> I think the best solution is to remove this DEFINE_PROP_BOOL() and if someone wants
> to enable it they would have to add firmware support and test it before re-enabling it.
>
>     Stefan
>
> >   static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)

Yeah, I'm not sure if PPI works with AARCH64 since I didn't bother to
change it to not use hard coded addresses. However, isn't that "ppi"
overridable from the command line? If so, should we add a check in
"realize" to error if PPI=true? Otherwise, it will just crash.

On 7/13/23 14:15, Joelle van Dyne wrote:
> On Thu, Jul 13, 2023 at 9:49 AM Stefan Berger <stefanb@linux.ibm.com> wrote:
>>
>>
>> The tpm-tis-device doesn't work for x86_64 but for aarch64.
>>
>>
>> We have this here in this file:
>>
>>       DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, false),
>>
>> I don't know whether ppi would work on aarch64. It needs firmware support like in edk2.
>> I think the best solution is to remove this DEFINE_PROP_BOOL() and if someone wants
>> to enable it they would have to add firmware support and test it before re-enabling it.
>>
>>      Stefan
>>
>>>    static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
> 
> Yeah, I'm not sure if PPI works with AARCH64 since I didn't bother to
> change it to not use hard coded addresses. However, isn't that "ppi"
> overridable from the command line? If so, should we add a check in
> "realize" to error if PPI=true? Otherwise, it will just crash.

Once the option is removed via my patch (cc'ed you), then you get this once you pass ppi=on on the command line:

qemu-system-aarch64: -device tpm-tis-device,tpmdev=tpm0,ppi=on: Property 'tpm-tis-device.ppi' not found

This disables it for good.

    Stefan