Re: [Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using

[Igor Mammedov <imammedo@redhat.com>]

On Sun,  3 Apr 2016 21:37:56 -0400
Stefan Berger <stefanb@us.ibm.com> wrote:

> Get the interrupt the TPM device model is using. Do not activate
> the interrupt in the ACPI description yet since the current
> default value clashes with other devices.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  hw/i386/acpi-build.c | 5 +++--
>  hw/tpm/tpm_tis.c     | 5 ++++-
>  include/sysemu/tpm.h | 6 +++---
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e11c721..ae872a1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -105,6 +105,7 @@ typedef struct AcpiMiscInfo {
>      bool is_piix4;
>      bool has_hpet;
>      TPMVersion tpm_version;
> +    uint32_t tpm_irq_num;
>      const unsigned char *dsdt_code;
>      unsigned dsdt_size;
>      uint16_t pvpanic_port;
> @@ -203,7 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
>      }
>  
>      info->has_hpet = hpet_find();
> -    info->tpm_version = tpm_get_version();
> +    info->tpm_version = tpm_get_parameters(&info->tpm_irq_num);
it's a bit confusing to read, provided that IRQ is an object property
I would just do here:

  obj  = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
  if (obj)
    info->tpm_irq_num = object_property_get_int(obj, "irq", NULL);

may be do the same for version too and do away with tpm_get_*()
wrapper that pokes into object internals. (result is 1 less global
symbol to link and no ifdef magic in tpm.h)

>      info->pvpanic_port = pvpanic_port();
>      info->applesmc_io_base = applesmc_port();
>  }
> @@ -2345,7 +2346,7 @@ build_dsdt(GArray *table_data, GArray *linker,
>              crs = aml_resource_template();
>              aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE,
>                         TPM_TIS_ADDR_SIZE, AML_READ_WRITE));
> -            //aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ));
> +            //aml_append(crs, aml_irq_no_flags(misc->tpm_irq_num));
>              aml_append(dev, aml_name_decl("_CRS", crs));
>              aml_append(sb_scope, dev);
>          }
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 381e726..9f5b2f7 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -967,9 +967,12 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
>  /*
>   * Get the TPMVersion of the backend device being used
>   */
> -TPMVersion tpm_tis_get_tpm_version(Object *obj)
> +TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num)
>  {
>      TPMState *s = TPM(obj);
> +    TPMTISEmuState *tis = &s->s.tis;
> +
> +    *irq_num = tis->irq_num;
>  
>      return tpm_backend_get_tpm_version(s->be_driver);
>  }
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index c8afa17..5be45d9 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -26,17 +26,17 @@ typedef enum  TPMVersion {
>      TPM_VERSION_2_0 = 2,
>  } TPMVersion;
>  
> -TPMVersion tpm_tis_get_tpm_version(Object *obj);
> +TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t
> *irq_num); 
>  #define TYPE_TPM_TIS                "tpm-tis"
>  
> -static inline TPMVersion tpm_get_version(void)
> +static inline TPMVersion tpm_get_parameters(uint32_t *irq_num)
>  {
>  #ifdef CONFIG_TPM
>      Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
>  
>      if (obj) {
> -        return tpm_tis_get_tpm_version(obj);
> +        return tpm_tis_get_tpm_parameters(obj, irq_num);
>      }
>  #endif
>      return TPM_VERSION_UNSPEC;