LinuxLists.cc - [Patch V7 0/3] Tegra TPM driver with HW flow control

2023-03-01 17:34:18

Subject: [Patch V7 0/3] Tegra TPM driver with HW flow control

2023-03-01 17:34:26

Subject: [Patch V7 1/3] spi: Add TPM HW flow flag

TPM specification [1] defines flow control over SPI. Client device can
insert a wait state on MISO when address is transmitted by controller
on MOSI. Detecting the wait state in software is only possible for
full duplex controllers. For controllers that support only half-
duplex, the wait state detection needs to be implemented in hardware.

Add a flag SPI_TPM_HW_FLOW for TPM device to set when software flow
control is not possible and hardware flow control is expected from
SPI controller.

Reference:
[1] https://trustedcomputinggroup.org/resource/pc-client-work-group-
pc-client-specific-tpm-interface-specification-tis/

Signed-off-by: Krishna Yarlagadda <[email protected]>
---
include/linux/spi/spi.h | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 4fa26b9a3572..b9e49ed42955 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -184,8 +184,18 @@ struct spi_device {
u8 chip_select;
u8 bits_per_word;
bool rt;
-#define SPI_NO_TX BIT(31) /* No transmit wire */
-#define SPI_NO_RX BIT(30) /* No receive wire */
+#define SPI_NO_TX BIT(31) /* No transmit wire */
+#define SPI_NO_RX BIT(30) /* No receive wire */
+ /*
+ * TPM specification defines flow control over SPI. Client device
+ * can insert a wait state on MISO when address is transmitted by
+ * controller on MOSI. Detecting the wait state in software is only
+ * possible for full duplex controllers. For controllers that support
+ * only half-duplex, the wait state detection needs to be implemented
+ * in hardware. TPM devices would set this flag when hardware flow
+ * control is expected from SPI controller.
+ */
+#define SPI_TPM_HW_FLOW BIT(29) /* TPM HW flow control */
/*
* All bits defined above should be covered by SPI_MODE_KERNEL_MASK.
* The SPI_MODE_KERNEL_MASK has the SPI_MODE_USER_MASK counterpart,
@@ -195,7 +205,7 @@ struct spi_device {
* These bits must not overlap. A static assert check should make sure of that.
* If adding extra bits, make sure to decrease the bit index below as well.
*/
-#define SPI_MODE_KERNEL_MASK (~(BIT(30) - 1))
+#define SPI_MODE_KERNEL_MASK (~(BIT(29) - 1))
u32 mode;
int irq;
void *controller_state;
--
2.17.1

2023-03-01 17:34:38

by Krishna Yarlagadda

[permalink] [raw]

Subject: [Patch V7 2/3] tpm_tis-spi: Add hardware wait polling

TPM devices may insert wait state on last clock cycle of ADDR phase.
For SPI controllers that support full-duplex transfers, this can be
detected using software by reading the MISO line. For SPI controllers
that only support half-duplex transfers, such as the Tegra QSPI, it is
not possible to detect the wait signal from software. The QSPI
controller in Tegra234 and Tegra241 implement hardware detection of the
wait signal which can be enabled in the controller for TPM devices.

The current TPM TIS driver only supports software detection of the wait
signal. To support SPI controllers that use hardware to detect the wait
signal, add the function tpm_tis_spi_hw_flow_transfer() and move the
existing code for software based detection into a function called
tpm_tis_spi_sw_flow_transfer(). SPI controllers that only support
half-duplex transfers will always call tpm_tis_spi_hw_flow_transfer()
because they cannot support software based detection. The bit
SPI_TPM_HW_FLOW is set to indicate to the SPI controller that hardware
detection is required and it is the responsibility of the SPI controller
driver to determine if this is supported or not.

For hardware flow control, CMD-ADDR-DATA messages are combined into a
single message where as for software flow control exiting method of
CMD-ADDR in a message and DATA in another is followed.

Signed-off-by: Krishna Yarlagadda <[email protected]>
---
drivers/char/tpm/tpm_tis_spi_main.c | 96 ++++++++++++++++++++++++++++-
1 file changed, 94 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_spi_main.c b/drivers/char/tpm/tpm_tis_spi_main.c
index a0963a3e92bd..fc6ee784ff76 100644
--- a/drivers/char/tpm/tpm_tis_spi_main.c
+++ b/drivers/char/tpm/tpm_tis_spi_main.c
@@ -71,8 +71,76 @@ static int tpm_tis_spi_flow_control(struct tpm_tis_spi_phy *phy,
return 0;
}

-int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
- u8 *in, const u8 *out)
+/*
+ * Half duplex controller with support for TPM wait state detection like
+ * Tegra QSPI need CMD, ADDR & DATA sent in single message to manage HW flow
+ * control. Each phase sent in different transfer for controller to idenity
+ * phase.
+ */
+static int tpm_tis_spi_hw_flow_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);
+ struct spi_transfer spi_xfer[3];
+ struct spi_message m;
+ u8 transfer_len;
+ int ret;
+
+ 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)
+ return ret;
+
+ if (in) {
+ memcpy(in, &phy->iobuf[4], transfer_len);
+ in += transfer_len;
+ }
+
+ len -= transfer_len;
+ }
+
+ return ret;
+}
+
+static int tpm_tis_spi_sw_flow_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;
@@ -140,6 +208,26 @@ int tpm_tis_spi_transfer(struct tpm_tis_data *data, u32 addr, u16 len,
return ret;
}

+int tpm_tis_spi_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);
+ struct spi_controller *ctlr = phy->spi_device->controller;
+
+ /*
+ * TPM flow control over SPI requires full duplex support.
+ * Send entire message to a half duplex controller to handle
+ * wait polling in controller.
+ * Set TPM HW flow control flag..
+ */
+ if (ctlr->flags & SPI_CONTROLLER_HALF_DUPLEX)
+ return tpm_tis_spi_hw_flow_transfer(data, addr, len, in,
+ out);
+ else
+ return tpm_tis_spi_sw_flow_transfer(data, addr, len, in,
+ out);
+}
+
static int tpm_tis_spi_read_bytes(struct tpm_tis_data *data, u32 addr,
u16 len, u8 *result, enum tpm_tis_io_mode io_mode)
{
@@ -172,6 +260,7 @@ static const struct tpm_tis_phy_ops tpm_spi_phy_ops = {
static int tpm_tis_spi_probe(struct spi_device *dev)
{
struct tpm_tis_spi_phy *phy;
+ struct spi_controller *ctlr = phy->spi_device->controller;
int irq;

phy = devm_kzalloc(&dev->dev, sizeof(struct tpm_tis_spi_phy),
@@ -181,6 +270,9 @@ static int tpm_tis_spi_probe(struct spi_device *dev)

phy->flow_control = tpm_tis_spi_flow_control;

+ if (ctlr->flags & SPI_CONTROLLER_HALF_DUPLEX)
+ phy->spi_device->mode |= SPI_TPM_HW_FLOW;
+
/* If the SPI device has an IRQ then use that */
if (dev->irq > 0)
irq = dev->irq;
--
2.17.1

2023-03-01 17:35:10

by Krishna Yarlagadda

[permalink] [raw]

Subject: [Patch V7 3/3] spi: tegra210-quad: Enable TPM wait polling

Trusted Platform Module requires flow control. As defined in TPM
interface specification, client would drive MISO line at same cycle as
last address bit on MOSI.
Tegra234 and Tegra241 QSPI controllers have TPM wait state detection
feature which is enabled for TPM client devices reported in SPI device
mode bits.

Signed-off-by: Krishna Yarlagadda <[email protected]>
---
drivers/spi/spi-tegra210-quad.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 0b9bc3b7f53a..82aec6cb7863 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -142,6 +142,7 @@

#define QSPI_GLOBAL_CONFIG 0X1a4
#define QSPI_CMB_SEQ_EN BIT(0)
+#define QSPI_TPM_WAIT_POLL_EN BIT(1)

#define QSPI_CMB_SEQ_ADDR 0x1a8
#define QSPI_ADDRESS_VALUE_SET(X) (((x) & 0xFFFF) << 0)
@@ -164,6 +165,7 @@
struct tegra_qspi_soc_data {
bool has_dma;
bool cmb_xfer_capable;
+ bool supports_tpm;
unsigned int cs_count;
};

@@ -1065,6 +1067,12 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,

/* Enable Combined sequence mode */
val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
+ if (spi->mode & SPI_TPM_HW_FLOW) {
+ if (tqspi->soc_data->supports_tpm)
+ val |= QSPI_TPM_WAIT_POLL_EN;
+ else
+ return -EIO;
+ }
val |= QSPI_CMB_SEQ_EN;
tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
/* Process individual transfer list */
@@ -1196,6 +1204,8 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
/* Disable Combined sequence mode */
val = tegra_qspi_readl(tqspi, QSPI_GLOBAL_CONFIG);
val &= ~QSPI_CMB_SEQ_EN;
+ if (tqspi->soc_data->supports_tpm)
+ val &= ~QSPI_TPM_WAIT_POLL_EN;
tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
list_for_each_entry(transfer, &msg->transfers, transfer_list) {
struct spi_transfer *xfer = transfer;
@@ -1454,24 +1464,28 @@ static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
static struct tegra_qspi_soc_data tegra210_qspi_soc_data = {
.has_dma = true,
.cmb_xfer_capable = false,
+ .supports_tpm = false,
.cs_count = 1,
};

static struct tegra_qspi_soc_data tegra186_qspi_soc_data = {
.has_dma = true,
.cmb_xfer_capable = true,
+ .supports_tpm = false,
.cs_count = 1,
};

static struct tegra_qspi_soc_data tegra234_qspi_soc_data = {
.has_dma = false,
.cmb_xfer_capable = true,
+ .supports_tpm = true,
.cs_count = 1,
};

static struct tegra_qspi_soc_data tegra241_qspi_soc_data = {
.has_dma = false,
.cmb_xfer_capable = true,
+ .supports_tpm = true,
.cs_count = 4,
};

--
2.17.1

2023-03-01 22:43:50

by kernel test robot

[permalink] [raw]

Subject: Re: [Patch V7 2/3] tpm_tis-spi: Add hardware wait polling

Hi Krishna,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.2 next-20230301]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Krishna-Yarlagadda/spi-Add-TPM-HW-flow-flag/20230302-013628
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link: https://lore.kernel.org/r/20230301173353.28673-3-kyarlagadda%40nvidia.com
patch subject: [Patch V7 2/3] tpm_tis-spi: Add hardware wait polling
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230302/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/d47344c6b9ab634483742457f6692b01f02c4698
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Krishna-Yarlagadda/spi-Add-TPM-HW-flow-flag/20230302-013628
git checkout d47344c6b9ab634483742457f6692b01f02c4698
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/char/tpm/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

drivers/char/tpm/tpm_tis_spi_main.c:335:36: warning: 'acpi_tis_spi_match' defined but not used [-Wunused-const-variable=]
335 | static const struct acpi_device_id acpi_tis_spi_match[] = {
| ^~~~~~~~~~~~~~~~~~
drivers/char/tpm/tpm_tis_spi_main.c: In function 'tpm_tis_spi_probe':
>> drivers/char/tpm/tpm_tis_spi_main.c:263:42: warning: 'phy' is used uninitialized [-Wuninitialized]
263 | struct spi_controller *ctlr = phy->spi_device->controller;
| ~~~^~~~~~~~~~~~
drivers/char/tpm/tpm_tis_spi_main.c:262:33: note: 'phy' was declared here
262 | struct tpm_tis_spi_phy *phy;
| ^~~

vim +/phy +263 drivers/char/tpm/tpm_tis_spi_main.c

259
260 static int tpm_tis_spi_probe(struct spi_device *dev)
261 {
262 struct tpm_tis_spi_phy *phy;
> 263 struct spi_controller *ctlr = phy->spi_device->controller;
264 int irq;
265
266 phy = devm_kzalloc(&dev->dev, sizeof(struct tpm_tis_spi_phy),
267 GFP_KERNEL);
268 if (!phy)
269 return -ENOMEM;
270
271 phy->flow_control = tpm_tis_spi_flow_control;
272
273 if (ctlr->flags & SPI_CONTROLLER_HALF_DUPLEX)
274 phy->spi_device->mode |= SPI_TPM_HW_FLOW;
275
276 /* If the SPI device has an IRQ then use that */
277 if (dev->irq > 0)
278 irq = dev->irq;
279 else
280 irq = -1;
281
282 init_completion(&phy->ready);
283 return tpm_tis_spi_init(dev, phy, irq, &tpm_spi_phy_ops);
284 }
285

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests