| Message ID | 20211103150910.69732-3-ilias.apalodimas@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | TPM cleanups and MMIO driver | expand |
Hi Ilias, On Wed, 3 Nov 2021 at 09:09, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Add support for devices that expose a TPMv2 though MMIO. > Apart from those devices, we can use the driver in our QEMU setups and > test TPM related code which is difficult to achieve using the sandbox > driver (e.g test the EFI TCG2 protocol). > > It's worth noting that a previous patch added TPMv2 TIS core functions, > which the current driver is consuming. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > drivers/tpm/Kconfig | 9 +++ > drivers/tpm/Makefile | 1 + > drivers/tpm/tpm2_tis_mmio.c | 152 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 162 insertions(+) > create mode 100644 drivers/tpm/tpm2_tis_mmio.c > > diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig > index 9eebab5cfd90..406ee8716e1e 100644 > --- a/drivers/tpm/Kconfig > +++ b/drivers/tpm/Kconfig > @@ -161,6 +161,15 @@ config TPM2_FTPM_TEE > help > This driver supports firmware TPM running in TEE. > > +config TPM2_MMIO > + bool "MMIO based TPM2 Interface" > + depends on TPM_V2 > + help > + This driver supports firmware TPM2.0 MMIO interface. > + The usual TPM operations and the 'tpm' command can be used to talk > + to the device using the standard TPM Interface Specification (TIS) > + protocol. > + > endif # TPM_V2 > > endmenu > diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile > index c65be5267002..494aa5a46d30 100644 > --- a/drivers/tpm/Makefile > +++ b/drivers/tpm/Makefile > @@ -14,3 +14,4 @@ obj-$(CONFIG_$(SPL_TPL_)TPM2_CR50_I2C) += cr50_i2c.o > obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o sandbox_common.o > obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o > obj-$(CONFIG_TPM2_FTPM_TEE) += tpm2_ftpm_tee.o > +obj-$(CONFIG_TPM2_MMIO) += tpm2_tis_core.o tpm2_tis_mmio.o > diff --git a/drivers/tpm/tpm2_tis_mmio.c b/drivers/tpm/tpm2_tis_mmio.c > new file mode 100644 > index 000000000000..3bd0b0871a83 > --- /dev/null > +++ b/drivers/tpm/tpm2_tis_mmio.c > @@ -0,0 +1,152 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * driver for mmio TCG/TIS TPM (trusted platform module). > + * > + * Specifications at www.trustedcomputinggroup.org > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <log.h> > +#include <tpm-v2.h> > +#include <linux/bitops.h> > +#include <linux/compiler.h> > +#include <linux/delay.h> > +#include <linux/errno.h> > +#include <linux/types.h> > +#include <linux/io.h> > +#include <linux/unaligned/be_byteshift.h> > +#include "tpm_tis.h" > +#include "tpm_internal.h" > + > +/** > + * struct tpm_tis_chip_data - Information about an MMIO TPM > + * @pcr_count: Number of PCR per bank > + * @pcr_select_min: Minimum size in bytes of the pcrSelect array > + * @iobase: Base address > + */ > +struct tpm_tis_chip_data { > + unsigned int pcr_count; > + unsigned int pcr_select_min; > + void __iomem *iobase; > +}; > + > +static int mmio_read_bytes(struct udevice *udev, u32 addr, u16 len, dev again > + u8 *result) > +{ > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); > + > + while (len--) > + *result++ = ioread8(drv_data->iobase + addr); > + return 0; > +} > + > +static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len, > + const u8 *value) > +{ > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); > + > + while (len--) > + iowrite8(*value++, drv_data->iobase + addr); So should this use regmap? > + return 0; > +} [..] Regards, Simon
Hi Simon, [...] > > + u8 *result) > > +{ > > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); > > + > > + while (len--) > > + *result++ = ioread8(drv_data->iobase + addr); > > + return 0; > > +} > > + > > +static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len, > > + const u8 *value) > > +{ > > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); > > + > > + while (len--) > > + iowrite8(*value++, drv_data->iobase + addr); > > So should this use regmap? > Isn't the point of regmap abstracting the bus access itself? Something along the lines of ******** ********** *********** * SPI ** --> * * --> * SPI DM ** --> Device ******** * * *********** * REGMAP * ******** * * * MMIO * --> * * --> ************** ******** ********** * MMIO access* --> Device ************** Right now we have discrete drivers for the SPI and MMIO TPMs. However using it makes sense if we want to merge parts of the SPI, MMIO and I2C drivers in the future. That though is not what this patchset deals with. Let's first clean up the crud of the TIS APIs duplication we've been carrying over various TPM drivers and worry about consolidating the bus accesses later. Thanks /Ilias > > + return 0; > > +} > [..] > > Regards, > Simon
On Fri, Nov 05, 2021 at 10:17:21AM +0200, Ilias Apalodimas wrote: > Hi Simon, > > [...] > > > > + u8 *result) > > > +{ > > > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); > > > + > > > + while (len--) > > > + *result++ = ioread8(drv_data->iobase + addr); > > > + return 0; > > > +} > > > + > > > +static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len, > > > + const u8 *value) > > > +{ > > > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); > > > + > > > + while (len--) > > > + iowrite8(*value++, drv_data->iobase + addr); > > > > So should this use regmap? > > > > Isn't the point of regmap abstracting the bus access itself? Something > along the lines of > > ******** ********** *********** > * SPI ** --> * * --> * SPI DM ** --> Device > ******** * * *********** > * REGMAP * > ******** * * > * MMIO * --> * * --> ************** > ******** ********** * MMIO access* --> Device > ************** > Hopefully I'll get the ASCII right this time... ******** ********** *********** * SPI ** --> * * --> * SPI DM ** --> Device ******** * * *********** * REGMAP * ******** * * * MMIO * --> * * --> ************** ******** ********** * MMIO access* --> Device ************** > Right now we have discrete drivers for the SPI and MMIO TPMs. > However using it makes sense if we want to merge parts of the SPI, MMIO and I2C > drivers in the future. That though is not what this patchset deals with. > Let's first clean up the crud of the TIS APIs duplication we've been > carrying over various TPM drivers and worry about consolidating the bus > accesses later. > > Thanks > /Ilias > > > > + return 0; > > > +} > > [..] > > > > Regards, > > Simon
Hi Ilias, On Fri, 5 Nov 2021 at 02:23, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > On Fri, Nov 05, 2021 at 10:17:21AM +0200, Ilias Apalodimas wrote: > > Hi Simon, > > > > [...] > > > > > > + u8 *result) > > > > +{ > > > > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); > > > > + > > > > + while (len--) > > > > + *result++ = ioread8(drv_data->iobase + addr); > > > > + return 0; > > > > +} > > > > + > > > > +static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len, > > > > + const u8 *value) > > > > +{ > > > > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); > > > > + > > > > + while (len--) > > > > + iowrite8(*value++, drv_data->iobase + addr); > > > > > > So should this use regmap? > > > > > > > Isn't the point of regmap abstracting the bus access itself? Something > > along the lines of It is for MMIO at present, but I suppose it could handle the bus. It would need to know about register numbers though, and we've never really figured out if it is a win or not. > > > > ******** ********** *********** > > * SPI ** --> * * --> * SPI DM ** --> Device > > ******** * * *********** > > * REGMAP * > > ******** * * > > * MMIO * --> * * --> ************** > > ******** ********** * MMIO access* --> Device > > ************** > > > > Hopefully I'll get the ASCII right this time... > > ******** ********** *********** > * SPI ** --> * * --> * SPI DM ** --> Device > ******** * * *********** > * REGMAP * > ******** * * > * MMIO * --> * * --> ************** > ******** ********** * MMIO access* --> Device > ************** > > Right now we have discrete drivers for the SPI and MMIO TPMs. > > However using it makes sense if we want to merge parts of the SPI, MMIO and I2C > > drivers in the future. That though is not what this patchset deals with. > > Let's first clean up the crud of the TIS APIs duplication we've been > > carrying over various TPM drivers and worry about consolidating the bus > > accesses later. That's OK with me. Regards, Simon
diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig index 9eebab5cfd90..406ee8716e1e 100644 --- a/drivers/tpm/Kconfig +++ b/drivers/tpm/Kconfig @@ -161,6 +161,15 @@ config TPM2_FTPM_TEE help This driver supports firmware TPM running in TEE. +config TPM2_MMIO + bool "MMIO based TPM2 Interface" + depends on TPM_V2 + help + This driver supports firmware TPM2.0 MMIO interface. + The usual TPM operations and the 'tpm' command can be used to talk + to the device using the standard TPM Interface Specification (TIS) + protocol. + endif # TPM_V2 endmenu diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile index c65be5267002..494aa5a46d30 100644 --- a/drivers/tpm/Makefile +++ b/drivers/tpm/Makefile @@ -14,3 +14,4 @@ obj-$(CONFIG_$(SPL_TPL_)TPM2_CR50_I2C) += cr50_i2c.o obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o sandbox_common.o obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o obj-$(CONFIG_TPM2_FTPM_TEE) += tpm2_ftpm_tee.o +obj-$(CONFIG_TPM2_MMIO) += tpm2_tis_core.o tpm2_tis_mmio.o diff --git a/drivers/tpm/tpm2_tis_mmio.c b/drivers/tpm/tpm2_tis_mmio.c new file mode 100644 index 000000000000..3bd0b0871a83 --- /dev/null +++ b/drivers/tpm/tpm2_tis_mmio.c @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * driver for mmio TCG/TIS TPM (trusted platform module). + * + * Specifications at www.trustedcomputinggroup.org + */ + +#include <common.h> +#include <dm.h> +#include <log.h> +#include <tpm-v2.h> +#include <linux/bitops.h> +#include <linux/compiler.h> +#include <linux/delay.h> +#include <linux/errno.h> +#include <linux/types.h> +#include <linux/io.h> +#include <linux/unaligned/be_byteshift.h> +#include "tpm_tis.h" +#include "tpm_internal.h" + +/** + * struct tpm_tis_chip_data - Information about an MMIO TPM + * @pcr_count: Number of PCR per bank + * @pcr_select_min: Minimum size in bytes of the pcrSelect array + * @iobase: Base address + */ +struct tpm_tis_chip_data { + unsigned int pcr_count; + unsigned int pcr_select_min; + void __iomem *iobase; +}; + +static int mmio_read_bytes(struct udevice *udev, u32 addr, u16 len, + u8 *result) +{ + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); + + while (len--) + *result++ = ioread8(drv_data->iobase + addr); + return 0; +} + +static int mmio_write_bytes(struct udevice *udev, u32 addr, u16 len, + const u8 *value) +{ + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); + + while (len--) + iowrite8(*value++, drv_data->iobase + addr); + return 0; +} + +static int mmio_read32(struct udevice *udev, u32 addr, u32 *result) +{ + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); + + *result = ioread32(drv_data->iobase + addr); + return 0; +} + +static int mmio_write32(struct udevice *udev, u32 addr, u32 value) +{ + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); + + iowrite32(value, drv_data->iobase + addr); + return 0; +} + +static struct tpm_tis_phy_ops phy_ops = { + .read_bytes = mmio_read_bytes, + .write_bytes = mmio_write_bytes, + .read32 = mmio_read32, + .write32 = mmio_write32, +}; + +static int tpm_tis_probe(struct udevice *udev) +{ + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); + struct tpm_chip_priv *priv = dev_get_uclass_priv(udev); + int ret = 0; + fdt_addr_t ioaddr; + u64 sz; + + ioaddr = dev_read_addr(udev); + if (ioaddr == FDT_ADDR_T_NONE) + return log_msg_ret("ioaddr", -EINVAL); + + ret = dev_read_u64(udev, "reg", &sz); + if (ret) + return -EINVAL; + + drv_data->iobase = ioremap(ioaddr, sz); + log_debug("Remapped TPM2 base: 0x%llx size: 0x%llx\n", ioaddr, sz); + tpm_tis_ops_register(udev, &phy_ops); + ret = tpm_tis_init(udev); + if (ret) + goto iounmap; + + priv->pcr_count = drv_data->pcr_count; + priv->pcr_select_min = drv_data->pcr_select_min; + /* + * Although the driver probably works with a TPMv1 our Kconfig + * limits the driver to TPMv2 only + */ + priv->version = TPM_V2; + + return ret; +iounmap: + iounmap(drv_data->iobase); + return -EINVAL; +} + +static int tpm_tis_remove(struct udevice *udev) +{ + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev); + + iounmap(drv_data->iobase); + return tpm_tis_cleanup(udev); +} + +static const struct tpm_ops tpm_tis_ops = { + .open = tpm_tis_open, + .close = tpm_tis_close, + .get_desc = tpm_tis_get_desc, + .send = tpm_tis_send, + .recv = tpm_tis_recv, + .cleanup = tpm_tis_cleanup, +}; + +static const struct tpm_tis_chip_data tpm_tis_std_chip_data = { + .pcr_count = 24, + .pcr_select_min = 3, +}; + +static const struct udevice_id tpm_tis_ids[] = { + { + .compatible = "tcg,tpm-tis-mmio", + .data = (ulong)&tpm_tis_std_chip_data, + }, + { } +}; + +U_BOOT_DRIVER(tpm_tis_mmio) = { + .name = "tpm_tis_mmio", + .id = UCLASS_TPM, + .of_match = tpm_tis_ids, + .ops = &tpm_tis_ops, + .probe = tpm_tis_probe, + .remove = tpm_tis_remove, + .priv_auto = sizeof(struct tpm_chip), +};
Add support for devices that expose a TPMv2 though MMIO. Apart from those devices, we can use the driver in our QEMU setups and test TPM related code which is difficult to achieve using the sandbox driver (e.g test the EFI TCG2 protocol). It's worth noting that a previous patch added TPMv2 TIS core functions, which the current driver is consuming. Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- drivers/tpm/Kconfig | 9 +++ drivers/tpm/Makefile | 1 + drivers/tpm/tpm2_tis_mmio.c | 152 ++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+) create mode 100644 drivers/tpm/tpm2_tis_mmio.c -- 2.33.1