RE: a question about falcon mode

[Chan Kim]

Hi Alex,
Thanks for the reply.
So I gather that to be able to use 'spl export fdt' to store the 'snapshot' to 
the storage I should make the spl program runnable at  least to that stage. 
(being able to load kernel image, dtb, initrd and give the spl export command 
to capture it to storage like SD card).
What I'm thinking of is to let another processor(cortext-M7 based scp, system 
control processor) load the u-boot-spl.bin into on-chip RAM and 
kernel(including initramfs)and dtb into SDRAM all from SD card (RAW mode, no 
file system) and let the AP(application processor) start from u-boot-spl.bin. 
In this scheme u-boot-spl doesn't need to load anything and DRAM is already 
initialized and it just needs to run from kernel image after very minimal setup 
(passing kernel arguments). Do you think this is possible or an absurd idea?
Any comment will be a big help for me.
Thanks!
Chan 

> -Original Message-
> From: Alex G. 
> Sent: Thursday, November 25, 2021 11:57 PM
> To: Chan Kim ; U-Boot Mailing List 
> Subject: Re: a question about falcon mode
> 
> On 11/25/21 1:07 AM, Chan Kim wrote:
> > Hello all,
> >
> > I'm trying to implement falcon mode for our board. Then should I first
> > implement the normal mode(spl + proper)?
> >
> > It looks like so while I'm reading doc/README.falcon. (It says, after
> > loading kernel, DT etc. I should give 'spl export' command).
> >
> 
> Falcon mode is a bit board dependent.  There are a couple of ways you
> could go about this.
> 
> The first is to have an "fdtargs" partition. This is where "spl export"
> comes in. Once you run "spl export", it will give a modified dtb at
> "$fdtargsaddr". It's that DTB that you need to write to your ftdargs
> partition. For example:
> 
>  > spl export fdt $loadaddr - $fdt_addr_r
>  > mmc write $fdtargsaddr 0x9800 0x8000
> 
> In this example the ftdargs partition starts at sector 0x9800, and is
> 0x800 sectors long.
> 
> 
> The second option is to forget about "spl export" and "fdtargs", and
> package your kernel, devicetree, and overlays in a FIT container. You'd
> make sure to enable SPL_LOAD_FIT_APPLY_OVERLAY. There isn't much more to
> this other than the usual gotcha's with FIT and overlays.
> 
> Alex
> 
> 

Re: [PATCH v6 3/3] efi_loader: Extend PCR's for firmware measurements

[Heinrich Schuchardt]

On 11/26/21 06:00, Ruchika Gupta wrote:

Firmwares before U-Boot may be capable of doing tpm measurements
and passing them to U-Boot in the form of eventlog. However there
may be scenarios where the firmwares don't have TPM driver and
are not capable of extending the measurements in the PCRs.
Based on TCG spec, if previous firnware has extended PCR's, PCR0
would not be 0. So, read the PCR0 to determine if the PCR's need
to be extended as eventlog is parsed or not.

Signed-off-by: Ruchika Gupta 
Reviewed-by: Ilias Apalodimas 
Tested-by: Ilias Apalodimas 
---
v6: Changed TPM2_DIGEST_LEN to TPM2_SHA512_DIGEST_SIZE

v5 : No change

v4 : No change

v3 :
Rebase changes on top of changes made in first patch series

v2 :
Removed check for PCR0 in eventlog

  lib/efi_loader/efi_tcg2.c | 75 +++
  1 file changed, 75 insertions(+)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index a789c44660..295070f3d8 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -199,6 +199,43 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, 
u32 pcr_index,
return EFI_SUCCESS;
  }

+/* tcg2_pcr_read - Read PCRs for a TPM2 device for a given tpml_digest_values
+ *
+ * @dev:   device


pcr_index is missing

Best regards

Heinrich


+ * @digest_list:   list of digest algorithms to extend
+ *
+ * @Return: status code
+ */
+static efi_status_t tcg2_pcr_read(struct udevice *dev, u32 pcr_index,
+ struct tpml_digest_values *digest_list)
+{
+   struct tpm_chip_priv *priv;
+   unsigned int updates, pcr_select_min;
+   u32 rc;
+   size_t i;
+
+   priv = dev_get_uclass_priv(dev);
+   if (!priv)
+   return EFI_DEVICE_ERROR;
+
+   pcr_select_min = priv->pcr_select_min;
+
+   for (i = 0; i < digest_list->count; i++) {
+   u16 hash_alg = digest_list->digests[i].hash_alg;
+   u8 *digest = (u8 *)_list->digests[i].digest;
+
+   rc = tpm2_pcr_read(dev, pcr_index, pcr_select_min,
+  hash_alg, digest, alg_to_len(hash_alg),
+  );
+   if (rc) {
+   EFI_PRINT("Failed to read PCR\n");
+   return EFI_DEVICE_ERROR;
+   }
+   }
+
+   return EFI_SUCCESS;
+}
+
  /* put_event - Append an agile event to an eventlog
   *
   * @pcr_index:PCR index
@@ -1428,6 +1465,8 @@ efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, 
void *log_buffer,
u32 pcr, pos;
u64 base;
u32 sz;
+   bool extend_pcr = false;
+   int i;

ret = platform_get_eventlog(dev, , );
if (ret != EFI_SUCCESS)
@@ -1449,6 +1488,26 @@ efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, 
void *log_buffer,
return EFI_COMPROMISED_DATA;
}

+   ret = tcg2_pcr_read(dev, 0, _list);
+   if (ret) {
+   log_err("Error reading PCR 0\n");
+   return ret;
+   }
+
+   /*
+* If PCR0 is 0, previous firmware didn't have the capability
+* to extend the PCR. In this scenario, extend the PCR as
+* the eventlog is parsed.
+*/
+   for (i = 0; i < digest_list.count; i++) {
+   u8 buffer[TPM2_SHA512_DIGEST_SIZE] =  { 0 };
+   u16 hash_alg = digest_list.digests[i].hash_alg;
+
+   if (!memcmp((u8 *)_list.digests[i].digest, buffer,
+   alg_to_len(hash_alg)))
+   extend_pcr = true;
+   }
+
while (pos < sz) {
ret = tcg2_parse_event(dev, buffer, sz, , _list,
   );
@@ -1456,6 +1515,22 @@ efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, 
void *log_buffer,
log_err("Error parsing event\n");
return ret;
}
+   if (extend_pcr) {
+   ret = tcg2_pcr_extend(dev, pcr, _list);
+   if (ret != EFI_SUCCESS) {
+   log_err("Error in extending PCR\n");
+   return ret;
+   }
+
+   /* Clear the digest for next event */
+   for (i = 0; i < digest_list.count; i++) {
+   u16 hash_alg = digest_list.digests[i].hash_alg;
+   u8 *digest =
+  (u8 *)_list.digests[i].digest;
+
+   memset(digest, 0, alg_to_len(hash_alg));
+   }
+   }
}

memcpy(log_buffer, buffer, sz);

Re: [PATCH v6 1/3] efi_loader: Add check for event log passed from firmware

[Heinrich Schuchardt]

On 11/26/21 06:00, Ruchika Gupta wrote:

Platforms may have support to measure their initial firmware components
and pass the event log to u-boot. The event log address can be passed
in property tpm_event_log_addr and tpm_event_log_size of the tpm node.
Platforms may choose their own specific mechanism to do so. A weak
function is added to check if even log has been passed to u-boot
from earlier firmware components. If available, the eventlog is parsed
to check for its correctness and further event logs are appended to the
passed log.

Signed-off-by: Ruchika Gupta 
Tested-by: Ilias Apalodimas 
Reviewed-by: Ilias Apalodimas 
---
v6: No change

v5:
Shift the efi_init_event_log() to a different location in the file.
This help fixes compilation issue introduced by calling 
efi_append_scrtm_version()
from it.

v4:
Add SCRTM version to log only if previous firmware doesn't pass the eventlog

v3:
Return as soon as you detect error

v2:
Moved firmware eventlog code parsing to tcg2_get_fw_eventlog()

  lib/efi_loader/efi_tcg2.c | 438 --
  1 file changed, 369 insertions(+), 69 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 8c1f22e337..a789c44660 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -324,6 +324,45 @@ __weak efi_status_t platform_get_tpm2_device(struct 
udevice **dev)
return EFI_NOT_FOUND;
  }

+/**
+ * platform_get_eventlog() - retrieve the eventlog address and size
+ *
+ * This function retrieves the eventlog address and size if the underlying
+ * firmware has done some measurements and passed them.
+ *
+ * This function may be overridden based on platform specific method of
+ * passing the eventlog address and size.
+ *
+ * @dev:   udevice
+ * @addr:  eventlog address
+ * @sz:eventlog size
+ * Return: status code
+ */
+__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr,
+ u32 *sz)


This function must be declared in a header to be overridden.


+{
+   const u64 *basep;
+   const u32 *sizep;
+
+   basep = dev_read_prop(dev, "tpm_event_log_addr", NULL);
+   if (!basep)
+   return EFI_NOT_FOUND;
+
+   *addr = be64_to_cpup((__force __be64 *)basep);
+
+   sizep = dev_read_prop(dev, "tpm_event_log_size", NULL);
+   if (!sizep)
+   return EFI_NOT_FOUND;
+
+   *sz = be32_to_cpup((__force __be32 *)sizep);
+   if (*sz == 0) {
+   log_debug("event log empty\n");
+   return EFI_NOT_FOUND;
+   }
+
+   return EFI_SUCCESS;
+}
+
  /**
   * tpm2_get_max_command_size() - get the supported max command size
   *
@@ -1181,6 +1220,250 @@ static const struct efi_tcg2_protocol efi_tcg2_protocol 
= {
.get_result_of_set_active_pcr_banks = 
efi_tcg2_get_result_of_set_active_pcr_banks,
  };

+/**
+ * parse_event_log_header() -  Parse and verify the event log header fields
+ *
+ * @buffer:Pointer to the event header
+ * @size:  Size of the eventlog
+ * @pos:   Position in buffer after event log header
+ *
+ * Return: status code
+ */
+efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos)


This function should be declared in a header or be static.

Should buffer have type struct tcg_pcr_event *?


+{
+   struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
+   int i = 0;
+
+   if (size < sizeof(*event_header))
+   return EFI_COMPROMISED_DATA;
+
+   if (get_unaligned_le32(_header->pcr_index) != 0 ||
+   get_unaligned_le32(_header->event_type) != EV_NO_ACTION)
+   return EFI_COMPROMISED_DATA;
+
+   for (i = 0; i < sizeof(event_header->digest); i++) {
+   if (event_header->digest[i] != 0)


if (event_header->digest[i])


+   return EFI_COMPROMISED_DATA;
+   }
+
+   *pos += sizeof(*event_header);


Do you re


+
+   return EFI_SUCCESS;
+}
+
+/**
+ * parse_specid_event() -  Parse and verify the specID Event in the eventlog
+ *
+ * @dev:   udevice
+ * @buffer:Pointer to the start of the eventlog
+ * @log_size:  Size of the eventlog
+ * @pos:   Offset in the evenlog where specID event starts
+ *
+ * Return: status code
+ * @posOffset in the eventlog where the specID event 
ends


Duplicate line


+ * @digest_list:   list of digests in the event


Misplaced line


+ */
+efi_status_t parse_specid_event(struct udevice *dev, void *buffer, u32 
log_size,
+   u32 *pos,
+   struct tpml_digest_values *digest_list)
+{


This function should be declared in a header or be static.


+   struct tcg_efi_spec_id_event *spec_event;
+   struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
+   size_t 

Re: [PATCH v5 1/3] mtd: spi-nor: macronix: add support for Macronix Octal

[liao jaime]

Hi Tudor

>
> On 11/26/21 7:28 AM, liao jaime wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> > content is safe
> >
> > Hi Tudor
>
> Hi,
>
> >
> >>
> >> On 11/18/21 12:13 PM, JaimeLiao wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> >>> the content is safe
> >>>
> >>
> >> Hi, Jaime,
> >>
> >>> Follow patch "f6adec1af4b2f5d3012480c6cdce7743b74a6156" for adding
> >>> Macronix flash in Octal DTR mode.
> >>>
> >>> Enable Octal DTR mode with 20 dummy cycles to allow running at the
> >>> maximum supported frequency.
> >>>  
> >>> -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7841/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf
> >>>
> >>
> >> I have submitted a similar patch at
> >> https://lore.kernel.org/all/20211103162454.179191-2-tudor.amba...@microchip.com/T/#me9b6e48c33fd545a9c0b5a5136778651ea64171a
> >>
> >> but I think yours has precedence, you're already at v5.
> >>
> >>> Signed-off-by: JaimeLiao 
> >>> ---
> >>>  drivers/mtd/spi/spi-nor-core.c | 83 ++
> >>>  include/linux/mtd/spi-nor.h| 12 -
> >>>  2 files changed, 93 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/spi/spi-nor-core.c 
> >>> b/drivers/mtd/spi/spi-nor-core.c
> >>> index d5d905fa5a..0a6550984b 100644
> >>> --- a/drivers/mtd/spi/spi-nor-core.c
> >>> +++ b/drivers/mtd/spi/spi-nor-core.c
> >>> @@ -3489,6 +3489,85 @@ static struct spi_nor_fixups mt35xu512aba_fixups = 
> >>> {
> >>>  };
> >>>  #endif /* CONFIG_SPI_FLASH_MT35XU */
> >>>
> >>> +#if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
> >>> +/**
> >>> + * spi_nor_macronix_octal_dtr_enable() - set DTR OPI Enable bit in 
> >>> Configuration Register 2.
> >>> + * @nor:   pointer to a 'struct spi_nor'
> >>> + *
> >>> + * Set the DTR OPI Enable (DOPI) bit in Configuration Register 2.
> >>> + * Bit 2 of  Configuration Register 2 is the DOPI bit for Macronix like 
> >>> OPI memories.
> >>> + *
> >>> + * Return: 0 on success, -errno otherwise.
> >>> + */
> >>> +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor)
> >>> +{
> >>> +   struct spi_mem_op op;
> >>> +   int ret;
> >>> +   u8 buf;
> >>> +
> >>> +   ret = write_enable(nor);
> >>> +   if (ret)
> >>> +   return ret;
> >>> +
> >>> +   buf = SPINOR_REG_MXIC_DC_20;
> >>> +   op = (struct spi_mem_op)
> >>> +   SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1),
> >>> +  SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1),
> >>> +  SPI_MEM_OP_NO_DUMMY,
> >>> +  SPI_MEM_OP_DATA_OUT(1, , 1));
> >>> +
> >>> +   ret = spi_mem_exec_op(nor->spi, );
> >>> +   if (ret)
> >>> +   return ret;
> >>> +
> >>> +   ret = spi_nor_wait_till_ready(nor);
> >>> +   if (ret)
> >>> +   return ret;
> >>> +
> >>> +   nor->read_dummy = MXIC_MAX_DC;
> >>> +   ret = write_enable(nor);
> >>> +   if (ret)
> >>> +   return ret;
> >>> +
> >>> +   buf = SPINOR_REG_MXIC_OPI_DTR_EN;
> >>> +   op = (struct spi_mem_op)
> >>> +   SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1),
> >>> +  SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 
> >>> 1),
> >>> +  SPI_MEM_OP_NO_DUMMY,
> >>> +  SPI_MEM_OP_DATA_OUT(1, , 1));
> >>> +
> >>> +   ret = spi_mem_exec_op(nor->spi, );
> >>> +   if (ret) {
> >>> +   dev_err(nor->dev, "Failed to enable octal DTR mode\n");
> >>> +   return ret;
> >>> +   }
> >>> +   nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
> >>> +
> >>> +   return 0;
> >>> +}
> >>> +
> >>> +static void macronix_octal_default_init(struct spi_nor *nor)
> >>> +{
> >>> +   nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
> >>> +}
> >>
> >> Can't we determine the octal dtr method by parsing SFDP?
> >>
> > It is a good idea for getting flash parameters by checking SFDP.
> > Your patchwork is on-going to solve the ID collision issues in Linux kernel.
> > I think U-boot should follow the method after ID collision patchwork
> > finish in Linux kenel.
> > So that octal dtr method follow the configuration for now.
> >
>
> What I meant was that if we can determine the octal dtr method from SFDP,
> we should use that. If we can't determine the octal dtr method from SFDP,
> or SFDP is skipped intentionally, then we can define explicit methods,
> as you did. But we'll need to differentiate between the two methods,
> otherwise maintaining the flash entries will become a burden.
>
Sure, I prefer to parsing information from SFDP but not got according
to configuration
and differentiate methods via configuration "SPI_NOR_SKIP_SFDP".
Hook corresponding function on .post_sfdp and .post_bfdp if
configuration SPI_NOR_SKIP_SFDP existing.
I need add the changes for v6 patch if you have any other idea.
But I hope that we can seperate octal dtr support 

Re: [PATCH v5 2/3] mtd: spi-nor-core: Adding different type of command extension in Soft Reset

[liao jaime]

Hi Tudor

>
> On 11/18/21 12:13 PM, JaimeLiao wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> > content is safe
> >
> > Power-on-Reset is a method to restore flash back to 1S-1S-1S mode from 
> > 8D-8D-8D
> > in the begging of probe.
> >
> > Command extension type is not standardized across flash vendors in DTR mode.
> >
> > For suiting different vendor flash devices, adding a flag to seperate types 
> > for
> > soft reset on boot.
> >
> > Signed-off-by: JaimeLiao 
> > ---
> >  drivers/mtd/spi/Kconfig| 7 +++
> >  drivers/mtd/spi/spi-nor-core.c | 7 ++-
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
> > index 1b2ef37e92..9b7d195770 100644
> > --- a/drivers/mtd/spi/Kconfig
> > +++ b/drivers/mtd/spi/Kconfig
> > @@ -97,6 +97,13 @@ config SPI_FLASH_SMART_HWCAPS
> >  can support a type of operation in a much more refined way compared
> >  to using flags like SPI_RX_DUAL, SPI_TX_QUAD, etc.
> >
> > +config SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT
>
> There will be many combinations of reset types and cmd_ext types, better to 
> introduce
> a way to select the reset type and then to select the cmd_ext type so that we 
> avoid
> introducing so many configs/dt props.
>
> I think Pratyush is working to fix this, would you sync with him?
Sure, this configuration is according Pratyush suggestion.

> Check the discussion at
> https://lore.kernel.org/all/20211103234950.202289-4-tudor.amba...@microchip.com/
>
> Cheers,
> ta
>
> > +   bool "Command extension type is INVERT for Software Reset on boot"
> > +   default n
> > +   help
> > +Because of SFDP information can not be get before boot.
> > +So define command extension type is INVERT when Software Reset on 
> > boot only.
> > +
> >  config SPI_FLASH_SOFT_RESET
> > bool "Software Reset support for SPI NOR flashes"
> > default n
> > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> > index 0a6550984b..2b6947cefc 100644
> > --- a/drivers/mtd/spi/spi-nor-core.c
> > +++ b/drivers/mtd/spi/spi-nor-core.c
> > @@ -3661,7 +3661,12 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
> > enum spi_nor_cmd_ext ext;
> >
> > ext = nor->cmd_ext_type;
> > -   nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
> > +   if (nor->cmd_ext_type == SPI_NOR_EXT_NONE) {
> > +   nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
> > +#if CONFIG_IS_ENABLED(SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT)
> > +   nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
> > +#endif /* SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT */
> > +   }
> >
> > op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 
> > 0),
> > SPI_MEM_OP_NO_DUMMY,
> > --
> > 2.17.1
> >
>

Re: [PATCH v5 1/3] mtd: spi-nor: macronix: add support for Macronix Octal

[liao jaime]

Hi Tudor

>
> On 11/18/21 12:13 PM, JaimeLiao wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> > content is safe
> >
>
> Hi, Jaime,
>
> > Follow patch "f6adec1af4b2f5d3012480c6cdce7743b74a6156" for adding
> > Macronix flash in Octal DTR mode.
> >
> > Enable Octal DTR mode with 20 dummy cycles to allow running at the
> > maximum supported frequency.
> >  
> > -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7841/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf
> >
>
> I have submitted a similar patch at
> https://lore.kernel.org/all/20211103162454.179191-2-tudor.amba...@microchip.com/T/#me9b6e48c33fd545a9c0b5a5136778651ea64171a
>
> but I think yours has precedence, you're already at v5.
>
> > Signed-off-by: JaimeLiao 
> > ---
> >  drivers/mtd/spi/spi-nor-core.c | 83 ++
> >  include/linux/mtd/spi-nor.h| 12 -
> >  2 files changed, 93 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> > index d5d905fa5a..0a6550984b 100644
> > --- a/drivers/mtd/spi/spi-nor-core.c
> > +++ b/drivers/mtd/spi/spi-nor-core.c
> > @@ -3489,6 +3489,85 @@ static struct spi_nor_fixups mt35xu512aba_fixups = {
> >  };
> >  #endif /* CONFIG_SPI_FLASH_MT35XU */
> >
> > +#if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
> > +/**
> > + * spi_nor_macronix_octal_dtr_enable() - set DTR OPI Enable bit in 
> > Configuration Register 2.
> > + * @nor:   pointer to a 'struct spi_nor'
> > + *
> > + * Set the DTR OPI Enable (DOPI) bit in Configuration Register 2.
> > + * Bit 2 of  Configuration Register 2 is the DOPI bit for Macronix like 
> > OPI memories.
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
> > +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor)
> > +{
> > +   struct spi_mem_op op;
> > +   int ret;
> > +   u8 buf;
> > +
> > +   ret = write_enable(nor);
> > +   if (ret)
> > +   return ret;
> > +
> > +   buf = SPINOR_REG_MXIC_DC_20;
> > +   op = (struct spi_mem_op)
> > +   SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1),
> > +  SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1),
> > +  SPI_MEM_OP_NO_DUMMY,
> > +  SPI_MEM_OP_DATA_OUT(1, , 1));
> > +
> > +   ret = spi_mem_exec_op(nor->spi, );
> > +   if (ret)
> > +   return ret;
> > +
> > +   ret = spi_nor_wait_till_ready(nor);
> > +   if (ret)
> > +   return ret;
> > +
> > +   nor->read_dummy = MXIC_MAX_DC;
> > +   ret = write_enable(nor);
> > +   if (ret)
> > +   return ret;
> > +
> > +   buf = SPINOR_REG_MXIC_OPI_DTR_EN;
> > +   op = (struct spi_mem_op)
> > +   SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1),
> > +  SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1),
> > +  SPI_MEM_OP_NO_DUMMY,
> > +  SPI_MEM_OP_DATA_OUT(1, , 1));
> > +
> > +   ret = spi_mem_exec_op(nor->spi, );
> > +   if (ret) {
> > +   dev_err(nor->dev, "Failed to enable octal DTR mode\n");
> > +   return ret;
> > +   }
> > +   nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
> > +
> > +   return 0;
> > +}
> > +
> > +static void macronix_octal_default_init(struct spi_nor *nor)
> > +{
> > +   nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
> > +}
>
> Can't we determine the octal dtr method by parsing SFDP?
>
It is a good idea for getting flash parameters by checking SFDP.
Your patchwork is on-going to solve the ID collision issues in Linux kernel.
I think U-boot should follow the method after ID collision patchwork
finish in Linux kenel.
So that octal dtr method follow the configuration for now.

Thanks
Jaime
> Cheers,
> ta
>
> > +
> > +static void macronix_octal_post_sfdp_fixup(struct spi_nor *nor,
> > +struct spi_nor_flash_parameter 
> > *params)
> > +{
> > +   /*
> > +* Adding SNOR_HWCAPS_PP_8_8_8_DTR in hwcaps.mask when
> > +* SPI_NOR_OCTAL_DTR_READ flag exists.
> > +*/
> > +   if (params->hwcaps.mask & SNOR_HWCAPS_READ_8_8_8_DTR)
> > +   params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
> > +}
> > +
> > +static struct spi_nor_fixups macronix_octal_fixups = {
> > +   .default_init = macronix_octal_default_init,
> > +   .post_sfdp = macronix_octal_post_sfdp_fixup,
> > +};
> > +#endif /* CONFIG_SPI_FLASH_MACRONIX */
> > +
> >  /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
> >   * @nor: pointer to a 'struct spi_nor'
> >   *
> > @@ -3655,6 +3734,10 @@ void spi_nor_set_fixups(struct spi_nor *nor)
> > if (!strcmp(nor->info->name, "mt35xu512aba"))
> > nor->fixups = _fixups;
> >  #endif
> > +
> > +#if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
> > +   nor->fixups = _octal_fixups;
> > +#endif /* 

[PATCH v6 3/3] efi_loader: Extend PCR's for firmware measurements

[Ruchika Gupta]

Firmwares before U-Boot may be capable of doing tpm measurements
and passing them to U-Boot in the form of eventlog. However there
may be scenarios where the firmwares don't have TPM driver and
are not capable of extending the measurements in the PCRs.
Based on TCG spec, if previous firnware has extended PCR's, PCR0
would not be 0. So, read the PCR0 to determine if the PCR's need
to be extended as eventlog is parsed or not.

Signed-off-by: Ruchika Gupta 
Reviewed-by: Ilias Apalodimas 
Tested-by: Ilias Apalodimas 
---
v6: Changed TPM2_DIGEST_LEN to TPM2_SHA512_DIGEST_SIZE

v5 : No change

v4 : No change

v3 : 
Rebase changes on top of changes made in first patch series

v2 : 
Removed check for PCR0 in eventlog

 lib/efi_loader/efi_tcg2.c | 75 +++
 1 file changed, 75 insertions(+)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index a789c44660..295070f3d8 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -199,6 +199,43 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, 
u32 pcr_index,
return EFI_SUCCESS;
 }
 
+/* tcg2_pcr_read - Read PCRs for a TPM2 device for a given tpml_digest_values
+ *
+ * @dev:   device
+ * @digest_list:   list of digest algorithms to extend
+ *
+ * @Return: status code
+ */
+static efi_status_t tcg2_pcr_read(struct udevice *dev, u32 pcr_index,
+ struct tpml_digest_values *digest_list)
+{
+   struct tpm_chip_priv *priv;
+   unsigned int updates, pcr_select_min;
+   u32 rc;
+   size_t i;
+
+   priv = dev_get_uclass_priv(dev);
+   if (!priv)
+   return EFI_DEVICE_ERROR;
+
+   pcr_select_min = priv->pcr_select_min;
+
+   for (i = 0; i < digest_list->count; i++) {
+   u16 hash_alg = digest_list->digests[i].hash_alg;
+   u8 *digest = (u8 *)_list->digests[i].digest;
+
+   rc = tpm2_pcr_read(dev, pcr_index, pcr_select_min,
+  hash_alg, digest, alg_to_len(hash_alg),
+  );
+   if (rc) {
+   EFI_PRINT("Failed to read PCR\n");
+   return EFI_DEVICE_ERROR;
+   }
+   }
+
+   return EFI_SUCCESS;
+}
+
 /* put_event - Append an agile event to an eventlog
  *
  * @pcr_index: PCR index
@@ -1428,6 +1465,8 @@ efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, 
void *log_buffer,
u32 pcr, pos;
u64 base;
u32 sz;
+   bool extend_pcr = false;
+   int i;
 
ret = platform_get_eventlog(dev, , );
if (ret != EFI_SUCCESS)
@@ -1449,6 +1488,26 @@ efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, 
void *log_buffer,
return EFI_COMPROMISED_DATA;
}
 
+   ret = tcg2_pcr_read(dev, 0, _list);
+   if (ret) {
+   log_err("Error reading PCR 0\n");
+   return ret;
+   }
+
+   /*
+* If PCR0 is 0, previous firmware didn't have the capability
+* to extend the PCR. In this scenario, extend the PCR as
+* the eventlog is parsed.
+*/
+   for (i = 0; i < digest_list.count; i++) {
+   u8 buffer[TPM2_SHA512_DIGEST_SIZE] =  { 0 };
+   u16 hash_alg = digest_list.digests[i].hash_alg;
+
+   if (!memcmp((u8 *)_list.digests[i].digest, buffer,
+   alg_to_len(hash_alg)))
+   extend_pcr = true;
+   }
+
while (pos < sz) {
ret = tcg2_parse_event(dev, buffer, sz, , _list,
   );
@@ -1456,6 +1515,22 @@ efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, 
void *log_buffer,
log_err("Error parsing event\n");
return ret;
}
+   if (extend_pcr) {
+   ret = tcg2_pcr_extend(dev, pcr, _list);
+   if (ret != EFI_SUCCESS) {
+   log_err("Error in extending PCR\n");
+   return ret;
+   }
+
+   /* Clear the digest for next event */
+   for (i = 0; i < digest_list.count; i++) {
+   u16 hash_alg = digest_list.digests[i].hash_alg;
+   u8 *digest =
+  (u8 *)_list.digests[i].digest;
+
+   memset(digest, 0, alg_to_len(hash_alg));
+   }
+   }
}
 
memcpy(log_buffer, buffer, sz);
-- 
2.25.1

[PATCH v6 2/3] tpm: use more algorithms than sha256 on pcr_read

[Ruchika Gupta]

The current tpm2_pcr_read is hardcoded using SHA256. Make the
actual command to TPM configurable to use wider range of algorithms.
The current command line is kept as is i.e limited to SHA-256 only.

Signed-off-by: Ruchika Gupta 
Reviewed-by: Ilias Apalodimas 
---
v6: No change

v5: No change

v4: No change

v3: No change

v2:
Change algorithm from u32 to u16
Add parameter description in function declaration

 cmd/tpm-v2.c |  3 ++-
 include/tpm-v2.h |  5 -
 lib/tpm-v2.c | 12 
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
index daae91100a..4ea5f9f094 100644
--- a/cmd/tpm-v2.c
+++ b/cmd/tpm-v2.c
@@ -151,7 +151,8 @@ static int do_tpm_pcr_read(struct cmd_tbl *cmdtp, int flag, 
int argc,
 
data = map_sysmem(simple_strtoul(argv[2], NULL, 0), 0);
 
-   rc = tpm2_pcr_read(dev, index, priv->pcr_select_min, data, );
+   rc = tpm2_pcr_read(dev, index, priv->pcr_select_min, TPM2_ALG_SHA256,
+  data, TPM2_DIGEST_LEN, );
if (!rc) {
printf("PCR #%u content (%u known updates):\n", index, updates);
print_byte_string(data, TPM2_DIGEST_LEN);
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index ceff7d245e..4e9dd52cb6 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -512,13 +512,16 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, 
const void *data,
  * @devTPM device
  * @idxIndex of the PCR
  * @idx_min_sz Minimum size in bytes of the pcrSelect array
+ * @algorithm  Algorithm used, defined in 'enum tpm2_algorithms'
  * @data   Output buffer for contents of the named PCR
+ * @digest_len  len of the data
  * @updatesOptional out parameter: number of updates for this PCR
  *
  * @return code of the operation
  */
 u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
- void *data, unsigned int *updates);
+ u16 algorithm, void *data, u32 digest_len,
+ unsigned int *updates);
 
 /**
  * Issue a TPM2_GetCapability command.  This implementation is limited
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 2e7b27bd6b..1bf627853a 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -254,7 +254,8 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, 
const void *data,
 }
 
 u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
- void *data, unsigned int *updates)
+ u16 algorithm, void *data, u32 digest_len,
+ unsigned int *updates)
 {
u8 idx_array_sz = max(idx_min_sz, DIV_ROUND_UP(idx, 8));
u8 command_v2[COMMAND_BUFFER_SIZE] = {
@@ -264,7 +265,7 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned 
int idx_min_sz,
 
/* TPML_PCR_SELECTION */
tpm_u32(1), /* Number of selections */
-   tpm_u16(TPM2_ALG_SHA256),   /* Algorithm of the hash */
+   tpm_u16(algorithm), /* Algorithm of the hash */
idx_array_sz,   /* Array size for selection */
/* bitmap(idx) Selected PCR bitmap */
};
@@ -283,10 +284,13 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned 
int idx_min_sz,
if (ret)
return ret;
 
+   if (digest_len > response_len)
+   return TPM_LIB_ERROR;
+
if (unpack_byte_string(response, response_len, "ds",
   10, ,
-  response_len - TPM2_DIGEST_LEN, data,
-  TPM2_DIGEST_LEN))
+  response_len - digest_len, data,
+  digest_len))
return TPM_LIB_ERROR;
 
if (updates)
-- 
2.25.1

[PATCH v6 1/3] efi_loader: Add check for event log passed from firmware

[Ruchika Gupta]

Platforms may have support to measure their initial firmware components
and pass the event log to u-boot. The event log address can be passed
in property tpm_event_log_addr and tpm_event_log_size of the tpm node.
Platforms may choose their own specific mechanism to do so. A weak
function is added to check if even log has been passed to u-boot
from earlier firmware components. If available, the eventlog is parsed
to check for its correctness and further event logs are appended to the
passed log.

Signed-off-by: Ruchika Gupta 
Tested-by: Ilias Apalodimas 
Reviewed-by: Ilias Apalodimas 
---
v6: No change

v5:
Shift the efi_init_event_log() to a different location in the file.
This help fixes compilation issue introduced by calling 
efi_append_scrtm_version()
from it.

v4:
Add SCRTM version to log only if previous firmware doesn't pass the eventlog

v3:
Return as soon as you detect error

v2:
Moved firmware eventlog code parsing to tcg2_get_fw_eventlog()

 lib/efi_loader/efi_tcg2.c | 438 --
 1 file changed, 369 insertions(+), 69 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 8c1f22e337..a789c44660 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -324,6 +324,45 @@ __weak efi_status_t platform_get_tpm2_device(struct 
udevice **dev)
return EFI_NOT_FOUND;
 }
 
+/**
+ * platform_get_eventlog() - retrieve the eventlog address and size
+ *
+ * This function retrieves the eventlog address and size if the underlying
+ * firmware has done some measurements and passed them.
+ *
+ * This function may be overridden based on platform specific method of
+ * passing the eventlog address and size.
+ *
+ * @dev:   udevice
+ * @addr:  eventlog address
+ * @sz:eventlog size
+ * Return: status code
+ */
+__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr,
+ u32 *sz)
+{
+   const u64 *basep;
+   const u32 *sizep;
+
+   basep = dev_read_prop(dev, "tpm_event_log_addr", NULL);
+   if (!basep)
+   return EFI_NOT_FOUND;
+
+   *addr = be64_to_cpup((__force __be64 *)basep);
+
+   sizep = dev_read_prop(dev, "tpm_event_log_size", NULL);
+   if (!sizep)
+   return EFI_NOT_FOUND;
+
+   *sz = be32_to_cpup((__force __be32 *)sizep);
+   if (*sz == 0) {
+   log_debug("event log empty\n");
+   return EFI_NOT_FOUND;
+   }
+
+   return EFI_SUCCESS;
+}
+
 /**
  * tpm2_get_max_command_size() - get the supported max command size
  *
@@ -1181,6 +1220,250 @@ static const struct efi_tcg2_protocol efi_tcg2_protocol 
= {
.get_result_of_set_active_pcr_banks = 
efi_tcg2_get_result_of_set_active_pcr_banks,
 };
 
+/**
+ * parse_event_log_header() -  Parse and verify the event log header fields
+ *
+ * @buffer:Pointer to the event header
+ * @size:  Size of the eventlog
+ * @pos:   Position in buffer after event log header
+ *
+ * Return: status code
+ */
+efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos)
+{
+   struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
+   int i = 0;
+
+   if (size < sizeof(*event_header))
+   return EFI_COMPROMISED_DATA;
+
+   if (get_unaligned_le32(_header->pcr_index) != 0 ||
+   get_unaligned_le32(_header->event_type) != EV_NO_ACTION)
+   return EFI_COMPROMISED_DATA;
+
+   for (i = 0; i < sizeof(event_header->digest); i++) {
+   if (event_header->digest[i] != 0)
+   return EFI_COMPROMISED_DATA;
+   }
+
+   *pos += sizeof(*event_header);
+
+   return EFI_SUCCESS;
+}
+
+/**
+ * parse_specid_event() -  Parse and verify the specID Event in the eventlog
+ *
+ * @dev:   udevice
+ * @buffer:Pointer to the start of the eventlog
+ * @log_size:  Size of the eventlog
+ * @pos:   Offset in the evenlog where specID event starts
+ *
+ * Return: status code
+ * @posOffset in the eventlog where the specID event 
ends
+ * @digest_list:   list of digests in the event
+ */
+efi_status_t parse_specid_event(struct udevice *dev, void *buffer, u32 
log_size,
+   u32 *pos,
+   struct tpml_digest_values *digest_list)
+{
+   struct tcg_efi_spec_id_event *spec_event;
+   struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
+   size_t spec_event_size;
+   u32 active = 0, supported = 0, pcr_count = 0, alg_count = 0;
+   u32 spec_active = 0;
+   u16 hash_alg, hash_sz;
+   u8 vendor_sz;
+   int err, i;
+
+   /* Check specID event data */
+   spec_event = (struct tcg_efi_spec_id_event *)((uintptr_t)buffer + *pos);
+   /* Check for signature */
+   if 

[PATCH v3] efi_loader: check tcg2 protocol installation outside the TCG protocol

[Masahisa Kojima]

There are functions that calls tcg2_agile_log_append() outside
of the TCG protocol invocation (e.g tcg2_measure_pe_image).
These functions must to check that tcg2 protocol is installed.
If not, measurement shall be skipped.

Together with above change, this commit also removes the
unnecessary tcg2_uninit() call in efi_tcg2_register(),
refactors efi_tcg2_register() not to include the process
that is not related to the tcg2 protocol registration.

Signed-off-by: Masahisa Kojima 
---
Changes in v3:
- add static qualifier to is_tcg2_protocol_installed()
- simplify is_tcg2_protocol_installed() return handling

Changes in v2:
- rebased on top of the TF-A eventlog handover support

 include/efi_loader.h   |  4 ++
 lib/efi_loader/efi_setup.c |  3 ++
 lib/efi_loader/efi_tcg2.c  | 89 +++---
 3 files changed, 81 insertions(+), 15 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index d52e399841..fe29c10906 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -525,6 +525,10 @@ efi_status_t efi_disk_register(void);
 efi_status_t efi_rng_register(void);
 /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
 efi_status_t efi_tcg2_register(void);
+/* Called by efi_init_obj_list() to do the required setup for the measurement 
*/
+efi_status_t efi_tcg2_setup_measurement(void);
+/* Called by efi_init_obj_list() to do initial measurement */
+efi_status_t efi_tcg2_do_initial_measurement(void);
 /* measure the pe-coff image, extend PCR and add Event Log */
 efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
   struct efi_loaded_image_obj *handle,
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index a2338d74af..781d10590d 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -271,6 +271,9 @@ efi_status_t efi_init_obj_list(void)
ret = efi_tcg2_register();
if (ret != EFI_SUCCESS)
goto out;
+
+   efi_tcg2_setup_measurement();
+   efi_tcg2_do_initial_measurement();
}
 
/* Secure boot */
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index b44eed0ec9..2196af49a6 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -153,6 +153,20 @@ static u16 alg_to_len(u16 hash_alg)
return 0;
 }
 
+/**
+ * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed
+ *
+ * @Return: true if tcg2 protocol is installed, false if not
+ */
+static bool is_tcg2_protocol_installed(void)
+{
+   struct efi_handler *handler;
+   efi_status_t ret;
+
+   ret = efi_search_protocol(efi_root, _guid_tcg2_protocol, );
+   return (ret == EFI_SUCCESS);
+}
+
 static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
 {
u32 len;
@@ -962,6 +976,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
IMAGE_NT_HEADERS32 *nt;
struct efi_handler *handler;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
return ret;
@@ -2140,6 +2157,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct 
efi_loaded_image_obj *ha
u32 event = 0;
struct smbios_entry *entry;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
if (tcg2_efi_app_invoked)
return EFI_SUCCESS;
 
@@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
efi_status_t ret;
struct udevice *dev;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
return ret;
@@ -2214,6 +2237,11 @@ efi_tcg2_notify_exit_boot_services(struct efi_event 
*event, void *context)
 
EFI_ENTRY("%p, %p", event, context);
 
+   if (!is_tcg2_protocol_installed()) {
+   ret =  EFI_NOT_READY;
+   goto out;
+   }
+
event_log.ebs_called = true;
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
@@ -2244,6 +2272,9 @@ efi_status_t 
efi_tcg2_notify_exit_boot_services_failed(void)
struct udevice *dev;
efi_status_t ret;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
goto out;
@@ -2313,6 +2344,45 @@ error:
return ret;
 }
 
+/**
+ * efi_tcg2_setup_measurement() - setup for the measurement
+ *
+ * Return: status code
+ */
+efi_status_t efi_tcg2_setup_measurement(void)
+{
+   efi_status_t ret;
+   struct efi_event *event;
+
+   ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
+  efi_tcg2_notify_exit_boot_services, NULL,
+  NULL, 

Re: [PATCH v2] efi_loader: check tcg2 protocol installation outside the TCG protocol

[Heinrich Schuchardt]

On 11/25/21 21:40, Ilias Apalodimas wrote:

Hi Heinrich,

[...]


u32 len;
@@ -962,6 +976,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
IMAGE_NT_HEADERS32 *nt;
struct efi_handler *handler;

+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
return ret;
@@ -2140,6 +2157,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct 
efi_loaded_image_obj *ha
u32 event = 0;
struct smbios_entry *entry;

+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
if (tcg2_efi_app_invoked)
return EFI_SUCCESS;

@@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
efi_status_t ret;
struct udevice *dev;

+   if (!is_tcg2_protocol_installed())


[...]

Heinrich, this whole patch is needed because installing  the tcg2 protocol
always returns EFI_SUCCESS.  The reason is that some sandbox tests with
sandbox_tpm used to fail.  Do you want to keep this or perhaps just failing
the boot now is the protocol fails to install is an option ?


Which test failed?


It's been a while, but if my memory serves me correctly, during the
protocol installation we need to call:
efi_init_event_log() -> create_specid_event() -> tpm2_get_pcr_info() ->
tpm2_get_capability().

That get_capability call wasn't supported in sandbox.  So the result was
EFI TCG2 stopping the boot process.  Simon did fix a few things on sandbox
since then, but I can't remember if capabilities was one of them.



We should consistently test the TCG2 protocol using swtpm both on QEMU
and on the sandbox. I am still waiting for Tom to apply

[U-BOOT-TEST-HOOKS,1/1] Enable TPMv2 emulation
https://patchwork.ozlabs.org/project/uboot/patch/2025101106.36479-1-heinrich.schucha...@canonical.com/

to move to that target.

Until then we can disable the tcg2 test or the TCG2 protocol on the sandbox.


That would be fine by me.  Not stopping the boot on failures introduces the
need for patches like this.  So you suggest we drop this and just fail the
boot ?


If the sandbox makes problems due to its incomplete TPM emulation I
would suggest:

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 700dc838dd..201a0d62e2 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -307,7 +307,7 @@ config EFI_RNG_PROTOCOL
 config EFI_TCG2_PROTOCOL
bool "EFI_TCG2_PROTOCOL support"
default y
-   depends on TPM_V2
+   depends on TPM_V2 && !SANDBOX

We can revert such a change once swtpm can be used to provide a tpm
emulation for the sandbox.

Best regards

Heinrich

Re: [PATCH v2] efi_loader: check tcg2 protocol installation outside the TCG protocol

[Ilias Apalodimas]

Hi Heinrich,

[...]

> > >   u32 len;
> > > @@ -962,6 +976,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 
> > > efi_size,
> > >   IMAGE_NT_HEADERS32 *nt;
> > >   struct efi_handler *handler;
> > > 
> > > + if (!is_tcg2_protocol_installed())
> > > + return EFI_NOT_READY;
> > > +
> > >   ret = platform_get_tpm2_device();
> > >   if (ret != EFI_SUCCESS)
> > >   return ret;
> > > @@ -2140,6 +2157,9 @@ efi_status_t 
> > > efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
> > >   u32 event = 0;
> > >   struct smbios_entry *entry;
> > > 
> > > + if (!is_tcg2_protocol_installed())
> > > + return EFI_NOT_READY;
> > > +
> > >   if (tcg2_efi_app_invoked)
> > >   return EFI_SUCCESS;
> > > 
> > > @@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
> > >   efi_status_t ret;
> > >   struct udevice *dev;
> > > 
> > > + if (!is_tcg2_protocol_installed())
> > 
> > [...]
> > 
> > Heinrich, this whole patch is needed because installing  the tcg2 protocol
> > always returns EFI_SUCCESS.  The reason is that some sandbox tests with
> > sandbox_tpm used to fail.  Do you want to keep this or perhaps just failing
> > the boot now is the protocol fails to install is an option ?
> 
> Which test failed?

It's been a while, but if my memory serves me correctly, during the
protocol installation we need to call: 
efi_init_event_log() -> create_specid_event() -> tpm2_get_pcr_info() ->
tpm2_get_capability().

That get_capability call wasn't supported in sandbox.  So the result was
EFI TCG2 stopping the boot process.  Simon did fix a few things on sandbox
since then, but I can't remember if capabilities was one of them.

> 
> We should consistently test the TCG2 protocol using swtpm both on QEMU
> and on the sandbox. I am still waiting for Tom to apply
> 
> [U-BOOT-TEST-HOOKS,1/1] Enable TPMv2 emulation
> https://patchwork.ozlabs.org/project/uboot/patch/2025101106.36479-1-heinrich.schucha...@canonical.com/
> 
> to move to that target.
> 
> Until then we can disable the tcg2 test or the TCG2 protocol on the sandbox.

That would be fine by me.  Not stopping the boot on failures introduces the
need for patches like this.  So you suggest we drop this and just fail the
boot ?

Thanks
/Ilias
> 
> Best regards
> 
> Heinrich

Re: [PATCH v2] efi_loader: check tcg2 protocol installation outside the TCG protocol

[Heinrich Schuchardt]

On 11/25/21 14:22, Ilias Apalodimas wrote:

Hi Kojima-san,

On Thu, Nov 25, 2021 at 08:36:28PM +0900, Masahisa Kojima wrote:

+/**


[...]


+ * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed
+ *
+ * @Return: true if tcg2 protocol is installed, false if not
+ */
+bool is_tcg2_protocol_installed(void)
+{
+   struct efi_handler *handler;
+   efi_status_t ret;
+
+   ret = efi_search_protocol(efi_root, _guid_tcg2_protocol, );
+   return ((ret == EFI_SUCCESS) ? true : false);
+}


return ret == EFI_SUCCESS; is enough here.


+
  static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
  {
u32 len;
@@ -962,6 +976,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
IMAGE_NT_HEADERS32 *nt;
struct efi_handler *handler;

+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
return ret;
@@ -2140,6 +2157,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct 
efi_loaded_image_obj *ha
u32 event = 0;
struct smbios_entry *entry;

+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
if (tcg2_efi_app_invoked)
return EFI_SUCCESS;

@@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
efi_status_t ret;
struct udevice *dev;

+   if (!is_tcg2_protocol_installed())


[...]

Heinrich, this whole patch is needed because installing  the tcg2 protocol
always returns EFI_SUCCESS.  The reason is that some sandbox tests with
sandbox_tpm used to fail.  Do you want to keep this or perhaps just failing
the boot now is the protocol fails to install is an option ?


Which test failed?

We should consistently test the TCG2 protocol using swtpm both on QEMU
and on the sandbox. I am still waiting for Tom to apply

[U-BOOT-TEST-HOOKS,1/1] Enable TPMv2 emulation
https://patchwork.ozlabs.org/project/uboot/patch/2025101106.36479-1-heinrich.schucha...@canonical.com/

to move to that target.

Until then we can disable the tcg2 test or the TCG2 protocol on the sandbox.

Best regards

Heinrich

[PATCH] configs: rock-pi-4: Enable rockchip efuse support

[Sjoerd Simons]

Enable efuse support for reading the cpuid#, serial# and generate a
board unique mac address

Signed-off-by: Sjoerd Simons 

---

 configs/rock-pi-4-rk3399_defconfig  | 1 +
 configs/rock-pi-4c-rk3399_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/rock-pi-4-rk3399_defconfig 
b/configs/rock-pi-4-rk3399_defconfig
index 9366eba8f15..032b9086699 100644
--- a/configs/rock-pi-4-rk3399_defconfig
+++ b/configs/rock-pi-4-rk3399_defconfig
@@ -31,6 +31,7 @@ CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names 
clock-names interrupt-parent
 CONFIG_ENV_IS_IN_MMC=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_ROCKCHIP_GPIO=y
+CONFIG_ROCKCHIP_EFUSE=y
 CONFIG_SYS_I2C_ROCKCHIP=y
 CONFIG_MISC=y
 CONFIG_MMC_DW=y
diff --git a/configs/rock-pi-4c-rk3399_defconfig 
b/configs/rock-pi-4c-rk3399_defconfig
index ac045d1492f..6f5e8666b0d 100644
--- a/configs/rock-pi-4c-rk3399_defconfig
+++ b/configs/rock-pi-4c-rk3399_defconfig
@@ -31,6 +31,7 @@ CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names 
clock-names interrupt-parent
 CONFIG_ENV_IS_IN_MMC=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_ROCKCHIP_GPIO=y
+CONFIG_ROCKCHIP_EFUSE=y
 CONFIG_SYS_I2C_ROCKCHIP=y
 CONFIG_MISC=y
 CONFIG_MMC_DW=y
-- 
2.34.0

[RESEND PATCH] rpi: Copy properties from firmware dtb to the loaded dtb

[Sjoerd Simons]

The RPI firmware adjusts several property values in the dtb it passes
to u-boot depending on the board/SoC revision. Inherit some of these
when u-boot loads a dtb itself. Specificaly copy:

* /model: The firmware provides a more specific string
* /memreserve: The firmware defines a reserved range, better keep it
* emmc2bus and pcie0 dma-ranges: The C0T revision of the bcm2711 Soc (as
  present on rpi 400 and some rpi 4B boards) has different values for
  these then the B0T revision. So these need to be adjusted to boot on
  these boards
* blconfig: The firmware defines the memory area where the blconfig
  stored. Copy those over so it can be enabled.
* /chosen/kaslr-seed: The firmware generates a kaslr seed, take advantage
  of that.

Signed-off-by: Sjoerd Simons 
---

 board/raspberrypi/rpi/rpi.c | 48 +
 1 file changed, 48 insertions(+)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 55afaa54d9f..cdde32c8143 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -499,10 +499,58 @@ void *board_fdt_blob_setup(int *err)
return (void *)fw_dtb_pointer;
 }
 
+int copy_property(void *dst, void *src, char *path, char *property)
+{
+   int dst_offset, src_offset;
+   const fdt32_t *prop;
+   int len;
+
+   src_offset = fdt_path_offset(src, path);
+   dst_offset = fdt_path_offset(dst, path);
+
+   if (src_offset < 0 || dst_offset < 0)
+   return -1;
+
+   prop = fdt_getprop(src, src_offset, property, );
+   if (!prop)
+   return -1;
+
+   return fdt_setprop(dst, dst_offset, property, prop, len);
+}
+
+/* Copy tweaks from the firmware dtb to the loaded dtb */
+void  update_fdt_from_fw(void *fdt, void *fw_fdt)
+{
+   /* Using dtb from firmware directly; leave it alone */
+   if (fdt == fw_fdt)
+   return;
+
+   /* The firmware provides a more precie model; so copy that */
+   copy_property(fdt, fw_fdt, "/", "model");
+
+   /* memory reserve as suggested by the firmware */
+   copy_property(fdt, fw_fdt, "/", "memreserve");
+
+   /* Adjust dma-ranges for the SD card and PCI bus as they can depend on
+* the SoC revision
+*/
+   copy_property(fdt, fw_fdt, "emmc2bus", "dma-ranges");
+   copy_property(fdt, fw_fdt, "pcie0", "dma-ranges");
+
+   /* Bootloader configuration template exposes as nvmem */
+   if (copy_property(fdt, fw_fdt, "blconfig", "reg") == 0)
+   copy_property(fdt, fw_fdt, "blconfig", "status");
+
+   /* kernel address randomisation seed as provided by the firmware */
+   copy_property(fdt, fw_fdt, "/chosen", "kaslr-seed");
+}
+
 int ft_board_setup(void *blob, struct bd_info *bd)
 {
int node;
 
+   update_fdt_from_fw(blob, (void *)fw_dtb_pointer);
+
node = fdt_node_offset_by_compatible(blob, -1, "simple-framebuffer");
if (node < 0)
lcd_dt_simplefb_add_node(blob);
-- 
2.34.0

Re: macb clock handling (Was: [PATCH v1 4/5] net: macb: Compatible as per device tree)

[Heiko Stübner]

Hi,

Am Donnerstag, 11. November 2021, 10:41:46 CET schrieb Heiko Stübner:
> not wanting to hijack this too much, but does the mac driver also need
> some sort of clock handling?
> 
> Because on the Icicle I have here, I'm running into "TX timeout" errors:
> 
> RISC-V # dhcp
> ethernet@20112000: PHY present at 9
> ethernet@20112000: Starting autonegotiation...
> ethernet@20112000: Autonegotiation complete
> ethernet@20112000: link up, 1000Mbps full-duplex (lpa: 0x3800)
> BOOTP broadcast 1
> ethernet@20112000: TX timeout
> BOOTP broadcast 2
> ethernet@20112000: TX timeout
> BOOTP broadcast 3
> ethernet@20112000: TX timeout
> BOOTP broadcast 4
> ethernet@20112000: TX timeout
> 
> The sifive variant of the macb distinguishes between speeds in its
> cllk_init callback, so I guess the Icicle might need that as well?

just for "the archive", i.e. when people read this in the future:

I got this solved by updating the HSS to a recent release (2021.11
in this case).

So now I do have working network in u-boot.


Heiko


> Am Freitag, 22. Oktober 2021, 10:56:47 CET schrieb Padmarao Begari:
> > Update compatible as per Microchip PolarFire SoC ethernet
> > device node.
> > 
> > Signed-off-by: Padmarao Begari 
> > ---
> >  drivers/net/macb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> > index 8c6461e717..1b867bd5c2 100644
> > --- a/drivers/net/macb.c
> > +++ b/drivers/net/macb.c
> > @@ -1502,7 +1502,7 @@ static const struct udevice_id macb_eth_ids[] = {
> > { .compatible = "cdns,zynq-gem" },
> > { .compatible = "sifive,fu540-c000-gem",
> >   .data = (ulong)_config },
> > -   { .compatible = "microchip,mpfs-mss-gem",
> > +   { .compatible = "microchip,mpfs-gem",
> >   .data = (ulong)_config },
> > { }
> >  };
> > 
> 
> 

[PATCH 3/3] mkimage: Allow to specify the signature algorithm on the command line

[Jan Kiszka]

From: Jan Kiszka 

This permits to prepare FIT image description that do not hard-code the
final choice of the signature algorithm, possibly requiring the user to
patch the sources.

When -o  is specified, this information is used in favor of the
'algo' property in the signature node. Furthermore, that property is set
accordingly when writing the image.

Signed-off-by: Jan Kiszka 
---
 doc/mkimage.1  |  5 +
 include/image.h|  3 ++-
 tools/fit_image.c  |  3 ++-
 tools/image-host.c | 48 +++---
 tools/imagetool.h  |  1 +
 tools/mkimage.c|  5 -
 6 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/doc/mkimage.1 b/doc/mkimage.1
index fea5288784..0734bd36a1 100644
--- a/doc/mkimage.1
+++ b/doc/mkimage.1
@@ -155,6 +155,11 @@ the corresponding public key is written into this file for 
for run-time
 verification. Typically the file here is the device tree binary used by
 CONFIG_OF_CONTROL in U-Boot.
 
+.TP
+.BI "\-o [" "signing algorithm" "]"
+Specifies the algorithm to be used for signing a FIT image. The default is
+taken from the target signature nodes 'algo' properties.
+
 .TP
 .BI "\-p [" "external position" "]"
 Place external data at a static external position. See \-E. Instead of writing
diff --git a/include/image.h b/include/image.h
index 16ccc5b10f..4a7e9bc9a1 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1031,6 +1031,7 @@ int fit_cipher_data(const char *keydir, void *keydest, 
void *fit,
  * @require_keys: Mark all keys as 'required'
  * @engine_id: Engine to use for signing
  * @cmdname:   Command name used when reporting errors
+ * @algo_name: Algorithm name, or NULL if to be read from FIT
  *
  * Adds hash values for all component images in the FIT blob.
  * Hashes are calculated for all component images which have hash subnodes
@@ -1045,7 +1046,7 @@ int fit_cipher_data(const char *keydir, void *keydest, 
void *fit,
 int fit_add_verification_data(const char *keydir, const char *keyfile,
  void *keydest, void *fit, const char *comment,
  int require_keys, const char *engine_id,
- const char *cmdname);
+ const char *cmdname, const char *algo_name);
 
 int fit_image_verify_with_data(const void *fit, int image_noffset,
   const void *data, size_t size);
diff --git a/tools/fit_image.c b/tools/fit_image.c
index f4f372ba62..428ddcf881 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -73,7 +73,8 @@ static int fit_add_file_data(struct image_tool_params 
*params, size_t size_inc,
params->comment,
params->require_keys,
params->engine_id,
-   params->cmdname);
+   params->cmdname,
+   params->algo_name);
}
 
if (dest_blob) {
diff --git a/tools/image-host.c b/tools/image-host.c
index a027155f3c..d2e67a06aa 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -107,7 +107,7 @@ static int fit_image_process_hash(void *fit, const char 
*image_name,
  */
 static int fit_image_write_sig(void *fit, int noffset, uint8_t *value,
int value_len, const char *comment, const char *region_prop,
-   int region_proplen, const char *cmdname)
+   int region_proplen, const char *cmdname, const char *algo_name)
 {
int string_size;
int ret;
@@ -150,6 +150,8 @@ static int fit_image_write_sig(void *fit, int noffset, 
uint8_t *value,
  strdata, sizeof(strdata));
}
}
+   if (algo_name && !ret)
+   ret = fdt_setprop_string(fit, noffset, "algo", algo_name);
 
return ret;
 }
@@ -157,17 +159,18 @@ static int fit_image_write_sig(void *fit, int noffset, 
uint8_t *value,
 static int fit_image_setup_sig(struct image_sign_info *info,
const char *keydir, const char *keyfile, void *fit,
const char *image_name, int noffset, const char *require_keys,
-   const char *engine_id)
+   const char *engine_id, const char *algo_name)
 {
const char *node_name;
-   const char *algo_name;
const char *padding_name;
 
node_name = fit_get_name(fit, noffset, NULL);
-   if (fit_image_hash_get_algo(fit, noffset, _name)) {
-   printf("Can't get algo property for '%s' signature node in '%s' 
image node\n",
-  node_name, image_name);
-   return -1;
+   if (!algo_name) {
+   if (fit_image_hash_get_algo(fit, noffset, _name)) {
+   printf("Can't get algo property for '%s' signature node 
in '%s' image node\n",
+  

[PATCH 2/3] mkimage: Drop unused OPT_STRING constant

[Jan Kiszka]

From: Jan Kiszka 

The actual opt string is inlined - and different. Seems this was a
left-over from older versions of 603e26f76346.

Signed-off-by: Jan Kiszka 
---
 tools/mkimage.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index fbe883ce36..a4844d0f18 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -146,7 +146,6 @@ static int add_content(int type, const char *fname)
return 0;
 }
 
-#define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
 static void process_args(int argc, char **argv)
 {
char *ptr;
-- 
2.31.1

[PATCH 1/3] image-fit: Make string of algo parameter constant

[Jan Kiszka]

From: Jan Kiszka 

Modifications would be invalid.

Signed-off-by: Jan Kiszka 
---
 boot/image-fit-sig.c | 2 +-
 boot/image-fit.c | 8 
 include/image.h  | 2 +-
 tools/image-host.c   | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index 63e5423c92..47d7633568 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -67,7 +67,7 @@ static int fit_image_setup_verify(struct image_sign_info 
*info,
  const void *fit, int noffset,
  int required_keynode, char **err_msgp)
 {
-   char *algo_name;
+   const char *algo_name;
const char *padding_name;
 
if (fdt_totalsize(fit) > CONFIG_VAL(FIT_SIGNATURE_MAX_SIZE)) {
diff --git a/boot/image-fit.c b/boot/image-fit.c
index 33b4a46028..3be6fe6eb9 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -191,7 +191,7 @@ static void fit_image_print_data(const void *fit, int 
noffset, const char *p,
const char *keyname;
uint8_t *value;
int value_len;
-   char *algo;
+   const char *algo;
const char *padding;
bool required;
int ret, i;
@@ -1063,11 +1063,11 @@ int fit_image_get_data_and_size(const void *fit, int 
noffset,
  * 0, on success
  * -1, on failure
  */
-int fit_image_hash_get_algo(const void *fit, int noffset, char **algo)
+int fit_image_hash_get_algo(const void *fit, int noffset, const char **algo)
 {
int len;
 
-   *algo = (char *)fdt_getprop(fit, noffset, FIT_ALGO_PROP, );
+   *algo = (const char *)fdt_getprop(fit, noffset, FIT_ALGO_PROP, );
if (*algo == NULL) {
fit_get_debug(fit, noffset, FIT_ALGO_PROP, len);
return -1;
@@ -1265,7 +1265,7 @@ static int fit_image_check_hash(const void *fit, int 
noffset, const void *data,
 {
uint8_t value[FIT_MAX_HASH_LEN];
int value_len;
-   char *algo;
+   const char *algo;
uint8_t *fit_value;
int fit_value_len;
int ignore;
diff --git a/include/image.h b/include/image.h
index fd662e74b4..16ccc5b10f 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1011,7 +1011,7 @@ int fit_image_get_data_size_unciphered(const void *fit, 
int noffset,
 int fit_image_get_data_and_size(const void *fit, int noffset,
const void **data, size_t *size);
 
-int fit_image_hash_get_algo(const void *fit, int noffset, char **algo);
+int fit_image_hash_get_algo(const void *fit, int noffset, const char **algo);
 int fit_image_hash_get_value(const void *fit, int noffset, uint8_t **value,
int *value_len);
 
diff --git a/tools/image-host.c b/tools/image-host.c
index a6b0a94420..a027155f3c 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -63,7 +63,7 @@ static int fit_image_process_hash(void *fit, const char 
*image_name,
uint8_t value[FIT_MAX_HASH_LEN];
const char *node_name;
int value_len;
-   char *algo;
+   const char *algo;
int ret;
 
node_name = fit_get_name(fit, noffset, NULL);
@@ -160,7 +160,7 @@ static int fit_image_setup_sig(struct image_sign_info *info,
const char *engine_id)
 {
const char *node_name;
-   char *algo_name;
+   const char *algo_name;
const char *padding_name;
 
node_name = fit_get_name(fit, noffset, NULL);
-- 
2.31.1

[PATCH 0/3] mkimage: allow to specify signing algorithm

[Jan Kiszka]

Another step to decouple the FIT image specification from the actual
signing: With these changes, the signature nodes can leave out an algo
property, mkimage will initialize that as well while signing. This way,
in-tree FIT source files can be prepared for gaining signatures without
defining the key type or size upfront, forcing users to patch the code
to change that.

Patch 1 is preparatory for this, patch 2 a drive-by cleanup.

A better solution would actually be if the algorithm was derived from
the provided key. But the underlying crypto layer seems to be rather
unprepared for that.

Jan

Jan Kiszka (3):
  image-fit: Make string of algo parameter constant
  mkimage: Drop unused OPT_STRING constant
  mkimage: Allow to specify the signature algorithm on the command line

 boot/image-fit-sig.c |  2 +-
 boot/image-fit.c |  8 +++
 doc/mkimage.1|  5 +
 include/image.h  |  5 +++--
 tools/fit_image.c|  3 ++-
 tools/image-host.c   | 50 +---
 tools/imagetool.h|  1 +
 tools/mkimage.c  |  6 --
 8 files changed, 49 insertions(+), 31 deletions(-)

-- 
2.31.1

Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)

[Simon Glass]

Hi François,

On Thu, 25 Nov 2021 at 10:01, François Ozog  wrote:
>
> Hi Simon,
>
> On Thu, 25 Nov 2021 at 17:49, Simon Glass  wrote:
>>
>> Hi François,
>>
>> On Thu, 25 Nov 2021 at 08:11, François Ozog  wrote:
>> >
>> > Hi Simon,
>> >
>> >
>> >
>> >
>> > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas 
>> >  wrote:
>> >>
>> >> +cc Sandrine
>> >>
>> >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
>> >>  wrote:
>> >> >
>> >> > Hi Simon,
>> >> >
>> >> >
>> >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass  wrote:
>> >> > >
>> >> > >
>> >> > > This series adds support for the FIP format as used by ARM Trusted
>> >> > > Firmware (in particular TF-A).
>> >> > >
>> >
>> > I will use a question you use often "what problem are you trying to 
>> > solve?". If FIP format is used it means that TF-A/BL2 is going to parse it 
>> > and verify the hashes/signatures according to TF-A scheme.
>> >
>> > The packager will embed in a FIP components like Secure Monitor, Secure 
>> > hypervisor, Secure partitions code and manifests.
>> >
>> > All in all, U-Boot will be representing a small percentage of the 
>> > functionality offered by secure firmware  as a whole and it feels odd to 
>> > make another implementation that is more "accessory" rather than critical 
>> > for the U-Boot community. It may be a good idea but I wish you could 
>> > explain it.
>>
>> Here is a talk about Binman, its goals and how it works.
>>
>> https://www.youtube.com/watch?v=L84ujgUXBOQ
>>
>> Think of Binman as a separate tool that brings everything together. It
>> has grown out of U-Boot, largely because U-Boot is the main firmware
>> in most cases. Getting U-Boot to start up and boot successfully is the
>> goal of this packaging process. There are lots of instructions in the
>> tree and elsewhere about how to build an image comprising U-Boot and
>> various binary blobs. Binman aims to provide documentation for that
>> process and an image description that can be used to build an image
>> reliably.
>>
>> We are slowly converting these text instructions into an in-tree image
>> description.
>>
>> I believe that Binman can help bring order to the chaos that is
>> otherwise only going to grow, with lots of shell scripts, manual
>> instructions, obscure binary tools, etc. Binman brings everything
>> together and makes it clear what is needing/missing to build an image.
>>
>> >
>> >> > > This allows images to be created containing a FIP, which itself 
>> >> > > contains
>> >> > > various binaries. With this, image creation can be handled from an 
>> >> > > in-tree
>> >> > > image description instead of needing to perform a lot of manual steps 
>> >> > > or
>> >> > > custom scripts to build the FIP.
>> >> > >
>> >
>> > That's not my experience of building a fip.  Packaging even Linux as a 
>> > BL33 (instead of U-Boot) is very simple.
>>
>> But not automatic. With Binman you don't need to worry about the
>> packaging. It is done for you. You just need to find all the binary
>> blobs that are needed.  This ability is quite important as firmware is
>> fragmenting and getting very complicated these days.
>>
>> Binman runs twice...once in the U-Boot tree to do a build and again
>> later to repackage, put in a final fdtmap, add signatures and any
>> final pieces needed.
>>
>> See this patch for an example of complicated build instructions with
>> Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
>> in the .rst file):
>>
>> https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-...@chromium.org/
>>
> That's indeed complicated! I can't comment whether this build process is 
> "canonical" as per TF-A standards so I'll let TF-A community comment.
> Have you factored in the signature issues that Jan@Siemens has in the overall 
> process?

In Chrome OS we package up the material that needs to be signed and
send it to a signing server, then get back a key.

You can use 'binman extract' to get the signing data, although perhaps
we could add a special option for it.

You can use 'binman replace' to add the signature in when you get it
back. Again we could create a special path for this if needed.

Regards,
Simon

Re: [PATCH v3 2/2] usb: gadget: Add CDC ACM function

[Pali Rohár]

On Thursday 25 November 2021 18:16:15 Loic Poulain wrote:
> Add support for CDC ACM using the new UDC and gadget API. This protocol
> can be used for serial over USB data transfer and is widely supported
> by various OS (GNU/Linux, MS-Windows, OSX...). The usual purpose of
> such link is to access device debug console and can be useful for
> products not exposing regular UART to the user.
> 
> A default stdio device named 'usbacm' is created, and can be used
> to redirect console to USB link over CDC ACM:
> 
> > setenv stdin usbacm; setenv stdout usbacm
> 
> Signed-off-by: Loic Poulain 
> ---
>  v2: - remove possible infinite recursipe print loop
>  - Remove acmconsole command, start function along the stdio device
>  v3: - Use __constant_cpu_to_le16() when possible
>  - Rename stdio dev to 'usbacm'
>  - Rename init function to drv_usbacm_init

Hello! Just one question: is in this v3 patch fixed stdout-only
configuration, or it still has a bug which you described in email?
https://lore.kernel.org/u-boot/CAMZdPi8185pAiO4w2QYMtWGAFJiX=ej_bjw1cusre990wr7...@mail.gmail.com/

Because neither in above change long nor in driver help description nor
in driver source code is any comment related to stdout-only device.

Note that stdout-only does not have to be too uncommon. For example
Nokia N900 has enabled UART, keyboard+lcd and usbtty by default and
sometimes I enabled input only from keyboard and looked at usbtty
terminal (as stdout-only) if everything is working fine (that every
pressed key on keyboard was echoed correctly back to usbtty).

> 
>  common/stdio.c  |   3 +
>  drivers/usb/gadget/Kconfig  |   9 +
>  drivers/usb/gadget/Makefile |   1 +
>  drivers/usb/gadget/f_acm.c  | 672 
> 
>  include/stdio_dev.h |   1 +
>  5 files changed, 686 insertions(+)
>  create mode 100644 drivers/usb/gadget/f_acm.c
> 
> diff --git a/common/stdio.c b/common/stdio.c
> index 4083e4e..e07e4a4 100644
> --- a/common/stdio.c
> +++ b/common/stdio.c
> @@ -381,6 +381,9 @@ int stdio_add_devices(void)
>  #ifdef CONFIG_USB_TTY
>   drv_usbtty_init();
>  #endif
> +#ifdef CONFIG_USB_FUNCTION_ACM
> + drv_usbacm_init ();
> +#endif
>   if (IS_ENABLED(CONFIG_NETCONSOLE))
>   drv_nc_init();
>  #ifdef CONFIG_JTAG_CONSOLE
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index 327ea86..d81a9c5 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -182,6 +182,15 @@ config USB_FUNCTION_THOR
> Enable Tizen's THOR download protocol support in U-Boot. It
> allows downloading images into memory and flash them to target device.
>  
> +config USB_FUNCTION_ACM
> + bool "Enable CDC ACM gadget"
> + select SYS_STDIO_DEREGISTER
> + select CIRCBUF
> + help
> +   ACM serial link. This function can be used to create a stdio device to
> +   interoperate with MS-Windows hosts or with the Linux-USB "cdc-acm"
> +   driver.
> +
>  endif # USB_GADGET_DOWNLOAD
>  
>  config USB_ETHER
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index f560068..d5d891b 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_USB_FUNCTION_MASS_STORAGE) += f_mass_storage.o
>  obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += f_fastboot.o
>  obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o
>  obj-$(CONFIG_USB_FUNCTION_ROCKUSB) += f_rockusb.o
> +obj-$(CONFIG_USB_FUNCTION_ACM)   += f_acm.o
>  endif
>  endif
>  ifdef CONFIG_USB_ETHER
> diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
> new file mode 100644
> index 000..388f73d
> --- /dev/null
> +++ b/drivers/usb/gadget/f_acm.c
> @@ -0,0 +1,672 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * USB CDC serial (ACM) function driver
> + *
> + * Copyright (C) 2003 Al Borchers (alborch...@steinerpoint.com)
> + * Copyright (C) 2008 by David Brownell
> + * Copyright (C) 2008 by Nokia Corporation
> + * Copyright (C) 2009 by Samsung Electronics
> + * Copyright (c) 2021, Linaro Ltd 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define REQ_SIZE_MAX 512
> +
> +struct f_acm {
> + int ctrl_id;
> + int data_id;
> +
> + struct usb_ep *ep_in;
> + struct usb_ep *ep_out;
> + struct usb_ep *ep_notify;
> +
> + struct usb_request *req_in;
> + struct usb_request *req_out;
> +
> + bool connected;
> + bool tx_on;
> +
> + circbuf_t rx_buf;
> + circbuf_t tx_buf;
> +
> + struct usb_function usb_function;
> +
> + struct usb_cdc_line_coding line_coding;
> + u16 handshake_bits;
> +#define ACM_CTRL_RTS BIT(1)  /* unused with full duplex */
> +#define ACM_CTRL_DTR BIT(0)  /* host is ready for data r/w */
> +
> + int controller_index;
> +};
> +
> +static struct f_acm 

[PATCH] rockchip: boot_mode: fix fastboot command

[John Keeping]

The USB controller index must be separated from the type argument,
otherwise the preboot command fails with the error:

Error: Wrong USB controller index format

Add the missing space to fix fastboot mode here.

Signed-off-by: John Keeping 
---

 arch/arm/mach-rockchip/boot_mode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/boot_mode.c 
b/arch/arm/mach-rockchip/boot_mode.c
index 2158934159..1a1a887fc2 100644
--- a/arch/arm/mach-rockchip/boot_mode.c
+++ b/arch/arm/mach-rockchip/boot_mode.c
@@ -95,7 +95,7 @@ int setup_boot_mode(void)
switch (boot_mode) {
case BOOT_FASTBOOT:
debug("%s: enter fastboot!\n", __func__);
-   env_set("preboot", "setenv preboot; fastboot usb0");
+   env_set("preboot", "setenv preboot; fastboot usb 0");
break;
case BOOT_UMS:
debug("%s: enter UMS!\n", __func__);
-- 
2.34.1

[PATCH 1/1] efi_selftest: simplify endian conversion for FDT test

[Heinrich Schuchardt]

UEFI code is always little-endian. Remove a superfluous test.

Remove a superfluous type conversion.

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_selftest/efi_selftest_fdt.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_fdt.c 
b/lib/efi_selftest/efi_selftest_fdt.c
index eae98208f6..412ba286ea 100644
--- a/lib/efi_selftest/efi_selftest_fdt.c
+++ b/lib/efi_selftest/efi_selftest_fdt.c
@@ -23,23 +23,24 @@ static const char *fdt;
 static const efi_guid_t fdt_guid = EFI_FDT_GUID;
 static const efi_guid_t acpi_guid = EFI_ACPI_TABLE_GUID;
 
-/*
- * Convert FDT value to host endianness.
+/**
+ * f2h() - convert FDT value to host endianness.
  *
- * @valFDT value
- * @return converted value
+ * UEFI code is always low endian. The FDT is big endian.
+ *
+ * @val:   FDT value
+ * Return: converted value
  */
 static uint32_t f2h(fdt32_t val)
 {
char *buf = (char *)
char i;
 
-#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
/* Swap the bytes */
i = buf[0]; buf[0] = buf[3]; buf[3] = i;
i = buf[1]; buf[1] = buf[2]; buf[2] = i;
-#endif
-   return *(uint32_t *)buf;
+
+   return val;
 }
 
 /**
-- 
2.32.0

[PATCH 4/4] rockchip: rk3399: Add support for chromebook_kevin

[Alper Nebi Yasak]

From: "Marty E. Plummer" 

Add support for Kevin, an RK3399-based convertible chromebook that is
very similar to Bob. This patch is mostly based on existing support for
Bob, with only minor changes for Kevin-specific things.

Unlike other Gru boards, coreboot sets Kevin's center logic to 925 mV,
so adjust it here in the dts as well. The rk3399-gru-kevin devicetree
has an unknown event code reference which has to be defined, set it
to the Linux counterpart. The new defconfig is copied from Bob with the
diffconfig:

 DEFAULT_DEVICE_TREE "rk3399-gru-bob" -> "rk3399-gru-kevin"
 DEFAULT_FDT_FILE "rockchip/rk3399-gru-bob.dtb" -> 
"rockchip/rk3399-gru-kevin.dtb"
 VIDEO_ROCKCHIP_MAX_XRES 1280 -> 2400
 VIDEO_ROCKCHIP_MAX_YRES 800 -> 1600
+TARGET_CHROMEBOOK_KEVIN y

With this Kevin can boot from SPI flash to a usable U-Boot prompt on the
display with the keyboard working, but cannot boot into Linux for
unknown reasons.

eMMC starts in a working state but fails to re-init, microSD card works
but at a lower-than-expected speed, USB works but causes a hang on
de-init. There are known workarounds to solve eMMC and USB issues.

Cc: Marty E. Plummer 
Cc: Simon Glass 
[Alper: commit message, resync config with Bob, update MAINTAINERS,
add to Rockchip doc, add Kconfig help message, set regulator]
Co-developed-by: Alper Nebi Yasak 
Signed-off-by: Alper Nebi Yasak 
---
Marty had signed-off an earlier version of this [1], but not the updated
version I continued on top of [2]. So I'm not sure if I can add his
sign-off to this as is, and I hope he can reply to this with a sign-off.

[1] https://patchwork.ozlabs.org/patch/1053386/
[2] https://patchwork.ozlabs.org/comment/2488899/

 arch/arm/dts/Makefile |   1 +
 arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi |  11 ++
 arch/arm/mach-rockchip/rk3399/Kconfig |  11 ++
 arch/arm/mach-rockchip/rk3399/rk3399.c|   4 +-
 arch/arm/mach-rockchip/spl.c  |   3 +-
 board/google/gru/Kconfig  |  16 +++
 board/google/gru/MAINTAINERS  |   8 ++
 board/google/gru/gru.c|   2 +-
 configs/chromebook_kevin_defconfig| 116 ++
 doc/board/rockchip/rockchip.rst   |   1 +
 include/dt-bindings/input/linux-event-codes.h |   3 +-
 11 files changed, 171 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi
 create mode 100644 configs/chromebook_kevin_defconfig

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 7f622fedbda7..d6883994f21a 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -132,6 +132,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3399) += \
rk3399-ficus.dtb \
rk3399-firefly.dtb \
rk3399-gru-bob.dtb \
+   rk3399-gru-kevin.dtb \
rk3399-khadas-edge.dtb \
rk3399-khadas-edge-captain.dtb \
rk3399-khadas-edge-v.dtb \
diff --git a/arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi 
b/arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi
new file mode 100644
index ..c03bd48e95d7
--- /dev/null
+++ b/arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Jagan Teki 
+ */
+
+#include "rk3399-gru-u-boot.dtsi"
+#include "rk3399-sdram-lpddr3-samsung-4GB-1866.dtsi"
+
+_centerlogic_pwm {
+   regulator-init-microvolt = <925000>;
+};
diff --git a/arch/arm/mach-rockchip/rk3399/Kconfig 
b/arch/arm/mach-rockchip/rk3399/Kconfig
index 17628f917127..0833e083d9ef 100644
--- a/arch/arm/mach-rockchip/rk3399/Kconfig
+++ b/arch/arm/mach-rockchip/rk3399/Kconfig
@@ -14,6 +14,17 @@ config TARGET_CHROMEBOOK_BOB
  display. It includes a Chrome OS EC (Cortex-M3) to provide access to
  the keyboard and battery functions.
 
+config TARGET_CHROMEBOOK_KEVIN
+   bool "Samsung Chromebook Plus (RK3399)"
+   select HAS_ROM
+   select ROCKCHIP_SPI_IMAGE
+   help
+ Kevin is a RK3399-based convertible chromebook. It has two USB 3.0
+ Type-C ports, 4GB of SDRAM, WiFi and a 12.3" 2400x1600 display. It
+ uses its USB ports for both power and external display. It includes
+ a Chromium OS EC (Cortex-M3) to provide access to the keyboard and
+ battery functions.
+
 config TARGET_EVB_RK3399
bool "RK3399 evaluation board"
help
diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c 
b/arch/arm/mach-rockchip/rk3399/rk3399.c
index 2bc8e60b99ba..e486f69e48c8 100644
--- a/arch/arm/mach-rockchip/rk3399/rk3399.c
+++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
@@ -118,7 +118,7 @@ void board_debug_uart_init(void)
 #define GPIO0_BASE 0xff72
 #define PMUGRF_BASE0xff32
struct rk3399_grf_regs * const grf = (void *)GRF_BASE;
-#ifdef CONFIG_TARGET_CHROMEBOOK_BOB
+#if defined(CONFIG_TARGET_CHROMEBOOK_BOB) || 
defined(CONFIG_TARGET_CHROMEBOOK_KEVIN)
struct rk3399_pmugrf_regs * const pmugrf = (void *)PMUGRF_BASE;
  

[PATCH 3/4] rockchip: bob: Enable more configs

[Alper Nebi Yasak]

This patch enables some configs that should be working on the Bob board,
based on what is observed to work on the Kevin board.

The Bob board uses an Embedded DisplayPort panel compatible with the
simple panel and Rockchip eDP drivers. Its backlight is controlled by
the Chromium OS Embedded Controller Pulse Width Modulator. Enable these
for the board.

Also set VIDEO_ROCKCHIP_MAX_{XRES,YRES} to 1280x800, the resolution of
its panel. This had to be done for the Kevin board, but it's untested if
this is actually necessary for Bob.

The Rockchip video driver needs to assert/deassert some resets, so also
enable the reset controller. RESET_ROCKCHIP defaults to y for this board
when DM_RESET=y, so it's enough to set that.

The Bob board has two USB 3.0 Type-C ports and one USB 2.0 Type-A port
on its right side. Enable the configs relevant to USB devices so these
can be used. This is despite a known issue with RK3399 boards where USB
de-init causes a hang, as there is a known workaround.

Some other rk3399-based devices enable support for the SoC's random
number generator in commit a475bef5340c ("configs: rk3399: enable rng on
firefly/rock960/rockpro64"), as it can provide a KASLR seed when booting
using UEFI. Enable it for Bob as well.

The default misc_init_r() for Rockchip boards sets cpuid and ethernet
MAC address based on e-fuse block. A previous patch extends this on Gru
boards to set registers related to SoC IO domains as is necessary on
these boards. Enable this function and configs for it on Bob.

The eMMC on this board is capable of running at a HS400 Enhanced Strobe
configuration, and the microSD slot at Ultra High Speed SDR104. Enable
the configs for these as the hardware supports these modes. There are
problems causing the devices to run at lower speeds, but these configs
are enabled in hope that these will be solved later. Enabling ADMA
currently makes the eMMC stop working, so it is kept disabled.

The microSD card slot on this board (and others based on Gru) is
connected to a GPIO controlled regulator (ppvar-sd-card-io), which must
be operable by U-Boot. Enable the relevant config option to allow this.

Bob boards also use the Winbond W25Q64DW SPI flash chip, enable support
for Winbond SPI flash chips in the board config so U-Boot can boot with
this chip.

Signed-off-by: Alper Nebi Yasak 
---

 configs/chromebook_bob_defconfig | 27 ++-
 include/configs/gru.h|  3 +++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/configs/chromebook_bob_defconfig b/configs/chromebook_bob_defconfig
index fe938c659172..048fa8e0c043 100644
--- a/configs/chromebook_bob_defconfig
+++ b/configs/chromebook_bob_defconfig
@@ -21,6 +21,7 @@ CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-gru-bob.dtb"
 # CONFIG_DISPLAY_CPUINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_BOARD_EARLY_INIT_R=y
+CONFIG_MISC_INIT_R=y
 CONFIG_BLOBLIST=y
 CONFIG_BLOBLIST_SIZE=0x1000
 CONFIG_BLOBLIST_ADDR=0x10
@@ -52,26 +53,40 @@ CONFIG_ROCKCHIP_GPIO=y
 CONFIG_I2C_CROS_EC_TUNNEL=y
 CONFIG_SYS_I2C_ROCKCHIP=y
 CONFIG_I2C_MUX=y
-CONFIG_DM_KEYBOARD=y
 CONFIG_CROS_EC_KEYB=y
+CONFIG_MISC=y
+CONFIG_ROCKCHIP_EFUSE=y
 CONFIG_CROS_EC=y
 CONFIG_CROS_EC_SPI=y
 CONFIG_PWRSEQ=y
 CONFIG_MMC_PWRSEQ=y
+CONFIG_MMC_IO_VOLTAGE=y
+CONFIG_MMC_UHS_SUPPORT=y
+CONFIG_MMC_HS400_ES_SUPPORT=y
+CONFIG_MMC_HS400_SUPPORT=y
 CONFIG_MMC_DW=y
 CONFIG_MMC_DW_ROCKCHIP=y
 CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_ROCKCHIP=y
 CONFIG_SF_DEFAULT_BUS=1
 CONFIG_SF_DEFAULT_SPEED=2000
 CONFIG_SPI_FLASH_GIGADEVICE=y
+CONFIG_SPI_FLASH_WINBOND=y
 CONFIG_DM_ETH=y
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_GMAC_ROCKCHIP=y
+CONFIG_PHY_ROCKCHIP_INNO_USB2=y
+CONFIG_PHY_ROCKCHIP_TYPEC=y
 CONFIG_PMIC_RK8XX=y
 CONFIG_REGULATOR_PWM=y
+CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_REGULATOR_RK8XX=y
+CONFIG_PWM_CROS_EC=y
 CONFIG_PWM_ROCKCHIP=y
+CONFIG_DM_RESET=y
+CONFIG_DM_RNG=y
+CONFIG_RNG_ROCKCHIP=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_ROCKCHIP_SPI=y
 CONFIG_SYSRESET=y
@@ -80,11 +95,21 @@ CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
+CONFIG_USB_OHCI_HCD=y
+CONFIG_USB_OHCI_GENERIC=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_KEYBOARD=y
 CONFIG_USB_HOST_ETHER=y
 CONFIG_USB_ETHER_ASIX=y
 CONFIG_USB_ETHER_ASIX88179=y
 CONFIG_USB_ETHER_MCS7830=y
 CONFIG_USB_ETHER_RTL8152=y
 CONFIG_USB_ETHER_SMSC95XX=y
+CONFIG_DM_VIDEO=y
+CONFIG_DISPLAY=y
+CONFIG_VIDEO_ROCKCHIP=y
+CONFIG_VIDEO_ROCKCHIP_MAX_XRES=1280
+CONFIG_VIDEO_ROCKCHIP_MAX_YRES=800
+CONFIG_DISPLAY_ROCKCHIP_EDP=y
 CONFIG_CMD_DHRYSTONE=y
 CONFIG_ERRNO_STR=y
diff --git a/include/configs/gru.h b/include/configs/gru.h
index be2dc79968c0..b1084bb21d4d 100644
--- a/include/configs/gru.h
+++ b/include/configs/gru.h
@@ -13,4 +13,7 @@
 
 #include 
 
+#define CONFIG_USB_OHCI_NEW
+#define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS 2
+
 #endif
-- 
2.34.0

[PATCH 2/4] rockchip: gru: Add more devicetree settings

[Alper Nebi Yasak]

From: Simon Glass 

This adds some devicetree settings for the Gru-based boards, based on
what works on a Kevin board.

Gru-based boards usually have an 8MiB SPI flash chip and boot from it.
Make the u-boot.rom file intended to be flashed on it match its size.
Add properties for booting from SPI, and only try to boot from SPI as
MMC and SD card don't seem to work in SPL yet.

The Chromium OS EC needs a delay between transactions so it can get
itself ready. Also it currently uses a non-standard way of specifying
the interrupt. Add these so that the EC works reliably.

The Rockchip Embedded DisplayPort driver is looking for a rockchip,panel
property to find the panel it should work on. Add the property for the
Gru-based boards.

The U-Boot GPIO controlled regulator driver only considers the
"enable-gpios" devicetree property, not the singular "enable-gpio" one.
Some devicetree source files have the singular form as they were added
to Linux kernel when it used that form, and imported to U-Boot as is.
Fix one instance of this in the Gru boards' devicetree to the form that
works in U-Boot.

The PWM controlled regulator driver complains that there is no init
voltage set for a regulator it drives, though it's not clear which one.
Set them all to the voltage levels coreboot sets them: 900 mV.

The RK3399 SoC needs to know the voltage level that some supplies
provides, including one fixed 1.8V audio-related regulator. Although
this synchronization is currently statically done in the board init
functions, a not-so-hypothetical driver that does this dynamically would
query the regulator only to get -ENODATA and be confused. Make sure
U-Boot knows this supply is at 1.8V by setting its limits to that.

Most of this is a reapplication of commit 08c85b57a5ec ("rockchip: gru:
Add extra device-tree settings") whose changes were removed during a
sync with Linux at commit 167efc2c7a46 ("arm64: dts: rk3399: Sync
v5.7-rc1 from Linux"). Apply things to rk3399-gru-u-boot.dtsi instead so
they don't get lost again.

Signed-off-by: Simon Glass 
[Alper: move to -u-boot.dtsi, rewrite commit message, add more nodes]
Co-developed-by: Alper Nebi Yasak 
Signed-off-by: Alper Nebi Yasak 
---
Kept sign-off and author as Simon based on the aforementioned commit.

 arch/arm/dts/rk3399-gru-u-boot.dtsi | 55 +
 1 file changed, 55 insertions(+)

diff --git a/arch/arm/dts/rk3399-gru-u-boot.dtsi 
b/arch/arm/dts/rk3399-gru-u-boot.dtsi
index 390ac2bb5a9a..33734e99be50 100644
--- a/arch/arm/dts/rk3399-gru-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-gru-u-boot.dtsi
@@ -5,6 +5,61 @@
 
 #include "rk3399-u-boot.dtsi"
 
+/ {
+   chosen {
+   u-boot,spl-boot-order = _flash;
+   };
+
+   config {
+   u-boot,spl-payload-offset = <0x4>;
+   };
+};
+
+ {
+   rom {
+   size = <0x80>;
+   };
+};
+
+_ec {
+   ec-interrupt = < RK_PA1 GPIO_ACTIVE_LOW>;
+};
+
+ {
+   rockchip,panel = <_panel>;
+};
+
+_audio {
+   regulator-min-microvolt = <180>;
+   regulator-max-microvolt = <180>;
+};
+
+_bigcpu_pwm {
+   regulator-init-microvolt = <90>;
+};
+
+_centerlogic_pwm {
+   regulator-init-microvolt = <90>;
+};
+
+_gpu_pwm {
+   regulator-init-microvolt = <90>;
+};
+
+_litcpu_pwm {
+   regulator-init-microvolt = <90>;
+};
+
+_sd_card_io {
+   enable-gpios = < 2 GPIO_ACTIVE_HIGH>;
+};
+
+ {
+   spi-activate-delay = <100>;
+   spi-max-frequency = <300>;
+   spi-deactivate-delay = <200>;
+};
+
 _flash {
u-boot,dm-pre-reloc;
 };
-- 
2.34.0

[PATCH 1/4] rockchip: gru: Set up SoC IO domain registers

[Alper Nebi Yasak]

The RK3399 SoC needs to know the voltage value provided by some
regulators, which is done by setting relevant register bits. Configure
these the way other RK3399 boards do, but with values set in coreboot.

Signed-off-by: Alper Nebi Yasak 
---
There is a driver for this on Rockchip's repo, I managed to forward-port
it and get it working. If that's more desirable than doing it per-board
like this I can send that upstream (but I'd prefer to do it after this).

 board/google/gru/gru.c | 54 ++
 1 file changed, 54 insertions(+)

diff --git a/board/google/gru/gru.c b/board/google/gru/gru.c
index 23080c1798b7..cddcb286a380 100644
--- a/board/google/gru/gru.c
+++ b/board/google/gru/gru.c
@@ -6,6 +6,17 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define GRF_IO_VSEL_BT656_SHIFT 0
+#define GRF_IO_VSEL_AUDIO_SHIFT 1
+#define PMUGRF_CON0_VSEL_SHIFT 8
+#define PMUGRF_CON0_VOL_SHIFT 9
 
 #ifdef CONFIG_SPL_BUILD
 /* provided to defeat compiler optimisation in board_init_f() */
@@ -54,3 +65,46 @@ int board_early_init_r(void)
return 0;
 }
 #endif
+
+#ifdef CONFIG_MISC_INIT_R
+static void setup_iodomain(void)
+{
+   struct rk3399_grf_regs *grf =
+  syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
+   struct rk3399_pmugrf_regs *pmugrf =
+  syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
+
+   /* BT656 and audio is in 1.8v domain */
+   rk_setreg(>io_vsel, (1 << GRF_IO_VSEL_BT656_SHIFT |
+ 1 << GRF_IO_VSEL_AUDIO_SHIFT));
+
+   /*
+* Set GPIO1 1.8v/3.0v source select to PMU1830_VOL
+* and explicitly configure that PMU1830_VOL to be 1.8V
+*/
+   rk_setreg(>soc_con0, (1 << PMUGRF_CON0_VSEL_SHIFT |
+ 1 << PMUGRF_CON0_VOL_SHIFT));
+}
+
+int misc_init_r(void)
+{
+   const u32 cpuid_offset = 0x7;
+   const u32 cpuid_length = 0x10;
+   u8 cpuid[cpuid_length];
+   int ret;
+
+   setup_iodomain();
+
+   ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);
+   if (ret)
+   return ret;
+
+   ret = rockchip_cpuid_set(cpuid, cpuid_length);
+   if (ret)
+   return ret;
+
+   ret = rockchip_setup_macaddr();
+
+   return ret;
+}
+#endif
-- 
2.34.0

[PATCH 0/4] rockchip: Improve support for Bob chromebook and add support for Kevin

[Alper Nebi Yasak]

I have recently started testing booting U-Boot from SPI on my gru-kevin
(as opposed to chainloading it from vendor coreboot + depthcharge) and
brought it to a better working state based on an initial support patch
from Marty [1][2] and some follow-up work by Simon [3].

I tried to keep them as the git author when I took things from their
work, but squashing other changes into those and rewriting commit
messages makes things a bit weird in my opinion, especially for keeping
their signoff. Do tell me if there is a better way to that.

As the Kevin and Bob boards are very similar. I assumed the config and
devicetree changes will be appropriate for Bob as well, and applied them
to it first. I do not have a Bob, so could not test on one myself, but
Simon did test an earlier version of this and it appears to work [4].

Other useful things for these boards:
- Patch to fix a hang when usb controllers exit [5]
- Series to support HS400ES mode as HS400 training fails [6]
- Hack to skip eMMC reinitialization so it keeps working [7]

[1] https://patchwork.ozlabs.org/patch/1053386/
[2] https://patchwork.ozlabs.org/comment/2488899/
[3] https://github.com/sjg20/u-boot/commits/kevin
[4] 
https://lore.kernel.org/u-boot/capnjgz23jd92y9ni8yw1ftl0a1sjhgouoakx13zkokn6t-s...@mail.gmail.com/
[5] 
https://patchwork.ozlabs.org/project/uboot/patch/20210406151059.1187379-1-icen...@aosc.io/
[6] https://patchwork.ozlabs.org/project/uboot/list/?series=269768
[7] https://patchwork.ozlabs.org/comment/2779784/


Alper Nebi Yasak (2):
  rockchip: gru: Set up SoC IO domain registers
  rockchip: bob: Enable more configs

Marty E. Plummer (1):
  rockchip: rk3399: Add support for chromebook_kevin

Simon Glass (1):
  rockchip: gru: Add more devicetree settings

 arch/arm/dts/Makefile |   1 +
 arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi |  11 ++
 arch/arm/dts/rk3399-gru-u-boot.dtsi   |  55 +
 arch/arm/mach-rockchip/rk3399/Kconfig |  11 ++
 arch/arm/mach-rockchip/rk3399/rk3399.c|   4 +-
 arch/arm/mach-rockchip/spl.c  |   3 +-
 board/google/gru/Kconfig  |  16 +++
 board/google/gru/MAINTAINERS  |   8 ++
 board/google/gru/gru.c|  56 -
 configs/chromebook_bob_defconfig  |  27 +++-
 configs/chromebook_kevin_defconfig| 116 ++
 doc/board/rockchip/rockchip.rst   |   1 +
 include/configs/gru.h |   3 +
 include/dt-bindings/input/linux-event-codes.h |   3 +-
 14 files changed, 309 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi
 create mode 100644 configs/chromebook_kevin_defconfig

-- 
2.34.0

Re: rk3399-gru-kevin: issues on bringup

[Alper Nebi Yasak]

On 25/11/2021 03:12, Simon Glass wrote:
> On Sun, 7 Nov 2021 at 10:26, Alper Nebi Yasak  
> wrote:
>> I did so and updated the branches, now the series goes like this:
>>
>>   rockchip: gru: Set up SoC IO domain registers
>>   rockchip: gru: Add more devicetree settings
>>   rockchip: bob: Enable more configs
>>   rockchip: rk3399: Add support for chromebook_kevin
>>
>> [...]
> 
> Are you going to send your patches?
> 
> I haven't got as far as booting into Linux, nor trying some other
> kernel, but we should get these patches in...

I got sidetracked while waiting for a reply from Marty... I'll send them
now and hope for the best.

[PATCH v3 1/2] lib/circbuf: Make circbuf selectable symbol

[Loic Poulain]

It is currenly only used from usbtty driver but make it properly
selectable via Kconfig symbol, for future usage.

Signed-off-by: Loic Poulain 
---
 v2: no change
 v3: add symbol description

 lib/Kconfig  | 3 +++
 lib/Makefile | 8 +++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index c535147..4c6ac2b 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -290,6 +290,9 @@ config TRACE_EARLY_ADDR
  the size is too small then the message which says the amount of early
  data being coped will the the same as the
 
+config CIRCBUF
+   bool "Enable circular buffer support"
+
 source lib/dhry/Kconfig
 
 menu "Security support"
diff --git a/lib/Makefile b/lib/Makefile
index 8ba745f..4daeee2 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -29,7 +29,13 @@ ifneq ($(CONFIG_CHARSET),)
 obj-y += charset.o
 endif
 endif
-obj-$(CONFIG_USB_TTY) += circbuf.o
+
+ifdef CONFIG_USB_TTY
+obj-y += circbuf.o
+else
+obj-$(CONFIG_CIRCBUF) += circbuf.o
+endif
+
 obj-y += crc8.o
 obj-y += crc16.o
 obj-$(CONFIG_ERRNO_STR) += errno_str.o
-- 
2.7.4

[PATCH v3 2/2] usb: gadget: Add CDC ACM function

[Loic Poulain]

Add support for CDC ACM using the new UDC and gadget API. This protocol
can be used for serial over USB data transfer and is widely supported
by various OS (GNU/Linux, MS-Windows, OSX...). The usual purpose of
such link is to access device debug console and can be useful for
products not exposing regular UART to the user.

A default stdio device named 'usbacm' is created, and can be used
to redirect console to USB link over CDC ACM:

> setenv stdin usbacm; setenv stdout usbacm

Signed-off-by: Loic Poulain 
---
 v2: - remove possible infinite recursipe print loop
 - Remove acmconsole command, start function along the stdio device
 v3: - Use __constant_cpu_to_le16() when possible
 - Rename stdio dev to 'usbacm'
 - Rename init function to drv_usbacm_init

 common/stdio.c  |   3 +
 drivers/usb/gadget/Kconfig  |   9 +
 drivers/usb/gadget/Makefile |   1 +
 drivers/usb/gadget/f_acm.c  | 672 
 include/stdio_dev.h |   1 +
 5 files changed, 686 insertions(+)
 create mode 100644 drivers/usb/gadget/f_acm.c

diff --git a/common/stdio.c b/common/stdio.c
index 4083e4e..e07e4a4 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -381,6 +381,9 @@ int stdio_add_devices(void)
 #ifdef CONFIG_USB_TTY
drv_usbtty_init();
 #endif
+#ifdef CONFIG_USB_FUNCTION_ACM
+   drv_usbacm_init ();
+#endif
if (IS_ENABLED(CONFIG_NETCONSOLE))
drv_nc_init();
 #ifdef CONFIG_JTAG_CONSOLE
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 327ea86..d81a9c5 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -182,6 +182,15 @@ config USB_FUNCTION_THOR
  Enable Tizen's THOR download protocol support in U-Boot. It
  allows downloading images into memory and flash them to target device.
 
+config USB_FUNCTION_ACM
+   bool "Enable CDC ACM gadget"
+   select SYS_STDIO_DEREGISTER
+   select CIRCBUF
+   help
+ ACM serial link. This function can be used to create a stdio device to
+ interoperate with MS-Windows hosts or with the Linux-USB "cdc-acm"
+ driver.
+
 endif # USB_GADGET_DOWNLOAD
 
 config USB_ETHER
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index f560068..d5d891b 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_USB_FUNCTION_MASS_STORAGE) += f_mass_storage.o
 obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += f_fastboot.o
 obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o
 obj-$(CONFIG_USB_FUNCTION_ROCKUSB) += f_rockusb.o
+obj-$(CONFIG_USB_FUNCTION_ACM) += f_acm.o
 endif
 endif
 ifdef CONFIG_USB_ETHER
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
new file mode 100644
index 000..388f73d
--- /dev/null
+++ b/drivers/usb/gadget/f_acm.c
@@ -0,0 +1,672 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * USB CDC serial (ACM) function driver
+ *
+ * Copyright (C) 2003 Al Borchers (alborch...@steinerpoint.com)
+ * Copyright (C) 2008 by David Brownell
+ * Copyright (C) 2008 by Nokia Corporation
+ * Copyright (C) 2009 by Samsung Electronics
+ * Copyright (c) 2021, Linaro Ltd 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define REQ_SIZE_MAX   512
+
+struct f_acm {
+   int ctrl_id;
+   int data_id;
+
+   struct usb_ep *ep_in;
+   struct usb_ep *ep_out;
+   struct usb_ep *ep_notify;
+
+   struct usb_request *req_in;
+   struct usb_request *req_out;
+
+   bool connected;
+   bool tx_on;
+
+   circbuf_t rx_buf;
+   circbuf_t tx_buf;
+
+   struct usb_function usb_function;
+
+   struct usb_cdc_line_coding line_coding;
+   u16 handshake_bits;
+#define ACM_CTRL_RTS   BIT(1)  /* unused with full duplex */
+#define ACM_CTRL_DTR   BIT(0)  /* host is ready for data r/w */
+
+   int controller_index;
+};
+
+static struct f_acm *default_acm_function;
+
+static inline struct f_acm *func_to_acm(struct usb_function *f)
+{
+   return container_of(f, struct f_acm, usb_function);
+}
+
+static inline struct f_acm *stdio_to_acm(struct stdio_dev *s)
+{
+   /* stdio dev is cloned on registration, do not use container_of */
+   return s->priv;
+}
+
+static struct usb_interface_assoc_descriptor
+acm_iad_descriptor = {
+   .bLength =  sizeof(acm_iad_descriptor),
+   .bDescriptorType =  USB_DT_INTERFACE_ASSOCIATION,
+   .bFirstInterface =  0,
+   .bInterfaceCount =  2,  // control + data
+   .bFunctionClass =   USB_CLASS_COMM,
+   .bFunctionSubClass =USB_CDC_SUBCLASS_ACM,
+   .bFunctionProtocol =USB_CDC_ACM_PROTO_AT_V25TER,
+};
+
+static struct usb_interface_descriptor acm_control_intf_desc = {
+   .bLength =  USB_DT_INTERFACE_SIZE,
+   .bDescriptorType =  USB_DT_INTERFACE,
+   .bNumEndpoints =1,

Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)

[François Ozog]

Hi Simon,

On Thu, 25 Nov 2021 at 17:49, Simon Glass  wrote:

> Hi François,
>
> On Thu, 25 Nov 2021 at 08:11, François Ozog 
> wrote:
> >
> > Hi Simon,
> >
> >
> >
> >
> > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas <
> ilias.apalodi...@linaro.org> wrote:
> >>
> >> +cc Sandrine
> >>
> >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
> >>  wrote:
> >> >
> >> > Hi Simon,
> >> >
> >> >
> >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass  wrote:
> >> > >
> >> > >
> >> > > This series adds support for the FIP format as used by ARM Trusted
> >> > > Firmware (in particular TF-A).
> >> > >
> >
> > I will use a question you use often "what problem are you trying to
> solve?". If FIP format is used it means that TF-A/BL2 is going to parse it
> and verify the hashes/signatures according to TF-A scheme.
> >
> > The packager will embed in a FIP components like Secure Monitor, Secure
> hypervisor, Secure partitions code and manifests.
> >
> > All in all, U-Boot will be representing a small percentage of the
> functionality offered by secure firmware  as a whole and it feels odd to
> make another implementation that is more "accessory" rather than critical
> for the U-Boot community. It may be a good idea but I wish you could
> explain it.
>
> Here is a talk about Binman, its goals and how it works.
>
> https://www.youtube.com/watch?v=L84ujgUXBOQ
>
> Think of Binman as a separate tool that brings everything together. It
> has grown out of U-Boot, largely because U-Boot is the main firmware
> in most cases. Getting U-Boot to start up and boot successfully is the
> goal of this packaging process. There are lots of instructions in the
> tree and elsewhere about how to build an image comprising U-Boot and
> various binary blobs. Binman aims to provide documentation for that
> process and an image description that can be used to build an image
> reliably.
>
> We are slowly converting these text instructions into an in-tree image
> description.
>
> I believe that Binman can help bring order to the chaos that is
> otherwise only going to grow, with lots of shell scripts, manual
> instructions, obscure binary tools, etc. Binman brings everything
> together and makes it clear what is needing/missing to build an image.
>
> >
> >> > > This allows images to be created containing a FIP, which itself
> contains
> >> > > various binaries. With this, image creation can be handled from an
> in-tree
> >> > > image description instead of needing to perform a lot of manual
> steps or
> >> > > custom scripts to build the FIP.
> >> > >
> >
> > That's not my experience of building a fip.  Packaging even Linux as a
> BL33 (instead of U-Boot) is very simple.
>
> But not automatic. With Binman you don't need to worry about the
> packaging. It is done for you. You just need to find all the binary
> blobs that are needed.  This ability is quite important as firmware is
> fragmenting and getting very complicated these days.
>
> Binman runs twice...once in the U-Boot tree to do a build and again
> later to repackage, put in a final fdtmap, add signatures and any
> final pieces needed.
>
> See this patch for an example of complicated build instructions with
> Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
> in the .rst file):
>
>
> https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-...@chromium.org/
>
> That's indeed complicated! I can't comment whether this build process is
"canonical" as per TF-A standards so I'll let TF-A community comment.
Have you factored in the signature issues that Jan@Siemens has in the
overall process?


> Regards,
> Simon
>


-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.o...@linaro.org | Skype: ffozog

Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)

[Simon Glass]

Hi François,

On Thu, 25 Nov 2021 at 08:11, François Ozog  wrote:
>
> Hi Simon,
>
>
>
>
> On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas  
> wrote:
>>
>> +cc Sandrine
>>
>> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
>>  wrote:
>> >
>> > Hi Simon,
>> >
>> >
>> > On Wed, 24 Nov 2021 at 06:09, Simon Glass  wrote:
>> > >
>> > >
>> > > This series adds support for the FIP format as used by ARM Trusted
>> > > Firmware (in particular TF-A).
>> > >
>
> I will use a question you use often "what problem are you trying to solve?". 
> If FIP format is used it means that TF-A/BL2 is going to parse it and verify 
> the hashes/signatures according to TF-A scheme.
>
> The packager will embed in a FIP components like Secure Monitor, Secure 
> hypervisor, Secure partitions code and manifests.
>
> All in all, U-Boot will be representing a small percentage of the 
> functionality offered by secure firmware  as a whole and it feels odd to make 
> another implementation that is more "accessory" rather than critical for the 
> U-Boot community. It may be a good idea but I wish you could explain it.

Here is a talk about Binman, its goals and how it works.

https://www.youtube.com/watch?v=L84ujgUXBOQ

Think of Binman as a separate tool that brings everything together. It
has grown out of U-Boot, largely because U-Boot is the main firmware
in most cases. Getting U-Boot to start up and boot successfully is the
goal of this packaging process. There are lots of instructions in the
tree and elsewhere about how to build an image comprising U-Boot and
various binary blobs. Binman aims to provide documentation for that
process and an image description that can be used to build an image
reliably.

We are slowly converting these text instructions into an in-tree image
description.

I believe that Binman can help bring order to the chaos that is
otherwise only going to grow, with lots of shell scripts, manual
instructions, obscure binary tools, etc. Binman brings everything
together and makes it clear what is needing/missing to build an image.

>
>> > > This allows images to be created containing a FIP, which itself contains
>> > > various binaries. With this, image creation can be handled from an 
>> > > in-tree
>> > > image description instead of needing to perform a lot of manual steps or
>> > > custom scripts to build the FIP.
>> > >
>
> That's not my experience of building a fip.  Packaging even Linux as a BL33 
> (instead of U-Boot) is very simple.

But not automatic. With Binman you don't need to worry about the
packaging. It is done for you. You just need to find all the binary
blobs that are needed.  This ability is quite important as firmware is
fragmenting and getting very complicated these days.

Binman runs twice...once in the U-Boot tree to do a build and again
later to repackage, put in a final fdtmap, add signatures and any
final pieces needed.

See this patch for an example of complicated build instructions with
Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
in the .rst file):

https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-...@chromium.org/

Regards,
Simon

Re: [PATCH v5 3/3] efi_loader: Extend PCR's for firmware measurements

[Ilias Apalodimas]

Hi Ruchika,

I just noticed the TPM_DIGEST_LEN usage


[...]
> > +* If PCR0 is 0, previous firmware didn't have the capability
> > +* to extend the PCR. In this scenario, extend the PCR as
> > +* the eventlog is parsed.
> > +*/
> > +   for (i = 0; i < digest_list.count; i++) {
> > +   u8 buffer[TPM2_DIGEST_LEN] =  { 0 };

This is only 32 bytes, you need TPM2_SHA512_DIGEST_SIZE to fit all digests

> > +   u16 hash_alg = digest_list.digests[i].hash_alg;
[...]

Regards
/Ilias

Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)

[François Ozog]

Hi Simon,




On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas 
wrote:

> +cc Sandrine
>
> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
>  wrote:
> >
> > Hi Simon,
> >
> >
> > On Wed, 24 Nov 2021 at 06:09, Simon Glass  wrote:
> > >
> > >
> > > This series adds support for the FIP format as used by ARM Trusted
> > > Firmware (in particular TF-A).
> > >
>
I will use a question you use often "what problem are you trying to
solve?". If FIP format is used it means that TF-A/BL2 is going to parse it
and verify the hashes/signatures according to TF-A scheme.

The packager will embed in a FIP components like Secure Monitor, Secure
hypervisor, Secure partitions code and manifests.

All in all, U-Boot will be representing a small percentage of the
functionality offered by secure firmware  as a whole and it feels odd to
make another implementation that is more "accessory" rather than critical
for the U-Boot community. It may be a good idea but I wish you could
explain it.

> > This allows images to be created containing a FIP, which itself contains
> > > various binaries. With this, image creation can be handled from an
> in-tree
> > > image description instead of needing to perform a lot of manual steps
> or
> > > custom scripts to build the FIP.
> > >
>
That's not my experience of building a fip.  Packaging even Linux as a BL33
(instead of U-Boot) is very simple.

> > >
> > > Simon Glass (2):
> > >   binman: Add a utility module for ATF FIP
> > >   binman: Add support for ATF FIP
> > >
> > >  scripts/pylint.base  |   9 +-
> > >  tools/binman/entries.rst | 154 ++
> > >  tools/binman/etype/atf_fip.py| 273 ++
> > >  tools/binman/fip_util.py | 653 +++
> > >  tools/binman/fip_util_test.py| 405 ++
> > >  tools/binman/ftest.py| 217 
> > >  tools/binman/main.py |   4 +-
> > >  tools/binman/test/203_fip.dts|  21 +
> > >  tools/binman/test/204_fip_other.dts  |  22 +
> > >  tools/binman/test/205_fip_no_type.dts|  15 +
> > >  tools/binman/test/206_fip_uuid.dts   |  22 +
> > >  tools/binman/test/207_fip_ls.dts |  25 +
> > >  tools/binman/test/208_fip_replace.dts|  33 ++
> > >  tools/binman/test/209_fip_missing.dts|  19 +
> > >  tools/binman/test/210_fip_size.dts   |  19 +
> > >  tools/binman/test/211_fip_bad_align.dts  |  18 +
> > >  tools/binman/test/212_fip_collection.dts |  24 +
> > >  17 files changed, 1929 insertions(+), 4 deletions(-)
> > >  create mode 100644 tools/binman/etype/atf_fip.py
> > >  create mode 100755 tools/binman/fip_util.py
> > >  create mode 100755 tools/binman/fip_util_test.py
> > >  create mode 100644 tools/binman/test/203_fip.dts
> > >  create mode 100644 tools/binman/test/204_fip_other.dts
> > >  create mode 100644 tools/binman/test/205_fip_no_type.dts
> > >  create mode 100644 tools/binman/test/206_fip_uuid.dts
> > >  create mode 100644 tools/binman/test/207_fip_ls.dts
> > >  create mode 100644 tools/binman/test/208_fip_replace.dts
> > >  create mode 100644 tools/binman/test/209_fip_missing.dts
> > >  create mode 100644 tools/binman/test/210_fip_size.dts
> > >  create mode 100644 tools/binman/test/211_fip_bad_align.dts
> > >  create mode 100644 tools/binman/test/212_fip_collection.dts
> > >
> > > --
> > > 2.34.0.rc2.393.gf8c9666880-goog
> > >
> >
> > My python is mediocre at best.  I'll try having a look, but CC'ing
> > TF-A developers would be a good idea.
> >
> > Thanks
> > /Ilias
>


-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.o...@linaro.org | Skype: ffozog

Re: a question about falcon mode

[Alex G.]

On 11/25/21 1:07 AM, Chan Kim wrote:

Hello all,

I'm trying to implement falcon mode for our board. Then should I first
implement the normal mode(spl + proper)?

It looks like so while I'm reading doc/README.falcon. (It says, after
loading kernel, DT etc. I should give 'spl export' command).



Falcon mode is a bit board dependent.  There are a couple of ways you 
could go about this.


The first is to have an "fdtargs" partition. This is where "spl export" 
comes in. Once you run "spl export", it will give a modified dtb at 
"$fdtargsaddr". It's that DTB that you need to write to your ftdargs 
partition. For example:


> spl export fdt $loadaddr - $fdt_addr_r
> mmc write $fdtargsaddr 0x9800 0x8000

In this example the ftdargs partition starts at sector 0x9800, and is 
0x800 sectors long.



The second option is to forget about "spl export" and "fdtargs", and 
package your kernel, devicetree, and overlays in a FIT container. You'd 
make sure to enable SPL_LOAD_FIT_APPLY_OVERLAY. There isn't much more to 
this other than the usual gotcha's with FIT and overlays.


Alex

Re: [PATCH 2/2] binman: Add support for ATF FIP

[Ilias Apalodimas]

+CC Sandrine

On Wed, 24 Nov 2021 at 06:09, Simon Glass  wrote:
>
> This format is used in firmware binaries so we may as well supported it.
>
> With this patch binman supports creating, listing and updating FIPs, as
> well as extracting files from one, provided that an FDTMAP is also present
> somewhere in the image.
>
> Signed-off-by: Simon Glass 
> ---
>
>  scripts/pylint.base  |   1 +
>  tools/binman/entries.rst | 154 +
>  tools/binman/etype/atf_fip.py| 273 +++
>  tools/binman/ftest.py| 217 ++
>  tools/binman/test/203_fip.dts|  21 ++
>  tools/binman/test/204_fip_other.dts  |  22 ++
>  tools/binman/test/205_fip_no_type.dts|  15 ++
>  tools/binman/test/206_fip_uuid.dts   |  22 ++
>  tools/binman/test/207_fip_ls.dts |  25 +++
>  tools/binman/test/208_fip_replace.dts|  33 +++
>  tools/binman/test/209_fip_missing.dts|  19 ++
>  tools/binman/test/210_fip_size.dts   |  19 ++
>  tools/binman/test/211_fip_bad_align.dts  |  18 ++
>  tools/binman/test/212_fip_collection.dts |  24 ++
>  14 files changed, 863 insertions(+)
>  create mode 100644 tools/binman/etype/atf_fip.py
>  create mode 100644 tools/binman/test/203_fip.dts
>  create mode 100644 tools/binman/test/204_fip_other.dts
>  create mode 100644 tools/binman/test/205_fip_no_type.dts
>  create mode 100644 tools/binman/test/206_fip_uuid.dts
>  create mode 100644 tools/binman/test/207_fip_ls.dts
>  create mode 100644 tools/binman/test/208_fip_replace.dts
>  create mode 100644 tools/binman/test/209_fip_missing.dts
>  create mode 100644 tools/binman/test/210_fip_size.dts
>  create mode 100644 tools/binman/test/211_fip_bad_align.dts
>  create mode 100644 tools/binman/test/212_fip_collection.dts
>
> diff --git a/scripts/pylint.base b/scripts/pylint.base
> index a35dbe34d2d..3d891edf261 100644
> --- a/scripts/pylint.base
> +++ b/scripts/pylint.base
> @@ -1,5 +1,6 @@
>  _testing 0.83
>  atf_bl31 -6.00
> +atf_fip 0.44
>  binman.cbfs_util 7.70
>  binman.cbfs_util_test 9.19
>  binman.cmdline 9.06
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index 748277c1cde..84f828a6352 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -25,6 +25,160 @@ about ATF.
>
>
>
> +Entry: atf-fip: ARM Trusted Firmware's Firmware Image Package (FIP)
> +---
> +
> +A FIP_ provides a way to group binaries in a firmware image, used by ARM's
> +Trusted Firmware A (TF-A) code. It is a simple format consisting of a
> +table of contents with information about the type, offset and size of the
> +binaries in the FIP. It is quite similar to FMAP, with the major difference
> +that it uses UUIDs to indicate the type of each entry.
> +
> +Note: It is recommended to always add an fdtmap to every image, as well as
> +any FIPs so that binman and other tools can access the entire image
> +correctly.
> +
> +The UUIDs correspond to useful names in `fiptool`, provided by ATF to
> +operate on FIPs. Binman uses these names to make it easier to understand
> +what is going on, although it is possible to provide a UUID if needed.
> +
> +The contents of the FIP are defined by subnodes of the atf-fip entry, e.g.::
> +
> +atf-fip {
> +soc-fw {
> +filename = "bl31.bin";
> +};
> +
> +scp-fwu-cfg {
> +filename = "bl2u.bin";
> +};
> +
> +u-boot {
> +fip-type = "nt-fw";
> +};
> +};
> +
> +This describes a FIP with three entries: soc-fw, scp-fwu-cfg and nt-fw.
> +You can use normal (non-external) binaries like U-Boot simply by adding a
> +FIP type, with the `fip-type` property, as above.
> +
> +Since FIP exists to bring blobs together, Binman assumes that all FIP
> +entries are external binaries. If a binary may not exist, you can use the
> +`--allow-missing` flag to Binman, in which case the image is still created,
> +even though it will not actually work.
> +
> +The size of the FIP depends on the size of the binaries. There is currently
> +no way to specify a fixed size. If the `atf-fip` node has a `size` entry,
> +this affects the space taken up by the `atf-fip` entry, but the FIP itself
> +does not expand to use that space.
> +
> +Some other FIP features are available with Binman. The header and the
> +entries have 64-bit flag works. The flag flags do not seem to be defined
> +anywhere, but you can use `fip-hdr-flags` and fip-flags` to set the values
> +of the header and entries respectively.
> +
> +FIP entries can be aligned to a particular power-of-two boundary. Use
> +fip-align for this.
> +
> +Binman only understands the entry types that are included in its
> +implementation. It is possible to specify a 16-byte UUID instead, using the
> +fip-uuid property. In this case Binman doesn't know what its type is, so
> +just uses the UUID. See the `u-boot` node in 

Re: [PATCH 1/2] binman: Add a utility module for ATF FIP

[Ilias Apalodimas]

+cc Sandrine

On Wed, 24 Nov 2021 at 06:09, Simon Glass  wrote:
>
> Add support for this format which is used by ARM Trusted Firmware to find
> firmware binaries to load.
>
> FIP is like a simpler version of FMAP but uses a UUID instead of a name,
> for each entry.
>
> It supports reading a FIP, writing a FIP and parsing the ATF source code
> to get a list of supported UUIDs.
>
> Signed-off-by: Simon Glass 
> ---
>
>  scripts/pylint.base   |   8 +-
>  tools/binman/fip_util.py  | 653 ++
>  tools/binman/fip_util_test.py | 405 +
>  tools/binman/main.py  |   4 +-
>  4 files changed, 1066 insertions(+), 4 deletions(-)
>  create mode 100755 tools/binman/fip_util.py
>  create mode 100755 tools/binman/fip_util_test.py
>
> diff --git a/scripts/pylint.base b/scripts/pylint.base
> index 4e82dd4a616..a35dbe34d2d 100644
> --- a/scripts/pylint.base
> +++ b/scripts/pylint.base
> @@ -9,11 +9,13 @@ binman.elf_test 5.41
>  binman.entry 2.39
>  binman.entry_test 5.29
>  binman.fdt_test 3.23
> +binman.fip_util 9.86
> +binman.fip_util_test 9.75
>  binman.fmap_util 6.67
>  binman.ftest 7.38
>  binman.image 6.39
>  binman.image_test 4.48
> -binman.main 4.26
> +binman.main 4.03
>  binman.setup 5.00
>  binman.state 3.30
>  blob -1.94
> @@ -33,7 +35,7 @@ buildman.main 1.43
>  buildman.test 6.10
>  buildman.toolchain 5.81
>  capsule_defs 5.00
> -cbfs -1.52
> +cbfs -1.44
>  collection 2.33
>  concurrencytest 6.77
>  conftest -3.84
> @@ -107,7 +109,7 @@ powerpc_mpc85xx_bootpg_resetvec -10.00
>  rkmux 6.76
>  rmboard 7.76
>  scp -6.00
> -section 4.25
> +section 4.31
>  sqfs_common 8.12
>  test 8.18
>  test_000_version 7.50
> diff --git a/tools/binman/fip_util.py b/tools/binman/fip_util.py
> new file mode 100755
> index 000..5f7dbc04d56
> --- /dev/null
> +++ b/tools/binman/fip_util.py
> @@ -0,0 +1,653 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright 2021 Google LLC
> +# Written by Simon Glass 
> +
> +"""Support for ARM's Firmware Image Package (FIP) format
> +
> +FIP is a format similar to FMAP[1] but with fewer features and an obscure 
> UUID
> +instead of the region name.
> +
> +It consists of a header and a table of entries, each pointing to a place in 
> the
> +firmware image where something can be found.
> +
> +[1] 
> https://chromium.googlesource.com/chromiumos/third_party/flashmap/+/refs/heads/master/lib/fmap.h
> +
> +If ATF updates, run this program to update the FIT_TYPE_LIST.
> +
> +ARM Trusted Firmware is available at:
> +
> +https://github.com/ARM-software/arm-trusted-firmware.git
> +"""
> +
> +from argparse import ArgumentParser
> +import collections
> +import io
> +import os
> +import re
> +import struct
> +import sys
> +from uuid import UUID
> +
> +OUR_FILE = os.path.realpath(__file__)
> +OUR_PATH = os.path.dirname(OUR_FILE)
> +
> +# Bring in the patman and dtoc libraries (but don't override the first path
> +# in PYTHONPATH)
> +sys.path.insert(2, os.path.join(OUR_PATH, '..'))
> +
> +# pylint: disable=C0413
> +from patman import command
> +from patman import tools
> +
> +# The TOC header, at the start of the FIP
> +HEADER_FORMAT = ' +HEADER_LEN = 0x10
> +HEADER_MAGIC = 0xaA640001
> +HEADER_SERIAL = 0x12345678
> +
> +# The entry header (a table of these comes after the TOC header)
> +UUID_LEN = 16
> +ENTRY_FORMAT = f'<{UUID_LEN}sQQQ'
> +ENTRY_SIZE = 0x28
> +
> +HEADER_NAMES = (
> +'name',
> +'serial',
> +'flags',
> +)
> +
> +ENTRY_NAMES = (
> +'uuid',
> +'offset',
> +'size',
> +'flags',
> +)
> +
> +# Set to True to enable output from running fiptool for debugging
> +VERBOSE = False
> +
> +# Use a class so we can convert the bytes, making the table more readable
> +# pylint: disable=R0903
> +class FipType:
> +"""A FIP entry type that we understand"""
> +def __init__(self, name, desc, uuid_bytes):
> +"""Create up a new type
> +
> +Args:
> +name (str): Short name for the type
> +desc (str): Longer description for the type
> +uuid_bytes (bytes): List of 16 bytes for the UUID
> +"""
> +self.name = name
> +self.desc = desc
> +self.uuid = bytes(uuid_bytes)
> +
> +# This is taken from tbbr_config.c in ARM Trusted Firmware
> +FIP_TYPE_LIST = [
> +# ToC Entry UUIDs
> +FipType('scp-fwu-cfg', 'SCP Firmware Updater Configuration FWU SCP_BL2U',
> +[0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
> + 0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10]),
> +FipType('ap-fwu-cfg', 'AP Firmware Updater Configuration BL2U',
> +[0x60, 0xb3, 0xeb, 0x37, 0xc1, 0xe5, 0xea, 0x41,
> + 0x9d, 0xf3, 0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01]),
> +FipType('fwu', 'Firmware Updater NS_BL2U',
> +[0x4f, 0x51, 0x1d, 0x11, 0x2b, 0xe5, 0x4e, 0x49,
> + 0xb4, 0xc5, 0x83, 0xc2, 0xf7, 0x15, 0x84, 0x0a]),
> +FipType('fwu-cert', 'Non-Trusted Firmware 

Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)

[Ilias Apalodimas]

+cc Sandrine

On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
 wrote:
>
> Hi Simon,
>
>
> On Wed, 24 Nov 2021 at 06:09, Simon Glass  wrote:
> >
> >
> > This series adds support for the FIP format as used by ARM Trusted
> > Firmware (in particular TF-A).
> >
> > This allows images to be created containing a FIP, which itself contains
> > various binaries. With this, image creation can be handled from an in-tree
> > image description instead of needing to perform a lot of manual steps or
> > custom scripts to build the FIP.
> >
> >
> > Simon Glass (2):
> >   binman: Add a utility module for ATF FIP
> >   binman: Add support for ATF FIP
> >
> >  scripts/pylint.base  |   9 +-
> >  tools/binman/entries.rst | 154 ++
> >  tools/binman/etype/atf_fip.py| 273 ++
> >  tools/binman/fip_util.py | 653 +++
> >  tools/binman/fip_util_test.py| 405 ++
> >  tools/binman/ftest.py| 217 
> >  tools/binman/main.py |   4 +-
> >  tools/binman/test/203_fip.dts|  21 +
> >  tools/binman/test/204_fip_other.dts  |  22 +
> >  tools/binman/test/205_fip_no_type.dts|  15 +
> >  tools/binman/test/206_fip_uuid.dts   |  22 +
> >  tools/binman/test/207_fip_ls.dts |  25 +
> >  tools/binman/test/208_fip_replace.dts|  33 ++
> >  tools/binman/test/209_fip_missing.dts|  19 +
> >  tools/binman/test/210_fip_size.dts   |  19 +
> >  tools/binman/test/211_fip_bad_align.dts  |  18 +
> >  tools/binman/test/212_fip_collection.dts |  24 +
> >  17 files changed, 1929 insertions(+), 4 deletions(-)
> >  create mode 100644 tools/binman/etype/atf_fip.py
> >  create mode 100755 tools/binman/fip_util.py
> >  create mode 100755 tools/binman/fip_util_test.py
> >  create mode 100644 tools/binman/test/203_fip.dts
> >  create mode 100644 tools/binman/test/204_fip_other.dts
> >  create mode 100644 tools/binman/test/205_fip_no_type.dts
> >  create mode 100644 tools/binman/test/206_fip_uuid.dts
> >  create mode 100644 tools/binman/test/207_fip_ls.dts
> >  create mode 100644 tools/binman/test/208_fip_replace.dts
> >  create mode 100644 tools/binman/test/209_fip_missing.dts
> >  create mode 100644 tools/binman/test/210_fip_size.dts
> >  create mode 100644 tools/binman/test/211_fip_bad_align.dts
> >  create mode 100644 tools/binman/test/212_fip_collection.dts
> >
> > --
> > 2.34.0.rc2.393.gf8c9666880-goog
> >
>
> My python is mediocre at best.  I'll try having a look, but CC'ing
> TF-A developers would be a good idea.
>
> Thanks
> /Ilias

Re: [PATCH v2] efi_loader: check tcg2 protocol installation outside the TCG protocol

[Ilias Apalodimas]

Hi Kojima-san,

On Thu, Nov 25, 2021 at 08:36:28PM +0900, Masahisa Kojima wrote:
> +/**

[...]

> + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed
> + *
> + * @Return: true if tcg2 protocol is installed, false if not
> + */
> +bool is_tcg2_protocol_installed(void)
> +{
> + struct efi_handler *handler;
> + efi_status_t ret;
> +
> + ret = efi_search_protocol(efi_root, _guid_tcg2_protocol, );
> + return ((ret == EFI_SUCCESS) ? true : false);
> +}

return ret == EFI_SUCCESS; is enough here. 

> +
>  static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
>  {
>   u32 len;
> @@ -962,6 +976,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 
> efi_size,
>   IMAGE_NT_HEADERS32 *nt;
>   struct efi_handler *handler;
>  
> + if (!is_tcg2_protocol_installed())
> + return EFI_NOT_READY;
> +
>   ret = platform_get_tpm2_device();
>   if (ret != EFI_SUCCESS)
>   return ret;
> @@ -2140,6 +2157,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct 
> efi_loaded_image_obj *ha
>   u32 event = 0;
>   struct smbios_entry *entry;
>  
> + if (!is_tcg2_protocol_installed())
> + return EFI_NOT_READY;
> +
>   if (tcg2_efi_app_invoked)
>   return EFI_SUCCESS;
>  
> @@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
>   efi_status_t ret;
>   struct udevice *dev;
>  
> + if (!is_tcg2_protocol_installed())

[...]

Heinrich, this whole patch is needed because installing  the tcg2 protocol
always returns EFI_SUCCESS.  The reason is that some sandbox tests with 
sandbox_tpm used to fail.  Do you want to keep this or perhaps just failing
the boot now is the protocol fails to install is an option ?


Thanks
/Ilias

Re: [PATCH] efi_loader: check tcg2 protocol installation outside the TCG protocol

[Masahisa Kojima]

Hi Ilias,

On Thu, 25 Nov 2021 at 17:25, Ilias Apalodimas
 wrote:
>
> Hello Kojima-san
>
> On Thu, 25 Nov 2021 at 08:04, Masahisa Kojima
>  wrote:
> >
> > There are functions that calls tcg2_agile_log_append() outside
> > of the TCG protocol invocation (e.g tcg2_measure_pe_image).
> > These functions must to check that tcg2 protocol is installed.
> > If not, measurement shall be skipped.
> >
> > Together with above change, this commit also removes the
> > unnecessary tcg2_uninit() call in efi_tcg2_register(),
> > refactors efi_tcg2_register() not to include the process
> > that is not related to the tcg2 protocol registration.
> >
> > Signed-off-by: Masahisa Kojima 
> > ---
> >  include/efi_loader.h   |  4 ++
> >  lib/efi_loader/efi_setup.c |  3 ++
> >  lib/efi_loader/efi_tcg2.c  | 89 +++---
> >  3 files changed, 81 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index d52e399841..fe29c10906 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -525,6 +525,10 @@ efi_status_t efi_disk_register(void);
> >  efi_status_t efi_rng_register(void);
> >  /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
> >  efi_status_t efi_tcg2_register(void);
> > +/* Called by efi_init_obj_list() to do the required setup for the 
> > measurement */
> > +efi_status_t efi_tcg2_setup_measurement(void);
> > +/* Called by efi_init_obj_list() to do initial measurement */
> > +efi_status_t efi_tcg2_do_initial_measurement(void);
> >  /* measure the pe-coff image, extend PCR and add Event Log */
> >  efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
> >struct efi_loaded_image_obj *handle,
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index a2338d74af..781d10590d 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -271,6 +271,9 @@ efi_status_t efi_init_obj_list(void)
> > ret = efi_tcg2_register();
> > if (ret != EFI_SUCCESS)
> > goto out;
> > +
> > +   efi_tcg2_setup_measurement();
> > +   efi_tcg2_do_initial_measurement();
> > }
> >
> > /* Secure boot */
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 8c1f22e337..46300cae79 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -153,6 +153,20 @@ static u16 alg_to_len(u16 hash_alg)
> > return 0;
> >  }
> >
> > +/**
> > + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed
> > + *
> > + * @Return: true if tcg2 protocol is installed, false if not
> > + */
> > +bool is_tcg2_protocol_installed(void)
> > +{
> > +   struct efi_handler *handler;
> > +   efi_status_t ret;
> > +
> > +   ret = efi_search_protocol(efi_root, _guid_tcg2_protocol, 
> > );
> > +   return ((ret == EFI_SUCCESS) ? true : false);
> > +}
> > +
> >  static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
> >  {
> > u32 len;
> > @@ -886,6 +900,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 
> > efi_size,
> > IMAGE_NT_HEADERS32 *nt;
> > struct efi_handler *handler;
> >
> > +   if (!is_tcg2_protocol_installed())
> > +   return EFI_NOT_READY;
> > +
> > ret = platform_get_tpm2_device();
> > if (ret != EFI_SUCCESS)
> > return ret;
> > @@ -1759,6 +1776,9 @@ efi_status_t 
> > efi_tcg2_measure_efi_app_invocation(struct efi_loaded_image_obj *ha
> > u32 event = 0;
> > struct smbios_entry *entry;
> >
> > +   if (!is_tcg2_protocol_installed())
> > +   return EFI_NOT_READY;
> > +
> > if (tcg2_efi_app_invoked)
> > return EFI_SUCCESS;
> >
> > @@ -1809,6 +1829,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
> > efi_status_t ret;
> > struct udevice *dev;
> >
> > +   if (!is_tcg2_protocol_installed())
> > +   return EFI_NOT_READY;
> > +
> > ret = platform_get_tpm2_device();
> > if (ret != EFI_SUCCESS)
> > return ret;
> > @@ -1833,6 +1856,11 @@ efi_tcg2_notify_exit_boot_services(struct efi_event 
> > *event, void *context)
> >
> > EFI_ENTRY("%p, %p", event, context);
> >
> > +   if (!is_tcg2_protocol_installed()) {
> > +   ret =  EFI_NOT_READY;
> > +   goto out;
> > +   }
> > +
> > event_log.ebs_called = true;
> > ret = platform_get_tpm2_device();
> > if (ret != EFI_SUCCESS)
> > @@ -1863,6 +1891,9 @@ efi_status_t 
> > efi_tcg2_notify_exit_boot_services_failed(void)
> > struct udevice *dev;
> > efi_status_t ret;
> >
> > +   if (!is_tcg2_protocol_installed())
> > +   return EFI_NOT_READY;
> > +
> > ret = platform_get_tpm2_device();
> > if (ret != EFI_SUCCESS)
> > goto 

[PATCH v2] efi_loader: check tcg2 protocol installation outside the TCG protocol

[Masahisa Kojima]

There are functions that calls tcg2_agile_log_append() outside
of the TCG protocol invocation (e.g tcg2_measure_pe_image).
These functions must to check that tcg2 protocol is installed.
If not, measurement shall be skipped.

Together with above change, this commit also removes the
unnecessary tcg2_uninit() call in efi_tcg2_register(),
refactors efi_tcg2_register() not to include the process
that is not related to the tcg2 protocol registration.

Signed-off-by: Masahisa Kojima 
---
Changes in v2:
- rebased on top of the TF-A eventlog handover support

 include/efi_loader.h   |  4 ++
 lib/efi_loader/efi_setup.c |  3 ++
 lib/efi_loader/efi_tcg2.c  | 89 +++---
 3 files changed, 81 insertions(+), 15 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index d52e399841..fe29c10906 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -525,6 +525,10 @@ efi_status_t efi_disk_register(void);
 efi_status_t efi_rng_register(void);
 /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
 efi_status_t efi_tcg2_register(void);
+/* Called by efi_init_obj_list() to do the required setup for the measurement 
*/
+efi_status_t efi_tcg2_setup_measurement(void);
+/* Called by efi_init_obj_list() to do initial measurement */
+efi_status_t efi_tcg2_do_initial_measurement(void);
 /* measure the pe-coff image, extend PCR and add Event Log */
 efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
   struct efi_loaded_image_obj *handle,
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index a2338d74af..781d10590d 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -271,6 +271,9 @@ efi_status_t efi_init_obj_list(void)
ret = efi_tcg2_register();
if (ret != EFI_SUCCESS)
goto out;
+
+   efi_tcg2_setup_measurement();
+   efi_tcg2_do_initial_measurement();
}
 
/* Secure boot */
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index b44eed0ec9..0a33f5ab5f 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -153,6 +153,20 @@ static u16 alg_to_len(u16 hash_alg)
return 0;
 }
 
+/**
+ * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed
+ *
+ * @Return: true if tcg2 protocol is installed, false if not
+ */
+bool is_tcg2_protocol_installed(void)
+{
+   struct efi_handler *handler;
+   efi_status_t ret;
+
+   ret = efi_search_protocol(efi_root, _guid_tcg2_protocol, );
+   return ((ret == EFI_SUCCESS) ? true : false);
+}
+
 static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
 {
u32 len;
@@ -962,6 +976,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
IMAGE_NT_HEADERS32 *nt;
struct efi_handler *handler;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
return ret;
@@ -2140,6 +2157,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct 
efi_loaded_image_obj *ha
u32 event = 0;
struct smbios_entry *entry;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
if (tcg2_efi_app_invoked)
return EFI_SUCCESS;
 
@@ -2190,6 +2210,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
efi_status_t ret;
struct udevice *dev;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
return ret;
@@ -2214,6 +2237,11 @@ efi_tcg2_notify_exit_boot_services(struct efi_event 
*event, void *context)
 
EFI_ENTRY("%p, %p", event, context);
 
+   if (!is_tcg2_protocol_installed()) {
+   ret =  EFI_NOT_READY;
+   goto out;
+   }
+
event_log.ebs_called = true;
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
@@ -2244,6 +2272,9 @@ efi_status_t 
efi_tcg2_notify_exit_boot_services_failed(void)
struct udevice *dev;
efi_status_t ret;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_NOT_READY;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
goto out;
@@ -2313,6 +2344,45 @@ error:
return ret;
 }
 
+/**
+ * efi_tcg2_setup_measurement() - setup for the measurement
+ *
+ * Return: status code
+ */
+efi_status_t efi_tcg2_setup_measurement(void)
+{
+   efi_status_t ret;
+   struct efi_event *event;
+
+   ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
+  efi_tcg2_notify_exit_boot_services, NULL,
+  NULL, );
+
+   return ret;
+}
+
+/**
+ * efi_tcg2_do_initial_measurement() - do initial measurement
+ *
+ * Return: 

Re: [PATCH v7 05/12] doc: update UEFI document for usage of mkeficapsule

[Ilias Apalodimas]

Akashi-san,

[...]
>
> -Since U-boot doesn't currently support SetVariable at runtime there's a 
> Kconfig
> -option (CONFIG_EFI_IGNORE_OSINDICATIONS) to disable the OsIndications 
> variable
> -check. If that option is enabled just copy your capsule to 
> \EFI\UpdateCapsule.
> -
> -If that option is disabled, you'll need to set the OsIndications variable 
> with::
> +Put capsule files under the directory mentioned above.
> +Then, following the UEFI specification, you'll need to set
> +the EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
> +bit in OsIndications variable with::
>
>  => setenv -e -nv -bs -rt -v OsIndications =0x04
>
> -Finally, the capsule update can be initiated either by rebooting the board,
> -which is the preferred method, or by issuing the following command::
> +Since U-boot doesn't currently support SetVariable at runtime, its value
> +won't be taken over across the reboot. If this is the case, you can skip
> +this feature check with the Kconfig option (CONFIG_EFI_IGNORE_OSINDICATIONS)
> +set.
>
> -=> efidebug capsule disk-update
> -
> -**The efidebug command is should only be used during debugging/development.**
> +Finally, the capsule update can be initiated by rebooting the board.
>
>  Enabling Capsule Authentication
>  ***
> @@ -324,82 +339,58 @@ be updated by verifying the capsule signature. The 
> capsule signature
>  is computed and prepended to the capsule payload at the time of
>  capsule generation. This signature is then verified by using the
>  public key stored as part of the X509 certificate. This certificate is
> -in the form of an efi signature list (esl) file, which is embedded as
> -part of U-Boot.
> +in the form of an efi signature list (esl) file, which is embedded in
> +a device tree.
>
>  The capsule authentication feature can be enabled through the
>  following config, in addition to the configs listed above for capsule
>  update::
>
>  CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> -CONFIG_EFI_CAPSULE_KEY_PATH=
>
>  The public and private keys used for the signing process are generated
> -and used by the steps highlighted below::
> +and used by the steps highlighted below.
>
> -1. Install utility commands on your host
> -   * OPENSSL
> +1. Install utility commands on your host
> +   * openssl
> * efitools
>
> -2. Create signing keys and certificate files on your host
> +2. Create signing keys and certificate files on your host::
>
>  $ openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=CRT/ \
>  -keyout CRT.key -out CRT.crt -nodes -days 365
>  $ cert-to-efi-sig-list CRT.crt CRT.esl
>
> -$ openssl x509 -in CRT.crt -out CRT.cer -outform DER
> -$ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
> -
> -$ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
> -$ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
> -
> -The capsule file can be generated by using the GenerateCapsule.py
> -script in EDKII::
> -
> -$ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
> -   --monotonic-count  --fw-version \
> -   --lsv  --guid \
> -  e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
> -  --update-image-index  --signer-private-cert \
> -  /path/to/CRT.pem --trusted-public-cert \
> -  /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
> -  
> -
> -Place the capsule generated in the above step on the EFI System
> -Partition under the EFI/UpdateCapsule directory
> -
> -Testing on QEMU
> -***
> +3. Run the following command to create and sign the capsule file::
>
> -Currently, support has been added on the QEMU ARM64 virt platform for
> -updating the U-Boot binary as a raw image when the platform is booted
> -in non-secure mode, i.e. with CONFIG_TFABOOT disabled. For this
> -configuration, the QEMU platform needs to be booted with
> -'secure=off'. The U-Boot binary placed on the first bank of the NOR
> -flash at offset 0x0. The U-Boot environment is placed on the second
> -NOR flash bank at offset 0x400.
> +$ mkeficapsule --monotonic-count 1 \
> +  --private-key CRT.key \
> +  --certificate CRT.crt \
> +  --index 1 --instance 0 \
> +  [--fit  | --raw ] \
> +  
>
> -The capsule update feature is enabled with the following configuration
> -settings::
> +4. Insert the signature list into a device tree in the following format::
>
> -CONFIG_MTD=y
> -CONFIG_FLASH_CFI_MTD=y
> -CONFIG_CMD_MTDPARTS=y
> -CONFIG_CMD_DFU=y
> -CONFIG_DFU_MTD=y
> -CONFIG_PCI_INIT_R=y
> -CONFIG_EFI_CAPSULE_ON_DISK=y
> -CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT=y
> -CONFIG_EFI_CAPSULE_FIRMWARE=y
> -CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> +{
> +signature {
> +capsule-key = [  ];
> +}
> +...
> +}
>
> -In addition, the following config needs to be disabled(QEMU 

Re: [PATCH v7 04/12] tools: mkeficapsule: add man page

[Ilias Apalodimas]

On Tue, 16 Nov 2021 at 06:33, AKASHI Takahiro
 wrote:
>
> Add a man page for mkeficapsule command.
>
> Signed-off-by: AKASHI Takahiro 
> Reviewed-by: Simon Glass 
> ---
>  MAINTAINERS|  1 +
>  doc/mkeficapsule.1 | 95 ++
>  2 files changed, 96 insertions(+)
>  create mode 100644 doc/mkeficapsule.1
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6db5354322fe..813674eb2898 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -722,6 +722,7 @@ S:  Maintained
>  T: git https://source.denx.de/u-boot/custodians/u-boot-efi.git
>  F: doc/api/efi.rst
>  F: doc/develop/uefi/*
> +F: doc/mkeficapsule.1
>  F: doc/usage/bootefi.rst
>  F: drivers/rtc/emul_rtc.c
>  F: include/capitalization.h
> diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> new file mode 100644
> index ..837e09ab451e
> --- /dev/null
> +++ b/doc/mkeficapsule.1
> @@ -0,0 +1,95 @@
> +.TH MAEFICAPSULE 1 "May 2021"
> +
> +.SH NAME
> +mkeficapsule \- Generate EFI capsule file for U-Boot
> +
> +.SH SYNOPSIS
> +.B mkeficapsule
> +.RB [\fIoptions\fP] " \fIcapsule-file\fP"
> +
> +.SH "DESCRIPTION"
> +The
> +\fBmkeficapsule\fP
> +command is used to create an EFI capsule file for use with the U-Boot
> +EFI capsule update.
> +A capsule file may contain various type of firmware blobs which
> +are to be applied to the system and must be placed in the specific
> +directory on the UEFI system partition. An update will be automatically
> +executed at next reboot.
> +
> +Optionally, a capsule file can be signed with a given private key.
> +In this case, the update will be authenticated by verifying the signature
> +before applying.
> +
> +\fBmkeficapsule\fP supports two different format of image files:
> +.TP
> +.I raw image
> +format is a single binary blob of any type of firmware.
> +
> +.TP
> +.I FIT (Flattened Image Tree) image
> +format
> +is the same as used in the new \fIuImage\fP format and allows for
> +multiple binary blobs in a single capsule file.
> +This type of image file can be generated by \fBmkimage\fP.
> +
> +.SH "OPTIONS"
> +One of \fB--fit\fP or \fB--raw\fP option must be specified.
> +
> +.TP
> +.BI "-f, --fit \fIfit-image-file\fP"
> +Specify a FIT image file
> +
> +.TP
> +.BI "-r, --raw \fIraw-image-file\fP"
> +Specify a raw image file
> +
> +.TP
> +.BI "-i, --index \fIindex\fP"
> +Specify an image index
> +
> +.TP
> +.BI "-I, --instance \fIinstance\fP"
> +Specify a hardware instance
> +
> +.TP
> +.BI "-h, --help"
> +Print a help message
> +
> +.TP 0
> +.B With signing:
> +
> +\fB--private-key\fP, \fB--certificate\fP and \fB--monotonic-count\fP are
> +all mandatory.
> +
> +.TP
> +.BI "-p, --private-key \fIprivate-key-file\fP"
> +Specify signer's private key file in PEM
> +
> +.TP
> +.BI "-c, --certificate \fIcertificate-file\fP"
> +Specify signer's certificate file in EFI certificate list format
> +
> +.TP
> +.BI "-m, --monotonic-count \fIcount\fP"
> +Specify a monotonic count which is set to be monotonically incremented
> +at every firmware update.
> +
> +.TP
> +.BI "-d, --dump_sig"
> +Dump signature data into *.p7 file
> +
> +.PP
> +.SH FILES
> +.TP
> +.BI "\fI/EFI/UpdateCapsule\fP"
> +The directory in which all capsule files be placed
> +
> +.SH SEE ALSO
> +.B mkimage
> +
> +.SH AUTHORS
> +Written by AKASHI Takahiro 
> +
> +.SH HOMEPAGE
> +http://www.denx.de/wiki/U-Boot/WebHome
> --
> 2.33.0
>
Acked-by: Ilias Apalodimas 

Re: [PATCH v7 03/12] tools: mkeficapsule: add firmwware image signing

[Ilias Apalodimas]

On Tue, 16 Nov 2021 at 06:33, AKASHI Takahiro
 wrote:
>
> With this enhancement, mkeficapsule will be able to sign a capsule
> file when it is created. A signature added will be used later
> in the verification at FMP's SetImage() call.
>
> To do that, We need specify additional command parameters:
>   -monotonic-cout  : monotonic count
>   -private-key  : private key file
>   -certificate  : certificate file
> Only when all of those parameters are given, a signature will be added
> to a capsule file.
>
> Users are expected to maintain and increment the monotonic count at
> every time of the update for each firmware image.
>
> Signed-off-by: AKASHI Takahiro 
> Reviewed-by: Simon Glass 
> ---
>  tools/Kconfig|   8 +
>  tools/Makefile   |   8 +-
>  tools/eficapsule.h   | 115 +
>  tools/mkeficapsule.c | 401 +++
>  4 files changed, 494 insertions(+), 38 deletions(-)
>  create mode 100644 tools/eficapsule.h
>
> diff --git a/tools/Kconfig b/tools/Kconfig
> index 91ce8ae3e516..117c921da3fe 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -90,4 +90,12 @@ config TOOLS_SHA512
> help
>   Enable SHA512 support in the tools builds
>
> +config TOOLS_MKEFICAPSULE
> +   bool "Build efimkcapsule command"
> +   default y if EFI_CAPSULE_ON_DISK
> +   help
> + This command allows users to create a UEFI capsule file and,
> + optionally sign that file. If you want to enable UEFI capsule
> + update feature on your target, you certainly need this.
> +
>  endmenu
> diff --git a/tools/Makefile b/tools/Makefile
> index 1763f44cac43..a10aa091021f 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -238,8 +238,12 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
>  hostprogs-$(CONFIG_ASN1_COMPILER)  += asn1_compiler
>  HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
>
> -mkeficapsule-objs  := mkeficapsule.o $(LIBFDT_OBJS)
> -hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule
> +HOSTLDLIBS_mkeficapsule += -luuid
> +ifeq ($(CONFIG_TOOLS_LIBCRYPTO),y)
> +HOSTLDLIBS_mkeficapsule += \
> +   $(shell pkg-config --libs libssl libcrypto 2> /dev/null || echo 
> "-lssl -lcrypto")
> +endif
> +hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
>
>  # We build some files with extra pedantic flags to try to minimize things
>  # that won't build on some weird host compiler -- though there are lots of
> diff --git a/tools/eficapsule.h b/tools/eficapsule.h
> new file mode 100644
> index ..8c1560bb0671
> --- /dev/null
> +++ b/tools/eficapsule.h
> @@ -0,0 +1,115 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright 2021 Linaro Limited
> + * Author: AKASHI Takahiro
> + *
> + * derived from efi.h and efi_api.h to make the file POSIX-compliant
> + */
> +
> +#ifndef _EFI_CAPSULE_H
> +#define _EFI_CAPSULE_H
> +
> +#include 
> +#include  /* WIN_CERTIFICATE */
> +
> +/*
> + * Gcc's predefined attributes are not recognized by clang.
> + */
> +#ifndef __packed
> +#define __packed   __attribute__((__packed__))
> +#endif
> +
> +#ifndef __aligned
> +#define __aligned(x)   __attribute__((__aligned__(x)))
> +#endif
> +
> +typedef struct {
> +   uint8_t b[16];
> +} efi_guid_t __aligned(8);
> +
> +#define EFI_GUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
> +   {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, \
> +   ((a) >> 24) & 0xff, \
> +   (b) & 0xff, ((b) >> 8) & 0xff, \
> +   (c) & 0xff, ((c) >> 8) & 0xff, \
> +   (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) } }
> +
> +#define EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID \
> +   EFI_GUID(0x6dcbd5ed, 0xe82d, 0x4c44, 0xbd, 0xa1, \
> +0x71, 0x94, 0x19, 0x9a, 0xd9, 0x2a)
> +
> +#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID \
> +   EFI_GUID(0xae13ff2d, 0x9ad4, 0x4e25, 0x9a, 0xc8, \
> +0x6d, 0x80, 0xb3, 0xb2, 0x21, 0x47)
> +
> +#define EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID \
> +   EFI_GUID(0xe2bb9c06, 0x70e9, 0x4b14, 0x97, 0xa3, \
> +0x5a, 0x79, 0x13, 0x17, 0x6e, 0x3f)
> +
> +#define EFI_CERT_TYPE_PKCS7_GUID \
> +   EFI_GUID(0x4aafd29d, 0x68df, 0x49ee, 0x8a, 0xa9, \
> +0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7)
> +
> +/* flags */
> +#define CAPSULE_FLAGS_PERSIST_ACROSS_RESET  0x0001
> +
> +struct efi_capsule_header {
> +   efi_guid_t capsule_guid;
> +   uint32_t header_size;
> +   uint32_t flags;
> +   uint32_t capsule_image_size;
> +} __packed;
> +
> +struct efi_firmware_management_capsule_header {
> +   uint32_t version;
> +   uint16_t embedded_driver_count;
> +   uint16_t payload_item_count;
> +   uint32_t item_offset_list[];
> +} __packed;
> +
> +/* image_capsule_support */
> +#define CAPSULE_SUPPORT_AUTHENTICATION  0x0001
> +
> +struct efi_firmware_management_capsule_image_header {
> +   uint32_t version;
> +   

Re: [PATCH] efi_loader: Check for pointers before appending to the eventlog

[Ilias Apalodimas]

Hi Heinrich,

On Tue, 23 Nov 2021 at 18:24, Ilias Apalodimas
 wrote:
>
> The TCG protocol returns EFI_SUCCESS even when it fails to install.
> If the protocol is not installed there should be no calls to
> tcg2_agile_log_append().  However there are calls to this function
> occurring outside the TCG protocol invocation (e.g tcg2_measure_pe_image).
>
> Let's deal with this and shield the code from crashing while complaining,
> so buggy code can be spotted and fixed.
>
> Signed-off-by: Ilias Apalodimas 
> ---
> Heinrich this is rebased on top of 
> https://lore.kernel.org/u-boot/20211123115335.125252-1-ruchika.gu...@linaro.org/
>  lib/efi_loader/efi_tcg2.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 133fe8291a92..57a1f37695f6 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -311,6 +311,17 @@ static efi_status_t tcg2_agile_log_append(u32 pcr_index, 
> u32 event_type,
> struct efi_tcg2_final_events_table *final_event;
> efi_status_t ret = EFI_SUCCESS;
>
> +   /*
> +* This should never happen.  This function should only be invoked if
> +* the TCG2 protocol has been installed.  However since we always
> +* return EFI_SUCCESS from efi_tcg2_register shield callers against
> +* crashing and complain
> +*/
> +   if (!event_log.final_buffer || !event_log.buffer) {
> +   log_err("EFI TCG2 protocol not installed\n");
> +   return EFI_INVALID_PARAMETER;
> +   }
> +
> /* if ExitBootServices hasn't been called update the normal log */
> if (!event_log.ebs_called) {
> if (event_log.truncated ||
> --
> 2.34.0
>

With the latest patch sent from Ruchika [1], this doesn't work
properly anymore.  I'll rebase/fix it once that one is applied.

[1] 
https://lore.kernel.org/u-boot/cac_iwjlpfwos2_x9e536jxwufq3g7jhuzyebf8s70mbeqfo...@mail.gmail.com/

Regards
/Ilias

[PATCH] board: stm32mp1: add support of nor1 device in dfu command

[Patrick Delaunay]

Add support of mtd backend for nor1 when this device is present on the
board, on STM32MP157C-EV1 for example, as the support of several MTD
spi-nor instance are now supported with commit b7f060565e31 ("mtd:
spi-nor: allow registering multiple MTDs when DM is enabled").

Signed-off-by: Patrick Delaunay 
---

 board/st/common/stm32mp_dfu.c |  4 
 doc/board/st/stm32mp1.rst | 18 ++
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/board/st/common/stm32mp_dfu.c b/board/st/common/stm32mp_dfu.c
index 00d1fb8f59..a3f0da5b5b 100644
--- a/board/st/common/stm32mp_dfu.c
+++ b/board/st/common/stm32mp_dfu.c
@@ -132,6 +132,10 @@ void set_dfu_alt_info(char *interface, char *devstr)
mtd = get_mtd_device_nm("nor0");
if (!IS_ERR_OR_NULL(mtd))
board_get_alt_info_mtd(mtd, buf);
+
+   mtd = get_mtd_device_nm("nor1");
+   if (!IS_ERR_OR_NULL(mtd))
+   board_get_alt_info_mtd(mtd, buf);
}
 
mtd = get_mtd_device_nm("nand0");
diff --git a/doc/board/st/stm32mp1.rst b/doc/board/st/stm32mp1.rst
index 42bb94148d..0c5d3a90f0 100644
--- a/doc/board/st/stm32mp1.rst
+++ b/doc/board/st/stm32mp1.rst
@@ -645,16 +645,18 @@ On EV1 board, booting from SD card, without OP-TEE_::
   dev: eMMC alt: 15 name: mmc1_rootfs layout: RAW_ADDR
   dev: eMMC alt: 16 name: mmc1_userfs layout: RAW_ADDR
   dev: MTD alt: 17 name: nor0 layout: RAW_ADDR
-  dev: MTD alt: 18 name: nand0 layout: RAW_ADDR
-  dev: VIRT alt: 19 name: OTP layout: RAW_ADDR
-  dev: VIRT alt: 20 name: PMIC layout: RAW_ADDR
+  dev: MTD alt: 18 name: nor1 layout: RAW_ADDR
+  dev: MTD alt: 19 name: nand0 layout: RAW_ADDR
+  dev: VIRT alt: 20 name: OTP layout: RAW_ADDR
+  dev: VIRT alt: 21 name: PMIC layout: RAW_ADDR
 
 All the supported device are exported for dfu-util tool::
 
   $> dfu-util -l
-  Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=20, 
name="PMIC", serial="00270038511934383330"
-  Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=19, 
name="OTP", serial="00270038511934383330"
-  Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=18, 
name="nand0", serial="00270038511934383330"
+  Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=21, 
name="PMIC", serial="00270038511934383330"
+  Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=20, 
name="OTP", serial="00270038511934383330"
+  Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=19, 
name="nand0", serial="00270038511934383330"
+  Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=18, 
name="nor1", serial="00270038511934383330"
   Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=17, 
name="nor0", serial="00270038511934383330"
   Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=16, 
name="mmc1_userfs", serial="00270038511934383330"
   Found DFU: [0483:df11] ver=, devnum=99, cfg=1, intf=0, alt=15, 
name="mmc1_rootfs", serial="00270038511934383330"
@@ -705,12 +707,12 @@ You can update the boot device:
 When the board is booting for nor0 or nand0,
 only the MTD partition on the boot devices are available, for example:
 
-- NOR (nor0 = alt 20) & NAND (nand0 = alt 26) ::
+- NOR (nor0 = alt 20, nor1 = alt 26) & NAND (nand0 = alt 27) :
 
   $> dfu-util -d 0483:5720 -a 21 -D tf-a-stm32mp157c-ev1.stm32
   $> dfu-util -d 0483:5720 -a 22 -D tf-a-stm32mp157c-ev1.stm32
   $> dfu-util -d 0483:5720 -a 23 -D fip-stm32mp157c-ev1.bin
-  $> dfu-util -d 0483:5720 -a 27 -D 
st-image-weston-openstlinux-weston-stm32mp1_nand_4_256_multivolume.ubi
+  $> dfu-util -d 0483:5720 -a 28 -D 
st-image-weston-openstlinux-weston-stm32mp1_nand_4_256_multivolume.ubi
 
 - NAND (nand0 = alt 21)::
 
-- 
2.25.1

Re: [PATCH v2 1/1] efi_loader: segfault in efi_clear_os_indications()

[Ilias Apalodimas]

On Wed, 24 Nov 2021 at 09:54, Heinrich Schuchardt
 wrote:
>
> If we call efi_clear_os_indications() before initializing the memory store
> for UEFI variables a NULL pointer dereference occurs.
>
> The error was observed on the sandbox with:
>
> usb start
> host bind 0 sandbox.img
> load host 0:1 $kernel_addr_r helloworld.efi
> bootefi $kernel_addr_r
>
> Here efi_resister_disk() failed due to an error in the BTRFS implementation.
>
> Move the logic to clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
> to the rest of the capsule code.
>
> If CONFIG_EFI_IGNORE_OSINDICATIONS=y, we should still clear the flag.
> If OsIndications does not exist, we should not create it as it is owned by
> the operating system.
>
> Fixes: 149108a3eb59 ("efi_loader: clear OsIndications")
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2:
>  move the logic to the rest of the capsule code
> ---
>  lib/efi_loader/efi_capsule.c | 45 
>  lib/efi_loader/efi_setup.c   | 36 +
>  2 files changed, 31 insertions(+), 50 deletions(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 502bcfca6e..8301eed631 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -1037,30 +1037,45 @@ efi_status_t __weak efi_load_capsule_drivers(void)
>  }
>
>  /**
> - * check_run_capsules - Check whether capsule update should run
> + * check_run_capsules() - check whether capsule update should run
>   *
>   * The spec says OsIndications must be set in order to run the capsule update
>   * on-disk.  Since U-Boot doesn't support runtime SetVariable, allow 
> capsules to
>   * run explicitly if CONFIG_EFI_IGNORE_OSINDICATIONS is selected
> + *
> + * Return: EFI_SUCCESS if update to run, EFI_NOT_FOUND otherwise
>   */
> -static bool check_run_capsules(void)
> +static efi_status_t check_run_capsules(void)
>  {
> u64 os_indications;
> efi_uintn_t size;
> -   efi_status_t ret;
> -
> -   if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS))
> -   return true;
> +   efi_status_t r;
>
> size = sizeof(os_indications);
> -   ret = efi_get_variable_int(L"OsIndications", 
> _global_variable_guid,
> -  NULL, , _indications, NULL);
> -   if (ret == EFI_SUCCESS &&
> -   (os_indications
> - & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED))
> -   return true;
> -
> -   return false;
> +   r = efi_get_variable_int(L"OsIndications", _global_variable_guid,
> +NULL, , _indications, NULL);
> +   if (r != EFI_SUCCESS || size != sizeof(os_indications))
> +   return EFI_NOT_FOUND;
> +
> +   if (os_indications &
> +   EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED) {
> +   os_indications &=
> +   ~EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED;
> +   r = efi_set_variable_int(L"OsIndications",
> +_global_variable_guid,
> +EFI_VARIABLE_NON_VOLATILE |
> +EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +EFI_VARIABLE_RUNTIME_ACCESS,
> +sizeof(os_indications),
> +_indications, false);
> +   if (r != EFI_SUCCESS)
> +   log_err("Setting %ls failed\n", L"OsIndications");
> +   return EFI_SUCCESS;
> +   } else if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS)) {
> +   return EFI_SUCCESS;
> +   } else  {
> +   return EFI_NOT_FOUND;
> +   }
>  }
>
>  /**
> @@ -1078,7 +1093,7 @@ efi_status_t efi_launch_capsules(void)
> unsigned int nfiles, index, i;
> efi_status_t ret;
>
> -   if (!check_run_capsules())
> +   if (check_run_capsules() != EFI_SUCCESS)
> return EFI_SUCCESS;
>
> index = get_last_capsule();
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index a2338d74af..1aba71cd96 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -175,36 +175,6 @@ static efi_status_t efi_init_os_indications(void)
>  }
>
>
> -/**
> - * efi_clear_os_indications() - clear OsIndications
> - *
> - * Clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
> - */
> -static efi_status_t efi_clear_os_indications(void)
> -{
> -   efi_uintn_t size;
> -   u64 os_indications;
> -   efi_status_t ret;
> -
> -   size = sizeof(os_indications);
> -   ret = efi_get_variable_int(L"OsIndications", 
> _global_variable_guid,
> -  NULL, , _indications, NULL);
> -   if (ret != EFI_SUCCESS)
> -   os_indications = 0;
> -   else
> -   os_indications &=
> -

[PATCH] pci: When disabling pref MEM set all base bits

[Pali Rohár]

It is common to set all base address bits to one and all limit address bits
to zero for disabling address forwarding. Forwarding is disabled when base
address is higher than limit address, so this change should not have any
effect.

Signed-off-by: Pali Rohár 
---
 drivers/pci/pci_auto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 6e5ed194f247..c0acf331398d 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -243,7 +243,7 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, 
int sub_bus)
cmdstat |= PCI_COMMAND_MEMORY;
} else {
/* We don't support prefetchable memory for now, so disable */
-   dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0x1000 |
+   dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0xfff0 |
prefechable_64);
dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT, 0x0 |
prefechable_64);
-- 
2.20.1

[PATCH] pci: Disable I/O forwarding during autoconfiguration if unsupported

[Pali Rohár]

If U-Boot does not have any I/O resource for assignment then disable I/O
forwarding in PCI bridge autoconfiguration code. Default initial state of
PCI bridge IO registers is unspecified, therefore they can be in enabled if
U-Boot does not touch them.

Signed-off-by: Pali Rohár 
---
 drivers/pci/pci_auto.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 7e6ee54be087..6e5ed194f247 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -265,6 +265,14 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, 
int sub_bus)
  (pci_io->bus_lower & 0x) >> 16);
 
cmdstat |= PCI_COMMAND_IO;
+   } else {
+   /* Disable I/O if unsupported */
+   dm_pci_write_config8(dev, PCI_IO_BASE, 0xf0 | io_32);
+   dm_pci_write_config8(dev, PCI_IO_LIMIT, 0x0 | io_32);
+   if (io_32 == PCI_IO_RANGE_TYPE_32) {
+   dm_pci_write_config16(dev, PCI_IO_BASE_UPPER16, 0x0);
+   dm_pci_write_config16(dev, PCI_IO_LIMIT_UPPER16, 0x0);
+   }
}
 
/* Enable memory and I/O accesses, enable bus master */
-- 
2.20.1

[PATCH] pci: Fix register for determining type of IO base address

[Pali Rohár]

Function dm_pciauto_prescan_setup_bridge() configures base address
registers, therefore it should read type of IO from base address registers
(and not from limit address registers).

Note that base and limit address registers should have same type, so this
change is just usage correction and has no functional change on correctly
working hardware.

Fixes: 8e85f36a8fab ("pci: Fix configuring io/memory base and limit registers 
of PCI bridges")
Signed-off-by: Pali Rohár 
---
 drivers/pci/pci_auto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 5af4ee6e56df..7e6ee54be087 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -197,7 +197,7 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, 
int sub_bus)
dm_pci_read_config16(dev, PCI_COMMAND, );
dm_pci_read_config16(dev, PCI_PREF_MEMORY_BASE, _64);
prefechable_64 &= PCI_PREF_RANGE_TYPE_MASK;
-   dm_pci_read_config8(dev, PCI_IO_LIMIT, _32);
+   dm_pci_read_config8(dev, PCI_IO_BASE, _32);
io_32 &= PCI_IO_RANGE_TYPE_MASK;
 
/* Configure bus number registers */
-- 
2.20.1

Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)

[Ilias Apalodimas]

Hi Simon,


On Wed, 24 Nov 2021 at 06:09, Simon Glass  wrote:
>
>
> This series adds support for the FIP format as used by ARM Trusted
> Firmware (in particular TF-A).
>
> This allows images to be created containing a FIP, which itself contains
> various binaries. With this, image creation can be handled from an in-tree
> image description instead of needing to perform a lot of manual steps or
> custom scripts to build the FIP.
>
>
> Simon Glass (2):
>   binman: Add a utility module for ATF FIP
>   binman: Add support for ATF FIP
>
>  scripts/pylint.base  |   9 +-
>  tools/binman/entries.rst | 154 ++
>  tools/binman/etype/atf_fip.py| 273 ++
>  tools/binman/fip_util.py | 653 +++
>  tools/binman/fip_util_test.py| 405 ++
>  tools/binman/ftest.py| 217 
>  tools/binman/main.py |   4 +-
>  tools/binman/test/203_fip.dts|  21 +
>  tools/binman/test/204_fip_other.dts  |  22 +
>  tools/binman/test/205_fip_no_type.dts|  15 +
>  tools/binman/test/206_fip_uuid.dts   |  22 +
>  tools/binman/test/207_fip_ls.dts |  25 +
>  tools/binman/test/208_fip_replace.dts|  33 ++
>  tools/binman/test/209_fip_missing.dts|  19 +
>  tools/binman/test/210_fip_size.dts   |  19 +
>  tools/binman/test/211_fip_bad_align.dts  |  18 +
>  tools/binman/test/212_fip_collection.dts |  24 +
>  17 files changed, 1929 insertions(+), 4 deletions(-)
>  create mode 100644 tools/binman/etype/atf_fip.py
>  create mode 100755 tools/binman/fip_util.py
>  create mode 100755 tools/binman/fip_util_test.py
>  create mode 100644 tools/binman/test/203_fip.dts
>  create mode 100644 tools/binman/test/204_fip_other.dts
>  create mode 100644 tools/binman/test/205_fip_no_type.dts
>  create mode 100644 tools/binman/test/206_fip_uuid.dts
>  create mode 100644 tools/binman/test/207_fip_ls.dts
>  create mode 100644 tools/binman/test/208_fip_replace.dts
>  create mode 100644 tools/binman/test/209_fip_missing.dts
>  create mode 100644 tools/binman/test/210_fip_size.dts
>  create mode 100644 tools/binman/test/211_fip_bad_align.dts
>  create mode 100644 tools/binman/test/212_fip_collection.dts
>
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>

My python is mediocre at best.  I'll try having a look, but CC'ing
TF-A developers would be a good idea.

Thanks
/Ilias

Re: [PATCH 7/7] RFC: Move Odroid-C2 to use binman to produce the image

[Neil Armstrong]

On 25/11/2021 01:12, Simon Glass wrote:
> Hi Neil,
> 
> On Wed, 24 Nov 2021 at 07:26, Neil Armstrong  wrote:
>>
>> Hi Simon,
>>
>> On 24/11/2021 05:09, Simon Glass wrote:
>>> This shows how binman can be used to replace the long and complicated
>>> instructions with an automated build. It is still complicated to read
>>> but users don't have to worry about the details.
>>
>> Thanks for demonstarting that !
> 
> Thanks for looking at it.
> 
>>
>> I'm really not confident about using proprietary tools from mainline u-boot
>> source tree.
> 
> Arguably people already are doing this. At least this way it is in the
> open. If people have the right tools installed it will just work, with
> no extra steps.

OK, no problem then

> 
>>
>> Will the binman step quietly fail if tools/bins aren't available ?
> 
> It handles the case where binaries are missing (that's the
> --allow-missing) but not tools. I think we can do a similar thing,
> where it just warns that the image won't work because of a missing
> tool.
> 
> When a blob is missing there are instructions to tell the user how to
> create it. For tools we could have instructions on where to download
> the tool.

Is there a easy way to bypass the binman step ? In case for example we
want to chainload the original u-boot binary, or wrap it using other
tools afterwards like for secure boot ?

> 
> Is someone working on upstreaming the tools?

There is some alternate open-source tools in C:

https://github.com/afaerber/meson-tools (GXBB, GXL & GXM only)
https://github.com/repk/gxlimg (GXBB, GXL, GXM & AXG only)
https://github.com/angerman/meson64-tools (developed for G12B, should work on 
G12A & SM1)

But no unified tool, all this should probably be rewritten in a binman plugin 
at some point.

> 
>>
>>> It needs some tidying up and only supports Odroid-C2 at present.
>>
>> --- C4
>>
>> But i get the spirit !
>>
>> Seems it should work as-is on allmost all boards except Odroid-C2 which has
>> only pre-signed binaries provided by HK.
> 
> OK. There are a lot of instructions in doc/board/amlogic - are they
> all mostly the same If so we can use a common binman description for
> all boards.
> 
>>
>> The only work will be to replace acs_tool.py for pre-G12 SoCs.
>>
>>>
>>> Signed-off-by: Simon Glass 
>>> ---
>>>
>>>  arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi | 107 
>>>  arch/arm/mach-meson/Kconfig  |   1 +
>>>  doc/board/amlogic/odroid-c4.rst  | 127 +--
>>>  scripts/pylint.base  |   1 +
>>>  tools/binman/etype/aml_encrypt.py| 124 ++
>>>  tools/binman/ftest.py|   3 +
>>>  tools/binman/missing-blob-help   |   6 +
>>>  tools/binman/test/213_aml_encrypt.dts|  38 ++
>>>  tools/binman/test/214_list_no_dtb.dts|  23 
>>>  9 files changed, 338 insertions(+), 92 deletions(-)
>>>  create mode 100644 tools/binman/etype/aml_encrypt.py
>>>  create mode 100644 tools/binman/test/213_aml_encrypt.dts
>>>  create mode 100644 tools/binman/test/214_list_no_dtb.dts
>>>
>>> diff --git a/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi 
>>> b/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi
>>> index 963bf96b256..b221ce6920b 100644
>>> --- a/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi
>>> +++ b/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi
>>> @@ -6,6 +6,113 @@
>>>
>>>  #include "meson-sm1-u-boot.dtsi"
>>>
>>> +/{
>>> + binman {
>>> + /* run --bootmk on all the included inputs */
>>> + aml-encrypt {
>>> + missing-msg = "aml-encrypt";
>>> + aml-algo = "g12a";
>>> + aml-op = "bootmk";
>>> + aml-level = "v3";
>>> +
>>> + /* produce a bl2, containing signed bl2 binaries */
>>> + bl2 {
>>> + type = "aml-encrypt";
>>> + aml-algo = "g12a";
>>> + aml-op = "bl2sig";
>>> +
>>> + /* sign the binary contaiing bl2 and acs */
>>> + aml-input {
>>> + type = "section";
>>> + bl2 {
>>> + type = "blob-ext";
>>> + size = <0xe000>;
>>> + filename = "bl2.bin";
>>> + };
>>> + acs {
>>> + type = "blob-ext";
>>> + size = <0x1000>;
>>> + filename = "acs.bin";
>>> + };
>>> + };
>>
>> This is nice way to get rid of blx_fix.sh !
> 
> Yes that sort of thing is 

Re: [PATCH] efi_loader: check tcg2 protocol installation outside the TCG protocol

[Ilias Apalodimas]

Hello Kojima-san

On Thu, 25 Nov 2021 at 08:04, Masahisa Kojima
 wrote:
>
> There are functions that calls tcg2_agile_log_append() outside
> of the TCG protocol invocation (e.g tcg2_measure_pe_image).
> These functions must to check that tcg2 protocol is installed.
> If not, measurement shall be skipped.
>
> Together with above change, this commit also removes the
> unnecessary tcg2_uninit() call in efi_tcg2_register(),
> refactors efi_tcg2_register() not to include the process
> that is not related to the tcg2 protocol registration.
>
> Signed-off-by: Masahisa Kojima 
> ---
>  include/efi_loader.h   |  4 ++
>  lib/efi_loader/efi_setup.c |  3 ++
>  lib/efi_loader/efi_tcg2.c  | 89 +++---
>  3 files changed, 81 insertions(+), 15 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index d52e399841..fe29c10906 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -525,6 +525,10 @@ efi_status_t efi_disk_register(void);
>  efi_status_t efi_rng_register(void);
>  /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
>  efi_status_t efi_tcg2_register(void);
> +/* Called by efi_init_obj_list() to do the required setup for the 
> measurement */
> +efi_status_t efi_tcg2_setup_measurement(void);
> +/* Called by efi_init_obj_list() to do initial measurement */
> +efi_status_t efi_tcg2_do_initial_measurement(void);
>  /* measure the pe-coff image, extend PCR and add Event Log */
>  efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
>struct efi_loaded_image_obj *handle,
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index a2338d74af..781d10590d 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -271,6 +271,9 @@ efi_status_t efi_init_obj_list(void)
> ret = efi_tcg2_register();
> if (ret != EFI_SUCCESS)
> goto out;
> +
> +   efi_tcg2_setup_measurement();
> +   efi_tcg2_do_initial_measurement();
> }
>
> /* Secure boot */
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 8c1f22e337..46300cae79 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -153,6 +153,20 @@ static u16 alg_to_len(u16 hash_alg)
> return 0;
>  }
>
> +/**
> + * is_tcg2_protocol_installed - chech whether tcg2 protocol is installed
> + *
> + * @Return: true if tcg2 protocol is installed, false if not
> + */
> +bool is_tcg2_protocol_installed(void)
> +{
> +   struct efi_handler *handler;
> +   efi_status_t ret;
> +
> +   ret = efi_search_protocol(efi_root, _guid_tcg2_protocol, 
> );
> +   return ((ret == EFI_SUCCESS) ? true : false);
> +}
> +
>  static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
>  {
> u32 len;
> @@ -886,6 +900,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 
> efi_size,
> IMAGE_NT_HEADERS32 *nt;
> struct efi_handler *handler;
>
> +   if (!is_tcg2_protocol_installed())
> +   return EFI_NOT_READY;
> +
> ret = platform_get_tpm2_device();
> if (ret != EFI_SUCCESS)
> return ret;
> @@ -1759,6 +1776,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct 
> efi_loaded_image_obj *ha
> u32 event = 0;
> struct smbios_entry *entry;
>
> +   if (!is_tcg2_protocol_installed())
> +   return EFI_NOT_READY;
> +
> if (tcg2_efi_app_invoked)
> return EFI_SUCCESS;
>
> @@ -1809,6 +1829,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
> efi_status_t ret;
> struct udevice *dev;
>
> +   if (!is_tcg2_protocol_installed())
> +   return EFI_NOT_READY;
> +
> ret = platform_get_tpm2_device();
> if (ret != EFI_SUCCESS)
> return ret;
> @@ -1833,6 +1856,11 @@ efi_tcg2_notify_exit_boot_services(struct efi_event 
> *event, void *context)
>
> EFI_ENTRY("%p, %p", event, context);
>
> +   if (!is_tcg2_protocol_installed()) {
> +   ret =  EFI_NOT_READY;
> +   goto out;
> +   }
> +
> event_log.ebs_called = true;
> ret = platform_get_tpm2_device();
> if (ret != EFI_SUCCESS)
> @@ -1863,6 +1891,9 @@ efi_status_t 
> efi_tcg2_notify_exit_boot_services_failed(void)
> struct udevice *dev;
> efi_status_t ret;
>
> +   if (!is_tcg2_protocol_installed())
> +   return EFI_NOT_READY;
> +
> ret = platform_get_tpm2_device();
> if (ret != EFI_SUCCESS)
> goto out;
> @@ -1932,6 +1963,45 @@ error:
> return ret;
>  }
>
> +/**
> + * efi_tcg2_setup_measurement() - setup for the measurement
> + *
> + * Return: status code
> + */
> +efi_status_t efi_tcg2_setup_measurement(void)
> +{
> +   efi_status_t ret;
> +   struct efi_event *event;
> +
> +   ret = 

Re: [PATCH v5 1/3] efi_loader: Add check for event log passed from firmware

[Ilias Apalodimas]

On Wed, 24 Nov 2021 at 17:09, Ruchika Gupta  wrote:
>
> Platforms may have support to measure their initial firmware components
> and pass the event log to u-boot. The event log address can be passed
> in property tpm_event_log_addr and tpm_event_log_size of the tpm node.
> Platforms may choose their own specific mechanism to do so. A weak
> function is added to check if even log has been passed to u-boot
> from earlier firmware components. If available, the eventlog is parsed
> to check for its correctness and further event logs are appended to the
> passed log.
>
> Signed-off-by: Ruchika Gupta 
> ---
> v5:
> Shift the efi_init_event_log() to a different location in the file.
> This help fixes compilation issue introduced by calling 
> efi_append_scrtm_version()
> from it.
>
> v4:
> Add SCRTM version to log only if previous firmware doesn't pass the eventlog
>
> v3:
> Return as soon as you detect error
>
> v2:
> Moved firmware eventlog code parsing to tcg2_get_fw_eventlog()
>
>  lib/efi_loader/efi_tcg2.c | 438 --
>  1 file changed, 369 insertions(+), 69 deletions(-)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 8c1f22e337..a789c44660 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -324,6 +324,45 @@ __weak efi_status_t platform_get_tpm2_device(struct 
> udevice **dev)
> return EFI_NOT_FOUND;
>  }
>
> +/**
> + * platform_get_eventlog() - retrieve the eventlog address and size
> + *
> + * This function retrieves the eventlog address and size if the underlying
> + * firmware has done some measurements and passed them.
> + *
> + * This function may be overridden based on platform specific method of
> + * passing the eventlog address and size.
> + *
> + * @dev:   udevice
> + * @addr:  eventlog address
> + * @sz:eventlog size
> + * Return: status code
> + */
> +__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr,
> + u32 *sz)
> +{
> +   const u64 *basep;
> +   const u32 *sizep;
> +
> +   basep = dev_read_prop(dev, "tpm_event_log_addr", NULL);
> +   if (!basep)
> +   return EFI_NOT_FOUND;
> +
> +   *addr = be64_to_cpup((__force __be64 *)basep);
> +
> +   sizep = dev_read_prop(dev, "tpm_event_log_size", NULL);
> +   if (!sizep)
> +   return EFI_NOT_FOUND;
> +
> +   *sz = be32_to_cpup((__force __be32 *)sizep);
> +   if (*sz == 0) {
> +   log_debug("event log empty\n");
> +   return EFI_NOT_FOUND;
> +   }
> +
> +   return EFI_SUCCESS;
> +}
> +
>  /**
>   * tpm2_get_max_command_size() - get the supported max command size
>   *
> @@ -1181,6 +1220,250 @@ static const struct efi_tcg2_protocol 
> efi_tcg2_protocol = {
> .get_result_of_set_active_pcr_banks = 
> efi_tcg2_get_result_of_set_active_pcr_banks,
>  };
>
> +/**
> + * parse_event_log_header() -  Parse and verify the event log header fields
> + *
> + * @buffer:Pointer to the event header
> + * @size:  Size of the eventlog
> + * @pos:   Position in buffer after event log header
> + *
> + * Return: status code
> + */
> +efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos)
> +{
> +   struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
> +   int i = 0;
> +
> +   if (size < sizeof(*event_header))
> +   return EFI_COMPROMISED_DATA;
> +
> +   if (get_unaligned_le32(_header->pcr_index) != 0 ||
> +   get_unaligned_le32(_header->event_type) != EV_NO_ACTION)
> +   return EFI_COMPROMISED_DATA;
> +
> +   for (i = 0; i < sizeof(event_header->digest); i++) {
> +   if (event_header->digest[i] != 0)
> +   return EFI_COMPROMISED_DATA;
> +   }
> +
> +   *pos += sizeof(*event_header);
> +
> +   return EFI_SUCCESS;
> +}
> +
> +/**
> + * parse_specid_event() -  Parse and verify the specID Event in the eventlog
> + *
> + * @dev:   udevice
> + * @buffer:Pointer to the start of the eventlog
> + * @log_size:  Size of the eventlog
> + * @pos:   Offset in the evenlog where specID event starts
> + *
> + * Return: status code
> + * @posOffset in the eventlog where the specID event 
> ends
> + * @digest_list:   list of digests in the event
> + */
> +efi_status_t parse_specid_event(struct udevice *dev, void *buffer, u32 
> log_size,
> +   u32 *pos,
> +   struct tpml_digest_values *digest_list)
> +{
> +   struct tcg_efi_spec_id_event *spec_event;
> +   struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
> +   size_t spec_event_size;
> +   u32 active = 0, supported = 0, pcr_count = 0, alg_count = 0;
> +   u32 spec_active = 0;
> +   u16 

Re: [PATCH v5 1/3] efi_loader: Add check for event log passed from firmware

[Ilias Apalodimas]

On Wed, 24 Nov 2021 at 17:09, Ruchika Gupta  wrote:
>
> Platforms may have support to measure their initial firmware components
> and pass the event log to u-boot. The event log address can be passed
> in property tpm_event_log_addr and tpm_event_log_size of the tpm node.
> Platforms may choose their own specific mechanism to do so. A weak
> function is added to check if even log has been passed to u-boot
> from earlier firmware components. If available, the eventlog is parsed
> to check for its correctness and further event logs are appended to the
> passed log.
>
> Signed-off-by: Ruchika Gupta 
> ---
> v5:
> Shift the efi_init_event_log() to a different location in the file.
> This help fixes compilation issue introduced by calling 
> efi_append_scrtm_version()
> from it.
>
> v4:
> Add SCRTM version to log only if previous firmware doesn't pass the eventlog
>
> v3:
> Return as soon as you detect error
>
> v2:
> Moved firmware eventlog code parsing to tcg2_get_fw_eventlog()
>
>  lib/efi_loader/efi_tcg2.c | 438 --
>  1 file changed, 369 insertions(+), 69 deletions(-)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 8c1f22e337..a789c44660 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -324,6 +324,45 @@ __weak efi_status_t platform_get_tpm2_device(struct 
> udevice **dev)
> return EFI_NOT_FOUND;
>  }
>
> +/**
> + * platform_get_eventlog() - retrieve the eventlog address and size
> + *
> + * This function retrieves the eventlog address and size if the underlying
> + * firmware has done some measurements and passed them.
> + *
> + * This function may be overridden based on platform specific method of
> + * passing the eventlog address and size.
> + *
> + * @dev:   udevice
> + * @addr:  eventlog address
> + * @sz:eventlog size
> + * Return: status code
> + */
> +__weak efi_status_t platform_get_eventlog(struct udevice *dev, u64 *addr,
> + u32 *sz)
> +{
> +   const u64 *basep;
> +   const u32 *sizep;
> +
> +   basep = dev_read_prop(dev, "tpm_event_log_addr", NULL);
> +   if (!basep)
> +   return EFI_NOT_FOUND;
> +
> +   *addr = be64_to_cpup((__force __be64 *)basep);
> +
> +   sizep = dev_read_prop(dev, "tpm_event_log_size", NULL);
> +   if (!sizep)
> +   return EFI_NOT_FOUND;
> +
> +   *sz = be32_to_cpup((__force __be32 *)sizep);
> +   if (*sz == 0) {
> +   log_debug("event log empty\n");
> +   return EFI_NOT_FOUND;
> +   }
> +
> +   return EFI_SUCCESS;
> +}
> +
>  /**
>   * tpm2_get_max_command_size() - get the supported max command size
>   *
> @@ -1181,6 +1220,250 @@ static const struct efi_tcg2_protocol 
> efi_tcg2_protocol = {
> .get_result_of_set_active_pcr_banks = 
> efi_tcg2_get_result_of_set_active_pcr_banks,
>  };
>
> +/**
> + * parse_event_log_header() -  Parse and verify the event log header fields
> + *
> + * @buffer:Pointer to the event header
> + * @size:  Size of the eventlog
> + * @pos:   Position in buffer after event log header
> + *
> + * Return: status code
> + */
> +efi_status_t parse_event_log_header(void *buffer, u32 size, u32 *pos)
> +{
> +   struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
> +   int i = 0;
> +
> +   if (size < sizeof(*event_header))
> +   return EFI_COMPROMISED_DATA;
> +
> +   if (get_unaligned_le32(_header->pcr_index) != 0 ||
> +   get_unaligned_le32(_header->event_type) != EV_NO_ACTION)
> +   return EFI_COMPROMISED_DATA;
> +
> +   for (i = 0; i < sizeof(event_header->digest); i++) {
> +   if (event_header->digest[i] != 0)
> +   return EFI_COMPROMISED_DATA;
> +   }
> +
> +   *pos += sizeof(*event_header);
> +
> +   return EFI_SUCCESS;
> +}
> +
> +/**
> + * parse_specid_event() -  Parse and verify the specID Event in the eventlog
> + *
> + * @dev:   udevice
> + * @buffer:Pointer to the start of the eventlog
> + * @log_size:  Size of the eventlog
> + * @pos:   Offset in the evenlog where specID event starts
> + *
> + * Return: status code
> + * @posOffset in the eventlog where the specID event 
> ends
> + * @digest_list:   list of digests in the event
> + */
> +efi_status_t parse_specid_event(struct udevice *dev, void *buffer, u32 
> log_size,
> +   u32 *pos,
> +   struct tpml_digest_values *digest_list)
> +{
> +   struct tcg_efi_spec_id_event *spec_event;
> +   struct tcg_pcr_event *event_header = (struct tcg_pcr_event *)buffer;
> +   size_t spec_event_size;
> +   u32 active = 0, supported = 0, pcr_count = 0, alg_count = 0;
> +   u32 spec_active = 0;
> +   u16 

Re: [PATCH v5 3/3] efi_loader: Extend PCR's for firmware measurements

[Ilias Apalodimas]

On Wed, 24 Nov 2021 at 17:09, Ruchika Gupta  wrote:
>
> Firmwares before U-Boot may be capable of doing tpm measurements
> and passing them to U-Boot in the form of eventlog. However there
> may be scenarios where the firmwares don't have TPM driver and
> are not capable of extending the measurements in the PCRs.
> Based on TCG spec, if previous firnware has extended PCR's, PCR0
> would not be 0. So, read the PCR0 to determine if the PCR's need
> to be extended as eventlog is parsed or not.
>
> Signed-off-by: Ruchika Gupta 
> Reviewed-by: Ilias Apalodimas 
> ---
> v5 : No change
>
> v4 : No change
>
> v3 :
> Rebase changes on top of changes made in first patch of series
>
> v2 :
> Removed check for PCR0 in eventlog
>
>  lib/efi_loader/efi_tcg2.c | 75 +++
>  1 file changed, 75 insertions(+)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index a789c44660..b44eed0ec9 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -199,6 +199,43 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, 
> u32 pcr_index,
> return EFI_SUCCESS;
>  }
>
> +/* tcg2_pcr_read - Read PCRs for a TPM2 device for a given tpml_digest_values
> + *
> + * @dev:   device
> + * @digest_list:   list of digest algorithms to extend
> + *
> + * @Return: status code
> + */
> +static efi_status_t tcg2_pcr_read(struct udevice *dev, u32 pcr_index,
> + struct tpml_digest_values *digest_list)
> +{
> +   struct tpm_chip_priv *priv;
> +   unsigned int updates, pcr_select_min;
> +   u32 rc;
> +   size_t i;
> +
> +   priv = dev_get_uclass_priv(dev);
> +   if (!priv)
> +   return EFI_DEVICE_ERROR;
> +
> +   pcr_select_min = priv->pcr_select_min;
> +
> +   for (i = 0; i < digest_list->count; i++) {
> +   u16 hash_alg = digest_list->digests[i].hash_alg;
> +   u8 *digest = (u8 *)_list->digests[i].digest;
> +
> +   rc = tpm2_pcr_read(dev, pcr_index, pcr_select_min,
> +  hash_alg, digest, alg_to_len(hash_alg),
> +  );
> +   if (rc) {
> +   EFI_PRINT("Failed to read PCR\n");
> +   return EFI_DEVICE_ERROR;
> +   }
> +   }
> +
> +   return EFI_SUCCESS;
> +}
> +
>  /* put_event - Append an agile event to an eventlog
>   *
>   * @pcr_index: PCR index
> @@ -1428,6 +1465,8 @@ efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, 
> void *log_buffer,
> u32 pcr, pos;
> u64 base;
> u32 sz;
> +   bool extend_pcr = false;
> +   int i;
>
> ret = platform_get_eventlog(dev, , );
> if (ret != EFI_SUCCESS)
> @@ -1449,6 +1488,26 @@ efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, 
> void *log_buffer,
> return EFI_COMPROMISED_DATA;
> }
>
> +   ret = tcg2_pcr_read(dev, 0, _list);
> +   if (ret) {
> +   log_err("Error reading PCR 0\n");
> +   return ret;
> +   }
> +
> +   /*
> +* If PCR0 is 0, previous firmware didn't have the capability
> +* to extend the PCR. In this scenario, extend the PCR as
> +* the eventlog is parsed.
> +*/
> +   for (i = 0; i < digest_list.count; i++) {
> +   u8 buffer[TPM2_DIGEST_LEN] =  { 0 };
> +   u16 hash_alg = digest_list.digests[i].hash_alg;
> +
> +   if (!memcmp((u8 *)_list.digests[i].digest, buffer,
> +   alg_to_len(hash_alg)))
> +   extend_pcr = true;
> +   }
> +
> while (pos < sz) {
> ret = tcg2_parse_event(dev, buffer, sz, , _list,
>);
> @@ -1456,6 +1515,22 @@ efi_status_t tcg2_get_fw_eventlog(struct udevice *dev, 
> void *log_buffer,
> log_err("Error parsing event\n");
> return ret;
> }
> +   if (extend_pcr) {
> +   ret = tcg2_pcr_extend(dev, pcr, _list);
> +   if (ret != EFI_SUCCESS) {
> +   log_err("Error in extending PCR\n");
> +   return ret;
> +   }
> +
> +   /* Clear the digest for next event */
> +   for (i = 0; i < digest_list.count; i++) {
> +   u16 hash_alg = 
> digest_list.digests[i].hash_alg;
> +   u8 *digest =
> +  (u8 *)_list.digests[i].digest;
> +
> +   memset(digest, 0, alg_to_len(hash_alg));
> +   }
> +   }
> }
>
> memcpy(log_buffer, buffer, sz);
> --
> 2.25.1
>
Tested-by: Ilias Apalodimas