From mboxrd@z Thu Jan 1 00:00:00 1970 From: christophe.ricard Date: Tue, 11 Aug 2015 23:44:01 +0200 Subject: [U-Boot] [PATCH 15/25] dm: tpm: Add a uclass for Trusted Platform Modules In-Reply-To: <1439304497-10081-16-git-send-email-sjg@chromium.org> References: <1439304497-10081-1-git-send-email-sjg@chromium.org> <1439304497-10081-16-git-send-email-sjg@chromium.org> Message-ID: <55CA6CA1.8040405@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, I think we are pretty inline for the uclass. Please find below some few remarks. On 11/08/2015 16:48, Simon Glass wrote: > Add a new uclass for TPMs which uses almost the same TIS (TPM Interface > Specification) as is currently implemented. Since init() is handled by the > normal driver model probe() method, we don't need to implement that. Also > rename the transfer method to xfer() which is a less clumbsy name. > > Once all drivers and users are converted to driver model we can remove the > old code. > > Signed-off-by: Simon Glass > --- > > drivers/tpm/Kconfig | 9 +++++ > drivers/tpm/Makefile | 2 + > drivers/tpm/tpm-uclass.c | 57 ++++++++++++++++++++++++++++ > include/dm/uclass-id.h | 1 + > include/tis.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 166 insertions(+) > create mode 100644 drivers/tpm/tpm-uclass.c > > diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig > index 993d2d7..800239e 100644 > --- a/drivers/tpm/Kconfig > +++ b/drivers/tpm/Kconfig > @@ -1,3 +1,12 @@ > +config DM_TPM > + bool "Enable driver model for Trusted Platform Module drivers" > + depends on DM && TPM > + help > + Enable driver model for TPMs. The TIS interface (tis_open(), > + tis_sendrecv(), etc.) is then implemented by the TPM uclass. Note > + that even with driver model only a single TPM is currently > + supported, since the tpm library assumes this. > + > config TPM_TIS_SANDBOX > bool "Enable sandbox TPM driver" > depends on SANDBOX > diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile > index 597966c..0d328f8 100644 > --- a/drivers/tpm/Makefile > +++ b/drivers/tpm/Makefile > @@ -3,6 +3,8 @@ > # SPDX-License-Identifier: GPL-2.0+ > # > > +obj-$(CONFIG_DM_TPM) += tpm-uclass.o > + > obj-$(CONFIG_TPM_ATMEL_TWI) += tpm_atmel_twi.o > obj-$(CONFIG_TPM_TIS_I2C) += tpm_tis_i2c.o > obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o > diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c > new file mode 100644 > index 0000000..ccade5b > --- /dev/null > +++ b/drivers/tpm/tpm-uclass.c > @@ -0,0 +1,57 @@ > +/* > + * Copyright (c) 2015 Google, Inc > + * Written by Simon Glass > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include > +#include > +#include > + > +int tis_open(struct udevice *dev) > +{ > + struct tpm_ops *ops = tpm_get_ops(dev); > + > + if (!ops->open) > + return -ENOSYS; > + > + return ops->open(dev); > +} > + > +int tis_close(struct udevice *dev) > +{ > + struct tpm_ops *ops = tpm_get_ops(dev); > + > + if (!ops->close) > + return -ENOSYS; > + > + return ops->close(dev); > +} > + > +int tis_get_desc(struct udevice *dev, char *buf, int size) > +{ > + struct tpm_ops *ops = tpm_get_ops(dev); > + > + if (!ops->get_desc) > + return -ENOSYS; > + > + return ops->get_desc(dev, buf, size); > +} > + > +int tis_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, > + uint8_t *recvbuf, size_t *recv_size) > +{ > + struct tpm_ops *ops = tpm_get_ops(dev); > + > + if (!ops->xfer) > + return -ENOSYS; > + > + return ops->xfer(dev, sendbuf, send_size, recvbuf, recv_size); > +} tis_xfer could be more generic and rely on tpm_transmit from original tpm.c. The command duration could be calculated at probe time during driver initialisation running one single getcapability command. > + > +UCLASS_DRIVER(tpm) = { > + .id = UCLASS_TPM, > + .name = "tpm", > + .flags = DM_UC_FLAG_SEQ_ALIAS, > +}; > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > index c744044..3eff895 100644 > --- a/include/dm/uclass-id.h > +++ b/include/dm/uclass-id.h > @@ -54,6 +54,7 @@ enum uclass_id { > UCLASS_SPI_GENERIC, /* Generic SPI flash target */ > UCLASS_SYSCON, /* System configuration device */ > UCLASS_THERMAL, /* Thermal sensor */ > + UCLASS_TPM, /* Trusted Platform Module TIS interface */ > UCLASS_USB, /* USB bus */ > UCLASS_USB_DEV_GENERIC, /* USB generic device */ > UCLASS_USB_HUB, /* USB hub */ > diff --git a/include/tis.h b/include/tis.h > index 40a1f86..6620554 100644 > --- a/include/tis.h > +++ b/include/tis.h > @@ -7,6 +7,102 @@ > #ifndef __TIS_H > #define __TIS_H > > +#ifdef CONFIG_DM_TPM > +struct tpm_ops { As per a previous comment, an init handler could be usefull. > + /** > + * open() - Request access to locality 0 for the caller > + * > + * After all commands have been completed the caller should call > + * tis_close(). > + * > + * @dev: Device to close > + * @return 0 ok OK, -ve on error > + */ > + int (*open)(struct udevice *dev); > + > + /** > + * tis_close() - Close the current session > + * > + * Releasing the locked locality. Returns 0 on success, -ve 1 on > + * failure (in case lock removal did not succeed). > + * > + * @dev: Device to close > + * @return 0 ok OK, -ve on error > + */ > + int (*close)(struct udevice *dev); > + > + /** > + * get_desc() - Get a text description of the TPM > + * > + * @dev: Device to check > + * @buf: Buffer to put the string > + * @size: Maximum size of buffer > + * @return length of string, or -ENOSPC it no space > + */ > + int (*get_desc)(struct udevice *dev, char *buf, int size); > + > + /** > + * xfer() - send data to the TPM and get response > + * > + * @dev: Device to talk to > + * @sendbuf: Buffer of the data to send > + * @send_size: Size of the data to send > + * @recvbuf: Buffer to save the response to > + * @recv_size: Pointer to the size of the response buffer > + * > + * Returns 0 on success (and places the number of response bytes at > + * recv_size) or -ve on failure. > + */ > + int (*xfer)(struct udevice *dev, const uint8_t *sendbuf, > + size_t send_size, uint8_t *recvbuf, size_t *recv_size); > +}; > + > +#define tpm_get_ops(dev) ((struct tpm_ops *)(dev)->driver->ops) why not device_get_ops(dev) ? > + > +/* > + * open() - Request access to locality 0 for the caller > + * > + * After all commands have been completed the caller is supposed to > + * call tis_close(). > + * > + * Returns 0 on success, -ve on failure. > + */ > +int tis_open(struct udevice *dev); > + > +/* > + * tis_close() - Close the current session > + * > + * Releasing the locked locality. Returns 0 on success, -ve 1 on > + * failure (in case lock removal did not succeed). > + */ > +int tis_close(struct udevice *dev); > + > +/** > + * tis_get_desc() - Get a text description of the TPM > + * > + * @dev: Device to check > + * @buf: Buffer to put the string > + * @size: Maximum size of buffer > + * @return length of string, or -ENOSPC it no space > + */ > +int tis_get_desc(struct udevice *dev, char *buf, int size); > + > +/* > + * tis_sendrecv() - send data to the TPM and get response > + * > + * @sendbuf - buffer of the data to send > + * @send_size size of the data to send > + * @recvbuf - memory to save the response to > + * @recv_len - pointer to the size of the response buffer > + * > + * Returns 0 on success (and places the number of response bytes at > + * recv_len) or -ve on failure. > + */ > +int tis_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, > + uint8_t *recvbuf, size_t *recv_size); > + As at the moment there is a 1 - 1 link with TPM and a platform, are you sure udevice should be a parameter ? > +#else > + > #include > > /* Low-level interface to access TPM */ > @@ -53,5 +149,6 @@ int tis_close(void); > */ > int tis_sendrecv(const uint8_t *sendbuf, size_t send_size, uint8_t *recvbuf, > size_t *recv_len); > +#endif > > #endif /* __TIS_H */ Best Regards Christophe