|
[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 |