[PATCH v3 0/5] hw/tpm: Remove instance @ppi_enabled field

[PATCH v3 0/5] hw/tpm: Remove instance @ppi_enabled field

[Philippe Mathieu-Daudé]

The "ppi" property and @ppi_enabled field were only used
for backward compatibility during the 3.1 release. We
removed all these legacy machines: the property isn't
required anymore. Still it was how the ACPI subsystem
was checking PPI use by TPM devices. This series convert
the per-instance field exposed as QOM property as a
single class (actually QOM interface) one.

Philippe Mathieu-Daudé (5):
  hw/tpm: Factor tpm_ppi_enabled() out
  hw/tpm: Add TPMIfClass::ppi_enabled field
  hw/tpm: Remove CRBState::ppi_enabled field
  hw/tpm: Propagate @ppi_enabled to tpm_tis_reset() and remove in
    TPMState
  hw/tpm: Simplify tpm_ppi_enabled()

 hw/tpm/tpm_tis.h        |  3 +--
 include/system/tpm.h    |  9 +++++++++
 hw/acpi/tpm.c           |  2 +-
 hw/i386/acpi-build.c    |  2 +-
 hw/tpm/tpm_crb.c        | 13 ++++---------
 hw/tpm/tpm_tis_common.c |  4 ++--
 hw/tpm/tpm_tis_i2c.c    |  2 +-
 hw/tpm/tpm_tis_isa.c    | 10 ++++------
 hw/tpm/tpm_tis_sysbus.c |  2 +-
 9 files changed, 24 insertions(+), 23 deletions(-)

-- 
2.53.0

[PATCH v3 1/5] hw/tpm: Factor tpm_ppi_enabled() out

[Philippe Mathieu-Daudé]

Factor tpm_ppi_enabled() out before modifying it in a unique place.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/tpm.h | 9 +++++++++
 hw/acpi/tpm.c        | 2 +-
 hw/i386/acpi-build.c | 2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/system/tpm.h b/include/system/tpm.h
index 1ee568b3b62..b90dd4e8cb0 100644
--- a/include/system/tpm.h
+++ b/include/system/tpm.h
@@ -13,6 +13,7 @@
 #define QEMU_TPM_H
 
 #include "qapi/qapi-types-tpm.h"
+#include "qapi/error.h"
 #include "qom/object.h"
 
 #ifdef CONFIG_TPM
@@ -78,6 +79,14 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
     return TPM_IF_GET_CLASS(ti)->get_version(ti);
 }
 
+static inline bool tpm_ppi_enabled(TPMIf *ti)
+{
+    if (!ti) {
+        return false;
+    }
+    return object_property_get_bool(OBJECT(ti), "ppi", &error_abort);
+}
+
 #else /* CONFIG_TPM */
 
 #define tpm_init()  (0)
diff --git a/hw/acpi/tpm.c b/hw/acpi/tpm.c
index cdc02275365..5fe95f2e3f1 100644
--- a/hw/acpi/tpm.c
+++ b/hw/acpi/tpm.c
@@ -25,7 +25,7 @@ void tpm_build_ppi_acpi(TPMIf *tpm, Aml *dev)
     Aml *method, *field, *ifctx, *ifctx2, *ifctx3, *func_mask,
         *not_implemented, *pak, *tpm2, *tpm3, *pprm, *pprq, *zero, *one;
 
-    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
+    if (!tpm_ppi_enabled(tpm)) {
         return;
     }
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f622b91b76a..4f01e2c476e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2218,7 +2218,7 @@ void acpi_setup(void)
                     tables.tcpalog->data, acpi_data_len(tables.tcpalog));
 
     tpm = tpm_find();
-    if (tpm && object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
+    if (tpm_ppi_enabled(tpm)) {
         tpm_config = (FwCfgTPMConfig) {
             .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
             .tpm_version = tpm_get_version(tpm),
-- 
2.53.0

[PATCH v3 2/5] hw/tpm: Add TPMIfClass::ppi_enabled field

[Philippe Mathieu-Daudé]

Each TMP derived device has a @ppi_enabled field, itself
exposed as a QOM property. External layers (like the ACPI
subsystem) wanting to know whether a device implements PPI
has to check for the QOM property available. This can be
simplified by declaring a single field in the TPM interface.

Here we add such field to TPMIfClass, before converting each
TPM devices to use it in the following commits.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/tpm.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/system/tpm.h b/include/system/tpm.h
index b90dd4e8cb0..9458ad6668a 100644
--- a/include/system/tpm.h
+++ b/include/system/tpm.h
@@ -43,6 +43,7 @@ struct TPMIfClass {
     enum TpmModel model;
     void (*request_completed)(TPMIf *obj, int ret);
     enum TPMVersion (*get_version)(TPMIf *obj);
+    bool ppi_enabled;
 };
 
 #define TYPE_TPM_TIS_ISA            "tpm-tis"
@@ -84,6 +85,9 @@ static inline bool tpm_ppi_enabled(TPMIf *ti)
     if (!ti) {
         return false;
     }
+    if (TPM_IF_GET_CLASS(ti)->ppi_enabled) {
+        return true;
+    }
     return object_property_get_bool(OBJECT(ti), "ppi", &error_abort);
 }
 
-- 
2.53.0

[PATCH v3 3/5] hw/tpm: Remove CRBState::ppi_enabled field

[Philippe Mathieu-Daudé]

The CRBState::ppi_enabled boolean was only set in the
hw_compat_3_1[] array, via the 'ppi=false' property.
We removed all machines using that array, and the array
itself in commit a861ffef237 ("hw/core/machine: Remove
the hw_compat_3_1[] array"). We can safely remove the
now unused property. Since CRB devices always use PPI,
simplify removing the CRBState::ppi_enabled field.
Set the generic TPMIfClass::ppi_enabled so ACPI subsystem
can keep checking its availability.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/tpm/tpm_crb.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 8723536f931..02701ab9480 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -43,7 +43,6 @@ struct CRBState {
 
     size_t be_buffer_size;
 
-    bool ppi_enabled;
     TPMPPI ppi;
 };
 typedef struct CRBState CRBState;
@@ -228,16 +227,13 @@ static const VMStateDescription vmstate_tpm_crb = {
 
 static const Property tpm_crb_properties[] = {
     DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe),
-    DEFINE_PROP_BOOL("ppi", CRBState, ppi_enabled, true),
 };
 
 static void tpm_crb_reset(void *dev)
 {
     CRBState *s = CRB(dev);
 
-    if (s->ppi_enabled) {
-        tpm_ppi_reset(&s->ppi);
-    }
+    tpm_ppi_reset(&s->ppi);
     tpm_backend_reset(s->tpmbe);
 
     memset(s->regs, 0, sizeof(s->regs));
@@ -303,10 +299,8 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(),
         TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
 
-    if (s->ppi_enabled) {
-        tpm_ppi_init(&s->ppi, get_system_memory(),
-                     TPM_PPI_ADDR_BASE, OBJECT(s));
-    }
+    tpm_ppi_init(&s->ppi, get_system_memory(),
+                 TPM_PPI_ADDR_BASE, OBJECT(s));
 
     if (xen_enabled()) {
         tpm_crb_reset(dev);
@@ -325,6 +319,7 @@ static void tpm_crb_class_init(ObjectClass *klass, const void *data)
     dc->vmsd  = &vmstate_tpm_crb;
     dc->user_creatable = true;
     tc->model = TPM_MODEL_TPM_CRB;
+    tc->ppi_enabled = true;
     tc->get_version = tpm_crb_get_version;
     tc->request_completed = tpm_crb_request_completed;
 
-- 
2.53.0

[PATCH v3 4/5] hw/tpm: Propagate @ppi_enabled to tpm_tis_reset() and remove in TPMState

[Philippe Mathieu-Daudé]

Of TMP devices using FIFO mode, only the ISA variant has PPI,
and call tpm_ppi_init() to initialize the PPI state. Propagate
@ppi_enabled to tpm_tis_reset() so it only resets the PPI part
when requested (ISA case) otherwise the PPI is in uninitialized
state. Remove the now unused TPMState::ppi_enabled field. Set
the generic TPMIfClass::ppi_enabled so ACPI subsystem can keep
checking its availability.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/tpm/tpm_tis.h        |  3 +--
 hw/tpm/tpm_tis_common.c |  4 ++--
 hw/tpm/tpm_tis_i2c.c    |  2 +-
 hw/tpm/tpm_tis_isa.c    | 10 ++++------
 hw/tpm/tpm_tis_sysbus.c |  2 +-
 5 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index 184632ff66b..1531620bf9d 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -75,7 +75,6 @@ typedef struct TPMState {
 
     size_t be_buffer_size;
 
-    bool ppi_enabled;
     TPMPPI ppi;
 } TPMState;
 
@@ -83,7 +82,7 @@ extern const VMStateDescription vmstate_locty;
 extern const MemoryRegionOps tpm_tis_memory_ops;
 
 int tpm_tis_pre_save(TPMState *s);
-void tpm_tis_reset(TPMState *s);
+void tpm_tis_reset(TPMState *s, bool ppi_enabled);
 enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
 void tpm_tis_request_completed(TPMState *s, int ret);
 uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index f594b15b8ab..a134d5c2059 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -813,7 +813,7 @@ enum TPMVersion tpm_tis_get_tpm_version(TPMState *s)
  * This function is called when the machine starts, resets or due to
  * S3 resume.
  */
-void tpm_tis_reset(TPMState *s)
+void tpm_tis_reset(TPMState *s, bool ppi_enabled)
 {
     int c;
 
@@ -821,7 +821,7 @@ void tpm_tis_reset(TPMState *s)
     s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
                             TPM_TIS_BUFFER_MAX);
 
-    if (s->ppi_enabled) {
+    if (ppi_enabled) {
         tpm_ppi_reset(&s->ppi);
     }
     tpm_backend_reset(s->be_driver);
diff --git a/hw/tpm/tpm_tis_i2c.c b/hw/tpm/tpm_tis_i2c.c
index 9f13e0ec120..b4f258c7bcb 100644
--- a/hw/tpm/tpm_tis_i2c.c
+++ b/hw/tpm/tpm_tis_i2c.c
@@ -523,7 +523,7 @@ static void tpm_tis_i2c_reset(DeviceState *dev)
     i2cst->csum_enable = 0;
     i2cst->loc_sel = 0x00;
 
-    return tpm_tis_reset(s);
+    return tpm_tis_reset(s, false);
 }
 
 static void tpm_tis_i2c_class_init(ObjectClass *klass, const void *data)
diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 61e95434f5b..1ca403241de 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -88,13 +88,12 @@ static void tpm_tis_isa_reset(DeviceState *dev)
     TPMStateISA *isadev = TPM_TIS_ISA(dev);
     TPMState *s = &isadev->state;
 
-    return tpm_tis_reset(s);
+    return tpm_tis_reset(s, true);
 }
 
 static const Property tpm_tis_isa_properties[] = {
     DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
     DEFINE_PROP_TPMBE("tpmdev", TPMStateISA, state.be_driver),
-    DEFINE_PROP_BOOL("ppi", TPMStateISA, state.ppi_enabled, true),
 };
 
 static void tpm_tis_isa_initfn(Object *obj)
@@ -132,10 +131,8 @@ static void tpm_tis_isa_realizefn(DeviceState *dev, Error **errp)
     memory_region_add_subregion(isa_address_space(ISA_DEVICE(dev)),
                                 TPM_TIS_ADDR_BASE, &s->mmio);
 
-    if (s->ppi_enabled) {
-        tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
-                     TPM_PPI_ADDR_BASE, OBJECT(dev));
-    }
+    tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
+                 TPM_PPI_ADDR_BASE, OBJECT(dev));
 }
 
 static void build_tpm_tis_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -175,6 +172,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, const void *data)
     device_class_set_props(dc, tpm_tis_isa_properties);
     dc->vmsd  = &vmstate_tpm_tis_isa;
     tc->model = TPM_MODEL_TPM_TIS;
+    tc->ppi_enabled = true;
     dc->realize = tpm_tis_isa_realizefn;
     device_class_set_legacy_reset(dc, tpm_tis_isa_reset);
     tc->request_completed = tpm_tis_isa_request_completed;
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index e9372e73163..dd30344d5ac 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -87,7 +87,7 @@ static void tpm_tis_sysbus_reset(DeviceState *dev)
     TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
     TPMState *s = &sbdev->state;
 
-    return tpm_tis_reset(s);
+    return tpm_tis_reset(s, false);
 }
 
 static const Property tpm_tis_sysbus_properties[] = {
-- 
2.53.0

[PATCH v3 5/5] hw/tpm: Simplify tpm_ppi_enabled()

[Philippe Mathieu-Daudé]

TPM instances don't expose any "ppi" property anymore,
remove that dead code.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/tpm.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/include/system/tpm.h b/include/system/tpm.h
index 9458ad6668a..874068d19a9 100644
--- a/include/system/tpm.h
+++ b/include/system/tpm.h
@@ -13,7 +13,6 @@
 #define QEMU_TPM_H
 
 #include "qapi/qapi-types-tpm.h"
-#include "qapi/error.h"
 #include "qom/object.h"
 
 #ifdef CONFIG_TPM
@@ -85,10 +84,7 @@ static inline bool tpm_ppi_enabled(TPMIf *ti)
     if (!ti) {
         return false;
     }
-    if (TPM_IF_GET_CLASS(ti)->ppi_enabled) {
-        return true;
-    }
-    return object_property_get_bool(OBJECT(ti), "ppi", &error_abort);
+    return TPM_IF_GET_CLASS(ti)->ppi_enabled;
 }
 
 #else /* CONFIG_TPM */
-- 
2.53.0

Re: [PATCH v3 2/5] hw/tpm: Add TPMIfClass::ppi_enabled field

[Stefan Berger]

On 3/17/26 8:02 AM, Philippe Mathieu-Daudé wrote:
> Each TMP derived device has a @ppi_enabled field, itself

TPM?

> exposed as a QOM property. External layers (like the ACPI
> subsystem) wanting to know whether a device implements PPI
> has to check for the QOM property available. This can be
> simplified by declaring a single field in the TPM interface.
> 
> Here we add such field to TPMIfClass, before converting each
> TPM devices to use it in the following commits.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/system/tpm.h | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/include/system/tpm.h b/include/system/tpm.h
> index b90dd4e8cb0..9458ad6668a 100644
> --- a/include/system/tpm.h
> +++ b/include/system/tpm.h
> @@ -43,6 +43,7 @@ struct TPMIfClass {
>       enum TpmModel model;
>       void (*request_completed)(TPMIf *obj, int ret);
>       enum TPMVersion (*get_version)(TPMIf *obj);
> +    bool ppi_enabled;
>   };
> 
>   #define TYPE_TPM_TIS_ISA            "tpm-tis"
> @@ -84,6 +85,9 @@ static inline bool tpm_ppi_enabled(TPMIf *ti)
>       if (!ti) {
>           return false;
>       }
> +    if (TPM_IF_GET_CLASS(ti)->ppi_enabled) {
> +        return true;
> +    }
>       return object_property_get_bool(OBJECT(ti), "ppi", &error_abort);
>   }
> 

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Re: [PATCH v3 1/5] hw/tpm: Factor tpm_ppi_enabled() out

[Stefan Berger]

On 3/17/26 8:02 AM, Philippe Mathieu-Daudé wrote:
> Factor tpm_ppi_enabled() out before modifying it in a unique place.

Factor-out ...

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/system/tpm.h | 9 +++++++++
>   hw/acpi/tpm.c        | 2 +-
>   hw/i386/acpi-build.c | 2 +-
>   3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/include/system/tpm.h b/include/system/tpm.h
> index 1ee568b3b62..b90dd4e8cb0 100644
> --- a/include/system/tpm.h
> +++ b/include/system/tpm.h
> @@ -13,6 +13,7 @@
>   #define QEMU_TPM_H
> 
>   #include "qapi/qapi-types-tpm.h"
> +#include "qapi/error.h"
>   #include "qom/object.h"
> 
>   #ifdef CONFIG_TPM
> @@ -78,6 +79,14 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
>       return TPM_IF_GET_CLASS(ti)->get_version(ti);
>   }
> 
> +static inline bool tpm_ppi_enabled(TPMIf *ti)
> +{
> +    if (!ti) {
> +        return false;
> +    }
> +    return object_property_get_bool(OBJECT(ti), "ppi", &error_abort);
> +}
> +
>   #else /* CONFIG_TPM */
> 
>   #define tpm_init()  (0)
> diff --git a/hw/acpi/tpm.c b/hw/acpi/tpm.c
> index cdc02275365..5fe95f2e3f1 100644
> --- a/hw/acpi/tpm.c
> +++ b/hw/acpi/tpm.c
> @@ -25,7 +25,7 @@ void tpm_build_ppi_acpi(TPMIf *tpm, Aml *dev)
>       Aml *method, *field, *ifctx, *ifctx2, *ifctx3, *func_mask,
>           *not_implemented, *pak, *tpm2, *tpm3, *pprm, *pprq, *zero, *one;
> 
> -    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
> +    if (!tpm_ppi_enabled(tpm)) {
>           return;
>       }
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f622b91b76a..4f01e2c476e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2218,7 +2218,7 @@ void acpi_setup(void)
>                       tables.tcpalog->data, acpi_data_len(tables.tcpalog));
> 
>       tpm = tpm_find();
> -    if (tpm && object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
> +    if (tpm_ppi_enabled(tpm)) {
>           tpm_config = (FwCfgTPMConfig) {
>               .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
>               .tpm_version = tpm_get_version(tpm),

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Re: [PATCH v3 3/5] hw/tpm: Remove CRBState::ppi_enabled field

[Stefan Berger]

On 3/17/26 8:02 AM, Philippe Mathieu-Daudé wrote:
> The CRBState::ppi_enabled boolean was only set in the
> hw_compat_3_1[] array, via the 'ppi=false' property.
> We removed all machines using that array, and the array
> itself in commit a861ffef237 ("hw/core/machine: Remove
> the hw_compat_3_1[] array"). We can safely remove the
> now unused property. Since CRB devices always use PPI,

s/use/uses

> simplify removing the CRBState::ppi_enabled field.
> Set the generic TPMIfClass::ppi_enabled so ACPI subsystem
> can keep checking its availability.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/tpm/tpm_crb.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index 8723536f931..02701ab9480 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -43,7 +43,6 @@ struct CRBState {
> 
>       size_t be_buffer_size;
> 
> -    bool ppi_enabled;
>       TPMPPI ppi;
>   };
>   typedef struct CRBState CRBState;
> @@ -228,16 +227,13 @@ static const VMStateDescription vmstate_tpm_crb = {
> 
>   static const Property tpm_crb_properties[] = {
>       DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe),
> -    DEFINE_PROP_BOOL("ppi", CRBState, ppi_enabled, true),
>   };
> 
>   static void tpm_crb_reset(void *dev)
>   {
>       CRBState *s = CRB(dev);
> 
> -    if (s->ppi_enabled) {
> -        tpm_ppi_reset(&s->ppi);
> -    }
> +    tpm_ppi_reset(&s->ppi);
>       tpm_backend_reset(s->tpmbe);
> 
>       memset(s->regs, 0, sizeof(s->regs));
> @@ -303,10 +299,8 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>       memory_region_add_subregion(get_system_memory(),
>           TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
> 
> -    if (s->ppi_enabled) {
> -        tpm_ppi_init(&s->ppi, get_system_memory(),
> -                     TPM_PPI_ADDR_BASE, OBJECT(s));
> -    }
> +    tpm_ppi_init(&s->ppi, get_system_memory(),
> +                 TPM_PPI_ADDR_BASE, OBJECT(s));
> 
>       if (xen_enabled()) {
>           tpm_crb_reset(dev);
> @@ -325,6 +319,7 @@ static void tpm_crb_class_init(ObjectClass *klass, const void *data)
>       dc->vmsd  = &vmstate_tpm_crb;
>       dc->user_creatable = true;
>       tc->model = TPM_MODEL_TPM_CRB;
> +    tc->ppi_enabled = true;
>       tc->get_version = tpm_crb_get_version;
>       tc->request_completed = tpm_crb_request_completed;
> 

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Re: [PATCH v3 4/5] hw/tpm: Propagate @ppi_enabled to tpm_tis_reset() and remove in TPMState

[Stefan Berger]

On 3/17/26 8:02 AM, Philippe Mathieu-Daudé wrote:
> Of TMP devices using FIFO mode, only the ISA variant has PPI,

Of the TPM devices ...

> and call tpm_ppi_init() to initialize the PPI state. Propagate

s/call/calls

> @ppi_enabled to tpm_tis_reset() so it only resets the PPI part
> when requested (ISA case) otherwise the PPI is in uninitialized
> state. Remove the now unused TPMState::ppi_enabled field. Set
> the generic TPMIfClass::ppi_enabled so ACPI subsystem can keep
> checking its availability.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/tpm/tpm_tis.h        |  3 +--
>   hw/tpm/tpm_tis_common.c |  4 ++--
>   hw/tpm/tpm_tis_i2c.c    |  2 +-
>   hw/tpm/tpm_tis_isa.c    | 10 ++++------
>   hw/tpm/tpm_tis_sysbus.c |  2 +-
>   5 files changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
> index 184632ff66b..1531620bf9d 100644
> --- a/hw/tpm/tpm_tis.h
> +++ b/hw/tpm/tpm_tis.h
> @@ -75,7 +75,6 @@ typedef struct TPMState {
>   
>       size_t be_buffer_size;
>   
> -    bool ppi_enabled;
>       TPMPPI ppi;
>   } TPMState;
>   
> @@ -83,7 +82,7 @@ extern const VMStateDescription vmstate_locty;
>   extern const MemoryRegionOps tpm_tis_memory_ops;
>   
>   int tpm_tis_pre_save(TPMState *s);
> -void tpm_tis_reset(TPMState *s);
> +void tpm_tis_reset(TPMState *s, bool ppi_enabled);
>   enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
>   void tpm_tis_request_completed(TPMState *s, int ret);
>   uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
> diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
> index f594b15b8ab..a134d5c2059 100644
> --- a/hw/tpm/tpm_tis_common.c
> +++ b/hw/tpm/tpm_tis_common.c
> @@ -813,7 +813,7 @@ enum TPMVersion tpm_tis_get_tpm_version(TPMState *s)
>    * This function is called when the machine starts, resets or due to
>    * S3 resume.
>    */
> -void tpm_tis_reset(TPMState *s)
> +void tpm_tis_reset(TPMState *s, bool ppi_enabled)
>   {
>       int c;
>   
> @@ -821,7 +821,7 @@ void tpm_tis_reset(TPMState *s)
>       s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
>                               TPM_TIS_BUFFER_MAX);
>   
> -    if (s->ppi_enabled) {
> +    if (ppi_enabled) {
>           tpm_ppi_reset(&s->ppi);
>       }
>       tpm_backend_reset(s->be_driver);
> diff --git a/hw/tpm/tpm_tis_i2c.c b/hw/tpm/tpm_tis_i2c.c
> index 9f13e0ec120..b4f258c7bcb 100644
> --- a/hw/tpm/tpm_tis_i2c.c
> +++ b/hw/tpm/tpm_tis_i2c.c
> @@ -523,7 +523,7 @@ static void tpm_tis_i2c_reset(DeviceState *dev)
>       i2cst->csum_enable = 0;
>       i2cst->loc_sel = 0x00;
>   
> -    return tpm_tis_reset(s);
> +    return tpm_tis_reset(s, false);
>   }
>   
>   static void tpm_tis_i2c_class_init(ObjectClass *klass, const void *data)
> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
> index 61e95434f5b..1ca403241de 100644
> --- a/hw/tpm/tpm_tis_isa.c
> +++ b/hw/tpm/tpm_tis_isa.c
> @@ -88,13 +88,12 @@ static void tpm_tis_isa_reset(DeviceState *dev)
>       TPMStateISA *isadev = TPM_TIS_ISA(dev);
>       TPMState *s = &isadev->state;
>   
> -    return tpm_tis_reset(s);
> +    return tpm_tis_reset(s, true);
>   }
>   
>   static const Property tpm_tis_isa_properties[] = {
>       DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
>       DEFINE_PROP_TPMBE("tpmdev", TPMStateISA, state.be_driver),
> -    DEFINE_PROP_BOOL("ppi", TPMStateISA, state.ppi_enabled, true),
>   };
>   
>   static void tpm_tis_isa_initfn(Object *obj)
> @@ -132,10 +131,8 @@ static void tpm_tis_isa_realizefn(DeviceState *dev, Error **errp)
>       memory_region_add_subregion(isa_address_space(ISA_DEVICE(dev)),
>                                   TPM_TIS_ADDR_BASE, &s->mmio);
>   
> -    if (s->ppi_enabled) {
> -        tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
> -                     TPM_PPI_ADDR_BASE, OBJECT(dev));
> -    }
> +    tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
> +                 TPM_PPI_ADDR_BASE, OBJECT(dev));
>   }
>   
>   static void build_tpm_tis_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
> @@ -175,6 +172,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, const void *data)
>       device_class_set_props(dc, tpm_tis_isa_properties);
>       dc->vmsd  = &vmstate_tpm_tis_isa;
>       tc->model = TPM_MODEL_TPM_TIS;
> +    tc->ppi_enabled = true;
>       dc->realize = tpm_tis_isa_realizefn;
>       device_class_set_legacy_reset(dc, tpm_tis_isa_reset);
>       tc->request_completed = tpm_tis_isa_request_completed;
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index e9372e73163..dd30344d5ac 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -87,7 +87,7 @@ static void tpm_tis_sysbus_reset(DeviceState *dev)
>       TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
>       TPMState *s = &sbdev->state;
>   
> -    return tpm_tis_reset(s);
> +    return tpm_tis_reset(s, false);
>   }
>   
>   static const Property tpm_tis_sysbus_properties[] = {

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Re: [PATCH v3 5/5] hw/tpm: Simplify tpm_ppi_enabled()

[Stefan Berger]

On 3/17/26 8:02 AM, Philippe Mathieu-Daudé wrote:
> TPM instances don't expose any "ppi" property anymore,
> remove that dead code.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/system/tpm.h | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/include/system/tpm.h b/include/system/tpm.h
> index 9458ad6668a..874068d19a9 100644
> --- a/include/system/tpm.h
> +++ b/include/system/tpm.h
> @@ -13,7 +13,6 @@
>   #define QEMU_TPM_H
>   
>   #include "qapi/qapi-types-tpm.h"
> -#include "qapi/error.h"
>   #include "qom/object.h"
>   
>   #ifdef CONFIG_TPM
> @@ -85,10 +84,7 @@ static inline bool tpm_ppi_enabled(TPMIf *ti)
>       if (!ti) {
>           return false;
>       }
> -    if (TPM_IF_GET_CLASS(ti)->ppi_enabled) {
> -        return true;
> -    }
> -    return object_property_get_bool(OBJECT(ti), "ppi", &error_abort);
> +    return TPM_IF_GET_CLASS(ti)->ppi_enabled;
>   }
>   
>   #else /* CONFIG_TPM */

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Re: [PATCH v3 2/5] hw/tpm: Add TPMIfClass::ppi_enabled field

[Philippe Mathieu-Daudé]

On 17/3/26 14:17, Stefan Berger wrote:
> 
> 
> On 3/17/26 8:02 AM, Philippe Mathieu-Daudé wrote:
>> Each TMP derived device has a @ppi_enabled field, itself
> 
> TPM?

Finger crossed =)

> 
>> exposed as a QOM property. External layers (like the ACPI
>> subsystem) wanting to know whether a device implements PPI
>> has to check for the QOM property available. This can be
>> simplified by declaring a single field in the TPM interface.
>>
>> Here we add such field to TPMIfClass, before converting each
>> TPM devices to use it in the following commits.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/system/tpm.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/include/system/tpm.h b/include/system/tpm.h
>> index b90dd4e8cb0..9458ad6668a 100644
>> --- a/include/system/tpm.h
>> +++ b/include/system/tpm.h
>> @@ -43,6 +43,7 @@ struct TPMIfClass {
>>       enum TpmModel model;
>>       void (*request_completed)(TPMIf *obj, int ret);
>>       enum TPMVersion (*get_version)(TPMIf *obj);
>> +    bool ppi_enabled;
>>   };
>>
>>   #define TYPE_TPM_TIS_ISA            "tpm-tis"
>> @@ -84,6 +85,9 @@ static inline bool tpm_ppi_enabled(TPMIf *ti)
>>       if (!ti) {
>>           return false;
>>       }
>> +    if (TPM_IF_GET_CLASS(ti)->ppi_enabled) {
>> +        return true;
>> +    }
>>       return object_property_get_bool(OBJECT(ti), "ppi", &error_abort);
>>   }
>>
> 
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> 

Thanks!

Re: [PATCH v3 3/5] hw/tpm: Remove CRBState::ppi_enabled field

[Philippe Mathieu-Daudé]

On 17/3/26 14:19, Stefan Berger wrote:
> 
> 
> On 3/17/26 8:02 AM, Philippe Mathieu-Daudé wrote:
>> The CRBState::ppi_enabled boolean was only set in the
>> hw_compat_3_1[] array, via the 'ppi=false' property.
>> We removed all machines using that array, and the array
>> itself in commit a861ffef237 ("hw/core/machine: Remove
>> the hw_compat_3_1[] array"). We can safely remove the
>> now unused property. Since CRB devices always use PPI,
> 
> s/use/uses

I used plural form. To you rather singular?

   "Since the CRB device always use PPI, ..."

> 
>> simplify removing the CRBState::ppi_enabled field.
>> Set the generic TPMIfClass::ppi_enabled so ACPI subsystem
>> can keep checking its availability.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/tpm/tpm_crb.c | 13 ++++---------
>>   1 file changed, 4 insertions(+), 9 deletions(-)

Re: [PATCH v3 3/5] hw/tpm: Remove CRBState::ppi_enabled field

[Stefan Berger]

On 3/17/26 11:16 AM, Philippe Mathieu-Daudé wrote:
> On 17/3/26 14:19, Stefan Berger wrote:
>>
>>
>> On 3/17/26 8:02 AM, Philippe Mathieu-Daudé wrote:
>>> The CRBState::ppi_enabled boolean was only set in the
>>> hw_compat_3_1[] array, via the 'ppi=false' property.
>>> We removed all machines using that array, and the array
>>> itself in commit a861ffef237 ("hw/core/machine: Remove
>>> the hw_compat_3_1[] array"). We can safely remove the
>>> now unused property. Since CRB devices always use PPI,
>>
>> s/use/uses
> 
> I used plural form. To you rather singular?

my error

> 
>    "Since the CRB device always use PPI, ..>
>>
>>> simplify removing the CRBState::ppi_enabled field.
>>> Set the generic TPMIfClass::ppi_enabled so ACPI subsystem
>>> can keep checking its availability.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/tpm/tpm_crb.c | 13 ++++---------
>>>   1 file changed, 4 insertions(+), 9 deletions(-)
> 

Re: [PATCH v3 0/5] hw/tpm: Remove instance @ppi_enabled field

[Philippe Mathieu-Daudé]

On 17/3/26 13:02, Philippe Mathieu-Daudé wrote:

> Philippe Mathieu-Daudé (5):
>    hw/tpm: Factor tpm_ppi_enabled() out
>    hw/tpm: Add TPMIfClass::ppi_enabled field
>    hw/tpm: Remove CRBState::ppi_enabled field
>    hw/tpm: Propagate @ppi_enabled to tpm_tis_reset() and remove in
>      TPMState
>    hw/tpm: Simplify tpm_ppi_enabled()

Series queued for 11.1 (typo fixed), thanks.