From: Daniel De G. <dg...@ty...> - 2013-07-22 16:21:12
|
On 07/22/2013 11:18 AM, David Vrabel wrote: > On 01/07/13 22:34, Daniel De Graaf wrote: >> This is a complete rewrite of the Xen TPM frontend driver, taking >> advantage of a simplified frontend/backend interface and adding support >> for cancellation and timeouts. The backend for this driver is provided >> by a vTPM stub domain using the interface in Xen 4.3. > [...] >> --- /dev/null >> +++ b/Documentation/xen-tpmfront.txt > > Suggest putting this in Documentation/tpm/. OK. >> --- /dev/null >> +++ b/drivers/char/tpm/xen-tpmfront.c > [...] >> +static void backend_changed(struct xenbus_device *dev, >> + enum xenbus_state backend_state) >> +{ >> + int val; > > Hrm. I don't like how every front/back pair invents their own variation > of the state machine. > > Please document the front and back state machines in > xen/include/public/io/tpmif.h (and the correspoding copy in Linux). Is there a standard state machine that would allow devices to avoid inventing their own? Otherwise, this is what I plan to add to the header: /* * Xenbus state machine * * Device open: * 1. Both ends start in XenbusStateInitialising * 2. Backend transitions to InitWait (frontend does not wait on this step) * 3. Frontend populates ring-ref, event-channel, feature-protocol-v2 * 4. Frontend transitions to Initialised * 5. Backend maps grant and event channel, verifies feature-protocol-v2 * 6. Backend transitions to Connected * 7. Frontend verifies feature-protocol-v2, transitions to Connected * * Device close: * 1. State is changed to XenbusStateClosing * 2. Frontend transitions to Closed * 3. Backend unmaps grant and event, changes state to InitWait */ >> + >> + switch (backend_state) { >> + case XenbusStateInitialised: >> + case XenbusStateConnected: > > if (dev->state == XenbusStateConnected) > break; > > Perhaps? Sure, although the spurious invocation is not seen with the mini-os backend and running this code twice is harmless. >> + if (xenbus_scanf(XBT_NIL, dev->otherend, >> + "feature-protocol-v2", "%d", &val) < 0) >> + val = 0; >> + if (!val) { >> + xenbus_dev_fatal(dev, -EINVAL, >> + "vTPM protocol 2 required"); >> + return; >> + } >> + xenbus_switch_state(dev, XenbusStateConnected); >> + break; >> + >> + case XenbusStateClosing: >> + case XenbusStateClosed: >> + device_unregister(&dev->dev); >> + xenbus_frontend_closed(dev); >> + break; >> + default: >> + break; >> + } >> +} > > David -- Daniel De Graaf National Security Agency |