Re: [Qemu-devel] [PATCH v2 03/28] tpm-backend: store TPMIf interface, improve backend_init()

[Stefan Berger]

On 11/06/2017 01:39 PM, Marc-André Lureau wrote:
> Store the TPM interface, the actual object may be different from
> TPMState. Keep a reference on the interface, and check the backend
> wasn't already initialized.
>
> Signed-off-by: Marc-André Lureau <address@hidden>

Reviewed-by: Stefan Berger <address@hidden>


> ---
>   include/sysemu/tpm.h         |  2 +-
>   include/sysemu/tpm_backend.h |  6 +++---
>   backends/tpm.c               | 11 +++++++++--
>   hw/tpm/tpm_emulator.c        |  4 ++--
>   hw/tpm/tpm_passthrough.c     |  4 ++--
>   hw/tpm/tpm_tis.c             |  2 +-
>   6 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
> index 452cdb9cb7..fb1719e5e4 100644
> --- a/include/sysemu/tpm.h
> +++ b/include/sysemu/tpm.h
> @@ -12,8 +12,8 @@
>   #ifndef QEMU_TPM_H
>   #define QEMU_TPM_H
>
> -#include "qemu/option.h"
>   #include "qom/object.h"
> +#include "qapi-types.h"
>
>   typedef struct TPMState TPMState;
>
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index 03ea5a3400..b5f21ed8f1 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -43,8 +43,8 @@ struct TPMBackend {
>       Object parent;
>
>       /*< protected >*/
> +    TPMIf *tpmif;
>       bool opened;
> -    TPMState *tpm_state;
>       GThreadPool *thread_pool;
>       bool had_startup_error;
>
> @@ -96,14 +96,14 @@ enum TpmType tpm_backend_get_type(TPMBackend *s);
>   /**
>    * tpm_backend_init:
>    * @s: the backend to initialized
> - * @state: TPMState
> + * @tpmif: TPM interface
>    * @datacb: callback for sending data to frontend
>    *
>    * Initialize the backend with the given variables.
>    *
>    * Returns 0 on success.
>    */
> -int tpm_backend_init(TPMBackend *s, TPMState *state);
> +int tpm_backend_init(TPMBackend *s, TPMIf *tpmif);
>
>   /**
>    * tpm_backend_startup_tpm:
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 1e416d7f90..86f0e7e915 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -43,9 +43,15 @@ enum TpmType tpm_backend_get_type(TPMBackend *s)
>       return k->type;
>   }
>
> -int tpm_backend_init(TPMBackend *s, TPMState *state)
> +int tpm_backend_init(TPMBackend *s, TPMIf *tpmif)
>   {
> -    s->tpm_state = state;
> +    if (s->tpmif) {
> +        return -1;
> +    }
> +
> +    s->tpmif = tpmif;
> +    object_ref(OBJECT(tpmif));
> +
>       s->had_startup_error = false;
>
>       return 0;
> @@ -193,6 +199,7 @@ static void tpm_backend_instance_finalize(Object *obj)
>   {
>       TPMBackend *s = TPM_BACKEND(obj);
>
> +    object_unref(OBJECT(s->tpmif));
>       g_free(s->id);
>       tpm_backend_thread_end(s);
>   }
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index 9aaec8e3ef..6bf025c7e2 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -176,7 +176,7 @@ static int tpm_emulator_set_locality(TPMEmulator *tpm_emu, 
> uint8_t locty_number,
>   static void tpm_emulator_handle_request(TPMBackend *tb, TPMBackendCmd *cmd)
>   {
>       TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
> -    TPMIfClass *tic = TPM_IF_GET_CLASS(tb->tpm_state);
> +    TPMIfClass *tic = TPM_IF_GET_CLASS(tb->tpmif);
>       Error *err = NULL;
>
>       DPRINTF("processing TPM command");
> @@ -191,7 +191,7 @@ static void tpm_emulator_handle_request(TPMBackend *tb, 
> TPMBackendCmd *cmd)
>           goto error;
>       }
>
> -    tic->request_completed(TPM_IF(tb->tpm_state));
> +    tic->request_completed(tb->tpmif);
>       return;
>
>   error:
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index c440aff4b2..2ad74badca 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -139,14 +139,14 @@ err_exit:
>   static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd *cmd)
>   {
>       TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> -    TPMIfClass *tic = TPM_IF_GET_CLASS(tb->tpm_state);
> +    TPMIfClass *tic = TPM_IF_GET_CLASS(tb->tpmif);
>
>       DPRINTF("tpm_passthrough: processing command %p\n", cmd);
>
>       tpm_passthrough_unix_tx_bufs(tpm_pt, cmd->in, cmd->in_len,
>                                    cmd->out, cmd->out_len, &cmd->selftest_done);
>
> -    tic->request_completed(TPM_IF(tb->tpm_state));
> +    tic->request_completed(tb->tpmif);
>   }
>
>   static void tpm_passthrough_reset(TPMBackend *tb)
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index c5c534d7f2..34ceec91c3 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -1078,7 +1078,7 @@ static void tpm_tis_realizefn(DeviceState *dev, Error 
> **errp)
>
>       s->be_driver->fe_model = TPM_MODEL_TPM_TIS;
>
> -    if (tpm_backend_init(s->be_driver, s)) {
> +    if (tpm_backend_init(s->be_driver, TPM_IF(s))) {
>           error_setg(errp, "tpm_tis: backend driver with id %s could not be "
>                      "initialized", s->backend);
>           return;