[PATCH 1/1] tpm2: Add a TPMv2 MMIO TIS driver

Ilias Apalodimas ilias.apalodimas at linaro.org
Fri Jul 2 22:29:19 CEST 2021


On Fri, Jul 02, 2021 at 07:54:13PM +0200, Heinrich Schuchardt wrote:
> On 7/2/21 2:52 PM, Ilias Apalodimas wrote:
> > Add a TPMv2 TIS MMIO compatible driver.
> > This useful for a couple of reasons.  First of all we can support a TPMv2
> > on devices that have this hardware (e.g the newly added SynQuacer box).
> > We can also use the driver in our QEMU setups and test the EFI TCG2
> > protocol, which cannot be tested with our sandbox.
> > 
> > Ideally we should create an abstraction for all TIS compatible TPMs we
> > support. Let's plug in the mmio driver first.
> 
> This would match Linux' linux/drivers/char/tpm/tpm_tis_core.c. The
> individual drivers provide the TIS level operations of struct
> tpm_tis_phy_ops and call tpm_tis_core_init() to load the tpm_tis_core
> module which supplies the TPM level operations of struct tpm_class_ops.
> 
> Why don't you start with a separate tis_core module supporting the
> tis_mmio driver? This would avoid pulling out the core functions out of
> this driver and provide a template for refactoring the other drivers.
> 

Yep, I am commenting more on this below.
[...]

> > +/*
> > + * swtpm driver for TCG/TIS TPM (trusted platform module).
> 
> swtpm is not easy to understand. I would prefer something like
> 
> 'Driver for the TPM software emulation provided by the swtpm package
> (https://github.com/stefanberger/swtpm)'.

Sure

> 
> > + * 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"
> > +
> > +enum tis_int_flags {
> > +	TPM_GLOBAL_INT_ENABLE = 0x80000000,
> > +	TPM_INTF_BURST_COUNT_STATIC = 0x100,
> > +	TPM_INTF_CMD_READY_INT = 0x080,
> > +	TPM_INTF_INT_EDGE_FALLING = 0x040,
> > +	TPM_INTF_INT_EDGE_RISING = 0x020,
> > +	TPM_INTF_INT_LEVEL_LOW = 0x010,
> > +	TPM_INTF_INT_LEVEL_HIGH = 0x008,
> > +	TPM_INTF_LOCALITY_CHANGE_INT = 0x004,
> > +	TPM_INTF_STS_VALID_INT = 0x002,
> > +	TPM_INTF_DATA_AVAIL_INT = 0x001,
> > +};
> 
> These definitions match Linux' tpm_tis_core.h.
> 
> Should they be moved to an include of the same name in U-Boot?

I got a tpm_tis.h in this series, so I might as well move those there

> 

[...]

> I would prefer if you would add these functions to a structure struct
> tpm_tis_phy_ops which you can use to pull out the core driver.

Me too :).  In fact that matches the design I proposed over IRC.  I just
didn't have too much free time to fix it, so I went ahead and posted the
driver matching what's already in U-Boot.  That being said, what's proposed
here is the right was forward.  Basically if we do it like this then any
future TPM TIS driver will only have to define the read/write ops for the
underlyng bus.

> 
> Best regards
> 
> Heinrich
> 
> > +
> > +static int tpm_tis_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 __maybe_unused int tpm_tis_read16(struct udevice *udev, u32 addr,
> > +					 u16 *result)
> > +{
> > +	struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> > +
> > +	*result = ioread16(drv_data->iobase + addr);
> > +	return 0;
> > +}
> > +
> > +static int tpm_tis_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 tpm_tis_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 int tpm_tis_get_desc(struct udevice *udev, char *buf, int size)
> > +{
> > +	struct tpm_chip *chip = dev_get_priv(udev);
> > +
> > +	if (size < 80)
> > +		return -ENOSPC;
> > +
> > +	return snprintf(buf, size,
> > +			"%s v2.0: VendorID 0x%04x, DeviceID 0x%04x, RevisionID 0x%02x [%s]",
> > +			udev->name, chip->vend_dev & 0xFFFF,
> > +			chip->vend_dev >> 16, chip->rid,
> > +			(chip->is_open ? "open" : "closed"));
> > +}
> > +
> > +static bool tpm_tis_check_locality(struct udevice *udev, int loc)
> > +{
> > +	struct tpm_chip *chip = dev_get_priv(udev);
> > +	u8 locality;
> > +
> > +	tpm_tis_read_bytes(udev, TPM_ACCESS(loc), 1, &locality);
> > +	if ((locality & (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID |
> > +			 TPM_ACCESS_REQUEST_USE)) ==
> > +			 (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) {
> > +		chip->locality = loc;
> > +		return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +static int tpm_tis_request_locality(struct udevice *udev, int loc)
> > +{
> > +	struct tpm_chip *chip = dev_get_priv(udev);
> > +	u8 buf = TPM_ACCESS_REQUEST_USE;
> > +	unsigned long start, stop;
> > +
> > +	if (tpm_tis_check_locality(udev, loc))
> > +		return 0;
> > +
> > +	tpm_tis_write_bytes(udev, TPM_ACCESS(loc), 1, &buf);
> > +	start = get_timer(0);
> > +	stop = chip->timeout_a;
> > +	do {
> > +		if (tpm_tis_check_locality(udev, loc))
> > +			return 0;
> > +		mdelay(TPM_TIMEOUT_MS);
> > +	} while (get_timer(start) < stop);
> > +
> > +	return -1;
> > +}
> > +
> > +static int tpm_tis_status(struct udevice *udev, u8 *status)
> > +{
> > +	struct tpm_chip *chip = dev_get_priv(udev);
> > +
> > +	if (chip->locality < 0)
> > +		return -EINVAL;
> > +
> > +	tpm_tis_read_bytes(udev, TPM_STS(chip->locality), 1, status);
> > +
> > +	if ((*status & TPM_STS_READ_ZERO)) {
> > +		log_err("TPM returned invalid status\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int tpm_tis_release_locality(struct udevice *udev, int loc)
> > +{
> > +	struct tpm_chip *chip = dev_get_priv(udev);
> > +	u8 buf = TPM_ACCESS_ACTIVE_LOCALITY;
> > +	int ret;
> > +
> > +	if (chip->locality < 0)
> > +		return 0;
> > +
> > +	ret = tpm_tis_write_bytes(udev, TPM_ACCESS(loc), 1, &buf);
> > +	chip->locality = -1;
> > +
> > +	return ret;
> > +}
> > +
> > +static int tpm_tis_wait_for_stat(struct udevice *udev, u8 mask,
> > +				 unsigned long timeout, u8 *status)
> > +{
> > +	unsigned long start = get_timer(0);
> > +	unsigned long stop = timeout;


More information about the U-Boot mailing list