[37/42] tpm: lookup the the TPM interface instead of TIS device

This will allow to introduce new devices implementing TPM.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/tpm/tpm_int.h     | 19 -------------------
 include/sysemu/tpm.h | 52 ++++++++++++++++++++++++++++++++++++++--------------
 hw/i386/acpi-build.c |  2 +-
 3 files changed, 39 insertions(+), 34 deletions(-)

On Tue, Oct 10, 2017 at 12:56:18AM +0200, Marc-André Lureau wrote:
[...]
> -static inline TPMVersion tpm_get_version(void)
> +static inline TPMIf *tpm_find(void)
>  {
> -#ifdef CONFIG_TPM
> -    Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
> +    Object *obj = object_resolve_path_type("", TYPE_TPM_IF, NULL);

Considering that tpm_crb_realizefn() will rely on tpm_find()
returning NULL if there are multiple TPM devices, I suggest
adding a "returns NULL unless there is exactly one TPM device"
comment, just like fw_cfg_find() and find_vmgenid_dev()

> +
> +    return TPM_IF(obj);
> +}
[...]

On 10/09/2017 06:56 PM, Marc-André Lureau wrote:
> This will allow to introduce new devices implementing TPM.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/tpm/tpm_int.h     | 19 -------------------
>   include/sysemu/tpm.h | 52 ++++++++++++++++++++++++++++++++++++++--------------
>   hw/i386/acpi-build.c |  2 +-
>   3 files changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
> index 90e97b9170..86fadc16d3 100644
> --- a/hw/tpm/tpm_int.h
> +++ b/hw/tpm/tpm_int.h
> @@ -15,25 +15,6 @@
>   #include "qemu/osdep.h"
>   #include "qom/object.h"
>
> -#define TYPE_TPM_IF "tpm-if"
> -#define TPM_IF_CLASS(klass) \
> -    OBJECT_CLASS_CHECK(TPMIfClass, (klass), TYPE_TPM_IF)
> -#define TPM_IF_GET_CLASS(obj) \
> -    OBJECT_GET_CLASS(TPMIfClass, (obj), TYPE_TPM_IF)
> -#define TPM_IF(obj) \
> -    INTERFACE_CHECK(TPMIf, (obj), TYPE_TPM_IF)
> -
> -typedef struct TPMIf {
> -    Object parent_obj;
> -} TPMIf;
> -
> -typedef struct TPMIfClass {
> -    InterfaceClass parent_class;
> -
> -    enum TpmModel model;
> -    void (*request_completed)(TPMIf *obj);
> -} TPMIfClass;
> -
>   #define TPM_STANDARD_CMDLINE_OPTS               \
>       { \
>           .name = "type", \
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 62b073beeb..dbd2b0cc7a 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -12,32 +12,56 @@
>   #ifndef QEMU_TPM_H
>   #define QEMU_TPM_H
>
> -#include "qemu/option.h"
> +#include "qom/object.h"
> +#include "qapi-types.h"
>
> -int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
> -int tpm_init(void);
> -void tpm_cleanup(void);
> -
> -typedef enum  TPMVersion {
> +typedef enum TPMVersion {
>       TPM_VERSION_UNSPEC = 0,
>       TPM_VERSION_1_2 = 1,
>       TPM_VERSION_2_0 = 2,
>   } TPMVersion;
>
> -TPMVersion tpm_tis_get_tpm_version(Object *obj);
> +#define TYPE_TPM_IF "tpm-if"
> +#define TPM_IF_CLASS(klass)                                 \
> +    OBJECT_CLASS_CHECK(TPMIfClass, (klass), TYPE_TPM_IF)
> +#define TPM_IF_GET_CLASS(obj)                           \
> +    OBJECT_GET_CLASS(TPMIfClass, (obj), TYPE_TPM_IF)
> +#define TPM_IF(obj)                             \
> +    INTERFACE_CHECK(TPMIf, (obj), TYPE_TPM_IF)
> +
> +typedef struct TPMIf {
> +    Object parent_obj;
> +} TPMIf;
> +
> +typedef struct TPMIfClass {
> +    InterfaceClass parent_class;
> +
> +    enum TpmModel model;
> +    void (*request_completed)(TPMIf *obj);
> +} TPMIfClass;
> +
> +int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
> +int tpm_init(void);
> +void tpm_cleanup(void);
>
>   #define TYPE_TPM_TIS                "tpm-tis"
>
> -static inline TPMVersion tpm_get_version(void)
> +static inline TPMIf *tpm_find(void)
>   {
> -#ifdef CONFIG_TPM
> -    Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
> +    Object *obj = object_resolve_path_type("", TYPE_TPM_IF, NULL);
> +
> +    return TPM_IF(obj);
> +}
>
> -    if (obj) {
> -        return tpm_tis_get_tpm_version(obj);
> +TPMVersion tpm_tis_get_tpm_version(Object *obj);
> +
> +static inline TPMVersion tpm_get_version(TPMIf *ti)
> +{
> +    if (!ti) {
> +        return TPM_VERSION_UNSPEC;
>       }
> -#endif
> -    return TPM_VERSION_UNSPEC;
> +
> +    return tpm_tis_get_tpm_version(OBJECT(ti));
>   }
>
>   #endif /* QEMU_TPM_H */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 2af37a9129..40371b6f75 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -208,7 +208,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_version(tpm_find());
>       info->pvpanic_port = pvpanic_port();
>       info->applesmc_io_base = applesmc_port();
>   }


Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

On 10/10/2017 04:21 PM, Eduardo Habkost wrote:
> On Tue, Oct 10, 2017 at 12:56:18AM +0200, Marc-André Lureau wrote:
> [...]
>> -static inline TPMVersion tpm_get_version(void)
>> +static inline TPMIf *tpm_find(void)
>>   {
>> -#ifdef CONFIG_TPM
>> -    Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
>> +    Object *obj = object_resolve_path_type("", TYPE_TPM_IF, NULL);
> Considering that tpm_crb_realizefn() will rely on tpm_find()
> returning NULL if there are multiple TPM devices, I suggest
> adding a "returns NULL unless there is exactly one TPM device"
> comment, just like fw_cfg_find() and find_vmgenid_dev()

I wonder whether the function couldn't have a better name. 
tpm_find_single() ?

    Stefan

>
>> +
>> +    return TPM_IF(obj);
>> +}
> [...]
>

Hi

----- Original Message -----
> On 10/10/2017 04:21 PM, Eduardo Habkost wrote:
> > On Tue, Oct 10, 2017 at 12:56:18AM +0200, Marc-André Lureau wrote:
> > [...]
> >> -static inline TPMVersion tpm_get_version(void)
> >> +static inline TPMIf *tpm_find(void)
> >>   {
> >> -#ifdef CONFIG_TPM
> >> -    Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
> >> +    Object *obj = object_resolve_path_type("", TYPE_TPM_IF, NULL);
> > Considering that tpm_crb_realizefn() will rely on tpm_find()
> > returning NULL if there are multiple TPM devices, I suggest
> > adding a "returns NULL unless there is exactly one TPM device"
> > comment, just like fw_cfg_find() and find_vmgenid_dev()
> 
> I wonder whether the function couldn't have a better name.
> tpm_find_single() ?
> 

As Eduardo said, there is precedence in QEMU codebase (fw_cfg_find() and find_vmgenid_dev())

I don't think foo_find() is a bad name - it returns NULL if there are multiple foo, which makes sense imho. I'll add the missing comment though.