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

Philippe Mathieu-Daudé posted 5 patches 2 weeks, 6 days ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Stefan Berger <stefanb@linux.vnet.ibm.com>
[PATCH v3 4/5] hw/tpm: Propagate @ppi_enabled to tpm_tis_reset() and remove in TPMState
Posted by Philippe Mathieu-Daudé 2 weeks, 6 days ago
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


Re: [PATCH v3 4/5] hw/tpm: Propagate @ppi_enabled to tpm_tis_reset() and remove in TPMState
Posted by Stefan Berger 2 weeks, 6 days ago

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>