Re: [PATCH] eth: asix88179: reset during probe
On 6/18/24 4:57 PM, Caleb Connolly wrote: In some cases (consistently in my case with an embedded board) the ethernet controller will time out on the first init but always succeed after reset. Let's reset the controller during probe so we always start with it in a known state, and don't have wait for the first asix_wait_link() to time out. Signed-off-by: Caleb Connolly Reviewed-by: Marek Vasut Please let me know if you need me to pick this up via usb tree, even if this is kind-of a -net patch . Thanks
[RFC] rockchip: add support for Radxa ROCK Pi E v3.0 which uses DDR4 SDRAM
rk3328-rock-pi-e-v3.dts is identical to rk3328-rock-pi-e.dts in
upstream. only difference between v3.0 and prior ver. is, using
rk3328-sdram-ddr4-666.dtsi instead of rk3328-sdram-ddr3-666.dtsi.
here is console output from ROCK Pi E v3.0:
```
U-Boot TPL 2024.07-rc4-dirty (Jun 23 2024 - 12:53:09)
DDR4, 333MHz
BW=32 Col=10 Bk=4 BG=2 CS0 Row=16 CS=1 Die BW=16 Size=2048MB
Trying to boot from BOOTROM
Returning to boot ROM...
U-Boot SPL 2024.07-rc4-dirty (Jun 23 2024 - 12:53:09 +0900)
Trying to boot from MMC2
```
there is an another way which can share same u-boot-rockchip.bin
between v3 and prior, using ddr blob from Rockchip instead of TPL in
U-Boot. is it acceptable?
Signed-off-by: FUKAUMI Naoki
---
arch/arm/dts/rk3328-rock-pi-e-v3-u-boot.dtsi | 43 +
arch/arm/dts/rk3328-rock-pi-e-v3.dts | 445
arch/arm/dts/rk3328.dtsi | 1943 ++
configs/rock-pi-e-v3-rk3328_defconfig| 97 +
4 files changed, 2528 insertions(+)
create mode 100644 arch/arm/dts/rk3328-rock-pi-e-v3-u-boot.dtsi
create mode 100644 arch/arm/dts/rk3328-rock-pi-e-v3.dts
create mode 100644 arch/arm/dts/rk3328.dtsi
create mode 100644 configs/rock-pi-e-v3-rk3328_defconfig
diff --git a/arch/arm/dts/rk3328-rock-pi-e-v3-u-boot.dtsi
b/arch/arm/dts/rk3328-rock-pi-e-v3-u-boot.dtsi
new file mode 100644
index 00..d7b22b01d7
--- /dev/null
+++ b/arch/arm/dts/rk3328-rock-pi-e-v3-u-boot.dtsi
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2020 Radxa
+ */
+
+#include "rk3328-u-boot.dtsi"
+#include "rk3328-sdram-ddr4-666.dtsi"
+
+/ {
+ smbios {
+ compatible = "u-boot,sysinfo-smbios";
+
+ smbios {
+ system {
+ manufacturer = "radxa";
+ product = "rock-pi-e_rk3328";
+ };
+
+ baseboard {
+ manufacturer = "radxa";
+ product = "rock-pi-e_rk3328";
+ };
+
+ chassis {
+ manufacturer = "radxa";
+ product = "rock-pi-e_rk3328";
+ };
+ };
+ };
+};
+
+_host {
+ phy-supply = <_host_5v>;
+};
+
+_host_5v {
+ /delete-property/ regulator-always-on;
+ /delete-property/ regulator-boot-on;
+};
+
+_sd {
+ bootph-pre-ram;
+};
diff --git a/arch/arm/dts/rk3328-rock-pi-e-v3.dts
b/arch/arm/dts/rk3328-rock-pi-e-v3.dts
new file mode 100644
index 00..3cda6c627b
--- /dev/null
+++ b/arch/arm/dts/rk3328-rock-pi-e-v3.dts
@@ -0,0 +1,445 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * (C) Copyright 2020 Chen-Yu Tsai
+ *
+ * Based on ./rk3328-rock64.dts, which is
+ *
+ * Copyright (c) 2017 PINE64
+ */
+
+/dts-v1/;
+
+#include
+#include
+#include
+#include
+
+#include "rk3328.dtsi"
+
+/ {
+ model = "Radxa ROCK Pi E";
+ compatible = "radxa,rockpi-e", "rockchip,rk3328";
+
+ aliases {
+ ethernet0 =
+ ethernet1 =
+ mmc0 =
+ mmc1 =
+ };
+
+ chosen {
+ stdout-path = "serial2:150n8";
+ };
+
+ adc-keys {
+ compatible = "adc-keys";
+ io-channels = < 0>;
+ io-channel-names = "buttons";
+ keyup-threshold-microvolt = <175>;
+
+ /* This button is unpopulated out of the factory. */
+ button-recovery {
+ label = "Recovery";
+ linux,code = ;
+ press-threshold-microvolt = <1>;
+ };
+ };
+
+ gmac_clkin: external-gmac-clock {
+ compatible = "fixed-clock";
+ clock-frequency = <12500>;
+ clock-output-names = "gmac_clkin";
+ #clock-cells = <0>;
+ };
+
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-0 = <_pin>;
+ pinctrl-names = "default";
+
+ led-0 {
+ color = ;
+ gpios = < RK_PA5 GPIO_ACTIVE_LOW>;
+ linux,default-trigger = "heartbeat";
+ };
+ };
+
+ vcc_sd: sdmmc-regulator {
+ compatible = "regulator-fixed";
+ gpio = < RK_PD6 GPIO_ACTIVE_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_pin>;
+ regulator-name = "vcc_sd";
+ regulator-boot-on;
+ vin-supply = <_io>;
+ };
+
+ vcc_host_5v: vcc-host-5v-regulator {
+ compatible = "regulator-fixed";
+ gpio = < RK_PA7 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_host_drv>;
+ enable-active-high;
+ regulator-name = "vcc_host_5v";
+
[PULL] u-boot-usb/master
This can also go into next if you prefer. The following changes since commit fe2ce09a0753634543c32cafe85eb87a625f76ca: Merge branch 'master' of https://source.denx.de/u-boot/custodians/u-boot-watchdog (2024-06-18 08:34:56 -0600) are available in the Git repository at: git://source.denx.de/u-boot-usb.git master for you to fetch changes up to 0db3941a24bf0b6fc54438e7b9cc66b18f08115b: usb: dwc3: add newlines to dev_vdbg calls in ep0 (2024-06-19 06:15:37 +0200) Caleb Connolly (1): usb: dwc3: add newlines to dev_vdbg calls in ep0 Heinrich Schuchardt (1): usb: informative message if no controller drivers/usb/dwc3/ep0.c| 46 +-- drivers/usb/host/usb-uclass.c | 2 +- 2 files changed, 24 insertions(+), 24 deletions(-)
Re: [PATCH 1/2] usb: dwc2: Extract USB DWC2 register definitions
On 5/22/24 4:22 PM, Kongyang Liu wrote:
Hi,
sorry for the late reply.
diff --git a/drivers/usb/common/dwc2_core.c b/drivers/usb/common/dwc2_core.c
new file mode 100644
index 00..2fa11fd59d
--- /dev/null
+++ b/drivers/usb/common/dwc2_core.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2024, Kongyang Liu
+ */
+
+#include
+#include
+#include
+#include
+
+#include "dwc2_core.h"
+
+void dwc2_flush_tx_fifo(struct dwc2_core_regs *regs, const int num)
+{
+ int ret;
+
+ log_debug("Flush Tx FIFO %d\n", num);
+
+ /* Wait for AHB master IDLE state */
+ ret = wait_for_bit_le32(>global_regs.grstctl, GRSTCTL_AHBIDLE,
true, 1000, false);
Just a quick design point, would it be possible to split this patch into
two, one which adds this .global_regs and changes the code accordingly,
and another which does the code refactoring/move ? That would make it
easier to review.
[PATCH] board: mpfs_icicle: change maintainer to Conor
From: Conor Dooley Padmarao is leaving Microchip soon, and suggested that I should take over maintaining the Icicle in U-Boot in his stead. Suggested-by: Padmarao Begari Signed-off-by: Conor Dooley --- CC: Padmarao Begari CC: Conor Dooley CC: Cyril Jean CC: Tom Rini CC: u-boot@lists.denx.de --- board/microchip/mpfs_icicle/MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/board/microchip/mpfs_icicle/MAINTAINERS b/board/microchip/mpfs_icicle/MAINTAINERS index 22f3b97d8b1..d092b5a8111 100644 --- a/board/microchip/mpfs_icicle/MAINTAINERS +++ b/board/microchip/mpfs_icicle/MAINTAINERS @@ -1,5 +1,5 @@ Microchip MPFS icicle -M: Padmarao Begari +M: Conor Dooley M: Cyril Jean S: Maintained F: board/microchip/mpfs_icicle/ -- 2.43.0
[PATCH] phy: rockchip: inno-hdmi: Fix missing readl base addr
inno_poll passes the reg offset that is used by readl_poll_sleep_timeout
without any base addr.
Fix it.
Bug:
inno_hdmi_phy phy@ff43: Pre-PLL locking failed
inno_hdmi_phy phy@ff43: PHY: Failed to power on phy@ff43: -110.
failed to power on phy (ret=-110)
inno_hdmi_phy phy@ff43: Pre-PLL locking failed
inno_hdmi_phy phy@ff43: PHY: Failed to power on phy@ff43: -110.
failed to power on phy (ret=-110)
Fixes: aa2271184603 ("phy: rockchip: Add Rockchip INNO HDMI PHY driver")
Suggested-by: Jonas Karlman
Signed-off-by: Jagan Teki
---
drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
index 3bb1a254ff..7459779dff 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
@@ -432,8 +432,8 @@ static inline void inno_update_bits(struct inno_hdmi_phy
*inno, u8 reg,
inno_write(inno, reg, tmp);
}
-#define inno_poll(reg, val, cond, sleep_us, timeout_us) \
- readl_poll_sleep_timeout((reg) * 4, val, cond, sleep_us, timeout_us)
+#define inno_poll(inno, reg, val, cond, sleep_us, timeout_us) \
+ readl_poll_sleep_timeout((inno)->regs + ((reg) * 4), val, cond,
sleep_us, timeout_us)
static unsigned long inno_hdmi_phy_get_tmdsclk(struct inno_hdmi_phy *inno,
unsigned long rate)
@@ -575,7 +575,7 @@ inno_hdmi_phy_rk3328_clk_set_rate(struct phy *phy,
inno_update_bits(inno, 0xa0, RK3328_PRE_PLL_POWER_DOWN, 0);
/* Wait for Pre-PLL lock */
- ret = inno_poll(0xa9, val, val & RK3328_PRE_PLL_LOCK_STATUS,
+ ret = inno_poll(inno, 0xa9, val, val & RK3328_PRE_PLL_LOCK_STATUS,
1000, 1);
if (ret) {
dev_err(phy->dev, "Pre-PLL locking failed\n");
@@ -674,7 +674,7 @@ inno_hdmi_phy_rk3328_power_on(struct phy *phy,
RK3328_TMDS_DRIVER_ENABLE);
/* Wait for post PLL lock */
- ret = inno_poll(0xaf, v, v & RK3328_POST_PLL_LOCK_STATUS,
+ ret = inno_poll(inno, 0xaf, v, v & RK3328_POST_PLL_LOCK_STATUS,
1000, 1);
if (ret) {
dev_err(phy->dev, "Post-PLL locking failed\n");
--
2.34.1
Re: [PATCH 2/7] efi_loader: fix the return values on efi_tcg
On Sat, 22 Jun 2024 at 21:01, Heinrich Schuchardt wrote:
>
>
>
> Am 22. Juni 2024 18:09:40 MESZ schrieb Ilias Apalodimas
> :
> >Hi Heinrich,
> >
> >[...]
> >
> >> > rc = tpm2_submit_command(dev, input_param_block,
> >> >output_param_block, _buf_size);
> >> > @@ -714,19 +721,20 @@ efi_tcg2_get_active_pcr_banks(struct
> >> > efi_tcg2_protocol *this,
> >> > u32 *active_pcr_banks)
> >> > {
> >> > struct udevice *dev;
> >> > - efi_status_t ret;
> >> > + efi_status_t ret = EFI_INVALID_PARAMETER;
> >> >
> >> > EFI_ENTRY("%p, %p", this, active_pcr_banks);
> >> >
> >> > - if (!this || !active_pcr_banks) {
> >> > - ret = EFI_INVALID_PARAMETER;
> >> > + if (!this || !active_pcr_banks)
> >> > goto out;
> >> > - }
> >> > - ret = tcg2_platform_get_tpm2();
> >> > - if (ret != EFI_SUCCESS)
> >> > +
> >> > + if (tcg2_platform_get_tpm2())
> >>
> >> EFI_INVALID_PARAMETER does not convey the problem type.
> >> Should we return EFI_DEVICE_ERROR here?
> >>
> >> The authors of the specification only foresaw one or more of the
> >> parameters being incorrect (EFI_INVALID_PARAMETER).
> >
> >I completely agree that the result is misleading. However, I'd prefer
> >sticking to the spec for now and maybe add a comment?
> >
> >
> >>
> >> > + goto out;
> >> > +
> >> > + if (tcg2_get_active_pcr_banks(dev, active_pcr_banks))
> >>
> >> EFI_DEVICE_ERROR?
> >
> >Same here
> >
> >Thanks for the qucik review!
> >/Ilias
>
>
> Reviewed-by: Heinrich Schuchardt
Thanks Heinrich.
FWIW I added a comment with the misleading result
>
>
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >> > goto out;
> >> >
> >> > - ret = tcg2_get_active_pcr_banks(dev, active_pcr_banks);
> >> > + ret = EFI_SUCCESS;
> >> >
> >> > out:
> >> > return EFI_EXIT(ret);
> >> > @@ -852,14 +860,15 @@ static efi_status_t measure_event(struct udevice
> >> > *dev, u32 pcr_index,
> >> > u32 event_type, u32 size, u8 event[])
> >> > {
> >> > struct tpml_digest_values digest_list;
> >> > - efi_status_t ret;
> >> > + efi_status_t ret = EFI_DEVICE_ERROR;
> >> > + int rc;
> >> >
> >> > - ret = tcg2_create_digest(dev, event, size, _list);
> >> > - if (ret != EFI_SUCCESS)
> >> > + rc = tcg2_create_digest(dev, event, size, _list);
> >> > + if (rc)
> >> > goto out;
> >> >
> >> > - ret = tcg2_pcr_extend(dev, pcr_index, _list);
> >> > - if (ret != EFI_SUCCESS)
> >> > + rc = tcg2_pcr_extend(dev, pcr_index, _list);
> >> > + if (rc)
> >> > goto out;
> >> >
> >> > ret = tcg2_agile_log_append(pcr_index, event_type, _list,
> >> > @@ -901,10 +910,10 @@ static efi_status_t efi_init_event_log(void)
> >> > struct tcg2_event_log elog;
> >> > struct udevice *dev;
> >> > efi_status_t ret;
> >> > + int rc;
> >> >
> >> > - ret = tcg2_platform_get_tpm2();
> >> > - if (ret != EFI_SUCCESS)
> >> > - return ret;
> >> > + if (tcg2_platform_get_tpm2())
> >> > + return EFI_DEVICE_ERROR;
> >> >
> >> > ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA,
> >> > TPM2_EVENT_LOG_SIZE,
> >> > (void **)_log.buffer);
> >> > @@ -933,9 +942,11 @@ static efi_status_t efi_init_event_log(void)
> >> >*/
> >> > elog.log = event_log.buffer;
> >> > elog.log_size = TPM2_EVENT_LOG_SIZE;
> >> > - ret = tcg2_log_prepare_buffer(dev, , false);
> >> > - if (ret != EFI_SUCCESS)
> >> > + rc = tcg2_log_prepare_buffer(dev, , false);
> >> > + if (rc) {
> >> > + ret = (rc == -ENOBUFS) ? EFI_BUFFER_TOO_SMALL :
> >> > EFI_DEVICE_ERROR;
> >> > goto free_pool;
> >> > + }
> >> >
> >> > event_log.pos = elog.log_position;
> >> >
> >> > @@ -1306,8 +1317,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
> >> > if (!is_tcg2_protocol_installed())
> >> > return EFI_SUCCESS;
> >> >
> >> > - ret = tcg2_platform_get_tpm2();
> >> > - if (ret != EFI_SUCCESS)
> >> > + if (tcg2_platform_get_tpm2())
> >> > return EFI_SECURITY_VIOLATION;
> >> >
> >> > rsvmap_size = size_of_rsvmap(dtb);
> >> > @@ -1356,8 +1366,7 @@ efi_status_t
> >> > efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
> >> > if (tcg2_efi_app_invoked)
> >> > return EFI_SUCCESS;
> >> >
> >> > - ret = tcg2_platform_get_tpm2();
> >> > - if (ret != EFI_SUCCESS)
> >> > + if (tcg2_platform_get_tpm2())
> >> > return EFI_SECURITY_VIOLATION;
> >> >
> >> > ret = tcg2_measure_boot_variable(dev);
> >> > @@ -1406,9 +1415,8 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
> >> > if (!is_tcg2_protocol_installed())
> >> > return EFI_SUCCESS;
> >> >
> >> > - ret = tcg2_platform_get_tpm2();
> >> > - if (ret
Re: [PATCH 2/7] efi_loader: fix the return values on efi_tcg
Am 22. Juni 2024 18:09:40 MESZ schrieb Ilias Apalodimas
:
>Hi Heinrich,
>
>[...]
>
>> > rc = tpm2_submit_command(dev, input_param_block,
>> >output_param_block, _buf_size);
>> > @@ -714,19 +721,20 @@ efi_tcg2_get_active_pcr_banks(struct
>> > efi_tcg2_protocol *this,
>> > u32 *active_pcr_banks)
>> > {
>> > struct udevice *dev;
>> > - efi_status_t ret;
>> > + efi_status_t ret = EFI_INVALID_PARAMETER;
>> >
>> > EFI_ENTRY("%p, %p", this, active_pcr_banks);
>> >
>> > - if (!this || !active_pcr_banks) {
>> > - ret = EFI_INVALID_PARAMETER;
>> > + if (!this || !active_pcr_banks)
>> > goto out;
>> > - }
>> > - ret = tcg2_platform_get_tpm2();
>> > - if (ret != EFI_SUCCESS)
>> > +
>> > + if (tcg2_platform_get_tpm2())
>>
>> EFI_INVALID_PARAMETER does not convey the problem type.
>> Should we return EFI_DEVICE_ERROR here?
>>
>> The authors of the specification only foresaw one or more of the
>> parameters being incorrect (EFI_INVALID_PARAMETER).
>
>I completely agree that the result is misleading. However, I'd prefer
>sticking to the spec for now and maybe add a comment?
>
>
>>
>> > + goto out;
>> > +
>> > + if (tcg2_get_active_pcr_banks(dev, active_pcr_banks))
>>
>> EFI_DEVICE_ERROR?
>
>Same here
>
>Thanks for the qucik review!
>/Ilias
Reviewed-by: Heinrich Schuchardt
>>
>> Best regards
>>
>> Heinrich
>>
>> > goto out;
>> >
>> > - ret = tcg2_get_active_pcr_banks(dev, active_pcr_banks);
>> > + ret = EFI_SUCCESS;
>> >
>> > out:
>> > return EFI_EXIT(ret);
>> > @@ -852,14 +860,15 @@ static efi_status_t measure_event(struct udevice
>> > *dev, u32 pcr_index,
>> > u32 event_type, u32 size, u8 event[])
>> > {
>> > struct tpml_digest_values digest_list;
>> > - efi_status_t ret;
>> > + efi_status_t ret = EFI_DEVICE_ERROR;
>> > + int rc;
>> >
>> > - ret = tcg2_create_digest(dev, event, size, _list);
>> > - if (ret != EFI_SUCCESS)
>> > + rc = tcg2_create_digest(dev, event, size, _list);
>> > + if (rc)
>> > goto out;
>> >
>> > - ret = tcg2_pcr_extend(dev, pcr_index, _list);
>> > - if (ret != EFI_SUCCESS)
>> > + rc = tcg2_pcr_extend(dev, pcr_index, _list);
>> > + if (rc)
>> > goto out;
>> >
>> > ret = tcg2_agile_log_append(pcr_index, event_type, _list,
>> > @@ -901,10 +910,10 @@ static efi_status_t efi_init_event_log(void)
>> > struct tcg2_event_log elog;
>> > struct udevice *dev;
>> > efi_status_t ret;
>> > + int rc;
>> >
>> > - ret = tcg2_platform_get_tpm2();
>> > - if (ret != EFI_SUCCESS)
>> > - return ret;
>> > + if (tcg2_platform_get_tpm2())
>> > + return EFI_DEVICE_ERROR;
>> >
>> > ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
>> > (void **)_log.buffer);
>> > @@ -933,9 +942,11 @@ static efi_status_t efi_init_event_log(void)
>> >*/
>> > elog.log = event_log.buffer;
>> > elog.log_size = TPM2_EVENT_LOG_SIZE;
>> > - ret = tcg2_log_prepare_buffer(dev, , false);
>> > - if (ret != EFI_SUCCESS)
>> > + rc = tcg2_log_prepare_buffer(dev, , false);
>> > + if (rc) {
>> > + ret = (rc == -ENOBUFS) ? EFI_BUFFER_TOO_SMALL :
>> > EFI_DEVICE_ERROR;
>> > goto free_pool;
>> > + }
>> >
>> > event_log.pos = elog.log_position;
>> >
>> > @@ -1306,8 +1317,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
>> > if (!is_tcg2_protocol_installed())
>> > return EFI_SUCCESS;
>> >
>> > - ret = tcg2_platform_get_tpm2();
>> > - if (ret != EFI_SUCCESS)
>> > + if (tcg2_platform_get_tpm2())
>> > return EFI_SECURITY_VIOLATION;
>> >
>> > rsvmap_size = size_of_rsvmap(dtb);
>> > @@ -1356,8 +1366,7 @@ efi_status_t
>> > efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
>> > if (tcg2_efi_app_invoked)
>> > return EFI_SUCCESS;
>> >
>> > - ret = tcg2_platform_get_tpm2();
>> > - if (ret != EFI_SUCCESS)
>> > + if (tcg2_platform_get_tpm2())
>> > return EFI_SECURITY_VIOLATION;
>> >
>> > ret = tcg2_measure_boot_variable(dev);
>> > @@ -1406,9 +1415,8 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
>> > if (!is_tcg2_protocol_installed())
>> > return EFI_SUCCESS;
>> >
>> > - ret = tcg2_platform_get_tpm2();
>> > - if (ret != EFI_SUCCESS)
>> > - return ret;
>> > + if (tcg2_platform_get_tpm2())
>> > + return EFI_SECURITY_VIOLATION;
>> >
>> > ret = measure_event(dev, 4, EV_EFI_ACTION,
>> > strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
>> > @@ -1437,9 +1445,10 @@ efi_tcg2_notify_exit_boot_services(struct efi_event
>> > *event, void *context)
>> > goto out;
>> > }
Re: [PATCH 4/7] tpm: Move TCG into a separate library
On Sat, 22 Jun 2024 at 19:36, Ilias Apalodimas
wrote:
>
> Hi
>
> again many thanks for the quick review
>
> On Sat, 22 Jun 2024 at 19:25, Heinrich Schuchardt wrote:
> >
> > On 22.06.24 16:35, Ilias Apalodimas wrote:
> > > commit 97707f12fdab ("tpm: Support boot measurements") moved out code
> > > from the EFI subsystem into the TPM one to support measurements when
> > > booting with !EFI.
> > >
> > > Those were moved directly into the TPM subsystem and in the tpm-v2.c
> > > library. In hindsight, it would have been better to move it in new
> > > files since the TCG2 is governed by its own spec and it's cleaner
> > > when we want to enable certian parts of the TPM functionality.
> >
> > Nits:
> >
> > %s/certian/certain/
> >
> > >
> > > So let's create a header file and another library and move the TCG
> > > specific bits there.
> > >
> > > Signed-off-by: Ilias Apalodimas
> > > ---
> > > boot/bootm.c | 1 +
> > > include/efi_tcg2.h | 1 +
> > > include/tpm-v2.h | 474 +-
> > > include/tpm_tcg2.h | 336 ++
> > > lib/Makefile | 2 +
> > > lib/tpm-v2.c | 676 +--
> > > lib/tpm_tcg2.c | 696 +
> >
> > The patch series were easier to review if moving header definitions were
> > separated from moving implementations.
So, I can't do that because I'll need an intermediate commit to
include tpm_tcg2.h to tpm-v2.h which I'd rather avoid.
I can make the diff smaller though. Are you ok with that ?
Thanks
/Ilias
> >
> > This patch contains changes that are not described in the commit
> > message, e.g.
> >
> > if (elog->log_size) {
> > if (log.found) {
> > if (elog->log_size < log.log_position)
> > - return -ENOBUFS;
> > + return -ENOSPC;
>
> And this is a great catch. this changed in patch#1 and the correct
> return is -ENOBUFS. I started working on 2 trees and obviously messed
> up this rebase... Thanks!
>
> >
> > I guess you wanted to put this into patch 1.
> >
> > Please, separate the patches adequately.
>
> Fair enough. I thought it was going to be hard not breaking
> compilation hence the big patch. I'll try splitting it
>
> >
> > + * Copyright (c) 2020 Linaro
> > + * Copyright (c) 2023 Linaro Limited
> >
> > The copyright lines look inconsistent. Linaro Limited exists under this
> > name since April 13th, 2010. Is the 2020 copyright for a different company?
> >
>
> I'll keep the older one, same company
>
> [...]
>
> > > +}
> > > +
> > > +__weak void tcg2_platform_startup_error(struct udevice *dev, int rc) {}
> > > +
> >
> > git warning: "new blank line at EOF".
> >
> > Otherwise looks good.
> >
> > Best regards
> >
> > Heinrich
>
> Thanks
> /Ilias
Pull request doc-2024-07-rc5-2
Dear Tom, The following changes since commit fe2ce09a0753634543c32cafe85eb87a625f76ca: Merge branch 'master' of https://source.denx.de/u-boot/custodians/u-boot-watchdog (2024-06-18 08:34:56 -0600) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/doc-2024-07-rc5-2 for you to fetch changes up to 271ca9ef8a37b8668b9858638175ac2da5c152df: doc: board: ti: Add capsule documentation for TI K3 devices (2024-06-22 17:04:19 +0200) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21289 Pull request doc-2024-07-rc5-2 Documentation: * Fix broken references to pytest suite and test writing * Fix links to FIT documentation * Add capsule documentation for TI K3 devices Alexander Dahl (2): doc: develop: testing: Fix broken reference to pytest suite help doc: develop: testing: Fix reference to test writing section Heinrich Schuchardt (3): cmd: link to doc/usage/fit/x86-fit-boot.rst doc: FIT links in develop/uefi/uefi.rst boot: links to FIT documentation in Kconfig Jonathan Humphreys (1): doc: board: ti: Add capsule documentation for TI K3 devices boot/Kconfig | 6 +++--- cmd/Kconfig | 2 +- doc/board/ti/k3.rst | 29 + doc/develop/testing.rst | 4 ++-- doc/develop/uefi/uefi.rst | 4 ++-- 5 files changed, 37 insertions(+), 8 deletions(-)
Re: [PATCH 7/7] tpm: allow the user to select the compiled algorithms
On Sat, 22 Jun 2024 at 19:34, Heinrich Schuchardt wrote:
>
> On 22.06.24 16:35, Ilias Apalodimas wrote:
> > Simon reports that after enabling all algorithms on the TPM some boards
> > fail since they don't have enough storage to accommodate the ~5KB growth.
> >
> > The choice of hash algorithms are determined by the platform and the TPM
> > configuration. Failing to cap a PCR in a bank which the platform left
> > active is a security vulnerability. It might allow unsealing of secrets
> > if an attacker can replay a good set of measurements into an unused bank.
> >
> > If MEASURED_BOOT or EFI_TCG2_PROTOCOL is enabled our Kconfig will enable
> > all supported hashing algorithms. We still want to allow users to add a
> > TPM and not enable measured boot via EFI or bootm though and at the same
> > time, control the compiled algorithms for size reasons.
> >
> > So let's add a function tpm2_allow_extend() which checks the TPM active
> > PCRs banks against the one U-Boot was compiled with.
> > If all the active PCRs banks are not enabled refuse to extend a PCR but
> > otherwise leave the TPM functional.
>
> The paragraph above is bit hard to read. I guess you mean:
>
> We only allow extending PCRs using one of the algorithms selected in the
> configuration.
Yes
>
> >
> > It's worth noting that this is only added on TPM2.0, since TPM1.2 is
> > lacking a lot of code at the moment to read the available PCRs.
> > We unconditionally enable SHA1 when a TPM is selected, which is the only
> > hashing algorithm v1.2 supports.
>
> Why do we need SHA1 if we cannot access PCRs on a TPM1.2?
You can. On TPM1.2 we don't have the functions to check for the active
PCR banks. So I am unconditionally enabling SHA1, which is the only
hashing algo TPM1.2 supports.
Thanks
/Ilias
>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Ilias Apalodimas
> > ---
> > boot/Kconfig | 4
> > include/tpm-v2.h | 59 +++-
> > lib/Kconfig | 6 ++---
> > lib/tpm-v2.c | 40 +---
> > 4 files changed, 87 insertions(+), 22 deletions(-)
> >
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > index 6f3096c15a6f..b061891e109c 100644
> > --- a/boot/Kconfig
> > +++ b/boot/Kconfig
> > @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT
> > config MEASURED_BOOT
> > bool "Measure boot images and configuration when booting without EFI"
> > depends on HASH && TPM_V2
> > + select SHA1
> > + select SHA256
> > + select SHA384
> > + select SHA512
> > help
> > This option enables measurement of the boot process when booting
> > without UEFI . Measurement involves creating cryptographic hashes
> > diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> > index eac04d1c6831..fccb07fa4695 100644
> > --- a/include/tpm-v2.h
> > +++ b/include/tpm-v2.h
> > @@ -277,48 +277,40 @@ struct digest_info {
> > #define TCG2_BOOT_HASH_ALG_SM3_256 0x0010
> >
> > static const struct digest_info hash_algo_list[] = {
> > +#if IS_ENABLED(CONFIG_SHA1)
> > {
> > "sha1",
> > TPM2_ALG_SHA1,
> > TCG2_BOOT_HASH_ALG_SHA1,
> > TPM2_SHA1_DIGEST_SIZE,
> > },
> > +#endif
> > +#if IS_ENABLED(CONFIG_SHA256)
> > {
> > "sha256",
> > TPM2_ALG_SHA256,
> > TCG2_BOOT_HASH_ALG_SHA256,
> > TPM2_SHA256_DIGEST_SIZE,
> > },
> > +#endif
> > +#if IS_ENABLED(CONFIG_SHA384)
> > {
> > "sha384",
> > TPM2_ALG_SHA384,
> > TCG2_BOOT_HASH_ALG_SHA384,
> > TPM2_SHA384_DIGEST_SIZE,
> > },
> > +#endif
> > +#if IS_ENABLED(CONFIG_SHA512)
> > {
> > "sha512",
> > TPM2_ALG_SHA512,
> > TCG2_BOOT_HASH_ALG_SHA512,
> > TPM2_SHA512_DIGEST_SIZE,
> > },
> > +#endif
> > };
> >
> > -static inline u16 tpm2_algorithm_to_len(enum tpm2_algorithms a)
> > -{
> > - switch (a) {
> > - case TPM2_ALG_SHA1:
> > - return TPM2_SHA1_DIGEST_SIZE;
> > - case TPM2_ALG_SHA256:
> > - return TPM2_SHA256_DIGEST_SIZE;
> > - case TPM2_ALG_SHA384:
> > - return TPM2_SHA384_DIGEST_SIZE;
> > - case TPM2_ALG_SHA512:
> > - return TPM2_SHA512_DIGEST_SIZE;
> > - default:
> > - return 0;
> > - }
> > -}
> > -
> > /* NV index attributes */
> > enum tpm_index_attrs {
> > TPMA_NV_PPWRITE = 1UL << 0,
> > @@ -711,6 +703,41 @@ enum tpm2_algorithms tpm2_name_to_algorithm(const char
> > *name);
> >*/
> > const char *tpm2_algorithm_name(enum tpm2_algorithms);
> >
> > +/**
> > + * tpm2_algorithm_to_len() - Return an algorithm length for supported
> > algorithm id
> > + *
> > + * @algorithm_id: algorithm defined in enum tpm2_algorithms
> > + * Return: len or 0 if not supported
> > + */
> > +u16 tpm2_algorithm_to_len(enum tpm2_algorithms algo);
> > +
>
Re: [PATCH] efi_loader: adjust config options for capsule updates
On Sat, 22 Jun 2024 at 19:36, Heinrich Schuchardt wrote: > > On 20.06.24 22:15, Ilias Apalodimas wrote: > > EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable > > at runtime is not supported and allow the platform to perform capsule > > updates on disk. With the recent changes boards can conditionally enable > > setvariable at runtime using EFI_RT_VOLATILE_STORE. > > > > Let's make that visible in our Kconfigs and enable EFI_IGNORE_OSINDICATIONS > > when set variable at runtime is disabled. > > > > Since EFI_RT_VOLATILE_STORE needs help from the OS to persist the > > variables, allow users to ignore OsIndications even if setvariable at > > runtime is enabled. > > > > Signed-off-by: Ilias Apalodimas > > So this v2: Yes sorry, forgot to add the tile and log... > > v2: > allow EFI_IGNORE_OSINDICATIONS if EFI_RT_VOLATILE_STORE=y > > Reviewed-by: Heinrich Schuchardt Thanks Heinrich > > > --- > > lib/efi_loader/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index ee71f417147a..6006e845cb1f 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -220,6 +220,7 @@ config EFI_CAPSULE_ON_DISK > > config EFI_IGNORE_OSINDICATIONS > > bool "Ignore OsIndications for CapsuleUpdate on-disk" > > depends on EFI_CAPSULE_ON_DISK > > + default y if !EFI_RT_VOLATILE_STORE > > help > > There are boards where U-Boot does not support SetVariable at > > runtime. > > Select this option if you want to use the capsule-on-disk feature > > -- > > 2.43.0 > > >
Re: [PATCH 4/7] tpm: Move TCG into a separate library
Hi
again many thanks for the quick review
On Sat, 22 Jun 2024 at 19:25, Heinrich Schuchardt wrote:
>
> On 22.06.24 16:35, Ilias Apalodimas wrote:
> > commit 97707f12fdab ("tpm: Support boot measurements") moved out code
> > from the EFI subsystem into the TPM one to support measurements when
> > booting with !EFI.
> >
> > Those were moved directly into the TPM subsystem and in the tpm-v2.c
> > library. In hindsight, it would have been better to move it in new
> > files since the TCG2 is governed by its own spec and it's cleaner
> > when we want to enable certian parts of the TPM functionality.
>
> Nits:
>
> %s/certian/certain/
>
> >
> > So let's create a header file and another library and move the TCG
> > specific bits there.
> >
> > Signed-off-by: Ilias Apalodimas
> > ---
> > boot/bootm.c | 1 +
> > include/efi_tcg2.h | 1 +
> > include/tpm-v2.h | 474 +-
> > include/tpm_tcg2.h | 336 ++
> > lib/Makefile | 2 +
> > lib/tpm-v2.c | 676 +--
> > lib/tpm_tcg2.c | 696 +
>
> The patch series were easier to review if moving header definitions were
> separated from moving implementations.
>
> This patch contains changes that are not described in the commit
> message, e.g.
>
> if (elog->log_size) {
> if (log.found) {
> if (elog->log_size < log.log_position)
> - return -ENOBUFS;
> + return -ENOSPC;
And this is a great catch. this changed in patch#1 and the correct
return is -ENOBUFS. I started working on 2 trees and obviously messed
up this rebase... Thanks!
>
> I guess you wanted to put this into patch 1.
>
> Please, separate the patches adequately.
Fair enough. I thought it was going to be hard not breaking
compilation hence the big patch. I'll try splitting it
>
> + * Copyright (c) 2020 Linaro
> + * Copyright (c) 2023 Linaro Limited
>
> The copyright lines look inconsistent. Linaro Limited exists under this
> name since April 13th, 2010. Is the 2020 copyright for a different company?
>
I'll keep the older one, same company
[...]
> > +}
> > +
> > +__weak void tcg2_platform_startup_error(struct udevice *dev, int rc) {}
> > +
>
> git warning: "new blank line at EOF".
>
> Otherwise looks good.
>
> Best regards
>
> Heinrich
Thanks
/Ilias
Re: [PATCH] efi_loader: adjust config options for capsule updates
On 20.06.24 22:15, Ilias Apalodimas wrote: EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable at runtime is not supported and allow the platform to perform capsule updates on disk. With the recent changes boards can conditionally enable setvariable at runtime using EFI_RT_VOLATILE_STORE. Let's make that visible in our Kconfigs and enable EFI_IGNORE_OSINDICATIONS when set variable at runtime is disabled. Since EFI_RT_VOLATILE_STORE needs help from the OS to persist the variables, allow users to ignore OsIndications even if setvariable at runtime is enabled. Signed-off-by: Ilias Apalodimas So this v2: v2: allow EFI_IGNORE_OSINDICATIONS if EFI_RT_VOLATILE_STORE=y Reviewed-by: Heinrich Schuchardt --- lib/efi_loader/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index ee71f417147a..6006e845cb1f 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -220,6 +220,7 @@ config EFI_CAPSULE_ON_DISK config EFI_IGNORE_OSINDICATIONS bool "Ignore OsIndications for CapsuleUpdate on-disk" depends on EFI_CAPSULE_ON_DISK + default y if !EFI_RT_VOLATILE_STORE help There are boards where U-Boot does not support SetVariable at runtime. Select this option if you want to use the capsule-on-disk feature -- 2.43.0
Re: [PATCH 7/7] tpm: allow the user to select the compiled algorithms
On 22.06.24 16:35, Ilias Apalodimas wrote:
Simon reports that after enabling all algorithms on the TPM some boards
fail since they don't have enough storage to accommodate the ~5KB growth.
The choice of hash algorithms are determined by the platform and the TPM
configuration. Failing to cap a PCR in a bank which the platform left
active is a security vulnerability. It might allow unsealing of secrets
if an attacker can replay a good set of measurements into an unused bank.
If MEASURED_BOOT or EFI_TCG2_PROTOCOL is enabled our Kconfig will enable
all supported hashing algorithms. We still want to allow users to add a
TPM and not enable measured boot via EFI or bootm though and at the same
time, control the compiled algorithms for size reasons.
So let's add a function tpm2_allow_extend() which checks the TPM active
PCRs banks against the one U-Boot was compiled with.
If all the active PCRs banks are not enabled refuse to extend a PCR but
otherwise leave the TPM functional.
The paragraph above is bit hard to read. I guess you mean:
We only allow extending PCRs using one of the algorithms selected in the
configuration.
It's worth noting that this is only added on TPM2.0, since TPM1.2 is
lacking a lot of code at the moment to read the available PCRs.
We unconditionally enable SHA1 when a TPM is selected, which is the only
hashing algorithm v1.2 supports.
Why do we need SHA1 if we cannot access PCRs on a TPM1.2?
Best regards
Heinrich
Signed-off-by: Ilias Apalodimas
---
boot/Kconfig | 4
include/tpm-v2.h | 59 +++-
lib/Kconfig | 6 ++---
lib/tpm-v2.c | 40 +---
4 files changed, 87 insertions(+), 22 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig
index 6f3096c15a6f..b061891e109c 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT
config MEASURED_BOOT
bool "Measure boot images and configuration when booting without EFI"
depends on HASH && TPM_V2
+ select SHA1
+ select SHA256
+ select SHA384
+ select SHA512
help
This option enables measurement of the boot process when booting
without UEFI . Measurement involves creating cryptographic hashes
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index eac04d1c6831..fccb07fa4695 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -277,48 +277,40 @@ struct digest_info {
#define TCG2_BOOT_HASH_ALG_SM3_256 0x0010
static const struct digest_info hash_algo_list[] = {
+#if IS_ENABLED(CONFIG_SHA1)
{
"sha1",
TPM2_ALG_SHA1,
TCG2_BOOT_HASH_ALG_SHA1,
TPM2_SHA1_DIGEST_SIZE,
},
+#endif
+#if IS_ENABLED(CONFIG_SHA256)
{
"sha256",
TPM2_ALG_SHA256,
TCG2_BOOT_HASH_ALG_SHA256,
TPM2_SHA256_DIGEST_SIZE,
},
+#endif
+#if IS_ENABLED(CONFIG_SHA384)
{
"sha384",
TPM2_ALG_SHA384,
TCG2_BOOT_HASH_ALG_SHA384,
TPM2_SHA384_DIGEST_SIZE,
},
+#endif
+#if IS_ENABLED(CONFIG_SHA512)
{
"sha512",
TPM2_ALG_SHA512,
TCG2_BOOT_HASH_ALG_SHA512,
TPM2_SHA512_DIGEST_SIZE,
},
+#endif
};
-static inline u16 tpm2_algorithm_to_len(enum tpm2_algorithms a)
-{
- switch (a) {
- case TPM2_ALG_SHA1:
- return TPM2_SHA1_DIGEST_SIZE;
- case TPM2_ALG_SHA256:
- return TPM2_SHA256_DIGEST_SIZE;
- case TPM2_ALG_SHA384:
- return TPM2_SHA384_DIGEST_SIZE;
- case TPM2_ALG_SHA512:
- return TPM2_SHA512_DIGEST_SIZE;
- default:
- return 0;
- }
-}
-
/* NV index attributes */
enum tpm_index_attrs {
TPMA_NV_PPWRITE = 1UL << 0,
@@ -711,6 +703,41 @@ enum tpm2_algorithms tpm2_name_to_algorithm(const char
*name);
*/
const char *tpm2_algorithm_name(enum tpm2_algorithms);
+/**
+ * tpm2_algorithm_to_len() - Return an algorithm length for supported
algorithm id
+ *
+ * @algorithm_id: algorithm defined in enum tpm2_algorithms
+ * Return: len or 0 if not supported
+ */
+u16 tpm2_algorithm_to_len(enum tpm2_algorithms algo);
+
+/*
+ * When measured boot is enabled via EFI or bootX commands all the algorithms
+ * above are selected by our Kconfigs. Due to U-Boots nature of being small
there
+ * are cases where we need some functionality from the TPM -- e.g storage or
RNG
+ * but we don't want to support measurements.
+ *
+ * The choice of hash algorithms are determined by the platform and the TPM
+ * configuration. Failing to cap a PCR in a bank which the platform left
+ * active is a security vulnerability. It permits the unsealing of secrets
+ * if an attacker can replay a good set of measurements into an unused bank.
+ *
+ * On top of that a previous stage
Re: [PATCH 2/7] efi_loader: fix the return values on efi_tcg
Hi Heinrich,
[...]
> > rc = tpm2_submit_command(dev, input_param_block,
> >output_param_block, _buf_size);
> > @@ -714,19 +721,20 @@ efi_tcg2_get_active_pcr_banks(struct
> > efi_tcg2_protocol *this,
> > u32 *active_pcr_banks)
> > {
> > struct udevice *dev;
> > - efi_status_t ret;
> > + efi_status_t ret = EFI_INVALID_PARAMETER;
> >
> > EFI_ENTRY("%p, %p", this, active_pcr_banks);
> >
> > - if (!this || !active_pcr_banks) {
> > - ret = EFI_INVALID_PARAMETER;
> > + if (!this || !active_pcr_banks)
> > goto out;
> > - }
> > - ret = tcg2_platform_get_tpm2();
> > - if (ret != EFI_SUCCESS)
> > +
> > + if (tcg2_platform_get_tpm2())
>
> EFI_INVALID_PARAMETER does not convey the problem type.
> Should we return EFI_DEVICE_ERROR here?
>
> The authors of the specification only foresaw one or more of the
> parameters being incorrect (EFI_INVALID_PARAMETER).
I completely agree that the result is misleading. However, I'd prefer
sticking to the spec for now and maybe add a comment?
>
> > + goto out;
> > +
> > + if (tcg2_get_active_pcr_banks(dev, active_pcr_banks))
>
> EFI_DEVICE_ERROR?
Same here
Thanks for the qucik review!
/Ilias
>
> Best regards
>
> Heinrich
>
> > goto out;
> >
> > - ret = tcg2_get_active_pcr_banks(dev, active_pcr_banks);
> > + ret = EFI_SUCCESS;
> >
> > out:
> > return EFI_EXIT(ret);
> > @@ -852,14 +860,15 @@ static efi_status_t measure_event(struct udevice
> > *dev, u32 pcr_index,
> > u32 event_type, u32 size, u8 event[])
> > {
> > struct tpml_digest_values digest_list;
> > - efi_status_t ret;
> > + efi_status_t ret = EFI_DEVICE_ERROR;
> > + int rc;
> >
> > - ret = tcg2_create_digest(dev, event, size, _list);
> > - if (ret != EFI_SUCCESS)
> > + rc = tcg2_create_digest(dev, event, size, _list);
> > + if (rc)
> > goto out;
> >
> > - ret = tcg2_pcr_extend(dev, pcr_index, _list);
> > - if (ret != EFI_SUCCESS)
> > + rc = tcg2_pcr_extend(dev, pcr_index, _list);
> > + if (rc)
> > goto out;
> >
> > ret = tcg2_agile_log_append(pcr_index, event_type, _list,
> > @@ -901,10 +910,10 @@ static efi_status_t efi_init_event_log(void)
> > struct tcg2_event_log elog;
> > struct udevice *dev;
> > efi_status_t ret;
> > + int rc;
> >
> > - ret = tcg2_platform_get_tpm2();
> > - if (ret != EFI_SUCCESS)
> > - return ret;
> > + if (tcg2_platform_get_tpm2())
> > + return EFI_DEVICE_ERROR;
> >
> > ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
> > (void **)_log.buffer);
> > @@ -933,9 +942,11 @@ static efi_status_t efi_init_event_log(void)
> >*/
> > elog.log = event_log.buffer;
> > elog.log_size = TPM2_EVENT_LOG_SIZE;
> > - ret = tcg2_log_prepare_buffer(dev, , false);
> > - if (ret != EFI_SUCCESS)
> > + rc = tcg2_log_prepare_buffer(dev, , false);
> > + if (rc) {
> > + ret = (rc == -ENOBUFS) ? EFI_BUFFER_TOO_SMALL :
> > EFI_DEVICE_ERROR;
> > goto free_pool;
> > + }
> >
> > event_log.pos = elog.log_position;
> >
> > @@ -1306,8 +1317,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
> > if (!is_tcg2_protocol_installed())
> > return EFI_SUCCESS;
> >
> > - ret = tcg2_platform_get_tpm2();
> > - if (ret != EFI_SUCCESS)
> > + if (tcg2_platform_get_tpm2())
> > return EFI_SECURITY_VIOLATION;
> >
> > rsvmap_size = size_of_rsvmap(dtb);
> > @@ -1356,8 +1366,7 @@ efi_status_t
> > efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
> > if (tcg2_efi_app_invoked)
> > return EFI_SUCCESS;
> >
> > - ret = tcg2_platform_get_tpm2();
> > - if (ret != EFI_SUCCESS)
> > + if (tcg2_platform_get_tpm2())
> > return EFI_SECURITY_VIOLATION;
> >
> > ret = tcg2_measure_boot_variable(dev);
> > @@ -1406,9 +1415,8 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
> > if (!is_tcg2_protocol_installed())
> > return EFI_SUCCESS;
> >
> > - ret = tcg2_platform_get_tpm2();
> > - if (ret != EFI_SUCCESS)
> > - return ret;
> > + if (tcg2_platform_get_tpm2())
> > + return EFI_SECURITY_VIOLATION;
> >
> > ret = measure_event(dev, 4, EV_EFI_ACTION,
> > strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
> > @@ -1437,9 +1445,10 @@ efi_tcg2_notify_exit_boot_services(struct efi_event
> > *event, void *context)
> > goto out;
> > }
> >
> > - ret = tcg2_platform_get_tpm2();
> > - if (ret != EFI_SUCCESS)
> > + if (tcg2_platform_get_tpm2()) {
> > + ret = EFI_SECURITY_VIOLATION;
> > goto out;
> > + }
> >
> > ret =
Re: [PATCH 1/7] tpm: fix the return code, if the eventlog buffer is full
On 22.06.24 16:35, Ilias Apalodimas wrote:
We currently return 'No space left on device' if the eventlong buffer
we allocated is not enough. On a similar check later on that function
during the call to tcg2_log_init() we return 'No buffer space
available'. So switch both error codes to -ENOBUFS since we are always
checking a buffer and not a device.
Fixes: commit 97707f12fdab ("tpm: Support boot measurements")
Signed-off-by: Ilias Apalodimas
Reviewed-by: Heinrich Schuchardt
---
lib/tpm-v2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index a67daed2f3c1..91526af33acb 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -554,7 +554,7 @@ int tcg2_log_prepare_buffer(struct udevice *dev, struct
tcg2_event_log *elog,
if (elog->log_size) {
if (log.found) {
if (elog->log_size < log.log_position)
- return -ENOSPC;
+ return -ENOBUFS;
/*
* Copy the discovered log into the user buffer
Re: [PATCH 3/7] efi_loader: remove duplicate TCG algo definitions
On 22.06.24 16:35, Ilias Apalodimas wrote:
commit 97707f12fdab ("tpm: Support boot measurements") moved some of the
EFI TCG code to the TPM subsystem. Those definitions are now in tpm-v2.h.
Let's remove the duplicate entries
The constants are not duplicate but unused.
97707f12fdab introduced different constant names (TPM2_ALG_*).
Otherwise
Reviewed-by: Heinrich Schuchardt
Signed-off-by: Ilias Apalodimas
---
include/efi_tcg2.h | 8
1 file changed, 8 deletions(-)
diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index a75b5a35b6e7..54490969b2d1 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -25,14 +25,6 @@
#define PE_COFF_IMAGE 0x0010
#define EFI_TCG2_MAX_PCR_INDEX 23
-
-/* Algorithm Registry */
-#define EFI_TCG2_BOOT_HASH_ALG_SHA10x0001
-#define EFI_TCG2_BOOT_HASH_ALG_SHA256 0x0002
-#define EFI_TCG2_BOOT_HASH_ALG_SHA384 0x0004
-#define EFI_TCG2_BOOT_HASH_ALG_SHA512 0x0008
-#define EFI_TCG2_BOOT_HASH_ALG_SM3_256 0x0010
-
#define EFI_TCG2_FINAL_EVENTS_TABLE_VERSION 1
#define TPM2_EVENT_LOG_SIZE CONFIG_EFI_TCG2_PROTOCOL_EVENTLOG_SIZE
Re: [PATCH 2/7] efi_loader: fix the return values on efi_tcg
On 22.06.24 16:35, Ilias Apalodimas wrote:
A while back we moved the core functions of the EFI TCG protocol to the
TPM APIs in order for them to be used with bootm, booti etc.
Some prototypes changed from returning efi_status_t to int, which is more
appropriate for the non-EFI APIs. However, some of the EFI callsites never
changed and we ended up assigning the int value to efi_status_t.
This is unlikely to cause any problems, apart from returning invalid
values on failures and violating the EFI spec. Let's fix them
by looking at the new return code and map it to the proper EFI return
code on failures.
Fixes: commit 97707f12fdab ("tpm: Support boot measurements")
Fixes: commit d6b55a420cfc ("efi_loader: startup the tpm device when installing the
protocol")
Signed-off-by: Ilias Apalodimas
---
lib/efi_loader/efi_tcg2.c | 121 --
1 file changed, 64 insertions(+), 57 deletions(-)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index d56bd5657c8a..10c09caac35a 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -257,8 +257,8 @@ efi_tcg2_get_capability(struct efi_tcg2_protocol *this,
capability->protocol_version.major = 1;
capability->protocol_version.minor = 1;
- efi_ret = tcg2_platform_get_tpm2();
- if (efi_ret != EFI_SUCCESS) {
+ ret = tcg2_platform_get_tpm2();
+ if (ret) {
capability->supported_event_logs = 0;
capability->hash_algorithm_bitmap = 0;
capability->tpm_present_flag = false;
@@ -353,8 +353,7 @@ efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this,
goto out;
}
- ret = tcg2_platform_get_tpm2();
- if (ret != EFI_SUCCESS) {
+ if (tcg2_platform_get_tpm2()) {
event_log_location = NULL;
event_log_last_entry = NULL;
*event_log_truncated = false;
@@ -389,7 +388,7 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64
efi_size,
void *new_efi = NULL;
u8 hash[TPM2_SHA512_DIGEST_SIZE];
struct udevice *dev;
- efi_status_t ret;
+ efi_status_t ret = EFI_SUCCESS;
u32 active;
int i;
@@ -404,12 +403,13 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64
efi_size,
goto out;
}
- ret = tcg2_platform_get_tpm2();
- if (ret != EFI_SUCCESS)
+ if (tcg2_platform_get_tpm2()) {
+ ret = EFI_DEVICE_ERROR;
goto out;
+ }
- ret = tcg2_get_active_pcr_banks(dev, );
- if (ret != EFI_SUCCESS) {
+ if (tcg2_get_active_pcr_banks(dev, )) {
+ ret = EFI_DEVICE_ERROR;
goto out;
}
@@ -473,12 +473,12 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64
efi_size,
IMAGE_DOS_HEADER *dos;
IMAGE_NT_HEADERS32 *nt;
struct efi_handler *handler;
+ int rc;
if (!is_tcg2_protocol_installed())
return EFI_SUCCESS;
- ret = tcg2_platform_get_tpm2();
- if (ret != EFI_SUCCESS)
+ if (tcg2_platform_get_tpm2())
return EFI_SECURITY_VIOLATION;
switch (handle->image_type) {
@@ -502,9 +502,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
if (ret != EFI_SUCCESS)
return ret;
- ret = tcg2_pcr_extend(dev, pcr_index, _list);
- if (ret != EFI_SUCCESS)
- return ret;
+ rc = tcg2_pcr_extend(dev, pcr_index, _list);
+ if (rc)
+ return EFI_DEVICE_ERROR;
ret = efi_search_protocol(>header,
_guid_loaded_image_device_path, );
@@ -574,9 +574,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol
*this, u64 flags,
struct efi_tcg2_event *efi_tcg_event)
{
struct udevice *dev;
- efi_status_t ret;
+ efi_status_t ret = EFI_SUCCESS;
u32 event_type, pcr_index, event_size;
struct tpml_digest_values digest_list;
+ int rc = 0;
EFI_ENTRY("%p, %llu, %llu, %llu, %p", this, flags, data_to_hash,
data_to_hash_len, efi_tcg_event);
@@ -586,9 +587,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol
*this, u64 flags,
goto out;
}
- ret = tcg2_platform_get_tpm2();
- if (ret != EFI_SUCCESS)
+ if (tcg2_platform_get_tpm2()) {
+ ret = EFI_DEVICE_ERROR;
goto out;
+ }
if (efi_tcg_event->size < efi_tcg_event->header.header_size +
sizeof(u32)) {
@@ -621,8 +623,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol
*this, u64 flags,
ret = tcg2_hash_pe_image((void *)(uintptr_t)data_to_hash,
data_to_hash_len, _list);
} else {
- ret = tcg2_create_digest(dev, (u8 *)(uintptr_t)data_to_hash,
-
Re: [PATCH 5/7] efi_loader: remove unneeded header files
On 22.06.24 16:35, Ilias Apalodimas wrote: efi_tcg2.h already includes tpm-v2.h. Remove it Signed-off-by: Ilias Apalodimas Reviewed by: Heinrich Schuchardt --- lib/efi_loader/efi_tcg2.c | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 10c09caac35a..c654d2cbd704 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include
Re: [PATCH 4/7] tpm: Move TCG into a separate library
On 22.06.24 16:35, Ilias Apalodimas wrote:
commit 97707f12fdab ("tpm: Support boot measurements") moved out code
from the EFI subsystem into the TPM one to support measurements when
booting with !EFI.
Those were moved directly into the TPM subsystem and in the tpm-v2.c
library. In hindsight, it would have been better to move it in new
files since the TCG2 is governed by its own spec and it's cleaner
when we want to enable certian parts of the TPM functionality.
Nits:
%s/certian/certain/
So let's create a header file and another library and move the TCG
specific bits there.
Signed-off-by: Ilias Apalodimas
---
boot/bootm.c | 1 +
include/efi_tcg2.h | 1 +
include/tpm-v2.h | 474 +-
include/tpm_tcg2.h | 336 ++
lib/Makefile | 2 +
lib/tpm-v2.c | 676 +--
lib/tpm_tcg2.c | 696 +
The patch series were easier to review if moving header definitions were
separated from moving implementations.
This patch contains changes that are not described in the commit
message, e.g.
if (elog->log_size) {
if (log.found) {
if (elog->log_size < log.log_position)
- return -ENOBUFS;
+ return -ENOSPC;
I guess you wanted to put this into patch 1.
Please, separate the patches adequately.
+ * Copyright (c) 2020 Linaro
+ * Copyright (c) 2023 Linaro Limited
The copyright lines look inconsistent. Linaro Limited exists under this
name since April 13th, 2010. Is the 2020 copyright for a different company?
7 files changed, 1114 insertions(+), 1072 deletions(-)
create mode 100644 include/tpm_tcg2.h
create mode 100644 lib/tpm_tcg2.c
diff --git a/boot/bootm.c b/boot/bootm.c
index 9879e1bba4eb..395b42cccd88 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#if defined(CONFIG_CMD_USB)
#include
#endif
diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index 54490969b2d1..8dfb1bc9527b 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -18,6 +18,7 @@
#include
#include
+#include
/* TPMV2 only */
#define TCG2_EVENT_LOG_FORMAT_TCG_2 0x0002
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index c9d5cb6d3e5a..c176e04c9952 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -26,14 +26,13 @@ struct udevice;
#define TPM2_SHA512_DIGEST_SIZE 64
#define TPM2_SM3_256_DIGEST_SIZE 32
+#define TPM2_HDR_LEN 10
+
#define TPM2_MAX_PCRS 32
#define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
#define TPM2_MAX_CAP_BUFFER 1024
#define TPM2_MAX_TPM_PROPERTIES ((TPM2_MAX_CAP_BUFFER - sizeof(u32) /*
TPM2_CAP */ - \
sizeof(u32)) / sizeof(struct
tpms_tagged_property))
-
-#define TPM2_HDR_LEN 10
-
/*
* We deviate from this draft of the specification by increasing the value of
* TPM2_NUM_PCR_BANKS from 3 to 16 to ensure compatibility with TPM2
@@ -55,211 +54,6 @@ struct udevice;
#define TPM2_PT_MAX_COMMAND_SIZE (u32)(TPM2_PT_FIXED + 30)
#define TPM2_PT_MAX_RESPONSE_SIZE (u32)(TPM2_PT_FIXED + 31)
-/*
- * event types, cf.
- * "TCG Server Management Domain Firmware Profile Specification",
- * rev 1.00, 2020-05-01
- */
-#define EV_POST_CODE ((u32)0x0001)
-#define EV_NO_ACTION ((u32)0x0003)
-#define EV_SEPARATOR ((u32)0x0004)
-#define EV_ACTION ((u32)0x0005)
-#define EV_TAG ((u32)0x0006)
-#define EV_S_CRTM_CONTENTS ((u32)0x0007)
-#define EV_S_CRTM_VERSION ((u32)0x0008)
-#define EV_CPU_MICROCODE ((u32)0x0009)
-#define EV_PLATFORM_CONFIG_FLAGS ((u32)0x000A)
-#define EV_TABLE_OF_DEVICES((u32)0x000B)
-#define EV_COMPACT_HASH((u32)0x000C)
-
-/*
- * event types, cf.
- * "TCG PC Client Platform Firmware Profile Specification", Family "2.0"
- * Level 00 Version 1.05 Revision 23, May 7, 2021
- */
-#define EV_EFI_EVENT_BASE ((u32)0x8000)
-#define EV_EFI_VARIABLE_DRIVER_CONFIG ((u32)0x8001)
-#define EV_EFI_VARIABLE_BOOT ((u32)0x8002)
-#define EV_EFI_BOOT_SERVICES_APPLICATION ((u32)0x8003)
-#define EV_EFI_BOOT_SERVICES_DRIVER((u32)0x8004)
-#define EV_EFI_RUNTIME_SERVICES_DRIVER ((u32)0x8005)
-#define EV_EFI_GPT_EVENT ((u32)0x8006)
-#define EV_EFI_ACTION ((u32)0x8007)
-#define EV_EFI_PLATFORM_FIRMWARE_BLOB ((u32)0x8008)
-#define EV_EFI_HANDOFF_TABLES ((u32)0x8009)
-#define EV_EFI_PLATFORM_FIRMWARE_BLOB2 ((u32)0x800A)
-#define EV_EFI_HANDOFF_TABLES2
Re: [PATCH 1/1] efi_selftest: can't have measured device-tree with kaslr-seed
On Sat, 22 Jun 2024 at 17:58, Heinrich Schuchardt
wrote:
>
> On 18.06.24 17:54, Ilias Apalodimas wrote:
> > On Tue, 18 Jun 2024 at 15:24, Heinrich Schuchardt
> > wrote:
> >>
> >> Test that we don't have a /chosen/kaslr-seed property if we measure the
> >> device-tree.
> >>
> >> Signed-off-by: Heinrich Schuchardt
> >> ---
> >> lib/efi_selftest/efi_selftest_fdt.c | 7 +++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/lib/efi_selftest/efi_selftest_fdt.c
> >> b/lib/efi_selftest/efi_selftest_fdt.c
> >> index aa3b13ae3ab..066d9581432 100644
> >> --- a/lib/efi_selftest/efi_selftest_fdt.c
> >> +++ b/lib/efi_selftest/efi_selftest_fdt.c
> >> @@ -227,6 +227,13 @@ static int execute(void)
> >> return EFI_ST_FAILURE;
> >> }
> >> }
> >> + if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
> >> + str = get_property(u"kaslr-seed", u"chosen");
> >> + if (str) {
> >> + efi_st_error("kaslr-seed with measured fdt\n");
> >> + return EFI_ST_FAILURE;
> >
> > When does this run? efi_try_purge_kaslr_seed() tries to remove the
> > kaslr-seed before measuring a DT. Are we safe enavbling the check
> > here?
>
> do_efi_selftest() is called after efi_install_fdt(). efi_install_fdt()
> invokes efi_try_purge_kaslr_seed().
>
> We would get an error here if efi_try_purge_kaslr_seed() were removed
> and measuring the DTB enabled.
>
> Best regards
Thanks! That's what I was wondering
Reviewed-by: Ilias Apalodimas
>
> Heinrich
>
> >
> > Thanks
> > /Ilias
> >> + }
> >> + }
> >> if (IS_ENABLED(CONFIG_RISCV)) {
> >> u32 fdt_hartid;
> >>
> >> --
> >> 2.45.1
> >>
>
Re: [PATCH 1/1] efi_selftest: can't have measured device-tree with kaslr-seed
On 18.06.24 17:54, Ilias Apalodimas wrote:
On Tue, 18 Jun 2024 at 15:24, Heinrich Schuchardt
wrote:
Test that we don't have a /chosen/kaslr-seed property if we measure the
device-tree.
Signed-off-by: Heinrich Schuchardt
---
lib/efi_selftest/efi_selftest_fdt.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_fdt.c
b/lib/efi_selftest/efi_selftest_fdt.c
index aa3b13ae3ab..066d9581432 100644
--- a/lib/efi_selftest/efi_selftest_fdt.c
+++ b/lib/efi_selftest/efi_selftest_fdt.c
@@ -227,6 +227,13 @@ static int execute(void)
return EFI_ST_FAILURE;
}
}
+ if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
+ str = get_property(u"kaslr-seed", u"chosen");
+ if (str) {
+ efi_st_error("kaslr-seed with measured fdt\n");
+ return EFI_ST_FAILURE;
When does this run? efi_try_purge_kaslr_seed() tries to remove the
kaslr-seed before measuring a DT. Are we safe enavbling the check
here?
do_efi_selftest() is called after efi_install_fdt(). efi_install_fdt()
invokes efi_try_purge_kaslr_seed().
We would get an error here if efi_try_purge_kaslr_seed() were removed
and measuring the DTB enabled.
Best regards
Heinrich
Thanks
/Ilias
+ }
+ }
if (IS_ENABLED(CONFIG_RISCV)) {
u32 fdt_hartid;
--
2.45.1
Re: [PATCH 1/2] tpm: Fix return code, if the eventlog buffer is full
On Thu, 20 Jun 2024 at 22:19, Ilias Apalodimas
wrote:
>
> On Thu, 20 Jun 2024 at 22:16, Ilias Apalodimas
> wrote:
> >
> > We currently return 'No space left on device' if the eventlong buffer
> > we allocated is not enough. On a similar check later on that function
> > during the call to tcg2_log_init() we return 'No buffer space
> > available'. So switch both error codes to -ENOBUFS since we are always
> > checking a buffer and not a device.
> >
> > Fixes: 97707f12fdab ("tpm: Support boot measurements")
>
> and now I wonder why checkpatch didn't warn on this. There's a
> 'commit' missing after the Fixes tag. Sorry!
>
> > Signed-off-by: Ilias Apalodimas
> > ---
> > lib/tpm-v2.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> > index a67daed2f3c1..91526af33acb 100644
> > --- a/lib/tpm-v2.c
> > +++ b/lib/tpm-v2.c
> > @@ -554,7 +554,7 @@ int tcg2_log_prepare_buffer(struct udevice *dev, struct
> > tcg2_event_log *elog,
> > if (elog->log_size) {
> > if (log.found) {
> > if (elog->log_size < log.log_position)
> > - return -ENOSPC;
> > + return -ENOBUFS;
> >
> > /*
> > * Copy the discovered log into the user
> > buffer
> > --
> > 2.45.2
> >
This is now included in
https://lore.kernel.org/u-boot/20240622143601.187723-1-ilias.apalodi...@linaro.org/T/#t
Cheers
/Ilias
[PATCH 7/7] tpm: allow the user to select the compiled algorithms
Simon reports that after enabling all algorithms on the TPM some boards
fail since they don't have enough storage to accommodate the ~5KB growth.
The choice of hash algorithms are determined by the platform and the TPM
configuration. Failing to cap a PCR in a bank which the platform left
active is a security vulnerability. It might allow unsealing of secrets
if an attacker can replay a good set of measurements into an unused bank.
If MEASURED_BOOT or EFI_TCG2_PROTOCOL is enabled our Kconfig will enable
all supported hashing algorithms. We still want to allow users to add a
TPM and not enable measured boot via EFI or bootm though and at the same
time, control the compiled algorithms for size reasons.
So let's add a function tpm2_allow_extend() which checks the TPM active
PCRs banks against the one U-Boot was compiled with.
If all the active PCRs banks are not enabled refuse to extend a PCR but
otherwise leave the TPM functional.
It's worth noting that this is only added on TPM2.0, since TPM1.2 is
lacking a lot of code at the moment to read the available PCRs.
We unconditionally enable SHA1 when a TPM is selected, which is the only
hashing algorithm v1.2 supports.
Signed-off-by: Ilias Apalodimas
---
boot/Kconfig | 4
include/tpm-v2.h | 59 +++-
lib/Kconfig | 6 ++---
lib/tpm-v2.c | 40 +---
4 files changed, 87 insertions(+), 22 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig
index 6f3096c15a6f..b061891e109c 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT
config MEASURED_BOOT
bool "Measure boot images and configuration when booting without EFI"
depends on HASH && TPM_V2
+ select SHA1
+ select SHA256
+ select SHA384
+ select SHA512
help
This option enables measurement of the boot process when booting
without UEFI . Measurement involves creating cryptographic hashes
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index eac04d1c6831..fccb07fa4695 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -277,48 +277,40 @@ struct digest_info {
#define TCG2_BOOT_HASH_ALG_SM3_256 0x0010
static const struct digest_info hash_algo_list[] = {
+#if IS_ENABLED(CONFIG_SHA1)
{
"sha1",
TPM2_ALG_SHA1,
TCG2_BOOT_HASH_ALG_SHA1,
TPM2_SHA1_DIGEST_SIZE,
},
+#endif
+#if IS_ENABLED(CONFIG_SHA256)
{
"sha256",
TPM2_ALG_SHA256,
TCG2_BOOT_HASH_ALG_SHA256,
TPM2_SHA256_DIGEST_SIZE,
},
+#endif
+#if IS_ENABLED(CONFIG_SHA384)
{
"sha384",
TPM2_ALG_SHA384,
TCG2_BOOT_HASH_ALG_SHA384,
TPM2_SHA384_DIGEST_SIZE,
},
+#endif
+#if IS_ENABLED(CONFIG_SHA512)
{
"sha512",
TPM2_ALG_SHA512,
TCG2_BOOT_HASH_ALG_SHA512,
TPM2_SHA512_DIGEST_SIZE,
},
+#endif
};
-static inline u16 tpm2_algorithm_to_len(enum tpm2_algorithms a)
-{
- switch (a) {
- case TPM2_ALG_SHA1:
- return TPM2_SHA1_DIGEST_SIZE;
- case TPM2_ALG_SHA256:
- return TPM2_SHA256_DIGEST_SIZE;
- case TPM2_ALG_SHA384:
- return TPM2_SHA384_DIGEST_SIZE;
- case TPM2_ALG_SHA512:
- return TPM2_SHA512_DIGEST_SIZE;
- default:
- return 0;
- }
-}
-
/* NV index attributes */
enum tpm_index_attrs {
TPMA_NV_PPWRITE = 1UL << 0,
@@ -711,6 +703,41 @@ enum tpm2_algorithms tpm2_name_to_algorithm(const char
*name);
*/
const char *tpm2_algorithm_name(enum tpm2_algorithms);
+/**
+ * tpm2_algorithm_to_len() - Return an algorithm length for supported
algorithm id
+ *
+ * @algorithm_id: algorithm defined in enum tpm2_algorithms
+ * Return: len or 0 if not supported
+ */
+u16 tpm2_algorithm_to_len(enum tpm2_algorithms algo);
+
+/*
+ * When measured boot is enabled via EFI or bootX commands all the algorithms
+ * above are selected by our Kconfigs. Due to U-Boots nature of being small
there
+ * are cases where we need some functionality from the TPM -- e.g storage or
RNG
+ * but we don't want to support measurements.
+ *
+ * The choice of hash algorithms are determined by the platform and the TPM
+ * configuration. Failing to cap a PCR in a bank which the platform left
+ * active is a security vulnerability. It permits the unsealing of secrets
+ * if an attacker can replay a good set of measurements into an unused bank.
+ *
+ * On top of that a previous stage bootloader (e.g TF-A), migh pass an eventlog
+ * since it doesn't have a TPM driver, which U-Boot needs to replace. The
algorit h
+ * choice is a compile time option in that case and we need to make sure we
conform.
+ *
+ * Add a variable here that sums the supported algorithms U-Boot was
[PATCH 6/7] tpm: Untangle tpm2_get_pcr_info()
This function was used on measured boot to retrieve the number of active
PCR banks and was designed to work with the TCG protocols.
Since we now have the need to retrieve the active PCRs outside the
measured boot context -- e.g use the in the command line, decouple the
function.
Create one that will only adheres to TCG TSS2.0 [0] specification called
tpm2_get_pcr_info() which can be used by the TPM2.0 APIs and a new one that
is called from the measured boot context called tcg2_get_pcr_info()
[0]
https://trustedcomputinggroup.org/wp-content/uploads/TSS_Overview_Common_Structures_Version-0.9_Revision-03_Review_030918.pdf
Signed-off-by: Ilias Apalodimas
---
include/tpm-v2.h | 16 ++---
include/tpm_tcg2.h| 13 +++
lib/efi_loader/efi_tcg2.c | 2 +-
lib/tpm-v2.c | 73 +--
lib/tpm_tcg2.c| 38 +++-
5 files changed, 86 insertions(+), 56 deletions(-)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index c176e04c9952..eac04d1c6831 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -521,14 +521,11 @@ u32 tpm2_get_capability(struct udevice *dev, u32
capability, u32 property,
* tpm2_get_pcr_info() - get the supported, active PCRs and number of banks
*
* @dev: TPM device
- * @supported_pcr: bitmask with the algorithms supported
- * @active_pcr:bitmask with the active algorithms
- * @pcr_banks: number of PCR banks
+ * @pcrs: struct tpml_pcr_selection of available PCRs
*
* @return 0 on success, code of operation or negative errno on failure
*/
-int tpm2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr,
- u32 *pcr_banks);
+int tpm2_get_pcr_info(struct udevice *dev, struct tpml_pcr_selection *pcrs);
/**
* Issue a TPM2_DictionaryAttackLockReset command.
@@ -714,4 +711,13 @@ enum tpm2_algorithms tpm2_name_to_algorithm(const char
*name);
*/
const char *tpm2_algorithm_name(enum tpm2_algorithms);
+/**
+ * tpm2_is_active_pcr() - check the pcr_select. If at least one of the PCRs
+ * supports the algorithm add it on the active ones
+ *
+ * @selection: PCR selection structure
+ * Return: True if the algorithm is active
+ */
+bool tpm2_is_active_pcr(struct tpms_pcr_selection *selection);
+
#endif /* __TPM_V2_H */
diff --git a/include/tpm_tcg2.h b/include/tpm_tcg2.h
index 77afdbb03e77..a0557898865e 100644
--- a/include/tpm_tcg2.h
+++ b/include/tpm_tcg2.h
@@ -82,6 +82,19 @@ struct tcg_pcr_event {
u8 event[];
} __packed;
+/**
+ * tcg2_get_pcr_info() - get the supported, active PCRs and number of banks
+ *
+ * @dev: TPM device
+ * @supported_pcr: bitmask with the algorithms supported
+ * @active_pcr:bitmask with the active algorithms
+ * @pcr_banks: number of PCR banks
+ *
+ * @return 0 on success, code of operation or negative errno on failure
+ */
+int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr,
+ u32 *pcr_banks);
+
/**
* Crypto Agile Log Entry Format
*
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index c654d2cbd704..2d2c015db053 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -279,7 +279,7 @@ efi_tcg2_get_capability(struct efi_tcg2_protocol *this,
/* Supported and active PCRs */
capability->hash_algorithm_bitmap = 0;
capability->active_pcr_banks = 0;
- ret = tpm2_get_pcr_info(dev, >hash_algorithm_bitmap,
+ ret = tcg2_get_pcr_info(dev, >hash_algorithm_bitmap,
>active_pcr_banks,
>number_of_pcr_banks);
if (ret) {
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 62ab804b4b38..36aace03cf4e 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -395,48 +395,26 @@ static int tpm2_get_num_pcr(struct udevice *dev, u32
*num_pcr)
return 0;
}
-static bool tpm2_is_active_pcr(struct tpms_pcr_selection *selection)
-{
- int i;
-
- /*
-* check the pcr_select. If at least one of the PCRs supports the
-* algorithm add it on the active ones
-*/
- for (i = 0; i < selection->size_of_select; i++) {
- if (selection->pcr_select[i])
- return true;
- }
-
- return false;
-}
-
-int tpm2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr,
- u32 *pcr_banks)
+int tpm2_get_pcr_info(struct udevice *dev, struct tpml_pcr_selection *pcrs)
{
u8 response[(sizeof(struct tpms_capability_data) -
offsetof(struct tpms_capability_data, data))];
- struct tpml_pcr_selection pcrs;
u32 num_pcr;
size_t i;
u32 ret;
- *supported_pcr = 0;
- *active_pcr = 0;
- *pcr_banks = 0;
- memset(response, 0, sizeof(response));
ret = tpm2_get_capability(dev,
[PATCH 5/7] efi_loader: remove unneeded header files
efi_tcg2.h already includes tpm-v2.h. Remove it Signed-off-by: Ilias Apalodimas --- lib/efi_loader/efi_tcg2.c | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 10c09caac35a..c654d2cbd704 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include -- 2.45.2
[PATCH 4/7] tpm: Move TCG into a separate library
commit 97707f12fdab ("tpm: Support boot measurements") moved out code
from the EFI subsystem into the TPM one to support measurements when
booting with !EFI.
Those were moved directly into the TPM subsystem and in the tpm-v2.c
library. In hindsight, it would have been better to move it in new
files since the TCG2 is governed by its own spec and it's cleaner
when we want to enable certian parts of the TPM functionality.
So let's create a header file and another library and move the TCG
specific bits there.
Signed-off-by: Ilias Apalodimas
---
boot/bootm.c | 1 +
include/efi_tcg2.h | 1 +
include/tpm-v2.h | 474 +-
include/tpm_tcg2.h | 336 ++
lib/Makefile | 2 +
lib/tpm-v2.c | 676 +--
lib/tpm_tcg2.c | 696 +
7 files changed, 1114 insertions(+), 1072 deletions(-)
create mode 100644 include/tpm_tcg2.h
create mode 100644 lib/tpm_tcg2.c
diff --git a/boot/bootm.c b/boot/bootm.c
index 9879e1bba4eb..395b42cccd88 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#if defined(CONFIG_CMD_USB)
#include
#endif
diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index 54490969b2d1..8dfb1bc9527b 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -18,6 +18,7 @@
#include
#include
+#include
/* TPMV2 only */
#define TCG2_EVENT_LOG_FORMAT_TCG_2 0x0002
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index c9d5cb6d3e5a..c176e04c9952 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -26,14 +26,13 @@ struct udevice;
#define TPM2_SHA512_DIGEST_SIZE64
#define TPM2_SM3_256_DIGEST_SIZE 32
+#define TPM2_HDR_LEN 10
+
#define TPM2_MAX_PCRS 32
#define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
#define TPM2_MAX_CAP_BUFFER 1024
#define TPM2_MAX_TPM_PROPERTIES ((TPM2_MAX_CAP_BUFFER - sizeof(u32) /*
TPM2_CAP */ - \
sizeof(u32)) / sizeof(struct
tpms_tagged_property))
-
-#define TPM2_HDR_LEN 10
-
/*
* We deviate from this draft of the specification by increasing the value of
* TPM2_NUM_PCR_BANKS from 3 to 16 to ensure compatibility with TPM2
@@ -55,211 +54,6 @@ struct udevice;
#define TPM2_PT_MAX_COMMAND_SIZE (u32)(TPM2_PT_FIXED + 30)
#define TPM2_PT_MAX_RESPONSE_SIZE (u32)(TPM2_PT_FIXED + 31)
-/*
- * event types, cf.
- * "TCG Server Management Domain Firmware Profile Specification",
- * rev 1.00, 2020-05-01
- */
-#define EV_POST_CODE ((u32)0x0001)
-#define EV_NO_ACTION ((u32)0x0003)
-#define EV_SEPARATOR ((u32)0x0004)
-#define EV_ACTION ((u32)0x0005)
-#define EV_TAG ((u32)0x0006)
-#define EV_S_CRTM_CONTENTS ((u32)0x0007)
-#define EV_S_CRTM_VERSION ((u32)0x0008)
-#define EV_CPU_MICROCODE ((u32)0x0009)
-#define EV_PLATFORM_CONFIG_FLAGS ((u32)0x000A)
-#define EV_TABLE_OF_DEVICES((u32)0x000B)
-#define EV_COMPACT_HASH((u32)0x000C)
-
-/*
- * event types, cf.
- * "TCG PC Client Platform Firmware Profile Specification", Family "2.0"
- * Level 00 Version 1.05 Revision 23, May 7, 2021
- */
-#define EV_EFI_EVENT_BASE ((u32)0x8000)
-#define EV_EFI_VARIABLE_DRIVER_CONFIG ((u32)0x8001)
-#define EV_EFI_VARIABLE_BOOT ((u32)0x8002)
-#define EV_EFI_BOOT_SERVICES_APPLICATION ((u32)0x8003)
-#define EV_EFI_BOOT_SERVICES_DRIVER((u32)0x8004)
-#define EV_EFI_RUNTIME_SERVICES_DRIVER ((u32)0x8005)
-#define EV_EFI_GPT_EVENT ((u32)0x8006)
-#define EV_EFI_ACTION ((u32)0x8007)
-#define EV_EFI_PLATFORM_FIRMWARE_BLOB ((u32)0x8008)
-#define EV_EFI_HANDOFF_TABLES ((u32)0x8009)
-#define EV_EFI_PLATFORM_FIRMWARE_BLOB2 ((u32)0x800A)
-#define EV_EFI_HANDOFF_TABLES2 ((u32)0x800B)
-#define EV_EFI_VARIABLE_BOOT2 ((u32)0x800C)
-#define EV_EFI_HCRTM_EVENT ((u32)0x8010)
-#define EV_EFI_VARIABLE_AUTHORITY ((u32)0x80E0)
-#define EV_EFI_SPDM_FIRMWARE_BLOB ((u32)0x80E1)
-#define EV_EFI_SPDM_FIRMWARE_CONFIG((u32)0x80E2)
-
-#define EFI_CALLING_EFI_APPLICATION \
- "Calling EFI Application from Boot Option"
-#define EFI_RETURNING_FROM_EFI_APPLICATION \
- "Returning from EFI Application from Boot Option"
-#define EFI_EXIT_BOOT_SERVICES_INVOCATION \
- "Exit Boot Services Invocation"
-#define EFI_EXIT_BOOT_SERVICES_FAILED \
- "Exit Boot Services Returned with Failure"
-#define EFI_EXIT_BOOT_SERVICES_SUCCEEDED\
- "Exit Boot Services Returned with Success"
-#define
[PATCH 3/7] efi_loader: remove duplicate TCG algo definitions
commit 97707f12fdab ("tpm: Support boot measurements") moved some of the
EFI TCG code to the TPM subsystem. Those definitions are now in tpm-v2.h.
Let's remove the duplicate entries
Signed-off-by: Ilias Apalodimas
---
include/efi_tcg2.h | 8
1 file changed, 8 deletions(-)
diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index a75b5a35b6e7..54490969b2d1 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -25,14 +25,6 @@
#define PE_COFF_IMAGE 0x0010
#define EFI_TCG2_MAX_PCR_INDEX 23
-
-/* Algorithm Registry */
-#define EFI_TCG2_BOOT_HASH_ALG_SHA10x0001
-#define EFI_TCG2_BOOT_HASH_ALG_SHA256 0x0002
-#define EFI_TCG2_BOOT_HASH_ALG_SHA384 0x0004
-#define EFI_TCG2_BOOT_HASH_ALG_SHA512 0x0008
-#define EFI_TCG2_BOOT_HASH_ALG_SM3_256 0x0010
-
#define EFI_TCG2_FINAL_EVENTS_TABLE_VERSION 1
#define TPM2_EVENT_LOG_SIZE CONFIG_EFI_TCG2_PROTOCOL_EVENTLOG_SIZE
--
2.45.2
[PATCH 2/7] efi_loader: fix the return values on efi_tcg
A while back we moved the core functions of the EFI TCG protocol to the
TPM APIs in order for them to be used with bootm, booti etc.
Some prototypes changed from returning efi_status_t to int, which is more
appropriate for the non-EFI APIs. However, some of the EFI callsites never
changed and we ended up assigning the int value to efi_status_t.
This is unlikely to cause any problems, apart from returning invalid
values on failures and violating the EFI spec. Let's fix them
by looking at the new return code and map it to the proper EFI return
code on failures.
Fixes: commit 97707f12fdab ("tpm: Support boot measurements")
Fixes: commit d6b55a420cfc ("efi_loader: startup the tpm device when installing
the protocol")
Signed-off-by: Ilias Apalodimas
---
lib/efi_loader/efi_tcg2.c | 121 --
1 file changed, 64 insertions(+), 57 deletions(-)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index d56bd5657c8a..10c09caac35a 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -257,8 +257,8 @@ efi_tcg2_get_capability(struct efi_tcg2_protocol *this,
capability->protocol_version.major = 1;
capability->protocol_version.minor = 1;
- efi_ret = tcg2_platform_get_tpm2();
- if (efi_ret != EFI_SUCCESS) {
+ ret = tcg2_platform_get_tpm2();
+ if (ret) {
capability->supported_event_logs = 0;
capability->hash_algorithm_bitmap = 0;
capability->tpm_present_flag = false;
@@ -353,8 +353,7 @@ efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this,
goto out;
}
- ret = tcg2_platform_get_tpm2();
- if (ret != EFI_SUCCESS) {
+ if (tcg2_platform_get_tpm2()) {
event_log_location = NULL;
event_log_last_entry = NULL;
*event_log_truncated = false;
@@ -389,7 +388,7 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64
efi_size,
void *new_efi = NULL;
u8 hash[TPM2_SHA512_DIGEST_SIZE];
struct udevice *dev;
- efi_status_t ret;
+ efi_status_t ret = EFI_SUCCESS;
u32 active;
int i;
@@ -404,12 +403,13 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64
efi_size,
goto out;
}
- ret = tcg2_platform_get_tpm2();
- if (ret != EFI_SUCCESS)
+ if (tcg2_platform_get_tpm2()) {
+ ret = EFI_DEVICE_ERROR;
goto out;
+ }
- ret = tcg2_get_active_pcr_banks(dev, );
- if (ret != EFI_SUCCESS) {
+ if (tcg2_get_active_pcr_banks(dev, )) {
+ ret = EFI_DEVICE_ERROR;
goto out;
}
@@ -473,12 +473,12 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64
efi_size,
IMAGE_DOS_HEADER *dos;
IMAGE_NT_HEADERS32 *nt;
struct efi_handler *handler;
+ int rc;
if (!is_tcg2_protocol_installed())
return EFI_SUCCESS;
- ret = tcg2_platform_get_tpm2();
- if (ret != EFI_SUCCESS)
+ if (tcg2_platform_get_tpm2())
return EFI_SECURITY_VIOLATION;
switch (handle->image_type) {
@@ -502,9 +502,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
if (ret != EFI_SUCCESS)
return ret;
- ret = tcg2_pcr_extend(dev, pcr_index, _list);
- if (ret != EFI_SUCCESS)
- return ret;
+ rc = tcg2_pcr_extend(dev, pcr_index, _list);
+ if (rc)
+ return EFI_DEVICE_ERROR;
ret = efi_search_protocol(>header,
_guid_loaded_image_device_path, );
@@ -574,9 +574,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol
*this, u64 flags,
struct efi_tcg2_event *efi_tcg_event)
{
struct udevice *dev;
- efi_status_t ret;
+ efi_status_t ret = EFI_SUCCESS;
u32 event_type, pcr_index, event_size;
struct tpml_digest_values digest_list;
+ int rc = 0;
EFI_ENTRY("%p, %llu, %llu, %llu, %p", this, flags, data_to_hash,
data_to_hash_len, efi_tcg_event);
@@ -586,9 +587,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol
*this, u64 flags,
goto out;
}
- ret = tcg2_platform_get_tpm2();
- if (ret != EFI_SUCCESS)
+ if (tcg2_platform_get_tpm2()) {
+ ret = EFI_DEVICE_ERROR;
goto out;
+ }
if (efi_tcg_event->size < efi_tcg_event->header.header_size +
sizeof(u32)) {
@@ -621,8 +623,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol
*this, u64 flags,
ret = tcg2_hash_pe_image((void *)(uintptr_t)data_to_hash,
data_to_hash_len, _list);
} else {
- ret = tcg2_create_digest(dev, (u8 *)(uintptr_t)data_to_hash,
-data_to_hash_len, _list);
+
[PATCH 1/7] tpm: fix the return code, if the eventlog buffer is full
We currently return 'No space left on device' if the eventlong buffer
we allocated is not enough. On a similar check later on that function
during the call to tcg2_log_init() we return 'No buffer space
available'. So switch both error codes to -ENOBUFS since we are always
checking a buffer and not a device.
Fixes: commit 97707f12fdab ("tpm: Support boot measurements")
Signed-off-by: Ilias Apalodimas
---
lib/tpm-v2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index a67daed2f3c1..91526af33acb 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -554,7 +554,7 @@ int tcg2_log_prepare_buffer(struct udevice *dev, struct
tcg2_event_log *elog,
if (elog->log_size) {
if (log.found) {
if (elog->log_size < log.log_position)
- return -ENOSPC;
+ return -ENOBUFS;
/*
* Copy the discovered log into the user buffer
--
2.45.2
[PATCH 0/7] The great TCG deduplication saga
Hi all, A while back some TPM measured boot code was moved out of EFI in order to support !EFI boot methods, e.g bootm, booti etc. Back then we decided to move the code in the TPM subsystem directly. In hindsight, we should have created a different library file that hosts all the TCG specific bits, but better late than never! Since the algorithms that the TPM supports are only known at runtime, we unconditionally enabled all hashing algorithms. Simon reported some breakage lately due to size limitations and he wanted to remove some of the supported algorithms from those configs. But that's not always safe depending on what the user expects the TPM to do. If MEASURED_BOOT or EFI_TCG2_PROTOCOL is enabled our Kconfig will enable all supported hashing algorithms. Nothing changes there. This is an attempt to allow users to add a TPM and not enable measured boot via EFI or bootm and at the same time, control the compiled algorithms for size reasons, without shooting themselves in the foot. Functionality has been added that checks the TPM active PCRs banks against the one U-Boot was compiled with. If all the active PCRs banks are not enabled refuse to extend a PCR but otherwise leave the TPM functional. patches #1, #2 have been reposted and are fixes from the code moving patches #3, #5 get rid of duplicat header entries patch #4 moves the TCG code out of the TPM in its own file patch #6 refactors a function so we can use it in both TCG & TPM now and finally patch #7 adds the desired functionality The u-boot CI seems happy, my internal CI that tests EFI measured boot in various scenarios is happy and the EFI eventlog hasn't changed at all pre/post patches. I haven't manged to test bootm etc, but that code hasn't changed at all and the CI tests are passing. Eddie any chance you can test it? Ilias Apalodimas (7): tpm: fix the return code, if the eventlog buffer is full efi_loader: fix the return values on efi_tcg efi_loader: remove duplicate TCG algo definitions tpm: Move TCG into a separate library efi_loader: remove unneeded header files tpm: Untangle tpm2_get_pcr_info() tpm: allow the user to select the compiled algorithms boot/Kconfig | 4 + boot/bootm.c | 1 + include/efi_tcg2.h| 9 +- include/tpm-v2.h | 541 +++ include/tpm_tcg2.h| 349 + lib/Kconfig | 6 +- lib/Makefile | 2 + lib/efi_loader/efi_tcg2.c | 124 +++--- lib/tpm-v2.c | 767 +++--- lib/tpm_tcg2.c| 732 10 files changed, 1335 insertions(+), 1200 deletions(-) create mode 100644 include/tpm_tcg2.h create mode 100644 lib/tpm_tcg2.c -- 2.45.2
Re: [PATCH v3 16/18] rockchip: Avoid #ifdefs in RK3399 SPL
Hi Kever,
On Fri, 21 Jun 2024 at 21:49, Kever Yang wrote:
>
> Hi Simon,
>
> On 2024/6/21 22:57, Simon Glass wrote:
> > Hi Jonas,
> >
> > On Fri, 21 Jun 2024 at 00:45, Jonas Karlman wrote:
> >> Hi Simon,
> >>
> >> On 2024-06-21 01:06, Simon Glass wrote:
> >>> The code here is confusing due to large blocks which are #ifdefed out.
> >>> Add a function phase_sdram_init() which returns whether SDRAM init
> >>> should happen in the current phase, using that as needed to control the
> >>> code flow.
> >>>
> >>> This increases code size by about 500 bytes in SPL when the cache is on,
> >>> since it must call the rather large rockchip_sdram_size() function.
> >>>
> >>> Signed-off-by: Simon Glass
> >>> ---
> >>>
> >>> Changes in v3:
> >>> - Split out the refactoring into a separate patch
> >>> - Drop the non-dcache optimisation, since the cache should normally be on
> >>>
> >>> drivers/ram/rockchip/sdram_rk3399.c | 47 -
> >>> 1 file changed, 26 insertions(+), 21 deletions(-)
> >>>
> >>> diff --git a/drivers/ram/rockchip/sdram_rk3399.c
> >>> b/drivers/ram/rockchip/sdram_rk3399.c
> >>> index 3c4e20f4e80..2f37dd712e7 100644
> >>> --- a/drivers/ram/rockchip/sdram_rk3399.c
> >>> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> >>> @@ -13,6 +13,7 @@
> >>> #include
> >>> #include
> >>> #include
> >>> +#include
> >>> #include
> >>> #include
> >>> #include
> >>> @@ -63,8 +64,6 @@ struct chan_info {
> >>> };
> >>>
> >>> struct dram_info {
> >>> -#if defined(CONFIG_TPL_BUILD) || \
> >>> - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
> >>>u32 pwrup_srefresh_exit[2];
> >>>struct chan_info chan[2];
> >>>struct clk ddr_clk;
> >>> @@ -75,7 +74,6 @@ struct dram_info {
> >>>struct rk3399_pmusgrf_regs *pmusgrf;
> >>>struct rk3399_ddr_cic_regs *cic;
> >>>const struct sdram_rk3399_ops *ops;
> >>> -#endif
> >>>struct ram_info info;
> >>>struct rk3399_pmugrf_regs *pmugrf;
> >>> };
> >>> @@ -92,9 +90,6 @@ struct sdram_rk3399_ops {
> >>>struct rk3399_sdram_params
> >>> *params);
> >>> };
> >>>
> >>> -#if defined(CONFIG_TPL_BUILD) || \
> >>> - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
> >>> -
> >>> struct rockchip_dmc_plat {
> >>> #if CONFIG_IS_ENABLED(OF_PLATDATA)
> >>>struct dtd_rockchip_rk3399_dmc dtplat;
> >>> @@ -191,6 +186,17 @@ struct io_setting {
> >>>},
> >>> };
> >>>
> >>> +/**
> >>> + * phase_sdram_init() - Check if this is the phase where SDRAM init
> >>> happens
> >>> + *
> >>> + * Returns: true to do SDRAM init in this phase, false to not
> >>> + */
> >>> +static bool phase_sdram_init(void)
> >>> +{
> >>> + return spl_phase() == PHASE_TPL ||
> >>> + (!IS_ENABLED(CONFIG_TPL) && !spl_in_proper());
> >>> +}
> >>> +
> >>> static struct io_setting *
> >>> lpddr4_get_io_settings(const struct rk3399_sdram_params *params, u32
> >>> mr5)
> >>> {
> >>> @@ -3024,7 +3030,7 @@ static int rk3399_dmc_of_to_plat(struct udevice
> >>> *dev)
> >>>struct rockchip_dmc_plat *plat = dev_get_plat(dev);
> >>>int ret;
> >>>
> >>> - if (!CONFIG_IS_ENABLED(OF_REAL))
> >>> + if (!CONFIG_IS_ENABLED(OF_REAL) || !phase_sdram_init())
> >>>return 0;
> >>>
> >>>ret = dev_read_u32_array(dev, "rockchip,sdram-params",
> >>> @@ -3138,22 +3144,24 @@ static int rk3399_dmc_init(struct udevice *dev)
> >>>
> >>>return 0;
> >>> }
> >>> -#endif
> >>>
> >>> static int rk3399_dmc_probe(struct udevice *dev)
> >>> {
> >>>struct dram_info *priv = dev_get_priv(dev);
> >>>
> >>> -#if defined(CONFIG_TPL_BUILD) || \
> >>> - (!defined(CONFIG_TPL) && defined(CONFIG_SPL_BUILD))
> >>> - if (rk3399_dmc_init(dev))
> >>> - return 0;
> >>> -#endif
> >>> - priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
> >>> - debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
> >>> - priv->info.base = CFG_SYS_SDRAM_BASE;
> >>> - priv->info.size =
> >>> - rockchip_sdram_size((phys_addr_t)>pmugrf->os_reg2);
> >>> + if (phase_sdram_init()) {
> >>> + if (rk3399_dmc_init(dev))
> >>> + return 0;
> >>> + } else {
> >>> + priv->pmugrf =
> >>> syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
> >>> + debug("%s: pmugrf = %p\n", __func__, priv->pmugrf);
> >>> + }
> >>> +
> >>> + if (!CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
> >> This check should be dropped, this driver intent is to initialize dram
> >> in first phase (normally TPL), and report the size to any other phase.
> > This whole patch can be dropped :-) Here I am just trying to avoid
> > code-size growth when the cache is off. But yes, it is not needed.
> >
> >> The main need for phase_sdram_init() and disable of DCACHE can probably
> >> be avoided if bob/kevin can move to use separate TPL and SPL instead of
> >> doing both dram
Re: [PATCH v4 00/14] Introduce the lwIP network stack
пт, 21 июн. 2024 г. в 21:42, Fabio Estevam : > > Hi Tim and Jerome, > > On Fri, Jun 21, 2024 at 1:08 PM Tim Harvey wrote: > > > I tried your to-upstream/v5-wip branch > > (042bea36eb9731079a3d7afffe3774d79e06ac5d) and it behaves the same. Do > > you have something else to try/test? > > Yes, when I tested older versions from Maxim I could never get lwIP to > work with i.MX. > > Jerome, > > Please try to run the lwIP series on any i.MX board, if possible. > > Thanks Packet not for us means that incoming packet DST MAC does not match to the MAC address inside lwip. I.e. to MAC address set into lwip when lwip_init was done. Original U-Boot network stack does not compare MACs but lwip does. There is something specific on this board, in general lwip with debug should print out MAC used during initialization. This MAC should match to the MAC from the incoming packet. -- Best regards, Maxim Uvarov
Re: [PATCH RFC] gpio: Fix probing of gpio-hogs
Tom Rini wrote:
Adding Marek, as the author of commit 48b3ecbedf82 ("gpio: Get rid of
gpio_hog_probe_all()").
Thanks! I don't claim this is the correct way to fix this, just that it
works.
Specifically, the two things I found that got gpio-hog working were
(a) adding an explicit probe instead of DM_FLAG_PROBE_AFTER_BIND in
gpio_post_bind(), or
(b) adding a .bind function in U_BOOT_DRIVER(mt7981_pinctrl) like
static int mtk_pinctrl_mt7981_bind(struct udevice *dev)
{
dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
return 0;
}
However, presumably (b) isn't right as it would (presumably) need
repeating in lots of other pinctrl drivers?
Best wishes,
Chris.
Re: [PATCH 1/3] pxe: Add debugging for booting
Hi
On Wed, Jun 19, 2024 at 2:34 PM Simon Glass wrote:
>
> Show which boot protocol is being used.
>
> Signed-off-by: Simon Glass
> ---
>
> boot/pxe_utils.c | 15 +++
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> index 4b22bb6f525..53ddb16be73 100644
> --- a/boot/pxe_utils.c
> +++ b/boot/pxe_utils.c
> @@ -4,6 +4,8 @@
> * Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved.
> */
>
> +#define LOG_CATEGORY LOGC_BOOT
> +
> #include
> #include
> #include
> @@ -762,17 +764,22 @@ static int label_boot(struct pxe_context *ctx, struct
> pxe_label *label)
>
> /* Try bootm for legacy and FIT format image */
> if (genimg_get_format(buf) != IMAGE_FORMAT_INVALID &&
> -IS_ENABLED(CONFIG_CMD_BOOTM))
> + IS_ENABLED(CONFIG_CMD_BOOTM)) {
> + log_debug("using bootm\n");
> do_bootm(ctx->cmdtp, 0, bootm_argc, bootm_argv);
> /* Try booting an AArch64 Linux kernel image */
> - else if (IS_ENABLED(CONFIG_CMD_BOOTI))
> + } else if (IS_ENABLED(CONFIG_CMD_BOOTI)) {
> + log_debug("using booti\n");
> do_booti(ctx->cmdtp, 0, bootm_argc, bootm_argv);
> /* Try booting a Image */
> - else if (IS_ENABLED(CONFIG_CMD_BOOTZ))
> + } else if (IS_ENABLED(CONFIG_CMD_BOOTZ)) {
> + log_debug("using bootz\n");
> do_bootz(ctx->cmdtp, 0, bootm_argc, bootm_argv);
> /* Try booting an x86_64 Linux kernel image */
> - else if (IS_ENABLED(CONFIG_CMD_ZBOOT))
> + } else if (IS_ENABLED(CONFIG_CMD_ZBOOT)) {
> + log_debug("using zboot\n");
> do_zboot_parent(ctx->cmdtp, 0, zboot_argc, zboot_argv, NULL);
> + }
>
> unmap_sysmem(buf);
>
> --
> 2.34.1
>
Reviewed-by: Michael Trimarchi
