| Message ID | 75b775d0526e72f292e0546a306b37680714686c.1695754856.git.lukas@wunner.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] dt-bindings: reset: Add Infineon SLB9670 TPM reset driver | 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
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. Here's the relevant section in the datasheet: "The OPTIGA TPM SLB 9670 features a sophisticated protection mechanism against dictionary attacks on TPM-based authorization data. Basically, the device counts the number of failed authorization attempts in a counter which is located in the non-volatile memory. An attacker who has physical access to the device could try to cirumvent that mechanism by resetting the device after the authorization attempt but before the updated failure counter has been written into the NVM. Certain countermeasures have been added to the OPTIGA TPM SLB 9670. In certain time windows during power-on or warm boot of the device, such reset events might influence the dictionary attack counters and trigger other security mechanisms as well. In worst case, this might trigger special security defense modes from which a recovery is very complex or even not possible. To avoid that the OPTIGA TPM SLB 9670 reaches such a security defense state, the RST# signal must not be asserted in certain time windows. After the deassertion of the RST# signal, the system should wait for a minimum time of tRSTIN before asserting RST# again (see Figure 3 and Table 11). TPM commands should only be started after tRSTIN has expired (see Figure 3 again). If a TPM command is running, RST# should not be asserted; otherwise, this might also trigger some security functions. When the TPM shall be reset, the command TPM2_Shutdown should be issued before the assertion of the RST# signal. https://www.infineon.com/dgdl/?fileId=5546d4626fc1ce0b016fc78270350cd6 Thanks, Lukas
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
On 18.12.2023 18:51, Lukas Wunner wrote: > 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. Infineon is here :) I'm sorry, the document is a little confusing, we'll fix that in the future. What the document wants to say is this: Any time you assert RST#, the TPM will reset. But if you reset the TPM at certain points in time, you will trigger some security functions. In general, as long as it only happens occasionally, this is not a problem (you can't avoid all power outages). Only if you happen to frequently issue resets (e.g. if your reset pin is not a dedicated TPM reset pin but is also used for other things), then you should make sure to wait at least t_RSTIN between those resets (and avoid interrupting TPM command execution). So in your case, you probably don't need to do anything special: Just assert RST# once and the TPM will reset. This should work with basically any TPM, so there is no need for a dedicated SLB9670 reset driver. Alexander
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");