| Message ID | 75b775d0526e72f292e0546a306b37680714686c.1695754856.git.lukas@wunner.de |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Hello Lino, hello Lukas, On Tue, Sep 26, 2023 at 09:09:36PM +0200, Lukas Wunner wrote: > From: Lino Sanfilippo <l.sanfilippo@kunbus.com> > > Normally the platform firmware is responsible for taking a Trusted > Platform Module out of reset on boot and storing measurements into it. > > However if the platform firmware is incapable of doing that -- as is the > case on the Raspberry Pi -- then the onus is on the kernel to take the > TPM out of reset before trying to attach a driver to it. > > Provide a reset driver for such platforms. > > The Infineon SLB9670 TPM requires a specific reset sequence on its RST# > pin which is documented in sections 5.4 and 5.5 of the datasheet: > > https://www.infineon.com/dgdl/Infineon-SLB%209670VQ2.0-DataSheet-v01_04-EN.pdf?fileId=5546d4626fc1ce0b016fc78270350cd6 > > The sequence with minimum wait intervals is as follows: > > deassert RST# > wait at least 60 ms > assert RST# > wait at least 2 usecs > deassert RST# > wait at least 60 ms > assert RST# > wait at least 2 usecs > deassert RST# > wait at least 60 ms before issuing the first TPM command Are you really sure that this change is required? I have seen the RST# Timing diagram in the datasheet, however I wonder if a reset is required at all during power-up, for example. Not to mention that I would have expected some firmware to implement such reset timing and I was not able to find any (I looked at arm/arm64), if this is really required I the driver can work at all? Which platform firmware implements such reset sequence? Not to mention that I was able to see the driver probe succeed in a similar setup to the one you are describing in the commit message (different board, arm64, but nothing done by the platform firmware). What am I missing? Thanks, Francesco
On Wed, Nov 22, 2023 at 12:34:09AM +0100, Francesco Dolcini wrote: > Not to mention that I would have expected some firmware to implement > such reset timing and I was not able to find any (I looked at > arm/arm64), if this is really required I the driver can work at all? ^^^ ...really required how the driver... Francesco
On Wed, Nov 22, 2023 at 12:33:58AM +0100, Francesco Dolcini wrote: > On Tue, Sep 26, 2023 at 09:09:36PM +0200, Lukas Wunner wrote: > > Normally the platform firmware is responsible for taking a Trusted > > Platform Module out of reset on boot and storing measurements into it. > > > > However if the platform firmware is incapable of doing that -- as is the > > case on the Raspberry Pi -- then the onus is on the kernel to take the > > TPM out of reset before trying to attach a driver to it. > > > > Provide a reset driver for such platforms. > > > > The Infineon SLB9670 TPM requires a specific reset sequence on its RST# > > pin which is documented in sections 5.4 and 5.5 of the datasheet: > > Are you really sure that this change is required? > I have seen the RST# Timing diagram in the datasheet, however I wonder > if a reset is required at all during power-up, for example. If the RST# pin is not toggled at all upon a warm reset (reboot), the TPM will remain in whatever state it was during the previous boot. Also, the pin controller connected to RST# might be reset upon a reboot (think of a SoC internal pin controller setting all its registers to 0) and RST# might be asserted as a result. It is then necessary to take the TPM out of reset. > Not to mention that I would have expected some firmware to implement > such reset timing and I was not able to find any (I looked at > arm/arm64), if this is really required I the driver can work at all? > Which platform firmware implements such reset sequence? I can't answer how a TPM is reset by firmware on arm/arm64, you'd have to ask an FAE at ARM. Normally I'd expect firmware in ROM do that so all subsequently executed code which is mutable (EFI, bootloader, kernel) can be measured. Again, on simple platforms such as the Raspberry Pi there's no support to reset a TPM in ROM. > Not to mention that I was able to see the driver probe succeed in a > similar setup to the one you are describing in the commit message > (different board, arm64, but nothing done by the platform firmware). Hm, is the RST# pin even connected on that board? Thanks, Lukas
Hello Lukas, On Wed, Nov 22, 2023 at 12:29:49PM +0100, Lukas Wunner wrote: > On Wed, Nov 22, 2023 at 12:33:58AM +0100, Francesco Dolcini wrote: > > On Tue, Sep 26, 2023 at 09:09:36PM +0200, Lukas Wunner wrote: > > > Normally the platform firmware is responsible for taking a Trusted > > > Platform Module out of reset on boot and storing measurements into it. > > > > > > However if the platform firmware is incapable of doing that -- as is the > > > case on the Raspberry Pi -- then the onus is on the kernel to take the > > > TPM out of reset before trying to attach a driver to it. > > > > > > Provide a reset driver for such platforms. > > > > > > The Infineon SLB9670 TPM requires a specific reset sequence on its RST# > > > pin which is documented in sections 5.4 and 5.5 of the datasheet: > > > > Are you really sure that this change is required? > > I have seen the RST# Timing diagram in the datasheet, however I wonder > > if a reset is required at all during power-up, for example. > > If the RST# pin is not toggled at all upon a warm reset (reboot), > the TPM will remain in whatever state it was during the previous boot. ... > Also, the pin controller connected to RST# might be reset upon a reboot > (think of a SoC internal pin controller setting all its registers to 0) > and RST# might be asserted as a result. It is then necessary to take > the TPM out of reset. Toggled at boot is different from what you are doing here. > > Not to mention that I was able to see the driver probe succeed in a > > similar setup to the one you are describing in the commit message > > (different board, arm64, but nothing done by the platform firmware). > > Hm, is the RST# pin even connected on that board? Yes, it's connected and it is asserted/de-asserted (aka toggled) during startup from the HW reset circuit. However this is not implementing the reset sequence you are implementing here. Francesco
Hello Lukas, On Thu, Nov 23, 2023 at 09:59:43AM +0100, Lukas Wunner wrote: > On Wed, Nov 22, 2023 at 04:15:18PM +0100, Francesco Dolcini wrote: > > On Wed, Nov 22, 2023 at 12:29:49PM +0100, Lukas Wunner wrote: > > > On Wed, Nov 22, 2023 at 12:33:58AM +0100, Francesco Dolcini wrote: > > > > Not to mention that I was able to see the driver probe succeed in a > > > > similar setup to the one you are describing in the commit message > > > > (different board, arm64, but nothing done by the platform firmware). > > > > > > Hm, is the RST# pin even connected on that board? > > > > Yes, it's connected and it is asserted/de-asserted (aka toggled) during > > startup from the HW reset circuit. However this is not implementing the > > reset sequence you are implementing here. > > Section 4.5 of the datasheet seems to indicate that unless the sequence > in Figure 3 is observed, the TPM may enter a defense mode against > dictionary attacks "from which a recovery is very complex or even not > possible." > > Simply toggling the RST# pin might therefore not be sufficient to ensure > the TPM is operable. I am trying to follow-up with infineon on this regard, do you already have any insight from them maybe? Maybe this procedure is relevant only when the device is in "security defense state"? Francesco
Hi Francesco, On Mon, Dec 18, 2023 at 06:34:00PM +0100, Francesco Dolcini wrote: > On Thu, Nov 23, 2023 at 09:59:43AM +0100, Lukas Wunner wrote: > > On Wed, Nov 22, 2023 at 04:15:18PM +0100, Francesco Dolcini wrote: > > > On Wed, Nov 22, 2023 at 12:29:49PM +0100, Lukas Wunner wrote: > > > > On Wed, Nov 22, 2023 at 12:33:58AM +0100, Francesco Dolcini wrote: > > > > > Not to mention that I was able to see the driver probe succeed in a > > > > > similar setup to the one you are describing in the commit message > > > > > (different board, arm64, but nothing done by the platform firmware). > > > > > > > > Hm, is the RST# pin even connected on that board? > > > > > > Yes, it's connected and it is asserted/de-asserted (aka toggled) during > > > startup from the HW reset circuit. However this is not implementing the > > > reset sequence you are implementing here. > > > > Section 4.5 of the datasheet seems to indicate that unless the sequence > > in Figure 3 is observed, the TPM may enter a defense mode against > > dictionary attacks "from which a recovery is very complex or even not > > possible." > > > > Simply toggling the RST# pin might therefore not be sufficient to ensure > > the TPM is operable. > > I am trying to follow-up with infineon on this regard, do you already > have any insight from them maybe? > > Maybe this procedure is relevant only when the device is in "security > defense state"? Sorry, I honestly don't know. A colleague has talked to an FAE at an Infineon reseller but they couldn't give a definitive answer either. I'm very interested to hear whatever you learn from Infineon. Thanks, Lukas
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index ccd59dd..3296e33 100644 --- a/drivers/reset/Kconfig +++ b/drivers/reset/Kconfig @@ -229,6 +229,15 @@ config RESET_SIMPLE - Allwinner SoCs - SiFive FU740 SoCs +config RESET_SLB9670 + tristate "Infineon SLB9670 TPM Reset Driver" + depends on TCG_TIS_SPI + help + This enables the reset driver for the Infineon SLB9670 Trusted + Platform Module. Only say Y here if your platform firmware is + incapable of taking the TPM out of reset on boot, requiring the + kernel to do so. + config RESET_SOCFPGA bool "SoCFPGA Reset Driver" if COMPILE_TEST && (!ARM || !ARCH_INTEL_SOCFPGA) default ARM && ARCH_INTEL_SOCFPGA diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index 8270da8..d9c182e 100644 --- a/drivers/reset/Makefile +++ b/drivers/reset/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_RESET_RASPBERRYPI) += reset-raspberrypi.o obj-$(CONFIG_RESET_RZG2L_USBPHY_CTRL) += reset-rzg2l-usbphy-ctrl.o obj-$(CONFIG_RESET_SCMI) += reset-scmi.o obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o +obj-$(CONFIG_RESET_SLB9670) += reset-slb9670.o obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_RESET_SUNPLUS) += reset-sunplus.o obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o diff --git a/drivers/reset/reset-slb9670.c b/drivers/reset/reset-slb9670.c new file mode 100644 index 00000000..cc09ab5 --- /dev/null +++ b/drivers/reset/reset-slb9670.c @@ -0,0 +1,141 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Reset driver for Infineon SLB9670 Trusted Platform Module + * + * Copyright (C) 2022 KUNBUS GmbH + */ + +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/mod_devicetable.h> +#include <linux/platform_device.h> +#include <linux/reset-controller.h> + +/* + * Time intervals used in the reset sequence: + * + * RSTIN: minimum time to hold the reset line deasserted + * WRST: minimum time to hold the reset line asserted + */ +#define SLB9670_TIME_RSTIN 60 /* msecs */ +#define SLB9670_TIME_WRST 2 /* usecs */ + +struct reset_slb9670 { + struct reset_controller_dev rcdev; + struct gpio_desc *gpio; +}; + +static inline struct reset_slb9670 * +to_reset_slb9670(struct reset_controller_dev *rcdev) +{ + return container_of(rcdev, struct reset_slb9670, rcdev); +} + +static int reset_slb9670_assert(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct reset_slb9670 *reset_slb9670 = to_reset_slb9670(rcdev); + + gpiod_set_value(reset_slb9670->gpio, 1); + return 0; +} + +static int reset_slb9670_deassert(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct reset_slb9670 *reset_slb9670 = to_reset_slb9670(rcdev); + + /* + * Perform the reset sequence: Deassert and assert the reset line twice + * and wait the respective time intervals. After a last wait interval + * of RSTIN the chip is ready to receive the first command. + */ + gpiod_set_value(reset_slb9670->gpio, 0); + msleep(SLB9670_TIME_RSTIN); + gpiod_set_value(reset_slb9670->gpio, 1); + udelay(SLB9670_TIME_WRST); + gpiod_set_value(reset_slb9670->gpio, 0); + msleep(SLB9670_TIME_RSTIN); + gpiod_set_value(reset_slb9670->gpio, 1); + udelay(SLB9670_TIME_WRST); + gpiod_set_value(reset_slb9670->gpio, 0); + msleep(SLB9670_TIME_RSTIN); + + return 0; +} + +static int reset_slb9670_reset(struct reset_controller_dev *rcdev, + unsigned long id) +{ + int ret; + + ret = reset_slb9670_assert(rcdev, id); + if (ret) + return ret; + + return reset_slb9670_deassert(rcdev, id); +} + +static int reset_slb9670_status(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct reset_slb9670 *reset_slb9670 = to_reset_slb9670(rcdev); + + return gpiod_get_value(reset_slb9670->gpio); +} + +static const struct reset_control_ops reset_slb9670_ops = { + .assert = reset_slb9670_assert, + .deassert = reset_slb9670_deassert, + .reset = reset_slb9670_reset, + .status = reset_slb9670_status, +}; + +static int reset_slb9670_of_xlate(struct reset_controller_dev *rcdev, + const struct of_phandle_args *reset_spec) +{ + return 0; +} + +static int reset_slb9670_probe(struct platform_device *pdev) +{ + struct reset_slb9670 *reset_slb9670; + struct device *dev = &pdev->dev; + + reset_slb9670 = devm_kzalloc(dev, sizeof(*reset_slb9670), GFP_KERNEL); + if (!reset_slb9670) + return -ENOMEM; + + reset_slb9670->gpio = devm_gpiod_get(dev, "reset", GPIOD_ASIS); + if (IS_ERR(reset_slb9670->gpio)) + return dev_err_probe(dev, PTR_ERR(reset_slb9670->gpio), + "cannot get reset gpio\n"); + + reset_slb9670->rcdev.nr_resets = 1; + reset_slb9670->rcdev.owner = THIS_MODULE; + reset_slb9670->rcdev.of_node = dev->of_node; + reset_slb9670->rcdev.ops = &reset_slb9670_ops; + reset_slb9670->rcdev.of_xlate = reset_slb9670_of_xlate; + reset_slb9670->rcdev.of_reset_n_cells = 0; + + return devm_reset_controller_register(dev, &reset_slb9670->rcdev); +} + +static const struct of_device_id reset_slb9670_dt_ids[] = { + { .compatible = "infineon,slb9670-reset" }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, reset_slb9670_dt_ids); + +static struct platform_driver reset_slb9670_driver = { + .probe = reset_slb9670_probe, + .driver = { + .name = "reset-slb9670", + .of_match_table = reset_slb9670_dt_ids, + }, +}; +module_platform_driver(reset_slb9670_driver); + +MODULE_DESCRIPTION("Infineon SLB9670 TPM Reset Driver"); +MODULE_AUTHOR("Lino Sanfilippo <l.sanfilippo@kunbus.com>"); +MODULE_LICENSE("GPL");