[v2] QOM-ify the TPM support

QOM-ified the TPM support with much code borrowed from the rng implementation.

What's missing may be that the tpm/tpm_passthrough.c be moved into backends/ .

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

---
 v1->v2:
  - followed to git revision e769bdc26

---
 backends/Makefile.objs |    2 
 backends/tpm.c         |  154 ++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/tpm.h     |  170 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/tpm/tpm.h      |    4 +
 tpm/tpm.c              |   11 ++-
 tpm/tpm_int.h          |   16 ----
 tpm/tpm_passthrough.c  |   94 +++++++++++++++++----------
 tpm/tpm_tis.c          |   21 +++---
 8 files changed, 413 insertions(+), 59 deletions(-)

Il 26/03/2013 17:28, Stefan Berger ha scritto:
> QOM-ified the TPM support with much code borrowed from the rng implementation.
> 
> What's missing may be that the tpm/tpm_passthrough.c be moved into backends/ .
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> 
> ---
>  v1->v2:
>   - followed to git revision e769bdc26
> 
> ---
>  backends/Makefile.objs |    2 
>  backends/tpm.c         |  154 ++++++++++++++++++++++++++++++++++++++++++++
>  include/qemu/tpm.h     |  170 +++++++++++++++++++++++++++++++++++++++++++++++++

I think these should be tpm_backend.[ch] (with the include file in
include/tpm).  Can you rename the existing files with that name first?

Paolo

>  include/tpm/tpm.h      |    4 +
>  tpm/tpm.c              |   11 ++-
>  tpm/tpm_int.h          |   16 ----
>  tpm/tpm_passthrough.c  |   94 +++++++++++++++++----------
>  tpm/tpm_tis.c          |   21 +++---
>  8 files changed, 413 insertions(+), 59 deletions(-)
> 
> Index: qemu-git.pt/backends/Makefile.objs
> ===================================================================
> --- qemu-git.pt.orig/backends/Makefile.objs
> +++ qemu-git.pt/backends/Makefile.objs
> @@ -4,3 +4,5 @@ common-obj-$(CONFIG_POSIX) += rng-random
>  common-obj-y += msmouse.o
>  common-obj-$(CONFIG_BRLAPI) += baum.o
>  $(obj)/baum.o: QEMU_CFLAGS += $(SDL_CFLAGS) 
> +
> +common-obj-$(CONFIG_TPM) += tpm.o
> Index: qemu-git.pt/backends/tpm.c
> ===================================================================
> --- /dev/null
> +++ qemu-git.pt/backends/tpm.c
> @@ -0,0 +1,154 @@
> +/*
> + * QEMU TPM Backend
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Stefan Berger   <stefanb@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + * Based on backends/rng.c by Anthony Liguori
> + */
> +
> +#include "qemu/tpm.h"
> +#include "tpm/tpm_int.h"
> +#include "qapi/qmp/qerror.h"
> +
> +enum TpmType tpm_backend_get_type(TPMBackend *s)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    return k->ops->type;
> +}
> +
> +const char *tpm_backend_get_desc(TPMBackend *s)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    return k->ops->desc();
> +}
> +
> +void tpm_backend_destroy(TPMBackend *s)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    return k->ops->destroy(s);
> +}
> +
> +int tpm_backend_init(TPMBackend *s, TPMState *state,
> +                     TPMRecvDataCB *datacb)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    return k->ops->init(s, state, datacb);
> +}
> +
> +int tpm_backend_startup_tpm(TPMBackend *s)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    return k->ops->startup_tpm(s);
> +}
> +
> +bool tpm_backend_had_startup_error(TPMBackend *s)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    return k->ops->had_startup_error(s);
> +}
> +
> +size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    return k->ops->realloc_buffer(sb);
> +}
> +
> +void tpm_backend_deliver_request(TPMBackend *s)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    k->ops->deliver_request(s);
> +}
> +
> +void tpm_backend_reset(TPMBackend *s)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    k->ops->reset(s);
> +}
> +
> +void tpm_backend_cancel_cmd(TPMBackend *s)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    k->ops->cancel_cmd(s);
> +}
> +
> +bool tpm_backend_get_tpm_established_flag(TPMBackend *s)
> +{
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    return k->ops->get_tpm_established_flag(s);
> +}
> +
> +static bool tpm_backend_prop_get_opened(Object *obj, Error **errp)
> +{
> +    TPMBackend *s = TPM_BACKEND(obj);
> +
> +    return s->opened;
> +}
> +
> +void tpm_backend_open(TPMBackend *s, Error **errp)
> +{
> +    object_property_set_bool(OBJECT(s), true, "opened", errp);
> +}
> +
> +static void tpm_backend_prop_set_opened(Object *obj, bool value, Error **errp)
> +{
> +    TPMBackend *s = TPM_BACKEND(obj);
> +    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> +
> +    if (value == s->opened) {
> +        return;
> +    }
> +
> +    if (!value && s->opened) {
> +        error_set(errp, QERR_PERMISSION_DENIED);
> +        return;
> +    }
> +
> +    if (k->opened) {
> +        k->opened(s, errp);
> +    }
> +
> +    if (!error_is_set(errp)) {
> +        s->opened = value;
> +    }
> +}
> +
> +static void tpm_backend_instance_init(Object *obj)
> +{
> +    object_property_add_bool(obj, "opened",
> +                             tpm_backend_prop_get_opened,
> +                             tpm_backend_prop_set_opened,
> +                             NULL);
> +}
> +
> +static const TypeInfo tpm_backend_info = {
> +    .name = TYPE_TPM_BACKEND,
> +    .parent = TYPE_OBJECT,
> +    .instance_size = sizeof(TPMBackend),
> +    .instance_init = tpm_backend_instance_init,
> +    .class_size = sizeof(TPMBackendClass),
> +    .abstract = true,
> +};
> +
> +static void register_types(void)
> +{
> +    type_register_static(&tpm_backend_info);
> +}
> +
> +type_init(register_types);
> Index: qemu-git.pt/include/qemu/tpm.h
> ===================================================================
> --- /dev/null
> +++ qemu-git.pt/include/qemu/tpm.h
> @@ -0,0 +1,170 @@
> +/*
> + * QEMU TPM Backend
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Stefan Berger  <stefanb@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef _QEMU_TPM_H
> +#define _QEMU_TPM_H
> +
> +#include "qom/object.h"
> +#include "qemu-common.h"
> +#include "qapi/error.h"
> +#include "qapi-types.h"
> +#include "qemu/option.h"
> +#include "tpm/tpm.h"
> +
> +#define TYPE_TPM_BACKEND "tpm-backend"
> +#define TPM_BACKEND(obj) \
> +    OBJECT_CHECK(TPMBackend, (obj), TYPE_TPM_BACKEND)
> +#define TPM_BACKEND_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(TPMBackendClass, (obj), TYPE_TPM_BACKEND)
> +#define TPM_BACKEND_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(TPMBackendClass, (klass), TYPE_TPM_BACKEND)
> +
> +typedef struct TPMBackendClass TPMBackendClass;
> +typedef struct TPMBackend TPMBackend;
> +
> +typedef struct TPMDriverOps TPMDriverOps;
> +
> +struct TPMBackendClass {
> +    ObjectClass parent_class;
> +
> +    const TPMDriverOps *ops;
> +
> +    void (*opened)(TPMBackend *s, Error **errp);
> +};
> +
> +struct TPMBackend {
> +    Object parent;
> +
> +    /*< protected >*/
> +    bool opened;
> +
> +    char *id;
> +    enum TpmModel fe_model;
> +    char *path;
> +    char *cancel_path;
> +    const TPMDriverOps *ops;
> +
> +    QLIST_ENTRY(TPMBackend) list;
> +};
> +
> +
> +/**
> + * tpm_backend_get_type:
> + * @s: the backend
> + *
> + * Returns the TpmType of the backend.
> + */
> +enum TpmType tpm_backend_get_type(TPMBackend *s);
> +
> +/**
> + * tpm_backend_get_desc:
> + * @s: the backend
> + *
> + * Returns a human readable description of the backend.
> + */
> +const char *tpm_backend_get_desc(TPMBackend *s);
> +
> +/**
> + * tpm_backend_destroy:
> + * @s: the backend to destroy
> + */
> +void tpm_backend_destroy(TPMBackend *s);
> +
> +/**
> + * tpm_backend_init:
> + * @s: the backend to initialized
> + * @state: TPMState
> + * @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,
> +                     TPMRecvDataCB *datacb);
> +
> +/**
> + * tpm_backend_startup_tpm:
> + * @s: the backend whose TPM support is to be started
> + *
> + * Returns 0 on success.
> + */
> +int tpm_backend_startup_tpm(TPMBackend *s);
> +
> +/**
> + * tpm_backend_had_startup_error:
> + * @s: the backend to query for a statup error
> + *
> + * Check whether the backend had an error during startup. Returns
> + * false if no error occurred and the backend can be used, true
> + * otherwise.
> + */
> +bool tpm_backend_had_startup_error(TPMBackend *s);
> +
> +/**
> + * tpm_backend_realloc_buffer:
> + * @s: the backend
> + * @sb: the TPMSizedBuffer to re-allocated to the size suitable for the
> + *      backend.
> + *
> + * This function returns the size of the allocated buffer
> + */
> +size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb);
> +
> +/**
> + * tpm_backend_deliver_request:
> + * @s: the backend to send the request to
> + *
> + * Send a request to the backend. The backend will then send the request
> + * to the TPM implementation.
> + */
> +void tpm_backend_deliver_request(TPMBackend *s);
> +
> +/**
> + * tpm_backend_reset:
> + * @s: the backend to reset
> + *
> + * Reset the backend into a well defined state with all previous errors
> + * reset.
> + */
> +void tpm_backend_reset(TPMBackend *s);
> +
> +/**
> + * tpm_backend_cancel_cmd:
> + * @s: the backend
> + *
> + * Cancel any ongoing command being processed by the TPM implementation
> + * on behalf of the QEMU guest.
> + */
> +void tpm_backend_cancel_cmd(TPMBackend *s);
> +
> +/**
> + * tpm_backend_get_tpm_established_flag:
> + * @s: the backend
> + *
> + * Get the TPM establishment flag. This function may be called very
> + * frequently by the frontend since for example in the TIS implementation
> + * this flag is part of a register.
> + */
> +bool tpm_backend_get_tpm_established_flag(TPMBackend *s);
> +
> +/**
> + * tpm_backend_open:
> + * @s: the backend to open
> + * @errp: a pointer to return the #Error object if an error occurs.
> + *
> + * This function will open the backend if it is not already open.  Calling this
> + * function on an already opened backend will not result in an error.
> + */
> +void tpm_backend_open(TPMBackend *s, Error **errp);
> +
> +#endif
> Index: qemu-git.pt/tpm/tpm_passthrough.c
> ===================================================================
> --- qemu-git.pt.orig/tpm/tpm_passthrough.c
> +++ qemu-git.pt/tpm/tpm_passthrough.c
> @@ -27,6 +27,7 @@
>  #include "qemu-common.h"
>  #include "qapi/error.h"
>  #include "qemu/sockets.h"
> +#include "qemu/tpm.h"
>  #include "tpm_int.h"
>  #include "hw/hw.h"
>  #include "hw/pc.h"
> @@ -43,8 +44,11 @@
>      do { } while (0)
>  #endif
>  
> -/* data structures */
> +#define TYPE_TPM_PASSTHROUGH "tpm-passthrough"
> +#define TPM_PASSTHROUGH(obj) \
> +    OBJECT_CHECK(TPMPassthruState, (obj), TYPE_TPM_PASSTHROUGH)
>  
> +/* data structures */
>  typedef struct TPMPassthruThreadParams {
>      TPMState *tpm_state;
>  
> @@ -53,6 +57,8 @@ typedef struct TPMPassthruThreadParams {
>  } TPMPassthruThreadParams;
>  
>  struct TPMPassthruState {
> +    TPMBackend parent;
> +
>      TPMBackendThread tbt;
>  
>      TPMPassthruThreadParams tpm_thread_params;
> @@ -65,6 +71,8 @@ struct TPMPassthruState {
>      bool had_startup_error;
>  };
>  
> +typedef struct TPMPassthruState TPMPassthruState;
> +
>  #define TPM_PASSTHROUGH_DEFAULT_DEVICE "/dev/tpm0"
>  
>  /* functions */
> @@ -149,7 +157,7 @@ static void tpm_passthrough_worker_threa
>                                            gpointer user_data)
>  {
>      TPMPassthruThreadParams *thr_parms = user_data;
> -    TPMPassthruState *tpm_pt = thr_parms->tb->s.tpm_pt;
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(thr_parms->tb);
>      TPMBackendCmd cmd = (TPMBackendCmd)data;
>  
>      DPRINTF("tpm_passthrough: processing command type %d\n", cmd);
> @@ -176,21 +184,21 @@ static void tpm_passthrough_worker_threa
>   */
>  static int tpm_passthrough_startup_tpm(TPMBackend *tb)
>  {
> -    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>  
>      /* terminate a running TPM */
>      tpm_backend_thread_end(&tpm_pt->tbt);
>  
>      tpm_backend_thread_create(&tpm_pt->tbt,
>                                tpm_passthrough_worker_thread,
> -                              &tb->s.tpm_pt->tpm_thread_params);
> +                              &tpm_pt->tpm_thread_params);
>  
>      return 0;
>  }
>  
>  static void tpm_passthrough_reset(TPMBackend *tb)
>  {
> -    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>  
>      DPRINTF("tpm_passthrough: CALL TO TPM_RESET!\n");
>  
> @@ -204,7 +212,7 @@ static void tpm_passthrough_reset(TPMBac
>  static int tpm_passthrough_init(TPMBackend *tb, TPMState *s,
>                                  TPMRecvDataCB *recv_data_cb)
>  {
> -    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>  
>      tpm_pt->tpm_thread_params.tpm_state = s;
>      tpm_pt->tpm_thread_params.recv_data_callback = recv_data_cb;
> @@ -220,7 +228,7 @@ static bool tpm_passthrough_get_tpm_esta
>  
>  static bool tpm_passthrough_get_startup_error(TPMBackend *tb)
>  {
> -    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>  
>      return tpm_pt->had_startup_error;
>  }
> @@ -238,14 +246,14 @@ static size_t tpm_passthrough_realloc_bu
>  
>  static void tpm_passthrough_deliver_request(TPMBackend *tb)
>  {
> -    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>  
>      tpm_backend_thread_deliver_request(&tpm_pt->tbt);
>  }
>  
>  static void tpm_passthrough_cancel_cmd(TPMBackend *tb)
>  {
> -    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>      int n;
>  
>      /*
> @@ -412,6 +420,7 @@ static int tpm_passthrough_open_sysfs_ca
>  
>  static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
>  {
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>      const char *value;
>  
>      value = qemu_opt_get(opts, "cancel-path");
> @@ -424,45 +433,45 @@ static int tpm_passthrough_handle_device
>          value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
>      }
>  
> -    tb->s.tpm_pt->tpm_dev = g_strdup(value);
> +    tpm_pt->tpm_dev = g_strdup(value);
>  
> -    tb->path = g_strdup(tb->s.tpm_pt->tpm_dev);
> +    tb->path = g_strdup(tpm_pt->tpm_dev);
>  
> -    tb->s.tpm_pt->tpm_fd = qemu_open(tb->s.tpm_pt->tpm_dev, O_RDWR);
> -    if (tb->s.tpm_pt->tpm_fd < 0) {
> +    tpm_pt->tpm_fd = qemu_open(tpm_pt->tpm_dev, O_RDWR);
> +    if (tpm_pt->tpm_fd < 0) {
>          error_report("Cannot access TPM device using '%s': %s\n",
> -                     tb->s.tpm_pt->tpm_dev, strerror(errno));
> +                     tpm_pt->tpm_dev, strerror(errno));
>          goto err_free_parameters;
>      }
>  
> -    if (tpm_passthrough_test_tpmdev(tb->s.tpm_pt->tpm_fd)) {
> +    if (tpm_passthrough_test_tpmdev(tpm_pt->tpm_fd)) {
>          error_report("'%s' is not a TPM device.\n",
> -                     tb->s.tpm_pt->tpm_dev);
> +                     tpm_pt->tpm_dev);
>          goto err_close_tpmdev;
>      }
>  
>      return 0;
>  
>   err_close_tpmdev:
> -    qemu_close(tb->s.tpm_pt->tpm_fd);
> -    tb->s.tpm_pt->tpm_fd = -1;
> +    qemu_close(tpm_pt->tpm_fd);
> +    tpm_pt->tpm_fd = -1;
>  
>   err_free_parameters:
>      g_free(tb->path);
>      tb->path = NULL;
>  
> -    g_free(tb->s.tpm_pt->tpm_dev);
> -    tb->s.tpm_pt->tpm_dev = NULL;
> +    g_free(tpm_pt->tpm_dev);
> +    tpm_pt->tpm_dev = NULL;
>  
>      return 1;
>  }
>  
>  static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id)
>  {
> -    TPMBackend *tb;
> +    Object *obj = object_new(TYPE_TPM_PASSTHROUGH);
> +    TPMBackend *tb = TPM_BACKEND(obj);
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>  
> -    tb = g_new0(TPMBackend, 1);
> -    tb->s.tpm_pt = g_new0(TPMPassthruState, 1);
>      tb->id = g_strdup(id);
>      /* let frontend set the fe_model to proper value */
>      tb->fe_model = -1;
> @@ -473,8 +482,8 @@ static TPMBackend *tpm_passthrough_creat
>          goto err_exit;
>      }
>  
> -    tb->s.tpm_pt->cancel_fd = tpm_passthrough_open_sysfs_cancel(tb);
> -    if (tb->s.tpm_pt->cancel_fd < 0) {
> +    tpm_pt->cancel_fd = tpm_passthrough_open_sysfs_cancel(tb);
> +    if (tpm_pt->cancel_fd < 0) {
>          goto err_exit;
>      }
>  
> @@ -482,29 +491,25 @@ static TPMBackend *tpm_passthrough_creat
>  
>  err_exit:
>      g_free(tb->id);
> -    g_free(tb->s.tpm_pt);
> -    g_free(tb);
>  
>      return NULL;
>  }
>  
>  static void tpm_passthrough_destroy(TPMBackend *tb)
>  {
> -    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
> +    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>  
>      tpm_passthrough_cancel_cmd(tb);
>  
>      tpm_backend_thread_end(&tpm_pt->tbt);
>  
>      qemu_close(tpm_pt->tpm_fd);
> -    qemu_close(tb->s.tpm_pt->cancel_fd);
> +    qemu_close(tpm_pt->cancel_fd);
>  
>      g_free(tb->id);
>      g_free(tb->path);
>      g_free(tb->cancel_path);
> -    g_free(tb->s.tpm_pt->tpm_dev);
> -    g_free(tb->s.tpm_pt);
> -    g_free(tb);
> +    g_free(tpm_pt->tpm_dev);
>  }
>  
>  const TPMDriverOps tpm_passthrough_driver = {
> @@ -522,8 +527,33 @@ const TPMDriverOps tpm_passthrough_drive
>      .get_tpm_established_flag = tpm_passthrough_get_tpm_established_flag,
>  };
>  
> +static void tpm_passthrough_inst_init(Object *obj)
> +{
> +}
> +
> +static void tpm_passthrough_inst_finalize(Object *obj)
> +{
> +}
> +
> +static void tpm_passthrough_class_init(ObjectClass *klass, void *data)
> +{
> +    TPMBackendClass *tbc = TPM_BACKEND_CLASS(klass);
> +
> +    tbc->ops = &tpm_passthrough_driver;
> +}
> +
> +static const TypeInfo tpm_passthrough_info = {
> +    .name = TYPE_TPM_PASSTHROUGH,
> +    .parent = TYPE_TPM_BACKEND,
> +    .instance_size = sizeof(TPMPassthruState),
> +    .class_init = tpm_passthrough_class_init,
> +    .instance_init = tpm_passthrough_inst_init,
> +    .instance_finalize = tpm_passthrough_inst_finalize,
> +};
> +
>  static void tpm_passthrough_register(void)
>  {
> +    type_register_static(&tpm_passthrough_info);
>      tpm_register_driver(&tpm_passthrough_driver);
>  }
>  
> Index: qemu-git.pt/tpm/tpm_tis.c
> ===================================================================
> --- qemu-git.pt.orig/tpm/tpm_tis.c
> +++ qemu-git.pt/tpm/tpm_tis.c
> @@ -19,6 +19,7 @@
>   * specification.
>   */
>  
> +#include "qemu/tpm.h"
>  #include "tpm_int.h"
>  #include "block/block.h"
>  #include "exec/address-spaces.h"
> @@ -160,7 +161,7 @@ static void tpm_tis_tpm_send(TPMState *s
>       */
>      tis->loc[locty].state = TPM_TIS_STATE_EXECUTION;
>  
> -    s->be_driver->ops->deliver_request(s->be_driver);
> +    tpm_backend_deliver_request(s->be_driver);
>  }
>  
>  /* raise an interrupt if allowed */
> @@ -284,7 +285,7 @@ static void tpm_tis_prep_abort(TPMState
>               * request the backend to cancel. Some backends may not
>               * support it
>               */
> -            s->be_driver->ops->cancel_cmd(s->be_driver);
> +            tpm_backend_cancel_cmd(s->be_driver);
>              return;
>          }
>      }
> @@ -426,7 +427,7 @@ static uint64_t tpm_tis_mmio_read(void *
>      uint8_t locty = tpm_tis_locality_from_addr(addr);
>      uint32_t avail;
>  
> -    if (s->be_driver->ops->had_startup_error(s->be_driver)) {
> +    if (tpm_backend_had_startup_error(s->be_driver)) {
>          return val;
>      }
>  
> @@ -438,7 +439,7 @@ static uint64_t tpm_tis_mmio_read(void *
>          if (tpm_tis_check_request_use_except(s, locty)) {
>              val |= TPM_TIS_ACCESS_PENDING_REQUEST;
>          }
> -        val |= !s->be_driver->ops->get_tpm_established_flag(s->be_driver);
> +        val |= !tpm_backend_get_tpm_established_flag(s->be_driver);
>          break;
>      case TPM_TIS_REG_INT_ENABLE:
>          val = tis->loc[locty].inte;
> @@ -529,7 +530,7 @@ static void tpm_tis_mmio_write_intern(vo
>          return;
>      }
>  
> -    if (s->be_driver->ops->had_startup_error(s->be_driver)) {
> +    if (tpm_backend_had_startup_error(s->be_driver)) {
>          return;
>      }
>  
> @@ -804,7 +805,7 @@ static const MemoryRegionOps tpm_tis_mem
>  
>  static int tpm_tis_do_startup_tpm(TPMState *s)
>  {
> -    return s->be_driver->ops->startup_tpm(s->be_driver);
> +    return tpm_backend_startup_tpm(s->be_driver);
>  }
>  
>  /*
> @@ -817,7 +818,7 @@ static void tpm_tis_reset(DeviceState *d
>      TPMTISEmuState *tis = &s->s.tis;
>      int c;
>  
> -    s->be_driver->ops->reset(s->be_driver);
> +    tpm_backend_reset(s->be_driver);
>  
>      tis->active_locty = TPM_TIS_NO_LOCALITY;
>      tis->next_locty = TPM_TIS_NO_LOCALITY;
> @@ -831,9 +832,9 @@ static void tpm_tis_reset(DeviceState *d
>          tis->loc[c].state = TPM_TIS_STATE_IDLE;
>  
>          tis->loc[c].w_offset = 0;
> -        s->be_driver->ops->realloc_buffer(&tis->loc[c].w_buffer);
> +        tpm_backend_realloc_buffer(s->be_driver, &tis->loc[c].w_buffer);
>          tis->loc[c].r_offset = 0;
> -        s->be_driver->ops->realloc_buffer(&tis->loc[c].r_buffer);
> +        tpm_backend_realloc_buffer(s->be_driver, &tis->loc[c].r_buffer);
>      }
>  
>      tpm_tis_do_startup_tpm(s);
> @@ -865,7 +866,7 @@ static void tpm_tis_realizefn(DeviceStat
>  
>      s->be_driver->fe_model = TPM_MODEL_TPM_TIS;
>  
> -    if (s->be_driver->ops->init(s->be_driver, s, tpm_tis_receive_cb)) {
> +    if (tpm_backend_init(s->be_driver, s, tpm_tis_receive_cb)) {
>          error_setg(errp, "tpm_tis: backend driver with id %s could not be "
>                     "initialized", s->backend);
>          return;
> Index: qemu-git.pt/tpm/tpm_int.h
> ===================================================================
> --- qemu-git.pt.orig/tpm/tpm_int.h
> +++ qemu-git.pt/tpm/tpm_int.h
> @@ -18,22 +18,6 @@
>  struct TPMDriverOps;
>  typedef struct TPMDriverOps TPMDriverOps;
>  
> -typedef struct TPMPassthruState TPMPassthruState;
> -
> -typedef struct TPMBackend {
> -    char *id;
> -    enum TpmModel fe_model;
> -    char *path;
> -    char *cancel_path;
> -    const TPMDriverOps *ops;
> -
> -    union {
> -        TPMPassthruState *tpm_pt;
> -    } s;
> -
> -    QLIST_ENTRY(TPMBackend) list;
> -} TPMBackend;
> -
>  /* overall state of the TPM interface */
>  typedef struct TPMState {
>      ISADevice busdev;
> Index: qemu-git.pt/tpm/tpm.c
> ===================================================================
> --- qemu-git.pt.orig/tpm/tpm.c
> +++ qemu-git.pt/tpm/tpm.c
> @@ -15,6 +15,7 @@
>  
>  #include "monitor/monitor.h"
>  #include "qapi/qmp/qerror.h"
> +#include "qemu/tpm.h"
>  #include "tpm_int.h"
>  #include "tpm/tpm.h"
>  #include "qemu/config-file.h"
> @@ -145,6 +146,7 @@ static int configure_tpm(QemuOpts *opts)
>      const char *id;
>      const TPMDriverOps *be;
>      TPMBackend *drv;
> +    Error *local_err = NULL;
>  
>      if (!QLIST_EMPTY(&tpm_backends)) {
>          error_report("Only one TPM is allowed.\n");
> @@ -177,6 +179,13 @@ static int configure_tpm(QemuOpts *opts)
>          return 1;
>      }
>  
> +    tpm_backend_open(drv, &local_err);
> +    if (local_err) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);
> +        return 1;
> +    }
> +
>      QLIST_INSERT_HEAD(&tpm_backends, drv, list);
>  
>      return 0;
> @@ -197,7 +206,7 @@ void tpm_cleanup(void)
>  
>      QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) {
>          QLIST_REMOVE(drv, list);
> -        drv->ops->destroy(drv);
> +        tpm_backend_destroy(drv);
>      }
>  }
>  
> Index: qemu-git.pt/include/tpm/tpm.h
> ===================================================================
> --- qemu-git.pt.orig/include/tpm/tpm.h
> +++ qemu-git.pt/include/tpm/tpm.h
> @@ -14,6 +14,10 @@
>  
>  #include "qemu/option.h"
>  
> +typedef struct TPMState TPMState;
> +typedef struct TPMSizedBuffer TPMSizedBuffer;
> +typedef void (TPMRecvDataCB)(TPMState *, uint8_t locty);
> +
>  int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
>  int tpm_init(void);
>  void tpm_cleanup(void);
> 
> 
>

On 03/27/2013 08:06 AM, Paolo Bonzini wrote:
> Il 26/03/2013 17:28, Stefan Berger ha scritto:
>> QOM-ified the TPM support with much code borrowed from the rng implementation.
>>
>> What's missing may be that the tpm/tpm_passthrough.c be moved into backends/ .
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>
>> ---
>>   v1->v2:
>>    - followed to git revision e769bdc26
>>
>> ---
>>   backends/Makefile.objs |    2
>>   backends/tpm.c         |  154 ++++++++++++++++++++++++++++++++++++++++++++
>>   include/qemu/tpm.h     |  170 +++++++++++++++++++++++++++++++++++++++++++++++++
> I think these should be tpm_backend.[ch] (with the include file in
> include/tpm).  Can you rename the existing files with that name first?

With the above file naming and directory placement I followed the pattern of

backends/rng.c
include/qemu/rng.h

So are you planning on having them renamed and moved as well?
My intention was to have tpm_passthrough moved into backends/.
There's a file tpm/tpm_backend.c -- you want me to rename this one even 
though its located in a different directory?

Since I am not running a git repository any move/rename would be a 
deletion of a file plus its addition.

    Stefan

> Paolo
>

Il 27/03/2013 13:21, Stefan Berger ha scritto:
> 
> With the above file naming and directory placement I followed the
> pattern of
> 
> backends/rng.c
> include/qemu/rng.h
> 
> So are you planning on having them renamed and moved as well?

Uff, we're really bad at consistent naming. :)

Given the above, I guess backends/tpm.c is fine.  Then let's do the
following:

   include/tpm/tpm.h -> include/sysemu/tpm.h
   include/qemu/rng.h -> include/backends/rng.h
   your new include -> include/backends/tpm.h

> My intention was to have tpm_passthrough moved into backends/.

I'm not sure that is the right thing to do since tpm_passthrough has
dependencies on DeviceState.  It is not a pure backend, and if I
understand correctly it would likely not work with other TPM front-ends
than tpm_tis.c

> There's a file tpm/tpm_backend.c -- you want me to rename this one even
> though its located in a different directory?

tpm_backend.c and tpm_backend.h seem misnamed to begin with, so that
would be a separate but welcome change.

> Since I am not running a git repository any move/rename would be a
> deletion of a file plus its addition.

I don't understand.

Paolo

On 03/27/2013 11:26 AM, Paolo Bonzini wrote:
> Il 27/03/2013 13:21, Stefan Berger ha scritto:
>> With the above file naming and directory placement I followed the
>> pattern of
>>
>> backends/rng.c
>> include/qemu/rng.h
>>
>> So are you planning on having them renamed and moved as well?
> Uff, we're really bad at consistent naming. :)
>
> Given the above, I guess backends/tpm.c is fine.  Then let's do the
> following:
>
>     include/tpm/tpm.h -> include/sysemu/tpm.h
I will make this a separate patch.
>     include/qemu/rng.h -> include/backends/rng.h
>     your new include -> include/backends/tpm.h
Ok, I will repost the same patch with the path for the include file changed.

    Stefan