[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/4] stubdom: Change vTPM shared page ABI



Ok I've taken time to look at your patch. I had to make some minor changes to make it work but other than that I was able to test it and it worked.
Comments inlined.

On 11/27/2012 10:14 AM, Daniel De Graaf wrote:
This changes the vTPM shared page ABI from a copy of the Xen network
interface to a single-page interface that better reflects the expected
behavior of a TPM: only a single request packet can be sent at any given
time, and every packet sent generates a single response packet. This
protocol change should also increase efficiency as it avoids mapping and
unmapping grants when possible.

Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
---
  extras/mini-os/include/tpmback.h  |   1 +
  extras/mini-os/include/tpmfront.h |   7 +-
  extras/mini-os/tpmback.c          | 159 +++++++++++++++++++------------------
  extras/mini-os/tpmfront.c         | 162 ++++++++++++++++++++++----------------
  xen/include/public/io/tpmif.h     |  45 +++--------
  5 files changed, 191 insertions(+), 183 deletions(-)

diff --git a/extras/mini-os/include/tpmback.h b/extras/mini-os/include/tpmback.h
index ff86732..ec9eda4 100644
--- a/extras/mini-os/include/tpmback.h
+++ b/extras/mini-os/include/tpmback.h
@@ -43,6 +43,7 @@

  struct tpmcmd {
     domid_t domid;              /* Domid of the frontend */
+   uint8_t locality;    /* Locality requested by the frontend */
     unsigned int handle;        /* Handle of the frontend */
     unsigned char uuid[16];                     /* uuid of the tpm interface */

diff --git a/extras/mini-os/include/tpmfront.h 
b/extras/mini-os/include/tpmfront.h
index fd2cb17..a0c7c4d 100644
--- a/extras/mini-os/include/tpmfront.h
+++ b/extras/mini-os/include/tpmfront.h
@@ -37,9 +37,7 @@ struct tpmfront_dev {
     grant_ref_t ring_ref;
     evtchn_port_t evtchn;

-   tpmif_tx_interface_t* tx;
-
-   void** pages;
+   vtpm_shared_page_t *page;

     domid_t bedomid;
     char* nodename;
@@ -77,6 +75,9 @@ void shutdown_tpmfront(struct tpmfront_dev* dev);
   * */f
  int tpmfront_cmd(struct tpmfront_dev* dev, uint8_t* req, size_t reqlen, 
uint8_t** resp, size_t* resplen);

+/* Set the locality used for communicating with a vTPM */
+int tpmfront_set_locality(struct tpmfront_dev* dev, int locality);
+
  #ifdef HAVE_LIBC
  #include <sys/stat.h>
  /* POSIX IO functions:
diff --git a/extras/mini-os/tpmback.c b/extras/mini-os/tpmback.c
index 658fed1..2d31061 100644
--- a/extras/mini-os/tpmback.c
+++ b/extras/mini-os/tpmback.c
@@ -86,10 +86,7 @@ struct tpmif {
     evtchn_port_t evtchn;

     /* Shared page */
-   tpmif_tx_interface_t* tx;
-
-   /* pointer to TPMIF_RX_RING_SIZE pages */
-   void** pages;
+   vtpm_shared_page_t *page;

     enum xenbus_state state;
     enum { DISCONNECTED, DISCONNECTING, CONNECTED } status;
@@ -266,6 +263,7 @@ int insert_tpmif(tpmif_t* tpmif)
     unsigned int i, j;
     tpmif_t* tmp;
     char* err;
+   char path[512];

     local_irq_save(flags);

@@ -303,6 +301,16 @@ int insert_tpmif(tpmif_t* tpmif)

     local_irq_restore(flags);

+   snprintf(path, 512, "backend/vtpm/%u/%u/feature-protocol-v2", (unsigned int) 
tpmif->domid, tpmif->handle);
+   if ((err = xenbus_write(XBT_NIL, path, "1")))
+   {
+      /* if we got an error here we should carefully remove the interface and 
then return */
+      TPMBACK_ERR("Unable to write feature-protocol-v2 node: %s\n", err);
+      free(err);
+      remove_tpmif(tpmif);
+      goto error_post_irq;
+   }
+
My preference is still to do away with the versioning stuff since tpm is just getting released. Its not even in linux yet so there is no confusion. We can even merge the linux patches together and resubmit as one if thats preferrable. Konrad, Ian, your final votes on that?

If we do go the versioning route, I'd rather a feature-protocol key with value=2 or something like that to make it upgradable. Also don't forget to add this to the linux side, its not in the patch you sent me yet.
     /*Listen for state changes on the new interface */
     if((err = xenbus_watch_path_token(XBT_NIL, tpmif->fe_state_path, 
tpmif->fe_state_path, &gtpmdev.events)))
     {
@@ -312,7 +320,6 @@ int insert_tpmif(tpmif_t* tpmif)
        remove_tpmif(tpmif);
        goto error_post_irq;
     }
-
     return 0;
  error:
     local_irq_restore(flags);
@@ -386,8 +393,7 @@ inline tpmif_t* __init_tpmif(domid_t domid, unsigned int 
handle)
     tpmif->fe_state_path = NULL;
     tpmif->state = XenbusStateInitialising;
     tpmif->status = DISCONNECTED;
-   tpmif->tx = NULL;
-   tpmif->pages = NULL;
+   tpmif->page = NULL;
     tpmif->flags = 0;
     memset(tpmif->uuid, 0, sizeof(tpmif->uuid));
     return tpmif;
@@ -395,9 +401,6 @@ inline tpmif_t* __init_tpmif(domid_t domid, unsigned int 
handle)

  void __free_tpmif(tpmif_t* tpmif)
  {
-   if(tpmif->pages) {
-      free(tpmif->pages);
-   }
     if(tpmif->fe_path) {
        free(tpmif->fe_path);
     }
@@ -430,12 +433,6 @@ tpmif_t* new_tpmif(domid_t domid, unsigned int handle)
        goto error;
     }

-   /* allocate pages to be used for shared mapping */
-   if((tpmif->pages = malloc(sizeof(void*) * TPMIF_TX_RING_SIZE)) == NULL) {
-      goto error;
-   }
-   memset(tpmif->pages, 0, sizeof(void*) * TPMIF_TX_RING_SIZE);
-
     if(tpmif_change_state(tpmif, XenbusStateInitWait)) {
        goto error;
     }
@@ -486,7 +483,7 @@ void free_tpmif(tpmif_t* tpmif)
        tpmif->status = DISCONNECTING;
        mask_evtchn(tpmif->evtchn);

-      if(gntmap_munmap(&gtpmdev.map, (unsigned long)tpmif->tx, 1)) {
+      if(gntmap_munmap(&gtpmdev.map, (unsigned long)tpmif->page, 1)) {
          TPMBACK_ERR("%u/%u Error occured while trying to unmap shared page\n", 
(unsigned int) tpmif->domid, tpmif->handle);
        }

@@ -529,9 +526,10 @@ void free_tpmif(tpmif_t* tpmif)
  void tpmback_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
  {
     tpmif_t* tpmif = (tpmif_t*) data;
-   tpmif_tx_request_t* tx = &tpmif->tx->ring[0].req;
-   /* Throw away 0 size events, these can trigger from event channel unmasking 
*/
-   if(tx->size == 0)
+   vtpm_shared_page_t* pg = tpmif->page;
+
+   /* Only pay attention if the request is ready */
+   if (pg->state == 0)
        return;

     TPMBACK_DEBUG("EVENT CHANNEL FIRE %u/%u\n", (unsigned int) tpmif->domid, 
tpmif->handle);
@@ -584,12 +582,26 @@ int connect_fe(tpmif_t* tpmif)
     }
     free(value);

+   /* Check that protocol v2 is being used */
+   snprintf(path, 512, "%s/feature-protocol-v2", tpmif->fe_path);
+   if((err = xenbus_read(XBT_NIL, path, &value))) {
+      TPMBACK_ERR("Unable to read %s during tpmback initialization! error = 
%s\n", path, err);
+      free(err);
+      return -1;
+   }
+   if(strcmp(value, "1")) {
+      TPMBACK_ERR("%s has an invalid value (%s)\n", path, value);
+      free(value);
+      return -1;
+   }
+   free(value);
+
+
     domid = tpmif->domid;
-   if((tpmif->tx = gntmap_map_grant_refs(&gtpmdev.map, 1, &domid, 0, &ringref, 
PROT_READ | PROT_WRITE)) == NULL) {
+   if((tpmif->page = gntmap_map_grant_refs(&gtpmdev.map, 1, &domid, 0, 
&ringref, PROT_READ | PROT_WRITE)) == NULL) {
        TPMBACK_ERR("Failed to map grant reference %u/%u\n", (unsigned int) 
tpmif->domid, tpmif->handle);
        return -1;
     }
-   memset(tpmif->tx, 0, PAGE_SIZE);

     /*Bind the event channel */
     if((evtchn_bind_interdomain(tpmif->domid, evtchn, tpmback_handler, tpmif, 
&tpmif->evtchn)))
@@ -618,10 +630,28 @@ error_post_evtchn:
     mask_evtchn(tpmif->evtchn);
     unbind_evtchn(tpmif->evtchn);
  error_post_map:
-   gntmap_munmap(&gtpmdev.map, (unsigned long)tpmif->tx, 1);
+   gntmap_munmap(&gtpmdev.map, (unsigned long)tpmif->page, 1);
     return -1;
  }

+static void disconnect_fe(tpmif_t* tpmif)
+{
+   if (tpmif->status == CONNECTED) {
+      tpmif->status = DISCONNECTING;
+      mask_evtchn(tpmif->evtchn);
+
+      if(gntmap_munmap(&gtpmdev.map, (unsigned long)tpmif->page, 1)) {
+        TPMBACK_ERR("%u/%u Error occured while trying to unmap shared page\n", 
(unsigned int) tpmif->domid, tpmif->handle);
+      }
+
+      unbind_evtchn(tpmif->evtchn);
+   }
+   tpmif->status = DISCONNECTED;
+   tpmif_change_state(tpmif, XenbusStateReconfigured);
+
+   TPMBACK_LOG("Frontend %u/%u disconnected\n", (unsigned int) tpmif->domid, 
tpmif->handle);
+}
+
This function is flagged by -Wunused-function. I think you meant to include this in your second patch that does the reconfig stuff.
  static int frontend_changed(tpmif_t* tpmif)
  {
     int state = xenbus_read_integer(tpmif->fe_state_path);
@@ -874,6 +904,7 @@ void shutdown_tpmback(void)
  inline void init_tpmcmd(tpmcmd_t* tpmcmd, domid_t domid, unsigned int handle, 
unsigned char uuid[16])
  {
     tpmcmd->domid = domid;
+   tpmcmd->locality = -1;
     tpmcmd->handle = handle;
     memcpy(tpmcmd->uuid, uuid, sizeof(tpmcmd->uuid));
     tpmcmd->req = NULL;
@@ -884,12 +915,12 @@ inline void init_tpmcmd(tpmcmd_t* tpmcmd, domid_t domid, 
unsigned int handle, un

  tpmcmd_t* get_request(tpmif_t* tpmif) {
     tpmcmd_t* cmd;
-   tpmif_tx_request_t* tx;
-   int offset;
-   int tocopy;
-   int i;
-   uint32_t domid;
+   vtpm_shared_page_t* shr;
+   unsigned int offset;
     int flags;
+#ifdef TPMBACK_PRINT_DEBUG
+   int i;
+#endif

     local_irq_save(flags);

@@ -899,35 +930,22 @@ tpmcmd_t* get_request(tpmif_t* tpmif) {
     }
     init_tpmcmd(cmd, tpmif->domid, tpmif->handle, tpmif->uuid);

-   tx = &tpmif->tx->ring[0].req;
-   cmd->req_len = tx->size;
+   shr = tpmif->page;
+   cmd->req_len = shr->length;
+   cmd->locality = shr->locality;
+   offset = sizeof(*shr) + 4*shr->nr_extra_pages;
+   if (offset > PAGE_SIZE || offset + cmd->req_len > PAGE_SIZE) {
+      TPMBACK_ERR("%u/%u Command size too long for shared page!\n", (unsigned int) 
tpmif->domid, tpmif->handle);
+      goto error;
+   }
     /* Allocate the buffer */
     if(cmd->req_len) {
        if((cmd->req = malloc(cmd->req_len)) == NULL) {
          goto error;
        }
     }
-   /* Copy the bits from the shared pages */
-   offset = 0;
-   for(i = 0; i < TPMIF_TX_RING_SIZE && offset < cmd->req_len; ++i) {
-      tx = &tpmif->tx->ring[i].req;
-
-      /* Map the page with the data */
-      domid = (uint32_t)tpmif->domid;
-      if((tpmif->pages[i] = gntmap_map_grant_refs(&gtpmdev.map, 1, &domid, 0, 
&tx->ref, PROT_READ)) == NULL) {
-        TPMBACK_ERR("%u/%u Unable to map shared page during read!\n", (unsigned int) 
tpmif->domid, tpmif->handle);
-        goto error;
-      }
-
-      /* do the copy now */
-      tocopy = min(cmd->req_len - offset, PAGE_SIZE);
-      memcpy(&cmd->req[offset], tpmif->pages[i], tocopy);
-      offset += tocopy;
-
-      /* release the page */
-      gntmap_munmap(&gtpmdev.map, (unsigned long)tpmif->pages[i], 1);
-
-   }
+   /* Copy the bits from the shared page(s) */
+   memcpy(cmd->req, offset + (uint8_t*)shr, cmd->req_len);

  #ifdef TPMBACK_PRINT_DEBUG
     TPMBACK_DEBUG("Received Tpm Command from %u/%u of size %u", (unsigned int) 
tpmif->domid, tpmif->handle, cmd->req_len);
@@ -958,38 +976,24 @@ error:

  void send_response(tpmcmd_t* cmd, tpmif_t* tpmif)
  {
-   tpmif_tx_request_t* tx;
-   int offset;
-   int i;
-   uint32_t domid;
-   int tocopy;
+   vtpm_shared_page_t* shr;
+   unsigned int offset;
     int flags;
+#ifdef TPMBACK_PRINT_DEBUG
+int i;
+#endif

     local_irq_save(flags);

-   tx = &tpmif->tx->ring[0].req;
-   tx->size = cmd->resp_len;
-
-   offset = 0;
-   for(i = 0; i < TPMIF_TX_RING_SIZE && offset < cmd->resp_len; ++i) {
-      tx = &tpmif->tx->ring[i].req;
-
-      /* Map the page with the data */
-      domid = (uint32_t)tpmif->domid;
-      if((tpmif->pages[i] = gntmap_map_grant_refs(&gtpmdev.map, 1, &domid, 0, 
&tx->ref, PROT_WRITE)) == NULL) {
-        TPMBACK_ERR("%u/%u Unable to map shared page during write!\n", (unsigned 
int) tpmif->domid, tpmif->handle);
-        goto error;
-      }
-
-      /* do the copy now */
-      tocopy = min(cmd->resp_len - offset, PAGE_SIZE);
-      memcpy(tpmif->pages[i], &cmd->resp[offset], tocopy);
-      offset += tocopy;
-
-      /* release the page */
-      gntmap_munmap(&gtpmdev.map, (unsigned long)tpmif->pages[i], 1);
+   shr = tpmif->page;
+   shr->length = cmd->resp_len;

+   offset = sizeof(*shr) + 4*shr->nr_extra_pages;
+   if (offset > PAGE_SIZE || offset + cmd->resp_len > PAGE_SIZE) {
+      TPMBACK_ERR("%u/%u Command size too long for shared page!\n", (unsigned int) 
tpmif->domid, tpmif->handle);
+      goto error;
     }
+   memcpy(offset + (uint8_t*)shr, cmd->resp, cmd->resp_len);

  #ifdef TPMBACK_PRINT_DEBUG
     TPMBACK_DEBUG("Sent response to %u/%u of size %u", (unsigned int) tpmif->domid, 
tpmif->handle, cmd->resp_len);
@@ -1003,6 +1007,7 @@ void send_response(tpmcmd_t* cmd, tpmif_t* tpmif)
  #endif
     /* clear the ready flag and send the event channel notice to the frontend 
*/
     tpmif_req_finished(tpmif);
+   shr->state = 0;
     notify_remote_via_evtchn(tpmif->evtchn);
  error:
     local_irq_restore(flags);
diff --git a/extras/mini-os/tpmfront.c b/extras/mini-os/tpmfront.c
index 0218d7f..c1cbab3 100644
--- a/extras/mini-os/tpmfront.c
+++ b/extras/mini-os/tpmfront.c
@@ -153,6 +153,32 @@ static int wait_for_backend_closed(xenbus_event_queue* 
events, char* path)

  }

+static int wait_for_backend_reconfig(xenbus_event_queue* events, char* path)
+{
+   int state;
+
+   TPMFRONT_LOG("Waiting for backend to reconfigure...\n");
+   while(1) {
+      state = xenbus_read_integer(path);
+      if ( state < 0)
+        state = XenbusStateUnknown;
+      switch(state) {
+        case XenbusStateUnknown:
+           TPMFRONT_ERR("Backend Unknown state, forcing shutdown\n");
+           return -1;
+        case XenbusStateClosed:
+           TPMFRONT_LOG("Backend Closed\n");
+           return 0;
+        case XenbusStateReconfigured:
+           TPMFRONT_LOG("Backend Reconfigured\n");
+           return 0;
+        default:
+           xenbus_wait_for_watch(events);
+      }
+   }
+
+}
+
Does this reconfigure stuff make sense without patch 2?
  static int wait_for_backend_state_changed(struct tpmfront_dev* dev, 
XenbusState state) {
     char* err;
     int ret = 0;
@@ -175,8 +201,11 @@ static int wait_for_backend_state_changed(struct 
tpmfront_dev* dev, XenbusState
        case XenbusStateClosed:
          ret = wait_for_backend_closed(&events, path);
          break;
-      default:
+      case XenbusStateReconfigured:
+        ret = wait_for_backend_reconfig(&events, path);
          break;
+      default:
+         TPMFRONT_ERR("Bad wait state %d, ignoring\n", state);
     }
See above comment

     if((err = xenbus_unwatch_path_token(XBT_NIL, path, path))) {
@@ -190,13 +219,13 @@ static int tpmfront_connect(struct tpmfront_dev* dev)
  {
     char* err;
     /* Create shared page */
-   dev->tx = (tpmif_tx_interface_t*) alloc_page();
-   if(dev->tx == NULL) {
+   dev->page = (vtpm_shared_page_t*) alloc_page();
+   if(dev->page == NULL) {
        TPMFRONT_ERR("Unable to allocate page for shared memory\n");
        goto error;
     }
-   memset(dev->tx, 0, PAGE_SIZE);
-   dev->ring_ref = gnttab_grant_access(dev->bedomid, virt_to_mfn(dev->tx), 0);
+   memset(dev->page, 0, PAGE_SIZE);
+   dev->ring_ref = gnttab_grant_access(dev->bedomid, virt_to_mfn(dev->page), 
0);
     TPMFRONT_DEBUG("grant ref is %lu\n", (unsigned long) dev->ring_ref);

     /*Create event channel */
@@ -228,7 +257,7 @@ error_postevtchn:
        unbind_evtchn(dev->evtchn);
  error_postmap:
        gnttab_end_access(dev->ring_ref);
-      free_page(dev->tx);
+      free_page(dev->page);
  error:
     return -1;
  }
@@ -240,7 +269,6 @@ struct tpmfront_dev* init_tpmfront(const char* _nodename)
     char path[512];
     char* value, *err;
     unsigned long long ival;
-   int i;

     printk("============= Init TPM Front ================\n");

@@ -279,6 +307,15 @@ struct tpmfront_dev* init_tpmfront(const char* _nodename)
        goto error;
     }

+   /* Publish protocol v2 feature */
+   snprintf(path, 512, "%s/feature-protocol-v2", dev->nodename);
+   if ((err = xenbus_write(XBT_NIL, path, "1")))
+   {
+      TPMFRONT_ERR("Unable to write feature-protocol-v2 node: %s\n", err);
+         free(err);
+      goto error;
+   }
+
     /* Create and publish grant reference and event channel */
     if (tpmfront_connect(dev)) {
        goto error;
@@ -289,18 +326,18 @@ struct tpmfront_dev* init_tpmfront(const char* _nodename)
        goto error;
     }

-   /* Allocate pages that will contain the messages */
-   dev->pages = malloc(sizeof(void*) * TPMIF_TX_RING_SIZE);
-   if(dev->pages == NULL) {
+   snprintf(path, 512, "%s/feature-protocol-v2", dev->bepath);
+   if((err = xenbus_read(XBT_NIL, path, &value))) {
+      TPMFRONT_ERR("Unable to read %s during tpmfront initialization! error = 
%s\n", path, err);
+      free(err);
        goto error;
     }
-   memset(dev->pages, 0, sizeof(void*) * TPMIF_TX_RING_SIZE);
-   for(i = 0; i < TPMIF_TX_RING_SIZE; ++i) {
-      dev->pages[i] = (void*)alloc_page();
-      if(dev->pages[i] == NULL) {
-        goto error;
-      }
+   if(strcmp(value, "1")) {
+      TPMFRONT_ERR("%s has an invalid value (%s)\n", path, value);
+      free(value);
+      goto error;
     }
+   free(value);

     TPMFRONT_LOG("Initialization Completed successfully\n");

@@ -314,12 +351,10 @@ void shutdown_tpmfront(struct tpmfront_dev* dev)
  {
     char* err;
     char path[512];
-   int i;
-   tpmif_tx_request_t* tx;
     if(dev == NULL) {
        return;
     }
-   TPMFRONT_LOG("Shutting down tpmfront\n");
+   TPMFRONT_LOG("Shutting down tpmfront%s\n", for_reconfig ? " for reconfigure" : 
"");
I think the above line belongs in your second patch. for_reconfig is undefined, causing compilation to fail.
     /* disconnect */
     if(dev->state == XenbusStateConnected) {
        dev->state = XenbusStateClosing;
@@ -349,27 +384,12 @@ void shutdown_tpmfront(struct tpmfront_dev* dev)
        /* Wait for the backend to close and unmap shared pages, ignore any 
errors */
        wait_for_backend_state_changed(dev, XenbusStateClosed);

-      /* Cleanup any shared pages */
-      if(dev->pages) {
-        for(i = 0; i < TPMIF_TX_RING_SIZE; ++i) {
-           if(dev->pages[i]) {
-              tx = &dev->tx->ring[i].req;
-              if(tx->ref != 0) {
-                 gnttab_end_access(tx->ref);
-              }
-              free_page(dev->pages[i]);
-           }
-        }
-        free(dev->pages);
-      }
-
        /* Close event channel and unmap shared page */
        mask_evtchn(dev->evtchn);
        unbind_evtchn(dev->evtchn);
        gnttab_end_access(dev->ring_ref);

-      free_page(dev->tx);
-
+      free_page(dev->page);
     }

     /* Cleanup memory usage */
@@ -387,13 +407,17 @@ void shutdown_tpmfront(struct tpmfront_dev* dev)

  int tpmfront_send(struct tpmfront_dev* dev, const uint8_t* msg, size_t length)
  {
+   unsigned int offset;
+   vtpm_shared_page_t* shr = NULL;
+#ifdef TPMFRONT_PRINT_DEBUG
     int i;
-   tpmif_tx_request_t* tx = NULL;
+#endif
     /* Error Checking */
     if(dev == NULL || dev->state != XenbusStateConnected) {
        TPMFRONT_ERR("Tried to send message through disconnected frontend\n");
        return -1;
     }
+   shr = dev->page;

  #ifdef TPMFRONT_PRINT_DEBUG
     TPMFRONT_DEBUG("Sending Msg to backend size=%u", (unsigned int) length);
@@ -407,19 +431,16 @@ int tpmfront_send(struct tpmfront_dev* dev, const 
uint8_t* msg, size_t length)
  #endif

     /* Copy to shared pages now */
-   for(i = 0; length > 0 && i < TPMIF_TX_RING_SIZE; ++i) {
-      /* Share the page */
-      tx = &dev->tx->ring[i].req;
-      tx->unused = 0;
-      tx->addr = virt_to_mach(dev->pages[i]);
-      tx->ref = gnttab_grant_access(dev->bedomid, virt_to_mfn(dev->pages[i]), 
0);
-      /* Copy the bits to the page */
-      tx->size = length > PAGE_SIZE ? PAGE_SIZE : length;
-      memcpy(dev->pages[i], &msg[i * PAGE_SIZE], tx->size);
-
-      /* Update counters */
-      length -= tx->size;
+   offset = sizeof(*shr);
+   if (length + offset > PAGE_SIZE) {
+      TPMFRONT_ERR("Message too long for shared page\n");
+      return -1;
     }
I think it might be useful to #define a TPMIF_CAP_PROP_BUFFER_SIZE in tpmif.h so people know ahead of time the largest command they can send. Then they can create arrays on the stack with the correct length etc..

This brings up another issue. According to the TPM PC Client spec section 11.3.8, there is a capability TPM_CAP_PROP_BUFFER_SIZE. We should probably patch the emulator in vtpm-stubdom to make it return TPMIF_CAP_PROP_BUFFER_SIZE for this capability. You don't have to worry about this now if you don't want to. I can do it later once this stuff is in. If you do decide to tackle it, make it a separate patch.

+   memcpy(offset + (uint8_t*)shr, msg, length);
+   shr->length = length;
+   barrier();
+   shr->state = 1;
+
     dev->waiting = 1;
     dev->resplen = 0;
  #ifdef HAVE_LIBC
@@ -434,44 +455,41 @@ int tpmfront_send(struct tpmfront_dev* dev, const 
uint8_t* msg, size_t length)
  }
  int tpmfront_recv(struct tpmfront_dev* dev, uint8_t** msg, size_t *length)
  {
-   tpmif_tx_request_t* tx;
-   int i;
+   unsigned int offset;
+   vtpm_shared_page_t* shr = NULL;
+#ifdef TPMFRONT_PRINT_DEBUG
+int i;
+#endif
     if(dev == NULL || dev->state != XenbusStateConnected) {
        TPMFRONT_ERR("Tried to receive message from disconnected frontend\n");
        return -1;
     }
     /*Wait for the response */
     wait_event(dev->waitq, (!dev->waiting));
+   shr = dev->page;
+   if (shr->state != 0)
+      goto quit;

     /* Initialize */
     *msg = NULL;
-   *length = 0;
+   *length = shr->length;
+   offset = sizeof(*shr);

-   /* special case, just quit */
-   tx = &dev->tx->ring[0].req;
-   if(tx->size == 0 ) {
-       goto quit;
-   }
-   /* Get the total size */
-   tx = &dev->tx->ring[0].req;
-   for(i = 0; i < TPMIF_TX_RING_SIZE && tx->size > 0; ++i) {
-      tx = &dev->tx->ring[i].req;
-      *length += tx->size;
+   if (*length + offset > PAGE_SIZE) {
+      TPMFRONT_ERR("Reply too long for shared page\n");
+      return -1;
     }
+
     /* Alloc the buffer */
     if(dev->respbuf) {
        free(dev->respbuf);
     }
     *msg = dev->respbuf = malloc(*length);
     dev->resplen = *length;
+
     /* Copy the bits */
-   tx = &dev->tx->ring[0].req;
-   for(i = 0; i < TPMIF_TX_RING_SIZE && tx->size > 0; ++i) {
-      tx = &dev->tx->ring[i].req;
-      memcpy(&(*msg)[i * PAGE_SIZE], dev->pages[i], tx->size);
-      gnttab_end_access(tx->ref);
-      tx->ref = 0;
-   }
+   memcpy(*msg, offset + (uint8_t*)shr, *length);
+
  #ifdef TPMFRONT_PRINT_DEBUG
     TPMFRONT_DEBUG("Received response from backend size=%u", (unsigned int) 
*length);
     for(i = 0; i < *length; ++i) {
@@ -504,6 +522,14 @@ int tpmfront_cmd(struct tpmfront_dev* dev, uint8_t* req, 
size_t reqlen, uint8_t*
     return 0;
  }

+int tpmfront_set_locality(struct tpmfront_dev* dev, int locality)
+{
+   if (!dev || !dev->page)
+      return -1;
+   dev->page->locality = locality;
+   return 0;
+}
+
  #ifdef HAVE_LIBC
  #include <errno.h>
  int tpmfront_open(struct tpmfront_dev* dev)
diff --git a/xen/include/public/io/tpmif.h b/xen/include/public/io/tpmif.h
index 02ccdab..afc9181 100644
--- a/xen/include/public/io/tpmif.h
+++ b/xen/include/public/io/tpmif.h
@@ -1,7 +1,7 @@
  
/******************************************************************************
   * tpmif.h
   *
- * TPM I/O interface for Xen guest OSes.
+ * TPM I/O interface for Xen guest OSes, v2
   *
   * Permission is hereby granted, free of charge, to any person obtaining a 
copy
   * of this software and associated documentation files (the "Software"), to
@@ -21,48 +21,23 @@
   * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
   * DEALINGS IN THE SOFTWARE.
   *
- * Copyright (c) 2005, IBM Corporation
- *
- * Author: Stefan Berger, stefanb@xxxxxxxxxx
- * Grant table support: Mahadevan Gomathisankaran
- *
- * This code has been derived from tools/libxc/xen/io/netif.h
- *
- * Copyright (c) 2003-2004, Keir Fraser
   */

  #ifndef __XEN_PUBLIC_IO_TPMIF_H__
  #define __XEN_PUBLIC_IO_TPMIF_H__

-#include "../grant_table.h"
+struct vtpm_shared_page {
+    uint16_t length;         /* request/response length in bytes */

-struct tpmif_tx_request {
-    unsigned long addr;   /* Machine address of packet.   */
-    grant_ref_t ref;      /* grant table access reference */
-    uint16_t unused;
-    uint16_t size;        /* Packet size in bytes.        */
-};
-typedef struct tpmif_tx_request tpmif_tx_request_t;
-
-/*
- * The TPMIF_TX_RING_SIZE defines the number of pages the
- * front-end and backend can exchange (= size of array).
- */
-typedef uint32_t TPMIF_RING_IDX;
-
-#define TPMIF_TX_RING_SIZE 1
-
-/* This structure must fit in a memory page. */
-
-struct tpmif_ring {
-    struct tpmif_tx_request req;
-};
-typedef struct tpmif_ring tpmif_ring_t;
+    uint8_t state;           /* 0 - response ready / idle
+                              * 1 - request ready / working */
+    uint8_t locality;        /* for the current request */
+    uint8_t padding[3];

-struct tpmif_tx_interface {
-    struct tpmif_ring ring[TPMIF_TX_RING_SIZE];
+    uint8_t nr_extra_pages;  /* extra pages for long packets; may be zero */
+    uint32_t extra_pages[0]; /* grant IDs; length is actually nr_extra_pages */
  };
-typedef struct tpmif_tx_interface tpmif_tx_interface_t;
+typedef struct vtpm_shared_page vtpm_shared_page_t;

  #endif

--
1.7.11.7



Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.