Re: [PATCH v3 09/14] bloblist: Handle alignment with a void entry

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 19:49, Simon Glass  wrote:
>
> Hi Ilias,
>
> On Wed, Dec 27, 2023 at 9:58 AM Ilias Apalodimas
>  wrote:
> >
> > On Mon, 18 Dec 2023 at 20:20, Raymond Mao  wrote:
> > >
> > > From: Simon Glass 
> > >
> > > Rather than setting the alignment using the header size, add an entirely
> > > new entry to cover the gap left by the alignment.
> >
> > Hmm, why? Does it make out life easier somehow if new entries get added?
>

Ah thanks. I did indeed miss that change on the spec

Thanks
/Ilias
> This was one of the spec changes. Not one that I agreed with, but there it is.
>
> Regards,
> Simon

Re: [PATCH v4 10/12] bloblist: Adjust the bloblist header

[Ilias Apalodimas]

Hi Raymond,

[...]

>
>  void bloblist_show_list(void)
> @@ -463,7 +477,7 @@ void bloblist_reloc(void *to, uint to_size, void *from, 
> uint from_size)
>
> memcpy(to, from, from_size);
> hdr = to;
> -   hdr->size = to_size;
> +   hdr->total_size = to_size;
>  }
>
>  int bloblist_init(void)
> @@ -493,7 +507,7 @@ int bloblist_init(void)
> addr, ret);
> } else {
> /* Get the real size, if it is not what we expected */
> -   size = gd->bloblist->size;
> +   size = gd->bloblist->total_size;
> }
> }
> if (ret) {
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 7024d7bf9e..4ec4b3d449 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -166,32 +166,33 @@ enum bloblist_tag_t {
>   * from the last.
>   *
>   * @magic: BLOBLIST_MAGIC
> + * @chksum: checksum for the entire bloblist allocated area. Since any of the
> + * blobs can be altered after being created, this checksum is only valid
> + * when the bloblist is finalized before jumping to the next stage of 
> boot.
> + * This is the value needed to make all checksummed bytes sum to 0
>   * @version: BLOBLIST_VERSION
>   * @hdr_size: Size of this header, normally sizeof(struct bloblist_hdr). The
>   * first bloblist_rec starts at this offset from the start of the header
> - * @flags: Space for BLOBLISTF... flags (none yet)
> - * @size: Total size of the bloblist (non-zero if valid) including this 
> header.
> - * The bloblist extends for this many bytes from the start of this 
> header.
> - * When adding new records, the bloblist can grow up to this size.
> - * @alloced: Total size allocated so far for this bloblist. This starts out 
> as
> + * @align_log2: Power of two of the maximum alignment required by this list
> + * @used_size: Size allocated so far for this bloblist. This starts out as
>   * sizeof(bloblist_hdr) since we need at least that much space to store a
>   * valid bloblist
> + * @total_size: The number of total bytes that the bloblist can occupy.
> + * Any blob producer must check if there is sufficient space before 
> adding
> + * a record to the bloblist.
> + * @flags: Space for BLOBLISTF... flags (none yet)
>   * @spare: Spare space (for future use)
> - * @chksum: checksum for the entire bloblist allocated area. Since any of the
> - * blobs can be altered after being created, this checksum is only valid
> - * when the bloblist is finalised before jumping to the next stage of 
> boot.
> - * This is the value needed to make all checksummed bytes sum to 0
>   */
>  struct bloblist_hdr {
> u32 magic;
> -   u32 version;
> -   u32 hdr_size;
> +   u8 chksum;
> +   u8 version;
> +   u8 hdr_size;
> +   u8 align_log2;
> +   u32 used_size;
> +   u32 total_size;
> u32 flags;
> -
> -   u32 size;
> -   u32 alloced;
> u32 spare;
> -   u32 chksum;
>  };

The patch generally looks ok, but while we are renaming things, can't
we just copy what the spec says instead of slightly changing the
names?

With this applied we end up with
  struct bloblist_hdr {
 u32 magic;
 u8 chksum;
 u8 version;
 u8 hdr_size;
 u8 align_log2;
 u32 used_size;
 u32 total_size;
 u32 flags;
 u32 spare;
  };

Can you at least rename the ones you touch here properly?
- Drop the patch that renames spare to _spare
- used_size -> size
- total_size -> max_size (which btw mean different things)
- spare -> reserved


Thanks
/Ilias

[...]

Thanks
/Ilias

[PATCH 2/2] cpu: riscv: set correct SMBIOS processor family value

[Heinrich Schuchardt]

The SMBIOS specification requires to set the processor family in the type 4
(Processor Information) table to specific values depending only on the
bitness of the system (0x200 for RV32 and 0x201 for RV64).

With this patch dmidecode shows

Handle 0x0004, DMI type 4, 48 bytes
Processor Information
Socket Designation: Not Specified
Type: Central Processor
Family: RV64

for qemu-riscv64_smode_defconfig.

Signed-off-by: Heinrich Schuchardt 
---
 drivers/cpu/riscv_cpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
index d6484d7f4b..034b9b49c0 100644
--- a/drivers/cpu/riscv_cpu.c
+++ b/drivers/cpu/riscv_cpu.c
@@ -98,6 +98,10 @@ static int riscv_cpu_bind(struct udevice *dev)
 
/* save the hart id */
plat->cpu_id = dev_read_addr(dev);
+   if (IS_ENABLED(CONFIG_64BIT))
+   plat->family = 0x201;
+   else
+   plat->family = 0x200;
/* first examine the property in current cpu node */
ret = dev_read_u32(dev, "timebase-frequency", >timebase_freq);
/* if not found, then look at the parent /cpus node */
-- 
2.43.0

[PATCH 1/2] smbios: enable setting processor family > 0xff

[Heinrich Schuchardt]

Many value of processor type exceed 0xff and have to be stored as u16
value. In the type 4 table set processor_family = 0xfe signaling that
field processor_family2 is used and write the actual value into the
processor_family2 field.

Signed-off-by: Heinrich Schuchardt 
---
 lib/smbios.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index 45480b01af..550b2471f9 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -467,7 +467,8 @@ static void smbios_write_type4_dm(struct smbios_type4 *t,
}
 #endif
 
-   t->processor_family = processor_family;
+   t->processor_family = 0xfe;
+   t->processor_family2 = processor_family;
t->processor_manufacturer = smbios_add_prop(ctx, NULL, vendor);
t->processor_version = smbios_add_prop(ctx, NULL, name);
 }
@@ -489,7 +490,6 @@ static int smbios_write_type4(ulong *current, int handle,
t->l1_cache_handle = 0x;
t->l2_cache_handle = 0x;
t->l3_cache_handle = 0x;
-   t->processor_family2 = t->processor_family;
 
len = t->length + smbios_string_table_len(ctx);
*current += len;
-- 
2.43.0

[PATCH 0/2] smbios: riscv: set correct SMBIOS processor family value

[Heinrich Schuchardt]

Many value of processor type exceed 0xff and have to be stored as u16
value. In the type 4 table set processor_family = 0xfe signaling that
field processor_family2 is used and write the actual value into the
processor_family2 field.

The values for RISC-V are:

* 0x200 for 32-bit
* 0x201 for 64-bit

With this series dmidecode shows

Handle 0x0004, DMI type 4, 48 bytes
Processor Information
Socket Designation: Not Specified
Type: Central Processor
Family: RV64

for qemu-riscv64_smode_defconfig.

Heinrich Schuchardt (2):
  smbios: enable setting processor family > 0xff
  cpu: riscv: set correct SMBIOS processor family value

 drivers/cpu/riscv_cpu.c | 4 
 lib/smbios.c| 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.43.0

Re: [PATCH v4 06/12] bloblist: Drop spare value from bloblist record

[Ilias Apalodimas]

Hi Raymond,

On Wed, 27 Dec 2023 at 23:08, Raymond Mao  wrote:
>
> From: Simon Glass 
>
> Drop spare value from bloblist record header.
>
> For now it is still present in the header, with an underscore, so that
> tests continue to pass.
>

Looking at it again, this commit makes no sense to me. Why is this
needed in the series? And why can't we just fold it into patch #10?

Thanks
/Ilias
> Signed-off-by: Simon Glass 
> Co-developed-by: Raymond Mao 
> Signed-off-by: Raymond Mao 
> ---
> Changes in v3
> - Keep the spare value in the bloblist header to align to FW handoff spec up 
> to commit 3592349.
>
>  common/bloblist.c  | 1 -
>  include/bloblist.h | 3 +--
>  2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 168993e0a7..88e2a0f5c0 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -165,7 +165,6 @@ static int bloblist_addrec(uint tag, int size, int 
> align_log2,
> rec->tag = tag;
> rec->hdr_size = data_start - hdr->alloced;
> rec->size = size;
> -   rec->spare = 0;
>
> /* Zero the record data */
> memset((void *)rec + rec_hdr_size(rec), '\0', rec->size);
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 7eff709ec8..68f97395b7 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -205,13 +205,12 @@ struct bloblist_hdr {
>   * record's data starts at this offset from the start of the record
>   * @size: Size of record in bytes, excluding the header size. This does not
>   * need to be aligned (e.g. 3 is OK).
> - * @spare: Spare space for other things
>   */
>  struct bloblist_rec {
> u32 tag;
> u32 hdr_size;
> u32 size;
> -   u32 spare;
> +   u32 _spare;
>  };
>
>  /**
> --
> 2.25.1
>

Re: [PATCH v4 01/12] bloblist: Update the tag numbering

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 23:07, Raymond Mao  wrote:
>
> From: Simon Glass 
>
> Align bloblist tags with the FW handoff spec v0.9.
> The most common ones are from 0.
> TF related ones are from 0x100.
> All non-standard ones from 0xfff000.
>
> Added new defined tags:
> BLOBLISTT_OPTEE_PAGABLE_PART for TF.
> BLOBLISTT_TPM_EVLOG and BLOBLISTT_TPM_CRB_BASE for TPM.
>
> Signed-off-by: Simon Glass 
> Co-developed-by: Raymond Mao 
> Signed-off-by: Raymond Mao 
> ---
> Changes in v2
> - Align bloblist tags to FW handoff spec v0.9.
> Changes in v3
> - Add TPM related tags
>
>  common/bloblist.c  | 18 ++---
>  include/bloblist.h | 67 +-
>  test/bloblist.c|  4 +--
>  3 files changed, 52 insertions(+), 37 deletions(-)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index a22f6c12b0..5606487f5b 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -36,16 +36,26 @@ static struct tag_name {
> enum bloblist_tag_t tag;
> const char *name;
>  } tag_name[] = {
> -   { BLOBLISTT_NONE, "(none)" },
> +   { BLOBLISTT_VOID, "(void)" },
>
> /* BLOBLISTT_AREA_FIRMWARE_TOP */
> +   { BLOBLISTT_CONTROL_FDT, "Control FDT" },
> +   { BLOBLISTT_HOB_BLOCK, "HOB block" },
> +   { BLOBLISTT_HOB_LIST, "HOB list" },
> +   { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
> +   { BLOBLISTT_TPM_EVLOG, "TPM event log defined by TCG EFI" },
> +   { BLOBLISTT_TPM_CRB_BASE, "TPM Command Response Buffer address" },
>
> /* BLOBLISTT_AREA_FIRMWARE */
> -   { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" },
> -   { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" },
> { BLOBLISTT_TPM2_TCG_LOG, "TPM v2 log space" },
> { BLOBLISTT_TCPA_LOG, "TPM log space" },
> -   { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
> +   { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" },
> +
> +   /* BLOBLISTT_AREA_TF */
> +   { BLOBLISTT_OPTEE_PAGABLE_PART, "OP-TEE pagable part" },
> +
> +   /* BLOBLISTT_AREA_OTHER */
> +   { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" },
> { BLOBLISTT_SMBIOS_TABLES, "SMBIOS tables for x86" },
> { BLOBLISTT_VBOOT_CTX, "Chrome OS vboot context" },
>
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 080cc46a12..92dbfda21b 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -81,7 +81,7 @@ enum {
>
>  /* Supported tags - add new ones to tag_name in bloblist.c */
>  enum bloblist_tag_t {
> -   BLOBLISTT_NONE = 0,
> +   BLOBLISTT_VOID = 0,
>
> /*
>  * Standard area to allocate blobs used across firmware components, 
> for
> @@ -89,42 +89,36 @@ enum bloblist_tag_t {
>  * projects.
>  */
> BLOBLISTT_AREA_FIRMWARE_TOP = 0x1,
> +   /*
> +* Devicetree for use by firmware. On some platforms this is passed to
> +* the OS also
> +*/
> +   BLOBLISTT_CONTROL_FDT = 1,
> +   BLOBLISTT_HOB_BLOCK = 2,
> +   BLOBLISTT_HOB_LIST = 3,
> +   BLOBLISTT_ACPI_TABLES = 4,
> +   BLOBLISTT_TPM_EVLOG = 5,
> +   BLOBLISTT_TPM_CRB_BASE = 6,
>
> /* Standard area to allocate blobs used across firmware components */
> -   BLOBLISTT_AREA_FIRMWARE = 0x100,
> +   BLOBLISTT_AREA_FIRMWARE = 0x10,
> +   BLOBLISTT_TPM2_TCG_LOG = 0x10,  /* TPM v2 log space */
> +   BLOBLISTT_TCPA_LOG = 0x11,  /* TPM log space */
> /*
>  * Advanced Configuration and Power Interface Global Non-Volatile
>  * Sleeping table. This forms part of the ACPI tables passed to Linux.
>  */
> -   BLOBLISTT_ACPI_GNVS = 0x100,
> -   BLOBLISTT_INTEL_VBT = 0x101,/* Intel Video-BIOS table */
> -   BLOBLISTT_TPM2_TCG_LOG = 0x102, /* TPM v2 log space */
> -   BLOBLISTT_TCPA_LOG = 0x103, /* TPM log space */
> -   BLOBLISTT_ACPI_TABLES = 0x104,  /* ACPI tables for x86 */
> -   BLOBLISTT_SMBIOS_TABLES = 0x105, /* SMBIOS tables for x86 */
> -   BLOBLISTT_VBOOT_CTX = 0x106,/* Chromium OS verified boot context 
> */
> +   BLOBLISTT_ACPI_GNVS = 0x12,
>
> -   /*
> -* Project-specific tags are permitted here. Projects can be open 
> source
> -* or not, but the format of the data must be fuily documented in an
> -* open source project, including all fields, bits, etc. Naming should
> -* be: BLOBLISTT__
> -*/
> -   BLOBLISTT_PROJECT_AREA = 0x8000,
> -   BLOBLISTT_U_BOOT_SPL_HANDOFF = 0x8000, /* Hand-off info from SPL */
> -   BLOBLISTT_VBE   = 0x8001,   /* VBE per-phase state */
> -   BLOBLISTT_U_BOOT_VIDEO = 0x8002, /* Video information from SPL */
> -
> -   /*
> -* Vendor-specific tags are permitted here. Projects can be open 
> source
> -* or not, but the format of the data must be fuily documented in an
> -* open source project, including all fields, bits, etc. Naming should
> -* be BLOBLISTT__
> -*/
> -   

Re: [PATCH v4] fdt: Allow the devicetree to come from a bloblist

[Ilias Apalodimas]

Hi Tom,

[...]

> >
> > No, but that is just a straw man. The DT is special and U-Boot reports
> > where it comes from.
> >
> > >
> > > This has been going back and forth for a while. I've lost count of how
> > > many times I repeated the same proposal, but here it goes again. We
> > > have OF_BOARD and BLOBLIST options. The bloblist and its properties
> > > are scannable at runtime. Can't we use the combination of these 2 can
> > > be used to imply we expect things from a bloblist. If we want to be
> > > stricter in the future and explicitly expect the DT from a bloblist,
> > > we could add a Kconfig option failing the boot if that's missing.
> >
> > I would like to have that Kconfig option now, not later. In my mind,
> > the boot must be deterministic, so that if OF_BLOBLIST is enabled, the
> > DT must come from there, or it is an error.
>
> Determinism doesn't require a CONFIG option, it just requires an if/else
> tree where we say what the "correct" priority list should be and then
> set a flag so that we can tell the user where we found it too. This also
> means that we can get whatever is going to use this mechanism to
> migrate over, and have less of a chicken-and-egg type of problem.
>
> > Also, repeating it doesn't make the proposal good. We agreed that
> > OF_BOARD would eventually go away, so building on top of it is not
> > setting us up for the future.
>
> I wonder if OF_BOARD will ever go away, and I'm not convinced it will
> either.

It won't :) -- ever. You will still have hardware that reads it from
an EEPROM for example, or something like that.
People usually keep the first-stage bootloaders simple, so adding more
drivers to read something is usually deferred to later, richer
bootloaders, unless necessary

> Unless you just mean re-naming it and making a few ad-hoc
> standards more easily re-usable, which also will need to happen
> regardless.

Yes, that was what I initially suggested.

Thanks
/Ilias
>
> --
> Tom

Re: [PATCH v4] fdt: Allow the devicetree to come from a bloblist

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 19:48, Simon Glass  wrote:
>
> Hi Ilias,
>
> On Wed, Dec 27, 2023 at 6:44 AM Ilias Apalodimas
>  wrote:
> >
> > Hi Tom,
> >
> > On Tue, 26 Dec 2023 at 14:07, Tom Rini  wrote:
> > >
> > > On Tue, Dec 26, 2023 at 09:46:25AM +, Simon Glass wrote:
> > >
> > > > Standard passage provides for a bloblist to be passed from one firmware
> > > > phase to the next. That can be used to pass the devicetree along as 
> > > > well.
> > > > Add an option to support this.
> > > >
> > > > Tests for this will be added as part of the Universal Payload work.
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > ---
> > > > The discussion on this was not resolved and is now important due to the
> > > > bloblist series from Raymond. So I am sending it again since I believe
> > > > this is a better starting point than building on OF_BOARD
> > >
> > > I really don't like adding another option for "DT is given to us". Why
> > > isn't adding another enum to fdt_source_t sufficient, and if we have
> > > bloblist enabled, that will look for and use if found? Maybe some other
> > > code needs to be restructured and cleaned up too?
> >
> > Same here. On top of that the bloblist has a few items in there, e.g a
> > TPM eventlog. What are we going to do? Add a Kconfig for each item?
>
> No, but that is just a straw man. The DT is special and U-Boot reports
> where it comes from.

The story is identical to the EventLog. It's 'special' as well and
U-Boot has to pick it up, otherwise the hardware and the EventLog will
end up with different values

>
> >
> > This has been going back and forth for a while. I've lost count of how
> > many times I repeated the same proposal, but here it goes again. We
> > have OF_BOARD and BLOBLIST options. The bloblist and its properties
> > are scannable at runtime. Can't we use the combination of these 2 can
> > be used to imply we expect things from a bloblist. If we want to be
> > stricter in the future and explicitly expect the DT from a bloblist,
> > we could add a Kconfig option failing the boot if that's missing.
>
> I would like to have that Kconfig option now, not later. In my mind,
> the boot must be deterministic, so that if OF_BLOBLIST is enabled, the
> DT must come from there, or it is an error.

And how is what I proposed not deterministic? What prevents us from
adding that Kconfig option now?
The only difference is that by default, U-Boot will try bloblist ->
board-specific method unless a Kconfig option prevents the fallback.
I've been working with vendors and helping them implement the firmware
handoff spec in their proprietary bootloaders and Raymond contributed
the implementation for TF-A. At the same time, I am pretty sure
vendors will need time to implement this properly. I don't see how
adding an implicit Kconfig option will help them push this forward.

IOW I prefer U-Boot to *always* scan for a bloblist as the first
discovery DT method (unless the DT comes bundled with U-Boot), rather
than expecting users to turn a Kconfig knob.

>
> Also, repeating it doesn't make the proposal good. We agreed that
> OF_BOARD would eventually go away, so building on top of it is not
> setting us up for the future.

No, we didn't [0]. In fact, I said I don't see OF_BOARD *ever* going
away. The only thing I suggested was to rename it

[...]

[0] 
https://lore.kernel.org/u-boot/cac_iwjkrktm4spyxpoddartj41a553vde+xm5gz3+jsntfq...@mail.gmail.com/

Thanks
/Ilias

[PATCH] cmd: net: nfs: Enable the NFS command by default

[Tejas Bhumkar]

Activated the default use of NFS command for booting
images via network using the NFS protocol.

Signed-off-by: Tejas Bhumkar 
---
 cmd/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index df6d71c103..a51b2d532f 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1905,6 +1905,7 @@ config CMD_RARP
 
 config CMD_NFS
bool "nfs"
+   default y
help
  Boot image via network using NFS protocol.
 
-- 
2.27.0

[GIT PULL] u-boot-riscv/next

[Leo Liang]

Hi Tom,

The following changes since commit 4b151562bb8e54160adedbc6a1c0c749c00a2f84:

  bootmeth: pass size to efi_binary_run() (2023-12-22 10:36:50 -0500)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-riscv.git next

for you to fetch changes up to 9924d44dbcd47bd3664fa9f1f9f24044d83eaebf:

  andes: ae350: Enable MISC_INIT_R for ae350 platform (2023-12-27 17:29:11 
+0800)

CI result shows no issue: 
https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/19106

- Andes: Enable Andes CPU memboost and ECC feature by default 
- Sifive: Add private L2 cache driver

Leo Yu-Chi Liang (6):
  andes: csr.h: Clean up CSR definition
  andes: ae350: Implement cache switch via Kconfig
  andes: cpu: Enable memboost feature
  andes: cpu: Enable cache and TLB ECC support
  andes: ae350: Save cpu name to env
  andes: ae350: Enable MISC_INIT_R for ae350 platform

Michal Simek (1):
  riscv: Extend board compatible string with "qemu,mbv"

Zong Li (2):
  cache: add sifive private L2 cache driver
  riscv: cache: support cache enable in SPL stage

 arch/riscv/cpu/andesv5/cpu.c| 33 ++---
 arch/riscv/dts/xilinx-mbv32.dts |  2 +-
 arch/riscv/include/asm/arch-andes/csr.h | 29 +-
 arch/riscv/include/asm/csr.h|  1 +
 arch/riscv/lib/sifive_cache.c   | 21 
 board/AndesTech/ae350/ae350.c   | 26 ++-
 configs/ae350_rv32_defconfig|  5 ++--
 configs/ae350_rv32_spl_defconfig|  5 ++--
 configs/ae350_rv32_spl_xip_defconfig|  5 ++--
 configs/ae350_rv32_xip_defconfig|  5 ++--
 configs/ae350_rv64_defconfig|  5 ++--
 configs/ae350_rv64_spl_defconfig|  5 ++--
 configs/ae350_rv64_spl_xip_defconfig|  5 ++--
 configs/ae350_rv64_xip_defconfig|  5 ++--
 drivers/cache/Kconfig   |  7 ++
 drivers/cache/Makefile  |  1 +
 drivers/cache/cache-sifive-pl2.c| 44 +
 17 files changed, 165 insertions(+), 39 deletions(-)
 create mode 100644 drivers/cache/cache-sifive-pl2.c

Best regards,
Leo

Re: I'm looking for the source code of a specific u-boot version.

[Tony Dinh]

Hi Mario and Heinrich,

On Wed, Dec 27, 2023 at 12:23 PM Mario Marietto  wrote:
>
> Hello.
>
> I'm trying to boot FreeBSD for arm32 bit as DomU on my ARM Chromebook
> SNOW with xen. Basically there are two ways to accomplish this task :
>
>
> 1) to write a patch that allows the FreeBSD kernel to boot as a zImage
> file. This could be accomplished applying this patch to a specific
> file that's on the source code of FreeBSD :
>
>
> https://xenbits.xen.org/gitweb/?p=p...8;hb=0782e25d98cc1391472717035f986c979edef0c9
>
>
>
> This patch was written by Julien Grall a lot of time ago and now it
> does not work anymore. This is the reason explain by the xen
> developers :
>
>
>
> It appears FreeBSD-CURRENT removed the last step converting the kernel
> file to kernel.bin.The patch can be readily rebased, but without
> kernel.bin that doesn't do too much.
>
>
>
> So,without a rebase of that patch the first option is not applicable.
> And I'm not able to fix it.
>
>
> 2) booting FreeBSD using U-Boot,as explained to me by a xen developer :
>
>
> I was trying to explain why and how Julien's patch works so that you
> could be the one to re-do something similar or fix the patch on the
> FreeBSD kernel that you are working with. I am happy to help review
> and write patches but I don't work with the FreeBSD kernel so I
> wouldn't be able to help you quickly. However, I might have a
> suggestion. Do you know if FreeBSD can be booted by U-Boot ?
> Because
> U-Boot definitely boots as Xen on ARM guest firmware/bootloader. You
> should be able to build U-Boot and use the U-Boot binary as Xen guest
> kernel, then U-Boot could load FreeBSD from disk or network and start
> it. For instance as domU config file:
>
> kernel="/home/petalinux/u-boot.bin"
> disk = [ '/home/petalinux/test.img,raw,xvda' ]
>
>
> Actually I'm working on the idea n. 2. Basically I need to find the
> proper u-boot file that's able to boot the image of FreeBSD that I
> have installed (13.2 for arm32 bit). Maybe I found it here :
>
>
> http://commondatastorage.googleapis.com/chromeos-localmirror/distfiles/nv_uboot-snow-simplefb.kpart.bz2
>
>
> I found that link inside this tutorial :
>
>
> https://wiki.freebsd.org/arm/Chromebook
>
>
> the version of u-boot that has been embedded in that file is the following 
> one :
>
>
> # strings nv_uboot-snow-simplefb.kpart | grep U-Boot
> U-Boot 2011.12-gc1f6280 (May 27 2013 - 15:06:59) for SMDK5250
>
>
> So the question is easy : I need to find the source code of that old
> version of u-boot,because once compiled,it will give me the proper
> u-boot.bin kernel / bootloader file that maybe will be able to boot
> FreeBSD.

Yes, it can boot a 32-bit ARM board. I'm not a FreeBSD person, but
I've helped a FreeBSD user booting a 32-bit ARM box with u-boot
(GoFlexHome Marvell Kirkwood 6281). The u-boot version was 2017.05. I
used an out-of-tree u-boot build. This u-boot executed the ubldr to
boot FreeBSD.

Please see here:
https://forum.doozan.com/read.php?3,49039,82059#msg-82059

It's been so long, I don't remember the details. Just to confirm what
Heinrich said that u-boot can indeed boot 32-bit FreeBSD, since many
years ago.

All the best,
Tony

>
>
> --
> Mario.

Re: [RFC PATCH 2/2] common: console: Add support of passing the saved console log to the OS

[Mark Kettenis]

> From: Bence Cs 
> Date: Thu, 28 Dec 2023 00:30:12 +0100
> 
> CONFIG_CONSOLE_RECORD_SAVE option allows for the recorded console log to
> be saved to a memory location, along with some metadata. This memory
> address is then passed to the booted OS via command line.

Hi,

Sorry, but I fear the concept of passing the memory location of
something via the concept is an alien concept to some OSes.  And even
on Linux there will be issues when you're booting using an EFI
bootloader.  So I think this needs some further thought.

A better approach would be to pass the address (and size) of the
buffer through an "u-boot,bootlog" property (or something like that)
in the /chosen node of the device tree.  And make sure the memory
block is present in the EFI memory map.

Cheers,

Mark

> 
> Signed-off-by: Bence Cs 
> ---
> 
> Notes:
> Some improvements to consider:
> * pass CONFIG_CONSOLE_RECORD_SAVE_BASE via FDT
> * or possibly add it to U-Boot env, so scripts can choose the passing 
> method
> * find a better place to call console_record_save(), ideally as late as 
> possible
> 
> Link: https://lists.denx.de/pipermail/u-boot/2023-December/541138.html
> 
>  boot/bootm.c  | 27 +--
>  common/Kconfig| 13 +
>  common/console.c  | 13 +
>  include/console.h | 19 +++
>  4 files changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 7a050ed41a..7f67f6a58e 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -857,6 +858,16 @@ static int process_subst(char *buf, int maxlen)
>   return ret;
>  }
>  
> +static int append_bootlog_base(char *buf, int maxlen)
> +{
> + int size = strlen(buf);
> +
> + if (size + 1 + strlen(" bootlog.base=0x" + 8 > maxlen)
> + return -ENOSPC;
> +
> + sprintf(buf + size, " bootlog.base=0x%08X", 
> CONFIG_CONSOLE_RECORD_SAVE_BASE);
> +}
> +
>  int bootm_process_cmdline(char *buf, int maxlen, int flags)
>  {
>   int ret;
> @@ -875,6 +886,11 @@ int bootm_process_cmdline(char *buf, int maxlen, int 
> flags)
>   if (ret)
>   return log_msg_ret("subst", ret);
>   }
> + if (IS_ENABLED(CONFIG_CONSOLE_RECORD_SAVE)) {
> + ret = append_bootlog_base(buf, maxlen);
> + if (ret)
> + return log_msg_ret("bootlogbase", ret);
> + }
>  
>   return 0;
>  }
> @@ -882,7 +898,7 @@ int bootm_process_cmdline(char *buf, int maxlen, int 
> flags)
>  int bootm_process_cmdline_env(int flags)
>  {
>   const int maxlen = MAX_CMDLINE_SIZE;
> - bool do_silent;
> + bool do_silent, do_pass_record;
>   const char *env;
>   char *buf;
>   int ret;
> @@ -890,7 +906,8 @@ int bootm_process_cmdline_env(int flags)
>   /* First check if any action is needed */
>   do_silent = IS_ENABLED(CONFIG_SILENT_CONSOLE) &&
>   !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY) && (flags & BOOTM_CL_SILENT);
> - if (!do_silent && !IS_ENABLED(CONFIG_BOOTARGS_SUBST))
> + do_pass_record = IS_ENABLED(CONFIG_CONSOLE_RECORD_SAVE);
> + if (!do_silent && !do_pass_record && !IS_ENABLED(CONFIG_BOOTARGS_SUBST))
>   return 0;
>  
>   env = env_get("bootargs");
> @@ -1056,6 +1073,13 @@ int bootm_run_states(struct bootm_info *bmi, int 
> states)
>   }
>  #endif
>  
> + if (ret)
> + return ret;
> +
> +#if CONFIG_IS_ENABLED(CONSOLE_RECORD_SAVE)
> + console_record_save();
> +#endif /* CONFIG_IS_ENABLED(CONSOLE_RECORD_SAVE) */
> +
>   /* From now on, we need the OS boot function */
>   if (ret)
>   return ret;
> diff --git a/common/Kconfig b/common/Kconfig
> index 0283701f1d..d70fa748c2 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -52,6 +52,19 @@ config CONSOLE_RECORD_IN_SIZE
> The buffer is allocated immediately after the malloc() region is
> ready.
>  
> +config CONSOLE_RECORD_SAVE
> + bool "Save a copy of the recording for the OS to read"
> + depends on CONSOLE_RECORD
> + help
> +   This option makes a copy of the recorded console log available at a 
> fixed
> +   RAM location, so that the booted OS can read it
> +
> +config CONSOLE_RECORD_SAVE_BASE
> + hex "Memory location to save the log to"
> + depends on CONSOLE_RECORD_SAVE
> + help
> +   The base address for the saved copy (will be passed to the booted OS)
> +
>  config SYS_CBSIZE
>   int "Console input buffer size"
>   default 2048 if ARCH_TEGRA || ARCH_VERSAL || ARCH_ZYNQ || ARCH_ZYNQMP 
> || \
> diff --git a/common/console.c b/common/console.c
> index 1ffda49c87..77a41d99f7 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -860,6 +860,19 @@ int console_in_puts(const char *str)
>  
>  #endif
>  
> +#if CONFIG_IS_ENABLED(CONSOLE_RECORD_SAVE)
> +void 

[RFC PATCH 0/2] Passing U-Boot logs to the OS

[Bence Cs]

The problem of sending boot logs to the booted OS has not been solved,
but it is high time we do it. This proposed patch aims to give a crude
solution, utilizing CONFIG_CONSOLE_RECORD to capture the logs and
saving them to RAM, along with a struct residing at
CONFIG_CONSOLE_RECORD_SAVE_BASE. There may be more sophisticated
methods than this, which may be implemented if time permits.

Please note that this patch is not the final solution, but we do
intend to finish it by the end of the MW. Also I am WFH for the
holidays, so if someone could smoke-test, I'd appreciate it.

The patch series was based on the current tip of `next` branch:
Commit: 4b151562bb bootmeth: pass size to efi_binary_run()

Previous discussion thread can be found here:
Link: https://lists.denx.de/pipermail/u-boot/2023-December/541138.html

Bence Cs (2):
  Add U_BOOT_VERSION_CODE macro for packing the version number into a
single int
  common: console: Add support of passing the saved console log to the
OS

 boot/bootm.c  | 27 +--
 common/Kconfig| 13 +
 common/console.c  | 13 +
 include/console.h | 19 +++
 include/version.h |  3 +++
 5 files changed, 73 insertions(+), 2 deletions(-)

-- 
2.25.1

[RFC PATCH 2/2] common: console: Add support of passing the saved console log to the OS

[Bence Cs]

CONFIG_CONSOLE_RECORD_SAVE option allows for the recorded console log to
be saved to a memory location, along with some metadata. This memory
address is then passed to the booted OS via command line.

Signed-off-by: Bence Cs 
---

Notes:
Some improvements to consider:
* pass CONFIG_CONSOLE_RECORD_SAVE_BASE via FDT
* or possibly add it to U-Boot env, so scripts can choose the passing method
* find a better place to call console_record_save(), ideally as late as 
possible

Link: https://lists.denx.de/pipermail/u-boot/2023-December/541138.html

 boot/bootm.c  | 27 +--
 common/Kconfig| 13 +
 common/console.c  | 13 +
 include/console.h | 19 +++
 4 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 7a050ed41a..7f67f6a58e 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -857,6 +858,16 @@ static int process_subst(char *buf, int maxlen)
return ret;
 }
 
+static int append_bootlog_base(char *buf, int maxlen)
+{
+   int size = strlen(buf);
+
+   if (size + 1 + strlen(" bootlog.base=0x" + 8 > maxlen)
+   return -ENOSPC;
+
+   sprintf(buf + size, " bootlog.base=0x%08X", 
CONFIG_CONSOLE_RECORD_SAVE_BASE);
+}
+
 int bootm_process_cmdline(char *buf, int maxlen, int flags)
 {
int ret;
@@ -875,6 +886,11 @@ int bootm_process_cmdline(char *buf, int maxlen, int flags)
if (ret)
return log_msg_ret("subst", ret);
}
+   if (IS_ENABLED(CONFIG_CONSOLE_RECORD_SAVE)) {
+   ret = append_bootlog_base(buf, maxlen);
+   if (ret)
+   return log_msg_ret("bootlogbase", ret);
+   }
 
return 0;
 }
@@ -882,7 +898,7 @@ int bootm_process_cmdline(char *buf, int maxlen, int flags)
 int bootm_process_cmdline_env(int flags)
 {
const int maxlen = MAX_CMDLINE_SIZE;
-   bool do_silent;
+   bool do_silent, do_pass_record;
const char *env;
char *buf;
int ret;
@@ -890,7 +906,8 @@ int bootm_process_cmdline_env(int flags)
/* First check if any action is needed */
do_silent = IS_ENABLED(CONFIG_SILENT_CONSOLE) &&
!IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY) && (flags & BOOTM_CL_SILENT);
-   if (!do_silent && !IS_ENABLED(CONFIG_BOOTARGS_SUBST))
+   do_pass_record = IS_ENABLED(CONFIG_CONSOLE_RECORD_SAVE);
+   if (!do_silent && !do_pass_record && !IS_ENABLED(CONFIG_BOOTARGS_SUBST))
return 0;
 
env = env_get("bootargs");
@@ -1056,6 +1073,13 @@ int bootm_run_states(struct bootm_info *bmi, int states)
}
 #endif
 
+   if (ret)
+   return ret;
+
+#if CONFIG_IS_ENABLED(CONSOLE_RECORD_SAVE)
+   console_record_save();
+#endif /* CONFIG_IS_ENABLED(CONSOLE_RECORD_SAVE) */
+
/* From now on, we need the OS boot function */
if (ret)
return ret;
diff --git a/common/Kconfig b/common/Kconfig
index 0283701f1d..d70fa748c2 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -52,6 +52,19 @@ config CONSOLE_RECORD_IN_SIZE
  The buffer is allocated immediately after the malloc() region is
  ready.
 
+config CONSOLE_RECORD_SAVE
+   bool "Save a copy of the recording for the OS to read"
+   depends on CONSOLE_RECORD
+   help
+ This option makes a copy of the recorded console log available at a 
fixed
+ RAM location, so that the booted OS can read it
+
+config CONSOLE_RECORD_SAVE_BASE
+   hex "Memory location to save the log to"
+   depends on CONSOLE_RECORD_SAVE
+   help
+ The base address for the saved copy (will be passed to the booted OS)
+
 config SYS_CBSIZE
int "Console input buffer size"
default 2048 if ARCH_TEGRA || ARCH_VERSAL || ARCH_ZYNQ || ARCH_ZYNQMP 
|| \
diff --git a/common/console.c b/common/console.c
index 1ffda49c87..77a41d99f7 100644
--- a/common/console.c
+++ b/common/console.c
@@ -860,6 +860,19 @@ int console_in_puts(const char *str)
 
 #endif
 
+#if CONFIG_IS_ENABLED(CONSOLE_RECORD_SAVE)
+void console_record_save(void)
+{
+   struct console_record_log *log = (struct console_record_log 
*)(CONFIG_CONSOLE_RECORD_SAVE_BASE);
+   char *buf = (char *)(log + 1);
+   // buffer starts at the end of the log struct
+   log->version = 1U;
+   log->uboot_version = U_BOOT_CURRENT_VERSION_CODE;
+   log->len = membuff_get((struct membuff *)>console_out, buf, 
console_record_avail());
+   log->magic = CONSOLE_RECORD_LOG_MAGIC;
+}
+#endif /* CONFIG_IS_ENABLED(CONSOLE_RECORD_SAVE) */
+
 /* test if ctrl-c was pressed */
 static int ctrlc_disabled = 0; /* see disable_ctrl() */
 static int ctrlc_was_pressed = 0;
diff --git a/include/console.h b/include/console.h
index e29817e57b..7a1b5ebffd 100644
--- a/include/console.h
+++ b/include/console.h
@@ 

[RFC PATCH 1/2] Add U_BOOT_VERSION_CODE macro for packing the version number into a single int

[Bence Cs]

Signed-off-by: Bence Cs 
---
 include/version.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/version.h b/include/version.h
index 5955b21e89..a8753deadf 100644
--- a/include/version.h
+++ b/include/version.h
@@ -11,4 +11,7 @@
 #include "generated/version_autogenerated.h"
 #endif
 
+#define U_BOOT_VERSION_CODE(num, patch) (((num) << 8) | ((patch) & 0xFF))
+#define U_BOOT_CURRENT_VERSION_CODE U_BOOT_VERSION_CODE(U_BOOT_VERSION_NUM, 
U_BOOT_VERSION_NUM_PATCH)
+
 #endif /* __VERSION_H__ */
-- 
2.25.1

[XEN] Re: I'm looking for the source code of a specific u-boot version.

[Heinrich Schuchardt]

Am 27. Dezember 2023 19:54:06 MEZ schrieb Mario Marietto 
:
>Hello.
>
>I'm trying to boot FreeBSD for arm32 bit as DomU on my ARM Chromebook
>SNOW with xen. Basically there are two ways to accomplish this task :
>
>
>1) to write a patch that allows the FreeBSD kernel to boot as a zImage
>file. This could be accomplished applying this patch to a specific
>file that's on the source code of FreeBSD :
>
>
>https://xenbits.xen.org/gitweb/?p=p...8;hb=0782e25d98cc1391472717035f986c979edef0c9
>
>
>
>This patch was written by Julien Grall a lot of time ago and now it
>does not work anymore. This is the reason explain by the xen
>developers :
>
>
>
>It appears FreeBSD-CURRENT removed the last step converting the kernel
>file to kernel.bin.The patch can be readily rebased, but without
>kernel.bin that doesn't do too much.
>
>
>
>So,without a rebase of that patch the first option is not applicable.
>And I'm not able to fix it.
>
>
>2) booting FreeBSD using U-Boot,as explained to me by a xen developer :
>
>
>I was trying to explain why and how Julien's patch works so that you
>could be the one to re-do something similar or fix the patch on the
>FreeBSD kernel that you are working with. I am happy to help review
>and write patches but I don't work with the FreeBSD kernel so I
>wouldn't be able to help you quickly. However, I might have a
>suggestion. Do you know if FreeBSD can be booted by U-Boot ? Because
>U-Boot definitely boots as Xen on ARM guest firmware/bootloader. You
>should be able to build U-Boot and use the U-Boot binary as Xen guest
>kernel, then U-Boot could load FreeBSD from disk or network and start
>it. For instance as domU config file:
>
>kernel="/home/petalinux/u-boot.bin"
>disk = [ '/home/petalinux/test.img,raw,xvda' ]
>
>
>Actually I'm working on the idea n. 2. Basically I need to find the
>proper u-boot file that's able to boot the image of FreeBSD that I
>have installed (13.2 for arm32 bit). Maybe I found it here :
>
>
>http://commondatastorage.googleapis.com/chromeos-localmirror/distfiles/nv_uboot-snow-simplefb.kpart.bz2
>
>
>I found that link inside this tutorial :
>
>
>https://wiki.freebsd.org/arm/Chromebook
>
>
>the version of u-boot that has been embedded in that file is the following one 
>:
>
>
># strings nv_uboot-snow-simplefb.kpart | grep U-Boot
>U-Boot 2011.12-gc1f6280 (May 27 2013 - 15:06:59) for SMDK5250
>
>
>So the question is easy : I need to find the source code of that old
>version of u-boot,because once compiled,it will give me the proper
>u-boot.bin kernel / bootloader file that maybe will be able to boot
>FreeBSD.
>
>

Hello Mario,

U-Boot supports booting EFI binaries since 2016. I don't know if some arm32 
bits are still missing in FreeBSD (see https://wiki.freebsd.org/UEFI). But 
looking at 
https://cgit.freebsd.org/src/plain/stand/efi/loader/Makefile.depend.arm and 
https://cgit.freebsd.org/src/plain/stand/efi/loader/arch/arm/start.S arm32 
should be  supported. I guess the wiki needs an update.

The U-Boot documentation only describes arm64 Xen support: 
https://docs.u-boot.org/en/latest/board/xen/xenguest_arm64.html .

I have CCed the U-Boot Xen maintainers hoping they can indicate the missing 
pieces for arm32.

Best regards

Heinrich

[PATCH v3 2/2] sunxi: restore modified memory

[Andrey Skvortsov]

Current sunxi DRAM initialisation code does several test accesses to the
DRAM array to detect aliasing effects and so determine the correct
row/column configuration. This changes the DRAM content, which breaks
use cases like soft reset and Linux's ramoops mechanism.

Fix this problem by saving and restoring the content of the DRAM cells
that is used for the test writes.

Signed-off-by: Andrey Skvortsov 
---
 arch/arm/mach-sunxi/dram_helpers.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-sunxi/dram_helpers.c 
b/arch/arm/mach-sunxi/dram_helpers.c
index 661186b648..e487f87bf3 100644
--- a/arch/arm/mach-sunxi/dram_helpers.c
+++ b/arch/arm/mach-sunxi/dram_helpers.c
@@ -32,13 +32,25 @@ void mctl_await_completion(u32 *reg, u32 mask, u32 val)
 #ifndef CONFIG_MACH_SUNIV
 bool mctl_mem_matches_base(u32 offset, ulong base)
 {
+   u32 val_base;
+   u32 val_offset;
+   bool ret;
+
+   /* Save original values */
+   val_base = readl(base);
+   val_offset = readl(base + offset);
+
/* Try to write different values to RAM at two addresses */
writel(0, base);
writel(0xaa55aa55, base + offset);
dsb();
/* Check if the same value is actually observed when reading back */
-   return readl(base) ==
-  readl(base + offset);
+   ret = readl(base) == readl(base + offset);
+
+   /* Restore original values */
+   writel(val_base, base);
+   writel(val_offset, base + offset);
+   return ret;
 }
 
 /*
-- 
2.42.0

[PATCH v3 1/2] sunxi: reorganize mctl_mem_matches_* functions

[Andrey Skvortsov]

mctl_mem_matches and mctl_mem_matches_base identical functions. To
avoid code duplication move them to dram_helpers and make
mctl_mem_matches use generic mctl_mem_matches_base.

Signed-off-by: Andrey Skvortsov 
---
 arch/arm/include/asm/arch-sunxi/dram.h |  1 +
 arch/arm/mach-sunxi/dram_helpers.c | 20 ++--
 arch/arm/mach-sunxi/dram_sunxi_dw.c| 13 -
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/arch/arm/include/asm/arch-sunxi/dram.h 
b/arch/arm/include/asm/arch-sunxi/dram.h
index 682daae6b1..9d21b49241 100644
--- a/arch/arm/include/asm/arch-sunxi/dram.h
+++ b/arch/arm/include/asm/arch-sunxi/dram.h
@@ -40,5 +40,6 @@
 unsigned long sunxi_dram_init(void);
 void mctl_await_completion(u32 *reg, u32 mask, u32 val);
 bool mctl_mem_matches(u32 offset);
+bool mctl_mem_matches_base(u32 offset, ulong base);
 
 #endif /* _SUNXI_DRAM_H */
diff --git a/arch/arm/mach-sunxi/dram_helpers.c 
b/arch/arm/mach-sunxi/dram_helpers.c
index cdf2750f1c..661186b648 100644
--- a/arch/arm/mach-sunxi/dram_helpers.c
+++ b/arch/arm/mach-sunxi/dram_helpers.c
@@ -25,19 +25,27 @@ void mctl_await_completion(u32 *reg, u32 mask, u32 val)
 }
 
 /*
- * Test if memory at offset offset matches memory at begin of DRAM
+ * Test if memory at offset matches memory at a certain base
  *
  * Note: dsb() is not available on ARMv5 in Thumb mode
  */
 #ifndef CONFIG_MACH_SUNIV
-bool mctl_mem_matches(u32 offset)
+bool mctl_mem_matches_base(u32 offset, ulong base)
 {
/* Try to write different values to RAM at two addresses */
-   writel(0, CFG_SYS_SDRAM_BASE);
-   writel(0xaa55aa55, (ulong)CFG_SYS_SDRAM_BASE + offset);
+   writel(0, base);
+   writel(0xaa55aa55, base + offset);
dsb();
/* Check if the same value is actually observed when reading back */
-   return readl(CFG_SYS_SDRAM_BASE) ==
-  readl((ulong)CFG_SYS_SDRAM_BASE + offset);
+   return readl(base) ==
+  readl(base + offset);
+}
+
+/*
+ * Test if memory at offset matches memory at begin of DRAM
+ */
+bool mctl_mem_matches(u32 offset)
+{
+   return mctl_mem_matches_base(offset, CFG_SYS_SDRAM_BASE);
 }
 #endif
diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c 
b/arch/arm/mach-sunxi/dram_sunxi_dw.c
index 9382d3d0be..2e8dd40b97 100644
--- a/arch/arm/mach-sunxi/dram_sunxi_dw.c
+++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c
@@ -652,19 +652,6 @@ static int mctl_channel_init(uint16_t socid, struct 
dram_para *para)
return 0;
 }
 
-/*
- * Test if memory at offset offset matches memory at a certain base
- */
-static bool mctl_mem_matches_base(u32 offset, ulong base)
-{
-   /* Try to write different values to RAM at two addresses */
-   writel(0, base);
-   writel(0xaa55aa55, base + offset);
-   dsb();
-   /* Check if the same value is actually observed when reading back */
-   return readl(base) ==
-  readl(base + offset);
-}
 
 static void mctl_auto_detect_dram_size_rank(uint16_t socid, struct dram_para 
*para, ulong base, struct rank_para *rank)
 {
-- 
2.42.0

[PATCH v3 0/2] sunxi: restore modified memory

[Andrey Skvortsov]

Changes in v3:
 - reorder patches
 - remove casts
 - change commit message for 'sunxi: restore modified memory' patch

Changes in v2:
 - rename temporary variables
 - fix types for temporary variables

Andrey Skvortsov (2):
  sunxi: reorganize mctl_mem_matches_* functions
  sunxi: restore modified memory

 arch/arm/include/asm/arch-sunxi/dram.h |  1 +
 arch/arm/mach-sunxi/dram_helpers.c | 32 +-
 arch/arm/mach-sunxi/dram_sunxi_dw.c| 13 ---
 3 files changed, 27 insertions(+), 19 deletions(-)

-- 
2.42.0

[PATCH v4 12/12] bloblist: Update documentation and header comment

[Raymond Mao]

From: Simon Glass 

Align the documentation with the v0.9 spec.

Signed-off-by: Simon Glass 
Signed-off-by: Raymond Mao 
Reviewed-by: Ilias Apalodimas 
---
 doc/develop/bloblist.rst | 4 +++-
 include/bloblist.h   | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/doc/develop/bloblist.rst b/doc/develop/bloblist.rst
index 81643c7674..28431039ad 100644
--- a/doc/develop/bloblist.rst
+++ b/doc/develop/bloblist.rst
@@ -14,6 +14,8 @@ structure defined by the code that owns it.
 For the design goals of bloblist, please see the comments at the top of the
 `bloblist.h` header file.
 
+Bloblist is an implementation with the `Firmware Handoff`_ protocol.
+
 Passing state through the boot process
 --
 
@@ -99,7 +101,7 @@ API documentation
 -
 
 .. kernel-doc:: include/bloblist.h
-
+.. _`Firmware Handoff`: https://github.com/FirmwareHandoff/firmware_handoff
 
 Simon Glass
 s...@chromium.org
diff --git a/include/bloblist.h b/include/bloblist.h
index 145e5c0242..84fc943819 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -66,6 +66,7 @@
  *
  * Copyright 2018 Google, Inc
  * Written by Simon Glass 
+ * Adjusted July 2023 to match Firmware handoff specification, Release 0.9
  */
 
 #ifndef __BLOBLIST_H
-- 
2.25.1

[PATCH v4 11/12] bloblist: Add alignment to bloblist_new()

[Raymond Mao]

From: Simon Glass 

Allow the alignment to be specified when creating a bloblist.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
Reviewed-by: Ilias Apalodimas 
---
Changes in v3
- Keep the flag argument to align to FW handoff spec up to commit 3592349.
Changes in v4
- Minor fix to use a full ternary operator in function bloblist_new().

 common/bloblist.c  |  5 +++--
 include/bloblist.h |  3 ++-
 test/bloblist.c| 40 ++--
 3 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 6e019087ff..2d373910b6 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -351,7 +351,7 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
return chksum;
 }
 
-int bloblist_new(ulong addr, uint size, uint flags)
+int bloblist_new(ulong addr, uint size, uint flags, uint align_log2)
 {
struct bloblist_hdr *hdr;
 
@@ -367,6 +367,7 @@ int bloblist_new(ulong addr, uint size, uint flags)
hdr->magic = BLOBLIST_MAGIC;
hdr->used_size = hdr->hdr_size;
hdr->total_size = size;
+   hdr->align_log2 = align_log2 ? align_log2 : BLOBLIST_BLOB_ALIGN_LOG2;
hdr->chksum = 0;
gd->bloblist = hdr;
 
@@ -522,7 +523,7 @@ int bloblist_init(void)
}
log_debug("Creating new bloblist size %lx at %lx\n", size,
  addr);
-   ret = bloblist_new(addr, size, 0);
+   ret = bloblist_new(addr, size, 0, 0);
} else {
log_debug("Found existing bloblist size %lx at %lx\n", size,
  addr);
diff --git a/include/bloblist.h b/include/bloblist.h
index 4ec4b3d449..145e5c0242 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -330,10 +330,11 @@ int bloblist_resize(uint tag, int new_size);
  * @addr: Address of bloblist
  * @size: Initial size for bloblist
  * @flags: Flags to use for bloblist
+ * @align_log2: Log base 2 of maximum alignment provided by this bloblist
  * Return: 0 if OK, -EFAULT if addr is not aligned correctly, -ENOSPC is the
  * area is not large enough
  */
-int bloblist_new(ulong addr, uint size, uint flags);
+int bloblist_new(ulong addr, uint size, uint flags, uint align_log2);
 
 /**
  * bloblist_check() - Check if a bloblist exists
diff --git a/test/bloblist.c b/test/bloblist.c
index 2b06ce844f..17d9dd03d0 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -72,15 +72,15 @@ static int bloblist_test_init(struct unit_test_state *uts)
hdr = clear_bloblist();
ut_asserteq(-ENOENT, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
ut_asserteq_ptr(NULL, bloblist_check_magic(TEST_ADDR));
-   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0));
ut_asserteq_ptr(hdr, bloblist_check_magic(TEST_ADDR));
hdr->version++;
ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR,
 TEST_BLOBLIST_SIZE));
 
-   ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc, 0));
-   ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0));
-   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+   ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc, 0, 0));
+   ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0, 0));
+   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0));
 
ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
ut_assertok(bloblist_finish());
@@ -106,7 +106,7 @@ static int bloblist_test_blob(struct unit_test_state *uts)
/* At the start there should be no records */
hdr = clear_bloblist();
ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE));
-   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0));
ut_asserteq(sizeof(struct bloblist_hdr), bloblist_get_size());
ut_asserteq(TEST_BLOBLIST_SIZE, bloblist_get_total_size());
ut_asserteq(TEST_ADDR, bloblist_get_base());
@@ -145,7 +145,7 @@ static int bloblist_test_blob_ensure(struct unit_test_state 
*uts)
 
/* At the start there should be no records */
clear_bloblist();
-   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0));
 
/* Test with an empty bloblist */
size = TEST_SIZE;
@@ -177,7 +177,7 @@ static int bloblist_test_bad_blob(struct unit_test_state 
*uts)
void *data;
 
hdr = clear_bloblist();
-   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0));
data = hdr + 1;
data += sizeof(struct bloblist_rec);
ut_asserteq_addr(data, bloblist_ensure(TEST_TAG, TEST_SIZE));

[PATCH v4 10/12] bloblist: Adjust the bloblist header

[Raymond Mao]

From: Simon Glass 

The v0.9 spec provides for a 24-byte header. Update the implementation
to match this.
Rename the fields of the bloblist header to align to the spec.
Adds an alignment field into the bloblist header.
Update the related bloblist APIs and UT testcases.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
---
Changes in v3
- Update the bloblist header to align to FW handoff spec up to commit 3592349.
- Update the related testcases.
Changes in v4
- Patch #14 from v3 is squashed into this patch.

 common/bloblist.c  | 86 +++---
 include/bloblist.h | 44 ++--
 test/bloblist.c| 37 ++--
 3 files changed, 95 insertions(+), 72 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 1c97d61e4a..6e019087ff 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -80,7 +80,7 @@ const char *bloblist_tag_name(enum bloblist_tag_t tag)
 
 static struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr)
 {
-   if (hdr->alloced <= hdr->hdr_size)
+   if (hdr->used_size <= hdr->hdr_size)
return NULL;
return (struct bloblist_rec *)((void *)hdr + hdr->hdr_size);
 }
@@ -119,7 +119,7 @@ static struct bloblist_rec *bloblist_next_blob(struct 
bloblist_hdr *hdr,
 {
ulong offset = bloblist_blob_end_ofs(hdr, rec);
 
-   if (offset >= hdr->alloced)
+   if (offset >= hdr->used_size)
return NULL;
return (struct bloblist_rec *)((void *)hdr + offset);
 }
@@ -156,9 +156,9 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
align_log2 = BLOBLIST_BLOB_ALIGN_LOG2;
 
/* Figure out where the new data will start */
-   data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec);
+   data_start = map_to_sysmem(hdr) + hdr->used_size + sizeof(*rec);
 
-   /* Align the address and then calculate the offset from ->alloced */
+   /* Align the address and then calculate the offset from used size */
aligned_start = ALIGN(data_start, 1U << align_log2) - data_start;
 
/* If we need to create a dummy record, create it */
@@ -172,19 +172,20 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
return log_msg_ret("void", ret);
 
/* start the record after that */
-   data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*vrec);
+   data_start = map_to_sysmem(hdr) + hdr->used_size + 
sizeof(*vrec);
}
 
/* Calculate the new allocated total */
new_alloced = data_start - map_to_sysmem(hdr) +
ALIGN(size, 1U << align_log2);
 
-   if (new_alloced > hdr->size) {
-   log_err("Failed to allocate %x bytes size=%x, need size=%x\n",
-   size, hdr->size, new_alloced);
+   if (new_alloced > hdr->total_size) {
+   log_err("Failed to allocate %x bytes\n", size);
+   log_err("Used size=%x, total size=%x\n",
+   hdr->used_size, hdr->total_size);
return log_msg_ret("bloblist add", -ENOSPC);
}
-   rec = (void *)hdr + hdr->alloced;
+   rec = (void *)hdr + hdr->used_size;
 
rec->tag_and_hdr_size = tag | sizeof(*rec) << BLOBLISTR_HDR_SIZE_SHIFT;
rec->size = size;
@@ -192,7 +193,7 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
/* Zero the record data */
memset((void *)rec + rec_hdr_size(rec), '\0', rec->size);
 
-   hdr->alloced = new_alloced;
+   hdr->used_size = new_alloced;
*recp = rec;
 
return 0;
@@ -287,29 +288,30 @@ static int bloblist_resize_rec(struct bloblist_hdr *hdr,
   int new_size)
 {
int expand_by;  /* Number of bytes to expand by (-ve to contract) */
-   int new_alloced;/* New value for @hdr->alloced */
+   int new_alloced;
ulong next_ofs; /* Offset of the record after @rec */
 
expand_by = ALIGN(new_size - rec->size, BLOBLIST_BLOB_ALIGN);
-   new_alloced = ALIGN(hdr->alloced + expand_by, BLOBLIST_BLOB_ALIGN);
+   new_alloced = ALIGN(hdr->used_size + expand_by, BLOBLIST_BLOB_ALIGN);
if (new_size < 0) {
log_debug("Attempt to shrink blob size below 0 (%x)\n",
  new_size);
return log_msg_ret("size", -EINVAL);
}
-   if (new_alloced > hdr->size) {
-   log_err("Failed to allocate %x bytes size=%x, need size=%x\n",
-   new_size, hdr->size, new_alloced);
+   if (new_alloced > hdr->total_size) {
+   log_err("Failed to allocate %x bytes\n", new_size);
+   log_err("Used size=%x, total size=%x\n",
+   hdr->used_size, hdr->total_size);
return log_msg_ret("alloc", -ENOSPC);
}
 
/* Move the following blobs up or down, if 

[PATCH v4 09/12] bloblist: Reduce blob-header size

[Raymond Mao]

From: Simon Glass 

The v0.9 spec provides for an 8-byte header for each blob, with fewer
fields.
The blob data start address should be aligned to the alignment specified
by the bloblist header.
Update the implementation to match this.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
---
Changes in v2
- Update the blob start address to align to the alignment required by
  the bloblist header.
- Define the macros of bloblist header size and bloblist record header
  size as the size of their structures.  
Changes in v3
- Update the calculation of the bloblist record offset to make sure
  that each bloblist record data section start address fulfills the
  alignment requirement.
- Update commit message.

 common/bloblist.c  | 23 +++
 include/bloblist.h | 33 ++---
 test/bloblist.c| 16 
 3 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 73dbbc01c0..1c97d61e4a 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -87,12 +87,14 @@ static struct bloblist_rec *bloblist_first_blob(struct 
bloblist_hdr *hdr)
 
 static inline uint rec_hdr_size(struct bloblist_rec *rec)
 {
-   return rec->hdr_size;
+   return (rec->tag_and_hdr_size & BLOBLISTR_HDR_SIZE_MASK) >>
+   BLOBLISTR_HDR_SIZE_SHIFT;
 }
 
 static inline uint rec_tag(struct bloblist_rec *rec)
 {
-   return rec->tag;
+   return (rec->tag_and_hdr_size & BLOBLISTR_TAG_MASK) >>
+   BLOBLISTR_TAG_SHIFT;
 }
 
 static ulong bloblist_blob_end_ofs(struct bloblist_hdr *hdr,
@@ -101,7 +103,13 @@ static ulong bloblist_blob_end_ofs(struct bloblist_hdr 
*hdr,
ulong offset;
 
offset = (void *)rec - (void *)hdr;
-   offset += rec_hdr_size(rec) + ALIGN(rec->size, BLOBLIST_ALIGN);
+   /*
+* The data section of next TE should start from an address aligned
+* to 1 << hdr->align_log2.
+*/
+   offset += rec_hdr_size(rec) + rec->size;
+   offset = round_up(offset + rec_hdr_size(rec), 1 << hdr->align_log2);
+   offset -= rec_hdr_size(rec);
 
return offset;
 }
@@ -145,7 +153,7 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
int data_start, aligned_start, new_alloced;
 
if (!align_log2)
-   align_log2 = BLOBLIST_ALIGN_LOG2;
+   align_log2 = BLOBLIST_BLOB_ALIGN_LOG2;
 
/* Figure out where the new data will start */
data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec);
@@ -178,8 +186,7 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
}
rec = (void *)hdr + hdr->alloced;
 
-   rec->tag = tag;
-   rec->hdr_size = sizeof(struct bloblist_rec);
+   rec->tag_and_hdr_size = tag | sizeof(*rec) << BLOBLISTR_HDR_SIZE_SHIFT;
rec->size = size;
 
/* Zero the record data */
@@ -283,8 +290,8 @@ static int bloblist_resize_rec(struct bloblist_hdr *hdr,
int new_alloced;/* New value for @hdr->alloced */
ulong next_ofs; /* Offset of the record after @rec */
 
-   expand_by = ALIGN(new_size - rec->size, BLOBLIST_ALIGN);
-   new_alloced = ALIGN(hdr->alloced + expand_by, BLOBLIST_ALIGN);
+   expand_by = ALIGN(new_size - rec->size, BLOBLIST_BLOB_ALIGN);
+   new_alloced = ALIGN(hdr->alloced + expand_by, BLOBLIST_BLOB_ALIGN);
if (new_size < 0) {
log_debug("Attempt to shrink blob size below 0 (%x)\n",
  new_size);
diff --git a/include/bloblist.h b/include/bloblist.h
index d2dcad69a1..7024d7bf9e 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -24,11 +24,11 @@
  * which would add to code size. For Thumb-2 the code size needed in SPL is
  * approximately 940 bytes (e.g. for chromebook_bob).
  *
- * 5. Bloblist uses 16-byte alignment internally and is designed to start on a
- * 16-byte boundary. Its headers are multiples of 16 bytes. This makes it 
easier
- * to deal with data structures which need this level of alignment, such as 
ACPI
- * tables. For use in SPL and TPL the alignment can be relaxed, since it can be
- * relocated to an aligned address in U-Boot proper.
+ * 5. Bloblist uses 8-byte alignment internally and is designed to start on a
+ * 8-byte boundary. Its headers are 8 bytes long. It is possible to achieve
+ * larger alignment (e.g. 16 bytes) by adding a dummy header, For use in SPL 
and
+ * TPL the alignment can be relaxed, since it can be relocated to an aligned
+ * address in U-Boot proper.
  *
  * 6. Bloblist is designed to be passed to Linux as reserved memory. While 
linux
  * doesn't understand the bloblist header, it can be passed the indivdual 
blobs.
@@ -77,6 +77,9 @@ enum {
BLOBLIST_VERSION= 1,
BLOBLIST_MAGIC  = 0x4a0fb10b,
 
+   BLOBLIST_BLOB_ALIGN_LOG2 = 3,
+   BLOBLIST_BLOB_ALIGN  = 1 << BLOBLIST_BLOB_ALIGN_LOG2,
+

[PATCH v4 08/12] bloblist: Handle alignment with a void entry

[Raymond Mao]

From: Simon Glass 

Rather than setting the alignment using the header size, add an entirely
new entry to cover the gap left by the alignment.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
Reviewed-by: Simon Glass 
---
 common/bloblist.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 705d9c6ae9..73dbbc01c0 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -142,7 +142,7 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
 {
struct bloblist_hdr *hdr = gd->bloblist;
struct bloblist_rec *rec;
-   int data_start, new_alloced;
+   int data_start, aligned_start, new_alloced;
 
if (!align_log2)
align_log2 = BLOBLIST_ALIGN_LOG2;
@@ -151,10 +151,25 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec);
 
/* Align the address and then calculate the offset from ->alloced */
-   data_start = ALIGN(data_start, 1U << align_log2) - map_to_sysmem(hdr);
+   aligned_start = ALIGN(data_start, 1U << align_log2) - data_start;
+
+   /* If we need to create a dummy record, create it */
+   if (aligned_start) {
+   int void_size = aligned_start - sizeof(*rec);
+   struct bloblist_rec *vrec;
+   int ret;
+
+   ret = bloblist_addrec(BLOBLISTT_VOID, void_size, 0, );
+   if (ret)
+   return log_msg_ret("void", ret);
+
+   /* start the record after that */
+   data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*vrec);
+   }
 
/* Calculate the new allocated total */
-   new_alloced = data_start + ALIGN(size, 1U << align_log2);
+   new_alloced = data_start - map_to_sysmem(hdr) +
+   ALIGN(size, 1U << align_log2);
 
if (new_alloced > hdr->size) {
log_err("Failed to allocate %x bytes size=%x, need size=%x\n",
@@ -164,7 +179,7 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
rec = (void *)hdr + hdr->alloced;
 
rec->tag = tag;
-   rec->hdr_size = data_start - hdr->alloced;
+   rec->hdr_size = sizeof(struct bloblist_rec);
rec->size = size;
 
/* Zero the record data */
-- 
2.25.1

[PATCH v4 07/12] bloblist: Checksum the entire bloblist

[Raymond Mao]

From: Simon Glass 

Use a sinple 8-bit checksum for bloblist, as specified by the spec
version 0.9.
Spec v0.9 specifies that the entire bloblist area is checksummed,
including unused portions. Update the code to follow this.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
---
Changes in v4
- Patch #7 and #8 from v3 are squashed into this patch.

 common/bloblist.c  | 13 -
 include/bloblist.h |  5 ++---
 test/bloblist.c| 10 --
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 88e2a0f5c0..705d9c6ae9 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -318,16 +319,10 @@ int bloblist_resize(uint tag, int new_size)
 
 static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
 {
-   struct bloblist_rec *rec;
-   u32 chksum;
+   u8 chksum;
 
-   chksum = crc32(0, (unsigned char *)hdr,
-  offsetof(struct bloblist_hdr, chksum));
-   foreach_rec(rec, hdr) {
-   chksum = crc32(chksum, (void *)rec, rec_hdr_size(rec));
-   chksum = crc32(chksum, (void *)rec + rec_hdr_size(rec),
-  rec->size);
-   }
+   chksum = table_compute_checksum(hdr, hdr->alloced);
+   chksum += hdr->chksum;
 
return chksum;
 }
diff --git a/include/bloblist.h b/include/bloblist.h
index 68f97395b7..d2dcad69a1 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -174,11 +174,10 @@ enum bloblist_tag_t {
  * sizeof(bloblist_hdr) since we need at least that much space to store a
  * valid bloblist
  * @spare: Spare space (for future use)
- * @chksum: CRC32 for the entire bloblist allocated area. Since any of the
+ * @chksum: checksum for the entire bloblist allocated area. Since any of the
  * blobs can be altered after being created, this checksum is only valid
  * when the bloblist is finalised before jumping to the next stage of boot.
- * Note that chksum is last to make it easier to exclude it from the
- * checksum calculation.
+ * This is the value needed to make all checksummed bytes sum to 0
  */
 struct bloblist_hdr {
u32 magic;
diff --git a/test/bloblist.c b/test/bloblist.c
index 8b435e27ca..49ac4b92ae 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -237,12 +237,18 @@ static int bloblist_test_checksum(struct unit_test_state 
*uts)
*data2 -= 1;
 
/*
-* Changing data outside the range of valid data should not affect
-* the checksum.
+* Changing data outside the range of valid data should affect the
+* checksum.
 */
ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
data[TEST_SIZE]++;
+   ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+   data[TEST_SIZE]--;
+   ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+
data2[TEST_SIZE2]++;
+   ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+   data[TEST_SIZE]--;
ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 
return 0;
-- 
2.25.1

[PATCH v4 06/12] bloblist: Drop spare value from bloblist record

[Raymond Mao]

From: Simon Glass 

Drop spare value from bloblist record header.

For now it is still present in the header, with an underscore, so that
tests continue to pass.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
---
Changes in v3
- Keep the spare value in the bloblist header to align to FW handoff spec up to 
commit 3592349.

 common/bloblist.c  | 1 -
 include/bloblist.h | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 168993e0a7..88e2a0f5c0 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -165,7 +165,6 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
rec->tag = tag;
rec->hdr_size = data_start - hdr->alloced;
rec->size = size;
-   rec->spare = 0;
 
/* Zero the record data */
memset((void *)rec + rec_hdr_size(rec), '\0', rec->size);
diff --git a/include/bloblist.h b/include/bloblist.h
index 7eff709ec8..68f97395b7 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -205,13 +205,12 @@ struct bloblist_hdr {
  * record's data starts at this offset from the start of the record
  * @size: Size of record in bytes, excluding the header size. This does not
  * need to be aligned (e.g. 3 is OK).
- * @spare: Spare space for other things
  */
 struct bloblist_rec {
u32 tag;
u32 hdr_size;
u32 size;
-   u32 spare;
+   u32 _spare;
 };
 
 /**
-- 
2.25.1

[PATCH v4 05/12] bloblist: Access record hdr_size and tag via a function

[Raymond Mao]

From: Simon Glass 

Convert accesses to tag and hdr_size via function for grouping tag and
hdr_size together later.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
---
Changes in v3
- Update commit message.

 common/bloblist.c | 38 +-
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 1e78a3d4b3..168993e0a7 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -84,13 +84,23 @@ static struct bloblist_rec *bloblist_first_blob(struct 
bloblist_hdr *hdr)
return (struct bloblist_rec *)((void *)hdr + hdr->hdr_size);
 }
 
+static inline uint rec_hdr_size(struct bloblist_rec *rec)
+{
+   return rec->hdr_size;
+}
+
+static inline uint rec_tag(struct bloblist_rec *rec)
+{
+   return rec->tag;
+}
+
 static ulong bloblist_blob_end_ofs(struct bloblist_hdr *hdr,
   struct bloblist_rec *rec)
 {
ulong offset;
 
offset = (void *)rec - (void *)hdr;
-   offset += rec->hdr_size + ALIGN(rec->size, BLOBLIST_ALIGN);
+   offset += rec_hdr_size(rec) + ALIGN(rec->size, BLOBLIST_ALIGN);
 
return offset;
 }
@@ -119,7 +129,7 @@ static struct bloblist_rec *bloblist_findrec(uint tag)
return NULL;
 
foreach_rec(rec, hdr) {
-   if (rec->tag == tag)
+   if (rec_tag(rec) == tag)
return rec;
}
 
@@ -158,7 +168,7 @@ static int bloblist_addrec(uint tag, int size, int 
align_log2,
rec->spare = 0;
 
/* Zero the record data */
-   memset((void *)rec + rec->hdr_size, '\0', rec->size);
+   memset((void *)rec + rec_hdr_size(rec), '\0', rec->size);
 
hdr->alloced = new_alloced;
*recp = rec;
@@ -199,7 +209,7 @@ void *bloblist_find(uint tag, int size)
if (size && size != rec->size)
return NULL;
 
-   return (void *)rec + rec->hdr_size;
+   return (void *)rec + rec_hdr_size(rec);
 }
 
 void *bloblist_add(uint tag, int size, int align_log2)
@@ -209,7 +219,7 @@ void *bloblist_add(uint tag, int size, int align_log2)
if (bloblist_addrec(tag, size, align_log2, ))
return NULL;
 
-   return (void *)rec + rec->hdr_size;
+   return (void *)rec + rec_hdr_size(rec);
 }
 
 int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp)
@@ -220,7 +230,7 @@ int bloblist_ensure_size(uint tag, int size, int 
align_log2, void **blobp)
ret = bloblist_ensurerec(tag, , size, align_log2);
if (ret)
return ret;
-   *blobp = (void *)rec + rec->hdr_size;
+   *blobp = (void *)rec + rec_hdr_size(rec);
 
return 0;
 }
@@ -232,7 +242,7 @@ void *bloblist_ensure(uint tag, int size)
if (bloblist_ensurerec(tag, , size, 0))
return NULL;
 
-   return (void *)rec + rec->hdr_size;
+   return (void *)rec + rec_hdr_size(rec);
 }
 
 int bloblist_ensure_size_ret(uint tag, int *sizep, void **blobp)
@@ -245,7 +255,7 @@ int bloblist_ensure_size_ret(uint tag, int *sizep, void 
**blobp)
*sizep = rec->size;
else if (ret)
return ret;
-   *blobp = (void *)rec + rec->hdr_size;
+   *blobp = (void *)rec + rec_hdr_size(rec);
 
return 0;
 }
@@ -281,7 +291,7 @@ static int bloblist_resize_rec(struct bloblist_hdr *hdr,
 
/* Zero the new part of the blob */
if (expand_by > 0) {
-   memset((void *)rec + rec->hdr_size + rec->size, '\0',
+   memset((void *)rec + rec_hdr_size(rec) + rec->size, '\0',
   new_size - rec->size);
}
 
@@ -315,8 +325,9 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
chksum = crc32(0, (unsigned char *)hdr,
   offsetof(struct bloblist_hdr, chksum));
foreach_rec(rec, hdr) {
-   chksum = crc32(chksum, (void *)rec, rec->hdr_size);
-   chksum = crc32(chksum, (void *)rec + rec->hdr_size, rec->size);
+   chksum = crc32(chksum, (void *)rec, rec_hdr_size(rec));
+   chksum = crc32(chksum, (void *)rec + rec_hdr_size(rec),
+  rec->size);
}
 
return chksum;
@@ -424,8 +435,9 @@ void bloblist_show_list(void)
for (rec = bloblist_first_blob(hdr); rec;
 rec = bloblist_next_blob(hdr, rec)) {
printf("%08lx  %8x  %4x %s\n",
-  (ulong)map_to_sysmem((void *)rec + rec->hdr_size),
-  rec->size, rec->tag, bloblist_tag_name(rec->tag));
+  (ulong)map_to_sysmem((void *)rec + rec_hdr_size(rec)),
+  rec->size, rec_tag(rec),
+  bloblist_tag_name(rec_tag(rec)));
}
 }
 
-- 
2.25.1

[PATCH v4 04/12] bloblist: Set version to 1

[Raymond Mao]

From: Simon Glass 

The new bloblist for v0.9 has version 1 so update this value.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
Reviewed-by: Ilias Apalodimas 
Reviewed-by: Simon Glass 
---
 include/bloblist.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/bloblist.h b/include/bloblist.h
index 72c785411d..7eff709ec8 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -74,7 +74,7 @@
 #include 
 
 enum {
-   BLOBLIST_VERSION= 0,
+   BLOBLIST_VERSION= 1,
BLOBLIST_MAGIC  = 0x4a0fb10b,
 
BLOBLIST_ALIGN_LOG2 = 3,
-- 
2.25.1

[PATCH v4 03/12] bloblist: Change the magic value

[Raymond Mao]

From: Simon Glass 

This uses a new value with spec v0.9 so change it.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
Reviewed-by: Ilias Apalodimas 
Reviewed-by: Simon Glass 
---
Changes in v2
- Update the bloblist magic to align to FW handoff spec v0.9.
Changes in v3
- Update the bloblist magic to align to FW handoff spec up to commit 3592349.

 include/bloblist.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/bloblist.h b/include/bloblist.h
index 5ad1337d1f..72c785411d 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -75,7 +75,7 @@
 
 enum {
BLOBLIST_VERSION= 0,
-   BLOBLIST_MAGIC  = 0xb00757a3,
+   BLOBLIST_MAGIC  = 0x4a0fb10b,
 
BLOBLIST_ALIGN_LOG2 = 3,
BLOBLIST_ALIGN  = 1 << BLOBLIST_ALIGN_LOG2,
-- 
2.25.1

[PATCH v4 02/12] bloblist: Adjust API to align in powers of 2

[Raymond Mao]

From: Simon Glass 

The updated bloblist structure stores the alignment as a power-of-two
value in its structures. Adjust the API to use this, to avoid needing to
calling ilog2().
Update the bloblist alignment from 16 bytes to 8 bytes.
Drop a stale comment while we are here.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
Reviewed-by: Simon Glass 
---
Changes in v2
- Update the bloblist alignment to align to FW handoff spec v0.9.
Changes in v4
- Update the commit message.

 arch/x86/lib/tables.c |  3 ++-
 common/bloblist.c | 24 +++-
 include/bloblist.h| 12 +++-
 test/bloblist.c   |  4 ++--
 4 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
index 5b5070f7ca..d43e77d373 100644
--- a/arch/x86/lib/tables.c
+++ b/arch/x86/lib/tables.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -104,7 +105,7 @@ int write_tables(void)
if (!gd->arch.table_end)
gd->arch.table_end = rom_addr;
rom_addr = (ulong)bloblist_add(table->tag, size,
-  table->align);
+  ilog2(table->align));
if (!rom_addr)
return log_msg_ret("bloblist", -ENOBUFS);
 
diff --git a/common/bloblist.c b/common/bloblist.c
index 5606487f5b..1e78a3d4b3 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -26,8 +26,6 @@
  * start address of the data in each blob is aligned as required. Note that
  * each blob's *data* is aligned to BLOBLIST_ALIGN regardless of the alignment
  * of the bloblist itself or the blob header.
- *
- * So far, only BLOBLIST_ALIGN alignment is supported.
  */
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -128,24 +126,24 @@ static struct bloblist_rec *bloblist_findrec(uint tag)
return NULL;
 }
 
-static int bloblist_addrec(uint tag, int size, int align,
+static int bloblist_addrec(uint tag, int size, int align_log2,
   struct bloblist_rec **recp)
 {
struct bloblist_hdr *hdr = gd->bloblist;
struct bloblist_rec *rec;
int data_start, new_alloced;
 
-   if (!align)
-   align = BLOBLIST_ALIGN;
+   if (!align_log2)
+   align_log2 = BLOBLIST_ALIGN_LOG2;
 
/* Figure out where the new data will start */
data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec);
 
/* Align the address and then calculate the offset from ->alloced */
-   data_start = ALIGN(data_start, align) - map_to_sysmem(hdr);
+   data_start = ALIGN(data_start, 1U << align_log2) - map_to_sysmem(hdr);
 
/* Calculate the new allocated total */
-   new_alloced = data_start + ALIGN(size, align);
+   new_alloced = data_start + ALIGN(size, 1U << align_log2);
 
if (new_alloced > hdr->size) {
log_err("Failed to allocate %x bytes size=%x, need size=%x\n",
@@ -169,7 +167,7 @@ static int bloblist_addrec(uint tag, int size, int align,
 }
 
 static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int size,
- int align)
+ int align_log2)
 {
struct bloblist_rec *rec;
 
@@ -182,7 +180,7 @@ static int bloblist_ensurerec(uint tag, struct bloblist_rec 
**recp, int size,
} else {
int ret;
 
-   ret = bloblist_addrec(tag, size, align, );
+   ret = bloblist_addrec(tag, size, align_log2, );
if (ret)
return ret;
}
@@ -204,22 +202,22 @@ void *bloblist_find(uint tag, int size)
return (void *)rec + rec->hdr_size;
 }
 
-void *bloblist_add(uint tag, int size, int align)
+void *bloblist_add(uint tag, int size, int align_log2)
 {
struct bloblist_rec *rec;
 
-   if (bloblist_addrec(tag, size, align, ))
+   if (bloblist_addrec(tag, size, align_log2, ))
return NULL;
 
return (void *)rec + rec->hdr_size;
 }
 
-int bloblist_ensure_size(uint tag, int size, int align, void **blobp)
+int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp)
 {
struct bloblist_rec *rec;
int ret;
 
-   ret = bloblist_ensurerec(tag, , size, align);
+   ret = bloblist_ensurerec(tag, , size, align_log2);
if (ret)
return ret;
*blobp = (void *)rec + rec->hdr_size;
diff --git a/include/bloblist.h b/include/bloblist.h
index 92dbfda21b..5ad1337d1f 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -76,7 +76,9 @@
 enum {
BLOBLIST_VERSION= 0,
BLOBLIST_MAGIC  = 0xb00757a3,
-   BLOBLIST_ALIGN  = 16,
+
+   BLOBLIST_ALIGN_LOG2 = 3,
+   BLOBLIST_ALIGN  = 1 << BLOBLIST_ALIGN_LOG2,
 };
 
 /* Supported tags - add new ones to 

[PATCH v4 01/12] bloblist: Update the tag numbering

[Raymond Mao]

From: Simon Glass 

Align bloblist tags with the FW handoff spec v0.9.
The most common ones are from 0.
TF related ones are from 0x100.
All non-standard ones from 0xfff000.

Added new defined tags:
BLOBLISTT_OPTEE_PAGABLE_PART for TF.
BLOBLISTT_TPM_EVLOG and BLOBLISTT_TPM_CRB_BASE for TPM.

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
---
Changes in v2
- Align bloblist tags to FW handoff spec v0.9.
Changes in v3
- Add TPM related tags

 common/bloblist.c  | 18 ++---
 include/bloblist.h | 67 +-
 test/bloblist.c|  4 +--
 3 files changed, 52 insertions(+), 37 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index a22f6c12b0..5606487f5b 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -36,16 +36,26 @@ static struct tag_name {
enum bloblist_tag_t tag;
const char *name;
 } tag_name[] = {
-   { BLOBLISTT_NONE, "(none)" },
+   { BLOBLISTT_VOID, "(void)" },
 
/* BLOBLISTT_AREA_FIRMWARE_TOP */
+   { BLOBLISTT_CONTROL_FDT, "Control FDT" },
+   { BLOBLISTT_HOB_BLOCK, "HOB block" },
+   { BLOBLISTT_HOB_LIST, "HOB list" },
+   { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
+   { BLOBLISTT_TPM_EVLOG, "TPM event log defined by TCG EFI" },
+   { BLOBLISTT_TPM_CRB_BASE, "TPM Command Response Buffer address" },
 
/* BLOBLISTT_AREA_FIRMWARE */
-   { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" },
-   { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" },
{ BLOBLISTT_TPM2_TCG_LOG, "TPM v2 log space" },
{ BLOBLISTT_TCPA_LOG, "TPM log space" },
-   { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
+   { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" },
+
+   /* BLOBLISTT_AREA_TF */
+   { BLOBLISTT_OPTEE_PAGABLE_PART, "OP-TEE pagable part" },
+
+   /* BLOBLISTT_AREA_OTHER */
+   { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" },
{ BLOBLISTT_SMBIOS_TABLES, "SMBIOS tables for x86" },
{ BLOBLISTT_VBOOT_CTX, "Chrome OS vboot context" },
 
diff --git a/include/bloblist.h b/include/bloblist.h
index 080cc46a12..92dbfda21b 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -81,7 +81,7 @@ enum {
 
 /* Supported tags - add new ones to tag_name in bloblist.c */
 enum bloblist_tag_t {
-   BLOBLISTT_NONE = 0,
+   BLOBLISTT_VOID = 0,
 
/*
 * Standard area to allocate blobs used across firmware components, for
@@ -89,42 +89,36 @@ enum bloblist_tag_t {
 * projects.
 */
BLOBLISTT_AREA_FIRMWARE_TOP = 0x1,
+   /*
+* Devicetree for use by firmware. On some platforms this is passed to
+* the OS also
+*/
+   BLOBLISTT_CONTROL_FDT = 1,
+   BLOBLISTT_HOB_BLOCK = 2,
+   BLOBLISTT_HOB_LIST = 3,
+   BLOBLISTT_ACPI_TABLES = 4,
+   BLOBLISTT_TPM_EVLOG = 5,
+   BLOBLISTT_TPM_CRB_BASE = 6,
 
/* Standard area to allocate blobs used across firmware components */
-   BLOBLISTT_AREA_FIRMWARE = 0x100,
+   BLOBLISTT_AREA_FIRMWARE = 0x10,
+   BLOBLISTT_TPM2_TCG_LOG = 0x10,  /* TPM v2 log space */
+   BLOBLISTT_TCPA_LOG = 0x11,  /* TPM log space */
/*
 * Advanced Configuration and Power Interface Global Non-Volatile
 * Sleeping table. This forms part of the ACPI tables passed to Linux.
 */
-   BLOBLISTT_ACPI_GNVS = 0x100,
-   BLOBLISTT_INTEL_VBT = 0x101,/* Intel Video-BIOS table */
-   BLOBLISTT_TPM2_TCG_LOG = 0x102, /* TPM v2 log space */
-   BLOBLISTT_TCPA_LOG = 0x103, /* TPM log space */
-   BLOBLISTT_ACPI_TABLES = 0x104,  /* ACPI tables for x86 */
-   BLOBLISTT_SMBIOS_TABLES = 0x105, /* SMBIOS tables for x86 */
-   BLOBLISTT_VBOOT_CTX = 0x106,/* Chromium OS verified boot context */
+   BLOBLISTT_ACPI_GNVS = 0x12,
 
-   /*
-* Project-specific tags are permitted here. Projects can be open source
-* or not, but the format of the data must be fuily documented in an
-* open source project, including all fields, bits, etc. Naming should
-* be: BLOBLISTT__
-*/
-   BLOBLISTT_PROJECT_AREA = 0x8000,
-   BLOBLISTT_U_BOOT_SPL_HANDOFF = 0x8000, /* Hand-off info from SPL */
-   BLOBLISTT_VBE   = 0x8001,   /* VBE per-phase state */
-   BLOBLISTT_U_BOOT_VIDEO = 0x8002, /* Video information from SPL */
-
-   /*
-* Vendor-specific tags are permitted here. Projects can be open source
-* or not, but the format of the data must be fuily documented in an
-* open source project, including all fields, bits, etc. Naming should
-* be BLOBLISTT__
-*/
-   BLOBLISTT_VENDOR_AREA = 0xc000,
+   /* Standard area to allocate blobs used for Trusted Firmware */
+   BLOBLISTT_AREA_TF = 0x100,
+   BLOBLISTT_OPTEE_PAGABLE_PART = 0x100,
 
-   /* Tags after this are not allocated for now */
-   BLOBLISTT_EXPANSION = 0x1,
+   /* 

[PATCH v4 00/12] Support Firmware Handoff spec via bloblist

[Raymond Mao]

Major changes:

Update bloblist to align to Firmware Handoff spec v0.9
(up to commit #3592349 of the spec).
(https://github.com/FirmwareHandoff/firmware_handoff).

Includes:
- Align bloblist tags with the FW handoff spec
- Add an explicit alignment field in the header
- Update bloblist magic and version
- Use a packed format for blob record header
- Change the checksum alorigthm
- Use a void entry to handle the alignment
- Adjust the headers of bloblist and blob record
- Align the bloblist record data section start address

>From v3, the implementation from boot arguments to bloblist and how to
load the FDT from the bloblist are moved to a forthcoming patch serie.

In v4, patch #7 and #14 from v3 are squashed.

Simon Glass (12):
  bloblist: Update the tag numbering
  bloblist: Adjust API to align in powers of 2
  bloblist: Change the magic value
  bloblist: Set version to 1
  bloblist: Access record hdr_size and tag via a function
  bloblist: Drop spare value from bloblist record
  bloblist: Checksum the entire bloblist
  bloblist: Handle alignment with a void entry
  bloblist: Reduce blob-header size
  bloblist: Adjust the bloblist header
  bloblist: Add alignment to bloblist_new()
  bloblist: Update documentation and header comment

 arch/x86/lib/tables.c|   3 +-
 common/bloblist.c| 205 ---
 doc/develop/bloblist.rst |   4 +-
 include/bloblist.h   | 166 ++-
 test/bloblist.c  | 105 +++-
 5 files changed, 287 insertions(+), 196 deletions(-)

-- 
2.25.1

Re: [PATCH v4] fdt: Allow the devicetree to come from a bloblist

[Tom Rini]

On Wed, Dec 27, 2023 at 05:48:44PM +, Simon Glass wrote:
> Hi Ilias,
> 
> On Wed, Dec 27, 2023 at 6:44 AM Ilias Apalodimas
>  wrote:
> >
> > Hi Tom,
> >
> > On Tue, 26 Dec 2023 at 14:07, Tom Rini  wrote:
> > >
> > > On Tue, Dec 26, 2023 at 09:46:25AM +, Simon Glass wrote:
> > >
> > > > Standard passage provides for a bloblist to be passed from one firmware
> > > > phase to the next. That can be used to pass the devicetree along as 
> > > > well.
> > > > Add an option to support this.
> > > >
> > > > Tests for this will be added as part of the Universal Payload work.
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > ---
> > > > The discussion on this was not resolved and is now important due to the
> > > > bloblist series from Raymond. So I am sending it again since I believe
> > > > this is a better starting point than building on OF_BOARD
> > >
> > > I really don't like adding another option for "DT is given to us". Why
> > > isn't adding another enum to fdt_source_t sufficient, and if we have
> > > bloblist enabled, that will look for and use if found? Maybe some other
> > > code needs to be restructured and cleaned up too?
> >
> > Same here. On top of that the bloblist has a few items in there, e.g a
> > TPM eventlog. What are we going to do? Add a Kconfig for each item?
> 
> No, but that is just a straw man. The DT is special and U-Boot reports
> where it comes from.
> 
> >
> > This has been going back and forth for a while. I've lost count of how
> > many times I repeated the same proposal, but here it goes again. We
> > have OF_BOARD and BLOBLIST options. The bloblist and its properties
> > are scannable at runtime. Can't we use the combination of these 2 can
> > be used to imply we expect things from a bloblist. If we want to be
> > stricter in the future and explicitly expect the DT from a bloblist,
> > we could add a Kconfig option failing the boot if that's missing.
> 
> I would like to have that Kconfig option now, not later. In my mind,
> the boot must be deterministic, so that if OF_BLOBLIST is enabled, the
> DT must come from there, or it is an error.

Determinism doesn't require a CONFIG option, it just requires an if/else
tree where we say what the "correct" priority list should be and then
set a flag so that we can tell the user where we found it too. This also
means that we can get whatever is going to use this mechanism to
migrate over, and have less of a chicken-and-egg type of problem.

> Also, repeating it doesn't make the proposal good. We agreed that
> OF_BOARD would eventually go away, so building on top of it is not
> setting us up for the future.

I wonder if OF_BOARD will ever go away, and I'm not convinced it will
either. Unless you just mean re-naming it and making a few ad-hoc
standards more easily re-usable, which also will need to happen
regardless.

-- 
Tom


signature.asc
Description: PGP signature

I'm looking for the source code of a specific u-boot version.

[Mario Marietto]

Hello.

I'm trying to boot FreeBSD for arm32 bit as DomU on my ARM Chromebook
SNOW with xen. Basically there are two ways to accomplish this task :


1) to write a patch that allows the FreeBSD kernel to boot as a zImage
file. This could be accomplished applying this patch to a specific
file that's on the source code of FreeBSD :


https://xenbits.xen.org/gitweb/?p=p...8;hb=0782e25d98cc1391472717035f986c979edef0c9



This patch was written by Julien Grall a lot of time ago and now it
does not work anymore. This is the reason explain by the xen
developers :



It appears FreeBSD-CURRENT removed the last step converting the kernel
file to kernel.bin.The patch can be readily rebased, but without
kernel.bin that doesn't do too much.



So,without a rebase of that patch the first option is not applicable.
And I'm not able to fix it.


2) booting FreeBSD using U-Boot,as explained to me by a xen developer :


I was trying to explain why and how Julien's patch works so that you
could be the one to re-do something similar or fix the patch on the
FreeBSD kernel that you are working with. I am happy to help review
and write patches but I don't work with the FreeBSD kernel so I
wouldn't be able to help you quickly. However, I might have a
suggestion. Do you know if FreeBSD can be booted by U-Boot ? Because
U-Boot definitely boots as Xen on ARM guest firmware/bootloader. You
should be able to build U-Boot and use the U-Boot binary as Xen guest
kernel, then U-Boot could load FreeBSD from disk or network and start
it. For instance as domU config file:

kernel="/home/petalinux/u-boot.bin"
disk = [ '/home/petalinux/test.img,raw,xvda' ]


Actually I'm working on the idea n. 2. Basically I need to find the
proper u-boot file that's able to boot the image of FreeBSD that I
have installed (13.2 for arm32 bit). Maybe I found it here :


http://commondatastorage.googleapis.com/chromeos-localmirror/distfiles/nv_uboot-snow-simplefb.kpart.bz2


I found that link inside this tutorial :


https://wiki.freebsd.org/arm/Chromebook


the version of u-boot that has been embedded in that file is the following one :


# strings nv_uboot-snow-simplefb.kpart | grep U-Boot
U-Boot 2011.12-gc1f6280 (May 27 2013 - 15:06:59) for SMDK5250


So the question is easy : I need to find the source code of that old
version of u-boot,because once compiled,it will give me the proper
u-boot.bin kernel / bootloader file that maybe will be able to boot
FreeBSD.


-- 
Mario.

Re: [PATCH v4] fdt: Allow the devicetree to come from a bloblist

[Tom Rini]

On Wed, Dec 27, 2023 at 05:48:42PM +, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, Dec 26, 2023 at 12:07 PM Tom Rini  wrote:
> >
> > On Tue, Dec 26, 2023 at 09:46:25AM +, Simon Glass wrote:
> >
> > > Standard passage provides for a bloblist to be passed from one firmware
> > > phase to the next. That can be used to pass the devicetree along as well.
> > > Add an option to support this.
> > >
> > > Tests for this will be added as part of the Universal Payload work.
> > >
> > > Signed-off-by: Simon Glass 
> > > ---
> > > The discussion on this was not resolved and is now important due to the
> > > bloblist series from Raymond. So I am sending it again since I believe
> > > this is a better starting point than building on OF_BOARD
> >
> > I really don't like adding another option for "DT is given to us". Why
> > isn't adding another enum to fdt_source_t sufficient
> 
> That is added by this patch, but...
> 
> >, and if we have
> > bloblist enabled, that will look for and use if found?
> 
> ...this is the question. I would like to be able to enable bloblist
> without *requiring* the DT to come from there, hence the separate
> Kconfig option.
> 
> > Maybe some other
> > code needs to be restructured and cleaned up too?
> 
> Possibly, but I really am not keen on this board-specific solution. I
> believe Ilias & I agreed that OF_BOARD should fade away, so I don't
> like adding another thing onto it.

I think you're missing something then. It should not be board specific
to just find the dtb via bloblist, without a new CONFIG. Whatever is
stopping that is what needs to be refactored and then you can just
extend fdt_source_t without a new CONFIG option.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 9/9] mtd: spi-nor-ids: Add Infineon(Cypress) s28hs02gt ID

[Dhruva Gole]

Hi!

On Dec 22, 2023 at 14:46:06 +0900, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano 
> 
> Infineon(Cypress) S28HS02GT is 1.8V, 2Gb (256MB) NOR Flash memory with
> Octal interface. It is a dual-die package parts and has same features
> with existing S28 series.
> 
> Signed-off-by: Takahiro Kuwano 
> ---
>  drivers/mtd/spi/spi-nor-ids.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index f86e7ff8e5..949ceee375 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -370,6 +370,7 @@ const struct flash_info spi_nor_ids[] = {
>   { INFO("s28hl01gt",  0x345a1b,  0, 256 * 1024, 512, 
> SPI_NOR_OCTAL_DTR_READ) },
>   { INFO("s28hs512t",  0x345b1a,  0, 256 * 1024, 256, 
> SPI_NOR_OCTAL_DTR_READ) },
>   { INFO("s28hs01gt",  0x345b1b,  0, 256 * 1024, 512, 
> SPI_NOR_OCTAL_DTR_READ) },
> + { INFO("s28hs02gt",  0x345b1c,  0, 256 * 1024, 1024, 
> SPI_NOR_OCTAL_DTR_READ) },

Reviewed-by: Dhruva Gole 

>  #endif
>  #endif
>  #ifdef CONFIG_SPI_FLASH_SST  /* SST */
> -- 
> 2.34.1
> 

-- 
Best regards,
Dhruva Gole 

Re: [PATCH v3 06/14] bloblist: Drop spare value from bloblist record

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 04:43, Ilias Apalodimas 
wrote:

> Hi Raymond,
>
> On Mon, 18 Dec 2023 at 20:20, Raymond Mao  wrote:
> >
> > From: Simon Glass 
> >
> > Drop spare value from bloblist record header.
> >
> > For now it is still present in the header, with an underscore, so that
> > tests continue to pass.
>
> I am not sure I understand the commit message. Doesn't the spec define
> this as 'reserved' now?
> On top of that, it's a better idea to follow the spec naming. It's
> easier to map the spec -> code for new readers. So I would rename
>
> tag -> tag_id
> size -> data_size and
> flags -> reserved.
>
> Can we at least do the flags -> reserved in this patch?
>
> The spec only defines a spare field for TL header (bloblist header) but
not for a TE header
(bloblist record header), so we just remove the spare field, no need to
update this patch
I think.

[...]

Regards,
Raymond

Re: [PATCH v3 09/14] bloblist: Handle alignment with a void entry

[Simon Glass]

Hi Ilias,

On Wed, Dec 27, 2023 at 9:58 AM Ilias Apalodimas
 wrote:
>
> On Mon, 18 Dec 2023 at 20:20, Raymond Mao  wrote:
> >
> > From: Simon Glass 
> >
> > Rather than setting the alignment using the header size, add an entirely
> > new entry to cover the gap left by the alignment.
>
> Hmm, why? Does it make out life easier somehow if new entries get added?

This was one of the spec changes. Not one that I agreed with, but there it is.

Regards,
Simon

Re: [PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

[Simon Glass]

Hi Sughosh,

On Mon, Dec 4, 2023 at 7:15 AM Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Sat, 2 Dec 2023 at 00:02, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu  wrote:
> > >
> > > hi Simon,
> > >
> > > On Thu, 30 Nov 2023 at 08:16, Simon Glass  wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu  
> > > > wrote:
> > > > >
> > > > > hi Ilias,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > > >  wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu 
> > > > > >  wrote:
> > > > > > >
> > > > > > > hi Simon,
> > > > > > >
> > > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Sughosh,
> > > > > > > >
> > > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > Add support for specifying the parameters needed for capsule
> > > > > > > > > generation through a config file, instead of passing them 
> > > > > > > > > through
> > > > > > > > > command-line. Parameters for more than a single capsule file 
> > > > > > > > > can be
> > > > > > > > > specified, resulting in generation of multiple capsules 
> > > > > > > > > through a
> > > > > > > > > single invocation of the command.
> > > > > > > > >
> > > > > > > > > The config file can then be passed to the mkeficapsule tool 
> > > > > > > > > in such
> > > > > > > > > manner
> > > > > > > > >
> > > > > > > > >  $ ./tools/mkeficapsule -f 
> > > > > > > > >
> > > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > > ---
> > > > > > > > >  tools/Kconfig  |  15 ++
> > > > > > > > >  tools/Makefile |   1 +
> > > > > > > > >  tools/eficapsule.h | 114 
> > > > > > > > >  tools/mkeficapsule.c   |  87 +
> > > > > > > > >  tools/mkeficapsule_parse.c | 352 
> > > > > > > > > +
> > > > > > > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > > > > > > >  create mode 100644 tools/mkeficapsule_parse.c
> > > > > > > >
> > > > > > > > This patch keeps coming back :-)
> > > > > > > >
> > > > > > > > Can we not add multiple capsules in the binman description? Why 
> > > > > > > > do we
> > > > > > > > need a new file format? How can binman decode images produced 
> > > > > > > > in this
> > > > > > > > way?
> > > > > > >
> > > > > > > So as Tom mentions, this brings parity with respect to the other
> > > > > > > capsule generation tool in EDKII that generates capsules. IIRC, 
> > > > > > > this
> > > > > > > is something which even Xilix was interested in, and Michal had 
> > > > > > > kind
> > > > > > > of gone through these patches earlier. Lastly, it would be good to
> > > > > > > have support in U-Boot's mkeficapsule tool for generating a single
> > > > > > > capsule file with multiple payloads, and having support for this
> > > > > > > functionality helps in that goal.
> > > > > > >
> > > > > > > Also, you might have noticed that, since your objection to the 
> > > > > > > last
> > > > > > > series, I have removed putting this in binman. So now, this 
> > > > > > > aspect of
> > > > > > > the capsule generation would only be supported through the
> > > > > > > command-line invocation of the tool.
> > > > > >
> > > > > > I think that overall the approach is sane. mkeficapsule is currently
> > > > > > supported and compiled for distros, so the multiple payload support 
> > > > > > is
> > > > > > useful. If we want to add support to binman, instead of rewriting 
> > > > > > this
> > > > > > in python, we could just call that tool for parsing and creating
> > > > > > capsules
> > > > >
> > > > > Given the amount of time these patches have been under review(also
> > > > > number of iterations), I would request that this series be reviewed
> > > > > and merged first. I think there is general consensus that there is
> > > > > value to have this functionality in the mkeficapsule tool. If it is
> > > > > deemed fit to support this through binman as well, that task can be
> > > > > taken up separately. Thanks.
> > > >
> > > > The point you are missing is that it is the entire goal of 'skirting
> > > > around' binman which is suspect.
> > > >
> > > > If there is a need to generate an output file from the build, we
> > > > should support it in binman. If people start creating configuration
> > > > files all over the place, then they are not using binman, right?
> > > >
> > > > I understand that there are pre-existing vendor-specific config files,
> > > > etc. that the EFI thing is a grey area, but I cannot imagine that this
> > > > patch would lead to a good outcome.
> > > >
> > > > The goal of binman is to bring order to the chaos of firmware
> > > > packaging...we cannot do that if it is not actually used.
> > > >
> > > > So let's figure out what is missing from binman's capsule generation
> > > > 

Re: [PATCH v3 01/14] bloblist: Update the tag numbering

[Simon Glass]

Hi Ilias,

On Wed, Dec 27, 2023 at 9:48 AM Ilias Apalodimas
 wrote:
>
> Hi Raymond,
>
> On Mon, 18 Dec 2023 at 20:19, Raymond Mao  wrote:
> >
> > From: Simon Glass 
> >
> > Align bloblist tags with the FW handoff spec v0.9.
> > The most common ones are from 0.
> > TF related ones are from 0x100.
> > All non-standard ones from 0xfff000.
> >
> > Added new defined tags:
> > BLOBLISTT_OPTEE_PAGABLE_PART for TF.
> > BLOBLISTT_TPM_EVLOG and BLOBLISTT_TPM_CRB_BASE for TPM.
> >
> > Signed-off-by: Simon Glass 
> > Co-developed-by: Raymond Mao 
> > Signed-off-by: Raymond Mao 
> > ---
> > Changes in v2
> > - Align bloblist tags to FW handoff spec v0.9.
> > Changes in v3
> > - Add TPM related tags
> >
> >  common/bloblist.c  | 18 ++---
> >  include/bloblist.h | 67 +-
> >  test/bloblist.c|  4 +--
> >  3 files changed, 52 insertions(+), 37 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index a22f6c12b0..5606487f5b 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -36,16 +36,26 @@ static struct tag_name {
> > enum bloblist_tag_t tag;
> > const char *name;
> >  } tag_name[] = {
> > -   { BLOBLISTT_NONE, "(none)" },
> > +   { BLOBLISTT_VOID, "(void)" },
> >
> > /* BLOBLISTT_AREA_FIRMWARE_TOP */
> > +   { BLOBLISTT_CONTROL_FDT, "Control FDT" },
> > +   { BLOBLISTT_HOB_BLOCK, "HOB block" },
> > +   { BLOBLISTT_HOB_LIST, "HOB list" },
> > +   { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
> > +   { BLOBLISTT_TPM_EVLOG, "TPM event log defined by TCG EFI" },
> > +   { BLOBLISTT_TPM_CRB_BASE, "TPM Command Response Buffer address" },
> >
> > /* BLOBLISTT_AREA_FIRMWARE */
> > -   { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" },
> > -   { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" },
> > { BLOBLISTT_TPM2_TCG_LOG, "TPM v2 log space" },
> > { BLOBLISTT_TCPA_LOG, "TPM log space" },
> > -   { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
> > +   { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" },
> > +
> > +   /* BLOBLISTT_AREA_TF */
> > +   { BLOBLISTT_OPTEE_PAGABLE_PART, "OP-TEE pagable part" },
> > +
> > +   /* BLOBLISTT_AREA_OTHER */
> > +   { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" },
> > { BLOBLISTT_SMBIOS_TABLES, "SMBIOS tables for x86" },
> > { BLOBLISTT_VBOOT_CTX, "Chrome OS vboot context" },
> >
> > diff --git a/include/bloblist.h b/include/bloblist.h
> > index 080cc46a12..92dbfda21b 100644
> > --- a/include/bloblist.h
> > +++ b/include/bloblist.h
> > @@ -81,7 +81,7 @@ enum {
> >
> >  /* Supported tags - add new ones to tag_name in bloblist.c */
> >  enum bloblist_tag_t {
> > -   BLOBLISTT_NONE = 0,
> > +   BLOBLISTT_VOID = 0,
> >
> > /*
> >  * Standard area to allocate blobs used across firmware components, 
> > for
> > @@ -89,42 +89,36 @@ enum bloblist_tag_t {
> >  * projects.
> >  */
> > BLOBLISTT_AREA_FIRMWARE_TOP = 0x1,
> > +   /*
> > +* Devicetree for use by firmware. On some platforms this is passed 
> > to
> > +* the OS also
> > +*/
> > +   BLOBLISTT_CONTROL_FDT = 1,
> > +   BLOBLISTT_HOB_BLOCK = 2,
> > +   BLOBLISTT_HOB_LIST = 3,
> > +   BLOBLISTT_ACPI_TABLES = 4,
> > +   BLOBLISTT_TPM_EVLOG = 5,
> > +   BLOBLISTT_TPM_CRB_BASE = 6,
> >
> > /* Standard area to allocate blobs used across firmware components 
> > */
> > -   BLOBLISTT_AREA_FIRMWARE = 0x100,
> > +   BLOBLISTT_AREA_FIRMWARE = 0x10,
> > +   BLOBLISTT_TPM2_TCG_LOG = 0x10,  /* TPM v2 log space */
> > +   BLOBLISTT_TCPA_LOG = 0x11,  /* TPM log space */
> > /*
> >  * Advanced Configuration and Power Interface Global Non-Volatile
> >  * Sleeping table. This forms part of the ACPI tables passed to 
> > Linux.
> >  */
> > -   BLOBLISTT_ACPI_GNVS = 0x100,
> > -   BLOBLISTT_INTEL_VBT = 0x101,/* Intel Video-BIOS table */
> > -   BLOBLISTT_TPM2_TCG_LOG = 0x102, /* TPM v2 log space */
> > -   BLOBLISTT_TCPA_LOG = 0x103, /* TPM log space */
> > -   BLOBLISTT_ACPI_TABLES = 0x104,  /* ACPI tables for x86 */
> > -   BLOBLISTT_SMBIOS_TABLES = 0x105, /* SMBIOS tables for x86 */
> > -   BLOBLISTT_VBOOT_CTX = 0x106,/* Chromium OS verified boot 
> > context */
> > +   BLOBLISTT_ACPI_GNVS = 0x12,
>
> Any idea if we have users that would be affected by this value change?

I suppose it is possible, but bloblist has so far been intended for
internal U-Boot use, so perhaps not? In any case, there isn't much we
can do about it.

Regards,
Simon

Re: [PATCH v2 4/5] lib: membuff: fix readline not returning line in case of overflow

[Simon Glass]

On Wed, Nov 15, 2023 at 3:39 PM Svyatoslav Ryhel  wrote:
>
> From: Ion Agorria 
>
> If line overflows readline it will not be returned, fix this behavior,
> make it optional and documented properly.
>
> Signed-off-by: Ion Agorria 
> Signed-off-by: Svyatoslav Ryhel 
> ---
>  boot/bootmeth_extlinux.c | 2 +-
>  common/console.c | 2 +-
>  include/membuff.h| 5 +++--
>  lib/membuff.c| 4 ++--
>  4 files changed, 7 insertions(+), 6 deletions(-)
>

Reviewed-by: Simon Glass 


> diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c
> index aa2a4591eb..ae0ad1d53e 100644
> --- a/boot/bootmeth_extlinux.c
> +++ b/boot/bootmeth_extlinux.c
> @@ -82,7 +82,7 @@ static int extlinux_fill_info(struct bootflow *bflow)
> log_debug("parsing bflow file size %x\n", bflow->size);
> membuff_init(, bflow->buf, bflow->size);
> membuff_putraw(, bflow->size, true, );
> -   while (len = membuff_readline(, line, sizeof(line) - 1, ' '), len) 
> {
> +   while (len = membuff_readline(, line, sizeof(line) - 1, ' ', 
> true), len) {
> char *tok, *p = line;
>
> tok = strsep(, " ");
> diff --git a/common/console.c b/common/console.c
> index 8a869b137e..cd56035171 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -846,7 +846,7 @@ bool console_record_overflow(void)
>  int console_record_readline(char *str, int maxlen)
>  {
> return membuff_readline((struct membuff *)>console_out, str,
> -   maxlen, '\0');
> +   maxlen, '\0', false);
>  }
>
>  int console_record_avail(void)
> diff --git a/include/membuff.h b/include/membuff.h
> index 21051b0c54..4eba626ce1 100644
> --- a/include/membuff.h
> +++ b/include/membuff.h
> @@ -192,10 +192,11 @@ int membuff_free(struct membuff *mb);
>   * @mb: membuff to adjust
>   * @str: Place to put the line
>   * @maxlen: Maximum line length (excluding terminator)
> + * @must_fit: If true then str is empty if line doesn't fit
>   * Return: number of bytes read (including terminator) if a line has been
> - *read, 0 if nothing was there
> + *read, 0 if nothing was there or line didn't fit when must_fit is 
> set
>   */
> -int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch);
> +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, 
> bool must_fit);
>
>  /**
>   * membuff_extend_by() - expand a membuff
> diff --git a/lib/membuff.c b/lib/membuff.c
> index 36dc43a523..964e504d5b 100644
> --- a/lib/membuff.c
> +++ b/lib/membuff.c
> @@ -288,7 +288,7 @@ int membuff_free(struct membuff *mb)
> (mb->end - mb->start) - 1 - membuff_avail(mb);
>  }
>
> -int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch)
> +int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, 
> bool must_fit)
>  {
> int len;  /* number of bytes read (!= string length) */
> char *s, *end;
> @@ -310,7 +310,7 @@ int membuff_readline(struct membuff *mb, char *str, int 
> maxlen, int minch)
> }
>
> /* couldn't get the whole string */
> -   if (!ok) {
> +   if (!ok && must_fit) {
> if (maxlen)
> *orig = '\0';
> return 0;
> --
> 2.40.1
>

Re: Proposal: FIT support for extension boards / overlays

[Simon Glass]

Hi Heinrich,

On Tue, Dec 12, 2023 at 3:43 PM Heinrich Schuchardt  wrote:
>
> On 12.12.23 15:05, Simon Glass wrote:
> > Hi,
> >
> > The devicetree files for a board can be quite large, perhaps around
> > 60KB. To boot on any supported board, many of them need to be
> > provided, typically hundreds.
> >
> > All boards for a particular SoC share common parts.  It would be
> > helpful to use overlays for common pieces, to reduce the overall size.
> >
> > Some boards have extension add-ons which have their own devicetree
> > overlays. It would be helpful to know which ones should be applied for
> > a particular extension.
> >
> > I propose implementing extensions in FIT. This has a new '/extensions'
> > node, so you can specify what extensions are available for each FIT
> > configuration.
> >
> > For example:
> >
> > / {
> >images {
> >  kernel {
> >// common kernel
> >  };
> >
> >  fdt-1 {
> >// FDT for board1
> >  };
> >
> >  fdto-1 {
> >// FDT overlay
> >  };
> >
> >  fdto-2 {
> >// FDT overlay
> >  };
> >
> >  fdto-3 {
> >// FDT overlay
> >  };
> >};
> >
> >configurations {
> >  conf-1 {
> >  compatible = ...
> >  fdt = "fdt-1";
> >  extensions = "ext1", "ext-2";
>
> Shouldn't there be braces around the item list?

I don't think so.

>
> How do you specify optional and required but otherwise unrelated
> extensions for a configuration?

Do we actually need to know which extensions are required or optional?
Do you have an example?

>
> >  };
> >};
> >
> >extensions {
> >  ext-1 {
> >  fdto = "fdto-1";   // FDT overlay for this 'cape' or 'hat'
> >  kernel = "kernel-1";
> >  compatible = "vendor,combined-device1";
> >  extensions = "ext-3";
> >  };
> >
> >  ext-2 {
> >  fdto = "fdto-2";   // fdt overlay for this 'cape'
> >  compatible = "vendor,device2";
> >  };
> >
> >  ext-3 {
> >  fdto = "fdto-3";
> >  compatible = "vendor,device3";
> >  };
> >};
> > };
> >
> > So FIT configurations have a list of supported extensions. The
> > extensions are hierarchical so that you can have ext-1 which can
> > optionally have ext-2 as well. This allows boards to share a common
>
> ext2 seems not to be related to ext-3. Do you mean ext-3 optionally
> extending ext-1? How would you specify that ext-n is required for ext-m
> to work?

Yes, I mean "optionally extending ext2". Again, I don't consider
required extensions here.

>
> The sequence of applying overlays may make a difference. I cannot see
> that your current suggestion defines a sequence in which the overlays
> are applied.
>
> > SoC to add in overlays as needed by their board. It also allows common
> > 'capes' or 'hats' to be specified only once and used by a group of
> > boards which share the same interface.
> >
> > Within U-Boot, extensions actually present are declared by a sysinfo
> > driver for the board, with new methods:
> >
> > get_compat() - determine the compatible strings for the current platform
> > get_ext() - get a list of compatible strings for extensions which are
> > actually present
>
> Do you expect U-Boot to have code that injects device-tree fragments
> with a compatible string into the control FDT after discovering add-ons?

Yes, that's right, by applying overlays.

>
> Why can't we simply write the compatible constraint into the overlay
> definition (fdto-#) and enumerate the overlays in the configuration?

Yes, that is the example quoted below.

>
> On some busses detection is problematic. Two alternative add-ons may use
> the same SPI address but need different FDT overlays.

If there is no way to detect the extension, then it cannot work anyway, right?

BTW I am not suggesting that the bus is used for detection, although I
suppose this is possible. More likely there are GPIOs which can be
decoded to indicate the extension, or perhaps an I2C EEPROM.


>
> Best regards
>
> Heinrich
>
>
> >
> > The extension compatible-strings are used to select the correct things
> > from the FIT, apply the overlays and produce the final DT.
> >
> > To make this simpler for the common case (without extensions), we can
> > allow multiple FDT images for a configuration, with the first one
> > being the base SoC .dtb and the others being the .dtbo overlay(s) for
> > the board:
> >
> > confi-1 {
> >  compatible = ...
> >  fdt = "fdt-1", "fdto-1";
> > };
> >
> > Comments welcome.

Regards,
Simon

Re: [PATCH 04/13] soc: samsung: Add Exynos USI driver

[Simon Glass]

Hi Sam,

On Wed, Dec 13, 2023 at 3:16 AM Sam Protsenko
 wrote:
>
> USIv2 IP-core is found on modern ARM64 Exynos SoCs (like Exynos850) and
> provides selectable serial protocol (one of: UART, SPI, I2C). USIv2
> registers usually reside in the same register map as a particular
> underlying protocol it implements, but have some particular offset. E.g.
> on Exynos850 the USI_UART has 0x1382 base address, where UART
> registers have 0x00..0x40 offsets, and USI registers have 0xc0..0xdc
> offsets. Desired protocol can be chosen via SW_CONF register from System
> Register block of the same domain as USI.
>
> Before starting to use a particular protocol, USIv2 must be configured
> properly:
>   1. Select protocol to be used via System Register
>   2. Clear "reset" flag in USI_CON
>   3. Configure HWACG behavior (e.g. for UART Rx the HWACG must be
>  disabled, so that the IP clock is not gated automatically); this is
>  done using USI_OPTION register
>   4. Keep both USI clocks (PCLK and IPCLK) running during USI registers
>  modification
>
> This driver implements the above behavior. Of course, USIv2 driver
> should be probed before UART/I2C/SPI drivers. It can be achieved by
> embedding UART/I2C/SPI nodes inside of the USI node (in Device Tree);
> driver then walks underlying nodes and instantiates those. Driver also
> handles USI configuration on PM resume, as register contents can be lost
> during CPU suspend.
>
> This driver is designed with different USI versions in mind. So it
> should be relatively easy to add new USI revisions to it later.
>
> Driver's code was copied over from Linux kernel [1] and adapted
> correspondingly for U-Boot API. UCLASS_MISC is used, and although no
> misc operations are implemented, it makes it easier to probe the driver
> this way (as compared to UCLASS_NOP) and keep the code compact.
>
> [1] drivers/soc/samsung/exynos-usi.c
>
> Signed-off-by: Sam Protsenko 
> ---
>  drivers/soc/Kconfig  |   1 +
>  drivers/soc/Makefile |   1 +
>  drivers/soc/samsung/Kconfig  |  23 
>  drivers/soc/samsung/Makefile |   3 +
>  drivers/soc/samsung/exynos-usi.c | 218 +++
>  5 files changed, 246 insertions(+)
>  create mode 100644 drivers/soc/samsung/Kconfig
>  create mode 100644 drivers/soc/samsung/Makefile
>  create mode 100644 drivers/soc/samsung/exynos-usi.c
>

Just a few nits here

Reviewed-by: Simon Glass 

> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index 85dac9de78a4..03433bc0e6d2 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -40,6 +40,7 @@ config SOC_XILINX_VERSAL_NET
>   This allows other drivers to verify the SoC familiy & revision using
>   matching SoC attributes.
>
> +source "drivers/soc/samsung/Kconfig"
>  source "drivers/soc/ti/Kconfig"
>
>  endmenu
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 84385650d46d..610bf816d40a 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -2,6 +2,7 @@
>  #
>  # Makefile for the U-Boot SOC specific device drivers.
>
> +obj-$(CONFIG_SOC_SAMSUNG) += samsung/
>  obj-$(CONFIG_SOC_TI) += ti/
>  obj-$(CONFIG_SOC_DEVICE) += soc-uclass.o
>  obj-$(CONFIG_SOC_DEVICE_TI_K3) += soc_ti_k3.o
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> new file mode 100644
> index ..ffb87fe79316
> --- /dev/null
> +++ b/drivers/soc/samsung/Kconfig
> @@ -0,0 +1,23 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +
> +menuconfig SOC_SAMSUNG
> +   bool "Samsung SoC drivers support"
> +
> +if SOC_SAMSUNG
> +
> +config EXYNOS_USI
> +   bool "Exynos USI (Universal Serial Interface) driver"
> +   depends on ARCH_EXYNOS
> +   select MISC
> +   select REGMAP
> +   select SYSCON
> +   help
> + Enable support for USI block. USI (Universal Serial Interface) is an
> + IP-core found in modern Samsung Exynos SoCs, like Exynos850 and
> + ExynosAutoV9. USI block can be configured to provide one of the
> + following serial protocols: UART, SPI or High Speed I2C.
> +
> + This driver allows one to configure USI for desired protocol, which
> + is usually done in USI node in Device Tree.
> +
> +endif
> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> new file mode 100644
> index ..833ac073fbfa
> --- /dev/null
> +++ b/drivers/soc/samsung/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +
> +obj-$(CONFIG_EXYNOS_USI)   += exynos-usi.o
> diff --git a/drivers/soc/samsung/exynos-usi.c 
> b/drivers/soc/samsung/exynos-usi.c
> new file mode 100644
> index ..23255177e6e3
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-usi.c
> @@ -0,0 +1,218 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023 Linaro Ltd.
> + * Author: Sam Protsenko 
> + *
> + * Samsung Exynos USI driver (Universal Serial Interface).
> + */
> +
> +#include 
> +#include 
> 

Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB

[Simon Glass]

Hi Heinrich,

On Tue, Dec 26, 2023 at 11:01 AM Heinrich Schuchardt  wrote:
>
> On 10/15/23 04:45, Simon Glass wrote:
> > When the SMBIOS table is written to an address above 4GB a 32-bit table
> > address is not large enough.
> >
> > Use an SMBIOS3 table in that case.
> >
> > Note that we cannot use efi_allocate_pages() since this function has
> > nothing to do with EFI. There is no equivalent function to allocate
> > memory below 4GB in U-Boot. One solution would be to create a separate
> > malloc() pool, or just always put the malloc() pool below 4GB.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > Changes in v2:
> > - Check the end of the table rather than the start.
> >
> >   include/smbios.h | 22 +-
> >   lib/smbios.c | 24 +++-
> >   2 files changed, 40 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/smbios.h b/include/smbios.h
> > index c9df2706f5a6..ddabb558299e 100644
> > --- a/include/smbios.h
> > +++ b/include/smbios.h
> > @@ -12,7 +12,8 @@
> >
> >   /* SMBIOS spec version implemented */
> >   #define SMBIOS_MAJOR_VER3
> > -#define SMBIOS_MINOR_VER 0
> > +#define SMBIOS_MINOR_VER 7
> > +
> >
> >   enum {
> >   SMBIOS_STR_MAX  = 64,   /* Maximum length allowed for a string */
> > @@ -54,6 +55,25 @@ struct __packed smbios_entry {
> >   u8 bcd_rev;
> >   };
> >
> > +struct __packed smbios3_entry {
> > + u8 anchor[5];
> > + u8 checksum;
> > + u8 length;
> > + u8 major_ver;
> > +
>
> This empty line is superfluous.
>
> > + u8 minor_ver;
> > + u8 docrev;
>
> 'doc_rev' would be more consistent with the other fields.
>
> > + u8 entry_point_rev;
> > + u8 reserved;
> > + u32 max_struct_size;
> > +
>
> This empty line is superfluous.
>
> Please, consider copying the comments from my patch
>
> [PATCH v2,1/2] smbios: SMBIOS 3.0 (64-bit) Entry Point structure
> https://patchwork.ozlabs.org/project/uboot/patch/20231223010334.248291-2-xypron.g...@gmx.de/
>
> Otherwise looks good to me.

Thanks, I added in your patch and resent the series.

I tested it with dmicode and it seems to work OK.

Regards,
Simon

Re: [PATCH v4] fdt: Allow the devicetree to come from a bloblist

[Simon Glass]

Hi Ilias,

On Wed, Dec 27, 2023 at 6:44 AM Ilias Apalodimas
 wrote:
>
> Hi Tom,
>
> On Tue, 26 Dec 2023 at 14:07, Tom Rini  wrote:
> >
> > On Tue, Dec 26, 2023 at 09:46:25AM +, Simon Glass wrote:
> >
> > > Standard passage provides for a bloblist to be passed from one firmware
> > > phase to the next. That can be used to pass the devicetree along as well.
> > > Add an option to support this.
> > >
> > > Tests for this will be added as part of the Universal Payload work.
> > >
> > > Signed-off-by: Simon Glass 
> > > ---
> > > The discussion on this was not resolved and is now important due to the
> > > bloblist series from Raymond. So I am sending it again since I believe
> > > this is a better starting point than building on OF_BOARD
> >
> > I really don't like adding another option for "DT is given to us". Why
> > isn't adding another enum to fdt_source_t sufficient, and if we have
> > bloblist enabled, that will look for and use if found? Maybe some other
> > code needs to be restructured and cleaned up too?
>
> Same here. On top of that the bloblist has a few items in there, e.g a
> TPM eventlog. What are we going to do? Add a Kconfig for each item?

No, but that is just a straw man. The DT is special and U-Boot reports
where it comes from.

>
> This has been going back and forth for a while. I've lost count of how
> many times I repeated the same proposal, but here it goes again. We
> have OF_BOARD and BLOBLIST options. The bloblist and its properties
> are scannable at runtime. Can't we use the combination of these 2 can
> be used to imply we expect things from a bloblist. If we want to be
> stricter in the future and explicitly expect the DT from a bloblist,
> we could add a Kconfig option failing the boot if that's missing.

I would like to have that Kconfig option now, not later. In my mind,
the boot must be deterministic, so that if OF_BLOBLIST is enabled, the
DT must come from there, or it is an error.

Also, repeating it doesn't make the proposal good. We agreed that
OF_BOARD would eventually go away, so building on top of it is not
setting us up for the future.

Regards,
Simon

Re: [PATCH v4] fdt: Allow the devicetree to come from a bloblist

[Simon Glass]

Hi Tom,

On Tue, Dec 26, 2023 at 12:07 PM Tom Rini  wrote:
>
> On Tue, Dec 26, 2023 at 09:46:25AM +, Simon Glass wrote:
>
> > Standard passage provides for a bloblist to be passed from one firmware
> > phase to the next. That can be used to pass the devicetree along as well.
> > Add an option to support this.
> >
> > Tests for this will be added as part of the Universal Payload work.
> >
> > Signed-off-by: Simon Glass 
> > ---
> > The discussion on this was not resolved and is now important due to the
> > bloblist series from Raymond. So I am sending it again since I believe
> > this is a better starting point than building on OF_BOARD
>
> I really don't like adding another option for "DT is given to us". Why
> isn't adding another enum to fdt_source_t sufficient

That is added by this patch, but...

>, and if we have
> bloblist enabled, that will look for and use if found?

...this is the question. I would like to be able to enable bloblist
without *requiring* the DT to come from there, hence the separate
Kconfig option.

> Maybe some other
> code needs to be restructured and cleaned up too?

Possibly, but I really am not keen on this board-specific solution. I
believe Ilias & I agreed that OF_BOARD should fade away, so I don't
like adding another thing onto it.

Regards,
Simon

[PATCH] mtd: spi-nor: ids: Add is25lx512 chip

[Tejas Bhumkar]

Added support for the ISSI OSPI flash part IS25LX512M.
Initial testing was performed on the Tenzing-se1 board using
SDR mode, covering basic erase, write, and readback operations.

Signed-off-by: Tejas Bhumkar 
---
 drivers/mtd/spi/spi-nor-ids.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index f86e7ff8e5..5ab1bc9ed3 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -234,6 +234,8 @@ const struct flash_info spi_nor_ids[] = {
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ INFO("is25wx256",  0x9d5b19, 0, 128 * 1024, 256,
SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ | 
SPI_NOR_4B_OPCODES) },
+   { INFO("is25lx512",  0x9d5a1a, 0, 64 * 1024, 1024,
+   SECT_4K | USE_FSR | SPI_NOR_4B_OPCODES | 
SPI_NOR_HAS_TB) },
 #endif
 #ifdef CONFIG_SPI_FLASH_MACRONIX   /* MACRONIX */
/* Macronix */
-- 
2.27.0

Re: [PATCH v3 2/9] bloblist: check bloblist with specified buffer size

[Raymond Mao]

Hi Simon,

On Tue, 26 Dec 2023 at 04:47, Simon Glass  wrote:

> Hi Raymond,
>
> On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao 
> wrote:
> >
> > Instead of expecting the bloblist total size to be the same as the
> > pre-allocated buffer size, practically we are more interested in
> > whether the pre-allocated buffer size is bigger than the bloblist
> > total size.
> >
> > Signed-off-by: Raymond Mao 
> > Reviewed-by: Ilias Apalodimas 
> > ---
> > Changes in v2
> > - New patch file created for v2.
> >
> >  common/bloblist.c | 2 +-
> >  test/bloblist.c   | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index ba17dd851a..db3fbb20cf 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -384,7 +384,7 @@ int bloblist_check(ulong addr, uint size)
>
> Can you please also update the function comment for bloblist_check() ?
>
> Yes. I will update it.

[...]

Regards,
Raymond

Re: [PATCH v3 6/9] qemu-arm: Get bloblist from boot arguments

[Raymond Mao]

Hi Simon,

On Tue, 26 Dec 2023 at 04:48, Simon Glass  wrote:

> Hi Raymond,
>
> On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao 
> wrote:
> >
> > Add platform custom function to get bloblist from boot arguments.
> > Check whether boot arguments aligns with the register conventions
> > defined in FW Handoff spec v0.9.
> > Add bloblist related options into qemu default config.
> >
> > Signed-off-by: Raymond Mao 
> > ---
> > Changes in v2
> > - Remove low level code for copying boot arguments.
> > - Refactor board_fdt_blob_setup() and remove direct access of
> gd->bloblist.
> > Changes in v3
> > - Optimize board_bloblist_from_boot_arg().
> >
> >  board/emulation/qemu-arm/qemu-arm.c | 30 +
> >  configs/qemu_arm64_defconfig|  3 +++
> >  2 files changed, 33 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> > index 942f1fff57..e225011bf0 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > @@ -4,6 +4,7 @@
> >   */
> >
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -102,6 +103,15 @@ static struct mm_region qemu_arm64_mem_map[] = {
> >  struct mm_region *mem_map = qemu_arm64_mem_map;
> >  #endif
> >
> > +/*
> > + * Boot parameters saved from start.S
> > + * saved_args[0]: FDT base address
> > + * saved_args[1]: Bloblist signature
> > + * saved_args[2]: must be 0
> > + * saved_args[3]: Bloblist base address
> > + */
> > +extern unsigned long saved_args[];
> > +
> >  int board_init(void)
> >  {
> > return 0;
> > @@ -144,6 +154,26 @@ void *board_fdt_blob_setup(int *err)
> > return (void *)CFG_SYS_SDRAM_BASE;
> >  }
> >
> > +int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
> > +{
> > +   int ret = -ENOENT;
> > +
> > +   if (!IS_ENABLED(CONFIG_OF_BOARD) || !IS_ENABLED(CONFIG_BLOBLIST))
> > +   return -ENOENT;
> > +
> > +   ret = bloblist_check(saved_args[3], size);
> > +   if (ret)
> > +   return ret;
>
> What about saved_args[1] ?
>
> The magic and version of register conventions cannot be checked now since
there is
a bug in the spec. PSB:
```
X1[23:0]: set to the TL signature (4a0f_b10b)
X1[31:24]: version of the register convention used. Set to 1 for the
AArch64 convention specified in this document.
```
Signature (4a0f_b10b) takes all [31:0] and no space for version of the
register convention.
This needs to be updated on TF-A and OP-TEE as well after the spec is
updated.
For now I will just skip the checking.


> > +
> > +   /* Check the register conventions */
> > +   ret = bloblist_check_reg_conv(saved_args[0], saved_args[2]);
> > +   if (!ret)
> > +   /* Relocate the bloblist to the fixed address */
> > +   ret = bloblist_reloc((void *)addr, CONFIG_BLOBLIST_SIZE);
> > +
> > +   return ret;
> > +}
> > +
>
> This should be a generic function, e.g. in arch/arm
>
> I didn't find a place under arch/arm that is good for connect with the
saved boot
args. Do you have any suggestions?

[...]

Regards,
Raymond

Re: [PATCH v3 8/9] fdt: update the document and Kconfig description

[Raymond Mao]

Hi Simon,

On Tue, 26 Dec 2023 at 04:48, Simon Glass  wrote:

> Hi Raymond,
>
> On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao 
> wrote:
> >
> > Update the document and Kconfig to describe the behavior of board
> > specific custom functions when CONFIG_OF_BOARD is defined.
> >
> > Signed-off-by: Raymond Mao 
> > ---
> >  doc/develop/devicetree/control.rst | 6 +++---
> >  dts/Kconfig| 7 +--
> >  2 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/doc/develop/devicetree/control.rst
> b/doc/develop/devicetree/control.rst
> > index cbb65c9b17..e061f4e812 100644
> > --- a/doc/develop/devicetree/control.rst
> > +++ b/doc/develop/devicetree/control.rst
> > @@ -104,9 +104,9 @@ in u-boot.bin so you can still just flash u-boot.bin
> onto your board. If you are
> >  using CONFIG_SPL_FRAMEWORK, then u-boot.img will be built to include
> the device
> >  tree binary.
> >
> > -If CONFIG_OF_BOARD is defined, a board-specific routine will provide the
> > -devicetree at runtime, for example if an earlier bootloader stage
> creates
> > -it and passes it to U-Boot.
> > +If CONFIG_OF_BOARD is defined, board-specific routines will provide the
> > +bloblist and devicetree at runtime, for example if an earlier
> bootloader stage
> > +creates it and passes it to U-Boot.
> >
> >  If CONFIG_SANDBOX is defined, then it will be read from a file on
> >  startup. Use the -d flag to U-Boot to specify the file to read, -D for
> the
> > diff --git a/dts/Kconfig b/dts/Kconfig
> > index 00c0aeff89..12d61dc748 100644
> > --- a/dts/Kconfig
> > +++ b/dts/Kconfig
> > @@ -110,8 +110,11 @@ config OF_BOARD
> > default y if SANDBOX || OF_HAS_PRIOR_STAGE
> > help
> >   If this option is enabled, the device tree is provided at
> runtime by
> > - a custom function called board_fdt_blob_setup(). The board must
> > - implement this function if it wishes to provide special
> behaviour.
> > + a custom function called board_fdt_blob_setup().
> > + If this option is enabled, bloblist is provided at runtime by a
> > + custom function called board_bloblist_from_boot_arg().
> > + The board must implement these functions if it wishes to
> provide
> > + special behaviour.
>
> Again, this spec is not board-specific, so it should work on any ARM
> board, without board-specific code, just by enabling the feature.
>
> dts/Kconfig is for all platforms. And from the view of a non-ARM platform,
this is
still platform-specific.

[...]

Regards,
Raymond

Re: [PATCH v3 1/9] bloblist: add API to check the register conventions

[Raymond Mao]

Hi Simon,

On Tue, 26 Dec 2023 at 04:48, Simon Glass  wrote:

> Hi Raymond,
>
> On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao 
> wrote:
> >
> > Add bloblist_check_reg_conv() to check whether the bloblist is compliant
> > to the register conventions defined in Firmware Handoff specification.
> > This API can be used for all Arm platforms.
> >
> > Signed-off-by: Raymond Mao 
> > ---
> > Changes in v2
> > - Refactor of bloblist_check_reg_conv().
> > Changes in v3
> > - bloblist_check_reg_conv() returns -ENOENT if OF_BOARD is disabled.
> >
> >  common/bloblist.c  | 13 +
> >  include/bloblist.h | 12 
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index 625e480f6b..ba17dd851a 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
> >
> > return 0;
> >  }
> > +
> > +int bloblist_check_reg_conv(ulong rfdt, ulong rzero)
> > +{
> > +   if (!IS_ENABLED(CONFIG_OF_BOARD))
> > +   return -ENOENT;
> > +
> > +   if (rzero || rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT,
> 0)) {
> > +   gd->bloblist = NULL;  /* Reset the gd bloblist pointer */
> > +   return -EIO;
> > +   }
>
> Where does the magic 4a0fb10b value get checked?
>
> The magic and version of register conventions cannot be checked now since
there is
a bug in the spec. PSB:
```
X1[23:0]: set to the TL signature (4a0f_b10b)
X1[31:24]: version of the register convention used. Set to 1 for the
AArch64 convention specified in this document.
```
Signature (4a0f_b10b) takes all [31:0] and no space for version of the
register convention.
This needs to be updated on TF-A and OP-TEE as well after the spec is
updated.
For now I will just skip the checking.

[...]

Regards,
Raymond

Re: [PATCH v3 4/9] arm: armv7: save boot arguments

[Raymond Mao]

Hi Simon,

On Tue, 26 Dec 2023 at 04:48, Simon Glass  wrote:

> Hi Raymond,
>
> On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao 
> wrote:
> >
> > Save boot arguments r[0-3] into an array for handover of bloblist from
> > previous boot stage.
> >
> > Signed-off-by: Raymond Mao 
> > ---
> > Changes in v2
> > - New patch file created for v2.
> > Changes in v3
> > - Swap value of r0 with r2.
> >
> >  arch/arm/cpu/armv7/start.S | 13 +
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > index 69e281b086..2ca63ca32c 100644
> > --- a/arch/arm/cpu/armv7/start.S
> > +++ b/arch/arm/cpu/armv7/start.S
> > @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
> >   *
> >
>  */
> >  WEAK(save_boot_params)
> > +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
> > +   ldr r12, =saved_args
> > +   /* Intentionally swapping r0 with r2 */
> > +   stm r12, {r2, r1, r0, r3}
>
> I thought the instruction was a bitmap, so what does the 'swapping'
> actually change?
>
> The swapping is due to different register conventions between aarch32 and
aarch64.
Actually swapping here is to avoid the complexity of swapping this in C
code.
I will update the comments here.

[...]

Regards,
Raymond

Re: [PATCH v3 4/9] arm: armv7: save boot arguments

[Raymond Mao]

Hi Ilias and Simon,

On Wed, 27 Dec 2023 at 03:30, Ilias Apalodimas 
wrote:

> Hi Raymond,
>
> On Fri, 22 Dec 2023 at 23:31, Raymond Mao  wrote:
> >
> > Save boot arguments r[0-3] into an array for handover of bloblist from
> > previous boot stage.
> >
> > Signed-off-by: Raymond Mao 
> > ---
> > Changes in v2
> > - New patch file created for v2.
> > Changes in v3
> > - Swap value of r0 with r2.
> >
> >  arch/arm/cpu/armv7/start.S | 13 +
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > index 69e281b086..2ca63ca32c 100644
> > --- a/arch/arm/cpu/armv7/start.S
> > +++ b/arch/arm/cpu/armv7/start.S
> > @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
> >   *
> >
>  */
> >  WEAK(save_boot_params)
> > +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
> > +   ldr r12, =saved_args
> > +   /* Intentionally swapping r0 with r2 */
>
> That isn't very helpful. It's obvious what the code does here. Add a
> comment on *why* (to keep the c function simpler etc)
>
> Yes. I will update the comment.

[...]

Regards,
Raymond

Re: [PATCH v3 3/9] bloblist: refactor of bloblist_reloc()

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 03:33, Ilias Apalodimas 
wrote:

> Hi Raymond
>
> On Fri, 22 Dec 2023 at 23:31, Raymond Mao  wrote:
> >
> > The current bloblist pointer and size can be retrieved from global
> > data, so we don't need to pass them from the function arguments.
> > This change also help to remove all external access of gd->bloblist
> > outside of bloblist module.
> >
> > Signed-off-by: Raymond Mao 
> > ---
> > Changes in v2
> > - New patch file created for v2.
> > Changes in v3
> > - Check the space size before copying the bloblist.
> > - Add return code of bloblist_reloc().
> >
> >  common/bloblist.c  | 10 --
> >  common/board_f.c   |  8 ++--
> >  include/bloblist.h |  8 
> >  test/bloblist.c|  6 ++
> >  4 files changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index db3fbb20cf..5ad1db137a 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -472,13 +472,19 @@ void bloblist_show_list(void)
> > }
> >  }
> >
> > -void bloblist_reloc(void *to, uint to_size, void *from, uint from_size)
> > +int bloblist_reloc(void *to, uint to_size)
> >  {
> > struct bloblist_hdr *hdr;
> >
> > -   memcpy(to, from, from_size);
> > +   if (to_size < gd->bloblist->total_size)
> > +   return -ENOSPC;
> > +
> > +   memcpy(to, gd->bloblist, gd->bloblist->total_size);
> > hdr = to;
> > hdr->total_size = to_size;
> > +   gd->bloblist = to;
> > +
> > +   return 0;
> >  }
> >
> >  int bloblist_init(void)
> > diff --git a/common/board_f.c b/common/board_f.c
> > index d4d7d01f8f..00b0430889 100644
> > --- a/common/board_f.c
> > +++ b/common/board_f.c
> > @@ -676,13 +676,9 @@ static int reloc_bloblist(void)
> > return 0;
> > }
> > if (gd->new_bloblist) {
> > -   int size = CONFIG_BLOBLIST_SIZE;
> > -
> > debug("Copying bloblist from %p to %p, size %x\n",
> > - gd->bloblist, gd->new_bloblist, size);
> > -   bloblist_reloc(gd->new_bloblist,
> CONFIG_BLOBLIST_SIZE_RELOC,
> > -  gd->bloblist, size);
> > -   gd->bloblist = gd->new_bloblist;
> > + gd->bloblist, gd->new_bloblist,
> gd->bloblist->total_size);
> > +   bloblist_reloc(gd->new_bloblist,
> CONFIG_BLOBLIST_SIZE_RELOC);
>
> Why aren't we checking bloblist_reloc() for an error?
>
> Yes. This should be checked as well here. Will update in v4.

[...]

Regards,
Raymond

Re: [PATCH v3 9/9] qemu-arm: get FDT from bloblist

[Raymond Mao]

Hi Ilias and Simon,

On Wed, 27 Dec 2023 at 04:32, Ilias Apalodimas 
wrote:

> On Tue, 26 Dec 2023 at 11:48, Simon Glass  wrote:
> >
> > Hi,
> >
> > On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao 
> wrote:
> > >
> > > Get devicetree from a bloblist if it exists.
> > > If not, fallback to get FDT from the specified memory address.
> > >
> > > Signed-off-by: Raymond Mao 
> > > ---
> > > Changes in v2
> > > - Refactor of board_fdt_blob_setup().
> > >
> > >  board/emulation/qemu-arm/qemu-arm.c | 12 ++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> >
> > I believe this patch will not be needed, since we have fdtdec_setup()
> > to do this.
>
> Yep, it's not. The board-specific portion should be limited to what
> happens if the bloblist is *not* detected by fdtdec_setup() (assuming
> the user wants to allow a fallback)
>
> But I think whether we allow a fallback or not is really something that
should be
considered by a board vendor other than a common logic in fdtdec_setup().
And a board vendor can also decide whether to allow a FDT that comes from
bloblist
by this custom function.

[...]

Regards,
Raymond

Re: [PATCH v3 06/14] bloblist: Drop spare value from bloblist record

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 04:43, Ilias Apalodimas 
wrote:

> Hi Raymond,
>
> On Mon, 18 Dec 2023 at 20:20, Raymond Mao  wrote:
> >
> > From: Simon Glass 
> >
> > Drop spare value from bloblist record header.
> >
> > For now it is still present in the header, with an underscore, so that
> > tests continue to pass.
>
> I am not sure I understand the commit message. Doesn't the spec define
> this as 'reserved' now?
> On top of that, it's a better idea to follow the spec naming. It's
> easier to map the spec -> code for new readers. So I would rename
>
> tag -> tag_id
> size -> data_size and
> flags -> reserved.
>
> Can we at least do the flags -> reserved in this patch?
>
> I will update the naming of members in v4.

Regards,
Raymond

Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 17:09, Raymond Mao  wrote:
>
> Hi Ilias,
>
> On Wed, 27 Dec 2023 at 09:58, Ilias Apalodimas  
> wrote:
>>
>> On Wed, 27 Dec 2023 at 16:50, Raymond Mao  wrote:
>> >
>> > Hi Ilias,
>> >
>> > On Wed, 27 Dec 2023 at 05:17, Ilias Apalodimas 
>> >  wrote:
>> >>
>> >>
>> >> [...]
>> >>
>> >> > - * @chksum: checksum for the entire bloblist allocated area. Since any 
>> >> > of the
>> >> > - *   blobs can be altered after being created, this checksum is only 
>> >> > valid
>> >> > - *   when the bloblist is finalised before jumping to the next stage 
>> >> > of boot.
>> >> > - *   This is the value needed to make all checksummed bytes sum to 0
>> >> >   */
>> >> >  struct bloblist_hdr {
>> >> >   u32 magic;
>> >> > - u32 version;
>> >> > - u32 hdr_size;
>> >> > - u32 flags;
>> >> > -
>> >> > - u32 size;
>> >> > + u8 chksum;
>> >> > + u8 version;
>> >> > + u8 hdr_size;
>> >> > + u8 align_log2;
>> >> >   u32 alloced;
>> >> > + u32 size;
>> >> > + u32 flags;
>> >> >   u32 spare;
>> >> > - u32 chksum;
>> >> >  };
>> >> >
>> >>
>> >> Aren't fields still missing from the current version?
>> >> e.g max_size and reserved?
>> >>
>> > They are all in. Please see:
>> > [PATCH v3 14/14] bloblist: Align bloblist used_size and total_size to spec
>>
>> Ok. But why aren't we squashing this to #14 then?
>>
> I can squash other changes with #14 except the ones for `align_log2`.
> And `align_log2` with patch #12 due to the dependency.

I think that's better.

Thanks
/Ilias
>
> Regards,
> Raymond
>
>>
>> Thanks
>> /Ilias
>> >
>> > [...]
>> >
>> > Regards,
>> > Raymond

Re: [PATCH v3 01/14] bloblist: Update the tag numbering

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 04:48, Ilias Apalodimas 
wrote:

> Hi Raymond,
>
> On Mon, 18 Dec 2023 at 20:19, Raymond Mao  wrote:
> >
> > From: Simon Glass 
> >
> > Align bloblist tags with the FW handoff spec v0.9.
> > The most common ones are from 0.
> > TF related ones are from 0x100.
> > All non-standard ones from 0xfff000.
> >
> > Added new defined tags:
> > BLOBLISTT_OPTEE_PAGABLE_PART for TF.
> > BLOBLISTT_TPM_EVLOG and BLOBLISTT_TPM_CRB_BASE for TPM.
> >
> > Signed-off-by: Simon Glass 
> > Co-developed-by: Raymond Mao 
> > Signed-off-by: Raymond Mao 
> > ---
> > Changes in v2
> > - Align bloblist tags to FW handoff spec v0.9.
> > Changes in v3
> > - Add TPM related tags
> >
> >  common/bloblist.c  | 18 ++---
> >  include/bloblist.h | 67 +-
> >  test/bloblist.c|  4 +--
> >  3 files changed, 52 insertions(+), 37 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index a22f6c12b0..5606487f5b 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -36,16 +36,26 @@ static struct tag_name {
> > enum bloblist_tag_t tag;
> > const char *name;
> >  } tag_name[] = {
> > -   { BLOBLISTT_NONE, "(none)" },
> > +   { BLOBLISTT_VOID, "(void)" },
> >
> > /* BLOBLISTT_AREA_FIRMWARE_TOP */
> > +   { BLOBLISTT_CONTROL_FDT, "Control FDT" },
> > +   { BLOBLISTT_HOB_BLOCK, "HOB block" },
> > +   { BLOBLISTT_HOB_LIST, "HOB list" },
> > +   { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
> > +   { BLOBLISTT_TPM_EVLOG, "TPM event log defined by TCG EFI" },
> > +   { BLOBLISTT_TPM_CRB_BASE, "TPM Command Response Buffer address"
> },
> >
> > /* BLOBLISTT_AREA_FIRMWARE */
> > -   { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" },
> > -   { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" },
> > { BLOBLISTT_TPM2_TCG_LOG, "TPM v2 log space" },
> > { BLOBLISTT_TCPA_LOG, "TPM log space" },
> > -   { BLOBLISTT_ACPI_TABLES, "ACPI tables for x86" },
> > +   { BLOBLISTT_ACPI_GNVS, "ACPI GNVS" },
> > +
> > +   /* BLOBLISTT_AREA_TF */
> > +   { BLOBLISTT_OPTEE_PAGABLE_PART, "OP-TEE pagable part" },
> > +
> > +   /* BLOBLISTT_AREA_OTHER */
> > +   { BLOBLISTT_INTEL_VBT, "Intel Video-BIOS table" },
> > { BLOBLISTT_SMBIOS_TABLES, "SMBIOS tables for x86" },
> > { BLOBLISTT_VBOOT_CTX, "Chrome OS vboot context" },
> >
> > diff --git a/include/bloblist.h b/include/bloblist.h
> > index 080cc46a12..92dbfda21b 100644
> > --- a/include/bloblist.h
> > +++ b/include/bloblist.h
> > @@ -81,7 +81,7 @@ enum {
> >
> >  /* Supported tags - add new ones to tag_name in bloblist.c */
> >  enum bloblist_tag_t {
> > -   BLOBLISTT_NONE = 0,
> > +   BLOBLISTT_VOID = 0,
> >
> > /*
> >  * Standard area to allocate blobs used across firmware
> components, for
> > @@ -89,42 +89,36 @@ enum bloblist_tag_t {
> >  * projects.
> >  */
> > BLOBLISTT_AREA_FIRMWARE_TOP = 0x1,
> > +   /*
> > +* Devicetree for use by firmware. On some platforms this is
> passed to
> > +* the OS also
> > +*/
> > +   BLOBLISTT_CONTROL_FDT = 1,
> > +   BLOBLISTT_HOB_BLOCK = 2,
> > +   BLOBLISTT_HOB_LIST = 3,
> > +   BLOBLISTT_ACPI_TABLES = 4,
> > +   BLOBLISTT_TPM_EVLOG = 5,
> > +   BLOBLISTT_TPM_CRB_BASE = 6,
> >
> > /* Standard area to allocate blobs used across firmware
> components */
> > -   BLOBLISTT_AREA_FIRMWARE = 0x100,
> > +   BLOBLISTT_AREA_FIRMWARE = 0x10,
> > +   BLOBLISTT_TPM2_TCG_LOG = 0x10,  /* TPM v2 log space */
> > +   BLOBLISTT_TCPA_LOG = 0x11,  /* TPM log space */
> > /*
> >  * Advanced Configuration and Power Interface Global Non-Volatile
> >  * Sleeping table. This forms part of the ACPI tables passed to
> Linux.
> >  */
> > -   BLOBLISTT_ACPI_GNVS = 0x100,
> > -   BLOBLISTT_INTEL_VBT = 0x101,/* Intel Video-BIOS table */
> > -   BLOBLISTT_TPM2_TCG_LOG = 0x102, /* TPM v2 log space */
> > -   BLOBLISTT_TCPA_LOG = 0x103, /* TPM log space */
> > -   BLOBLISTT_ACPI_TABLES = 0x104,  /* ACPI tables for x86 */
> > -   BLOBLISTT_SMBIOS_TABLES = 0x105, /* SMBIOS tables for x86 */
> > -   BLOBLISTT_VBOOT_CTX = 0x106,/* Chromium OS verified boot
> context */
> > +   BLOBLISTT_ACPI_GNVS = 0x12,
>
> Any idea if we have users that would be affected by this value change?
>
> BLOBLISTT_ACPI_TABLES, BLOBLISTT_TPM2_TCG_LOG and BLOBLISTT_ACPI_TABLES
are being used by x86.
BLOBLISTT_ACPI_GNVS is being used by x86 and chromebook.

Regards,
Raymond

Re: [PATCH v3 02/14] bloblist: Adjust API to align in powers of 2

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 04:53, Ilias Apalodimas 
wrote:

> Hi,
>
> On Mon, 18 Dec 2023 at 20:19, Raymond Mao  wrote:
> >
> > From: Simon Glass 
> >
> > The updated bloblist structure stores the alignment as a power-of-two
> > value in its structures. Adjust the API to use this, to avoid needing to
> > calling ilog2().
> >
> > Drop a stale comment while we are here.
>
> The patch does more than what the description suggests. So either the
> new 8b alignment is wrong or the commit message needs and update and
> explain why we are changing this.
>
> The 8-byte-alignment is correct and matching with the spec.
I will update the commit message.

[...]

Regards,
Raymond

Re: [PATCH v3 08/14] bloblist: Checksum the entire bloblist

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 04:57, Ilias Apalodimas 
wrote:

> Hi Raymond,
>
> On Mon, 18 Dec 2023 at 20:20, Raymond Mao  wrote:
> >
> > From: Simon Glass 
> >
> > Spec v0.9 specifies that the entire bloblist area is checksummed,
> > including unused portions. Update the code to follow this.
> >
> > Signed-off-by: Simon Glass 
> > Co-developed-by: Raymond Mao 
> > Signed-off-by: Raymond Mao 
> > Reviewed-by: Simon Glass 
> > ---
> >  common/bloblist.c |  9 +
> >  test/bloblist.c   | 10 --
> >  2 files changed, 9 insertions(+), 10 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index 32692d8319..705d9c6ae9 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -319,17 +319,10 @@ int bloblist_resize(uint tag, int new_size)
> >
> >  static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
> >  {
> > -   struct bloblist_rec *rec;
> > u8 chksum;
> >
> > -   chksum = table_compute_checksum(hdr, hdr->hdr_size);
> > +   chksum = table_compute_checksum(hdr, hdr->alloced);
> > chksum += hdr->chksum;
> > -   foreach_rec(rec, hdr) {
> > -   chksum -= table_compute_checksum((void *)rec,
> > -rec_hdr_size(rec));
> > -   chksum -= table_compute_checksum((void *)rec +
> > -rec_hdr_size(rec),
> rec->size);
> > -   }
>
> Why do we need patch #7 if it gets removed immediately after?
> Can't we just squash those two with a proper commit message, since the
> spec requires checksumming the entire list?
>
> Yes. I think #7 and #8 can be squashed into one patch.
Will do this in V4.

Regards,
Raymond

Re: [PATCH v2 2/2] efi_loader: support fmp versioning for multi bank update

[Ilias Apalodimas]

Kojima-san

On Thu, Dec 21, 2023 at 06:52:58PM +0900, Masahisa Kojima wrote:
> This commit stores the firmware version into the array
> of fmp_state structure to support the fmp versioning
> for multi bank update. The index of the array is identified
> by the bank index.

Why do we all of them? Can't we just always store/use the version of the active
bank?

>
> This modification keeps the backward compatibility with
> the existing versioning feature.
>

Thanks
/Ilias
> Signed-off-by: Masahisa Kojima 
> ---
>  lib/efi_loader/efi_firmware.c | 69 +++
>  1 file changed, 54 insertions(+), 15 deletions(-)
>
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 9b8630625f..5459f3d38d 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -207,18 +207,10 @@ void efi_firmware_fill_version_info(struct 
> efi_firmware_image_descriptor *image_
>  {
>   u16 varname[13]; /* u"FmpState" */
>   efi_status_t ret;
> - efi_uintn_t size;
> - struct fmp_state var_state = { 0 };
> -
> - efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> - fw_array->image_index);
> - size = sizeof(var_state);
> - ret = efi_get_variable_int(varname, _array->image_type_id,
> -NULL, , _state, NULL);
> - if (ret == EFI_SUCCESS)
> - image_info->version = var_state.fw_version;
> - else
> - image_info->version = 0;
> + efi_uintn_t size, expected_size;
> + uint num_banks = 1;
> + uint active_index = 0;
> + struct fmp_state *var_state;
>
>   efi_firmware_get_lsv_from_dtb(fw_array->image_index,
> _array->image_type_id,
> @@ -227,6 +219,31 @@ void efi_firmware_fill_version_info(struct 
> efi_firmware_image_descriptor *image_
>   image_info->version_name = NULL; /* not supported */
>   image_info->last_attempt_version = 0;
>   image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS;
> + image_info->version = 0;
> +
> + /* get the fw_version */
> + efi_create_indexed_name(varname, sizeof(varname), "FmpState",
> + fw_array->image_index);
> + if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> + ret = fwu_get_active_index(_index);
> + if (ret)
> + return;
> +
> + num_banks = CONFIG_FWU_NUM_BANKS;
> + }
> +
> + size = num_banks * sizeof(*var_state);
> + expected_size = size;
> + var_state = calloc(1, size);
> + if (!var_state)
> + return;
> +
> + ret = efi_get_variable_int(varname, _array->image_type_id,
> +NULL, , var_state, NULL);
> + if (ret == EFI_SUCCESS && expected_size == size)
> + image_info->version = var_state[active_index].fw_version;
> +
> + free(var_state);
>  }
>
>  /**
> @@ -362,8 +379,11 @@ efi_status_t efi_firmware_set_fmp_state_var(struct 
> fmp_state *state, u8 image_in
>  {
>   u16 varname[13]; /* u"FmpState" */
>   efi_status_t ret;
> + uint num_banks = 1;
> + uint update_bank = 0;
> + efi_uintn_t size;
>   efi_guid_t *image_type_id;
> - struct fmp_state var_state = { 0 };
> + struct fmp_state *var_state;
>
>   image_type_id = efi_firmware_get_image_type_id(image_index);
>   if (!image_type_id)
> @@ -372,19 +392,38 @@ efi_status_t efi_firmware_set_fmp_state_var(struct 
> fmp_state *state, u8 image_in
>   efi_create_indexed_name(varname, sizeof(varname), "FmpState",
>   image_index);
>
> + if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> + ret = fwu_plat_get_update_index(_bank);
> + if (ret)
> + return EFI_INVALID_PARAMETER;
> +
> + num_banks = CONFIG_FWU_NUM_BANKS;
> + }
> +
> + size = num_banks * sizeof(*var_state);
> + var_state = calloc(1, size);
> + if (!var_state)
> + return EFI_OUT_OF_RESOURCES;
> +
> + ret = efi_get_variable_int(varname, image_type_id, NULL, ,
> +var_state, NULL);
> +
>   /*
>* Only the fw_version is set here.
>* lowest_supported_version in FmpState variable is ignored since
>* it can be tampered if the file based EFI variable storage is used.
>*/
> - var_state.fw_version = state->fw_version;
> + var_state[update_bank].fw_version = state->fw_version;
>
> + size = num_banks * sizeof(*var_state);
>   ret = efi_set_variable_int(varname, image_type_id,
>  EFI_VARIABLE_READ_ONLY |
>  EFI_VARIABLE_NON_VOLATILE |
>  EFI_VARIABLE_BOOTSERVICE_ACCESS |
>  EFI_VARIABLE_RUNTIME_ACCESS,
> -sizeof(var_state), _state, false);
> +  

Re: [PATCH v3 12/14] bloblist: Add alignment to bloblist_new()

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 05:10, Ilias Apalodimas 
wrote:

> On Mon, 18 Dec 2023 at 20:20, Raymond Mao  wrote:
> >
> > From: Simon Glass 
> >
> > Allow the alignment to be specified when creating a bloblist.
> >
> > Signed-off-by: Simon Glass 
> > Co-developed-by: Raymond Mao 
> > Signed-off-by: Raymond Mao 
> > ---
> > Changes in v3
> > - Keep the flag argument to align to FW handoff spec up to commit
> 3592349.
> >
> >  common/bloblist.c  |  5 +++--
> >  include/bloblist.h |  3 ++-
> >  test/bloblist.c| 40 ++--
> >  3 files changed, 27 insertions(+), 21 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index 1c97d61e4a..6d079c58f7 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -349,7 +349,7 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr
> *hdr)
> > return chksum;
> >  }
> >
> > -int bloblist_new(ulong addr, uint size, uint flags)
> > +int bloblist_new(ulong addr, uint size, uint flags, uint align_log2)
> >  {
> > struct bloblist_hdr *hdr;
> >
> > @@ -365,6 +365,7 @@ int bloblist_new(ulong addr, uint size, uint flags)
> > hdr->magic = BLOBLIST_MAGIC;
> > hdr->size = size;
> > hdr->alloced = hdr->hdr_size;
> > +   hdr->align_log2 = align_log2 ?: BLOBLIST_BLOB_ALIGN_LOG2;
>
> nit, but can we write this as a full ternary operator in the next release.
>
> Yes, I agree. Will change this in V4.

Regards,
Raymond

Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 09:58, Ilias Apalodimas 
wrote:

> On Wed, 27 Dec 2023 at 16:50, Raymond Mao  wrote:
> >
> > Hi Ilias,
> >
> > On Wed, 27 Dec 2023 at 05:17, Ilias Apalodimas <
> ilias.apalodi...@linaro.org> wrote:
> >>
> >>
> >> [...]
> >>
> >> > - * @chksum: checksum for the entire bloblist allocated area. Since
> any of the
> >> > - *   blobs can be altered after being created, this checksum is only
> valid
> >> > - *   when the bloblist is finalised before jumping to the next stage
> of boot.
> >> > - *   This is the value needed to make all checksummed bytes sum to 0
> >> >   */
> >> >  struct bloblist_hdr {
> >> >   u32 magic;
> >> > - u32 version;
> >> > - u32 hdr_size;
> >> > - u32 flags;
> >> > -
> >> > - u32 size;
> >> > + u8 chksum;
> >> > + u8 version;
> >> > + u8 hdr_size;
> >> > + u8 align_log2;
> >> >   u32 alloced;
> >> > + u32 size;
> >> > + u32 flags;
> >> >   u32 spare;
> >> > - u32 chksum;
> >> >  };
> >> >
> >>
> >> Aren't fields still missing from the current version?
> >> e.g max_size and reserved?
> >>
> > They are all in. Please see:
> > [PATCH v3 14/14] bloblist: Align bloblist used_size and total_size to
> spec
>
> Ok. But why aren't we squashing this to #14 then?
>
> I can squash other changes with #14 except the ones for `align_log2`.
And `align_log2` with patch #12 due to the dependency.

Regards,
Raymond


> Thanks
> /Ilias
> >
> > [...]
> >
> > Regards,
> > Raymond
>

Re: [PATCH v3 09/14] bloblist: Handle alignment with a void entry

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 04:58, Ilias Apalodimas 
wrote:

> On Mon, 18 Dec 2023 at 20:20, Raymond Mao  wrote:
> >
> > From: Simon Glass 
> >
> > Rather than setting the alignment using the header size, add an entirely
> > new entry to cover the gap left by the alignment.
>
> Hmm, why? Does it make out life easier somehow if new entries get added?
>
> Yes, currently we just mark the unused or removed areas with a void tag.
But I think this is beneficial because later on we can support the
fragments management
by using this tag to collect, merge and re-use the fragmented spaces.

Regards,
Raymond

>
> >
> > Signed-off-by: Simon Glass 
> > Co-developed-by: Raymond Mao 
> > Signed-off-by: Raymond Mao 
> > Reviewed-by: Simon Glass 
> > ---
> >  common/bloblist.c | 23 +++
> >  1 file changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index 705d9c6ae9..73dbbc01c0 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -142,7 +142,7 @@ static int bloblist_addrec(uint tag, int size, int
> align_log2,
> >  {
> > struct bloblist_hdr *hdr = gd->bloblist;
> > struct bloblist_rec *rec;
> > -   int data_start, new_alloced;
> > +   int data_start, aligned_start, new_alloced;
> >
> > if (!align_log2)
> > align_log2 = BLOBLIST_ALIGN_LOG2;
> > @@ -151,10 +151,25 @@ static int bloblist_addrec(uint tag, int size, int
> align_log2,
> > data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec);
> >
> > /* Align the address and then calculate the offset from
> ->alloced */
> > -   data_start = ALIGN(data_start, 1U << align_log2) -
> map_to_sysmem(hdr);
> > +   aligned_start = ALIGN(data_start, 1U << align_log2) - data_start;
> > +
> > +   /* If we need to create a dummy record, create it */
> > +   if (aligned_start) {
> > +   int void_size = aligned_start - sizeof(*rec);
> > +   struct bloblist_rec *vrec;
> > +   int ret;
> > +
> > +   ret = bloblist_addrec(BLOBLISTT_VOID, void_size, 0,
> );
> > +   if (ret)
> > +   return log_msg_ret("void", ret);
> > +
> > +   /* start the record after that */
> > +   data_start = map_to_sysmem(hdr) + hdr->alloced +
> sizeof(*vrec);
> > +   }
> >
> > /* Calculate the new allocated total */
> > -   new_alloced = data_start + ALIGN(size, 1U << align_log2);
> > +   new_alloced = data_start - map_to_sysmem(hdr) +
> > +   ALIGN(size, 1U << align_log2);
> >
> > if (new_alloced > hdr->size) {
> > log_err("Failed to allocate %x bytes size=%x, need
> size=%x\n",
> > @@ -164,7 +179,7 @@ static int bloblist_addrec(uint tag, int size, int
> align_log2,
> > rec = (void *)hdr + hdr->alloced;
> >
> > rec->tag = tag;
> > -   rec->hdr_size = data_start - hdr->alloced;
> > +   rec->hdr_size = sizeof(struct bloblist_rec);
> > rec->size = size;
> >
> > /* Zero the record data */
> > --
> > 2.25.1
> >
>

Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 16:50, Raymond Mao  wrote:
>
> Hi Ilias,
>
> On Wed, 27 Dec 2023 at 05:17, Ilias Apalodimas  
> wrote:
>>
>>
>> [...]
>>
>> > - * @chksum: checksum for the entire bloblist allocated area. Since any of 
>> > the
>> > - *   blobs can be altered after being created, this checksum is only valid
>> > - *   when the bloblist is finalised before jumping to the next stage of 
>> > boot.
>> > - *   This is the value needed to make all checksummed bytes sum to 0
>> >   */
>> >  struct bloblist_hdr {
>> >   u32 magic;
>> > - u32 version;
>> > - u32 hdr_size;
>> > - u32 flags;
>> > -
>> > - u32 size;
>> > + u8 chksum;
>> > + u8 version;
>> > + u8 hdr_size;
>> > + u8 align_log2;
>> >   u32 alloced;
>> > + u32 size;
>> > + u32 flags;
>> >   u32 spare;
>> > - u32 chksum;
>> >  };
>> >
>>
>> Aren't fields still missing from the current version?
>> e.g max_size and reserved?
>>
> They are all in. Please see:
> [PATCH v3 14/14] bloblist: Align bloblist used_size and total_size to spec

Ok. But why aren't we squashing this to #14 then?

Thanks
/Ilias
>
> [...]
>
> Regards,
> Raymond

Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header

[Raymond Mao]

Hi Ilias,

On Wed, 27 Dec 2023 at 05:17, Ilias Apalodimas 
wrote:

>
> [...]
>
> > - * @chksum: checksum for the entire bloblist allocated area. Since any
> of the
> > - *   blobs can be altered after being created, this checksum is only
> valid
> > - *   when the bloblist is finalised before jumping to the next stage of
> boot.
> > - *   This is the value needed to make all checksummed bytes sum to 0
> >   */
> >  struct bloblist_hdr {
> >   u32 magic;
> > - u32 version;
> > - u32 hdr_size;
> > - u32 flags;
> > -
> > - u32 size;
> > + u8 chksum;
> > + u8 version;
> > + u8 hdr_size;
> > + u8 align_log2;
> >   u32 alloced;
> > + u32 size;
> > + u32 flags;
> >   u32 spare;
> > - u32 chksum;
> >  };
> >
>
> Aren't fields still missing from the current version?
> e.g max_size and reserved?
>
> They are all in. Please see:
[PATCH v3 14/14] bloblist: Align bloblist used_size and total_size to spec

[...]

Regards,
Raymond

Re: [PATCH v2 1/2] fwu: fix fwu_get_image_index interface

[Ilias Apalodimas]

On Thu, 21 Dec 2023 at 11:54, Masahisa Kojima
 wrote:
>
> The capsule update uses the DFU framework for updating
> storage. fwu_get_image_index() currently returns the
> image_index calculated by (dfu_alt_num + 1), but this is
> different from the image_index in UEFI terminology.
>
> Since capsule update implementation calls dfu_write_by_alt
> function, it is better that FWU returns the dfu_alt_num.
>
> Signed-off-by: Masahisa Kojima 
> ---
>  include/fwu.h | 13 +
>  lib/efi_loader/efi_firmware.c | 11 +--
>  lib/fwu_updates/fwu.c | 32 
>  3 files changed, 26 insertions(+), 30 deletions(-)
>
> diff --git a/include/fwu.h b/include/fwu.h
> index ac5c5de870..eb5638f4f3 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -122,21 +122,18 @@ int fwu_get_active_index(uint *active_idxp);
>  int fwu_set_active_index(uint active_idx);
>
>  /**
> - * fwu_get_image_index() - Get the Image Index to be used for capsule update
> - * @image_index: The Image Index for the image
> - *
> - * The FWU multi bank update feature computes the value of image_index at
> - * runtime, based on the bank to which the image needs to be written to.
> - * Derive the image_index value for the image.
> + * fwu_get_dfu_alt_num() - Get the dfu_alt_num to be used for capsule update
> + * @image_index:   The Image Index for the image
> + * @alt_num:   pointer to store dfu_alt_num
>   *
>   * Currently, the capsule update driver uses the DFU framework for
>   * the updates. This function gets the DFU alt number which is to
> - * be used as the Image Index
> + * be used for capsule update.
>   *
>   * Return: 0 if OK, -ve on error
>   *
>   */
> -int fwu_get_image_index(u8 *image_index);
> +int fwu_get_dfu_alt_num(u8 image_index, u8 *alt_num);
>
>  /**
>   * fwu_revert_boot_index() - Revert the active index in the FWU metadata
> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
> index 9abb29f1df..9b8630625f 100644
> --- a/lib/efi_loader/efi_firmware.c
> +++ b/lib/efi_loader/efi_firmware.c
> @@ -611,6 +611,7 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> u16 **abort_reason)
>  {
> int ret;
> +   u8 dfu_alt_num;
> efi_status_t status;
> struct fmp_state state = { 0 };
>
> @@ -625,19 +626,25 @@ efi_status_t EFIAPI efi_firmware_raw_set_image(
> if (status != EFI_SUCCESS)
> return EFI_EXIT(status);
>
> +   /*
> +* dfu_alt_num is assigned from 0 while image_index starts from 1.
> +* dfu_alt_num is calculated by (image_index - 1) when multi bank 
> update
> +* is not used.
> +*/
> +   dfu_alt_num = image_index - 1;
> if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
> /*
>  * Based on the value of update bank, derive the
>  * image index value.
>  */
> -   ret = fwu_get_image_index(_index);
> +   ret = fwu_get_dfu_alt_num(image_index, _alt_num);
> if (ret) {
> log_debug("Unable to get FWU image_index\n");
> return EFI_EXIT(EFI_DEVICE_ERROR);
> }
> }
>
> -   if (dfu_write_by_alt(image_index - 1, (void *)image, image_size,
> +   if (dfu_write_by_alt(dfu_alt_num, (void *)image, image_size,
>  NULL, NULL))
> return EFI_EXIT(EFI_DEVICE_ERROR);
>
> diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
> index b580574015..86518108c2 100644
> --- a/lib/fwu_updates/fwu.c
> +++ b/lib/fwu_updates/fwu.c
> @@ -125,16 +125,14 @@ static int in_trial_state(struct fwu_mdata *mdata)
> return 0;
>  }
>
> -static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
> +static int fwu_get_image_type_id(u8 image_index, efi_guid_t *image_type_id)
>  {
> -   u8 index;
> int i;
> struct efi_fw_image *image;
>
> -   index = *image_index;
> image = update_info.images;
> for (i = 0; i < update_info.num_images; i++) {
> -   if (index == image[i].image_index) {
> +   if (image_index == image[i].image_index) {
> guidcpy(image_type_id, [i].image_type_id);
> return 0;
> }
> @@ -332,24 +330,20 @@ int fwu_set_active_index(uint active_idx)
>  }
>
>  /**
> - * fwu_get_image_index() - Get the Image Index to be used for capsule update
> - * @image_index: The Image Index for the image
> - *
> - * The FWU multi bank update feature computes the value of image_index at
> - * runtime, based on the bank to which the image needs to be written to.
> - * Derive the image_index value for the image.
> + * fwu_get_dfu_alt_num() - Get the dfu_alt_num to be used for capsule update
> + * @image_index:   The Image Index for the image
> + * @alt_num:   pointer to store dfu_alt_num
>   *
>   * 

Re: [PATCH 5/5] test: unit test for smbios command

[Ilias Apalodimas]

On Sat, Dec 23, 2023 at 01:44:29AM +0100, Heinrich Schuchardt wrote:
> Provide a unit test for the smbios command.
>
> Provide different test functions for QEMU, sandbox, and other systems.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  test/py/tests/test_smbios.py | 47 
>  1 file changed, 47 insertions(+)
>  create mode 100644 test/py/tests/test_smbios.py
>
> diff --git a/test/py/tests/test_smbios.py b/test/py/tests/test_smbios.py
> new file mode 100644
> index 00..86d8d07539
> --- /dev/null
> +++ b/test/py/tests/test_smbios.py
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +"""Test smbios command"""
> +
> +import pytest
> +
> +@pytest.mark.buildconfigspec('cmd_smbios')
> +@pytest.mark.notbuildconfigspec('qfw_smbios')
> +@pytest.mark.notbuildconfigspec('sandbox')
> +def test_cmd_smbios(u_boot_console):
> +"""Run the smbios command"""
> +output = u_boot_console.run_command('smbios')
> +assert 'DMI type 0,' in output
> +assert 'String 1: U-Boot' in output
> +assert 'DMI type 1,' in output
> +assert 'DMI type 2,' in output
> +assert 'DMI type 3,' in output
> +assert 'DMI type 4,' in output
> +assert 'DMI type 127,' in output
> +
> +@pytest.mark.buildconfigspec('cmd_smbios')
> +@pytest.mark.buildconfigspec('qfw_smbios')
> +@pytest.mark.notbuildconfigspec('sandbox')
> +# TODO:
> +# QEMU v8.2.0 lacks SMBIOS support for RISC-V
> +# Once support is available in our Docker image we can remove the constraint.
> +@pytest.mark.notbuildconfigspec('riscv')
> +def test_cmd_smbios_qemu(u_boot_console):
> +"""Run the smbios command on QEMU"""
> +output = u_boot_console.run_command('smbios')
> +assert 'DMI type 1,' in output
> +assert 'Manufacturer: QEMU' in output
> +assert 'DMI type 127,' in output
> +
> +@pytest.mark.buildconfigspec('cmd_smbios')
> +@pytest.mark.buildconfigspec('sandbox')
> +def test_cmd_smbios_sandbox(u_boot_console):
> +"""Run the smbios command on the sandbox"""
> +output = u_boot_console.run_command('smbios')
> +assert 'DMI type 0,' in output
> +assert 'String 1: U-Boot' in output
> +assert 'DMI type 1,' in output
> +assert 'Manufacturer: sandbox' in output
> +assert 'DMI type 2,' in output
> +assert 'DMI type 3,' in output
> +assert 'DMI type 4,' in output
> +assert 'DMI type 127,' in output
> --
> 2.43.0
>

Acked-by: Ilias Apalodimas 

Re: [PATCH 2/5] smbios: type2: contained object handles

[Ilias Apalodimas]

On Sat, Dec 23, 2023 at 01:44:26AM +0100, Heinrich Schuchardt wrote:
> The type 2 structure must include information about the contained objects.
> It is fine to set the number of contained object handles to 0.
>
> Add the missing field.
>
> Fixes: 721e992a8af5 ("x86: Add SMBIOS table support")
> Signed-off-by: Heinrich Schuchardt 
> ---
>  include/smbios.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/smbios.h b/include/smbios.h
> index e601283d29..88c19ae062 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -139,6 +139,7 @@ struct __packed smbios_type2 {
>   u8 chassis_location;
>   u16 chassis_handle;
>   u8 board_type;
> + u8 number_contained_objects;
>   char eos[SMBIOS_STRUCT_EOS_BYTES];
>  };
>
> --
> 2.43.0
>
Reviewed-by: Ilias Apalodimas 

Re: Adding EFI runtime support to the Arm's FF-A bus

[Ilias Apalodimas]

[...]

> > > > > > I know. Last time I checked CentOS (or was it Ubuntu?) tried to
> > > > > > set EFI variables and the installer just failed. Might be fixed now,
> > > > > > though.
> > > > >
> > > > > It's not. The error message is less scary in distros nowadays, but
> > > > > setting the variable for Boot is still not working. That being
> > > > > said I have a plan to fix it for variables stored in a file, that's
> > > > > why we standardized the efi variable format in EBBR.
> > >
> > > Does this mean supporting SetVariableRT for files ?
> > > If yes, would it work without the driver model in runtime?
> >
> > Yes.
> >
> > The reasoning here is pretty simple. You can't keep alive drivers etc
> > for devices that the *kernel* eventually owns. The proposal is to
> > ignore the EFI spec and teach the kernel to write those directly. We
> > already do that when the variables are stored in an RPMB, we need to
> > figure out a decent scalable architecture for the rest.
> >
> Thanks for explaining this.
> It would be a good idea to provide EFI var storage info like location,
> offset, maxsize to linux
> and linux can modify vars as required.
>
> Eg.
>
> location=espfile://u-boot-efi.vars offset=0 maxsize=-1
> or
> location=spi://0 offset=0x3D maxsize=0x
>

Yes, you need the flash, offset and size, but I as trying to figure out if
we hand over those as an EFI config table. I haven't managed to do any
coding though yet.

/Ilias
> Then the linux kernel is able to modify vars correctly.
> I think SPI might be simpler to implement vs file as there can be.
> many different
> ESP partitions on multiple drives or no ESP partition at all.
>
> Kind regards,
> Shantur
>
> > /Ilias
> > >
> > > Kind regards,
> > > Shantur

Re: [PATCH 0/8] An effort to bring DT bindings compliance within U-boot

[Tom Rini]

On Wed, Dec 27, 2023 at 07:56:26AM +, Simon Glass wrote:
> Hi Sumit,
> 
> On Tue, Dec 26, 2023 at 10:06 AM Sumit Garg  wrote:
> >
> > Hi Simon,
> >
> > On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
> > >
> > > Hi Sumit,
> > >
> > > On Fri, Dec 22, 2023 at 5:34 AM Sumit Garg  wrote:
> > > >
> > > > On Thu, 21 Dec 2023 at 20:48, Simon Glass  wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Thu, Dec 21, 2023 at 3:03 PM Tom Rini  wrote:
> > > > > >
> > > > > > On Thu, Dec 14, 2023 at 07:20:55PM +0530, Sumit Garg wrote:
> > > > > >
> > > > > > > Prerquisite
> > > > > > > ---
> > > > > > >
> > > > > > > This patch series requires devicetree-rebasing git repo to be 
> > > > > > > added as a
> > > > > > > subtree to the main U-boot repo via:
> > > > > > >
> > > > > > > $ git subtree add --prefix devicetree-rebasing \
> > > > > > >   
> > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
> > > > > > >  \
> > > > > > >   v6.6-dts --squash
> > > > > >
> > > > > > So, I've played with subtree a little and I think this is the right 
> > > > > > way
> > > > > > forward in these cases. If anyone wants to take a look at how this 
> > > > > > works
> > > > > > in practice, take a look at:
> > > > > > https://source.denx.de/u-boot/u-boot/-/commits/WIP/u-boot-with-devicetree-rebasing-since-v6.1/?ref_type=heads
> > > > > > In that tree I started with the v6.1-dts tag, sync'd all the 
> > > > > > configs (to
> > > > > > have an example of a normal commit) and then did a merge of each tag
> > > > > > until v6.6-dts, so provide some history. And git log looks like 
> > > > > > what I
> > > > > > want to see, the squash commit has clear references to what we are
> > > > > > getting and I make a merge commit that says what I did. If you pull 
> > > > > > the
> > > > > > tree and checkout the branch, all the code is right there already,
> > > > > > nothing further to do. Same with tarball releases. The only thing I
> > > > > > don't like is the size growth there, but we'll reclaim some of it 
> > > > > > when
> > > > > > we delete our obsolete bindings, and then obsolete dts files.
> > > > >
> > > > > I spent a bit of time with subtree as well, as part of reviewing this
> > > > > series, using the instructions Sumit provided. It seems OK to me. We
> > > > > have to accept that it adds code and there will be changes/churn, but
> > > > > it is not too different to accepting patches on those files within
> > > > > U-Boot. We will bring in files which U-Boot doesn't use, but U-Boot
> > > > > does support a good proportion of the boards supported by Linux, so I
> > > > > don't see that as a big cost.
> > > > >
> > > > > From my experimentation, subtrees seem to have no impact on buildman,
> > > > > which is great. Am I missing anything?
> > > >
> > > > No it shouldn't cause any regression on existing tools/CI/build
> > > > systems. It is just a version controlled way of importing third party
> > > > source code as a tarball.
> > > >
> > > > >
> > > > > I still worry about the board-level 'switch' between U-Boot DT and
> > > > > upstream ones. I believe that should be at the SoC level instead.
> > > > >
> > > >
> > > > Probably I wasn't clear enough in my earlier replies but this series
> > > > motivates for a SoC level switch only. Patch #7 is essentially a
> > > > switch for Amlogic meson-gxbb SoC and its derived boards.
> > >
> > > OK good...but that's not quite what I mean. You have a fragment like
> > > this in multiple defconfig files:
> > >
> > > +CONFIG_OF_UPSTREAM=y
> > > +CONFIG_DEFAULT_DEVICE_TREE="amlogic/meson-gxbb-odroidc2"
> > >
> > > instead, OF_UPSTREAM should be enabled via 'imply' for the SoC, in the
> > > Kconfig.
> >
> > Okay I got your point. It makes sense to me. I will do that for v3.
> >
> > > We should not have to specify the filename like this. It is
> > > laborious and error-prone.
> >
> > The only differentiation in naming here is just silicon vendor prefix
> > addition (amlogic/ in this case). The reason for this being that I
> > just want to mirror the whole silicon vendor directory from DT
> > rebasing rather than mirroring individual DT files. Given this do you
> > have any better alternatives?
> 
> My current opinion is that a better approach would be to move the
> files first (in the U-Boot tree). That would be easier if we could
> just copy the Linux arch/xxx/boot/dts Makefiles but for now that is
> not possible. The Kconfig options for each SoC are similar but there
> are various differences.
> 
> Having a different layout is just a pain and it will get worse, as
> people add new boards, since they need to know to add the correct
> directory.
> 
> I will see if I can devise a small series to point this in what I
> consider to be the right direction, in line with Linux, and the U-Boot
> commit 3284c8b8 ("dts: generate multiple device tree blobs").

I think we should just go with the approach Sumit has already taken, and
encourage 

Re: [PATCH] include: env: ti: default_findfdt: Follow the bootstd/distro conventions

[Nishanth Menon]

On 07:08-20231227, Nishanth Menon wrote:
> Distroboot and bootstd both mandate a findfdt variable pointing to the
> correct device tree blob. Current mechanism calls a find_fdt function
> to set this variable. We do not need a find_fdt command to set the
> environment variable to a single dtb. Simplify the default_findfdt to
> remove variable expansion while at it.
> 
> For legacy scripts depending on a TI convention of name_fdt, provide a
> find_fdt wrapper that behaves like before.
> 
> NOTE: As part of this change, we also drop the cooked up
> default_device_tree_arch default_device_tree_subarch variables.
> 
> Reported-by: Jonathan Humphreys 
> Signed-off-by: Nishanth Menon 
> ---
>  include/env/ti/default_findfdt.env | 13 -
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/include/env/ti/default_findfdt.env 
> b/include/env/ti/default_findfdt.env
> index a2b51dd923bb..1a1ab8406c9e 100644
> --- a/include/env/ti/default_findfdt.env
> +++ b/include/env/ti/default_findfdt.env
> @@ -1,12 +1,7 @@
> -default_device_tree=CONFIG_DEFAULT_DEVICE_TREE
> -default_device_tree_arch=ti
>  #ifdef CONFIG_ARM64
> -findfdt=
> - setenv name_fdt ${default_device_tree_arch}/${default_device_tree}.dtb;
> - setenv fdtfile ${name_fdt}
> +fdtfile=ti/CONFIG_DEFAULT_DEVICE_TREE.dtb;
>  #else
> -default_device_tree_subarch=omap
> -findfdt=
> - setenv name_fdt 
> ${default_device_tree_arch}/${default_device_tree_subarch}/${default_device_tree}.dtb;
> - setenv fdtfile ${name_fdt}
> +fdtfile=ti/omap/CONFIG_DEFAULT_DEVICE_TREE.dtb;
>  #endif
> +
> +findfdt=setenv name_fdt ${fdt_file}
> -- 
> 2.43.0
> 


This should have been RFC
Generates quotes


fdtfile=ti/"k3-am62a7-sk".dtb;

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH 00/26] sync am65x device tree with Linux v6.7-rc1

[Tom Rini]

On Wed, Dec 27, 2023 at 06:39:28AM -0600, Nishanth Menon wrote:
> On 15:00-20231221, Tom Rini wrote:
> > On Thu, Dec 21, 2023 at 11:43:46AM -0600, Bryan Brattlof wrote:
> > 
> > > Hello Everyone!
> > > 
> > > This series gets the am65x booting again along with syncing the device
> > > tree files with v6.7-rc1 Linux.
> > > 
> > > The bulk of these patches unify the WKUP SPL board file with the arm64
> > > files to make future syncs from Linux much easier. In the end the DTBs
> > > should look a lot like what the DTBs look like for the am64x which
> > > is fairly similar to the am65x.
> > > 
> > > For those interested in what UART boot looks like:
> > >https://paste.sr.ht/~bryanb/7df8a645dc548912cd806abd5ecab967ef3287bc
> > > 
> > > Thanks for reviewing and happy holidays
> > > ~Bryan
> > 
> > For the series,
> > Tested-by: Tom Rini 
> > 
> > Nishanth, does this path work for you?
> > 
> 
> Other than the minor comments provided in the relevant patches,
> 
> The following files also need sync up with v6.7-rc1.
> 
> k3-am65-iot2050-common-pg2.dtsi
> k3-am65-iot2050-common.dtsi
> k3-am6528-iot2050-basic-common.dtsi
> k3-am6548-iot2050-advanced-common.dtsi
> k3-am6548-iot2050-advanced-m2.dts
> 
> Jan - could you look at syncing those once an update to this series is
> posted? We could probably have a smoother v6.8-rc1 from then on.

I hope that by v6.8-rc1 time we'll have merged Sumit's patch series and
so instead TI platforms will move to the v6.7-dts tag there, and then
v6.8-dts once that's out.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 0/9] dts: Move to SoC-specific build rules

[Tom Rini]

On Wed, Dec 27, 2023 at 08:23:56AM +, Simon Glass wrote:

> U-Boot builds devicetree binaries from its source tree. As part of the
> Kconfig conversion, the Makefiles were updated to align with how this
> is done in Linux: a single target for each SoC is used to build all the
> .dtb files for that SoC.
> 
> Since then, the Makefiles have devolved in some cases, resulting in
> lots of target-specific build rules. Also Linux has moved to using
> subdirectories for each vendor.
> 
> Recent work aims to allow U-Boot to directly use devicetree files from
> Linux. This would be easier if the directory structure were the same.
> Another recent discussion involved dropping the build rules altogether.
> 
> This series makes a start at cleaning up some of the build rules, to
> reduce the amount of code and make it easier to add new boards for the
> same SoC.
> 
> One issue is that the ARCH_xxx Kconfig options between U-Boot and Linux
> are not always the same. Given the large number of SoCs and boards
> supported by U-Boot, it would be useful to align these where possible.

I don't know why we should start with this now, and further not being on
top of Sumit's series to remove our duplicate dts files. And that's
where we can have the conversation about for which cases it even makes
sense to build all of the dts files for a given SoC.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 1/9] microblaze: dts: Use the normal build rule

[Tom Rini]

On Wed, Dec 27, 2023 at 08:23:57AM +, Simon Glass wrote:
> Build devicetree files using the normal SoC-generic rule. For
> microblaze there is actually only one SoC and one board.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  arch/microblaze/dts/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/microblaze/dts/Makefile b/arch/microblaze/dts/Makefile
> index 427a8f9aaca..adc76ddf21f 100644
> --- a/arch/microblaze/dts/Makefile
> +++ b/arch/microblaze/dts/Makefile
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0+
>  
> -dtb-y += $(shell echo $(CONFIG_DEFAULT_DEVICE_TREE)).dtb
> +dtb-$(CONFIG_MICROBLAZE) += microblaze-generic.dtb
>  
>  include $(srctree)/scripts/Makefile.dts

This (and nios2 and perhaps a few other arches) show that it would be
easier to just drop the dts- line as it adds nothing over what
scripts/Makefile.dts gives us. I wonder how this will interact with
OF_UPSTREAM and how it compares with arch/microblaze/boot/dts/system.dts
in the linux kernel.

-- 
Tom


signature.asc
Description: PGP signature

[PATCH] include: env: ti: default_findfdt: Follow the bootstd/distro conventions

[Nishanth Menon]

Distroboot and bootstd both mandate a findfdt variable pointing to the
correct device tree blob. Current mechanism calls a find_fdt function
to set this variable. We do not need a find_fdt command to set the
environment variable to a single dtb. Simplify the default_findfdt to
remove variable expansion while at it.

For legacy scripts depending on a TI convention of name_fdt, provide a
find_fdt wrapper that behaves like before.

NOTE: As part of this change, we also drop the cooked up
default_device_tree_arch default_device_tree_subarch variables.

Reported-by: Jonathan Humphreys 
Signed-off-by: Nishanth Menon 
---
 include/env/ti/default_findfdt.env | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/include/env/ti/default_findfdt.env 
b/include/env/ti/default_findfdt.env
index a2b51dd923bb..1a1ab8406c9e 100644
--- a/include/env/ti/default_findfdt.env
+++ b/include/env/ti/default_findfdt.env
@@ -1,12 +1,7 @@
-default_device_tree=CONFIG_DEFAULT_DEVICE_TREE
-default_device_tree_arch=ti
 #ifdef CONFIG_ARM64
-findfdt=
-   setenv name_fdt ${default_device_tree_arch}/${default_device_tree}.dtb;
-   setenv fdtfile ${name_fdt}
+fdtfile=ti/CONFIG_DEFAULT_DEVICE_TREE.dtb;
 #else
-default_device_tree_subarch=omap
-findfdt=
-   setenv name_fdt 
${default_device_tree_arch}/${default_device_tree_subarch}/${default_device_tree}.dtb;
-   setenv fdtfile ${name_fdt}
+fdtfile=ti/omap/CONFIG_DEFAULT_DEVICE_TREE.dtb;
 #endif
+
+findfdt=setenv name_fdt ${fdt_file}
-- 
2.43.0

Re: [PATCH] include: env: ti: ti_common: Run main_cpsw0_qsgmii_phyinit conditionally

[Nishanth Menon]

On 16:06-20231221, Tom Rini wrote:
> On Mon, Dec 11, 2023 at 04:12:09PM +0530, Siddharth Vadapalli wrote:
> 
> > From: Manorit Chawdhry 
> > 
> > The main_cpsw0_qsgmii_phyinit command is defined only for certain TI
> > SoCs which have the do_main_cpsw0_qsgmii_phyinit variable set.
> > 
> > Add a check to ensure that the main_cpsw0_qsgmii_phyinit command is run
> > only for such SoCs.
> > 
> > Signed-off-by: Manorit Chawdhry 
> > Signed-off-by: Siddharth Vadapalli 
> > Reviewed-by: Tom Rini 
> > Reviewed-by: Mattijs Korpershoek 
> 
> Applied to u-boot/next, thanks!

Shouldn't main_cpsw0_qsgmii_phyinit be part of the driver?? why are we
scripting up driver initialization?

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH 00/26] sync am65x device tree with Linux v6.7-rc1

[Nishanth Menon]

On 15:00-20231221, Tom Rini wrote:
> On Thu, Dec 21, 2023 at 11:43:46AM -0600, Bryan Brattlof wrote:
> 
> > Hello Everyone!
> > 
> > This series gets the am65x booting again along with syncing the device
> > tree files with v6.7-rc1 Linux.
> > 
> > The bulk of these patches unify the WKUP SPL board file with the arm64
> > files to make future syncs from Linux much easier. In the end the DTBs
> > should look a lot like what the DTBs look like for the am64x which
> > is fairly similar to the am65x.
> > 
> > For those interested in what UART boot looks like:
> >https://paste.sr.ht/~bryanb/7df8a645dc548912cd806abd5ecab967ef3287bc
> > 
> > Thanks for reviewing and happy holidays
> > ~Bryan
> 
> For the series,
> Tested-by: Tom Rini 
> 
> Nishanth, does this path work for you?
> 

Other than the minor comments provided in the relevant patches,

The following files also need sync up with v6.7-rc1.

k3-am65-iot2050-common-pg2.dtsi
k3-am65-iot2050-common.dtsi
k3-am6528-iot2050-basic-common.dtsi
k3-am6548-iot2050-advanced-common.dtsi
k3-am6548-iot2050-advanced-m2.dts

Jan - could you look at syncing those once an update to this series is
posted? We could probably have a smoother v6.8-rc1 from then on.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH 16/26] arm: dts: k3-am654: remove duplicate sdhci1 pinmux node

[Nishanth Menon]

On 11:44-20231221, Bryan Brattlof wrote:
> With the Linux and U-Boot board dtb files unified, we now have a
> duplicate sdhci1 pinmux node. Remove it
> 
> Signed-off-by: Bryan Brattlof 
> ---
>  arch/arm/dts/k3-am654-r5-base-board.dts | 22 +++---
>  1 file changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
> b/arch/arm/dts/k3-am654-r5-base-board.dts
> index 856917bdfaa5d..c4096d1b754c4 100644
> --- a/arch/arm/dts/k3-am654-r5-base-board.dts
> +++ b/arch/arm/dts/k3-am654-r5-base-board.dts
> @@ -137,19 +137,6 @@
>  };
>  
>  _pmx0 {
> - main_mmc1_pins_default: main_mmc1_pins_default {
> - pinctrl-single,pins = <
> - AM65X_IOPAD(0x02d4, PIN_INPUT_PULLDOWN, 0)  /* 
> (C27) MMC1_CLK */
> - AM65X_IOPAD(0x02d8, PIN_INPUT_PULLUP, 0)/* 
> (C28) MMC1_CMD */
> - AM65X_IOPAD(0x02d0, PIN_INPUT_PULLUP, 0)/* 
> (D28) MMC1_DAT0 */
> - AM65X_IOPAD(0x02cc, PIN_INPUT_PULLUP, 0)/* 
> (E27) MMC1_DAT1 */
> - AM65X_IOPAD(0x02c8, PIN_INPUT_PULLUP, 0)/* 
> (D26) MMC1_DAT2 */
> - AM65X_IOPAD(0x02c4, PIN_INPUT_PULLUP, 0)/* 
> (D27) MMC1_DAT3 */
> - AM65X_IOPAD(0x02dc, PIN_INPUT_PULLUP, 0)/* 
> (B24) MMC1_SDCD */
> - AM65X_IOPAD(0x02e0, PIN_INPUT, 0)   /* 
> (C24) MMC1_SDWP */
> - >;
> - };
> -
>   usb0_pins_default: usb0_pins_default {
>   pinctrl-single,pins = <
>   AM65X_IOPAD(0x02bc, PIN_OUTPUT, 0) /* (AD9) 
> USB0_DRVVBUS */
> @@ -176,12 +163,17 @@
>   /delete-property/ power-domains;
>  };
>  
> +/* MMC is probed to pull in firmware, so any clock
> + * or power-domain operation will fail as we do not
> + * have the firmware running at this point. Delete the
> + * power-domain properties to avoid making calls to
> + * SYSFW before it is loaded. Public ROM has already
> + * set it up for us anyway.
> + */

Same - please fix the comment style here as well.

>   {
>   clock-names = "clk_xin";
>   clocks = <_200mhz>;
> - pinctrl-0 = <_mmc1_pins_default>;
>   /delete-property/ power-domains;
> - ti,driver-strength-ohm = <50>;
>  };
>  
>  _i2c0 {
> -- 
> 2.43.0
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH 15/26] arm: dts: k3-am654: remove duplicate sdhci0 pinmux node

[Nishanth Menon]

On 11:44-20231221, Bryan Brattlof wrote:
> With the Linux and U-Boot board dtb files unified, we now have
> a duplicate sdhci0 pinmux node. Remove it
> 
> Signed-off-by: Bryan Brattlof 
> ---
>  arch/arm/dts/k3-am654-r5-base-board.dts | 25 +++--
>  1 file changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
> b/arch/arm/dts/k3-am654-r5-base-board.dts
> index 66953eb5581ae..856917bdfaa5d 100644
> --- a/arch/arm/dts/k3-am654-r5-base-board.dts
> +++ b/arch/arm/dts/k3-am654-r5-base-board.dts
> @@ -137,22 +137,6 @@
>  };
>  
>  _pmx0 {
> - main_mmc0_pins_default: main_mmc0_pins_default {
> - pinctrl-single,pins = <
> - AM65X_IOPAD(0x01a8, PIN_INPUT_PULLDOWN, 0)  /* 
> (B25) MMC0_CLK */
> - AM65X_IOPAD(0x01aC, PIN_INPUT_PULLUP, 0)/* 
> (B27) MMC0_CMD */
> - AM65X_IOPAD(0x01a4, PIN_INPUT_PULLUP, 0)/* 
> (A26) MMC0_DAT0 */
> - AM65X_IOPAD(0x01a0, PIN_INPUT_PULLUP, 0)/* 
> (E25) MMC0_DAT1 */
> - AM65X_IOPAD(0x019c, PIN_INPUT_PULLUP, 0)/* 
> (C26) MMC0_DAT2 */
> - AM65X_IOPAD(0x0198, PIN_INPUT_PULLUP, 0)/* 
> (A25) MMC0_DAT3 */
> - AM65X_IOPAD(0x0194, PIN_INPUT_PULLUP, 0)/* 
> (E24) MMC0_DAT4 */
> - AM65X_IOPAD(0x0190, PIN_INPUT_PULLUP, 0)/* 
> (A24) MMC0_DAT5 */
> - AM65X_IOPAD(0x018c, PIN_INPUT_PULLUP, 0)/* 
> (B26) MMC0_DAT6 */
> - AM65X_IOPAD(0x0188, PIN_INPUT_PULLUP, 0)/* 
> (D25) MMC0_DAT7 */
> - AM65X_IOPAD(0x01b0, PIN_INPUT, 0)   /* 
> (C25) MMC0_DS */
> - >;
> - };
> -
>   main_mmc1_pins_default: main_mmc1_pins_default {
>   pinctrl-single,pins = <
>   AM65X_IOPAD(0x02d4, PIN_INPUT_PULLDOWN, 0)  /* 
> (C27) MMC1_CLK */
> @@ -179,12 +163,17 @@
>   pinctrl-0 = <_vtt_pins_default>;
>  };
>  
> +/* MMC is probed to pull in firmware, so any clock
> + * or power-domain operation will fail as we do not
> + * have the firmware running at this point. Delete the
> + * power-domain properties to avoid making calls to
> + * SYSFW before it is loaded. Public ROM has already
> + * set it up for us anyway.
> + */

Please fix your comment style here:

/*
 * blah blah
 */
>   {
>   clock-names = "clk_xin";
>   clocks = <_200mhz>;
> - pinctrl-0 = <_mmc0_pins_default>;
>   /delete-property/ power-domains;
> - ti,driver-strength-ohm = <50>;
>  };
>  
>   {
> -- 
> 2.43.0
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH 26/26] arm: dts: k3-am654: convert bootph-pre-ram to bootph-all

[Nishanth Menon]

On 11:44-20231221, Bryan Brattlof wrote:
[..]
> diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi 
> b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
> index a9b2d0d2a3036..770c7d129339f 100644
> --- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
[...]
>  _uart0 {
> - bootph-pre-ram;
> + bootph-all;
>  };

Consider moving this to r5 dts - there is no use in having this in
u-boot. should be pre-ram?

>  
>  _uart0 {
> - bootph-pre-ram;
> + bootph-all;
>  };
Same here:
Consider moving this to r5 dts - there is no use in having this in
u-boot. should be pre-ram?

[...]

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH 12/26] arm: dts: k3-am654: add needed regs to udmap nodes

[Nishanth Menon]

On 11:43-20231221, Bryan Brattlof wrote:
> Ethernet is one of a few IPs in U-Boot that depend on DMA to operate.
> However there are a few missing registers ranges in the udmap nodes
> need to properly setup DMA for the am65x.
> 
> A fix has been added to the Linux kernel[0] to add these ranges however
> they have not made it to a Linux tag. To keep DMA operational until the
> next DT sync from Linux, add these ranges to the *-u-boot.dtsi with a
> note for our future selves.
> 
> [0] https://lore.kernel.org/r/20231213135138.929517-2-vigne...@ti.com
> Signed-off-by: Bryan Brattlof 
> ---
>  arch/arm/dts/k3-am654-base-board-u-boot.dtsi | 32 
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi 
> b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
> index 9cd8c353c515b..645241da322a5 100644
> --- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
> @@ -260,3 +260,35 @@
>  _r5fss0 {
>   ti,cluster-mode = <0>;
>  };
> +
> +/* The DMA driver requires a few extra register ranges
> + * which are missing for the am65x. A patch has been
> + * sent and will be synced after the v6.8-rc1 linux
> + * tag is published
> + */

Please fix multi-line comment style:
/*
 * blah blah
 */
instead of
/* blah blah
 * more stuff
 */
> +_udmap {
> + reg = <0x0 0x3115 0x0 0x100>,
> +   <0x0 0x3400 0x0 0x10>,
> +   <0x0 0x3500 0x0 0x10>,
> +   <0x0 0x30b0 0x0 0x1>,
> +   <0x0 0x30c0 0x0 0x1>,
> +   <0x0 0x30d0 0x0 0x8000>;
> + reg-names = "gcfg", "rchanrt", "tchanrt",
> + "tchan", "rchan", "rflow";
> +};
> +
> +/* The DMA driver requires a few extra register ranges
> + * which are missing for the am65x. A patch has been
> + * sent and will be synced after the v6.8-rc1 linux
> + * tag is published
> + */

and here.

> +_udmap {
> + reg = <0x0 0x285c 0x0 0x100>,
> +   <0x0 0x2a80 0x0 0x4>,
> +   <0x0 0x2aa0 0x0 0x4>,
> +   <0x0 0x284a 0x0 0x4000>,
> +   <0x0 0x284c 0x0 0x4000>,
> +   <0x0 0x2840 0x0 0x2000>;
> + reg-names = "gcfg", "rchanrt", "tchanrt",
> + "tchan", "rchan", "rflow";
> +};
> -- 
> 2.43.0
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

[PATCH v1] arm: dts: rockchip: rk3288: move to 64 bit reg size

[Johan Jonker]

To make automatic Rockchip DT syncing possible from Linux to U-boot prepare
rk3288.dtsi by moving to 64 bit reg size.

Signed-off-by: Johan Jonker 
---
 arch/arm/dts/rk3288-evb.dtsi |   2 +-
 arch/arm/dts/rk3288-firefly.dtsi |   2 +-
 arch/arm/dts/rk3288-miqi.dtsi|   2 +-
 arch/arm/dts/rk3288-phycore-som.dtsi |   2 +-
 arch/arm/dts/rk3288-popmetal.dtsi|   2 +-
 arch/arm/dts/rk3288-rock2-som.dtsi   |   2 +-
 arch/arm/dts/rk3288-tinker.dtsi  |   2 +-
 arch/arm/dts/rk3288-u-boot.dtsi  |  14 +-
 arch/arm/dts/rk3288-veyron.dtsi  |   2 +-
 arch/arm/dts/rk3288.dtsi | 259 +++
 arch/arm/mach-rockchip/Kconfig   |   1 +
 11 files changed, 165 insertions(+), 125 deletions(-)

diff --git a/arch/arm/dts/rk3288-evb.dtsi b/arch/arm/dts/rk3288-evb.dtsi
index 72da8847344c..0e347beb154d 100644
--- a/arch/arm/dts/rk3288-evb.dtsi
+++ b/arch/arm/dts/rk3288-evb.dtsi
@@ -7,7 +7,7 @@

 / {
memory {
-   reg = <0 0x8000>;
+   reg = <0x0 0x0 0x0 0x8000>;
};

ext_gmac: external-gmac-clock {
diff --git a/arch/arm/dts/rk3288-firefly.dtsi b/arch/arm/dts/rk3288-firefly.dtsi
index 1117d3913ed7..0824b19ee642 100644
--- a/arch/arm/dts/rk3288-firefly.dtsi
+++ b/arch/arm/dts/rk3288-firefly.dtsi
@@ -7,7 +7,7 @@

 / {
memory {
-   reg = <0 0x8000>;
+   reg = <0x0 0x0 0x0 0x8000>;
};

ext_gmac: external-gmac-clock {
diff --git a/arch/arm/dts/rk3288-miqi.dtsi b/arch/arm/dts/rk3288-miqi.dtsi
index 00c8613d6d73..c56e1109e3ac 100644
--- a/arch/arm/dts/rk3288-miqi.dtsi
+++ b/arch/arm/dts/rk3288-miqi.dtsi
@@ -8,7 +8,7 @@
 / {
memory {
device_type = "memory";
-   reg = <0 0x8000>;
+   reg = <0x0 0x0 0x0 0x8000>;
};

ext_gmac: external-gmac-clock {
diff --git a/arch/arm/dts/rk3288-phycore-som.dtsi 
b/arch/arm/dts/rk3288-phycore-som.dtsi
index 70c00308d736..bde5910ff625 100644
--- a/arch/arm/dts/rk3288-phycore-som.dtsi
+++ b/arch/arm/dts/rk3288-phycore-som.dtsi
@@ -55,7 +55,7 @@
 */
memory {
device_type = "memory";
-   reg = <0 0x800>;
+   reg = <0x0 0x0 0x0 0x8000>;
};

aliases {
diff --git a/arch/arm/dts/rk3288-popmetal.dtsi 
b/arch/arm/dts/rk3288-popmetal.dtsi
index d732a70678ba..ecff641b1099 100644
--- a/arch/arm/dts/rk3288-popmetal.dtsi
+++ b/arch/arm/dts/rk3288-popmetal.dtsi
@@ -44,7 +44,7 @@
 / {
memory{
device_type = "memory";
-   reg = <0 0x8000>;
+   reg = <0x0 0x0 0x0 0x8000>;
};

ext_gmac: external-gmac-clock {
diff --git a/arch/arm/dts/rk3288-rock2-som.dtsi 
b/arch/arm/dts/rk3288-rock2-som.dtsi
index 1ece66f3e162..58e32fbb80f6 100644
--- a/arch/arm/dts/rk3288-rock2-som.dtsi
+++ b/arch/arm/dts/rk3288-rock2-som.dtsi
@@ -43,7 +43,7 @@

 / {
memory {
-   reg = <0x0 0x8000>;
+   reg = <0x0 0x0 0x0 0x8000>;
device_type = "memory";
};

diff --git a/arch/arm/dts/rk3288-tinker.dtsi b/arch/arm/dts/rk3288-tinker.dtsi
index 46460ae455e2..62b4beb25100 100644
--- a/arch/arm/dts/rk3288-tinker.dtsi
+++ b/arch/arm/dts/rk3288-tinker.dtsi
@@ -44,7 +44,7 @@
 / {
memory {
device_type = "memory";
-   reg = <0x0 0x8000>;
+   reg = <0x0 0x0 0x0 0x8000>;
};

ext_gmac: external-gmac-clock {
diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi
index c4c5a2d225c4..a43d320ade7b 100644
--- a/arch/arm/dts/rk3288-u-boot.dtsi
+++ b/arch/arm/dts/rk3288-u-boot.dtsi
@@ -29,10 +29,10 @@

dmc: dmc@ff61 {
compatible = "rockchip,rk3288-dmc", "syscon";
-   reg = <0xff61 0x3fc
-  0xff62 0x294
-  0xff63 0x3fc
-  0xff64 0x294>;
+   reg = <0x0 0xff61 0x0 0x3fc
+  0x0 0xff62 0x0 0x294
+  0x0 0xff63 0x0 0x3fc
+  0x0 0xff64 0x0 0x294>;
clocks = < PCLK_DDRUPCTL0>, < PCLK_PUBL0>,
 < PCLK_DDRUPCTL1>, < PCLK_PUBL1>,
 < ARMCLK>;
@@ -50,7 +50,7 @@

noc: syscon@ffac {
compatible = "rockchip,rk3288-noc", "syscon";
-   reg = <0xffac 0x2000>;
+   reg = <0x0 0xffac 0x0 0x2000>;
bootph-all;
};
 };
@@ -134,3 +134,7 @@
  {
bootph-all;
 };
+
+ {
+   bootph-all;
+};
diff --git a/arch/arm/dts/rk3288-veyron.dtsi b/arch/arm/dts/rk3288-veyron.dtsi
index 434b0d494e5e..99406151bf59 100644
--- a/arch/arm/dts/rk3288-veyron.dtsi
+++ b/arch/arm/dts/rk3288-veyron.dtsi
@@ -11,7 +11,7 @@

 / {
memory {
-   reg = <0x0 0x8000>;
+   reg = <0x0 0x0 0x0 0x8000>;
 

Re: [PATCH v4 0/7] smbios: Deal with tables beyond 4GB

[Ilias Apalodimas]

Hi Simon,

Thanks this looks good

On Wed, 27 Dec 2023 at 09:40, Simon Glass  wrote:
>
> When the malloc() region extends beyond 4GB on ARM we may end up with
> an SMBIOS table in that region.
>
> Add support for writing an SMBIOS3 table, which supports a 64-bit
> address.
>
> Note that this problem does not happen on x86 since it requires the
> tables to be placed just below 1MB in memory, unless
> CONFIG_BLOBLIST_TABLES is enabled.
>
>
> Changes in v4:
> - Bring in this patch from Heinrich's series
> - Check the start of the table rather than the end
>
> Changes in v2:
> - Check the end of the table rather than the start.
> - Add a new patch to correct gd_smbios_start()
> - Add a note about why unmap_system() is called
>
> Heinrich Schuchardt (1):
>   smbios: SMBIOS 3.0 (64-bit) Entry Point structure
>
> Simon Glass (6):
>   smbios: Refactor 32-bit code into an else statement
>   smbios: Move the rest of the SMBIOS2 code
>   smbios: Use SMBIOS 3.0 to support an address above 4GB
>   smbios: Correct gd_smbios_start()
>   efi: Use the correct GUID for the SMBIOS table
>   smbios: Require the caller to align the SMBIOS table
>
>  include/asm-generic/global_data.h |  2 +-
>  include/efi_api.h |  4 ++
>  include/smbios.h  | 37 --
>  lib/efi_loader/efi_smbios.c   | 14 +++--
>  lib/smbios.c  | 85 +++
>  5 files changed, 101 insertions(+), 41 deletions(-)
>
> --
> 2.34.1
>

For the series
Tested-by: Ilias Apalodimas 

Re: [PATCH v4 6/7] efi: Use the correct GUID for the SMBIOS table

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 09:40, Simon Glass  wrote:
>
> EFI does not use the 'anchor string' to determine the SMBIOS table
> version, instead preferring to have two separate GUIDs. Use the correct
> one, depending on the table version.
>
> Call unmap_system() to balance to the use of map_sysmem()
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Heinrich Schuchardt 
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Add a note about why unmap_system() is called
>
>  include/efi_api.h   |  4 
>  lib/efi_loader/efi_smbios.c | 12 ++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 0e92cb8a7f6..ab40b1b5ddf 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -433,6 +433,10 @@ struct efi_runtime_services {
> EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
>  0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>
> +#define SMBIOS3_TABLE_GUID \
> +   EFI_GUID(0xf2fd1544, 0x9794, 0x4a2c, \
> +0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94)
> +
>  #define EFI_LOAD_FILE_PROTOCOL_GUID \
> EFI_GUID(0x56ec3091, 0x954c, 0x11d2, \
>  0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
> index 49adc87e45a..5cbce6dc4eb 100644
> --- a/lib/efi_loader/efi_smbios.c
> +++ b/lib/efi_loader/efi_smbios.c
> @@ -14,6 +14,8 @@
>  #include 
>  #include 
>
> +const efi_guid_t smbios3_guid = SMBIOS3_TABLE_GUID;
> +
>  enum {
> TABLE_SIZE  = SZ_4K,
>  };
> @@ -25,8 +27,10 @@ enum {
>   */
>  efi_status_t efi_smbios_register(void)
>  {
> +   const efi_guid_t *guid;
> ulong addr;
> efi_status_t ret;
> +   void *buf;
>
> addr = gd_smbios_start();
> if (!addr) {
> @@ -42,8 +46,12 @@ efi_status_t efi_smbios_register(void)
> log_debug("EFI using SMBIOS tables at %lx\n", addr);
>
> /* Install SMBIOS information as configuration table */
> -   return efi_install_configuration_table(_guid,
> -  map_sysmem(addr, 0));
> +   buf = map_sysmem(addr, 0);
> +   guid = !memcmp(buf, "_SM_", 4) ? _guid : _guid;
> +   ret = efi_install_configuration_table(guid, buf);
> +   unmap_sysmem(buf);
> +
> +   return ret;
>  }
>
>  static int install_smbios_table(void)
> --
> 2.34.1
>

Reviewed-by: Ilias Apalodimas 

Re: [PATCH v4 7/7] smbios: Require the caller to align the SMBIOS table

[Ilias Apalodimas]

Hi Simon,

I commented on v3 as well, but in case you miss that

On Wed, 27 Dec 2023 at 09:40, Simon Glass  wrote:
>
> All callers handle this alignment, so drop the unnecessary code. This
> simplifies things a little.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Heinrich Schuchardt 
> ---
>
> (no changes since v1)
>
>  include/smbios.h | 5 +
>  lib/smbios.c | 2 --
>  2 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/include/smbios.h b/include/smbios.h
> index 77be58887a2..879b8a814b8 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -258,12 +258,9 @@ static inline void fill_smbios_header(void *table, int 
> type,
>   *
>   * This writes SMBIOS table at a given address.
>   *
> - * @addr:  start address to write SMBIOS table. If this is not
> - * 16-byte-aligned then it will be aligned before the table is
> - * written.
> + * @addr:  start address to write SMBIOS table (must be 16-byte-aligned)
>   * Return: end address of SMBIOS table (and start address for next entry)
>   * or NULL in case of an error
> - *
>   */
>  ulong write_smbios_table(ulong addr);
>
> diff --git a/lib/smbios.c b/lib/smbios.c
> index 7f79d969c92..cfd451e4088 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -563,8 +563,6 @@ ulong write_smbios_table(ulong addr)
> ctx.dev = NULL;
> }
>
> -   /* 16 byte align the table address */
> -   addr = ALIGN(addr, 16);

I think this is wrong.  It will break SMBIOS on a user error. I am
fine replacing that with a check instead and error out if the address
is not 16b aligned

Thanks
/Ilias
> start_addr = addr;
>
> /*
> --
> 2.34.1
>

Re: [PATCH v4 5/7] smbios: Correct gd_smbios_start()

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 09:40, Simon Glass  wrote:
>
> This should access arch-specific properties. Fix it and update the
> existing usage.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Heinrich Schuchardt 
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Add a new patch to correct gd_smbios_start()
>
>  include/asm-generic/global_data.h | 2 +-
>  lib/efi_loader/efi_smbios.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/global_data.h 
> b/include/asm-generic/global_data.h
> index e8c6412e3f8..c1f7818817f 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -553,7 +553,7 @@ static_assert(sizeof(struct global_data) == GD_SIZE);
>  #endif
>
>  #ifdef CONFIG_SMBIOS
> -#define gd_smbios_start()  gd->smbios_start
> +#define gd_smbios_start()  gd->arch.smbios_start
>  #define gd_set_smbios_start(addr)  gd->arch.smbios_start = addr
>  #else
>  #define gd_smbios_start()  0UL
> diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
> index bbb8421ce14..49adc87e45a 100644
> --- a/lib/efi_loader/efi_smbios.c
> +++ b/lib/efi_loader/efi_smbios.c
> @@ -28,7 +28,7 @@ efi_status_t efi_smbios_register(void)
> ulong addr;
> efi_status_t ret;
>
> -   addr = gd->arch.smbios_start;
> +   addr = gd_smbios_start();
> if (!addr) {
> log_err("No SMBIOS tables to install\n");
> return EFI_NOT_FOUND;
> --
> 2.34.1
>

Reviewed-by: Ilias Apalodimas 

Re: [PATCH v4 4/7] smbios: Use SMBIOS 3.0 to support an address above 4GB

[Ilias Apalodimas]

Hi Simon,

On Wed, 27 Dec 2023 at 09:40, Simon Glass  wrote:
>
> When the SMBIOS table is written to an address above 4GB a 32-bit table
> address is not large enough.
>
> Use an SMBIOS3 table in that case.

Maybe I missed this on the previous revisions, but is there a reason
we don't always use SMBIOS3 now? And perhaps try to install SMBIOS2 if
1. we fail
2. and the address is < 4GB

Thanks
/Ilias
>
> Note that we cannot use efi_allocate_pages() since this function has
> nothing to do with EFI. There is no equivalent function to allocate
> memory below 4GB in U-Boot. One solution would be to create a separate
> malloc() pool, or just always put the malloc() pool below 4GB.
>
> - Use log_debug() for warning
> - Rebase on Heinrich's smbios.h patch
> - Set the checksum for SMBIOS3
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v4:
> - Check the start of the table rather than the end
>
> Changes in v2:
> - Check the end of the table rather than the start.
>
>  include/smbios.h |  6 +-
>  lib/smbios.c | 30 +-
>  2 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/include/smbios.h b/include/smbios.h
> index e601283d293..77be58887a2 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -12,7 +12,7 @@
>
>  /* SMBIOS spec version implemented */
>  #define SMBIOS_MAJOR_VER   3
> -#define SMBIOS_MINOR_VER   0
> +#define SMBIOS_MINOR_VER   7
>
>  enum {
> SMBIOS_STR_MAX  = 64,   /* Maximum length allowed for a string */
> @@ -80,6 +80,10 @@ struct __packed smbios3_entry {
> u64 struct_table_address;
>  };
>
> +/* These two structures should use the same amount of 16-byte-aligned space 
> */
> +static_assert(ALIGN(16, sizeof(struct smbios_entry)) ==
> + ALIGN(16, sizeof(struct smbios3_entry)));
> +
>  /* BIOS characteristics */
>  #define BIOS_CHARACTERISTICS_PCI_SUPPORTED (1 << 7)
>  #define BIOS_CHARACTERISTICS_UPGRADEABLE   (1 << 11)
> diff --git a/lib/smbios.c b/lib/smbios.c
> index eea72670bd9..7f79d969c92 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -567,7 +567,11 @@ ulong write_smbios_table(ulong addr)
> addr = ALIGN(addr, 16);
> start_addr = addr;
>
> -   addr += sizeof(struct smbios_entry);
> +   /*
> +* So far we don't know which struct will be used, but they both end
> +* up using the same amount of 16-bit-aligned space
> +*/
> +   addr += max(sizeof(struct smbios_entry), sizeof(struct 
> smbios3_entry));
> addr = ALIGN(addr, 16);
> tables = addr;
>
> @@ -590,16 +594,32 @@ ulong write_smbios_table(ulong addr)
>  * We must use a pointer here so things work correctly on sandbox. The
>  * user of this table is not aware of the mapping of addresses to
>  * sandbox's DRAM buffer.
> +*
> +* Check the address of the end of the tables. If it is above 4GB then
> +* it is sensible to use SMBIOS3 even if the start of the table is 
> below
> +* 4GB (this case is very unlikely to happen in practice)
>  */
> table_addr = (ulong)map_sysmem(tables, 0);
> -   if (sizeof(table_addr) > sizeof(u32) && table_addr > (ulong)UINT_MAX) 
> {
> +   if (sizeof(table_addr) > sizeof(u32) && addr >= (ulong)UINT_MAX) {
> +   struct smbios3_entry *se;
> /*
>  * We need to put this >32-bit pointer into the table but the
>  * field is only 32 bits wide.
>  */
> -   printf("WARNING: SMBIOS table_address overflow %llx\n",
> -  (unsigned long long)table_addr);
> -   addr = 0;
> +   log_debug("WARNING: Using SMBIOS3.0 due to table-address 
> overflow %lx\n",
> + table_addr);
> +   se = map_sysmem(start_addr, sizeof(struct smbios_entry));
> +   memset(se, '\0', sizeof(struct smbios_entry));
> +   memcpy(se->anchor, "_SM3_", 5);
> +   se->length = sizeof(struct smbios3_entry);
> +   se->major_ver = SMBIOS_MAJOR_VER;
> +   se->minor_ver = SMBIOS_MINOR_VER;
> +   se->doc_rev = 0;
> +   se->entry_point_rev = 1;
> +   se->max_struct_size = len;
> +   se->struct_table_address = table_addr;
> +   se->checksum = table_compute_checksum(se,
> +   sizeof(struct smbios3_entry));
> } else {
> struct smbios_entry *se;
>
> --
> 2.34.1
>

Re: [PATCH v4 2/7] smbios: Move the rest of the SMBIOS2 code

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 09:40, Simon Glass  wrote:
>
> Move all of this logic into the else clause, since it will not be used
> for SMBIOS3
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Heinrich Schuchardt 
> ---
>
> (no changes since v1)
>
>  lib/smbios.c | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/lib/smbios.c b/lib/smbios.c
> index b417f8a9057..eea72670bd9 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -544,9 +544,8 @@ static struct smbios_write_method smbios_write_funcs[] = {
>  ulong write_smbios_table(ulong addr)
>  {
> ofnode parent_node = ofnode_null();
> -   struct smbios_entry *se;
> +   ulong table_addr, start_addr;
> struct smbios_ctx ctx;
> -   ulong table_addr;
> ulong tables;
> int len = 0;
> int max_struct_size = 0;
> @@ -566,9 +565,7 @@ ulong write_smbios_table(ulong addr)
>
> /* 16 byte align the table address */
> addr = ALIGN(addr, 16);
> -
> -   se = map_sysmem(addr, sizeof(struct smbios_entry));
> -   memset(se, 0, sizeof(struct smbios_entry));
> +   start_addr = addr;
>
> addr += sizeof(struct smbios_entry);
> addr = ALIGN(addr, 16);
> @@ -603,8 +600,11 @@ ulong write_smbios_table(ulong addr)
> printf("WARNING: SMBIOS table_address overflow %llx\n",
>(unsigned long long)table_addr);
> addr = 0;
> -   goto out;
> } else {
> +   struct smbios_entry *se;
> +
> +   se = map_sysmem(start_addr, sizeof(struct smbios_entry));
> +   memset(se, '\0', sizeof(struct smbios_entry));
> memcpy(se->anchor, "_SM_", 4);
> se->length = sizeof(struct smbios_entry);
> se->major_ver = SMBIOS_MAJOR_VER;
> @@ -625,9 +625,8 @@ ulong write_smbios_table(ulong addr)
>isize);
> se->checksum = table_compute_checksum(se,
>   sizeof(struct smbios_entry));
> +   unmap_sysmem(se);
> }
> -out:
> -   unmap_sysmem(se);
>
> return addr;
>  }
> --
> 2.34.1
>

Reviewed-by: Ilias Apalodimas 

Re: [PATCH v4 1/7] smbios: Refactor 32-bit code into an else statement

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 09:40, Simon Glass  wrote:
>
> In preparation for adding support for SMBIOS3 move this code into an
> else statement. There is no functional change.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Heinrich Schuchardt 
> ---
>
> (no changes since v1)
>
>  lib/smbios.c | 38 +-
>  1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/lib/smbios.c b/lib/smbios.c
> index 45480b01af4..b417f8a9057 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -589,14 +589,6 @@ ulong write_smbios_table(ulong addr)
> len += tmp;
> }
>
> -   memcpy(se->anchor, "_SM_", 4);
> -   se->length = sizeof(struct smbios_entry);
> -   se->major_ver = SMBIOS_MAJOR_VER;
> -   se->minor_ver = SMBIOS_MINOR_VER;
> -   se->max_struct_size = max_struct_size;
> -   memcpy(se->intermediate_anchor, "_DMI_", 5);
> -   se->struct_table_length = len;
> -
> /*
>  * We must use a pointer here so things work correctly on sandbox. The
>  * user of this table is not aware of the mapping of addresses to
> @@ -612,16 +604,28 @@ ulong write_smbios_table(ulong addr)
>(unsigned long long)table_addr);
> addr = 0;
> goto out;
> +   } else {
> +   memcpy(se->anchor, "_SM_", 4);
> +   se->length = sizeof(struct smbios_entry);
> +   se->major_ver = SMBIOS_MAJOR_VER;
> +   se->minor_ver = SMBIOS_MINOR_VER;
> +   se->max_struct_size = max_struct_size;
> +   memcpy(se->intermediate_anchor, "_DMI_", 5);
> +   se->struct_table_length = len;
> +
> +   se->struct_table_address = table_addr;
> +
> +   se->struct_count = handle;
> +
> +   /* calculate checksums */
> +   istart = (char *)se + SMBIOS_INTERMEDIATE_OFFSET;
> +   isize = sizeof(struct smbios_entry) -
> +   SMBIOS_INTERMEDIATE_OFFSET;
> +   se->intermediate_checksum = table_compute_checksum(istart,
> +  isize);
> +   se->checksum = table_compute_checksum(se,
> + sizeof(struct smbios_entry));
> }
> -   se->struct_table_address = table_addr;
> -
> -   se->struct_count = handle;
> -
> -   /* calculate checksums */
> -   istart = (char *)se + SMBIOS_INTERMEDIATE_OFFSET;
> -   isize = sizeof(struct smbios_entry) - SMBIOS_INTERMEDIATE_OFFSET;
> -   se->intermediate_checksum = table_compute_checksum(istart, isize);
> -   se->checksum = table_compute_checksum(se, sizeof(struct 
> smbios_entry));
>  out:
> unmap_sysmem(se);
>
> --
> 2.34.1
>
Reviewed-by: Ilias Apalodimas 

Re: [PATCH v4 3/7] smbios: SMBIOS 3.0 (64-bit) Entry Point structure

[Ilias Apalodimas]

On Wed, 27 Dec 2023 at 09:40, Simon Glass  wrote:
>
> From: Heinrich Schuchardt 
>
> Add definition of the SMBIOS 3.0 (64-bit) Entry Point structure.
>
> Signed-off-by: Heinrich Schuchardt 
> Reviewed-by: Simon Glass 
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v4:
> - Bring in this patch from Heinrich's series
>
>  include/smbios.h | 26 ++
>  1 file changed, 26 insertions(+)
>
> diff --git a/include/smbios.h b/include/smbios.h
> index c9df2706f5a..e601283d293 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -54,6 +54,32 @@ struct __packed smbios_entry {
> u8 bcd_rev;
>  };
>
> +/**
> + * struct smbios3_entry - SMBIOS 3.0 (64-bit) Entry Point structure
> + */
> +struct __packed smbios3_entry {
> +   /** @anchor: anchor string */
> +   u8 anchor[5];
> +   /** @checksum: checksum of the entry point structure */
> +   u8 checksum;
> +   /** @length: length of the entry point structure */
> +   u8 length;
> +   /** @major_ver: major version of the SMBIOS specification */
> +   u8 major_ver;
> +   /** @minor_ver: minor version of the SMBIOS specification */
> +   u8 minor_ver;
> +   /** @docrev: revision of the SMBIOS specification */
> +   u8 doc_rev;
> +   /** @entry_point_rev: revision of the entry point structure */
> +   u8 entry_point_rev;
> +   /** @reserved: reserved */
> +   u8 reserved;
> +   /** maximum size of SMBIOS table */
> +   u32 max_struct_size;
> +   /** @struct_table_address: 64-bit physical starting address */
> +   u64 struct_table_address;
> +};
> +
>  /* BIOS characteristics */
>  #define BIOS_CHARACTERISTICS_PCI_SUPPORTED (1 << 7)
>  #define BIOS_CHARACTERISTICS_UPGRADEABLE   (1 << 11)
> --
> 2.34.1
>

Reviewed-by: Ilias Apalodimas 

Re: [PATCH v2 1/6] smbios: Refactor 32-bit code into an else statement

[Ilias Apalodimas]

On Tue, 26 Dec 2023 at 12:46, Heinrich Schuchardt  wrote:
>
> On 10/15/23 04:45, Simon Glass wrote:
> > In preparation for adding support for SMBIOS3 move this code into an
> > else statement. There is no functional change.
> >
> > Signed-off-by: Simon Glass 
>
> Reviewed-by: Heinrich Schuchardt 

Reviewed-by: Ilias Apalodimas 

Re: [PATCH v2 1/2] smbios: SMBIOS 3.0 (64-bit) Entry Point structure

[Ilias Apalodimas]

On Sat, 23 Dec 2023 at 03:09, Heinrich Schuchardt  wrote:
>
> From: Heinrich Schuchardt 
>
> Add definition of the SMBIOS 3.0 (64-bit) Entry Point structure.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2:
> no change
> ---
>  include/smbios.h | 26 ++
>  1 file changed, 26 insertions(+)
>
> diff --git a/include/smbios.h b/include/smbios.h
> index c9df2706f5..e601283d29 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -54,6 +54,32 @@ struct __packed smbios_entry {
> u8 bcd_rev;
>  };
>
> +/**
> + * struct smbios3_entry - SMBIOS 3.0 (64-bit) Entry Point structure
> + */
> +struct __packed smbios3_entry {
> +   /** @anchor: anchor string */
> +   u8 anchor[5];
> +   /** @checksum: checksum of the entry point structure */
> +   u8 checksum;
> +   /** @length: length of the entry point structure */
> +   u8 length;
> +   /** @major_ver: major version of the SMBIOS specification */
> +   u8 major_ver;
> +   /** @minor_ver: minor version of the SMBIOS specification */
> +   u8 minor_ver;
> +   /** @docrev: revision of the SMBIOS specification */
> +   u8 doc_rev;
> +   /** @entry_point_rev: revision of the entry point structure */
> +   u8 entry_point_rev;
> +   /** @reserved: reserved */
> +   u8 reserved;
> +   /** maximum size of SMBIOS table */
> +   u32 max_struct_size;
> +   /** @struct_table_address: 64-bit physical starting address */
> +   u64 struct_table_address;
> +};
> +
>  /* BIOS characteristics */
>  #define BIOS_CHARACTERISTICS_PCI_SUPPORTED (1 << 7)
>  #define BIOS_CHARACTERISTICS_UPGRADEABLE   (1 << 11)
> --
> 2.43.0
>

Reviewed-by: Ilias Apalodimas 

Re: [PATCH v2 6/6] smbios: Require the caller to align the SMBIOS table

[Ilias Apalodimas]

On Tue, 26 Dec 2023 at 14:46, Heinrich Schuchardt  wrote:
>
> On 10/15/23 04:45, Simon Glass wrote:
> > All callers handle this alignment, so drop the unnecessary code. This
> > simplifies things a little.
> >
> > Signed-off-by: Simon Glass 
>
> Reviewed: Heinrich Schuchardt 
>
> > ---
> >
> > (no changes since v1)
> >
> >   include/smbios.h | 5 +
> >   lib/smbios.c | 2 --
> >   2 files changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/include/smbios.h b/include/smbios.h
> > index ddabb558299e..31d997287588 100644
> > --- a/include/smbios.h
> > +++ b/include/smbios.h
> > @@ -248,12 +248,9 @@ static inline void fill_smbios_header(void *table, int 
> > type,
> >*
> >* This writes SMBIOS table at a given address.
> >*
> > - * @addr:start address to write SMBIOS table. If this is not
> > - *   16-byte-aligned then it will be aligned before the table is
> > - *   written.
> > + * @addr:start address to write SMBIOS table (must be 16-byte-aligned)
> >* Return:  end address of SMBIOS table (and start address for next entry)
> >*  or NULL in case of an error
> > - *
> >*/
> >   ulong write_smbios_table(ulong addr);
> >
> > diff --git a/lib/smbios.c b/lib/smbios.c
> > index 92e98388084f..5c9f108496d6 100644
> > --- a/lib/smbios.c
> > +++ b/lib/smbios.c
> > @@ -483,8 +483,6 @@ ulong write_smbios_table(ulong addr)
> >   ctx.dev = NULL;
> >   }
> >
> > - /* 16 byte align the table address */
> > - addr = ALIGN(addr, 16);

I don't think this is fine. Failing to do so would break the SMBIOS
tables. It's fine to require this but in that case, we should have add
a check for the 16b alignment

Thanks
/Ilias
> >   start_addr = addr;
> >
> >   /*
>

Re: [PATCH 1/1] smbios: type2: contained object handles

[Ilias Apalodimas]

On Fri, 22 Dec 2023 at 22:16, Heinrich Schuchardt  wrote:
>
> The type 2 structure must include information about the contained objects.
> It is fine to set the number of contained object handles to 0.
>
> Add the missing field.
>
> Fixes: 721e992a8af5 ("x86: Add SMBIOS table support")
> Signed-off-by: Heinrich Schuchardt 
> ---
>  include/smbios.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/smbios.h b/include/smbios.h
> index e601283d29..88c19ae062 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -139,6 +139,7 @@ struct __packed smbios_type2 {
> u8 chassis_location;
> u16 chassis_handle;
> u8 board_type;
> +   u8 number_contained_objects;
> char eos[SMBIOS_STRUCT_EOS_BYTES];
>  };
>
> --
> 2.43.0
>

Reviewed-by: Ilias Apalodimas 

Re: [RFC PATCH 12/16] arm: dts: k3-am65x-binman: Add ICSSG2 overlay and configuration

[MD Danish Anwar]

On 20/12/23 4:10 pm, Roger Quadros wrote:
> 
> 
> On 19/12/2023 12:34, MD Danish Anwar wrote:
>> Add ICSSG2 overlay and configuration to tispl and u-boot images.
>>
>> Signed-off-by: MD Danish Anwar 
>> ---
>>  arch/arm/dts/k3-am65x-binman.dtsi | 85 +++
>>  1 file changed, 85 insertions(+)
>>
>> diff --git a/arch/arm/dts/k3-am65x-binman.dtsi 
>> b/arch/arm/dts/k3-am65x-binman.dtsi
>> index 8cc24da1f3..9a0c0fca47 100644
>> --- a/arch/arm/dts/k3-am65x-binman.dtsi
>> +++ b/arch/arm/dts/k3-am65x-binman.dtsi
>> @@ -98,6 +98,8 @@
>>  #define SPL_AM654_EVM_DTB "spl/dts/k3-am654-base-board.dtb"
>>  #define AM654_EVM_DTB "u-boot.dtb"
>>  
>> +#define AM654_EVM_ICSSG2_DTBO "arch/arm/dts/k3-am654-icssg2.dtbo"
>> +
>>   {
>>  ti-spl {
>>  insert-template = <_spl_template>;
>> @@ -124,6 +126,20 @@
>>  filename = SPL_AM654_EVM_DTB;
>>  };
>>  };
>> +
>> +fdt-1 {
>> +description = "k3-am654-icssg2 overlay";
>> +type = "flat_dt";
>> +arch = "arm";
>> +compression = "none";
>> +ti-secure {
>> +content = 
>> <_am65x_evm_icssg2_dtb>;
>> +keyfile = "custMpk.pem";
>> +};
>> +spl_am65x_evm_icssg2_dtb: blob-ext {
>> +filename = 
>> AM654_EVM_ICSSG2_DTBO;
>> +};
> 
> This is wrong.
> 
> ICSSG2 Ethernet should be part of the fdt-0 configuration as the 2 Ethernet 
> ports
> on the board are hardwired to ICSSG2. Not having them working by default
> is an invalid configuration.
> 

ICSSG2 ethernet ports should be enabled by default. But the ICSSG2 nodes
is added in the overlay file (k3-am654-icssg2.dtso) in kernel so they
are added in same overlay file in u-boot as well.

I am keeping,
fdt-0  as k3-am654-base-board dtb,
fdt-1  as k3-am654-icssg2 dtbo,
conf-0 as k3-am654-base-board and
conf-1 as k3-am654-icssg2.

Do you want me to keep k3-am654-icssg2 dtbo as fdt-0 and
k3-am654-base-board as fdt-1? I tried doing this but this results into
u-boot getting stuck. The tispl and u-boot images are not able to load
if I swap fdt-0 and fdt-1 , and conf-0 and conf-1.

If the current combination doesn't look OK, please let me know what
should be the correct combinations for fdt-0, fdt-1, conf-0 and conf-1.

>> +};
>>  };
>>  
>>  configurations {
>> @@ -135,6 +151,13 @@
>>  loadables = "tee", "dm", "spl";
>>  fdt = "fdt-0";
>>  };
>> +
>> +conf-1 {
>> +description = "k3-am654-icssg2";
>> +firmware = "atf";
>> +loadables = "tee", "dm", "spl";
>> +fdt = "fdt-0", "fdt-1";
>> +};
>>  };
>>  };
>>  };s
>> @@ -168,6 +191,24 @@
>>  };
>>  };
>>  
>> +fdt-1 {
>> +description = "k3-am654-icssg2 overlay";
>> +type = "flat_dt";
>> +arch = "arm";
>> +compression = "none";
>> +ti-secure {
>> +content = 
>> <_evm_icssg2_dtb>;
>> +keyfile = "custMpk.pem";
>> +
>> +};
>> +am65x_evm_icssg2_dtb: blob-ext {
>> +filename = 
>> AM654_EVM_ICSSG2_DTBO;
>> +};
>> +hash {
>> +algo = "crc32";
>> +};
>> +};
>> +
>>  };
>>  
>>  configurations {
>> @@ -179,6 +220,13 @@
>>  loadables = "uboot";
>>  fdt = "fdt-0";
>>  };
>> +
>> +conf-1 {
>> +description = "k3-am654-icssg2";
>> +firmware = "uboot";
>> +loadables = "uboot";
>> +fdt = 

Re: [PATCH 4/5] doc: man-page for smbios command

[Ilias Apalodimas]

On Sat, 23 Dec 2023 at 02:57, Heinrich Schuchardt  wrote:
>
> Provide a man-page for the smbios command.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  doc/usage/cmd/smbios.rst | 93 
>  doc/usage/index.rst  |  1 +
>  2 files changed, 94 insertions(+)
>  create mode 100644 doc/usage/cmd/smbios.rst
>
> diff --git a/doc/usage/cmd/smbios.rst b/doc/usage/cmd/smbios.rst
> new file mode 100644
> index 00..1ffd706d7d
> --- /dev/null
> +++ b/doc/usage/cmd/smbios.rst
> @@ -0,0 +1,93 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later:
> +
> +smbios command
> +==
> +
> +Synopsis
> +
> +
> +::
> +
> +smbios
> +
> +Description
> +---
> +
> +The smbios command displays information from the SMBIOS tables.
> +
> +Examples
> +
> +
> +The example below shows an example output of the smbios command.
> +
> +::
> +
> +=> smbios
> +SMBIOS 2.8.0 present.
> +8 structures occupying 81 bytes
> +Table at 0x6d35018
> +
> +Handle 0x0100, DMI type 1, 27 bytes at 0x6d35018
> +System Information
> +Manufacturer: QEMU
> +Product Name: Standard PC (i440FX + PIIX, 1996)
> +Version: pc-i440fx-2.5
> +Serial Number:
> +UUID ----
> +Wake Up Type:
> +Serial Number:
> +SKU Number:
> +
> +Handle 0x0300, DMI type 3, 22 bytes at 0x6d35069
> +Header and Data:
> +: 03 16 00 03 01 01 02 00 00 03 03 03 02 00 00 00
> +0010: 00 00 00 00 00 00
> +Strings:
> +String 1: QEMU
> +String 2: pc-i440fx-2.5
> +
> +Handle 0x0400, DMI type 4, 42 bytes at 0x6d35093
> +Header and Data:
> +: 04 2a 00 04 01 03 01 02 63 06 00 00 fd ab 81 07
> +0010: 03 00 00 00 d0 07 d0 07 41 01 ff ff ff ff ff ff
> +0020: 00 00 00 01 01 01 02 00 01 00
> +Strings:
> +String 1: CPU 0
> +String 2: QEMU
> +String 3: pc-i440fx-2.5
> +
> +Handle 0x1000, DMI type 16, 23 bytes at 0x6d350d7
> +Header and Data:
> +: 10 17 00 10 01 03 06 00 00 02 00 fe ff 01 00 00
> +0010: 00 00 00 00 00 00 00
> +
> +Handle 0x1100, DMI type 17, 40 bytes at 0x6d350f0
> +Header and Data:
> +: 11 28 00 11 00 10 fe ff ff ff ff ff 80 00 09 00
> +0010: 01 00 07 02 00 00 00 02 00 00 00 00 00 00 00 00
> +0020: 00 00 00 00 00 00 00 00
> +Strings:
> +String 1: DIMM 0
> +String 2: QEMU
> +
> +Handle 0x1300, DMI type 19, 31 bytes at 0x6d35125
> +Header and Data:
> +: 13 1f 00 13 00 00 00 00 ff ff 01 00 00 10 01 00
> +0010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +
> +Handle 0x2000, DMI type 32, 11 bytes at 0x6d35146
> +Header and Data:
> +: 20 0b 00 20 00 00 00 00 00 00 00
> +
> +Handle 0x7f00, DMI type 127, 4 bytes at 0x6d35153
> +End Of Table
> +
> +Configuration
> +-
> +
> +The command is only available if CONFIG_CMD_SMBIOS=y.
> +
> +Return value
> +
> +
> +The return value $? is 0 (true) on success, 1 (false) otherwise.
> diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> index 1a626c03c2..eb2b2311f8 100644
> --- a/doc/usage/index.rst
> +++ b/doc/usage/index.rst
> @@ -102,6 +102,7 @@ Shell commands
> cmd/size
> cmd/sleep
> cmd/sm
> +   cmd/smbios
> cmd/sound
> cmd/source
> cmd/temperature
> --
> 2.43.0
>

Reviewed-by: Ilias Apalodimas 

Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header

[Ilias Apalodimas]

[...]

> - * @chksum: checksum for the entire bloblist allocated area. Since any of the
> - *   blobs can be altered after being created, this checksum is only valid
> - *   when the bloblist is finalised before jumping to the next stage of boot.
> - *   This is the value needed to make all checksummed bytes sum to 0
>   */
>  struct bloblist_hdr {
>   u32 magic;
> - u32 version;
> - u32 hdr_size;
> - u32 flags;
> -
> - u32 size;
> + u8 chksum;
> + u8 version;
> + u8 hdr_size;
> + u8 align_log2;
>   u32 alloced;
> + u32 size;
> + u32 flags;
>   u32 spare;
> - u32 chksum;
>  };
>

Aren't fields still missing from the current version?
e.g max_size and reserved?

>  /**
> diff --git a/test/bloblist.c b/test/bloblist.c
> index e6070041d3..a083259d0c 100644
> --- a/test/bloblist.c
> +++ b/test/bloblist.c
> @@ -78,7 +78,7 @@ static int bloblist_test_init(struct unit_test_state *uts)
>   ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR,
>TEST_BLOBLIST_SIZE));
>
> - ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0x10, 0));
> + ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc, 0));
>   ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0));
>   ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
>
> @@ -272,8 +272,8 @@ static int bloblist_test_cmd_info(struct unit_test_state 
> *uts)
>   run_command("bloblist info", 0);
>   ut_assert_nextline("base: %lx", (ulong)map_to_sysmem(hdr));
>   ut_assert_nextline("size: 4001 KiB");
> - ut_assert_nextline("alloced:  58 88 Bytes");
> - ut_assert_nextline("free: 3a8936 Bytes");
> + ut_assert_nextline("alloced:  50 80 Bytes");
> + ut_assert_nextline("free: 3b0944 Bytes");
>   ut_assert_console_end();
>   ut_unsilence_console(uts);
>
> --
> 2.25.1
>

Re: [PATCH v3 12/14] bloblist: Add alignment to bloblist_new()

[Ilias Apalodimas]

On Mon, 18 Dec 2023 at 20:20, Raymond Mao  wrote:
>
> From: Simon Glass 
>
> Allow the alignment to be specified when creating a bloblist.
>
> Signed-off-by: Simon Glass 
> Co-developed-by: Raymond Mao 
> Signed-off-by: Raymond Mao 
> ---
> Changes in v3
> - Keep the flag argument to align to FW handoff spec up to commit 3592349.
>
>  common/bloblist.c  |  5 +++--
>  include/bloblist.h |  3 ++-
>  test/bloblist.c| 40 ++--
>  3 files changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 1c97d61e4a..6d079c58f7 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -349,7 +349,7 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
> return chksum;
>  }
>
> -int bloblist_new(ulong addr, uint size, uint flags)
> +int bloblist_new(ulong addr, uint size, uint flags, uint align_log2)
>  {
> struct bloblist_hdr *hdr;
>
> @@ -365,6 +365,7 @@ int bloblist_new(ulong addr, uint size, uint flags)
> hdr->magic = BLOBLIST_MAGIC;
> hdr->size = size;
> hdr->alloced = hdr->hdr_size;
> +   hdr->align_log2 = align_log2 ?: BLOBLIST_BLOB_ALIGN_LOG2;

nit, but can we write this as a full ternary operator in the next release.

[...]

Reviewed-by: Ilias Apalodimas