[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 2/4] stubdom/vtpm: Add reconfiguration support
On 11/29/2012 02:28 PM, Daniel De Graaf wrote:
On 11/29/2012 01:53 PM, Matthew Fioravante wrote:
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?
The vtpm backend in mini-os cleans up after itself destructively when it is
closed: it removes xenstore entries and frees the data structures, and so
requires the xenstore directory to be re-created. I originally wanted to keep
the shutdown semantics the same (to preserve the vtpm auto-shutdown), although
that's not reflected in this patch queue.
I think the correct method here is to make the backend act like it does for the
reconfigure on close, and trigger free only when the xenstore entry vanishes.
A cursory look at blkback in linux and it looks like the backend driver
doesn't tear down the xenstore entries at all. I think its supposed to
be the job of libxl to do that no? So I think I agree with you that what
we really want is to change the backend to leave xenstore alone and
maintain enough state so that the device can reconnect later.
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
|
