| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d356a20f-629c-a534-7a80-b96d0940056d@molgen.mpg.de>
Date: Mon, 6 Feb 2023 12:02:56 +0100
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Krishna Yarlagadda <kyarlagadda@...dia.com>
Cc: robh+dt@...nel.org, broonie@...nel.org, peterhuewe@....de,
jgg@...pe.ca, jarkko@...nel.org, krzysztof.kozlowski+dt@...aro.org,
linux-spi@...r.kernel.org, linux-tegra@...r.kernel.org,
linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org,
thierry.reding@...il.com, jonathanh@...dia.com,
skomatineni@...dia.com, ldewangan@...dia.com
Subject: Re: [Patch V2 2/4] tpm: tegra: Support SPI tpm wait state detect
Dear Krishna,
Thank you for your patch.
Am 03.02.23 um 14:01 schrieb Krishna Yarlagadda:
> Tegra234 and Tegra241 chips have QSPI controller that supports TCG
> TIS hardware flow control. Since the controller only supports half
> duplex, sw wait polling method implemented in tpm_tis_spi does not
> suffice. Added extending driver to disable sw flow control and send
I’d use imperative mood and maybe use another verb:
Add dedicated Tegra driver …
> all transfers in single message.
Please add how you tested and benchmarked this patch.
> Signed-off-by: Krishna Yarlagadda <kyarlagadda@...dia.com>
> ---
> drivers/char/tpm/Makefile | 1 +
> drivers/char/tpm/tpm_tis_spi.h | 1 +
> drivers/char/tpm/tpm_tis_spi_main.c | 4 +-
> drivers/char/tpm/tpm_tis_spi_tegra.c | 123 +++++++++++++++++++++++++++
> 4 files changed, 128 insertions(+), 1 deletion(-)
> create mode 100644 drivers/char/tpm/tpm_tis_spi_tegra.c
>
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 0222b1ddb310..445b15493cb3 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_TCG_TIS_SYNQUACER) += tpm_tis_synquacer.o
>
> obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
> tpm_tis_spi-y := tpm_tis_spi_main.o
> +tpm_tis_spi-y += tpm_tis_spi_tegra.o
> tpm_tis_spi-$(CONFIG_TCG_TIS_SPI_CR50) += tpm_tis_spi_cr50.o
>
> obj-$(CONFIG_TCG_TIS_I2C_CR50) += tpm_tis_i2c_cr50.o
> diff --git a/drivers/char/tpm/tpm_tis_spi.h b/drivers/char/tpm/tpm_tis_spi.h
> index d0f66f6f1931..feaea14b428b 100644
> --- a/drivers/char/tpm/tpm_tis_spi.h
> +++ b/drivers/char/tpm/tpm_tis_spi.h
> @@ -31,6 +31,7 @@ extern int tpm_tis_spi_init(struct spi_device *spi, struct tpm_tis_spi_phy *phy,
> extern int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
> u8 *in, const u8 *out);
>
> +extern int tegra_tpm_spi_probe(struct spi_device *spi);
> #ifdef CONFIG_TCG_TIS_SPI_CR50
> extern int cr50_spi_probe(struct spi_device *spi);
> #else
> diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
> index a0963a3e92bd..5d4502a4461a 100644
> --- a/drivers/char/tpm/tpm_tis_spi_main.c
> +++ b/drivers/char/tpm/tpm_tis_spi_main.c
> @@ -198,7 +198,7 @@ static int tpm_tis_spi_driver_probe(struct spi_device *spi)
> const struct spi_device_id *spi_dev_id = spi_get_device_id(spi);
> tpm_tis_spi_probe_func probe_func;
>
> - probe_func = of_device_get_match_data(&spi->dev);
> + probe_func = device_get_match_data(&spi->dev);
> if (!probe_func) {
> if (spi_dev_id) {
> probe_func = (tpm_tis_spi_probe_func)spi_dev_id->driver_data;
> @@ -227,6 +227,7 @@ static const struct spi_device_id tpm_tis_spi_id[] = {
> { "tpm_tis_spi", (unsigned long)tpm_tis_spi_probe },
> { "tpm_tis-spi", (unsigned long)tpm_tis_spi_probe },
> { "cr50", (unsigned long)cr50_spi_probe },
> + { "tegra-tpm-spi", (unsigned long)tegra_tpm_spi_probe },
> {}
> };
> MODULE_DEVICE_TABLE(spi, tpm_tis_spi_id);
> @@ -236,6 +237,7 @@ static const struct of_device_id of_tis_spi_match[] = {
> { .compatible = "infineon,slb9670", .data = tpm_tis_spi_probe },
> { .compatible = "tcg,tpm_tis-spi", .data = tpm_tis_spi_probe },
> { .compatible = "google,cr50", .data = cr50_spi_probe },
> + { .compatible = "nvidia,tegra-tpm-spi", .data = tegra_tpm_spi_probe },
> {}
> };
> MODULE_DEVICE_TABLE(of, of_tis_spi_match);
> diff --git a/drivers/char/tpm/tpm_tis_spi_tegra.c b/drivers/char/tpm/tpm_tis_spi_tegra.c
> new file mode 100644
> index 000000000000..23f20684513d
> --- /dev/null
> +++ b/drivers/char/tpm/tpm_tis_spi_tegra.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 NVIDIA CORPORATION.
> + *
> + * This device driver implements TEGRA QSPI hw wait detection for chips
> + *
> + * It is based on tpm_tis_spi driver by Peter Huewe and Christophe Ricard.
> + */
> +
> +#include <linux/completion.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm.h>
> +#include <linux/spi/spi.h>
> +#include <linux/wait.h>
> +
> +#include "tpm_tis_core.h"
> +#include "tpm_tis_spi.h"
> +
> +#define MAX_SPI_FRAMESIZE 64
> +
> +int tpm_tis_spi_tegra_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
> + u8 *in, const u8 *out)
> +{
> + struct tpm_tis_spi_phy *phy = to_tpm_tis_spi_phy(data);
> + int ret = 0;
> + struct spi_message m;
> + struct spi_transfer spi_xfer[3];
> + u8 transfer_len;
Just use `unsigned int`? [1]
> +
> + spi_bus_lock(phy->spi_device->master);
> +
> + while (len) {
> + transfer_len = min_t(u16, len, MAX_SPI_FRAMESIZE);
> +
> + spi_message_init(&m);
> + phy->iobuf[0] = (in ? 0x80 : 0) | (transfer_len - 1);
> + phy->iobuf[1] = 0xd4;
> + phy->iobuf[2] = addr >> 8;
> + phy->iobuf[3] = addr;
> +
> + memset(&spi_xfer, 0, sizeof(spi_xfer));
> +
> + spi_xfer[0].tx_buf = phy->iobuf;
> + spi_xfer[0].len = 1;
> + spi_message_add_tail(&spi_xfer[0], &m);
> +
> + spi_xfer[1].tx_buf = phy->iobuf + 1;
> + spi_xfer[1].len = 3;
> + spi_message_add_tail(&spi_xfer[1], &m);
> +
> + if (out) {
> + spi_xfer[2].tx_buf = &phy->iobuf[4];
> + spi_xfer[2].rx_buf = NULL;
> + memcpy(&phy->iobuf[4], out, transfer_len);
> + out += transfer_len;
> + }
> + if (in) {
> + spi_xfer[2].tx_buf = NULL;
> + spi_xfer[2].rx_buf = &phy->iobuf[4];
> + }
> + spi_xfer[2].len = transfer_len;
> + spi_message_add_tail(&spi_xfer[2], &m);
> +
> + reinit_completion(&phy->ready);
> + ret = spi_sync_locked(phy->spi_device, &m);
> + if (ret < 0)
> + goto exit;
> +
> + if (in) {
> + memcpy(in, &phy->iobuf[4], transfer_len);
> + in += transfer_len;
> + }
> +
> + len -= transfer_len;
> + }
> +
> +exit:
> + spi_bus_unlock(phy->spi_device->master);
> + return ret;
> +}
> +
> +static int tpm_tis_spi_tegra_read_bytes(struct tpm_tis_data *data, u32 addr,
> + u16 len, u8 *result,
> + enum tpm_tis_io_mode io_mode)
> +{
> + return tpm_tis_spi_tegra_transfer(data, addr, len, result, NULL);
> +}
> +
> +static int tpm_tis_spi_tegra_write_bytes(struct tpm_tis_data *data, u32 addr,
> + u16 len, const u8 *value,
> + enum tpm_tis_io_mode io_mode)
> +{
> + return tpm_tis_spi_tegra_transfer(data, addr, len, NULL, value);
> +}
> +
> +static const struct tpm_tis_phy_ops tegra_tpm_spi_phy_ops = {
> + .read_bytes = tpm_tis_spi_tegra_read_bytes,
> + .write_bytes = tpm_tis_spi_tegra_write_bytes,
> +};
> +
> +int tegra_tpm_spi_probe(struct spi_device *dev)
> +{
> + struct tpm_tis_spi_phy *phy;
> + int irq;
> +
> + phy = devm_kzalloc(&dev->dev, sizeof(struct tpm_tis_spi_phy),
> + GFP_KERNEL);
> + if (!phy)
> + return -ENOMEM;
> +
> + phy->flow_control = NULL;
> +
> + /* If the SPI device has an IRQ then use that */
> + if (dev->irq > 0)
> + irq = dev->irq;
> + else
> + irq = -1;
Use ternary operator?
irq = dev->irq > 0 ? dev->irq : -1;
> +
> + init_completion(&phy->ready);
> + return tpm_tis_spi_init(dev, phy, irq, &tegra_tpm_spi_phy_ops);
> +}
Kind regards,
Paul
[1]: https://notabs.org/coding/smallIntsBigPenalty.htm
Powered by blists - more mailing lists