[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 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. > 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(); >> > > -- Daniel De Graaf National Security Agency _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |