qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 04/35] tpm: reorganize headers and split hardwar

From: Corey Bryant
Subject: Re: [Qemu-devel] [PATCH 04/35] tpm: reorganize headers and split hardware part
Date: Wed, 20 Mar 2013 09:49:39 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130219 Thunderbird/17.0.3 


On 03/18/2013 01:34 PM, Paolo Bonzini wrote:

The TPM subsystem does not have a good front-end/back-end separation.
I think it has very good front-end/back-end separation, but perhaps you mean the code could be moved around and better organized. 


However, we can at least try to split the user interface (tpm.c) from
the implementation (hw/tpm).


Here's a general break-down of front-end/back-end/general code

tpm_tis.c = TIS front-end (the only front-end at this time)
tpm_passthrough.c = Passthrough backend (the only back-end at this time)
tpm.c = general code
tpm_backend.c = backend thread pool functions (could use a better file name?) 



The patches makes tpm.c not include tpm_int.h; instead it moves more
stuff to tpm_backend.h.

Signed-off-by: Paolo Bonzini <address@hidden>
---
  Makefile.objs                      |  2 +-
  default-configs/i386-softmmu.mak   |  2 +-
  default-configs/x86_64-softmmu.mak |  2 +-
  hw/Makefile.objs                   |  1 +
  {tpm => hw/tpm}/Makefile.objs      |  3 +-
  {tpm => hw/tpm}/tpm_backend.c      | 14 ++++++++++
  {tpm => hw/tpm}/tpm_int.h          | 55 ++----------------------------------
  {tpm => hw/tpm}/tpm_passthrough.c  |  0
  {tpm => hw/tpm}/tpm_tis.c          |  0
  {tpm => hw/tpm}/tpm_tis.h          |  5 ----
  {tpm => include/tpm}/tpm_backend.h | 57 ++++++++++++++++++++++++++++++++++++++
  tpm/tpm.c => tpm.c                 | 18 ++----------
  12 files changed, 80 insertions(+), 79 deletions(-)
  rename {tpm => hw/tpm}/Makefile.objs (61%)
  rename {tpm => hw/tpm}/tpm_backend.c (82%)
  rename {tpm => hw/tpm}/tpm_int.h (49%)
  rename {tpm => hw/tpm}/tpm_passthrough.c (100%)
  rename {tpm => hw/tpm}/tpm_tis.c (100%)
  rename {tpm => hw/tpm}/tpm_tis.h (94%)
  rename {tpm => include/tpm}/tpm_backend.h (50%)
  rename tpm/tpm.c => tpm.c (93%)

diff --git a/Makefile.objs b/Makefile.objs
index f99841c..ff3a6b3 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -74,7 +74,7 @@ common-obj-y += bt-host.o bt-vhci.o

  common-obj-y += dma-helpers.o
  common-obj-y += vl.o
-common-obj-y += tpm/
+common-obj-y += tpm.o

  common-obj-$(CONFIG_SLIRP) += slirp/

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 7d8908f..f70594d 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -26,4 +26,4 @@ CONFIG_HPET=y
  CONFIG_APPLESMC=y
  CONFIG_I8259=y
  CONFIG_PFLASH_CFI01=y
-CONFIG_TPM_TIS=$(CONFIG_TPM)
+CONFIG_TPM_TIS=y
diff --git a/default-configs/x86_64-softmmu.mak 
b/default-configs/x86_64-softmmu.mak
index e87e644..66c4855 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -26,4 +26,4 @@ CONFIG_HPET=y
  CONFIG_APPLESMC=y
  CONFIG_I8259=y
  CONFIG_PFLASH_CFI01=y
-CONFIG_TPM_TIS=$(CONFIG_TPM)
+CONFIG_TPM_TIS=y
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 09fea2c..5626292 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -25,6 +25,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += scsi/
  devices-dirs-$(CONFIG_SOFTMMU) += sd/
  devices-dirs-$(CONFIG_SOFTMMU) += ssi/
  devices-dirs-$(CONFIG_SOFTMMU) += timer/
+devices-dirs-$(CONFIG_TPM) += tpm/
  devices-dirs-$(CONFIG_SOFTMMU) += usb/
  devices-dirs-$(CONFIG_SOFTMMU) += virtio/
  devices-dirs-$(CONFIG_SOFTMMU) += watchdog/
diff --git a/tpm/Makefile.objs b/hw/tpm/Makefile.objs
similarity index 61%
rename from tpm/Makefile.objs
rename to hw/tpm/Makefile.objs
index 366e4a7..8bbed7a 100644
--- a/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,4 +1,3 @@
-common-obj-y = tpm.o
-common-obj-$(CONFIG_TPM) += tpm_backend.o
+common-obj-y += tpm_backend.o
  common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
  common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
diff --git a/tpm/tpm_backend.c b/hw/tpm/tpm_backend.c
similarity index 82%
rename from tpm/tpm_backend.c
rename to hw/tpm/tpm_backend.c
index 4144ef7..31d833c 100644
--- a/tpm/tpm_backend.c
+++ b/hw/tpm/tpm_backend.c
@@ -56,3 +56,17 @@ void tpm_backend_thread_tpm_reset(TPMBackendThread *tbt,
                             NULL);
      }
  }
+
+/*
+ * Write an error message in the given output buffer.
+ */
+void tpm_write_fatal_error_response(uint8_t *out, uint32_t out_len)
+{
+    if (out_len >= sizeof(struct tpm_resp_hdr)) {
+        struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
+
+        resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
+        resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
+        resp->errcode = cpu_to_be32(TPM_FAIL);
+    }
+}
I don't think moving this from tpm.c to tpm_backend.c helps anything. Maybe just renaming some of the files mentioned above might make the front-end vs back-end vs general code more intuitive. 

--
Regards,
Corey Bryant


diff --git a/tpm/tpm_int.h b/hw/tpm/tpm_int.h
similarity index 49%
rename from tpm/tpm_int.h
rename to hw/tpm/tpm_int.h
index f705643..d5f7bb8 100644
--- a/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -15,27 +15,8 @@
  #include "exec/memory.h"
  #include "tpm/tpm_tis.h"

-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 {
+struct TPMState {
      ISADevice busdev;
      MemoryRegion mmio;

@@ -48,38 +29,10 @@ typedef struct TPMState {

      char *backend;
      TPMBackend *be_driver;
-} TPMState;
+};

  #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)

-typedef void (TPMRecvDataCB)(TPMState *, uint8_t locty);
-
-struct TPMDriverOps {
-    enum TpmType type;
-    /* get a descriptive text of the backend to display to the user */
-    const char *(*desc)(void);
-
-    TPMBackend *(*create)(QemuOpts *opts, const char *id);
-    void (*destroy)(TPMBackend *t);
-
-    /* initialize the backend */
-    int (*init)(TPMBackend *t, TPMState *s, TPMRecvDataCB *datacb);
-    /* start up the TPM on the backend */
-    int (*startup_tpm)(TPMBackend *t);
-    /* returns true if nothing will ever answer TPM requests */
-    bool (*had_startup_error)(TPMBackend *t);
-
-    size_t (*realloc_buffer)(TPMSizedBuffer *sb);
-
-    void (*deliver_request)(TPMBackend *t);
-
-    void (*reset)(TPMBackend *t);
-
-    void (*cancel_cmd)(TPMBackend *t);
-
-    bool (*get_tpm_established_flag)(TPMBackend *t);
-};
-
  struct tpm_req_hdr {
      uint16_t tag;
      uint32_t len;
@@ -105,10 +58,6 @@ struct tpm_resp_hdr {
  #define TPM_ORD_GetTicks          0xf1

  TPMBackend *qemu_find_tpm(const char *id);
-int tpm_register_model(enum TpmModel model);
-int tpm_register_driver(const TPMDriverOps *tdo);
-void tpm_display_backend_drivers(void);
-const TPMDriverOps *tpm_get_backend_driver(const char *type);
  void tpm_write_fatal_error_response(uint8_t *out, uint32_t out_len);

  extern const TPMDriverOps tpm_passthrough_driver;
diff --git a/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
similarity index 100%
rename from tpm/tpm_passthrough.c
rename to hw/tpm/tpm_passthrough.c
diff --git a/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
similarity index 100%
rename from tpm/tpm_tis.c
rename to hw/tpm/tpm_tis.c
diff --git a/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
similarity index 94%
rename from tpm/tpm_tis.h
rename to hw/tpm/tpm_tis.h
index 963682c..916152a 100644
--- a/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -35,11 +35,6 @@
  #define TYPE_TPM_TIS                "tpm-tis"


-typedef struct TPMSizedBuffer {
-    uint32_t size;
-    uint8_t  *buffer;
-} TPMSizedBuffer;
-
  typedef enum {
      TPM_TIS_STATE_IDLE = 0,
      TPM_TIS_STATE_READY,
diff --git a/tpm/tpm_backend.h b/include/tpm/tpm_backend.h
similarity index 50%
rename from tpm/tpm_backend.h
rename to include/tpm/tpm_backend.h
index 05d94d0..f5390b4 100644
--- a/tpm/tpm_backend.h
+++ b/include/tpm/tpm_backend.h
@@ -42,4 +42,61 @@ typedef enum TPMBackendCmd {
      TPM_BACKEND_CMD_TPM_RESET,
  } TPMBackendCmd;

+struct TPMDriverOps;
+typedef struct TPMDriverOps TPMDriverOps;
+
+typedef struct TPMState TPMState;
+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;
+
+typedef void (TPMRecvDataCB)(TPMState *, uint8_t locty);
+
+typedef struct TPMSizedBuffer {
+    uint32_t size;
+    uint8_t  *buffer;
+} TPMSizedBuffer;
+
+struct TPMDriverOps {
+    enum TpmType type;
+    /* get a descriptive text of the backend to display to the user */
+    const char *(*desc)(void);
+
+    TPMBackend *(*create)(QemuOpts *opts, const char *id);
+    void (*destroy)(TPMBackend *t);
+
+    /* initialize the backend */
+    int (*init)(TPMBackend *t, TPMState *s, TPMRecvDataCB *datacb);
+    /* start up the TPM on the backend */
+    int (*startup_tpm)(TPMBackend *t);
+    /* returns true if nothing will ever answer TPM requests */
+    bool (*had_startup_error)(TPMBackend *t);
+
+    size_t (*realloc_buffer)(TPMSizedBuffer *sb);
+
+    void (*deliver_request)(TPMBackend *t);
+
+    void (*reset)(TPMBackend *t);
+
+    void (*cancel_cmd)(TPMBackend *t);
+
+    bool (*get_tpm_established_flag)(TPMBackend *t);
+};
+
+const TPMDriverOps *tpm_get_backend_driver(const char *type);
+int tpm_register_model(enum TpmModel model);
+int tpm_register_driver(const TPMDriverOps *tdo);
+
  #endif /* TPM_TPM_BACKEND_H */
diff --git a/tpm/tpm.c b/tpm.c
similarity index 93%
rename from tpm/tpm.c
rename to tpm.c
index ffd2495..49ac4cc 100644
--- a/tpm/tpm.c
+++ b/tpm.c
@@ -15,7 +15,7 @@

  #include "monitor/monitor.h"
  #include "qapi/qmp/qerror.h"
-#include "tpm_int.h"
+#include "tpm/tpm_backend.h"
  #include "tpm/tpm.h"
  #include "qemu/config-file.h"
  #include "qmp-commands.h"
@@ -61,20 +61,6 @@ static bool tpm_model_is_registered(enum TpmModel model)
      return false;
  }

-/*
- * Write an error message in the given output buffer.
- */
-void tpm_write_fatal_error_response(uint8_t *out, uint32_t out_len)
-{
-    if (out_len >= sizeof(struct tpm_resp_hdr)) {
-        struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out;
-
-        resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND);
-        resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr));
-        resp->errcode = cpu_to_be32(TPM_FAIL);
-    }
-}
-
  const TPMDriverOps *tpm_get_backend_driver(const char *type)
  {
      int i;
@@ -108,7 +94,7 @@ int tpm_register_driver(const TPMDriverOps *tdo)
   * Walk the list of available TPM backend drivers and display them on the
   * screen.
   */
-void tpm_display_backend_drivers(void)
+static void tpm_display_backend_drivers(void)
  {
      int i;
reply via email to
[Prev in Thread] Current Thread [Next in Thread]