[PATCH v2 2/5] tpm: Extend TPMIfClass with get_irqnum() function

Stefan Berger posted 5 patches 5 years, 5 months ago
Maintainers: Stefan Berger <stefanb@linux.ibm.com>, Richard Henderson <rth@twiddle.net>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
There is a newer version of this series
[PATCH v2 2/5] tpm: Extend TPMIfClass with get_irqnum() function
Posted by Stefan Berger 5 years, 5 months ago
From: Stefan Berger <stefanb@sbct-2.pok.ibm.com>

Implement get_irqnum() as part of the TPMIfClass to be get the assigned
IRQ number.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/tpm/tpm_tis_isa.c    |  9 +++++++++
 hw/tpm/tpm_tis_sysbus.c |  9 +++++++++
 include/sysemu/tpm.h    | 10 ++++++++++
 3 files changed, 28 insertions(+)

diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 30ba37079d..63b62f4c21 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_isa_get_tpm_version(TPMIf *ti)
     return tpm_tis_get_tpm_version(s);
 }
 
+static uint8_t tpm_tis_isa_get_irqnum(TPMIf *ti)
+{
+    TPMStateISA *isadev = TPM_TIS_ISA(ti);
+    TPMState *s = &isadev->state;
+
+    return s->irq_num;
+}
+
 static void tpm_tis_isa_reset(DeviceState *dev)
 {
     TPMStateISA *isadev = TPM_TIS_ISA(dev);
@@ -148,6 +156,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, void *data)
     dc->reset = tpm_tis_isa_reset;
     tc->request_completed = tpm_tis_isa_request_completed;
     tc->get_version = tpm_tis_isa_get_tpm_version;
+    tc->get_irqnum = tpm_tis_isa_get_irqnum;
 }
 
 static const TypeInfo tpm_tis_isa_info = {
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
index eced1fc843..6cf45e5057 100644
--- a/hw/tpm/tpm_tis_sysbus.c
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_sysbus_get_tpm_version(TPMIf *ti)
     return tpm_tis_get_tpm_version(s);
 }
 
+static uint8_t tpm_tis_sysbus_get_irqnum(TPMIf *ti)
+{
+    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
+    TPMState *s = &sbdev->state;
+
+    return s->irq_num;
+}
+
 static void tpm_tis_sysbus_reset(DeviceState *dev)
 {
     TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
@@ -137,6 +145,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
     dc->reset = tpm_tis_sysbus_reset;
     tc->request_completed = tpm_tis_sysbus_request_completed;
     tc->get_version = tpm_tis_sysbus_get_tpm_version;
+    tc->get_irqnum = tpm_tis_sysbus_get_irqnum;
 }
 
 static const TypeInfo tpm_tis_sysbus_info = {
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 03fb25941c..b94a8a2b16 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -41,6 +41,7 @@ typedef struct TPMIfClass {
     enum TpmModel model;
     void (*request_completed)(TPMIf *obj, int ret);
     enum TPMVersion (*get_version)(TPMIf *obj);
+    uint8_t (*get_irqnum)(TPMIf *obj);
 } TPMIfClass;
 
 #define TYPE_TPM_TIS_ISA            "tpm-tis"
@@ -74,4 +75,13 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
     return TPM_IF_GET_CLASS(ti)->get_version(ti);
 }
 
+static inline uint8_t tpm_get_irqnum(TPMIf *ti)
+{
+    if (!ti || !TPM_IF_GET_CLASS(ti)->get_irqnum) {
+        return 0;
+    }
+
+    return TPM_IF_GET_CLASS(ti)->get_irqnum(ti);
+}
+
 #endif /* QEMU_TPM_H */
-- 
2.24.1


Re: [PATCH v2 2/5] tpm: Extend TPMIfClass with get_irqnum() function
Posted by Marc-André Lureau 5 years, 5 months ago
Hi

On Mon, Jun 15, 2020 at 6:23 PM Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
>
> From: Stefan Berger <stefanb@sbct-2.pok.ibm.com>
>
> Implement get_irqnum() as part of the TPMIfClass to be get the assigned

^^ to get

> IRQ number.
>

Since it is TIS ISA specific (at least for now), perhaps a dedicated
tpm_tis_get_irq_num() is more appropriate?

> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  hw/tpm/tpm_tis_isa.c    |  9 +++++++++
>  hw/tpm/tpm_tis_sysbus.c |  9 +++++++++
>  include/sysemu/tpm.h    | 10 ++++++++++
>  3 files changed, 28 insertions(+)
>
> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
> index 30ba37079d..63b62f4c21 100644
> --- a/hw/tpm/tpm_tis_isa.c
> +++ b/hw/tpm/tpm_tis_isa.c
> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_isa_get_tpm_version(TPMIf *ti)
>      return tpm_tis_get_tpm_version(s);
>  }
>
> +static uint8_t tpm_tis_isa_get_irqnum(TPMIf *ti)
> +{
> +    TPMStateISA *isadev = TPM_TIS_ISA(ti);
> +    TPMState *s = &isadev->state;
> +
> +    return s->irq_num;
> +}
> +
>  static void tpm_tis_isa_reset(DeviceState *dev)
>  {
>      TPMStateISA *isadev = TPM_TIS_ISA(dev);
> @@ -148,6 +156,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, void *data)
>      dc->reset = tpm_tis_isa_reset;
>      tc->request_completed = tpm_tis_isa_request_completed;
>      tc->get_version = tpm_tis_isa_get_tpm_version;
> +    tc->get_irqnum = tpm_tis_isa_get_irqnum;
>  }
>
>  static const TypeInfo tpm_tis_isa_info = {
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index eced1fc843..6cf45e5057 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_sysbus_get_tpm_version(TPMIf *ti)
>      return tpm_tis_get_tpm_version(s);
>  }
>
> +static uint8_t tpm_tis_sysbus_get_irqnum(TPMIf *ti)
> +{
> +    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
> +    TPMState *s = &sbdev->state;
> +
> +    return s->irq_num;
> +}
> +
>  static void tpm_tis_sysbus_reset(DeviceState *dev)
>  {
>      TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
> @@ -137,6 +145,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
>      dc->reset = tpm_tis_sysbus_reset;
>      tc->request_completed = tpm_tis_sysbus_request_completed;
>      tc->get_version = tpm_tis_sysbus_get_tpm_version;
> +    tc->get_irqnum = tpm_tis_sysbus_get_irqnum;
>  }
>
>  static const TypeInfo tpm_tis_sysbus_info = {
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 03fb25941c..b94a8a2b16 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -41,6 +41,7 @@ typedef struct TPMIfClass {
>      enum TpmModel model;
>      void (*request_completed)(TPMIf *obj, int ret);
>      enum TPMVersion (*get_version)(TPMIf *obj);
> +    uint8_t (*get_irqnum)(TPMIf *obj);
>  } TPMIfClass;
>
>  #define TYPE_TPM_TIS_ISA            "tpm-tis"
> @@ -74,4 +75,13 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
>      return TPM_IF_GET_CLASS(ti)->get_version(ti);
>  }
>
> +static inline uint8_t tpm_get_irqnum(TPMIf *ti)
> +{
> +    if (!ti || !TPM_IF_GET_CLASS(ti)->get_irqnum) {
> +        return 0;
> +    }
> +
> +    return TPM_IF_GET_CLASS(ti)->get_irqnum(ti);
> +}
> +
>  #endif /* QEMU_TPM_H */
> --
> 2.24.1
>


Re: [PATCH v2 2/5] tpm: Extend TPMIfClass with get_irqnum() function
Posted by Stefan Berger 5 years, 5 months ago
On 6/15/20 11:11 AM, Marc-André Lureau wrote:
> Hi
>
> On Mon, Jun 15, 2020 at 6:23 PM Stefan Berger
> <stefanb@linux.vnet.ibm.com> wrote:
>> From: Stefan Berger <stefanb@sbct-2.pok.ibm.com>
>>
>> Implement get_irqnum() as part of the TPMIfClass to be get the assigned
> ^^ to get
>
>> IRQ number.
>>
> Since it is TIS ISA specific (at least for now), perhaps a dedicated
> tpm_tis_get_irq_num() is more appropriate?


There's no caller from ARM at the moment but at least it is preparing it 
to support getting the IRQ number since it also allows passing it in via 
command lie. So it wouldn't have any side-effects on ARM for as long as 
no extra code was added there. And once sysbus was to use it, it would 
funnel through the same tpm_get_irqnum code. Other option is to drop the 
parts for sysbus entirely.


>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>>   hw/tpm/tpm_tis_isa.c    |  9 +++++++++
>>   hw/tpm/tpm_tis_sysbus.c |  9 +++++++++
>>   include/sysemu/tpm.h    | 10 ++++++++++
>>   3 files changed, 28 insertions(+)
>>
>> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
>> index 30ba37079d..63b62f4c21 100644
>> --- a/hw/tpm/tpm_tis_isa.c
>> +++ b/hw/tpm/tpm_tis_isa.c
>> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_isa_get_tpm_version(TPMIf *ti)
>>       return tpm_tis_get_tpm_version(s);
>>   }
>>
>> +static uint8_t tpm_tis_isa_get_irqnum(TPMIf *ti)
>> +{
>> +    TPMStateISA *isadev = TPM_TIS_ISA(ti);
>> +    TPMState *s = &isadev->state;
>> +
>> +    return s->irq_num;
>> +}
>> +
>>   static void tpm_tis_isa_reset(DeviceState *dev)
>>   {
>>       TPMStateISA *isadev = TPM_TIS_ISA(dev);
>> @@ -148,6 +156,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, void *data)
>>       dc->reset = tpm_tis_isa_reset;
>>       tc->request_completed = tpm_tis_isa_request_completed;
>>       tc->get_version = tpm_tis_isa_get_tpm_version;
>> +    tc->get_irqnum = tpm_tis_isa_get_irqnum;
>>   }
>>
>>   static const TypeInfo tpm_tis_isa_info = {
>> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
>> index eced1fc843..6cf45e5057 100644
>> --- a/hw/tpm/tpm_tis_sysbus.c
>> +++ b/hw/tpm/tpm_tis_sysbus.c
>> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_sysbus_get_tpm_version(TPMIf *ti)
>>       return tpm_tis_get_tpm_version(s);
>>   }
>>
>> +static uint8_t tpm_tis_sysbus_get_irqnum(TPMIf *ti)
>> +{
>> +    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
>> +    TPMState *s = &sbdev->state;
>> +
>> +    return s->irq_num;
>> +}
>> +
>>   static void tpm_tis_sysbus_reset(DeviceState *dev)
>>   {
>>       TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
>> @@ -137,6 +145,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
>>       dc->reset = tpm_tis_sysbus_reset;
>>       tc->request_completed = tpm_tis_sysbus_request_completed;
>>       tc->get_version = tpm_tis_sysbus_get_tpm_version;
>> +    tc->get_irqnum = tpm_tis_sysbus_get_irqnum;
>>   }
>>
>>   static const TypeInfo tpm_tis_sysbus_info = {
>> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
>> index 03fb25941c..b94a8a2b16 100644
>> --- a/include/sysemu/tpm.h
>> +++ b/include/sysemu/tpm.h
>> @@ -41,6 +41,7 @@ typedef struct TPMIfClass {
>>       enum TpmModel model;
>>       void (*request_completed)(TPMIf *obj, int ret);
>>       enum TPMVersion (*get_version)(TPMIf *obj);
>> +    uint8_t (*get_irqnum)(TPMIf *obj);
>>   } TPMIfClass;
>>
>>   #define TYPE_TPM_TIS_ISA            "tpm-tis"
>> @@ -74,4 +75,13 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
>>       return TPM_IF_GET_CLASS(ti)->get_version(ti);
>>   }
>>
>> +static inline uint8_t tpm_get_irqnum(TPMIf *ti)
>> +{
>> +    if (!ti || !TPM_IF_GET_CLASS(ti)->get_irqnum) {
>> +        return 0;
>> +    }
>> +
>> +    return TPM_IF_GET_CLASS(ti)->get_irqnum(ti);
>> +}
>> +
>>   #endif /* QEMU_TPM_H */
>> --
>> 2.24.1
>>


Re: [PATCH v2 2/5] tpm: Extend TPMIfClass with get_irqnum() function
Posted by Auger Eric 5 years, 5 months ago
Hi Stefan,

On 6/15/20 5:44 PM, Stefan Berger wrote:
> On 6/15/20 11:11 AM, Marc-André Lureau wrote:
>> Hi
>>
>> On Mon, Jun 15, 2020 at 6:23 PM Stefan Berger
>> <stefanb@linux.vnet.ibm.com> wrote:
>>> From: Stefan Berger <stefanb@sbct-2.pok.ibm.com>
>>>
>>> Implement get_irqnum() as part of the TPMIfClass to be get the assigned
>> ^^ to get
>>
>>> IRQ number.
>>>
>> Since it is TIS ISA specific (at least for now), perhaps a dedicated
>> tpm_tis_get_irq_num() is more appropriate?
> 
> 
> There's no caller from ARM at the moment but at least it is preparing it
> to support getting the IRQ number since it also allows passing it in via
> command lie. So it wouldn't have any side-effects on ARM for as long as
> no extra code was added there. And once sysbus was to use it, it would
> funnel through the same tpm_get_irqnum code. Other option is to drop the
> parts for sysbus entirely.

As there is no caller on ARM side, I think I would leave the ops
undefined for the sysbus device. But as already reported it does not
break anything on ARM side.

Thanks

Eric
> 
> 
>>
>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>>> ---
>>>   hw/tpm/tpm_tis_isa.c    |  9 +++++++++
>>>   hw/tpm/tpm_tis_sysbus.c |  9 +++++++++
>>>   include/sysemu/tpm.h    | 10 ++++++++++
>>>   3 files changed, 28 insertions(+)
>>>
>>> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
>>> index 30ba37079d..63b62f4c21 100644
>>> --- a/hw/tpm/tpm_tis_isa.c
>>> +++ b/hw/tpm/tpm_tis_isa.c
>>> @@ -80,6 +80,14 @@ static enum TPMVersion
>>> tpm_tis_isa_get_tpm_version(TPMIf *ti)
>>>       return tpm_tis_get_tpm_version(s);
>>>   }
>>>
>>> +static uint8_t tpm_tis_isa_get_irqnum(TPMIf *ti)
>>> +{
>>> +    TPMStateISA *isadev = TPM_TIS_ISA(ti);
>>> +    TPMState *s = &isadev->state;
>>> +
>>> +    return s->irq_num;
>>> +}
>>> +
>>>   static void tpm_tis_isa_reset(DeviceState *dev)
>>>   {
>>>       TPMStateISA *isadev = TPM_TIS_ISA(dev);
>>> @@ -148,6 +156,7 @@ static void tpm_tis_isa_class_init(ObjectClass
>>> *klass, void *data)
>>>       dc->reset = tpm_tis_isa_reset;
>>>       tc->request_completed = tpm_tis_isa_request_completed;
>>>       tc->get_version = tpm_tis_isa_get_tpm_version;
>>> +    tc->get_irqnum = tpm_tis_isa_get_irqnum;
>>>   }
>>>
>>>   static const TypeInfo tpm_tis_isa_info = {
>>> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
>>> index eced1fc843..6cf45e5057 100644
>>> --- a/hw/tpm/tpm_tis_sysbus.c
>>> +++ b/hw/tpm/tpm_tis_sysbus.c
>>> @@ -80,6 +80,14 @@ static enum TPMVersion
>>> tpm_tis_sysbus_get_tpm_version(TPMIf *ti)
>>>       return tpm_tis_get_tpm_version(s);
>>>   }
>>>
>>> +static uint8_t tpm_tis_sysbus_get_irqnum(TPMIf *ti)
>>> +{
>>> +    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
>>> +    TPMState *s = &sbdev->state;
>>> +
>>> +    return s->irq_num;
>>> +}
>>> +
>>>   static void tpm_tis_sysbus_reset(DeviceState *dev)
>>>   {
>>>       TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
>>> @@ -137,6 +145,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass
>>> *klass, void *data)
>>>       dc->reset = tpm_tis_sysbus_reset;
>>>       tc->request_completed = tpm_tis_sysbus_request_completed;
>>>       tc->get_version = tpm_tis_sysbus_get_tpm_version;
>>> +    tc->get_irqnum = tpm_tis_sysbus_get_irqnum;
>>>   }
>>>
>>>   static const TypeInfo tpm_tis_sysbus_info = {
>>> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
>>> index 03fb25941c..b94a8a2b16 100644
>>> --- a/include/sysemu/tpm.h
>>> +++ b/include/sysemu/tpm.h
>>> @@ -41,6 +41,7 @@ typedef struct TPMIfClass {
>>>       enum TpmModel model;
>>>       void (*request_completed)(TPMIf *obj, int ret);
>>>       enum TPMVersion (*get_version)(TPMIf *obj);
>>> +    uint8_t (*get_irqnum)(TPMIf *obj);
>>>   } TPMIfClass;
>>>
>>>   #define TYPE_TPM_TIS_ISA            "tpm-tis"
>>> @@ -74,4 +75,13 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
>>>       return TPM_IF_GET_CLASS(ti)->get_version(ti);
>>>   }
>>>
>>> +static inline uint8_t tpm_get_irqnum(TPMIf *ti)
>>> +{
>>> +    if (!ti || !TPM_IF_GET_CLASS(ti)->get_irqnum) {
>>> +        return 0;
>>> +    }
>>> +
>>> +    return TPM_IF_GET_CLASS(ti)->get_irqnum(ti);
>>> +}
>>> +
>>>   #endif /* QEMU_TPM_H */
>>> -- 
>>> 2.24.1
>>>
> 


Re: [PATCH v2 2/5] tpm: Extend TPMIfClass with get_irqnum() function
Posted by Philippe Mathieu-Daudé 5 years, 5 months ago
On 6/15/20 4:23 PM, Stefan Berger wrote:
> From: Stefan Berger <stefanb@sbct-2.pok.ibm.com>
> 
> Implement get_irqnum() as part of the TPMIfClass to be get the assigned
> IRQ number.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  hw/tpm/tpm_tis_isa.c    |  9 +++++++++
>  hw/tpm/tpm_tis_sysbus.c |  9 +++++++++
>  include/sysemu/tpm.h    | 10 ++++++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
> index 30ba37079d..63b62f4c21 100644
> --- a/hw/tpm/tpm_tis_isa.c
> +++ b/hw/tpm/tpm_tis_isa.c
> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_isa_get_tpm_version(TPMIf *ti)
>      return tpm_tis_get_tpm_version(s);
>  }
>  
> +static uint8_t tpm_tis_isa_get_irqnum(TPMIf *ti)
> +{
> +    TPMStateISA *isadev = TPM_TIS_ISA(ti);
> +    TPMState *s = &isadev->state;
> +
> +    return s->irq_num;
> +}
> +
>  static void tpm_tis_isa_reset(DeviceState *dev)
>  {
>      TPMStateISA *isadev = TPM_TIS_ISA(dev);
> @@ -148,6 +156,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, void *data)
>      dc->reset = tpm_tis_isa_reset;
>      tc->request_completed = tpm_tis_isa_request_completed;
>      tc->get_version = tpm_tis_isa_get_tpm_version;
> +    tc->get_irqnum = tpm_tis_isa_get_irqnum;
>  }
>  
>  static const TypeInfo tpm_tis_isa_info = {
> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
> index eced1fc843..6cf45e5057 100644
> --- a/hw/tpm/tpm_tis_sysbus.c
> +++ b/hw/tpm/tpm_tis_sysbus.c
> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_sysbus_get_tpm_version(TPMIf *ti)
>      return tpm_tis_get_tpm_version(s);
>  }
>  
> +static uint8_t tpm_tis_sysbus_get_irqnum(TPMIf *ti)
> +{
> +    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
> +    TPMState *s = &sbdev->state;
> +
> +    return s->irq_num;
> +}
> +
>  static void tpm_tis_sysbus_reset(DeviceState *dev)
>  {
>      TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
> @@ -137,6 +145,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
>      dc->reset = tpm_tis_sysbus_reset;
>      tc->request_completed = tpm_tis_sysbus_request_completed;
>      tc->get_version = tpm_tis_sysbus_get_tpm_version;
> +    tc->get_irqnum = tpm_tis_sysbus_get_irqnum;
>  }
>  
>  static const TypeInfo tpm_tis_sysbus_info = {
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 03fb25941c..b94a8a2b16 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -41,6 +41,7 @@ typedef struct TPMIfClass {
>      enum TpmModel model;
>      void (*request_completed)(TPMIf *obj, int ret);
>      enum TPMVersion (*get_version)(TPMIf *obj);
> +    uint8_t (*get_irqnum)(TPMIf *obj);
>  } TPMIfClass;
>  
>  #define TYPE_TPM_TIS_ISA            "tpm-tis"
> @@ -74,4 +75,13 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
>      return TPM_IF_GET_CLASS(ti)->get_version(ti);
>  }
>  
> +static inline uint8_t tpm_get_irqnum(TPMIf *ti)
> +{
> +    if (!ti || !TPM_IF_GET_CLASS(ti)->get_irqnum) {
> +        return 0;

0 is valid...

Shouldn't we use an Error* argument?

Else return -1 when invalid?

> +    }
> +
> +    return TPM_IF_GET_CLASS(ti)->get_irqnum(ti);
> +}
> +
>  #endif /* QEMU_TPM_H */
> 


Re: [PATCH v2 2/5] tpm: Extend TPMIfClass with get_irqnum() function
Posted by Stefan Berger 5 years, 5 months ago
On 6/15/20 11:54 AM, Philippe Mathieu-Daudé wrote:
> On 6/15/20 4:23 PM, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@sbct-2.pok.ibm.com>
>>
>> Implement get_irqnum() as part of the TPMIfClass to be get the assigned
>> IRQ number.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>>   hw/tpm/tpm_tis_isa.c    |  9 +++++++++
>>   hw/tpm/tpm_tis_sysbus.c |  9 +++++++++
>>   include/sysemu/tpm.h    | 10 ++++++++++
>>   3 files changed, 28 insertions(+)
>>
>> diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
>> index 30ba37079d..63b62f4c21 100644
>> --- a/hw/tpm/tpm_tis_isa.c
>> +++ b/hw/tpm/tpm_tis_isa.c
>> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_isa_get_tpm_version(TPMIf *ti)
>>       return tpm_tis_get_tpm_version(s);
>>   }
>>   
>> +static uint8_t tpm_tis_isa_get_irqnum(TPMIf *ti)
>> +{
>> +    TPMStateISA *isadev = TPM_TIS_ISA(ti);
>> +    TPMState *s = &isadev->state;
>> +
>> +    return s->irq_num;
>> +}
>> +
>>   static void tpm_tis_isa_reset(DeviceState *dev)
>>   {
>>       TPMStateISA *isadev = TPM_TIS_ISA(dev);
>> @@ -148,6 +156,7 @@ static void tpm_tis_isa_class_init(ObjectClass *klass, void *data)
>>       dc->reset = tpm_tis_isa_reset;
>>       tc->request_completed = tpm_tis_isa_request_completed;
>>       tc->get_version = tpm_tis_isa_get_tpm_version;
>> +    tc->get_irqnum = tpm_tis_isa_get_irqnum;
>>   }
>>   
>>   static const TypeInfo tpm_tis_isa_info = {
>> diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
>> index eced1fc843..6cf45e5057 100644
>> --- a/hw/tpm/tpm_tis_sysbus.c
>> +++ b/hw/tpm/tpm_tis_sysbus.c
>> @@ -80,6 +80,14 @@ static enum TPMVersion tpm_tis_sysbus_get_tpm_version(TPMIf *ti)
>>       return tpm_tis_get_tpm_version(s);
>>   }
>>   
>> +static uint8_t tpm_tis_sysbus_get_irqnum(TPMIf *ti)
>> +{
>> +    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
>> +    TPMState *s = &sbdev->state;
>> +
>> +    return s->irq_num;
>> +}
>> +
>>   static void tpm_tis_sysbus_reset(DeviceState *dev)
>>   {
>>       TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
>> @@ -137,6 +145,7 @@ static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
>>       dc->reset = tpm_tis_sysbus_reset;
>>       tc->request_completed = tpm_tis_sysbus_request_completed;
>>       tc->get_version = tpm_tis_sysbus_get_tpm_version;
>> +    tc->get_irqnum = tpm_tis_sysbus_get_irqnum;
>>   }
>>   
>>   static const TypeInfo tpm_tis_sysbus_info = {
>> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
>> index 03fb25941c..b94a8a2b16 100644
>> --- a/include/sysemu/tpm.h
>> +++ b/include/sysemu/tpm.h
>> @@ -41,6 +41,7 @@ typedef struct TPMIfClass {
>>       enum TpmModel model;
>>       void (*request_completed)(TPMIf *obj, int ret);
>>       enum TPMVersion (*get_version)(TPMIf *obj);
>> +    uint8_t (*get_irqnum)(TPMIf *obj);
>>   } TPMIfClass;
>>   
>>   #define TYPE_TPM_TIS_ISA            "tpm-tis"
>> @@ -74,4 +75,13 @@ static inline TPMVersion tpm_get_version(TPMIf *ti)
>>       return TPM_IF_GET_CLASS(ti)->get_version(ti);
>>   }
>>   
>> +static inline uint8_t tpm_get_irqnum(TPMIf *ti)
>> +{
>> +    if (!ti || !TPM_IF_GET_CLASS(ti)->get_irqnum) {
>> +        return 0;
> 0 is valid...
>
> Shouldn't we use an Error* argument?
>
> Else return -1 when invalid?

Indeed. Let me fix this. Thanks.


>
>> +    }
>> +
>> +    return TPM_IF_GET_CLASS(ti)->get_irqnum(ti);
>> +}
>> +
>>   #endif /* QEMU_TPM_H */
>>