Re: [Xen-devel] [PATCH v4] drivers/tpm: add xen tpmfront interface

[David Vrabel <david.vrabel@xxxxxxxxxx>]

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

> --- /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).

> +
> +     switch (backend_state) {
> +     case XenbusStateInitialised:
> +     case XenbusStateConnected:

                if (dev->state == XenbusStateConnected)
                        break;

Perhaps?

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel