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

Re: [Xen-devel] [PATCH 2/4] stubdom/vtpm: Add reconfiguration support



The purpose of this is to allow 2 entities in the same vm to use tpm drivers, pv_grub and the linux guest. The Xenbus Reconfigure states are new to me. Is this intended behavior in line with the original purpose of the Reconfigure states or are we hijacking them to do something not in the original xen front/back driver spec?

It looks like pv-grub just shuts down blkfront and the others without any reconfigure magic. Given that vtpm-stubdom will no longer automatically shutdown with your later patch, is there any reason we cannot just do the same here?

On 11/27/2012 10:14 AM, Daniel De Graaf wrote:
Allow the vtpm device to be disconnected and reconnected so that a
bootloader (like pv-grub) can submit measurements and return the vtpm
device to its initial state before booting the target kernel.

Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
---
  extras/mini-os/include/tpmfront.h |  2 +-
  extras/mini-os/lib/sys.c          |  2 +-
  extras/mini-os/tpmback.c          |  5 +++++
  extras/mini-os/tpmfront.c         | 15 +++++++++------
  stubdom/vtpm/vtpm.c               |  2 +-
  5 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/extras/mini-os/include/tpmfront.h 
b/extras/mini-os/include/tpmfront.h
index a0c7c4d..913faa4 100644
--- a/extras/mini-os/include/tpmfront.h
+++ b/extras/mini-os/include/tpmfront.h
@@ -61,7 +61,7 @@ struct tpmfront_dev {
  /*Initialize frontend */
  struct tpmfront_dev* init_tpmfront(const char* nodename);
  /*Shutdown frontend */
-void shutdown_tpmfront(struct tpmfront_dev* dev);
+void shutdown_tpmfront(struct tpmfront_dev* dev, int for_reconfig);
/* Send a tpm command to the backend and wait for the response
   *
diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c
index 3cc3340..03da4f0 100644
--- a/extras/mini-os/lib/sys.c
+++ b/extras/mini-os/lib/sys.c
@@ -459,7 +459,7 @@ int close(int fd)
  #endif
  #ifdef CONFIG_TPMFRONT
        case FTYPE_TPMFRONT:
-            shutdown_tpmfront(files[fd].tpmfront.dev);
+            shutdown_tpmfront(files[fd].tpmfront.dev, 0);
            files[fd].type = FTYPE_NONE;
            return 0;
  #endif
diff --git a/extras/mini-os/tpmback.c b/extras/mini-os/tpmback.c
index 2d31061..ea42235 100644
--- a/extras/mini-os/tpmback.c
+++ b/extras/mini-os/tpmback.c
@@ -664,6 +664,7 @@ static int frontend_changed(tpmif_t* tpmif)
     switch (state) {
        case XenbusStateInitialising:
        case XenbusStateInitialised:
+      case XenbusStateReconfigured:
         break;
case XenbusStateConnected:
@@ -678,6 +679,10 @@ static int frontend_changed(tpmif_t* tpmif)
         tpmif_change_state(tpmif, XenbusStateClosing);
         break;
+ case XenbusStateReconfiguring:
+         disconnect_fe(tpmif);
+        break;
+
        case XenbusStateUnknown: /* keep it here */
        case XenbusStateClosed:
         free_tpmif(tpmif);
diff --git a/extras/mini-os/tpmfront.c b/extras/mini-os/tpmfront.c
index c1cbab3..b725ba0 100644
--- a/extras/mini-os/tpmfront.c
+++ b/extras/mini-os/tpmfront.c
@@ -344,10 +344,10 @@ struct tpmfront_dev* init_tpmfront(const char* _nodename)
     return dev;
error:
-   shutdown_tpmfront(dev);
+   shutdown_tpmfront(dev, 0);
     return NULL;
  }
-void shutdown_tpmfront(struct tpmfront_dev* dev)
+void shutdown_tpmfront(struct tpmfront_dev* dev, int for_reconfig)
It might be cleaner to create a new function like reconfigure_tpmfront() or something like that instead of adding an option to shutdown_tpmfront().
  {
     char* err;
     char path[512];
@@ -357,8 +357,7 @@ void shutdown_tpmfront(struct tpmfront_dev* dev)
     TPMFRONT_LOG("Shutting down tpmfront%s\n", for_reconfig ? " for reconfigure" : 
"");
     /* disconnect */
     if(dev->state == XenbusStateConnected) {
-      dev->state = XenbusStateClosing;
-      //FIXME: Transaction for this?
+      dev->state = for_reconfig ? XenbusStateReconfiguring : 
XenbusStateClosing;
        /* Tell backend we are closing */
        if((err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%u", (unsigned 
int) dev->state))) {
         free(err);
@@ -374,15 +373,19 @@ void shutdown_tpmfront(struct tpmfront_dev* dev)
         free(err);
        }
+ if (for_reconfig)
+         wait_for_backend_state_changed(dev, XenbusStateReconfigured);
+
        /* Tell backend we are closed */
-      dev->state = XenbusStateClosed;
+      dev->state = for_reconfig ? XenbusStateInitialising : XenbusStateClosed;
        if((err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%u", (unsigned 
int) dev->state))) {
         TPMFRONT_ERR("Unable to write to %s, error was %s", dev->nodename, 
err);
         free(err);
        }
/* Wait for the backend to close and unmap shared pages, ignore any errors */
-      wait_for_backend_state_changed(dev, XenbusStateClosed);
+      if (!for_reconfig)
+         wait_for_backend_state_changed(dev, XenbusStateClosed);
/* Close event channel and unmap shared page */
        mask_evtchn(dev->evtchn);
diff --git a/stubdom/vtpm/vtpm.c b/stubdom/vtpm/vtpm.c
index 71aef78..c33e078 100644
--- a/stubdom/vtpm/vtpm.c
+++ b/stubdom/vtpm/vtpm.c
@@ -394,7 +394,7 @@ abort_postvtpmblk:
  abort_postrng:
/* Close devices */
-   shutdown_tpmfront(tpmfront_dev);
+   shutdown_tpmfront(tpmfront_dev, 0);
  abort_posttpmfront:
     shutdown_tpmback();


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®.