Re: [Qemu-devel] [PATCH 21/42] tpm: move recv_data_callback to TPM interface

[Stefan Berger]

On 10/09/2017 06:56 PM, Marc-André Lureau wrote:
> Simplify the TPM backend setup, move callback to TPM interface.
>
> Signed-off-by: Marc-André Lureau <address@hidden>

Reviewed-by: Stefan Berger <address@hidden>

> ---
>   hw/tpm/tpm_int.h             |  3 +++
>   include/sysemu/tpm_backend.h |  6 +-----
>   backends/tpm.c               |  4 +---
>   hw/tpm/tpm_emulator.c        |  3 ++-
>   hw/tpm/tpm_passthrough.c     |  3 ++-
>   hw/tpm/tpm_tis.c             | 11 ++++++-----
>   6 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
> index eb02e7760c..9c045b6691 100644
> --- a/hw/tpm/tpm_int.h
> +++ b/hw/tpm/tpm_int.h
> @@ -29,6 +29,9 @@ typedef struct TPMIf {
>
>   typedef struct TPMIfClass {
>       InterfaceClass parent_class;
> +
> +    /* run in thread pool by backend */
> +    void (*request_completed)(TPMIf *obj);
>   } TPMIfClass;
>
>   #define TPM_STANDARD_CMDLINE_OPTS               \
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index 3bb90be3de..03ea5a3400 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -30,8 +30,6 @@
>   typedef struct TPMBackendClass TPMBackendClass;
>   typedef struct TPMBackend TPMBackend;
>
> -typedef void (TPMRecvDataCB)(TPMState *);
> -
>   typedef struct TPMBackendCmd {
>       uint8_t locty;
>       const uint8_t *in;
> @@ -48,7 +46,6 @@ struct TPMBackend {
>       bool opened;
>       TPMState *tpm_state;
>       GThreadPool *thread_pool;
> -    TPMRecvDataCB *recv_data_callback;
>       bool had_startup_error;
>
>       /* <public> */
> @@ -106,8 +103,7 @@ enum TpmType tpm_backend_get_type(TPMBackend *s);
>    *
>    * Returns 0 on success.
>    */
> -int tpm_backend_init(TPMBackend *s, TPMState *state,
> -                     TPMRecvDataCB *datacb);
> +int tpm_backend_init(TPMBackend *s, TPMState *state);
>
>   /**
>    * tpm_backend_startup_tpm:
> diff --git a/backends/tpm.c b/backends/tpm.c
> index 87c5c09179..5763f6f369 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -44,11 +44,9 @@ enum TpmType tpm_backend_get_type(TPMBackend *s)
>       return k->type;
>   }
>
> -int tpm_backend_init(TPMBackend *s, TPMState *state,
> -                     TPMRecvDataCB *datacb)
> +int tpm_backend_init(TPMBackend *s, TPMState *state)
>   {
>       s->tpm_state = state;
> -    s->recv_data_callback = datacb;
>       s->had_startup_error = false;
>
>       return 0;
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index 407ac97651..f04f4e0830 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -176,6 +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);
>       Error *err = NULL;
>
>       DPRINTF("processing TPM command");
> @@ -190,7 +191,7 @@ static void tpm_emulator_handle_request(TPMBackend *tb, 
> TPMBackendCmd *cmd)
>           goto error;
>       }
>
> -    tb->recv_data_callback(tb->tpm_state);
> +    tic->request_completed(TPM_IF(tb->tpm_state));
>       return;
>
>   error:
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 4274164a61..c440aff4b2 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -139,13 +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);
>
>       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);
>
> -    tb->recv_data_callback(tb->tpm_state);
> +    tic->request_completed(TPM_IF(tb->tpm_state));
>   }
>
>   static void tpm_passthrough_reset(TPMBackend *tb)
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index dbb50043ac..8c5cac5fa5 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -430,11 +430,10 @@ static void tpm_tis_receive_bh(void *opaque)
>                         TPM_TIS_INT_DATA_AVAILABLE | TPM_TIS_INT_STS_VALID);
>   }
>
> -/*
> - * Callback from the TPM to indicate that the response was received.
> - */
> -static void tpm_tis_receive_cb(TPMState *s)
> +static void tpm_tis_request_completed(TPMIf *ti)
>   {
> +    TPMState *s = TPM(ti);
> +
>       bool is_selftest_done = s->cmd.selftest_done;
>       uint8_t locty = s->cmd.locty;
>       uint8_t l;
> @@ -1078,7 +1077,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, tpm_tis_receive_cb)) {
> +    if (tpm_backend_init(s->be_driver, s)) {
>           error_setg(errp, "tpm_tis: backend driver with id %s could not be "
>                      "initialized", s->backend);
>           return;
> @@ -1110,11 +1109,13 @@ static void tpm_tis_initfn(Object *obj)
>   static void tpm_tis_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> +    TPMIfClass *tc = TPM_IF_CLASS(klass);
>
>       dc->realize = tpm_tis_realizefn;
>       dc->props = tpm_tis_properties;
>       dc->reset = tpm_tis_reset;
>       dc->vmsd  = &vmstate_tpm_tis;
> +    tc->request_completed = tpm_tis_request_completed;
>   }
>
>   static const TypeInfo tpm_tis_info = {