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

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 3/5] hw/tpm: Remove CRBState::ppi_enabled field
Posted by Philippe Mathieu-Daudé 2 weeks, 6 days ago
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


Re: [PATCH v3 3/5] hw/tpm: Remove CRBState::ppi_enabled field
Posted by Stefan Berger 2 weeks, 6 days ago

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 3/5] hw/tpm: Remove CRBState::ppi_enabled field
Posted by Philippe Mathieu-Daudé 2 weeks, 6 days ago
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
Posted by Stefan Berger 2 weeks, 6 days ago

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(-)
>