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

[Auger Eric]

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