Re: [PATCH] efi_loader: add /dtbs search path

2024-07-22 Thread Heinrich Schuchardt

On 7/22/24 19:55, Caleb Connolly wrote:

Add an additional search path /dtbs, this is where dtbs are installed on
postmarketOS and potentially other distros.

Signed-off-by: Caleb Connolly 
---
  lib/efi_loader/efi_fdt.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/lib/efi_loader/efi_fdt.c b/lib/efi_loader/efi_fdt.c
index 86ba00c2bdd9..4777943b80df 100644
--- a/lib/efi_loader/efi_fdt.c
+++ b/lib/efi_loader/efi_fdt.c
@@ -42,8 +42,11 @@ int efi_get_distro_fdt_name(char *fname, int size, int seq)
break;
case 2:
prefix = "/dtb/current";
break;
+   case 3:
+   prefix = "/dtbs";


Debian's flash-kernel also installs into /dtbs.

Reviewed-by: Heinrich Schuchardt 


+   break;
default:
return log_msg_ret("pref", -EINVAL);
}





Re: [PATCH 1/1] board: fix compatible property Milk-V Mars CM

2024-07-21 Thread Heinrich Schuchardt

On 7/21/24 23:27, E Shattow wrote:

P.S. my suggestion below

On Fri, Jul 19, 2024 at 5:06 PM E Shattow  wrote:


On Fri, Jul 19, 2024 at 4:12 PM Heinrich Schuchardt
 wrote:


For the Milk-V Mars CM (lite) we have only been copying sizeof(void *)
bytes to the compatible property instead of the whole string list.


This const char array must be so that we may get an accurate data
length with sizeof() but it highlights there are helper functions for
get of fdt stringlist and not for set of fdt stringlist.



Fixes: de3229599d4f ("board: add support for Milk-V Mars CM")
Reported-by: E Shattow 
Signed-off-by: Heinrich Schuchardt 
---
  board/starfive/visionfive2/spl.c | 15 ---
  1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
index b794b73b6bd..f55c6b5d34c 100644
--- a/board/starfive/visionfive2/spl.c
+++ b/board/starfive/visionfive2/spl.c
@@ -170,23 +170,32 @@ void spl_fdt_fixup_mars_cm(void *fdt)
  {
 const char *compat;
 const char *model;
+   int compat_size;

 spl_fdt_fixup_mars(fdt);

 if (!get_mmc_size_from_eeprom()) {
 int offset;
+   static const char
+   compat_cm_lite[] = "milkv,mars-cm-lite\0starfive,jh7110";

 model = "Milk-V Mars CM Lite";
-   compat = "milkv,mars-cm-lite\0starfive,jh7110";
+   compat = compat_cm_lite;
+   compat_size = sizeof(compat_cm_lite);

 offset = fdt_path_offset(fdt, 
"/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
 /* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE, GPI_NONE) */
 fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
 } else {
+   static const char
+   compat_cm[] = "milkv,mars-cm\0starfive,jh7110";
+
 model = "Milk-V Mars CM";
-   compat = "milkv,mars-cm\0starfive,jh7110";
+   compat = compat_cm;
+   compat_size = sizeof(compat_cm);
 }
-   fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat, 
sizeof(compat));
+   fdt_setprop(fdt, fdt_path_offset(fdt, "/"),
+   "compatible", compat, compat_size);
 fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
  }

--
2.45.2



-E


What about something like as follows?

  void spl_fdt_fixup_mars(void *fdt)
  {
-   static const char compat[] = "milkv,mars\0starfive,jh7110";
 u32 phandle;
 u8 i;
 int offset;
 int ret;

-   fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible",
compat, sizeof(compat));
-   fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
-  "Milk-V Mars");
+   offset = fdt_path_offset(fdt, "/");
+   fdt_setprop_string(fdt,offset, "compatible", "milkv,mars");
+   fdt_appendprop_string(fdt, offset, "compatible", "starfive,jh7110");
+   fdt_setprop_string(fdt,offset, "model",  "Milk-V Mars");

 /* gmac0 */
 offset = fdt_path_offset(fdt, "/soc/clock-controller@1700");
@@ -164,30 +226,31 @@ void spl_fdt_fixup_mars(void *fdt)
 break;
 }
 }
  }

  void spl_fdt_fixup_mars_cm(void *fdt)
  {
-   const char *compat;
-   const char *model;
+   int offset;

 spl_fdt_fixup_mars(fdt);

 if (!get_mmc_size_from_eeprom()) {
-   int offset;
-
-   model = "Milk-V Mars CM Lite";
-   compat = "milkv,mars-cm-lite\0starfive,jh7110";
+   offset = fdt_path_offset(fdt, "/");
+   fdt_setprop_string(fdt, offset, "compatible",
"milkv,mars-cm-lite");
+   fdt_appendprop_string(fdt, offset, "compatible",
"starfive,jh7110");
+   fdt_setprop_string(fdt, offset, "model", "Milk-V Mars CM Lite");

 offset = fdt_path_offset(fdt,
"/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
 /* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE, GPI_NONE) */
 fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
 } else {
-   model = "Milk-V Mars CM";
-   compat = "milkv,mars-cm\0starfive,jh7110";
+   offset = fdt_path_offset(fdt, "/");
+   fdt_setprop_string(fdt, offset, "compatible", "milkv,mars-cm");
+   fdt_appendprop_string(fdt, offset, "compatible",
"starfive,jh7110");
+ 

[PATCH v2 1/1] mmc: consider cd-gpios in Synopsys DesignWare driver

2024-07-21 Thread Heinrich Schuchardt
The JH7110 SoC uses a GPIO for card detect.

* In the of_to_plat function check if a cd-gpios definition exists and
  request the GPIO.
* In the getcd function return the GPIO value in this case.

Reported-by: Conor Dooley 
Signed-off-by: Heinrich Schuchardt 
---
v2:
Check that DM_GPIO is enabled to avoid build failure
on ARC platforms
---
 drivers/mmc/snps_dw_mmc.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/mmc/snps_dw_mmc.c b/drivers/mmc/snps_dw_mmc.c
index 9bdbe5070b1..aef05cb932f 100644
--- a/drivers/mmc/snps_dw_mmc.c
+++ b/drivers/mmc/snps_dw_mmc.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -29,6 +30,7 @@ struct snps_dwmci_plat {
 struct snps_dwmci_priv_data {
struct dwmci_host   host;
u32 f_max;
+   struct gpio_desccd_gpio;
 };
 
 static int snps_dwmmc_clk_setup(struct udevice *dev)
@@ -106,6 +108,10 @@ static int snps_dwmmc_of_to_plat(struct udevice *dev)
if (!ret && priv->f_max < CLOCK_MIN)
return -EINVAL;
 
+   if (CONFIG_IS_ENABLED(DM_GPIO))
+   gpio_request_by_name(dev, "cd-gpios", 0, >cd_gpio,
+GPIOD_IS_IN);
+
host->fifo_mode = dev_read_bool(dev, "fifo-mode");
host->name = dev->name;
host->dev_index = 0;
@@ -119,6 +125,9 @@ int snps_dwmmc_getcd(struct udevice *dev)
struct snps_dwmci_priv_data *priv = dev_get_priv(dev);
struct dwmci_host *host = >host;
 
+   if (CONFIG_IS_ENABLED(DM_GPIO) && dm_gpio_is_valid(>cd_gpio))
+   return dm_gpio_get_value(>cd_gpio);
+
return !(dwmci_readl(host, DWMCI_CDETECT) & 1);
 }
 
-- 
2.45.2



[PATCH 1/1] board: fix compatible property Milk-V Mars CM

2024-07-19 Thread Heinrich Schuchardt
For the Milk-V Mars CM (lite) we have only been copying sizeof(void *)
bytes to the compatible property instead of the whole string list.

Fixes: de3229599d4f ("board: add support for Milk-V Mars CM")
Reported-by: E Shattow 
Signed-off-by: Heinrich Schuchardt 
---
 board/starfive/visionfive2/spl.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
index b794b73b6bd..f55c6b5d34c 100644
--- a/board/starfive/visionfive2/spl.c
+++ b/board/starfive/visionfive2/spl.c
@@ -170,23 +170,32 @@ void spl_fdt_fixup_mars_cm(void *fdt)
 {
const char *compat;
const char *model;
+   int compat_size;
 
spl_fdt_fixup_mars(fdt);
 
if (!get_mmc_size_from_eeprom()) {
int offset;
+   static const char
+   compat_cm_lite[] = "milkv,mars-cm-lite\0starfive,jh7110";
 
model = "Milk-V Mars CM Lite";
-   compat = "milkv,mars-cm-lite\0starfive,jh7110";
+   compat = compat_cm_lite;
+   compat_size = sizeof(compat_cm_lite);
 
offset = fdt_path_offset(fdt, 
"/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
/* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE, GPI_NONE) */
fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
} else {
+   static const char
+   compat_cm[] = "milkv,mars-cm\0starfive,jh7110";
+
model = "Milk-V Mars CM";
-   compat = "milkv,mars-cm\0starfive,jh7110";
+   compat = compat_cm;
+   compat_size = sizeof(compat_cm);
}
-   fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat, 
sizeof(compat));
+   fdt_setprop(fdt, fdt_path_offset(fdt, "/"),
+   "compatible", compat, compat_size);
fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
 }
 
-- 
2.45.2



[PATCH 1/1] doc: define html_context in conf.py

2024-07-19 Thread Heinrich Schuchardt
The dictionary html_context is not passed into conf.py but must be created
there. See
https://dev.readthedocs.io/en/latest/design/theme-context.html#customizing-the-context

Fixes: df86796028df ("doc: enable ReadTheDocs addon management")
Signed-off-by: Heinrich Schuchardt 
---
 doc/conf.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/conf.py b/doc/conf.py
index e79134cc3d7..ced3a6723fc 100644
--- a/doc/conf.py
+++ b/doc/conf.py
@@ -26,7 +26,9 @@ html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "")
 
 # Tell Jinja2 templates the build is running on Read the Docs
 if os.environ.get("READTHEDOCS", "") == "True":
-html_context["READTHEDOCS"] = True
+html_context = {
+'READTHEDOCS' : True,
+}
 
 # If extensions (or modules to document with autodoc) are in another directory,
 # add these directories to sys.path here. If the directory is relative to the
-- 
2.45.2



Pull request efi-2024-10-rc1-3

2024-07-19 Thread Heinrich Schuchardt

Dear Tom,

The following changes since commit 45956736ac7c9c8b04522789c20fb45ff95a:

  Merge patch series "bootstd: Add Android support" (2024-07-18
17:03:47 -0600)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-efi.git
tags/efi-2024-10-rc1-3

for you to fetch changes up to 38b000881ebc0a48b0a814fce9f52dfe62ac644b:

  doc: Describe the bootstd settings (2024-07-19 14:01:46 +0200)

No issues were reported on Gitlab:

https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21685


Pull request efi-2024-10-rc1-3

Documentation:

* move out-of-tree building info to HTML
* enable ReadTheDocs addon management
* Remove FIT documentation that is elsewhere
* Update table of contents for FIT images
* Add description for more boot methods

UEFI:

* Correct finding distro device-path for media devices
* Fix typo in EFI_RT_VOLATILE_STORE description

Other:

* MAINTAINERS: Rename BOOTDEVICE

----
Heinrich Schuchardt (3):
  efi_loader: find distro device-path for media devices
  doc: move out-of-tree building info to HTML
  doc: enable ReadTheDocs addon management

Michal Simek (1):
  efi_loader: Fix typo in EFI_RT_VOLATILE_STORE description

Sam Povilus (2):
  doc: Remove FIT documentation that is elsewhere
  doc: add missing table of content links

Simon Glass (14):
  MAINTAINERS: Rename BOOTDEVICE
  doc: Move bootstd into its own directory
  doc: Mention automatic binding of bootmeths
  doc: Add a description for bootmeth_extlinux
  doc: Add a description for bootmeth_pxe
  doc: Add a description for bootmeth_qfw
  doc: Add a description for bootmeth_cros
  doc: Add a description for bootmeth_sandbox
  bootstd: Tidy up comments on the boothmeth drivers
  bootstd: Correct handling of script from network
  doc: Add a description for bootmeth_script
  doc: Add a link to VBE from the bootstd docs
  boot: Correct indentation in efi bootmeth
  doc: Describe the bootstd settings

 MAINTAINERS   |   4 +-
 README|  20 -
 boot/bootmeth_efi.c   |   3 +-
 boot/bootmeth_extlinux.c  |   2 +-
 boot/bootmeth_qfw.c   |   2 +-
 boot/bootmeth_sandbox.c   |   2 +-
 boot/bootmeth_script.c|  53 +-
 doc/board/starfive/milk-v_mars_cm.rst |   2 +-
 doc/build/gcc.rst |  28 +
 doc/conf.py   |   6 +
 doc/develop/board_best_practices.rst  |   2 +-
 doc/develop/bootstd/cros.rst  |  33 ++
 doc/develop/bootstd/extlinux.rst  |  29 +
 doc/develop/bootstd/index.rst |  15 +
 doc/develop/{bootstd.rst => bootstd/overview.rst} |  45 +-
 doc/develop/bootstd/pxelinux.rst  |  27 +
 doc/develop/bootstd/qfw.rst   |  20 +
 doc/develop/bootstd/sandbox.rst   |  17 +
 doc/develop/bootstd/script.rst|  52 ++
 doc/develop/index.rst |   2 +-
 doc/usage/cmd/bootdev.rst |   2 +-
 doc/usage/cmd/bootflow.rst|   2 +-
 doc/usage/cmd/bootmeth.rst|   2 +-
 doc/usage/environment.rst |   2 +-
 doc/usage/fit/index.rst   |  25 +-
 doc/usage/fit/source_file_format.rst  | 684
+-
 include/efi_loader.h  |   2 +-
 lib/efi_loader/Kconfig|   2 +-
 lib/efi_loader/efi_bootmgr.c  |   2 +-
 lib/efi_loader/efi_fdt.c  |  33 +-
 30 files changed, 350 insertions(+), 770 deletions(-)
 create mode 100644 doc/develop/bootstd/cros.rst
 create mode 100644 doc/develop/bootstd/extlinux.rst
 create mode 100644 doc/develop/bootstd/index.rst
 rename doc/develop/{bootstd.rst => bootstd/overview.rst} (95%)
 create mode 100644 doc/develop/bootstd/pxelinux.rst
 create mode 100644 doc/develop/bootstd/qfw.rst
 create mode 100644 doc/develop/bootstd/sandbox.rst
 create mode 100644 doc/develop/bootstd/script.rst



Re: [PATCH v2 10/21] upl: Add support for reading a upl handoff

2024-07-18 Thread Heinrich Schuchardt

On 13.07.24 09:00, Simon Glass wrote:

Universal Payload provides a standard way of handing off control between
two firmware phases. Add support for reading the handoff information into
a structure.

Signed-off-by: Simon Glass 
---

(no changes since v1)

  MAINTAINERS   |   7 +
  boot/Kconfig  |  19 ++
  boot/Makefile |   3 +
  boot/upl_common.c |  60 
  boot/upl_common.h |  24 ++
  boot/upl_read.c   | 588 ++
  configs/sandbox_defconfig |   1 +
  include/upl.h | 352 +++
  8 files changed, 1054 insertions(+)
  create mode 100644 boot/upl_common.c
  create mode 100644 boot/upl_common.h
  create mode 100644 boot/upl_read.c
  create mode 100644 include/upl.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 2c6de3a1d84..23852fbde38 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1694,6 +1694,13 @@ M:   Neha Malcom Francis 
  S:Maintained
  F:drivers/ufs/

+UPL
+M: Simon Glass 
+S: Maintained
+T: git https://source.denx.de/u-boot/custodians/u-boot-dm.git
+F: boot/upl*
+F: include/upl.h
+
  USB
  M:Marek Vasut 
  S:Maintained
diff --git a/boot/Kconfig b/boot/Kconfig
index ffcae840a50..f371772acb9 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -719,6 +719,25 @@ config BOOTMETH_SCRIPT
  This provides a way to try out standard boot on an existing boot flow.
  It is not enabled by default to save space.

+config UPL
+   bool "upl - Universal Payload Specification"
+   imply UPL_READ
+   help
+ Provides support for UPL payloads and handoff information. U-Boot
+ supports generating and accepting handoff information. The mkimage
+ tool will eventually support creating payloads.
+
+if UPL
+
+config UPL_READ
+   bool "upl - Support reading a Universal Payload handoff"
+   help
+ Provides support for decoding a UPL-format payload into a C structure
+ which can be used elsewhere in U-Boot. This is just the reading
+ implementation, useful for trying it out.
+
+endif  # UPL
+
  endif # BOOTSTD

  config LEGACY_IMAGE_FORMAT
diff --git a/boot/Makefile b/boot/Makefile
index 84ccfeaecec..c4720d93ca3 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -43,6 +43,9 @@ endif
  obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o
  obj-$(CONFIG_$(SPL_TPL_)FDT_SIMPLEFB) += fdt_simplefb.o

+obj-$(CONFIG_$(SPL_TPL_)UPL) += upl_common.o
+obj-$(CONFIG_$(SPL_TPL_)UPL_READ) += upl_read.o
+
  obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o
  obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += fdt_region.o
  obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o
diff --git a/boot/upl_common.c b/boot/upl_common.c
new file mode 100644
index 000..3924423abd5
--- /dev/null
+++ b/boot/upl_common.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * UPL handoff command functions
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass 
+ */
+
+#define LOG_CATEGORY UCLASS_BOOTSTD
+
+#include 
+#include 
+
+/* Names of bootmodes */
+const char *const bootmode_names[UPLBM_COUNT] = {
+   [UPLBM_FULL]= "full",
+   [UPLBM_MINIMAL] = "minimal",
+   [UPLBM_FAST]= "fast",
+   [UPLBM_DIAG]= "diag",
+   [UPLBM_DEFAULT] = "default",
+   [UPLBM_S2]  = "s2",
+   [UPLBM_S3]  = "s3",
+   [UPLBM_S4]  = "s4",
+   [UPLBM_S5]  = "s5",
+   [UPLBM_FACTORY] = "factory",
+   [UPLBM_FLASH]   = "flash",
+   [UPLBM_RECOVERY] = "recovery",
+};
+
+/* Names of memory usages */
+const char *const usage_names[UPLUS_COUNT] = {
+   [UPLUS_ACPI_RECLAIM]= "acpi-reclaim",
+   [UPLUS_ACPI_NVS]= "acpi-nvs",
+   [UPLUS_BOOT_CODE]   = "boot-code",
+   [UPLUS_BOOT_DATA]   = "boot-data",
+   [UPLUS_RUNTIME_CODE]= "runtime-code",
+   [UPLUS_RUNTIME_DATA]= "runtime-data",
+};
+
+/* Names of access types */
+const char *const access_types[UPLUS_COUNT] = {
+   [UPLAT_MMIO]= "mmio",
+   [UPLAT_IO]  = "io",
+};
+
+/* Names of graphics formats */
+const char *const graphics_formats[UPLUS_COUNT] = {
+   [UPLGF_ARGB32]  = "a8r8g8b8",
+   [UPLGF_ABGR32]  = "a8b8g8r8",
+   [UPLGF_ABGR64]  = "a16b16g16r16",
+};
+
+void upl_init(struct upl *upl)
+{
+   memset(upl, '\0', sizeof(struct upl));
+   alist_init_struct(>image, struct upl_image);
+   alist_init_struct(>mem, struct upl_mem);
+   alist_init_struct(>memmap, struct upl_memmap);
+   alist_init_struct(>memres, struct upl_memres);
+}
diff --git a/boot/upl_common.h b/boot/upl_common.h
new file mode 100644
index 000..cc517dc1de9
--- /dev/null
+++ b/boot/upl_common.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * UPL handoff command functions
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass 
+ */
+
+#ifndef __UPL_COMMON_H
+#define __UPL_COMMON_H
+
+/* Names of bootmodes */
+extern const char 

[PATCH 1/1] efi_loader: require EFI boot manager for EBBR compliance

2024-07-18 Thread Heinrich Schuchardt
A system has to support booting via the boot manager to be EBBR compliant.
See the reference to variables Boot in the specification.

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 2fb24d7af9a..3b703fea007 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -486,6 +486,7 @@ config EFI_ECPT
 
 config EFI_EBBR_2_1_CONFORMANCE
bool "Add the EBBRv2.1 conformance entry to the ECPT table"
+   depends on BOOTMETH_EFI_BOOTMGR
depends on EFI_ECPT
depends on EFI_LOADER_HII
depends on EFI_RISCV_BOOT_PROTOCOL || !RISCV
-- 
2.45.2



Re: [PATCH v2 00/21] Universal Payload initial series

2024-07-18 Thread Heinrich Schuchardt

On 16.07.24 21:08, Tom Rini wrote:

On Sat, Jul 13, 2024 at 09:40:17PM +0200, Heinrich Schuchardt wrote:



Am 13. Juli 2024 10:12:50 MESZ schrieb Mark Kettenis :

From: Simon Glass 
Date: Sat, 13 Jul 2024 08:00:34 +0100

Universal Payload (UPL) is an Industry Standard for firmware
components[1].


I think you have some trouble understanding the concept of industry
standard ;).  I guess you want this to become an industry standard.
Firmly https://xkcd.com/927/ territory if you ask me.


UPL is designed to improve interoperability within the
firmware industry, allowing mixing and matching of projects with less
friction and fewer project-specific implementations. UPL is
cross-platform, supporting ARM, x86 and RISC-V initially.

This series provides some initial support for this, targeting 0.9.1 and
sandbox only.

Features still to come include:
- Support for architectures
- FIT validation
- Handoff validation
- Interoperability tests

This series is available at dm/uplb-working and requires the alist
series at dm/alist-working[2]


Why is this series needed?


Because UPL is a standard, supported by other projects, which requires
more than Just Nothing Else on top of what we do today. Please do
provide constructive feedback on the changes themselves, but bringing in
UPL support proper is something we should (and will) do. Thanks!



SPL loading a FIT image (e.g. EDK II + OpenSBI) seems to be very close
to UPL. What is missing?

UPL is meant as the interface between two firmware pieces and not
between firmware and the OS. Why should we let main U-Boot support UPL?
Isn't SPL support sufficient?

Which other projects implement the UPL standard?

Best regards

Heinrich


[PATCH] cmd: move CMD_DHCP6 options beneath CMD_DHCP6

2024-07-17 Thread Heinrich Schuchardt
All Kconfig options that depend on CONFIG_CMD_DHCP6 should immediately
follow it.

Signed-off-by: Heinrich Schuchardt 
---
 cmd/Kconfig | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 0cf0d8ad8ab..323e473cd64 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1810,6 +1810,23 @@ config CMD_DHCP6
  Will perform 4-message exchange with DHCPv6 server, requesting
  the minimum required options to TFTP boot. Complies with RFC 8415.
 
+if CMD_DHCP6
+
+config DHCP6_PXE_CLIENTARCH
+   hex
+   default 0x16 if ARM64
+   default 0x15 if ARM
+   default 0xFF
+
+config DHCP6_PXE_DHCP_OPTION
+   bool "Request & store 'pxe_configfile' from DHCP6 server"
+
+config DHCP6_ENTERPRISE_ID
+   int "Enterprise ID to send in DHCPv6 Vendor Class Option"
+   default 0
+
+endif
+
 config BOOTP_MAY_FAIL
bool "Allow for the BOOTP/DHCP server to not be found"
depends on CMD_BOOTP
@@ -1927,23 +1944,6 @@ config BOOTP_VCI_STRING
default "U-Boot.arm" if ARM
default "U-Boot"
 
-if CMD_DHCP6
-
-config DHCP6_PXE_CLIENTARCH
-   hex
-   default 0x16 if ARM64
-   default 0x15 if ARM
-   default 0xFF
-
-config DHCP6_PXE_DHCP_OPTION
-   bool "Request & store 'pxe_configfile' from DHCP6 server"
-
-config DHCP6_ENTERPRISE_ID
-   int "Enterprise ID to send in DHCPv6 Vendor Class Option"
-   default 0
-
-endif
-
 config CMD_TFTPBOOT
bool "tftpboot"
default y
-- 
2.45.2



Re: Request for hosting a boot-firmware repository in u-boot git (denx and GitHub)

2024-07-16 Thread Heinrich Schuchardt

On 7/16/24 22:01, Tom Rini wrote:

On Tue, Jul 16, 2024 at 09:35:18PM +0200, Heinrich Schuchardt wrote:

On 7/16/24 21:13, Tom Rini wrote:

On Thu, Jun 20, 2024 at 04:35:39PM -0500, Nishanth Menon wrote:

Hi Team,

We have briefly discussed this topic on IRC[1]. I would like to
propose a new boot-firmware repository similar to the Linux-firmware
repository under the aegis of u-boot hosting.

In addition to TI, it looks like some NXP[2] and Rockchip[3]
platforms seem to require additional closed-source/open-source
binaries to have a complete bootable image. Distribution rights and
locations of these binaries are challenging, and there needs to be a
standard for how and where they are hosted for end users.

Further, looking ahead to future architectures:
* IP firmware: More and more IP vendors are embedding their own
"specialized controllers" and require firmware for the operation
(similar to Rockchip's DDR controller, I guess),
* boot stage firmware: Additional stages of the boot process involve
vendor intermediate firmware, such as power configuration.
* Security enclave binaries: While I see a few folks trying to have an
open-source s/w architecture, many PKA and PQC systems still require
prop binaries for IP reasons.

NOTE: I am not judging any company(including TI) for reasons why some
firmware is proprietary, but I hate to have the end users and other
system (distro) maintainers have to deal with hell trying to make the
life of end users easy to live with.

In the case of TI's K3 architecture devices, we have two binary blobs
that are critical for the boot process.

1. TIFS Firmware / DMSC firmware[4]—This is the security enclave
firmware. It is often encrypted, and sources are not public (due to
various business/regulatory reasons).
2. DM Firmware[5] - There is a source in public in some cases and
binary only in others - essentially limited function binary to be
put up in the device management uC. In cases where the source is
available, the build procedure is, in my personal opinion, pretty
arcane, and even though in theory it is practical, in practice, not
friendly - efforts are going to simplify it, even probably integrate
it with a more opensource ecosystem, but that is talking "look at the
tea leaves" stuff.
3. Low Power Management (LPM) binaries: tifs stub: another encrypted
binary that gives the tifs system context restore logic before
retrieving tifs firmware and a corresponding DM restoration binary.

All told, this is not unlike the situation that necessitated the
creation of a Linux firmware repository.

Options that I see:

1. Let the status quo be - SoC vendors maintain random locations and
random rules to maintain boot firmware.
2. Ask Linux-firmware to host the binaries in a single canonical
location
3. Host a boot-firmware repository - u-boot repo may be the more
logical location.

* (1) isn't the correct answer.

* (2) Though I haven't seen any policy from the Linux-firmware
community mandating anything of the form, the binaries we are talking
of may not belong to Linux-firmware as they aren't strictly speaking
something Linux kernel will load (since the bootloader has that
responsibility), and in some cases may not even directly talk to
(security enclave or DDR firmware stuff). I am adding Josh to this
mail to see if he has any opinions on the topic (but keeping
from cross posting on linux-firmware list, unless folks feel it is
OK).

On (3):
Proposal:

* Create a boot firmware repository in Denx and/or GitHub (if
financials are a hurdle, I hope we can solve it as a community).
* Limit binaries only to those consumed part of the u-boot scope.

* Limit binaries only to those that do not have an opensource project
(Trusted Firmware-A/M, OP-TEE, etc..) or depend entirely on vendor
source or are binary only in nature (subject to licensing terms below)
* Limit binaries to some pre-established size to prevent repository
explosion - say, 512Kib?
* Follow the same rules of integration and licensing guidelines as
Linux-firmware[6].
* Similar rules as Linux-firmware guidelines of ABI backward and
forward compatibility.
* Set a workflow update flow and a compatibility requirements document

If we agree to have boot firmware under the stewardship of u-boot, we
should also set other rules, which is excellent to discuss.

Thoughts?


I believe that fundamentally, this is a problem that exists beyond both
just "U-Boot needs some binaries" and "TI has some binaries that
bootloaders need". So a generic solution is appropriate, and some sort
of community-based hosting of these needs (with appropriate licensing
from the IP owners) makes sense. Looking around at the binaries I have
to keep locally to use NXP platforms, and TI platforms and Rockchip
platforms, it's far from ideal. Having one place to get them all from
would make life

Re: Request for hosting a boot-firmware repository in u-boot git (denx and GitHub)

2024-07-16 Thread Heinrich Schuchardt

On 7/16/24 21:13, Tom Rini wrote:

On Thu, Jun 20, 2024 at 04:35:39PM -0500, Nishanth Menon wrote:

Hi Team,

We have briefly discussed this topic on IRC[1]. I would like to
propose a new boot-firmware repository similar to the Linux-firmware
repository under the aegis of u-boot hosting.

In addition to TI, it looks like some NXP[2] and Rockchip[3]
platforms seem to require additional closed-source/open-source
binaries to have a complete bootable image. Distribution rights and
locations of these binaries are challenging, and there needs to be a
standard for how and where they are hosted for end users.

Further, looking ahead to future architectures:
* IP firmware: More and more IP vendors are embedding their own
   "specialized controllers" and require firmware for the operation
   (similar to Rockchip's DDR controller, I guess),
* boot stage firmware: Additional stages of the boot process involve
   vendor intermediate firmware, such as power configuration.
* Security enclave binaries: While I see a few folks trying to have an
   open-source s/w architecture, many PKA and PQC systems still require
   prop binaries for IP reasons.

NOTE: I am not judging any company(including TI) for reasons why some
firmware is proprietary, but I hate to have the end users and other
system (distro) maintainers have to deal with hell trying to make the
life of end users easy to live with.

In the case of TI's K3 architecture devices, we have two binary blobs
that are critical for the boot process.

1. TIFS Firmware / DMSC firmware[4]—This is the security enclave
   firmware. It is often encrypted, and sources are not public (due to
   various business/regulatory reasons).
2. DM Firmware[5] - There is a source in public in some cases and
   binary only in others - essentially limited function binary to be
   put up in the device management uC. In cases where the source is
   available, the build procedure is, in my personal opinion, pretty
   arcane, and even though in theory it is practical, in practice, not
   friendly - efforts are going to simplify it, even probably integrate
   it with a more opensource ecosystem, but that is talking "look at the
   tea leaves" stuff.
3. Low Power Management (LPM) binaries: tifs stub: another encrypted
   binary that gives the tifs system context restore logic before
   retrieving tifs firmware and a corresponding DM restoration binary.

All told, this is not unlike the situation that necessitated the
creation of a Linux firmware repository.

Options that I see:

1. Let the status quo be - SoC vendors maintain random locations and
   random rules to maintain boot firmware.
2. Ask Linux-firmware to host the binaries in a single canonical
   location
3. Host a boot-firmware repository - u-boot repo may be the more
   logical location.

* (1) isn't the correct answer.

* (2) Though I haven't seen any policy from the Linux-firmware
   community mandating anything of the form, the binaries we are talking
   of may not belong to Linux-firmware as they aren't strictly speaking
   something Linux kernel will load (since the bootloader has that
   responsibility), and in some cases may not even directly talk to
   (security enclave or DDR firmware stuff). I am adding Josh to this
   mail to see if he has any opinions on the topic (but keeping
   from cross posting on linux-firmware list, unless folks feel it is
   OK).

On (3):
Proposal:

* Create a boot firmware repository in Denx and/or GitHub (if
   financials are a hurdle, I hope we can solve it as a community).
* Limit binaries only to those consumed part of the u-boot scope.

* Limit binaries only to those that do not have an opensource project
   (Trusted Firmware-A/M, OP-TEE, etc..) or depend entirely on vendor
   source or are binary only in nature (subject to licensing terms below)
* Limit binaries to some pre-established size to prevent repository
   explosion - say, 512Kib?
* Follow the same rules of integration and licensing guidelines as
   Linux-firmware[6].
* Similar rules as Linux-firmware guidelines of ABI backward and
   forward compatibility.
* Set a workflow update flow and a compatibility requirements document

If we agree to have boot firmware under the stewardship of u-boot, we
should also set other rules, which is excellent to discuss.

Thoughts?


I believe that fundamentally, this is a problem that exists beyond both
just "U-Boot needs some binaries" and "TI has some binaries that
bootloaders need". So a generic solution is appropriate, and some sort
of community-based hosting of these needs (with appropriate licensing
from the IP owners) makes sense. Looking around at the binaries I have
to keep locally to use NXP platforms, and TI platforms and Rockchip
platforms, it's far from ideal. Having one place to get them all from
would make life easier for a lot of developers and also frankly for a
lot of end customers of these chips.



Some thought needs to be given to the license implications of these 
binaries for 

[PATCH 1/1] doc: enable ReadTheDocs addon management

2024-07-16 Thread Heinrich Schuchardt
Up to now ReadTheDocs has been injecting code when building on their
platform. This includes for instance improvements for the search function.

To maintain the current output ReadTheDocs requires setting html_baseurl
and html_context in conf.py.

See: https://about.readthedocs.com/blog/2024/07/addons-by-default/

Signed-off-by: Heinrich Schuchardt 
---
 doc/conf.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/doc/conf.py b/doc/conf.py
index c9138a5a5d4..e79134cc3d7 100644
--- a/doc/conf.py
+++ b/doc/conf.py
@@ -21,6 +21,12 @@ from subprocess import check_output
 # Get Sphinx version
 major, minor, patch = sphinx.version_info[:3]
 
+# Set canonical URL from the Read the Docs Domain
+html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "")
+
+# Tell Jinja2 templates the build is running on Read the Docs
+if os.environ.get("READTHEDOCS", "") == "True":
+html_context["READTHEDOCS"] = True
 
 # If extensions (or modules to document with autodoc) are in another directory,
 # add these directories to sys.path here. If the directory is relative to the
-- 
2.45.2



[PATCH v2 1/1] boot: provide CONFIG_BOOTMETH_QFW Kconfig parameter

2024-07-16 Thread Heinrich Schuchardt
U-Boot is often used conjunction with QEMU to boot via EFI or syslinux.
Here the QFW boot method is not needed.

At least for qemu-riscv64_smode_defconfig the kernel parameter is used
to specify the U-Boot binary. Trying to run U-Boot as a kernel makes
no sense.

Provide Kconfig parameter CONFIG_BOOTMETH_QFW to decide if the QFW boot
method shall be provided.

Signed-off-by: Heinrich Schuchardt 
---
v2:
default y on all architectures
---
 boot/Kconfig  | 9 +
 boot/Makefile | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index 95300b008c5..81e7ef197ae 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -568,6 +568,15 @@ config BOOTMETH_EFI_BOOTMGR
  the EFI binary to be launched is determined. To set the EFI variables
  use the eficonfig command.
 
+config BOOTMETH_QFW
+   bool "Boot method using QEMU parameters"
+   depends on QFW
+   default y
+   help
+Use QEMU parameters -kernel, -initrd, -append to determine the kernel,
+initial RAM disk, and kernel command line parameters to boot an
+operating system. U-Boot's control device-tree is passed to the kernel.
+
 config BOOTMETH_VBE
bool "Bootdev support for Verified Boot for Embedded"
depends on FIT
diff --git a/boot/Makefile b/boot/Makefile
index 84ccfeaecec..92e6adb887c 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -11,7 +11,6 @@ obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
 obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
 
 obj-$(CONFIG_PXE_UTILS) += pxe_utils.o
-obj-$(CONFIG_QFW) += bootmeth_qfw.o
 
 endif
 
@@ -31,6 +30,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX) += 
bootmeth_extlinux.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX_PXE) += bootmeth_pxe.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EFILOADER) += bootmeth_efi.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_QFW) += bootmeth_qfw.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o
 obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o
-- 
2.45.2



Re: [PATCH 1/1] boot: provide CONFIG_BOOTMETH_QFW Kconfig parameter

2024-07-16 Thread Heinrich Schuchardt

On 7/16/24 09:04, Simon Glass wrote:

Hi Heinrich,

On Tue, 16 Jul 2024 at 03:40, Heinrich Schuchardt
 wrote:


U-Boot is often used conjunction with QEMU to boot via EFI or syslinux.
Here the QFW boot method is not needed.

At least for qemu-riscv64_smode_defconfig the kernel parameter is used
to specify the U-Boot binary. Trying to run U-Boot as a kernel makes
no sense.

Provide Kconfig parameter CONFIG_BOOTMETH_QFW to decide if the QFW boot
method shall be provided.

Disable the QFW boot method for all architectures but the sandbox by
default.

Signed-off-by: Heinrich Schuchardt 
---
  boot/Kconfig  | 9 +
  boot/Makefile | 2 +-
  2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index 95300b008c5..d7e034c89e7 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -568,6 +568,15 @@ config BOOTMETH_EFI_BOOTMGR
   the EFI binary to be launched is determined. To set the EFI variables
   use the eficonfig command.

+config BOOTMETH_QFW
+   bool "Boot method using QEMU parameters"
+   depends on QFW
+   default y if SANDBOX
+   help
+Use QEMU parameters -kernel, -initrd, -append to determine the kernel,
+initial RAM disk, and kernel command line parameters to boot an
+operating system. U-Boot's control device-tree is passed to the kernel.


I added the bootmeth to deal with this part of the old qemu-arm.h :

/* Try files from QEMU's -kernel/-initrd, through the QEMU firmware device. */
#define BOOTENV_DEV_QFW(devtypeu, devtypel, instance) \
"bootcmd_qfw= " \
"if qfw load $kernel_addr_r $ramdisk_addr_r; then " \
"  booti $kernel_addr_r $ramdisk_addr_r:$filesize $fdtcontroladdr; " \
"  if test $? -eq 1; then " \
"bootz $kernel_addr_r $ramdisk_addr_r:$filesize $fdtcontroladdr; " \
"  fi ; " \
"fi\0"
#define BOOTENV_DEV_NAME_QFW(devtypeu, devtypel, instance) "qfw "

Are you sure this is not used?


Those lines were only on ARM. On RISC-V the -bios parameter is used to 
replace the OpenSBI embedded in QEMU and -kernel is used for S-mode U-Boot.


So on RISC-V this boot method would only be usable for 
CONFIG_SPL_RISCV_MMODE=y or CONFIG_RISCV_MMODE=y.


Let me resubmit with default=y. That way the method is at least 
customizable.


Best regards

Heirnich




+
  config BOOTMETH_VBE
 bool "Bootdev support for Verified Boot for Embedded"
 depends on FIT
diff --git a/boot/Makefile b/boot/Makefile
index 84ccfeaecec..92e6adb887c 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -11,7 +11,6 @@ obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
  obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o

  obj-$(CONFIG_PXE_UTILS) += pxe_utils.o
-obj-$(CONFIG_QFW) += bootmeth_qfw.o

  endif

@@ -31,6 +30,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX) += 
bootmeth_extlinux.o
  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX_PXE) += bootmeth_pxe.o
  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EFILOADER) += bootmeth_efi.o
  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_QFW) += bootmeth_qfw.o
  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
  obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o
  obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o
--
2.45.2



Regards,
Simon




Re: [PATCH] doc: fit: add ELF image to list of image formats

2024-07-15 Thread Heinrich Schuchardt

On 7/15/24 21:33, Maxim Moskalets wrote:

Signed-off-by: Maxim Moskalets 


Please, always provide a commit message. Otherwise looks good to me.

This series suggested to eliminate source_file_format.rst:

https://patchwork.ozlabs.org/project/uboot/list/?series=414308

Best regards

Heinrich


---
  doc/usage/fit/source_file_format.rst | 1 +
  1 file changed, 1 insertion(+)

diff --git a/doc/usage/fit/source_file_format.rst 
b/doc/usage/fit/source_file_format.rst
index 15990e3ff54..cb5da5af2dd 100644
--- a/doc/usage/fit/source_file_format.rst
+++ b/doc/usage/fit/source_file_format.rst
@@ -287,6 +287,7 @@ os
  arm-trusted-firmware  ARM Trusted Firmware
  dell  Dell
  efi   EFI Firmware
+elf   ELF Image
  esix  Esix
  freebsd   FreeBSD
  integrity INTEGRITY




[PATCH 1/1] boot: provide CONFIG_BOOTMETH_QFW Kconfig parameter

2024-07-15 Thread Heinrich Schuchardt
U-Boot is often used conjunction with QEMU to boot via EFI or syslinux.
Here the QFW boot method is not needed.

At least for qemu-riscv64_smode_defconfig the kernel parameter is used
to specify the U-Boot binary. Trying to run U-Boot as a kernel makes
no sense.

Provide Kconfig parameter CONFIG_BOOTMETH_QFW to decide if the QFW boot
method shall be provided.

Disable the QFW boot method for all architectures but the sandbox by
default.

Signed-off-by: Heinrich Schuchardt 
---
 boot/Kconfig  | 9 +
 boot/Makefile | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index 95300b008c5..d7e034c89e7 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -568,6 +568,15 @@ config BOOTMETH_EFI_BOOTMGR
  the EFI binary to be launched is determined. To set the EFI variables
  use the eficonfig command.
 
+config BOOTMETH_QFW
+   bool "Boot method using QEMU parameters"
+   depends on QFW
+   default y if SANDBOX
+   help
+Use QEMU parameters -kernel, -initrd, -append to determine the kernel,
+initial RAM disk, and kernel command line parameters to boot an
+operating system. U-Boot's control device-tree is passed to the kernel.
+
 config BOOTMETH_VBE
bool "Bootdev support for Verified Boot for Embedded"
depends on FIT
diff --git a/boot/Makefile b/boot/Makefile
index 84ccfeaecec..92e6adb887c 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -11,7 +11,6 @@ obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
 obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
 
 obj-$(CONFIG_PXE_UTILS) += pxe_utils.o
-obj-$(CONFIG_QFW) += bootmeth_qfw.o
 
 endif
 
@@ -31,6 +30,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX) += 
bootmeth_extlinux.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EXTLINUX_PXE) += bootmeth_pxe.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_EFILOADER) += bootmeth_efi.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_QFW) += bootmeth_qfw.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o
 obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o
-- 
2.45.2



Re: [PATCH 08/13] bootstd: Tidy up comments on the boothmeth drivers

2024-07-15 Thread Heinrich Schuchardt

On 7/15/24 12:13, Simon Glass wrote:

Fix a typo in the comment and add one to the EFI driver too.

Signed-off-by: Simon Glass 


Reviewed-by: Heinrich Schuchardt 


---

  boot/bootmeth_efi.c  | 1 +
  boot/bootmeth_extlinux.c | 2 +-
  boot/bootmeth_script.c   | 2 +-
  3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 5a4c125835a..56a6e47f5b2 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -394,6 +394,7 @@ static const struct udevice_id distro_efi_bootmeth_ids[] = {
{ }
  };

+/* Put a number before 'efi' to provide a default ordering */
  U_BOOT_DRIVER(bootmeth_4efi) = {
.name   = "bootmeth_efi",
.id = UCLASS_BOOTMETH,
diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c
index 9b55686948f..fbb05ef928e 100644
--- a/boot/bootmeth_extlinux.c
+++ b/boot/bootmeth_extlinux.c
@@ -183,7 +183,7 @@ static const struct udevice_id extlinux_bootmeth_ids[] = {
{ }
  };

-/* Put an number before 'extlinux' to provide a default ordering */
+/* Put a number before 'extlinux' to provide a default ordering */
  U_BOOT_DRIVER(bootmeth_1extlinux) = {
.name   = "bootmeth_extlinux",
.id = UCLASS_BOOTMETH,
diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
index 0e05d28d4d9..24da47c7259 100644
--- a/boot/bootmeth_script.c
+++ b/boot/bootmeth_script.c
@@ -250,7 +250,7 @@ static const struct udevice_id script_bootmeth_ids[] = {
{ }
  };

-/* Put an number before 'script' to provide a default ordering */
+/* Put a number before 'script' to provide a default ordering */
  U_BOOT_DRIVER(bootmeth_2script) = {
.name   = "bootmeth_script",
.id = UCLASS_BOOTMETH,




Re: [PATCH 11/13] doc: Add a link to VBE from the bootstd docs

2024-07-15 Thread Heinrich Schuchardt

On 7/15/24 12:13, Simon Glass wrote:

Link to this page to make it easier to find the VBE docs.

Signed-off-by: Simon Glass 
---

  doc/develop/bootstd/overview.rst | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/develop/bootstd/overview.rst b/doc/develop/bootstd/overview.rst
index 7d31d5e6427..0d7454246f6 100644
--- a/doc/develop/bootstd/overview.rst
+++ b/doc/develop/bootstd/overview.rst
@@ -416,7 +416,7 @@ Bootmeth drivers are provided for:
 - :doc:`extlinux / syslinux ` boot from a network (PXE)
 - :doc:`U-Boot scripts 

Re: [PATCH 12/13] boot: Correct indentation in efi bootmeth

2024-07-15 Thread Heinrich Schuchardt

On 7/15/24 12:13, Simon Glass wrote:

Fix a minor indentation / whitespace problem in a comment.

Signed-off-by: Simon Glass 


Reviewed-by: Heinrich Schuchardt 


---

  boot/bootmeth_efi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 56a6e47f5b2..39232eb2e25 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -265,7 +265,7 @@ static int distro_efi_read_bootflow_net(struct bootflow 
*bflow)
return log_msg_ret("sz", -EINVAL);
bflow->size = size;

-/* bootfile should be setup by dhcp*/
+   /* bootfile should be setup by dhcp */
bootfile_name = env_get("bootfile");
if (!bootfile_name)
return log_msg_ret("bootfile_name", ret);




Re: [PATCH 06/13] doc: Add a description for bootmeth_cros

2024-07-15 Thread Heinrich Schuchardt

On 7/15/24 12:13, Simon Glass wrote:

Add documentation for the cros bootmeth.


%s/bootmeth/boot method/

throughout the patch.



Signed-off-by: Simon Glass 
---

  doc/develop/bootstd/cros.rst | 33 
  doc/develop/bootstd/index.rst|  1 +
  doc/develop/bootstd/overview.rst |  1 +
  3 files changed, 35 insertions(+)
  create mode 100644 doc/develop/bootstd/cros.rst

diff --git a/doc/develop/bootstd/cros.rst b/doc/develop/bootstd/cros.rst
new file mode 100644
index 000..96f148837ac
--- /dev/null
+++ b/doc/develop/bootstd/cros.rst
@@ -0,0 +1,33 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+ChromiumOS Bootmeth
+===
+
+ChromiumOS provides a mechanism for booting its Operating System from a block
+device, described
+`here 
`_.
+
+U-Boot includes support for reading the associated data structures from the
+device and identifying a bootable ChromiumOS image. This structure includes the
+kernel itself, boot arguments (kernel command line), as well as the x86 setup
+block (for x86 only).
+
+When invoked on a bootdev, this bootmeth searches for kernel partitions with


What does 'invoked on a boot device' mean? Do you mean "if ChromimumOS
selects a boot device"?


+the appropriate GUID (Globally Unique Identifier). When found, the information
+is loaded and a bootflow is created.
+
+When the bootflow is booted, the bootmeth reads the kernel and boot arguments.
+It then boots the kernel using zboot (on x86) or bootm (on ARM). The boot
+arguments are adjusted to replace %U with the UUID of the selected kernel
+partition. This results in the correct root disk being used, which is the next
+partition after the kernel partition.
+
+For ARM, a :doc:`/usage/fit/index` is used. The `CONFIG_FIT_BEST_MATCH` option
+must be enabled for U-Boot to select the correct devicetree to boot with.
+
+Note that a ChromiumOS image typically has two copies of the OS, each with its


%s/OS/operating system/


+own kernel and root disk. There is no initial ramdisk (initrd). This means that
+this bootmeth typically locates two separate images.
+
+The compatible string "u-boot,cros" is used for the driver. The driver is
+automatically instantiated if there are no bootmeth drivers in the devicetree.


The last sentence should be moved to the overview.

Please, mention the relevant configuration option for the boot method
driver.

Best regards

Heinrich



diff --git a/doc/develop/bootstd/index.rst b/doc/develop/bootstd/index.rst
index f8fce7207ce..69fd3c2d2eb 100644
--- a/doc/develop/bootstd/index.rst
+++ b/doc/develop/bootstd/index.rst
@@ -10,3 +10,4 @@ Standard Boot
 extlinux
 pxelinux
 qfw
+   cros
diff --git a/doc/develop/bootstd/overview.rst b/doc/develop/bootstd/overview.rst
index bcc2c00c775..f12e93236a7 100644
--- a/doc/develop/bootstd/overview.rst
+++ b/doc/develop/bootstd/overview.rst
@@ -417,6 +417,7 @@ Bootmeth drivers are provided for:
 - U-Boot scripts from disk, network or SPI flash
 - EFI boot using bootefi from disk
 - VBE
+   - :doc:`ChromiumOS ` ChromiumOS boot from a disk
 - EFI boot using boot manager
 - :doc:`QFW `: QEMU firmware interface





Re: [PATCH 05/13] doc: Add a description for bootmeth_qfw

2024-07-15 Thread Heinrich Schuchardt

On 7/15/24 12:13, Simon Glass wrote:

Add documentation for the qfw bootmeth.



%s/bootmeth/boot method/



Fix up the compatible string to drop the 'extlinux' part, which is not
relevant to this bootmeth.

Signed-off-by: Simon Glass 
---

  boot/bootmeth_qfw.c  |  2 +-
  doc/develop/bootstd/index.rst|  1 +
  doc/develop/bootstd/overview.rst |  1 +
  doc/develop/bootstd/qfw.rst  | 21 +
  4 files changed, 24 insertions(+), 1 deletion(-)
  create mode 100644 doc/develop/bootstd/qfw.rst

diff --git a/boot/bootmeth_qfw.c b/boot/bootmeth_qfw.c
index dfaa944594e..2f8e00cf350 100644
--- a/boot/bootmeth_qfw.c
+++ b/boot/bootmeth_qfw.c
@@ -88,7 +88,7 @@ static struct bootmeth_ops qfw_bootmeth_ops = {
  };

  static const struct udevice_id qfw_bootmeth_ids[] = {
-   { .compatible = "u-boot,qfw-extlinux" },
+   { .compatible = "u-boot,qfw-bootmeth" },
{ }
  };

diff --git a/doc/develop/bootstd/index.rst b/doc/develop/bootstd/index.rst
index 5052afe448f..f8fce7207ce 100644
--- a/doc/develop/bootstd/index.rst
+++ b/doc/develop/bootstd/index.rst
@@ -9,3 +9,4 @@ Standard Boot
 overview
 extlinux
 pxelinux
+   qfw
diff --git a/doc/develop/bootstd/overview.rst b/doc/develop/bootstd/overview.rst
index fd0692daf0d..bcc2c00c775 100644
--- a/doc/develop/bootstd/overview.rst
+++ b/doc/develop/bootstd/overview.rst
@@ -418,6 +418,7 @@ Bootmeth drivers are provided for:
 - EFI boot using bootefi from disk
 - VBE
 - EFI boot using boot manager
+   - :doc:`QFW `: QEMU firmware interface


  Command interface
diff --git a/doc/develop/bootstd/qfw.rst b/doc/develop/bootstd/qfw.rst
new file mode 100644
index 000..b0d47fd246a
--- /dev/null
+++ b/doc/develop/bootstd/qfw.rst
@@ -0,0 +1,21 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+QFW Bootmeth
+
+
+`QEMU https://www.qemu.org/>`_ is a system emulator which is able to boot
+Operating Systems. QEMU provides specific support for booting an OS image
+provided on the QEMU command line.
+
+When invoked on a bootdev for UCLASS_QFW, this bootmeth reads the kernel
+provided by the QEMU `-kernel` argument, the iniital ramdisk and provided by


%s/iniital/initial/


+`-initrd` and the boot arguments (command line) provided by `-append` into
+memory ready for booting.
+
+When the bootflow is booted, the bootmeth tries the `booti` command first, then


Boot methods should work if the command line is deactivated. We should
not use run_command() but extract the functionality into library code.

Why would you first try to run booti and then bootz irrespective of the
availability of the commands? Function qfw_boot() needs rework.

There should be a configuration symbol to disable the boot method when
CONFIG_QFW is enabled.

The bootmethod should no be built if booti and bootz are not available


+falls back to the `bootz` command. U-Boot's 'control' devicetree is passed
+through to the kernel.
+
+The compatible string "u-boot,qfw-bootmeth" is used for the driver. The driver
+is automatically instantiated if there are no bootmeth drivers in the
+devicetree.


This paragraph should be moved to the overview.

Please, mention the configuration requirements.

Best regards

Heinrich



Re: [PATCH 04/13] doc: Add a description for bootmeth_pxe

2024-07-15 Thread Heinrich Schuchardt

On 7/15/24 12:13, Simon Glass wrote:

Add documentation for the pxe bootmeth.

Signed-off-by: Simon Glass 
---

  doc/develop/bootstd/index.rst|  1 +
  doc/develop/bootstd/overview.rst |  2 +-
  doc/develop/bootstd/pxelinux.rst | 27 +++
  3 files changed, 29 insertions(+), 1 deletion(-)
  create mode 100644 doc/develop/bootstd/pxelinux.rst

diff --git a/doc/develop/bootstd/index.rst b/doc/develop/bootstd/index.rst
index 5bbb3d633a3..5052afe448f 100644
--- a/doc/develop/bootstd/index.rst
+++ b/doc/develop/bootstd/index.rst
@@ -8,3 +8,4 @@ Standard Boot

 overview
 extlinux
+   pxelinux
diff --git a/doc/develop/bootstd/overview.rst b/doc/develop/bootstd/overview.rst
index 086a0b1281d..fd0692daf0d 100644
--- a/doc/develop/bootstd/overview.rst
+++ b/doc/develop/bootstd/overview.rst
@@ -413,7 +413,7 @@ Available bootmeth drivers
  Bootmeth drivers are provided for:

 - :doc:`extlinux / syslinux ` boot from a disk
-   - extlinux boot from a network (PXE)
+   - :doc:`extlinux / syslinux ` boot from a network (PXE)
 - U-Boot scripts from disk, network or SPI flash
 - EFI boot using bootefi from disk
 - VBE
diff --git a/doc/develop/bootstd/pxelinux.rst b/doc/develop/bootstd/pxelinux.rst
new file mode 100644
index 000..08ba67e09c5
--- /dev/null
+++ b/doc/develop/bootstd/pxelinux.rst
@@ -0,0 +1,27 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+PXE Bootmeth
+
+
+PXE (Preboot eXecution-Environment) provides a way to boot an Operating System


%s/Operating System/operating system/


+over a network interface. The PXE bootmeth supports PXELINUX and allows U-Boot 
to


%s/bootmeth/boot method/


+provide a menu of possible Operating Systems from which the user can choose.


PXELINUX is a binary. Does U-Boot really provide a menu when the
PXELINUX binary is downloaded and executed?


+
+U-Boot includes a parser for the `extlinux.conf` file described
+`here 
`_.
+It consists primarily of a list of named OSes along with the kernel, initial
+ramdisk and other settings. The file is retrieved from a network server using
+tftpboot.


Does this paragraph relate to PXELINUX?

tftpboot is a command and not a protocol. Shouldn't boot methods work
when the command line interface is disabled? I guess you want to replace

%s/tftpboot/the TFTP protocol/


+
+When invoked on a bootdev, this bootmeth searches for the file and creates a


%s/bootdev/boot device/
%s/bootmeth/boot method/


+bootflow if found. See


%s/bootflow/boot flow/


+`PXELINUX `_ for
+a full description of the search procedure.
+
+When the bootflow is booted, the bootmeth calls pxe_setup_ctx() to set up the
+context, then pxe_process() to process the file. Depending on the contents, 
this
+may boot an OS or provide a list of options to the user, perhaps with a 
timeout.
+
+The compatible string "u-boot,extlinux-pxe" is used for the driver. The driver
+is automatically instantiated if there are no bootmeth drivers in the
+devicetree.


Device-trees contain compatible strings for drivers, not the drivers
themselves.

If every configured boot method is enabled if there is no compatible
string for boot methods in the device-tree, why don't we mention this in
the overview instead of repeating ourselves?

Please, mention the relevant configuration option.

Best regards

Heinrich


Re: [PATCH 03/13] doc: Add a description for bootmeth_extlinux

2024-07-15 Thread Heinrich Schuchardt

On 7/15/24 12:13, Simon Glass wrote:

Add documentation for the extlinux bootmeth.

Signed-off-by: Simon Glass 
---

  doc/develop/bootstd/extlinux.rst | 27 +++
  doc/develop/bootstd/index.rst|  1 +
  doc/develop/bootstd/overview.rst |  2 +-
  3 files changed, 29 insertions(+), 1 deletion(-)
  create mode 100644 doc/develop/bootstd/extlinux.rst

diff --git a/doc/develop/bootstd/extlinux.rst b/doc/develop/bootstd/extlinux.rst
new file mode 100644
index 000..28490f38899
--- /dev/null
+++ b/doc/develop/bootstd/extlinux.rst
@@ -0,0 +1,27 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+Extlinux Bootmeth
+=
+
+`Extlinux 
`_
+(sometimes called syslinux) allows U-Boot to provide a menu of possible


%s/possible/available/


+Operating Systems from which the user can choose.


%s/Operating Systems/operating systems/


+
+U-Boot includes a parser for the `extlinux.conf` file. It consists primarily of
+a list of named OSes along with the kernel, initial ramdisk and other settings.


Please, avoid abbreviations.

%s/OSes/operating systems/


+The file is stored in the `extlinux/` subdirectory, possibly under the `boot/`
+subdirectory. This list of prefixes ({"/", "/boot"} by default) can be selected
+with the `filename-prefixes` property in the bootstd device.


%s/bootstd/boot standard/


+
+Note that PXE (Preboot eXecution-Environment) uses the same file format, but in
+a network context.
+
+When invoked on a bootdev, this bootmeth searches for the file and creates a


Please, replace throughout the text:

%s/bootdev/boot device/
%s/bootmeth/boot method/



+bootflow if found.


%s/bootflow/bootflow/


+
+When the bootflow is booted, the bootmeth calls pxe_setup_ctx() to set up the
+context, then pxe_process() to process the file. Depending on the contents, 
this
+may boot an OS or provide a list of options to the user, perhaps with a 
timeout.


%/OS/operating system/


+
+The compatible string "u-boot,extlinux" is used for the driver. The driver is
+automatically instantiated if there are no bootmeth drivers in the devicetree.


Device-tree never contain boot method drivers. Do you mean:

If no boot method driver is selected by a compatible string in the
device-tree and CONFIG_BOOTMETH_EXTLINUX=y, the extlinux boot method
driver is enabled by default.

Please, mention that the driver is only available if
CONFIG_BOOTMETH_EXTLINUX=y.


diff --git a/doc/develop/bootstd/index.rst b/doc/develop/bootstd/index.rst
index f4f87c7787c..5bbb3d633a3 100644
--- a/doc/develop/bootstd/index.rst
+++ b/doc/develop/bootstd/index.rst
@@ -7,3 +7,4 @@ Standard Boot
 :maxdepth: 2

 overview
+   extlinux
diff --git a/doc/develop/bootstd/overview.rst b/doc/develop/bootstd/overview.rst
index 761f61a573b..086a0b1281d 100644
--- a/doc/develop/bootstd/overview.rst
+++ b/doc/develop/bootstd/overview.rst
@@ -412,7 +412,7 @@ Available bootmeth drivers

  Bootmeth drivers are provided for:


%s/Bootmeth/Boot method/



-   - extlinux / syslinux boot from a disk

> +   - :doc:`extlinux / syslinux ` boot from a disk

After "provided for:" I would expect "booting from a storage device".

You can't use the infinitive here. The days of disks are counted.

Best regards

Heinrich


 - extlinux boot from a network (PXE)
 - U-Boot scripts from disk, network or SPI flash
 - EFI boot using bootefi from disk




Re: [PATCH 02/13] doc: Move bootstd into its own directory

2024-07-15 Thread Heinrich Schuchardt

On 7/15/24 12:13, Simon Glass wrote:

Before adding more files, move the bootstd docs into a new directory,
with an index.

Signed-off-by: Simon Glass 
---

  MAINTAINERS   |  2 +-
  doc/board/starfive/milk-v_mars_cm.rst |  2 +-
  doc/develop/board_best_practices.rst  |  2 +-
  doc/develop/bootstd/index.rst |  9 +
  doc/develop/{bootstd.rst => bootstd/overview.rst} | 14 +++---
  doc/develop/index.rst |  2 +-
  doc/usage/cmd/bootdev.rst |  2 +-
  doc/usage/cmd/bootflow.rst|  2 +-
  doc/usage/cmd/bootmeth.rst|  2 +-
  doc/usage/environment.rst |  2 +-
  10 files changed, 24 insertions(+), 15 deletions(-)
  create mode 100644 doc/develop/bootstd/index.rst
  rename doc/develop/{bootstd.rst => bootstd/overview.rst} (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9bee9284cca..86b830aa997 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -927,7 +927,7 @@ F:  boot/bootmeth*.c
  F:boot/bootstd.c
  F:cmd/bootdev.c
  F:cmd/bootflow.c
-F: doc/develop/bootstd.rst
+F: doc/develop/bootstd/
  F:doc/usage/bootdev.rst
  F:doc/usage/bootflow.rst
  F:doc/usage/bootmeth.rst
diff --git a/doc/board/starfive/milk-v_mars_cm.rst 
b/doc/board/starfive/milk-v_mars_cm.rst
index b31de6043bb..52d4e5e9098 100644
--- a/doc/board/starfive/milk-v_mars_cm.rst
+++ b/doc/board/starfive/milk-v_mars_cm.rst
@@ -89,7 +89,7 @@ provide a default value.

  The variable *$fdtfile* is used in the boot process to automatically load
  a device-tree provided by the operating system. For details of the boot
-process refer to the :doc:`U-Boot Standard Boot <../../../develop/bootstd>`
+process refer to the :doc:`/develop/bootstd/index`
  description.

  Boot source selection
diff --git a/doc/develop/board_best_practices.rst 
b/doc/develop/board_best_practices.rst
index f44401eab7d..09632c80ce7 100644
--- a/doc/develop/board_best_practices.rst
+++ b/doc/develop/board_best_practices.rst
@@ -7,7 +7,7 @@ In addition to the regular best practices such as using 
:doc:`checkpatch` and
  following the :doc:`docstyle` and the :doc:`codingstyle` there are some things
  which are specific to creating a new board port.

-* Implement :doc:`bootstd` to ensure that most operating systems will be
+* Implement :doc:`bootstd/index` to ensure that most operating systems will be
supported by the platform.

  * The platform defconfig file must be generated via `make savedefconfig`.
diff --git a/doc/develop/bootstd/index.rst b/doc/develop/bootstd/index.rst
new file mode 100644
index 000..f4f87c7787c
--- /dev/null
+++ b/doc/develop/bootstd/index.rst
@@ -0,0 +1,9 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+Standard Boot
+=
+
+.. toctree::
+   :maxdepth: 2
+
+   overview
diff --git a/doc/develop/bootstd.rst b/doc/develop/bootstd/overview.rst
similarity index 99%
rename from doc/develop/bootstd.rst
rename to doc/develop/bootstd/overview.rst
index 34631089ae0..761f61a573b 100644
--- a/doc/develop/bootstd.rst
+++ b/doc/develop/bootstd/overview.rst
@@ -1,7 +1,7 @@
  .. SPDX-License-Identifier: GPL-2.0+:

-U-Boot Standard Boot
-
+Standard Boot Overview
+==





The text is not optimized for readability, e.g.

"- bootdev  - a device which can hold or access a distro (e.g. MMC,
Ethernet)"

Please, consistently replace the non-words in the boot standard
documentation:

%s/bootdev/boot device/
%s/bootmeth/boot method/
%s/bootflow/boot flow/

"distro" isn't a dictionary word either. Do you mean operating system?
That term would still not catch what a boot device is:

The boot device is the device that holds the next boot stage.

This could for instance be GRUB which in turn will offer a menu allowing
to select one of multiple operating systems.

Or that next boot stage could be an operating system kernel together
with its initial ram disk and a device-tree.

Best regards

Heinrich




  Introduction
  
@@ -17,7 +17,7 @@ introduces the following concepts:
  For Linux, the distro (Linux distribution, e.g. Debian, Fedora) is responsible
  for creating a bootflow for each kernel combination that it wants to offer.
  These bootflows are stored on media so they can be discovered by U-Boot. This
-feature is typically called `distro boot` (see :doc:`distro`) because it is
+feature is typically called `distro boot` (see :doc:`../distro`) because it is
  a way for distributions to boot on any hardware.

  Traditionally U-Boot has relied on scripts to implement this feature. See
@@ -32,7 +32,7 @@ way to boot with U-Boot. The feature is extensible to 
different Operating
  Systems (such as Chromium OS) and devices (beyond just block and network
  devices). It supports EFI boot and EFI bootmgr too.

-Finally, standard boot supports the operation of :doc:`vbe`.
+Finally, standard 

[PATCH 1/1] drivers/mtd/nvmxip: nvmxip.h is a global include

2024-07-15 Thread Heinrich Schuchardt
include/nvmxip.h is a global and not a local include.
So we should use angle brackets.

Fixes: dc3abd8006c5 ("nvmxip: move header to include")
Signed-off-by: Heinrich Schuchardt 
---
 drivers/mtd/nvmxip/nvmxip-uclass.c | 2 +-
 drivers/mtd/nvmxip/nvmxip.c| 2 +-
 drivers/mtd/nvmxip/nvmxip_qspi.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c 
b/drivers/mtd/nvmxip/nvmxip-uclass.c
index 95dfa58def1..254f04e0b99 100644
--- a/drivers/mtd/nvmxip/nvmxip-uclass.c
+++ b/drivers/mtd/nvmxip/nvmxip-uclass.c
@@ -8,11 +8,11 @@
 
 #include 
 #include 
+#include 
 #if CONFIG_IS_ENABLED(SANDBOX64)
 #include 
 #endif
 #include 
-#include "nvmxip.h"
 
 /* LBA Macros */
 
diff --git a/drivers/mtd/nvmxip/nvmxip.c b/drivers/mtd/nvmxip/nvmxip.c
index 229938db380..594500f0c65 100644
--- a/drivers/mtd/nvmxip/nvmxip.c
+++ b/drivers/mtd/nvmxip/nvmxip.c
@@ -9,10 +9,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include "nvmxip.h"
 
 /**
  * nvmxip_blk_read() - block device read operation
diff --git a/drivers/mtd/nvmxip/nvmxip_qspi.c b/drivers/mtd/nvmxip/nvmxip_qspi.c
index 460887c7da3..f14a822b5d5 100644
--- a/drivers/mtd/nvmxip/nvmxip_qspi.c
+++ b/drivers/mtd/nvmxip/nvmxip_qspi.c
@@ -8,8 +8,8 @@
 
 #include 
 #include 
+#include 
 #include 
-#include "nvmxip.h"
 
 #include 
 DECLARE_GLOBAL_DATA_PTR;
-- 
2.45.2



Re: [PATCH 1/2] riscv: spacemit: bananapi_f3: initial support added

2024-07-14 Thread Heinrich Schuchardt

On 7/14/24 17:08, Kongyang Liu wrote:

Add basic support for SpacemiT's Banana Pi F3 board

Signed-off-by: Kongyang Liu 

---

  arch/riscv/Kconfig |   5 +
  arch/riscv/cpu/k1/Kconfig  |  18 ++
  arch/riscv/cpu/k1/Makefile |   6 +
  arch/riscv/cpu/k1/cpu.c|   9 +
  arch/riscv/cpu/k1/dram.c   |  39 +++
  arch/riscv/dts/Makefile|   1 +
  arch/riscv/dts/k1-bananapi-f3.dts  |  20 ++
  arch/riscv/dts/k1.dtsi | 375 +
  board/spacemit/bananapi_f3/Kconfig |  25 ++
  board/spacemit/bananapi_f3/MAINTAINERS |   6 +
  board/spacemit/bananapi_f3/Makefile|   5 +
  board/spacemit/bananapi_f3/board.c |   9 +
  configs/bananapi_f3_defconfig  |  20 ++
  include/configs/bananapi_f3.h  |  15 +
  14 files changed, 553 insertions(+)
  create mode 100644 arch/riscv/cpu/k1/Kconfig
  create mode 100644 arch/riscv/cpu/k1/Makefile
  create mode 100644 arch/riscv/cpu/k1/cpu.c
  create mode 100644 arch/riscv/cpu/k1/dram.c
  create mode 100644 arch/riscv/dts/k1-bananapi-f3.dts
  create mode 100644 arch/riscv/dts/k1.dtsi
  create mode 100644 board/spacemit/bananapi_f3/Kconfig
  create mode 100644 board/spacemit/bananapi_f3/MAINTAINERS
  create mode 100644 board/spacemit/bananapi_f3/Makefile
  create mode 100644 board/spacemit/bananapi_f3/board.c
  create mode 100644 configs/bananapi_f3_defconfig
  create mode 100644 include/configs/bananapi_f3.h

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index fa3b016c52..211f19a585 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -11,6 +11,9 @@ choice
  config TARGET_ANDES_AE350
bool "Support Andes ae350"

+config TARGET_BANANAPI_F3
+   bool "Support BananaPi F3 Board"
+
  config TARGET_MICROCHIP_ICICLE
bool "Support Microchip PolarFire-SoC Icicle Board"

@@ -88,6 +91,7 @@ source "board/sifive/unleashed/Kconfig"
  source "board/sifive/unmatched/Kconfig"
  source "board/sipeed/maix/Kconfig"
  source "board/sophgo/milkv_duo/Kconfig"
+source "board/spacemit/bananapi_f3/Kconfig"
  source "board/starfive/visionfive2/Kconfig"
  source "board/thead/th1520_lpi4a/Kconfig"
  source "board/xilinx/mbv/Kconfig"
@@ -99,6 +103,7 @@ source "arch/riscv/cpu/fu540/Kconfig"
  source "arch/riscv/cpu/fu740/Kconfig"
  source "arch/riscv/cpu/generic/Kconfig"
  source "arch/riscv/cpu/jh7110/Kconfig"
+source "arch/riscv/cpu/k1/Kconfig"

  # architecture-specific options below

diff --git a/arch/riscv/cpu/k1/Kconfig b/arch/riscv/cpu/k1/Kconfig
new file mode 100644
index 00..79c9fefb66
--- /dev/null
+++ b/arch/riscv/cpu/k1/Kconfig
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (C) 2024, Kongyang Liu 
+
+config SPACEMIT_K1
+   bool
+   select BINMAN
+   select ARCH_EARLY_INIT_R
+   select SYS_CACHE_SHIFT_6
+   imply CPU
+   imply CPU_RISCV
+   imply RISCV_TIMER if (RISCV_SMODE || SPL_RISCV_SMODE)
+   imply RISCV_ACLINT if RISCV_MMODE
+   imply SPL_RISCV_ACLINT if SPL_RISCV_MMODE
+   imply CMD_CPU
+   imply SPL_CPU
+   imply SPL_OPENSBI
+   imply SPL_LOAD_FIT
diff --git a/arch/riscv/cpu/k1/Makefile b/arch/riscv/cpu/k1/Makefile
new file mode 100644
index 00..da12e0f64e
--- /dev/null
+++ b/arch/riscv/cpu/k1/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (c) 2024, Kongyang Liu 
+
+obj-y += dram.o
+obj-y += cpu.o
diff --git a/arch/riscv/cpu/k1/cpu.c b/arch/riscv/cpu/k1/cpu.c
new file mode 100644
index 00..233a6a3d64
--- /dev/null
+++ b/arch/riscv/cpu/k1/cpu.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0+


Thanks a lot for adding the board.

GPL-2.0-or-later, see https://spdx.org/licenses/GPL-2.0-or-later.html


+/*
+ * Copyright (c) 2024, Kongyang Liu 
+ */
+
+int cleanup_before_linux(void)
+{
+   return 0;
+}
diff --git a/arch/riscv/cpu/k1/dram.c b/arch/riscv/cpu/k1/dram.c
new file mode 100644
index 00..41a596eef4
--- /dev/null
+++ b/arch/riscv/cpu/k1/dram.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0+


ditto


+/*
+ * Copyright (c) 2024, Kongyang Liu 
+ */
+
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int dram_init(void)
+{
+   gd->ram_base = CFG_SYS_SDRAM_BASE;
+   /* TODO get ram size from ddr controller */
+   gd->ram_size = SZ_4G;
+   return 0;
+}
+
+int dram_init_banksize(void)
+{
+   gd->bd->bi_dram[0].start = CFG_SYS_SDRAM_BASE;
+   gd->bd->bi_dram[0].size = min_t(phys_size_t, gd->ram_size, SZ_2G);
+
+   if (gd->ram_size > SZ_2G && CONFIG_NR_DRAM_BANKS > 1) {
+   gd->bd->bi_dram[1].start = 0x1;
+   gd->bd->bi_dram[1].size = gd->ram_size - SZ_2G;
+   }
+
+   return 0;
+}
+
+phys_addr_t board_get_usable_ram_top(phys_size_t total_size)
+{
+   if (gd->ram_size > SZ_2G)
+   return SZ_2G;
+
+   return gd->ram_size;
+}
diff --git a/arch/riscv/dts/Makefile 

Re: [PATCH 2/2] doc: spacemit: bananapi_f3: document Banana Pi F3 board

2024-07-14 Thread Heinrich Schuchardt

On 7/14/24 17:08, Kongyang Liu wrote:

Add document for Banana Pi F3 board which based on SpacemiT's K1 SoC.

Signed-off-by: Kongyang Liu 
---

  doc/board/index.rst|  1 +
  doc/board/spacemit/bananapi_f3.rst | 78 ++
  doc/board/spacemit/index.rst   |  8 +++
  3 files changed, 87 insertions(+)
  create mode 100644 doc/board/spacemit/bananapi_f3.rst
  create mode 100644 doc/board/spacemit/index.rst

diff --git a/doc/board/index.rst b/doc/board/index.rst
index 417c128c7a..367da2d623 100644
--- a/doc/board/index.rst
+++ b/doc/board/index.rst
@@ -51,6 +51,7 @@ Board-specific doc
 sipeed/index
 socionext/index
 sophgo/index
+   spacemit/index
 st/index
 starfive/index
 ste/index
diff --git a/doc/board/spacemit/bananapi_f3.rst 
b/doc/board/spacemit/bananapi_f3.rst
new file mode 100644
index 00..465040b94f
--- /dev/null
+++ b/doc/board/spacemit/bananapi_f3.rst
@@ -0,0 +1,78 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Banana Pi F3
+
+
+About This
+--
+Banana Pi F3 board is a industrial grade RISC-V development board, it
+design with SpacemiT K1 8 core RISC-V chip, CPU integrates 2.0 TOPs AI
+computing power. 4G DDR and 16G eMMC onboard.2x GbE Ethernet prot, 4x USB
+3.0 and PCIe for M.2 interface, support HDMI and Dual MIPI-CSI Camera.
+
+Building
+
+1. Add the RISC-V toolchain to your PATH.
+2. Setup ARCH & cross compilation environment variable:
+
+.. code-block:: console
+
+   export CROSS_COMPILE=
+
+3. Before building U-Boot, OpenSBI should be built first. OpenSBI can be
+built for SpacemiT K1 SoC as below:
+
+.. code-block:: console
+
+   git clone https://github.com/cyyself/opensbi -b k1-opensbi
+   cd opensbi
+   make PLATFORM=generic
+
+4. Then build U-Boot as following:
+
+.. code-block:: console
+
+   cd 
+   make bananapi_f3_defconfig
+   make OPENSBI=/build/platform/generic/firmware/fw_dynamic.bin
+
+This will generate u-boot.itb


Thanks a lot for the board documentation.

Could you, please, describe how to install the u-boot.itb.

To where should it be copied?
Does it need to be signed to be accepted by the vendor SPL?

Best regards

Heinrich


+
+Booting
+~~~
+Currently, we use a modified vendor's U-Boot SPL to load a FIT image that
+includes OpenSBI and U-Boot. Fully describing how to boot into U-Boot is a
+challenging task. And the booting method will be added after the SPL
+support is available.
+
+Sample boot log from Banana Pi F3 board
+~~~
+.. code-block:: none
+
+   U-Boot 2024.07-00686-g608f2d51760c (Jul 08 2024 - 14:53:51 +0800)
+
+   DRAM:  4 GiB
+   Core:  18 devices, 7 uclasses, devicetree: separate
+   Loading Environment from nowhere... OK
+   In:serial@d4017000
+   Out:   serial@d4017000
+   Err:   serial@d4017000
+   Net:   No ethernet found.
+   bananapi_f3# cpu detail
+ 0: cpu@0  spacemit,x60
+  ID = 0, freq = 0 Hz: MMU
+ 1: cpu@1  spacemit,x60
+  ID = 1, freq = 0 Hz: MMU
+ 2: cpu@2  spacemit,x60
+  ID = 2, freq = 0 Hz: MMU
+ 3: cpu@3  spacemit,x60
+  ID = 3, freq = 0 Hz: MMU
+ 4: cpu@4  spacemit,x60
+  ID = 4, freq = 0 Hz: MMU
+ 5: cpu@5  spacemit,x60
+  ID = 5, freq = 0 Hz: MMU
+ 6: cpu@6  spacemit,x60
+  ID = 6, freq = 0 Hz: MMU
+ 7: cpu@7  spacemit,x60
+  ID = 7, freq = 0 Hz: MMU
+   bananapi_f3#
diff --git a/doc/board/spacemit/index.rst b/doc/board/spacemit/index.rst
new file mode 100644
index 00..cc2bd6ab0a
--- /dev/null
+++ b/doc/board/spacemit/index.rst
@@ -0,0 +1,8 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+SpacemiT
+
+.. toctree::
+   :maxdepth: 1
+
+   bananapi_f3




Pull request efi-2024-10-rc1-2

2024-07-14 Thread Heinrich Schuchardt

Dear Tom,

The following changes since commit b182816c1fb436916661949213c543bf4d42250b:

  turris_1x: Normalize Kconfig usage (2024-07-13 10:42:15 -0600)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-efi.git
tags/efi-2024-10-rc1-2

for you to fetch changes up to da909648e13fdfb5306123f4bc2a89bf2cbf682e:

  doc: add bootelf command documentation (2024-07-14 09:56:24 +0200)

Gitlab CI showed no issues:
https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21597


Pull request efi-2024-10-rc1-2

Documentation:

* use current versions of certifi and urllib3 for building documentation
* add bootelf command documentation
* correct the description of the Goldfish RTC driver
* correct heading level of itest examples

UEFI:

* print device-tree in dtbdump.efi
* consider CONFIG_EFI_IGNORE_OSINDICATIONS in TestEfiCapsuleFirmwareRaw
* correct cmd_capsule_esl_gen invocation in scripts/Makefile.lib
* use capsule CRT instead of ESL file when building capsules

Others:

* let ENV_IS_IN_EXT4 enable SYS_MMC_ENV_DEV
* check if CONFIG_SYS_MMC_ENV_DEV is defined in mmc_get_env_dev


Heinrich Schuchardt (5):
  efi_loader: print device-tree in dtbdump.efi
  rtc: fix the description of the Goldfish RTC driver
  doc: fix heading level of itest examples
  env: ENV_IS_IN_EXT4 should enable SYS_MMC_ENV_DEV
  sunxi: CONFIG_SYS_MMC_ENV_DEV undeclared

Ilias Apalodimas (1):
  test: test for ignore OsIndications

Jonathan Humphreys (2):
  scripts/Makefile.lib: fixes: Embed capsule public key in
platform's dtb
  scripts/Makefile.lib: EFI: Use capsule CRT instead of ESL file

Maxim Moskalets (1):
  doc: add bootelf command documentation

Tom Rini (1):
  doc/sphinx/requirements.txt: Bump certifi and urllib3

 board/sandbox/capsule_pub_esl_good.esl | Bin 831 -> 0 bytes
 board/sunxi/board.c|   2 +-
 configs/sandbox_defconfig  |   2 +-
 configs/sandbox_flattree_defconfig |   2 +-
 doc/develop/uefi/uefi.rst  |   8 +-
 doc/sphinx/requirements.txt|   4 +-
 doc/usage/cmd/bootelf.rst  |  61 +
 doc/usage/cmd/itest.rst|   2 +-
 doc/usage/index.rst|   1 +
 drivers/rtc/goldfish_rtc.c |   4 +-
 env/Kconfig|   2 +-
 lib/efi_loader/Kconfig |  12 +-
 lib/efi_loader/dtbdump.c   | 261
+
 scripts/Makefile.lib   |  24 +-
 .../test_efi_capsule/test_capsule_firmware_raw.py  |  21 +-
 15 files changed, 375 insertions(+), 31 deletions(-)
 delete mode 100644 board/sandbox/capsule_pub_esl_good.esl
 create mode 100644 doc/usage/cmd/bootelf.rst


[PATCH 1/1] doc: move out-of-tree building info to HTML

2024-07-14 Thread Heinrich Schuchardt
Move the information about out-of-tree building
from README to the generated HTML documentation.

Signed-off-by: Heinrich Schuchardt 
---
 README| 20 
 doc/build/gcc.rst | 28 
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/README b/README
index b89768f1791..b76449b70a8 100644
--- a/README
+++ b/README
@@ -1684,26 +1684,6 @@ images ready for download to / installation on your 
system:
 - "u-boot" is an image in ELF binary format
 - "u-boot.srec" is in Motorola S-Record format
 
-By default the build is performed locally and the objects are saved
-in the source directory. One of the two methods can be used to change
-this behavior and build U-Boot to some external directory:
-
-1. Add O= to the make command line invocations:
-
-   make O=/tmp/build distclean
-   make O=/tmp/build NAME_defconfig
-   make O=/tmp/build all
-
-2. Set environment variable KBUILD_OUTPUT to point to the desired location:
-
-   export KBUILD_OUTPUT=/tmp/build
-   make distclean
-   make NAME_defconfig
-   make all
-
-Note that the command line "O=" setting overrides the KBUILD_OUTPUT environment
-variable.
-
 User specific CPPFLAGS, AFLAGS and CFLAGS can be passed to the compiler by
 setting the according environment variables KCPPFLAGS, KAFLAGS and KCFLAGS.
 For example to treat all compiler warnings as errors:
diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
index 3c646577272..abafe73206c 100644
--- a/doc/build/gcc.rst
+++ b/doc/build/gcc.rst
@@ -118,6 +118,34 @@ Assuming cross compiling on Debian for ARMv8 this would be
 
 CROSS_COMPILE=aarch64-linux-gnu- make
 
+Out-of-tree building
+
+
+By default building is performed locally and the objects are saved in the 
source
+directory. To build out-out-tree use one of the two methods below:
+
+Add O= parameter to the make command line:
+
+.. code-block:: bash
+
+make O=/tmp/build distclean
+make O=/tmp/build NAME_defconfig
+make O=/tmp/build
+
+Use environment variable KBUILD_OUTPUT:
+
+.. code-block:: bash
+
+export KBUILD_OUTPUT=/tmp/build
+make distclean
+make NAME_defconfig
+make
+
+.. note::
+
+The command line "O=" parameter overrides the KBUILD_OUTPUT environment
+variable.
+
 Build parameters
 
 
-- 
2.45.2



Re: [PATCH] part: efi: Rely on a definition to specify the primary GPT position

2024-07-14 Thread Heinrich Schuchardt

On 7/13/24 23:16, Roman Stratiienko wrote:

Hello Heinrich,

Thank you for your comments.

Just a few thoughts about this patch.

1. I was also sending this patch as a noninvasive, semi-cosmetic
change. Relying on hardcoded values is usually not a good practice.
2. Shifting of primary GPT works in u-boot and Linux kernel with minor
tweaks. See patches [1] and [2] for more context. I'm not going to
upstream them.
3. Accepting this patch upstream helps me to reduce the dependency
chain and potential conflicts during the rebase.


Hello Roman,

could you, please, answer my questions below.

Best regards

Heinrich



BR, Roman.

[1]: 
https://github.com/GloDroidCommunity/amlogic-series/blob/8cc5ee7df1358092f77f43aa0c9661883473e5af/patches-aosp/glodroid/bootloader/u-boot/0005-GLODROID-Shift-GPT-position-to-support-Amlogic-platf.patch
[2]: 
https://github.com/GloDroidCommunity/amlogic-series/blob/8cc5ee7df1358092f77f43aa0c9661883473e5af/patches-aosp/glodroid/kernel/common-android14-6.1-lts/0001-GLODROID-Allow-placing-primary-GPT-to-2MiB-offset-to.patch

пн, 10 черв. 2024 р. о 15:22 Heinrich Schuchardt  пише:


On 19.05.24 17:35, Roman Stratiienko wrote:

Use GPT_PRIMARY_PARTITION_TABLE_LBA as the only source for a primary GPT
location instead of hardcoding it every time.

Sometimes, we need to shift the primary GPT location.
Having the location in the single place simplifies such shifting.


In gdisk there is an expert command "j - move the main partition table".
This corresponds to CONFIG_EFI_PARTITION_ENTRIES_OFF. It does not move
the GPT primary header but only the partition entries. So sector 1 is
still occupied.

If I read you correctly, CONFIG_EFI_PARTITION_ENTRIES_OFF is not doing
what you need. Instead of only moving the partition entries of the
primary partition table, you want to move the complete primary GPT table.

Somehow the operating system must find the primary GPT table.

When writing the partition table shouldn't the offset be reflected in
the protective MBR fields StartingCHS and StartingLBA? See Table 5.4
"Protective MBR Partition Record protecting the entire disk*" of the
UEFI 2.10 specification.

Does Linux support this? For testing remove the backup partition table!

GPT_PRIMARY_PARTITION_TABLE_LBA should be made configurable in this case.

Best regards

Heinrich



Signed-off-by: Roman Stratiienko 
---
   disk/part_efi.c | 10 +-
   1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 4ce9243ef2..b55e251ea1 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -387,7 +387,7 @@ int write_gpt_table(struct blk_desc *desc, gpt_header 
*gpt_h, gpt_entry *gpt_e)
   gpt_h->header_crc32 = cpu_to_le32(calc_crc32);

   /* Write the First GPT to the block right after the Legacy MBR */
- if (blk_dwrite(desc, 1, 1, gpt_h) != 1)
+ if (blk_dwrite(desc, (lbaint_t)le64_to_cpu(gpt_h->my_lba), 1, gpt_h) != 1)
   goto err;

   if (blk_dwrite(desc, le64_to_cpu(gpt_h->partition_entry_lba),
@@ -534,7 +534,7 @@ int gpt_fill_pte(struct blk_desc *desc,

   static uint32_t partition_entries_offset(struct blk_desc *desc)
   {
- uint32_t offset_blks = 2;
+ uint32_t offset_blks = GPT_PRIMARY_PARTITION_TABLE_LBA + 1;
   uint32_t __maybe_unused offset_bytes;
   int __maybe_unused config_offset;

@@ -572,8 +572,8 @@ static uint32_t partition_entries_offset(struct blk_desc 
*desc)
* The earliest LBA this can be at is LBA#2 (i.e. right behind
* the (protective) MBR and the GPT header.
*/
- if (offset_blks < 2)
- offset_blks = 2;
+ if (offset_blks < (GPT_PRIMARY_PARTITION_TABLE_LBA + 1))
+ offset_blks = GPT_PRIMARY_PARTITION_TABLE_LBA + 1;

   return offset_blks;
   }
@@ -584,7 +584,7 @@ int gpt_fill_header(struct blk_desc *desc, gpt_header 
*gpt_h, char *str_guid,
   gpt_h->signature = cpu_to_le64(GPT_HEADER_SIGNATURE_UBOOT);
   gpt_h->revision = cpu_to_le32(GPT_HEADER_REVISION_V1);
   gpt_h->header_size = cpu_to_le32(sizeof(gpt_header));
- gpt_h->my_lba = cpu_to_le64(1);
+ gpt_h->my_lba = cpu_to_le64(GPT_PRIMARY_PARTITION_TABLE_LBA);
   gpt_h->alternate_lba = cpu_to_le64(desc->lba - 1);
   gpt_h->last_usable_lba = cpu_to_le64(desc->lba - 34);
   gpt_h->partition_entry_lba =






Re: [PATCH v2 00/21] Universal Payload initial series

2024-07-13 Thread Heinrich Schuchardt



Am 13. Juli 2024 10:12:50 MESZ schrieb Mark Kettenis :
>> From: Simon Glass 
>> Date: Sat, 13 Jul 2024 08:00:34 +0100
>> 
>> Universal Payload (UPL) is an Industry Standard for firmware
>> components[1].
>
>I think you have some trouble understanding the concept of industry
>standard ;).  I guess you want this to become an industry standard.
>Firmly https://xkcd.com/927/ territory if you ask me.
>
>> UPL is designed to improve interoperability within the
>> firmware industry, allowing mixing and matching of projects with less
>> friction and fewer project-specific implementations. UPL is
>> cross-platform, supporting ARM, x86 and RISC-V initially.
>> 
>> This series provides some initial support for this, targeting 0.9.1 and
>> sandbox only.
>> 
>> Features still to come include:
>> - Support for architectures
>> - FIT validation
>> - Handoff validation
>> - Interoperability tests
>> 
>> This series is available at dm/uplb-working and requires the alist
>> series at dm/alist-working[2]

Why is this series needed?

SPL loading a FIT image as described in the "universal" payload spec is already 
implemented in U-Boot and for instance works fine for running EDK II on RISC-V.

I cannot see what functionality is missing in U-Boot.

The UPL seems to be half cooked:

The FIT file format is already described elsewhere.

The relocation chapter describes that it should better be removed.

The device-tree chapter duplicates content of the device-tree specification and 
ignores the existing standardization via the Linux project.

The authors seem not to have given any thought to ACPI.

So let us wait if that UPL thing ever lifts off before wasting a code line on 
it.

Best regards

Heinrich

>> 
>> [1] https://universalpayload.github.io/spec/
>> [2] https://patchwork.ozlabs.org/project/uboot/list/?series=414642
>> 
>> Changes in v2:
>> - Put the upl tests under their own subcommand to avoid bootstd init
>> - Add a function to init the UPL process in SPL
>> - Hang when something goes wrong, to avoid a broken boot
>> - Add a runtime flag to enable UPL
>> - Add a link to the spec
>> - Add a link to the command
>> - Support CI testing
>> - Rework to use alist instead of fixed-length arrays
>> 
>> Simon Glass (21):
>>   sandbox: Use const in os_jump_to_file()
>>   sandbox: Fix a comment in os_find_u_boot()
>>   test: Move some SPL-loading test-code into sandbox common
>>   sandbox: Enable SPL_LOAD_BLOCK
>>   fdt: Don't overwrite bloblist devicetree
>>   sandbox: fdt: Avoid overwriting an existing fdt
>>   sandbox: Return error code from read/write/seek
>>   sandbox: Add ELF file to VPL u-boot.img
>>   sandbox: Set up global_data earlier
>>   upl: Add support for reading a upl handoff
>>   upl: Add support for writing a upl handoff
>>   upl: Add basic tests
>>   upl: Add a command
>>   upl: Add support for Universal Payload in SPL
>>   spl: Plumb in the Universal Payload handoff
>>   upl: Plumb in universal payload to the init process
>>   sandbox_vpl: Enable Universal Payload
>>   upl: Add initial documentation
>>   sandbox: Add a flag to enable UPL
>>   sandbox: Add an SPL loader for UPL
>>   upl: Add an end-to-end test
>> 
>>  MAINTAINERS   |  13 +
>>  Makefile  |   4 +-
>>  arch/sandbox/cpu/cpu.c|   2 +
>>  arch/sandbox/cpu/os.c |  30 +-
>>  arch/sandbox/cpu/spl.c| 116 +-
>>  arch/sandbox/cpu/start.c  |  18 +-
>>  arch/sandbox/include/asm/spl.h|  15 +
>>  arch/sandbox/include/asm/state.h  |   1 +
>>  boot/Kconfig  |  70 
>>  boot/Makefile |   4 +
>>  boot/upl_common.c |  60 +++
>>  boot/upl_common.h |  24 ++
>>  boot/upl_read.c   | 588 
>>  boot/upl_write.c  | 622 ++
>>  cmd/Kconfig   |   7 +
>>  cmd/Makefile  |   1 +
>>  cmd/upl.c | 118 ++
>>  common/board_f.c  |  22 ++
>>  common/board_r.c  |   2 +
>>  common/spl/Kconfig|   1 +
>>  common/spl/Makefile   |   2 +
>>  common/spl/spl.c  |   8 +
>>  common/spl/spl_fit.c  |  22 ++
>>  common/spl/spl_upl.c  | 171 
>>  configs/sandbox_defconfig |   1 +
>>  configs/sandbox_vpl_defconfig |   4 +
>>  doc/usage/cmd/upl.rst | 186 +
>>  doc/usage/index.rst   |   2 +
>>  doc/usage/upl.rst |  46 +++
>>  drivers/block/sandbox.c   |   4 +-
>>  drivers/usb/emul/sandbox_flash.c  |   2 +-
>>  fs/sandbox/sandboxfs.c|   6 +-
>>  include/asm-generic/global_data.h |  19 +
>>  include/os.h  |   6 +-
>>  include/spl.h |  16 +
>>  include/test/suites.h |   1 +
>>  include/upl.h | 361 +
>>  

[PATCH 1/1] efi_loader: find distro device-path for media devices

2024-07-13 Thread Heinrich Schuchardt
The auto-generated load options for media device do not contain a partition
node. We cannot expect the simple file protocol here.

Get the partition device-path via the loaded image protocol.

Fixes: e91b68fd6b83 ("efi_loader: load distro dtb in bootmgr")
Reported-by: E Shattow 
Signed-off-by: Heinrich Schuchardt 
---
 include/efi_loader.h |  2 +-
 lib/efi_loader/efi_bootmgr.c |  2 +-
 lib/efi_loader/efi_fdt.c | 33 +++--
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 6c993e1a694..2a06fbe4704 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -1205,6 +1205,6 @@ efi_status_t efi_load_option_dp_join(struct 
efi_device_path **dp,
 
 int efi_get_distro_fdt_name(char *fname, int size, int seq);
 
-void efi_load_distro_fdt(void **fdt, efi_uintn_t *fdt_size);
+void efi_load_distro_fdt(efi_handle_t handle, void **fdt, efi_uintn_t 
*fdt_size);
 
 #endif /* _EFI_LOADER_H */
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 304ed43595c..589d3996b68 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -1277,7 +1277,7 @@ efi_status_t efi_bootmgr_run(void *fdt)
if (fdt_lo)
fdt = fdt_lo;
if (!fdt) {
-   efi_load_distro_fdt(_distro, _size);
+   efi_load_distro_fdt(handle, _distro, _size);
fdt = fdt_distro;
}
}
diff --git a/lib/efi_loader/efi_fdt.c b/lib/efi_loader/efi_fdt.c
index 86ba00c2bdd..4ccf2055be3 100644
--- a/lib/efi_loader/efi_fdt.c
+++ b/lib/efi_loader/efi_fdt.c
@@ -75,28 +75,34 @@ int efi_get_distro_fdt_name(char *fname, int size, int seq)
 /**
  * efi_load_distro_fdt() - load distro device-tree
  *
+ * @handle:handle of loaded image
  * @fdt:   on return device-tree, must be freed via efi_free_pages()
  * @fdt_size:  buffer size
  */
-void efi_load_distro_fdt(void **fdt, efi_uintn_t *fdt_size)
+void efi_load_distro_fdt(efi_handle_t handle, void **fdt, efi_uintn_t 
*fdt_size)
 {
-   struct efi_device_path *rem, *dp;
+   struct efi_device_path *dp;
efi_status_t  ret;
+   struct efi_handler *handler;
+   struct efi_loaded_image *loaded_image;
efi_handle_t device;
 
*fdt = NULL;
 
-   dp = efi_get_dp_from_boot(NULL);
-   if (!dp)
+   /* Get boot device from loaded image protocol */
+   ret = efi_search_protocol(handle, _guid_loaded_image, );
+   if (ret != EFI_SUCCESS)
return;
-   device = efi_dp_find_obj(dp, NULL, );
-   ret = efi_search_protocol(device, _simple_file_system_protocol_guid,
- NULL);
+   loaded_image = handler->protocol_interface;
+   device = loaded_image->device_handle;
+
+   /* Get device path of boot device */
+   ret = efi_search_protocol(device, _guid_device_path, );
if (ret != EFI_SUCCESS)
-   goto err;
-   memcpy(rem, , sizeof(END));
+   return;
+   dp = handler->protocol_interface;
 
-   /* try the various available names */
+   /* Try the various available names */
for (int seq = 0; ; ++seq) {
struct efi_device_path *file;
char buf[255];
@@ -108,10 +114,9 @@ void efi_load_distro_fdt(void **fdt, efi_uintn_t *fdt_size)
break;
ret = efi_load_image_from_path(true, file, fdt, fdt_size);
efi_free_pool(file);
-   if (ret == EFI_SUCCESS)
+   if (ret == EFI_SUCCESS) {
+   log_debug("Fdt %pD loaded\n", file);
break;
+   }
}
-
-err:
-   efi_free_pool(dp);
 }
-- 
2.45.2



Re: [PATCH] fat: fat2rtc: Sanitize timestamps

2024-07-12 Thread Heinrich Schuchardt

On 12.07.24 11:51, Richard Weinberger wrote:

Am Freitag, 12. Juli 2024, 11:46:08 CEST schrieb 'Heinrich Schuchardt' via 
upstream:

Am 12. Juli 2024 10:24:54 MESZ schrieb Richard Weinberger :

Make sure that tm_mday and tm_mon are within the expected
range. Upper layers such as rtc_calc_weekday() will use
them as lookup keys for arrays and this can cause out of
bounds memory accesses.


rtc_calc_weekday() might receive invalid input from other sources. Shouldn't 
the function always validate its input before array access?


It depends on the overall design.
Functions like strlen() also assume that you provide a valid string,
so rtc_calc_weekday() can assume too that the passed rtc_time structure 
contains valid data.

In doubt, let's fix both FAT and rtc_calc_weekday().


Other source locations where the content of struct rtc_time is not
(fully) validated before calling rtc_calc_weekday are

mc146818_get()
mk_date()

to name a few.

Other RTC drivers might also be placing garbage in a struct rtc_time, e.g.

omap_rtc_get()
m41t62_update_rtc_time()

Best regards

Heinrich


Re: [PATCH 1/2] ext4: Fix integer overflow in ext4fs_read_symlink()

2024-07-12 Thread Heinrich Schuchardt

On 12.07.24 13:14, Richard Weinberger wrote:

Am Freitag, 12. Juli 2024, 13:10:12 CEST schrieb 'Heinrich Schuchardt' via 
upstream:

On 02.07.24 21:42, Richard Weinberger wrote:

While zalloc() takes a size_t type, adding 1 to the le32 variable
will overflow.
A carefully crafted ext4 filesystem can exhibit an inode size of 0x
and as consequence zalloc() will do a zero allocation.

Later in the function the inode size is again used for copying data.
So an attacker can overwrite memory.

Avoid the overflow by using the __builtin_add_overflow() helper.

Signed-off-by: Richard Weinberger 
---
   fs/ext4/ext4_common.c | 7 ++-
   1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 2ff0dca249..32364b72fb 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2183,13 +2183,18 @@ static char *ext4fs_read_symlink(struct ext2fs_node 
*node)
struct ext2fs_node *diro = node;
int status;
loff_t actread;
+   size_t alloc_size;

if (!diro->inode_read) {
status = ext4fs_read_inode(diro->data, diro->ino, >inode);
if (status == 0)
return NULL;
}
-   symlink = zalloc(le32_to_cpu(diro->inode.size) + 1);
+
+   if (__builtin_add_overflow(le32_to_cpu(diro->inode.size), 1, 
_size))


U-Boot is freestanding code. You cannot use built-ins.


Hm, I see man built-ins in the U-Boot source.
Why is this one special?


See the definition of COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW in
include/linux/compiler-clang.h.

Best regards

Heinrich


Re: [PATCH 2/2] ext4: Fix zalloc()

2024-07-12 Thread Heinrich Schuchardt

On 02.07.24 21:42, Richard Weinberger wrote:

The zalloc() function suffers from two problems.
1. If memalign() fails it will return NULL and memset() will use a NULL pointer.
2. memalign() itself seems to crash when more than 2^32 bytes are requested.

So, check the return value of memalign() and allocate only of size is less than
CONFIG_SYS_MALLOC_LEN.

Signed-off-by: Richard Weinberger 
---
FWIW, I didn't investigate further why memalign() fails for large sizes.
Maybe this is an issue on it's own.

Thanks,
//richard
---
  fs/ext4/ext4_common.h | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/ext4_common.h b/fs/ext4/ext4_common.h
index 84500e990a..0d1f72ae01 100644
--- a/fs/ext4/ext4_common.h
+++ b/fs/ext4/ext4_common.h
@@ -43,8 +43,14 @@

  static inline void *zalloc(size_t size)
  {
-   void *p = memalign(ARCH_DMA_MINALIGN, size);
-   memset(p, 0, size);
+   void *p = NULL;
+
+   if (size < CONFIG_SYS_MALLOC_LEN)
+   p = memalign(ARCH_DMA_MINALIGN, size);


Memalign() is called in many code locations.

If memalign() has a bug, it needs to be fixed in memalign. We should not
try to work around it in all callers.

Best regards

Heinrich


+
+   if (p)
+   memset(p, 0, size);
+
return p;
  }





Re: [PATCH 1/2] ext4: Fix integer overflow in ext4fs_read_symlink()

2024-07-12 Thread Heinrich Schuchardt

On 02.07.24 21:42, Richard Weinberger wrote:

While zalloc() takes a size_t type, adding 1 to the le32 variable
will overflow.
A carefully crafted ext4 filesystem can exhibit an inode size of 0x
and as consequence zalloc() will do a zero allocation.

Later in the function the inode size is again used for copying data.
So an attacker can overwrite memory.

Avoid the overflow by using the __builtin_add_overflow() helper.

Signed-off-by: Richard Weinberger 
---
  fs/ext4/ext4_common.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 2ff0dca249..32364b72fb 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2183,13 +2183,18 @@ static char *ext4fs_read_symlink(struct ext2fs_node 
*node)
struct ext2fs_node *diro = node;
int status;
loff_t actread;
+   size_t alloc_size;

if (!diro->inode_read) {
status = ext4fs_read_inode(diro->data, diro->ino, >inode);
if (status == 0)
return NULL;
}
-   symlink = zalloc(le32_to_cpu(diro->inode.size) + 1);
+
+   if (__builtin_add_overflow(le32_to_cpu(diro->inode.size), 1, 
_size))


U-Boot is freestanding code. You cannot use built-ins.

Best regards

Heinrich


+   return NULL;
+
+   symlink = zalloc(alloc_size);
if (!symlink)
return NULL;





Re: [PATCH] fat: fat2rtc: Sanitize timestamps

2024-07-12 Thread Heinrich Schuchardt



Am 12. Juli 2024 10:24:54 MESZ schrieb Richard Weinberger :
>Make sure that tm_mday and tm_mon are within the expected
>range. Upper layers such as rtc_calc_weekday() will use
>them as lookup keys for arrays and this can cause out of
>bounds memory accesses.

rtc_calc_weekday() might receive invalid input from other sources. Shouldn't 
the function always validate its input before array access?

Having a library function for validating a struct rtc_time would be preferable 
to repeating ourselves in code.

Best regards

Heinrich

>
>Signed-off-by: Richard Weinberger 
>---
> fs/fat/fat.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>index 2dd9d4e72d..f9096e8932 100644
>--- a/fs/fat/fat.c
>+++ b/fs/fat/fat.c
>@@ -1253,8 +1253,9 @@ out:
>  */
> static void __maybe_unused fat2rtc(u16 date, u16 time, struct rtc_time *tm)
> {
>-  tm->tm_mday = date & 0x1f;
>-  tm->tm_mon = (date & 0x1e0) >> 5;
>+  tm->tm_mday = max(1, date & 0x1f);
>+  tm->tm_mon = clamp((date & 0x1e0) >> 5, 1, 12);
>+
>   tm->tm_year = (date >> 9) + 1980;
> 
>   tm->tm_sec = (time & 0x1f) << 1;


Re: [RFC v3 1/2] doc: Remove FIT documentation that is elsewhere

2024-07-08 Thread Heinrich Schuchardt

On 08.07.24 17:39, Sam Povilus wrote:

FIT documentation is now a separate project, instead of having a
duplicate, we should point at the other project.

Signed-off-by: Sam Povilus 
---
  doc/usage/fit/index.rst  |   5 +-
  doc/usage/fit/source_file_format.rst | 682 +--
  2 files changed, 5 insertions(+), 682 deletions(-)

diff --git a/doc/usage/fit/index.rst b/doc/usage/fit/index.rst
index bd25bd30b2..af2e481212 100644
--- a/doc/usage/fit/index.rst
+++ b/doc/usage/fit/index.rst
@@ -4,13 +4,12 @@ Flat Image Tree (FIT)
  =

  U-Boot uses Flat Image Tree (FIT) as a standard file format for packaging
-images that it it reads and boots. Documentation about FIT is available at
-doc/uImage.FIT
+images that it it reads and boots. Documentation about FIT is available in


%s/it it/it/


+`the Flattened Image Tree project `_.

  .. toctree::
  :maxdepth: 1

-source_file_format


It would preferable not to remove the line in this patch instead of
adding it in the second patch again.


  howto
  x86-fit-boot
  signature
diff --git a/doc/usage/fit/source_file_format.rst 
b/doc/usage/fit/source_file_format.rst
index b2b1e42bd7..0c44329a82 100644
--- a/doc/usage/fit/source_file_format.rst
+++ b/doc/usage/fit/source_file_format.rst
@@ -3,682 +3,6 @@
  Flattened Image Tree (FIT) Format
  =

-Introduction
-
-
-The number of elements playing a role in the kernel booting process has
-increased over time and now typically includes the devicetree, kernel image and
-possibly a ramdisk image. Generally, all must be placed in the system memory 
and
-booted together.
-
-For firmware images a similar process has taken place, with various binaries
-loaded at different addresses, such as ARM's ATF, OpenSBI, FPGA and U-Boot
-itself.
-
-FIT provides a flexible and extensible format to deal with this complexity. It
-provides support for multiple components. It also supports multiple
-configurations, so that the same FIT can be used to boot multiple boards, with
-some components in common (e.g. kernel) and some specific to that board (e.g.
-devicetree).
-
-Terminology
-~~~
-
-This document defines FIT by providing FDT (Flat Device Tree) bindings. These
-describe the final form of the FIT at the moment when it is used. The user
-perspective may be simpler, as some of the properties (like timestamps and
-hashes) are filled in automatically by the U-Boot mkimage tool.
-
-To avoid confusion with the kernel FDT the following naming convention is used:
-
-FIT
-Flattened Image Tree
-
-FIT is formally a flattened devicetree (in the libfdt meaning), which conforms
-to bindings defined in this document.
-
-.its
-image tree source
-
-.itb
-flattened image tree blob
-
-Image-building procedure
-
-
-The following picture shows how the FIT is prepared. Input consists of
-image source file (.its) and a set of data files. Image is created with the
-help of standard U-Boot mkimage tool which in turn uses dtc (device tree
-compiler) to produce image tree blob (.itb). The resulting .itb file is the
-actual binary of a new FIT::
-
-tqm5200.its
-+
-vmlinux.bin.gz mkimage + dtc   xfer to target
-eldk-4.2-ramdisk  --> tqm5200.itb --> boot
-tqm5200.dtb  /|\
-  |
- 'new FIT'
-
-Steps:
-
-#. Create .its file, automatically filled-in properties are omitted
-
-#. Call mkimage tool on a .its file
-
-#. mkimage calls dtc to create .itb image and assures that
-   missing properties are added
-
-#. .itb (new FIT) is uploaded onto the target and used therein
-
-
-Unique identifiers
-~~
-
-To identify FIT sub-nodes representing images, hashes, configurations (which
-are defined in the following sections), the "unit name" of the given sub-node
-is used as it's identifier as it assures uniqueness without additional
-checking required.
-
-
-External data
-~
-
-FIT is normally built initially with image data in the 'data' property of each
-image node. It is also possible for this data to reside outside the FIT itself.
-This allows the 'FDT' part of the FIT to be quite small, so that it can be
-loaded and scanned without loading a large amount of data. Then when an image 
is
-needed it can be loaded from an external source.
-
-External FITs use 'data-offset' or 'data-position' instead of 'data'.
-
-The mkimage tool can convert a FIT to use external data using the `-E` 
argument,
-optionally using `-p` to specific a fixed position.
-
-It is often desirable to align each image to a block size or cache-line size
-(e.g. 512 bytes), so that there is no need to copy it to an aligned address 
when
-reading the image data. The mkimage tool provides a `-B` argument to support
-this.
-
-Root-node 

Re: [RFC v3 2/2] doc: add missing table of content links

2024-07-08 Thread Heinrich Schuchardt

On 08.07.24 18:04, Heinrich Schuchardt wrote:

On 08.07.24 17:39, Sam Povilus wrote:

add missing table of content links, make alphabetical

Signed-off-by: Sam Povilus 
---
  doc/usage/fit/index.rst | 21 ++---
  1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/doc/usage/fit/index.rst b/doc/usage/fit/index.rst
index af2e481212..ed28dc89ae 100644
--- a/doc/usage/fit/index.rst
+++ b/doc/usage/fit/index.rst
@@ -10,9 +10,24 @@ images that it it reads and boots. Documentation
about FIT is available in
  .. toctree::
  :maxdepth: 1

+    beaglebone_vboot
  howto
-    x86-fit-boot
+    kernel_fdt
+    kernel_fdts_compressed
+    kernel
+    multi
+    multi_spl
+    multi-with-fpga
+    multi-with-loadables
+    overlay-fdt-boot
+    sec_firmware_ppa
  signature
+    sign-configs
+    sign-images
+    source_file_format


In patch 1 you delete this page.

Run 'make html' fails on this.

When changing documentation, please, build it.


Got that wrong.

The trailing blank line can be removed when merging.

Reviewed-by: Heinrich Schuchardt 




+    uefi
+    update3
+    update_uboot
  verified-boot
-    beaglebone_vboot
-    overlay-fdt-boot
+    x86-fit-boot
+


We should avoid trailing blank lines at the end of files.

Otherwise looks good to me.

Best regards

Heinrich





Re: [RFC v3 2/2] doc: add missing table of content links

2024-07-08 Thread Heinrich Schuchardt

On 08.07.24 17:39, Sam Povilus wrote:

add missing table of content links, make alphabetical

Signed-off-by: Sam Povilus 
---
  doc/usage/fit/index.rst | 21 ++---
  1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/doc/usage/fit/index.rst b/doc/usage/fit/index.rst
index af2e481212..ed28dc89ae 100644
--- a/doc/usage/fit/index.rst
+++ b/doc/usage/fit/index.rst
@@ -10,9 +10,24 @@ images that it it reads and boots. Documentation about FIT 
is available in
  .. toctree::
  :maxdepth: 1

+beaglebone_vboot
  howto
-x86-fit-boot
+kernel_fdt
+kernel_fdts_compressed
+kernel
+multi
+multi_spl
+multi-with-fpga
+multi-with-loadables
+overlay-fdt-boot
+sec_firmware_ppa
  signature
+sign-configs
+sign-images
+source_file_format


In patch 1 you delete this page.

Run 'make html' fails on this.

When changing documentation, please, build it.


+uefi
+update3
+update_uboot
  verified-boot
-beaglebone_vboot
-overlay-fdt-boot
+x86-fit-boot
+


We should avoid trailing blank lines at the end of files.

Otherwise looks good to me.

Best regards

Heinrich



Re: [PATCH v4 02/10] lib: uuid: add UUID v5 support

2024-07-05 Thread Heinrich Schuchardt

On 7/2/24 15:30, Caleb Connolly wrote:

Add support for generating version 5 UUIDs, these are determistic and work
by hashing a "namespace" UUID together with some unique data. One intended
usecase is to allow for dynamically generate payload UUIDs for UEFI
capsule updates, so that supported boards can have their own UUIDs
without needing to hardcode them.

In addition, move the common bit twiddling code from gen_ran_uuid into a
separate function and rewrite it not to use clrsetbits (which is not
available when building as part of host tools).

Tests for this are added in an upcoming patch.

Signed-off-by: Caleb Connolly 
---
  include/uuid.h | 17 +++--
  lib/Kconfig|  1 +
  lib/uuid.c | 58 +++---
  3 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/include/uuid.h b/include/uuid.h
index f5a941250f48..1f4fa103b5e9 100644
--- a/include/uuid.h
+++ b/include/uuid.h
@@ -10,8 +10,9 @@
  #ifndef __UUID_H__
  #define __UUID_H__

  #include 
+#include 

  /*
   * UUID - Universally Unique IDentifier - 128 bits unique number.
   *There are 5 versions and one variant of UUID defined by RFC4122
@@ -45,10 +46,10 @@
   * where x is a hexadecimal character. Fields are separated by '-'s.
   * When converting to a binary UUID, le means the field should be converted
   * to little endian and be means it should be converted to big endian.
   *
- * UUID is also used as GUID (Globally Unique Identifier) with the same binary
- * format but it differs in string format like below.
+ * UUID is also used as GUID (Globally Unique Identifier) with the same format
+ * but with some fields stored in little endian.
   *
   * GUID:
   * 0914   19   24
   * ----
@@ -142,8 +143,20 @@ void gen_rand_uuid(unsigned char *uuid_bin);
   * @param  - uuid output type: UUID - 0, GUID - 1
   */
  void gen_rand_uuid_str(char *uuid_str, int str_format);

+struct efi_guid;
+
+/**
+ * gen_v5_guid() - generate little endian v5 GUID from namespace and other 
seed data.
+ *
+ * @namespace:   pointer to UUID namespace salt
+ * @guid:pointer to allocated GUID output
+ * @...: NULL terminated list of seed data as pairs of pointers
+ *   to data and their lengths
+ */
+void gen_v5_guid(const struct uuid *namespace, struct efi_guid *guid, ...);
+
  /**
   * uuid_str_to_le_bin() - Convert string UUID to little endian binary data.
   * @uuid_str: pointer to UUID string
   * @uuid_bin: pointer to allocated array for little endian output [16B]
diff --git a/lib/Kconfig b/lib/Kconfig
index 189e6eb31aa1..9aa882d5f882 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -72,8 +72,9 @@ config HAVE_PRIVATE_LIBGCC
bool

  config LIB_UUID
bool
+   select SHA1

  config RANDOM_UUID
bool "GPT Random UUID generation"
select LIB_UUID
diff --git a/lib/uuid.c b/lib/uuid.c
index dfa2320ba267..7d0a8273d157 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -21,8 +21,9 @@
  #include 
  #include 
  #include 
  #include 
+#include 

  int uuid_str_valid(const char *uuid)
  {
int i, valid;
@@ -368,8 +369,57 @@ void uuid_bin_to_str(const unsigned char *uuid_bin, char 
*uuid_str,
}
}
  }

+static void configure_uuid(struct uuid *uuid, unsigned char version)
+{
+   uint16_t tmp;
+
+   /* Configure variant/version bits */
+   tmp = be16_to_cpu(uuid->time_hi_and_version);
+   tmp = (tmp & ~UUID_VERSION_MASK) | (version << UUID_VERSION_SHIFT);


As you wrote above time_hi is stored low-endian in a GUID and big endian
in UUID. So the version is in the byte 6 for a UUID and in byte 7 in a GUID.

E.g. on my hard disk I find:

Partition unique GUID: 003EF8CE-F7E9-467F-B80D-22ED2E6E93D7
CE F8 3E 00  E9 F7 7F 46  B8 0D 22 ED  2E 6E 93 D7

Where 4 is the version.


+   uuid->time_hi_and_version = cpu_to_be16(tmp);
+
+   uuid->clock_seq_hi_and_reserved &= ~UUID_VARIANT_MASK;
+   uuid->clock_seq_hi_and_reserved |= (UUID_VARIANT << UUID_VARIANT_SHIFT);
+}
+
+void gen_v5_guid(const struct uuid *namespace, struct efi_guid *guid, ...)
+{
+   sha1_context ctx;
+   va_list args;
+   const uint8_t *data;
+   uint32_t *tmp32;
+   uint16_t *tmp16;
+   uint8_t hash[SHA1_SUM_LEN];
+
+   sha1_starts();
+   /* Hash the namespace UUID as salt */
+   sha1_update(, (unsigned char *)namespace, UUID_BIN_LEN);
+   va_start(args, guid);
+
+   while ((data = va_arg(args, const uint8_t *))) {
+   unsigned int len = va_arg(args, size_t);
+   sha1_update(, data, len);
+   }
+
+   va_end(args);
+   sha1_finish(, hash);
+
+   /* Truncate the hash into output UUID, it is already big endian */
+   memcpy(guid, hash, sizeof(*guid));
+
+   configure_uuid((struct uuid *)guid, 5);



Please, provide a test that in the resulting GUID output we find the
variant and version information at 

Re: [PATCH v2 0/7] Add Starfive JH7110 Cadence USB driver

2024-07-04 Thread Heinrich Schuchardt

On 7/4/24 07:50, Minda Chen wrote:

Add Starfive JH7110 Cadence USB driver and related PHY driver.
So the codes can be used in visionfive2 and star64 7110 board.

The driver is almost the same with kernel driver.

Test with Star64 JH7110 board USB 3.0 + USB 2.0 host.
The code can work.

- Star64 using USB 3.0 and USB 2.0 host must add below board dts setting.


Does this mean a future patch for spl_fdt_fixup_star64() is needed?

Did you also have a look at the USB on the Milk V Mars CM?

Best regards

Heinrich



1. usb pin setting
usb_pins: usb0-0 {
driver-vbus-pin {
pinmux = ;
bias-disable;
input-disable;
input-schmitt-disable;
slew-rate = <0>;
};
};

2. related dts node setting.
 {
status = "disabled";
};

 {
starfive,sys-syscon = <_syscon 0x18>;
starfive,stg-syscon = <_syscon 0x148 0x1f4>;
status = "okay";
};

 {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
status = "okay";
};

_cdns3 {
phys = <>, <>;
phy-names = "cdns3,usb2-phy", "cdns3,usb3-phy";
dr_mode = "host";
status = "okay";
};

- previous version
   v1: 
https://patchwork.ozlabs.org/project/uboot/cover/20240504150358.19600-1-minda.c...@starfivetech.com/

- patch description.

patch1: Add set phy mode function in cdns3 core driver
 which is used by Starfive JH7110.
patch2-3: USB and PCIe 2.0 (usb 3.0) PHY drivier
patch4: Cadence USB wrapper driver.
patch5: Add JH7110 USB default overcurrent pin.
patch6-8 dts, config and maintainers update.

- change:
v2:
- patch 1 Move the added code to cdns3_core_init_role(). Must
  set PHY mode before calling cdns3 role start function.
- patch 1-4 correct the code format.(follow Marek's comments.)
- patch 2 Add set 125M clock in PHY init function.
- Add new patch5.

Minda Chen (8):
   usb: cdns3: Set USB PHY mode in cdns3_core_init_role()
   phy: starfive: Add Starfive JH7110 USB 2.0 PHY driver
   phy: starfive: Add Starfive JH7110 PCIe 2.0 PHY driver
   usb: cdns: starfive: Add cdns USB driver
   pinctrl: starfive: Setup USB default disable overcurrent pin
   configs: starfive: Add visionfive2 cadence USB configuration
   dts: starfive: Add JH7110 Cadence USB dts node
   MAINTAINERS: Update Starfive visionfive2 maintain files.

  .../dts/jh7110-starfive-visionfive-2.dtsi |   5 +
  arch/riscv/dts/jh7110.dtsi|  52 +
  board/starfive/visionfive2/MAINTAINERS|   2 +
  configs/starfive_visionfive2_defconfig|   9 +
  drivers/phy/Kconfig   |   1 +
  drivers/phy/Makefile  |   1 +
  drivers/phy/starfive/Kconfig  |  21 ++
  drivers/phy/starfive/Makefile |   7 +
  drivers/phy/starfive/phy-jh7110-pcie.c| 202 ++
  drivers/phy/starfive/phy-jh7110-usb2.c| 135 
  drivers/pinctrl/starfive/pinctrl-jh7110-sys.c |  11 +-
  drivers/usb/cdns3/Kconfig |   7 +
  drivers/usb/cdns3/Makefile|   2 +
  drivers/usb/cdns3/cdns3-starfive.c| 183 
  drivers/usb/cdns3/core.c  |  25 +++
  15 files changed, 661 insertions(+), 2 deletions(-)
  create mode 100644 drivers/phy/starfive/Kconfig
  create mode 100644 drivers/phy/starfive/Makefile
  create mode 100644 drivers/phy/starfive/phy-jh7110-pcie.c
  create mode 100644 drivers/phy/starfive/phy-jh7110-usb2.c
  create mode 100644 drivers/usb/cdns3/cdns3-starfive.c


base-commit: 8937bb265a7f2251c1bd999784a4ef10e9c6080d




Re: [RFC 1/1] doc: Remove FIT documentation that is elsewhere

2024-07-01 Thread Heinrich Schuchardt



Am 1. Juli 2024 21:52:12 MESZ schrieb Sam Povilus :
>
>
>On 6/18/2024 12:03 AM, Heinrich Schuchardt wrote:
>> Caution: This message originated from an External Source. Use proper caution 
>> when opening attachments, clicking links, or responding.
>> 
>> 
>> On 6/17/24 22:20, Sam Povilus wrote:
>>> FIT documentation is now a separate project, instead of having a
>>> duplicate, we should point at the other project.
>>> 
>>> Signed-off-by: Sam Povilus 
>>> ---
>>>   doc/usage/fit/index.rst  |  14 +-
>>>   doc/usage/fit/source_file_format.rst | 682 +--
>>>   2 files changed, 5 insertions(+), 691 deletions(-)
>>> 
>>> diff --git a/doc/usage/fit/index.rst b/doc/usage/fit/index.rst
>>> index bd25bd30b2..12a8c6fb16 100644
>>> --- a/doc/usage/fit/index.rst
>>> +++ b/doc/usage/fit/index.rst
>>> @@ -4,16 +4,6 @@ Flat Image Tree (FIT)
>>>   =
>>> 
>>>   U-Boot uses Flat Image Tree (FIT) as a standard file format for packaging
>>> -images that it it reads and boots. Documentation about FIT is available at
>>> -doc/uImage.FIT
>>> +images that it it reads and boots. Documentation about FIT is available in
>>> +`the Flattened Image Tree project <https://fitspec.osfw.foundation/>`_.
>>> 
>>> -.. toctree::
>>> -    :maxdepth: 1
>>> -
>>> -    source_file_format
>> 
>> This is the only entry to be replaced by the external link.
>> 
>>> -    howto
>>> -    x86-fit-boot
>>> -    signature
>>> -    verified-boot
>>> -    beaglebone_vboot
>>> -    overlay-fdt-boot
>> 
>> All of these files are staying around and must be kept linked.
>
>I would argue that if this list is not being maintained, it is better to 
>remove it than to try to keep it up to date. Maybe that should be a separate 
>patch though?

Users will not be able to find the documents in the html documentation if you 
remove the links. Please, do not remove links for existing documents.

Best regards

Heinrich
>
>> 
>> Many links are missing here. The complete list should be:
>> 
>>      beaglebone_vboot
>>      howto
>>      kernel_fdt
>>      kernel_fdts_compressed
>>      kernel
>>      multi
>>      multi_spl
>>      multi-with-fpga
>>      multi-with-loadables
>>      overlay-fdt-boot
>>      sec_firmware_ppa
>>      signature
>>      sign-configs
>>      sign-images
>>      uefi
>>      update3
>>      update_uboot
>>      verified-boot
>>      x86-fit-boot
>> 
>> These other documents should be carefully redacted. E.g. howto.rst
>> refers to the FIT file format as "the new uImage format".
>
>I agree that it would be nice to take a pass through all this documentation 
>and clean it up, but I think there is value in doing small cleanups as one has 
>time.
>
>> 
>> Best regards
>> 
>> Heinrich
>> 
>>> diff --git a/doc/usage/fit/source_file_format.rst 
>>> b/doc/usage/fit/source_file_format.rst
>>> index b2b1e42bd7..0c44329a82 100644
>>> --- a/doc/usage/fit/source_file_format.rst
>>> +++ b/doc/usage/fit/source_file_format.rst
>>> @@ -3,682 +3,6 @@
>>>   Flattened Image Tree (FIT) Format
>>>   =
>>> 
>>> -Introduction
>>> -
>>> -
>>> -The number of elements playing a role in the kernel booting process has
>>> -increased over time and now typically includes the devicetree, kernel 
>>> image and
>>> -possibly a ramdisk image. Generally, all must be placed in the system 
>>> memory and
>>> -booted together.
>>> -
>>> -For firmware images a similar process has taken place, with various 
>>> binaries
>>> -loaded at different addresses, such as ARM's ATF, OpenSBI, FPGA and U-Boot
>>> -itself.
>>> -
>>> -FIT provides a flexible and extensible format to deal with this 
>>> complexity. It
>>> -provides support for multiple components. It also supports multiple
>>> -configurations, so that the same FIT can be used to boot multiple boards, 
>>> with
>>> -some components in common (e.g. kernel) and some specific to that board 
>>> (e.g.
>>> -devicetree).
>>> -
>>> -Terminology
>>> -~~~
>>> -
>>> -This document defines FIT by providing FDT (Flat Device Tree) bindings

[PATCH 1/1] doc: fix heading level of itest examples

2024-07-01 Thread Heinrich Schuchardt
The Examples section should be on the second heading level.

Signed-off-by: Heinrich Schuchardt 
---
 doc/usage/cmd/itest.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/usage/cmd/itest.rst b/doc/usage/cmd/itest.rst
index 9c307fb4bf4..adcad05b2d4 100644
--- a/doc/usage/cmd/itest.rst
+++ b/doc/usage/cmd/itest.rst
@@ -58,7 +58,7 @@ op
  ==
 
 Examples
-
+
 
 The itest command sets the result variable $? to true (0) or false (1):
 
-- 
2.45.2



[PATCH 1/1] rtc: fix the description of the Goldfish RTC driver

2024-07-01 Thread Heinrich Schuchardt
Replace the incorrect description that was copied from another driver.

Signed-off-by: Heinrich Schuchardt 
---
 drivers/rtc/goldfish_rtc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/goldfish_rtc.c b/drivers/rtc/goldfish_rtc.c
index 3231eb0daf8..e63a2766c76 100644
--- a/drivers/rtc/goldfish_rtc.c
+++ b/drivers/rtc/goldfish_rtc.c
@@ -2,7 +2,9 @@
 /*
  * Copyright 2023, Heinrich Schuchardt 
  *
- * This driver emulates a real time clock based on timer ticks.
+ * This driver supports the Google Goldfish virtual platform RTC device.
+ * The device is provided by the RISC-V virt machine in QEMU. It exposes
+ * a 64-bit nanosecond timer via two memory-mapped 32-bit registers.
  */
 
 #include 
-- 
2.45.2



[NEXT] Pull request efi-2024-10-rc1

2024-06-30 Thread Heinrich Schuchardt

Dear Tom,

The following changes since commit 899b088674b6905710ce546f0a8848662904852a:

  Merge patch series "pxe: Add debugging for booting" (2024-06-26
13:17:52 -0600)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-efi.git
tags/efi-2024-10-rc1

for you to fetch changes up to 636480e4e7088d05d7ff77af72ca00443c62b3e9:

  doc: develop: Add a general section on gdb usage (2024-06-30 13:58:31
+0200)
Gitlab CI showed no issues:
https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21383


Pull request efi-2024-10-rc1

Documentation:
  Update requirements.txt to use current Python module versions
  Add a page describing debugging U-Boot with GDB
  FIT: describe data-size as a conditionally mandatory property
  Correct link to FIT specification in SPL code.
  Correct kaslrseed command long text description

UEFI:
  Add unit test checking that don't have kaslr-seed when measuring boot
  Deduplicate code for measured boot.

Other:
  Print size information in fwu command


Alexander Dahl (1):
  doc: develop: Add a general section on gdb usage

Bastian Germann (1):
  doc: fit: Make data-size a conditionally mandatory property

Heinrich Schuchardt (4):
  doc: update requirements.txt
  spl: correct link to FIT specification
  efi_selftest: can't have measured device-tree with kaslr-seed
  cmd: correct kaslrseed description

Ilias Apalodimas (6):
  efi_loader: remove unused TCG algo definitions
  tpm: Move TCG headers into a separate file
  tpm: Move TCG functions into a separate file
  efi_loader: remove unneeded header files
  tpm: Untangle tpm2_get_pcr_info()
  tpm: allow the user to select the compiled algorithms

Michal Simek (1):
  cmd: fwu: Also print information about size

 boot/Kconfig |   4 +
 boot/bootm.c |   1 +
 cmd/fwu_mdata.c  |   1 +
 cmd/kaslrseed.c  |   2 +-
 common/spl/spl_fit.c |   2 +-
 doc/develop/gdb.rst  | 171 
 doc/develop/index.rst|   1 +
 doc/sphinx/requirements.txt  |  16 +-
 doc/usage/fit/source_file_format.rst |   6 +-
 include/efi_tcg2.h   |   9 +-
 include/tpm-v2.h | 388 +++---
 include/tpm_tcg2.h   | 348 
 lib/Kconfig  |   6 +-
 lib/Makefile |   2 +
 lib/efi_loader/efi_tcg2.c|   3 +-
 lib/efi_selftest/efi_selftest_fdt.c  |   7 +
 lib/tpm-v2.c | 767
+++
 lib/tpm_tcg2.c   | 731
+
 18 files changed, 1386 insertions(+), 1079 deletions(-)
 create mode 100644 doc/develop/gdb.rst
 create mode 100644 include/tpm_tcg2.h
 create mode 100644 lib/tpm_tcg2.c


Re: [PATCH] doc: add bootelf command documentation

2024-06-30 Thread Heinrich Schuchardt

On 6/30/24 12:37, Maxim Moskalets wrote:

Signed-off-by: Maxim Moskalets 
---
  doc/usage/cmd/bootelf.rst | 52 +++
  1 file changed, 52 insertions(+)
  create mode 100644 doc/usage/cmd/bootelf.rst

diff --git a/doc/usage/cmd/bootelf.rst b/doc/usage/cmd/bootelf.rst
new file mode 100644
index 00..5472a90fe2
--- /dev/null
+++ b/doc/usage/cmd/bootelf.rst
@@ -0,0 +1,52 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright 2024, Maxim Moskalets 
+
+.. index::
+   single: bootelf (command)
+
+bootelf command
+===
+
+Synopsis
+
+
+::
+
+bootelf [-p|-s] [-d ] [ []...]
+
+Description
+---
+
+The *bootelf* command is used to launch a ELF binary at *image_addr*. If
+*image_addr* is not specified, the bootelf command will try to find image in
+*image_load_addr* variable (*CONFIG\_SYS\_LOAD\_ADDR* by default).
+
+Args after *image_addr* will be passed to application in common *argc*, *argv*
+format.
+
+A command sequence to run a ELF image using FDT might look like
+
+::
+
+load mmc 0:1 ${loadaddr} /kernel.elf
+load mmc 0:1 ${fdt_addr_r} /soc-board.dtb
+bootelf -d ${fdt_addr_r} ${loadaddr} ${loadaddr}


Thank you for providing this man-page.

Please, move the example to an Examples section. See other man-pages.


+
+image_addr
+Address of the ELF binary.
+
+fdt_addr
+Address of the device-tree. This argument in only needed if bootable
+application uses FDT that requires additional setup (like /memory node).
+
+arg
+Any text arguments for bootable application. This is usually the address
+of the device-tree.
+
+Flags:
+
+-p|-s
+Load ELF image via program headers (-p) or via section headers (-s).


Please, separate -p -s into different lines.


+
+-d
+Setup FDT by address. Available only if CONFIG_CMD_ELF_FDT_SETUP is 
enabled.


Please, add a Configuration section like in the other man-pages
describing which configuration setting enables the command.

Best regards

Heinrich


Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-30 Thread Heinrich Schuchardt

On 6/22/24 18:38, Ilias Apalodimas wrote:

On Sat, 22 Jun 2024 at 19:36, Heinrich Schuchardt  wrote:


On 20.06.24 22:15, Ilias Apalodimas wrote:

EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
at runtime is not supported and allow the platform to perform capsule
updates on disk. With the recent changes boards can conditionally enable
setvariable at runtime using EFI_RT_VOLATILE_STORE.

Let's make that visible in our Kconfigs and enable EFI_IGNORE_OSINDICATIONS
when set variable at runtime is disabled.

Since EFI_RT_VOLATILE_STORE needs help from the OS to persist the
variables, allow users to ignore OsIndications even if setvariable at
runtime is enabled.

Signed-off-by: Ilias Apalodimas 


So this v2:


Yes sorry, forgot to add the tile and log...


With this patch I get a failure on the sandbox in the CI:

https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21382

Without the patch the sandbox runs fine:

https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21383

Best regards

Heinrich





v2:
 allow EFI_IGNORE_OSINDICATIONS if EFI_RT_VOLATILE_STORE=y

Reviewed-by: Heinrich Schuchardt 


Thanks Heinrich




---
   lib/efi_loader/Kconfig | 1 +
   1 file changed, 1 insertion(+)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index ee71f417147a..6006e845cb1f 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -220,6 +220,7 @@ config EFI_CAPSULE_ON_DISK
   config EFI_IGNORE_OSINDICATIONS
   bool "Ignore OsIndications for CapsuleUpdate on-disk"
   depends on EFI_CAPSULE_ON_DISK
+ default y if !EFI_RT_VOLATILE_STORE
   help
 There are boards where U-Boot does not support SetVariable at runtime.
 Select this option if you want to use the capsule-on-disk feature
--
2.43.0







[PATCH v2 1/1] efi_loader: print device-tree in dtbdump.efi

2024-06-29 Thread Heinrich Schuchardt
The dtbdump.efi binary can be used for testing the EFI_DT_FIXUP_PROTOCOL.
It provides a command to load a file and have it fixed up and a
command to save the resulting file.

Add a command 'dump' for displaying the device-tree.

Signed-off-by: Heinrich Schuchardt 
---
v2:
print leading '/dts-v1/;'
print memory reservation block
allow space in string property value to match dtc logic
correct formatting of byte strings
---
 lib/efi_loader/dtbdump.c | 261 +++
 1 file changed, 261 insertions(+)

diff --git a/lib/efi_loader/dtbdump.c b/lib/efi_loader/dtbdump.c
index 5f39cf22da7..116593d9e25 100644
--- a/lib/efi_loader/dtbdump.c
+++ b/lib/efi_loader/dtbdump.c
@@ -40,6 +40,53 @@ static void print(u16 *string)
cout->output_string(cout, string);
 }
 
+/**
+ * print_char() - print character
+ *
+ * 0x00 is replaced by '", "'.
+ *
+ * @c: - character
+ */
+static void print_char(unsigned char c)
+{
+   u16 out[2] = u"?";
+
+   if (!c) {
+   print(u"\", \"");
+   return;
+   }
+
+   if (c > 0x1f && c < 0x80)
+   out[0] = c;
+
+   print(out);
+}
+
+/**
+ * print_hex_digit() - print hexadecimal digit
+ *
+ * @digit: digit to print
+ */
+static void print_hex_digit(unsigned char digit)
+{
+   if (digit < 10)
+   digit += '0';
+   else
+   digit += 'a' - 10;
+   print_char(digit);
+}
+
+/**
+ * printx() - print hexadecimal byte
+ *
+ * @val:   value to print
+ */
+static void printx(unsigned char val)
+{
+   print_hex_digit(val >> 4);
+   print_hex_digit(val & 0xf);
+}
+
 /**
  * error() - print error string
  *
@@ -227,6 +274,7 @@ bool starts_with(u16 *string, u16 *keyword)
  */
 void do_help(void)
 {
+   error(u"dump   - print device-tree\r\n");
error(u"load  - load device-tree from file\r\n");
error(u"save  - save device-tree to file\r\n");
error(u"exit   - exit the shell\r\n");
@@ -489,6 +537,217 @@ efi_status_t do_save(u16 *filename)
return ret;
 }
 
+/**
+ * indent() - print a number of tabstops
+ *
+ * @level: indentation level
+ */
+static void indent(u32 level)
+{
+   for (; level; --level)
+   print(u"\t");
+}
+
+/**
+ * is_string_value() - determine if property is a string
+ *
+ * If a property is a string, an x-string, or a u32 cannot be deducted
+ * from the device-tree. Therefore a heuristic is used.
+ *
+ * @str:   pointer to device-tree property
+ * @len:   length of the device-tree property
+ * Return: 1 for string, 0 otherwise
+ */
+static int is_string_value(const unsigned char *str, u32 len)
+{
+   int nonzero_flag = 0;
+
+   /* Zero length or not ending with 0x00 */
+   if (!len || str[len - 1])
+   return 0;
+
+   for (u32 i = 0; i < len; ++i) {
+   if (!str[i]) {
+   /* Zero length string or two consecutive 0x00 */
+   if (!nonzero_flag)
+   return 0;
+
+   nonzero_flag = 0;
+
+   continue;
+   }
+   /* Non-printable */
+   if (str[i] < 0x20 || str[i] >= 0x80)
+   return 0;
+
+   nonzero_flag = 1;
+   }
+
+   return 1;
+}
+
+/**
+ * print_property() - print device-tree property
+ *
+ * If a property is a string, an x-string, or a u32 cannot be deducted
+ * from the device-tree. Therefore a heuristic is used.
+ *
+ * @str:   property value
+ * @len:   length of property value
+ */
+static void print_property(const unsigned char *val, u32 len)
+{
+   if (is_string_value(val, len)) {
+   /* string */
+   print(u"\"");
+   for (int i = 0; i < len - 1; ++i)
+   print_char(val[i]);
+   print(u"\"");
+   } else if (len & 0x3) {
+   /* byte string */
+   print(u"[");
+   for (int i = 0; i < len; ++i) {
+   if (i)
+   print(u" ");
+   printx(val[i]);
+   }
+   print(u"]\"");
+   } else {
+   /* cell list */
+   print(u"<");
+   for (u32 i = 0; i < len; ++i) {
+   if ((i & 0x3) == 0) {
+   if (i > 0)
+   print(u" ");
+   print(u"0x");
+   }
+   printx(val[i]);
+   }
+   print(u">");
+   }
+}
+
+/**
+ * print_mem_res_block() - print memory reservation

[PATCH 1/1] efi_loader: print device-tree in dtbdump.efi

2024-06-28 Thread Heinrich Schuchardt
The dtbdump.efi binary can be used for testing the EFI_DT_FIXUP_PROTOCOL.
It provides a command to load a file and have it fixed up and a
command to save the resulting file.

Add a command 'dump' for displaying the device-tree.

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/dtbdump.c | 228 +++
 1 file changed, 228 insertions(+)

diff --git a/lib/efi_loader/dtbdump.c b/lib/efi_loader/dtbdump.c
index 5f39cf22da7..8dbd69a56ef 100644
--- a/lib/efi_loader/dtbdump.c
+++ b/lib/efi_loader/dtbdump.c
@@ -40,6 +40,53 @@ static void print(u16 *string)
cout->output_string(cout, string);
 }
 
+/**
+ * print_char() - print character
+ *
+ * 0x00 is replaced by '", "'.
+ *
+ * @c: - character
+ */
+static void print_char(unsigned char c)
+{
+   u16 out[2] = u"?";
+
+   if (!c) {
+   print(u"\", \"");
+   return;
+   }
+
+   if (c > 0x1f && c < 0x80)
+   out[0] = c;
+
+   print(out);
+}
+
+/**
+ * print_hex_digit() - print hexadecimal digit
+ *
+ * @digit: digit to print
+ */
+static void print_hex_digit(unsigned char digit)
+{
+   if (digit < 10)
+   digit += '0';
+   else
+   digit += 'a' - 10;
+   print_char(digit);
+}
+
+/**
+ * printx() - print hexadecimal byte
+ *
+ * @val:   value to print
+ */
+static void printx(unsigned char val)
+{
+   print_hex_digit(val >> 4);
+   print_hex_digit(val & 0xf);
+}
+
 /**
  * error() - print error string
  *
@@ -227,6 +274,7 @@ bool starts_with(u16 *string, u16 *keyword)
  */
 void do_help(void)
 {
+   error(u"dump   - print device-tree\r\n");
error(u"load  - load device-tree from file\r\n");
error(u"save  - save device-tree to file\r\n");
error(u"exit   - exit the shell\r\n");
@@ -489,6 +537,184 @@ efi_status_t do_save(u16 *filename)
return ret;
 }
 
+/**
+ * indent() - print a number of tabstops
+ *
+ * @level: indentation level
+ */
+static void indent(u32 level)
+{
+   for (; level; --level)
+   print(u"\t");
+}
+
+/**
+ * is_string_value() - determine if property is a string
+ *
+ * If a property is a string, an x-string, or a u32 cannot be deducted
+ * from the device-tree. Therefore a heuristic is used.
+ *
+ * @str:   pointer to device-tree property
+ * @len:   length of the device-tree property
+ * Return: 1 for string, 0 otherwise
+ */
+static int is_string_value(const unsigned char *str, u32 len)
+{
+   int nonzero_flag = 0;
+
+   /* Zero length or not ending with 0x00 */
+   if (!len || str[len - 1])
+   return 0;
+
+   for (u32 i = 0; i < len; ++i) {
+   if (!str[i]) {
+   /* Zero length string or two consecutive 0x00 */
+   if (!nonzero_flag)
+   return 0;
+
+   nonzero_flag = 0;
+
+   continue;
+   }
+   /* Space or non-printable */
+   if (str[i] <= ' ' || str[i] >= 0x80)
+   return 0;
+
+   nonzero_flag = 1;
+   }
+
+   return 1;
+}
+
+/**
+ * print_property() - print device-tree property
+ *
+ * If a property is a string, an x-string, or a u32 cannot be deducted
+ * from the device-tree. Therefore a heuristic is used.
+ *
+ * @str:   property value
+ * @len:   length of property value
+ */
+static void print_property(const unsigned char *val, u32 len)
+{
+   if (is_string_value(val, len)) {
+   print(u"\"");
+   for (int i = 0; i < len - 1; ++i)
+   print_char(val[i]);
+   print(u"\"");
+   } else if (len & 0x3) {
+   print(u"<0x");
+   for (int i = 0; i < len; ++i)
+   printx(val[i]);
+   print(u">\"");
+   } else {
+   print(u"<");
+   for (u32 i = 0; i < len; ++i) {
+   if ((i & 0x3) == 0) {
+   if (i > 0)
+   print(u" ");
+   print(u"0x");
+   }
+   printx(val[i]);
+   }
+   print(u">");
+   }
+}
+
+/**
+ * do_dump() - print device-tree
+ */
+static efi_status_t do_dump(void)
+{
+   const char *fdt;
+   struct fdt_header *header;
+   const u32 *end;
+   const u32 *pos;
+   const char *strings;
+   u32 level = 0;
+
+   fdt = get_dtb(systable);
+   if (!fdt) {
+   error(u"DTB not found\r\n");
+   return EFI_NOT_FOUND;
+   }
+
+   header = (struct fdt_hea

Re: [PATCH next] doc: develop: Add a general section on gdb usage

2024-06-27 Thread Heinrich Schuchardt

On 6/26/24 12:47, Alexander Dahl wrote:

Mashed up from different sources linked below, including the now gone
Wiki and doc/README.arm-relocation file.  Tested on a custom board with
AT91 SAMA5D2 SoC and Segger J-Link Base adapter.  This is only generic
advice here, the usage is not board specific.  Some board docs have more
specific instructions on using gdb with a particular board.

Link: 
https://www.slideshare.net/slideshow/embedded-recipes-2019-introduction-to-jtag-debugging/177511981
Link: https://boundarydevices.com/debugging-using-segger-j-link-jtag/
Link: 
https://web.archive.org/web/20141224200032/http://www.denx.de/wiki/view/DULG/DebuggingUBoot
Link: 
https://web.archive.org/web/20141206064148/http://www.denx.de/wiki/view/DULG/GDBScripts1
Suggested-by: Marek Vasut 
Signed-off-by: Alexander Dahl 
---
  doc/develop/gdb.rst   | 154 ++
  doc/develop/index.rst |   1 +
  2 files changed, 155 insertions(+)
  create mode 100644 doc/develop/gdb.rst

diff --git a/doc/develop/gdb.rst b/doc/develop/gdb.rst
new file mode 100644
index 000..18d97857ce1
--- /dev/null
+++ b/doc/develop/gdb.rst
@@ -0,0 +1,154 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright (c) 2024 Alexander Dahl
+
+Debugging U-Boot with gdb
+=
+
+Using a JTAG adapter it is possible to debug a running U-Boot with gdb.
+A common way is to connect a debug adapter to the JTAG connector of your
+board, run gdbserver, connect gdb to gdbserver, and use gdb as usual.
+
+Preparing build
+---
+
+Building U-Boot with debug symbols and without optimizations is
+recommended for easier debugging::
+
+CONFIG_CC_OPTIMIZE_FOR_DEBUG=y
+
+Otherwise build, install, and run U-Boot as usual.
+
+Using OpenOCD as GDB server
+---
+
+[OpenOCD]_ is a FOSS tool supporting hardware debug probes, and
+providing a GDB interface.
+It is readily available in major Linux distributions or you can build it
+from source.
+Example for starting OpenOCD from Debian with a J-Link adapter and a
+board using an AT91 SAMA5D2 SoC:
+
+.. code-block:: console
+
+% openocd -f interface/jlink.cfg -f target/at91sama5d2.cfg -c 'adapter 
speed 4000'
+Open On-Chip Debugger 0.12.0
+Licensed under GNU GPL v2
+For bug reports, read
+http://openocd.org/doc/doxygen/bugs.html
+Info : auto-selecting first available session transport "jtag". To override use 
'transport select '.
+adapter speed: 4000 kHz
+
+Info : Listening on port  for tcl connections
+Info : Listening on port  for telnet connections
+Info : J-Link V10 compiled Jan 30 2023 11:28:07
+Info : Hardware version: 10.10
+Info : VTarget = 3.244 V
+Info : clock speed 4000 kHz
+Info : JTAG tap: at91sama5d2.cpu tap/device found: 0x5ba00477 (mfg: 0x23b 
(ARM Ltd), part: 0xba00, ver: 0x5)
+Info : at91sama5d2.cpu_a5.0: hardware has 3 breakpoints, 2 watchpoints
+Info : at91sama5d2.cpu_a5.0: MPIDR level2 0, cluster 0, core 0, mono core, 
no SMT
+Info : starting gdb server for at91sama5d2.cpu_a5.0 on 
+Info : Listening on port  for gdb connections
+
+Notice the port  the OpenOCD server is listening on for gdb
+connections.
+
+Running a gdb session
+--
+
+You need a gdb suited for your target.
+This can be gdb coming with your toolchain or *gdb-multiarch* available
+in your Linux distribution.
+
+.. code-block:: console
+
+% gdb-multiarch u-boot
+GNU gdb (Debian 13.1-3) 13.1
+Copyright (C) 2023 Free Software Foundation, Inc.
+License GPLv3+: GNU GPL version 3 or later 

+This is free software: you are free to change and redistribute it.
+There is NO WARRANTY, to the extent permitted by law.
+Type "show copying" and "show warranty" for details.
+This GDB was configured as "x86_64-linux-gnu".
+Type "show configuration" for configuration details.
+For bug reporting instructions, please see:
+.
+Find the GDB manual and other documentation resources online at:
+.
+
+For help, type "help".
+Type "apropos word" to search for commands related to "word"...
+Reading symbols from u-boot...
+(gdb)
+
+Notice in the above command-line *u-boot* is the binary file from your
+build folder, for which you might need to adapt the path when calling
+gdb.
+
+Connect to the gdb server then:


Thank you for this helpful document.

Please add:

.. code-block:: bash

gdb-multiarch u-boot -ex 'target remote localhost:'

Please, mention that the same debug port can be exposed on QEMU with
parameter '-gdb tcp::', e.g.

qemu-system-riscv64 -M virt -nographic -gdb tcp:: -kernel u-boot


+
+.. code-block:: console
+
+(gdb) target extended-remote :
+Remote debugging using :
+0x27fa9ac6 in ?? ()
+(gdb)
+
+This is fine 

Re: [PATCH] bootstd: cros: store partition type in an efi_guid_t

2024-06-27 Thread Heinrich Schuchardt



Am 27. Juni 2024 19:06:29 MESZ schrieb "Vincent Stehlé" 
:
>The scan_part() function uses a struct uuid to store the little-endian
>partition type GUID, but this structure should be used only to contain a
>big-endian UUID. Use an efi_guid_t instead.
>
>Signed-off-by: Vincent Stehlé 
>Cc: Simon Glass 
>Cc: Tom Rini 
>---
> boot/bootmeth_cros.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c
>index f015f2e1c75..1f83c14aeab 100644
>--- a/boot/bootmeth_cros.c
>+++ b/boot/bootmeth_cros.c
>@@ -148,7 +148,7 @@ static int scan_part(struct udevice *blk, int partnum,
> {
>   struct blk_desc *desc = dev_get_uclass_plat(blk);
>   struct vb2_keyblock *hdr;
>-  struct uuid type;
>+  efi_guid_t type;

Does Chrome OS only support GPT partitioning?

>   ulong num_blks;
>   int ret;
> 
>@@ -161,7 +161,7 @@ static int scan_part(struct udevice *blk, int partnum,
> 
>   /* Check for kernel partition type */
>   log_debug("part %x: type=%s\n", partnum, info->type_guid);
>-  if (uuid_str_to_bin(info->type_guid, (u8 *), UUID_STR_FORMAT_GUID))
>+  if (uuid_str_to_bin(info->type_guid, type.b, UUID_STR_FORMAT_GUID))
>   return log_msg_ret("typ", -EINVAL);

struct disk_partition containing a string which is only needed in the CLI 
instead of the 16 byte GUID was a bad idea to start with. Shouldn't we replace 
it or add least add a GUID field instead of first converting to string and than 
back to GUID?

> 
>   if (memcmp(_kern_type, , sizeof(type)))

You could use the guidcmp() macro here.

Best regards

Heinrich



Re: [PATCH 14/14] smbios: Correct error handling when writing tables

2024-06-23 Thread Heinrich Schuchardt



Am 23. Juni 2024 22:30:33 MESZ schrieb Simon Glass :
>Since write_smbios_table() returns an address, we cannot use it to
>return and error number. Also, failing on sysinfo_detect() breaks

IS_ERR_VALUE() could serve as template for conveying errors in addresses.



>existing boards, e.g. chromebook_link
>
>Correct this by logging and swallowing the error.
>
>Signed-off-by: Simon Glass 
>Fixes: a5a57562856 ("lib: smbios: Detect system properties via...")
>---
>
>(no changes since v1)
>
> lib/smbios.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/lib/smbios.c b/lib/smbios.c
>index fb6eaf1d5ca..4126466e34a 100644
>--- a/lib/smbios.c
>+++ b/lib/smbios.c
>@@ -5,6 +5,8 @@
>  * Adapted from coreboot src/arch/x86/smbios.c
>  */
> 
>+#define LOG_CATEGORY  LOGC_BOARD
>+
> #include 
> #include 
> #include 
>@@ -596,8 +598,12 @@ ulong write_smbios_table(ulong addr)
> 
>   parent_node = dev_read_subnode(ctx.dev, "smbios");
>   ret = sysinfo_detect(ctx.dev);
>-  if (ret)
>-  return ret;
>+
>+  /*
>+   * ignore the error since many boards don't implement
>+   * this and we can still use the info in the devicetree
>+   */
>+  ret = log_msg_ret("sys", ret);

Can we make this a debug message?
It is nothing an end user should worry about.

Best regards

Heinrich

>   }
>   } else {
>   ctx.dev = NULL;


[U-BOOT-TEST][PATCH 1/1] travis-ci: SiFive Unleashed: avoid format warning

2024-06-23 Thread Heinrich Schuchardt
QEMU expects the file format to be specified even for raw files.
Currently this change only suppresses a warning.

Signed-off-by: Heinrich Schuchardt 
---
 bin/travis-ci/conf.sifive_unleashed_sdcard_qemu  | 2 +-
 bin/travis-ci/conf.sifive_unleashed_spi-nor_qemu | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/bin/travis-ci/conf.sifive_unleashed_sdcard_qemu 
b/bin/travis-ci/conf.sifive_unleashed_sdcard_qemu
index f3c3da1..e84ce7d 100644
--- a/bin/travis-ci/conf.sifive_unleashed_sdcard_qemu
+++ b/bin/travis-ci/conf.sifive_unleashed_sdcard_qemu
@@ -6,6 +6,6 @@ console_impl=qemu
 qemu_machine="sifive_u,msel=11"
 qemu_binary="qemu-system-riscv64"
 qemu_extra_args="-smp 5 -m 8G -nographic -nic 
user,tftp=${UBOOT_TRAVIS_BUILD_DIR}"
-qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/spl/u-boot-spl.bin -drive 
file=${U_BOOT_BUILD_DIR}/sdcard.img,if=sd"
+qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/spl/u-boot-spl.bin -drive 
file=${U_BOOT_BUILD_DIR}/sdcard.img,format=raw,if=sd"
 reset_impl=none
 flash_impl=none
diff --git a/bin/travis-ci/conf.sifive_unleashed_spi-nor_qemu 
b/bin/travis-ci/conf.sifive_unleashed_spi-nor_qemu
index e28bfc4..56a7a0c 100644
--- a/bin/travis-ci/conf.sifive_unleashed_spi-nor_qemu
+++ b/bin/travis-ci/conf.sifive_unleashed_spi-nor_qemu
@@ -6,6 +6,6 @@ console_impl=qemu
 qemu_machine="sifive_u,msel=6"
 qemu_binary="qemu-system-riscv64"
 qemu_extra_args="-smp 5 -m 8G -nographic -nic 
user,tftp=${UBOOT_TRAVIS_BUILD_DIR}"
-qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/spl/u-boot-spl.bin -drive 
file=${U_BOOT_BUILD_DIR}/spi-nor.img,if=mtd"
+qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/spl/u-boot-spl.bin -drive 
file=${U_BOOT_BUILD_DIR}/spi-nor.img,format=raw,if=mtd"
 reset_impl=none
 flash_impl=none
-- 
2.45.1



Re: [PATCH 2/7] efi_loader: fix the return values on efi_tcg

2024-06-22 Thread Heinrich Schuchardt



Am 22. Juni 2024 18:09:40 MESZ schrieb Ilias Apalodimas 
:
>Hi Heinrich,
>
>[...]
>
>> >   rc = tpm2_submit_command(dev, input_param_block,
>> >output_param_block, _buf_size);
>> > @@ -714,19 +721,20 @@ efi_tcg2_get_active_pcr_banks(struct 
>> > efi_tcg2_protocol *this,
>> > u32 *active_pcr_banks)
>> >   {
>> >   struct udevice *dev;
>> > - efi_status_t ret;
>> > + efi_status_t ret = EFI_INVALID_PARAMETER;
>> >
>> >   EFI_ENTRY("%p, %p", this, active_pcr_banks);
>> >
>> > - if (!this || !active_pcr_banks) {
>> > - ret = EFI_INVALID_PARAMETER;
>> > + if (!this || !active_pcr_banks)
>> >   goto out;
>> > - }
>> > - ret = tcg2_platform_get_tpm2();
>> > - if (ret != EFI_SUCCESS)
>> > +
>> > + if (tcg2_platform_get_tpm2())
>>
>> EFI_INVALID_PARAMETER does not convey the problem type.
>> Should we return EFI_DEVICE_ERROR here?
>>
>> The authors of the specification only foresaw one or more of the
>> parameters being incorrect (EFI_INVALID_PARAMETER).
>
>I completely agree that the result is misleading. However, I'd prefer
>sticking to the spec for now and maybe add a comment?
>
>
>>
>> > + goto out;
>> > +
>> > + if (tcg2_get_active_pcr_banks(dev, active_pcr_banks))
>>
>> EFI_DEVICE_ERROR?
>
>Same here
>
>Thanks for the qucik review!
>/Ilias


Reviewed-by: Heinrich Schuchardt 


>>
>> Best regards
>>
>> Heinrich
>>
>> >   goto out;
>> >
>> > - ret = tcg2_get_active_pcr_banks(dev, active_pcr_banks);
>> > + ret = EFI_SUCCESS;
>> >
>> >   out:
>> >   return EFI_EXIT(ret);
>> > @@ -852,14 +860,15 @@ static efi_status_t measure_event(struct udevice 
>> > *dev, u32 pcr_index,
>> > u32 event_type, u32 size, u8 event[])
>> >   {
>> >   struct tpml_digest_values digest_list;
>> > - efi_status_t ret;
>> > + efi_status_t ret = EFI_DEVICE_ERROR;
>> > + int rc;
>> >
>> > - ret = tcg2_create_digest(dev, event, size, _list);
>> > - if (ret != EFI_SUCCESS)
>> > + rc = tcg2_create_digest(dev, event, size, _list);
>> > + if (rc)
>> >   goto out;
>> >
>> > - ret = tcg2_pcr_extend(dev, pcr_index, _list);
>> > - if (ret != EFI_SUCCESS)
>> > + rc = tcg2_pcr_extend(dev, pcr_index, _list);
>> > + if (rc)
>> >   goto out;
>> >
>> >   ret = tcg2_agile_log_append(pcr_index, event_type, _list,
>> > @@ -901,10 +910,10 @@ static efi_status_t efi_init_event_log(void)
>> >   struct tcg2_event_log elog;
>> >   struct udevice *dev;
>> >   efi_status_t ret;
>> > + int rc;
>> >
>> > - ret = tcg2_platform_get_tpm2();
>> > - if (ret != EFI_SUCCESS)
>> > - return ret;
>> > + if (tcg2_platform_get_tpm2())
>> > + return EFI_DEVICE_ERROR;
>> >
>> >   ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
>> >   (void **)_log.buffer);
>> > @@ -933,9 +942,11 @@ static efi_status_t efi_init_event_log(void)
>> >*/
>> >   elog.log = event_log.buffer;
>> >   elog.log_size = TPM2_EVENT_LOG_SIZE;
>> > - ret = tcg2_log_prepare_buffer(dev, , false);
>> > - if (ret != EFI_SUCCESS)
>> > + rc = tcg2_log_prepare_buffer(dev, , false);
>> > + if (rc) {
>> > + ret = (rc == -ENOBUFS) ? EFI_BUFFER_TOO_SMALL : 
>> > EFI_DEVICE_ERROR;
>> >   goto free_pool;
>> > + }
>> >
>> >   event_log.pos = elog.log_position;
>> >
>> > @@ -1306,8 +1317,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
>> >   if (!is_tcg2_protocol_installed())
>> >   return EFI_SUCCESS;
>> >
>> > - ret = tcg2_platform_get_tpm2();
>> > - if (ret != EFI_SUCCESS)
>> > + if (tcg2_platform_get_tpm2())
>> >   return EFI_SECURITY_VIOLATION;
>> >
>> >   rsvmap_size = size_of_rsvmap(dtb);
>> > @@ -1356,8 +1366,7 @@ efi_status_t 
>> > efi_tcg2_measure_efi_app_in

Pull request doc-2024-07-rc5-2

2024-06-22 Thread Heinrich Schuchardt

Dear Tom,

The following changes since commit fe2ce09a0753634543c32cafe85eb87a625f76ca:

  Merge branch 'master' of
https://source.denx.de/u-boot/custodians/u-boot-watchdog (2024-06-18
08:34:56 -0600)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-efi.git
tags/doc-2024-07-rc5-2

for you to fetch changes up to 271ca9ef8a37b8668b9858638175ac2da5c152df:

  doc: board: ti: Add capsule documentation for TI K3 devices
(2024-06-22 17:04:19 +0200)

Gitlab CI showed no issues:
https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/21289


Pull request doc-2024-07-rc5-2

Documentation:

* Fix broken references to pytest suite and test writing
* Fix links to FIT documentation
* Add capsule documentation for TI K3 devices


Alexander Dahl (2):
  doc: develop: testing: Fix broken reference to pytest suite help
  doc: develop: testing: Fix reference to test writing section

Heinrich Schuchardt (3):
  cmd: link to doc/usage/fit/x86-fit-boot.rst
  doc: FIT links in develop/uefi/uefi.rst
  boot: links to FIT documentation in Kconfig

Jonathan Humphreys (1):
  doc: board: ti: Add capsule documentation for TI K3 devices

 boot/Kconfig  |  6 +++---
 cmd/Kconfig   |  2 +-
 doc/board/ti/k3.rst   | 29 +
 doc/develop/testing.rst   |  4 ++--
 doc/develop/uefi/uefi.rst |  4 ++--
 5 files changed, 37 insertions(+), 8 deletions(-)


Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-22 Thread Heinrich Schuchardt

On 20.06.24 22:15, Ilias Apalodimas wrote:

EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
at runtime is not supported and allow the platform to perform capsule
updates on disk. With the recent changes boards can conditionally enable
setvariable at runtime using EFI_RT_VOLATILE_STORE.

Let's make that visible in our Kconfigs and enable EFI_IGNORE_OSINDICATIONS
when set variable at runtime is disabled.

Since EFI_RT_VOLATILE_STORE needs help from the OS to persist the
variables, allow users to ignore OsIndications even if setvariable at
runtime is enabled.

Signed-off-by: Ilias Apalodimas 


So this v2:

v2:
allow EFI_IGNORE_OSINDICATIONS if EFI_RT_VOLATILE_STORE=y

Reviewed-by: Heinrich Schuchardt 


---
  lib/efi_loader/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index ee71f417147a..6006e845cb1f 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -220,6 +220,7 @@ config EFI_CAPSULE_ON_DISK
  config EFI_IGNORE_OSINDICATIONS
bool "Ignore OsIndications for CapsuleUpdate on-disk"
depends on EFI_CAPSULE_ON_DISK
+   default y if !EFI_RT_VOLATILE_STORE
help
  There are boards where U-Boot does not support SetVariable at runtime.
  Select this option if you want to use the capsule-on-disk feature
--
2.43.0





Re: [PATCH 7/7] tpm: allow the user to select the compiled algorithms

2024-06-22 Thread Heinrich Schuchardt

On 22.06.24 16:35, Ilias Apalodimas wrote:

Simon reports that after enabling all algorithms on the TPM some boards
fail since they don't have enough storage to accommodate the ~5KB growth.

The choice of hash algorithms are determined by the platform and the TPM
configuration. Failing to cap a PCR in a bank which the platform left
active is a security vulnerability. It might allow  unsealing of secrets
if an attacker can replay a good set of measurements into an unused bank.

If MEASURED_BOOT or EFI_TCG2_PROTOCOL is enabled our Kconfig will enable
all supported hashing algorithms. We still want to allow users to add a
TPM and not enable measured boot via EFI or bootm though and at the same
time, control the compiled algorithms for size reasons.

So let's add a function tpm2_allow_extend() which checks the TPM active
PCRs banks against the one U-Boot was compiled with.
If all the active PCRs banks are not enabled refuse to extend a PCR but
otherwise leave the TPM functional.


The paragraph above is bit hard to read. I guess you mean:

We only allow extending PCRs using one of the algorithms selected in the
configuration.



It's worth noting that this is only added on TPM2.0, since TPM1.2 is
lacking a lot of code at the moment to read the available PCRs.
We unconditionally enable SHA1 when a TPM is selected, which is the only
hashing algorithm v1.2 supports.


Why do we need SHA1 if we cannot access PCRs on a TPM1.2?

Best regards

Heinrich



Signed-off-by: Ilias Apalodimas 
---
  boot/Kconfig |  4 
  include/tpm-v2.h | 59 +++-
  lib/Kconfig  |  6 ++---
  lib/tpm-v2.c | 40 +---
  4 files changed, 87 insertions(+), 22 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index 6f3096c15a6f..b061891e109c 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT
  config MEASURED_BOOT
bool "Measure boot images and configuration when booting without EFI"
depends on HASH && TPM_V2
+   select SHA1
+   select SHA256
+   select SHA384
+   select SHA512
help
  This option enables measurement of the boot process when booting
  without UEFI . Measurement involves creating cryptographic hashes
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index eac04d1c6831..fccb07fa4695 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -277,48 +277,40 @@ struct digest_info {
  #define TCG2_BOOT_HASH_ALG_SM3_256 0x0010

  static const struct digest_info hash_algo_list[] = {
+#if IS_ENABLED(CONFIG_SHA1)
{
"sha1",
TPM2_ALG_SHA1,
TCG2_BOOT_HASH_ALG_SHA1,
TPM2_SHA1_DIGEST_SIZE,
},
+#endif
+#if IS_ENABLED(CONFIG_SHA256)
{
"sha256",
TPM2_ALG_SHA256,
TCG2_BOOT_HASH_ALG_SHA256,
TPM2_SHA256_DIGEST_SIZE,
},
+#endif
+#if IS_ENABLED(CONFIG_SHA384)
{
"sha384",
TPM2_ALG_SHA384,
TCG2_BOOT_HASH_ALG_SHA384,
TPM2_SHA384_DIGEST_SIZE,
},
+#endif
+#if IS_ENABLED(CONFIG_SHA512)
{
"sha512",
TPM2_ALG_SHA512,
TCG2_BOOT_HASH_ALG_SHA512,
TPM2_SHA512_DIGEST_SIZE,
},
+#endif
  };

-static inline u16 tpm2_algorithm_to_len(enum tpm2_algorithms a)
-{
-   switch (a) {
-   case TPM2_ALG_SHA1:
-   return TPM2_SHA1_DIGEST_SIZE;
-   case TPM2_ALG_SHA256:
-   return TPM2_SHA256_DIGEST_SIZE;
-   case TPM2_ALG_SHA384:
-   return TPM2_SHA384_DIGEST_SIZE;
-   case TPM2_ALG_SHA512:
-   return TPM2_SHA512_DIGEST_SIZE;
-   default:
-   return 0;
-   }
-}
-
  /* NV index attributes */
  enum tpm_index_attrs {
TPMA_NV_PPWRITE = 1UL << 0,
@@ -711,6 +703,41 @@ enum tpm2_algorithms tpm2_name_to_algorithm(const char 
*name);
   */
  const char *tpm2_algorithm_name(enum tpm2_algorithms);

+/**
+ * tpm2_algorithm_to_len() - Return an algorithm length for supported 
algorithm id
+ *
+ * @algorithm_id: algorithm defined in enum tpm2_algorithms
+ * Return: len or 0 if not supported
+ */
+u16 tpm2_algorithm_to_len(enum tpm2_algorithms algo);
+
+/*
+ * When measured boot is enabled via EFI or bootX commands all the algorithms
+ * above are selected by our Kconfigs. Due to U-Boots nature of being small 
there
+ * are cases where we need some functionality from the TPM -- e.g storage or 
RNG
+ * but we don't want to support measurements.
+ *
+ * The choice of hash algorithms are determined by the platform and the TPM
+ * configuration. Failing to cap a PCR in a bank which the platform left
+ * active is a security vulnerability. It permits the unsealing of secrets
+ * if an attacker can replay a good set of measurements into an unused bank.
+ *
+ * On top of that a previous stage 

Re: [PATCH 1/7] tpm: fix the return code, if the eventlog buffer is full

2024-06-22 Thread Heinrich Schuchardt

On 22.06.24 16:35, Ilias Apalodimas wrote:

We currently return 'No space left on device' if the eventlong buffer
we allocated is not enough. On a similar check later on that function
during the call to tcg2_log_init() we return 'No buffer space
available'. So switch both error codes to -ENOBUFS since we are always
checking a buffer and not a device.

Fixes: commit 97707f12fdab ("tpm: Support boot measurements")
Signed-off-by: Ilias Apalodimas 


Reviewed-by: Heinrich Schuchardt 


---
  lib/tpm-v2.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index a67daed2f3c1..91526af33acb 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -554,7 +554,7 @@ int tcg2_log_prepare_buffer(struct udevice *dev, struct 
tcg2_event_log *elog,
if (elog->log_size) {
if (log.found) {
if (elog->log_size < log.log_position)
-   return -ENOSPC;
+   return -ENOBUFS;

/*
 * Copy the discovered log into the user buffer




Re: [PATCH 3/7] efi_loader: remove duplicate TCG algo definitions

2024-06-22 Thread Heinrich Schuchardt

On 22.06.24 16:35, Ilias Apalodimas wrote:

commit 97707f12fdab ("tpm: Support boot measurements") moved some of the
EFI TCG code to the TPM subsystem. Those definitions are now in tpm-v2.h.
Let's remove the duplicate entries


The constants are not duplicate but unused.
97707f12fdab introduced different constant names (TPM2_ALG_*).

Otherwise

Reviewed-by: Heinrich Schuchardt 



Signed-off-by: Ilias Apalodimas 
---
  include/efi_tcg2.h | 8 
  1 file changed, 8 deletions(-)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index a75b5a35b6e7..54490969b2d1 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -25,14 +25,6 @@
  #define PE_COFF_IMAGE 0x0010

  #define EFI_TCG2_MAX_PCR_INDEX 23
-
-/* Algorithm Registry */
-#define EFI_TCG2_BOOT_HASH_ALG_SHA10x0001
-#define EFI_TCG2_BOOT_HASH_ALG_SHA256  0x0002
-#define EFI_TCG2_BOOT_HASH_ALG_SHA384  0x0004
-#define EFI_TCG2_BOOT_HASH_ALG_SHA512  0x0008
-#define EFI_TCG2_BOOT_HASH_ALG_SM3_256 0x0010
-
  #define EFI_TCG2_FINAL_EVENTS_TABLE_VERSION 1

  #define TPM2_EVENT_LOG_SIZE CONFIG_EFI_TCG2_PROTOCOL_EVENTLOG_SIZE




Re: [PATCH 2/7] efi_loader: fix the return values on efi_tcg

2024-06-22 Thread Heinrich Schuchardt

On 22.06.24 16:35, Ilias Apalodimas wrote:

A while back we moved the core functions of the EFI TCG protocol to the
TPM APIs in order for them to be used with bootm, booti etc.
Some prototypes changed from returning efi_status_t to int, which is more
appropriate for the non-EFI APIs. However, some of the EFI callsites never
changed and we ended up assigning the int value to efi_status_t.

This is unlikely to cause any problems, apart from returning invalid
values on failures and violating the EFI spec. Let's fix them
by looking at the new return code and map it to the proper EFI return
code on failures.

Fixes: commit 97707f12fdab ("tpm: Support boot measurements")
Fixes: commit d6b55a420cfc ("efi_loader: startup the tpm device when installing the 
protocol")
Signed-off-by: Ilias Apalodimas 
---
  lib/efi_loader/efi_tcg2.c | 121 --
  1 file changed, 64 insertions(+), 57 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index d56bd5657c8a..10c09caac35a 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -257,8 +257,8 @@ efi_tcg2_get_capability(struct efi_tcg2_protocol *this,
capability->protocol_version.major = 1;
capability->protocol_version.minor = 1;

-   efi_ret = tcg2_platform_get_tpm2();
-   if (efi_ret != EFI_SUCCESS) {
+   ret = tcg2_platform_get_tpm2();
+   if (ret) {
capability->supported_event_logs = 0;
capability->hash_algorithm_bitmap = 0;
capability->tpm_present_flag = false;
@@ -353,8 +353,7 @@ efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this,
goto out;
}

-   ret = tcg2_platform_get_tpm2();
-   if (ret != EFI_SUCCESS) {
+   if (tcg2_platform_get_tpm2()) {
event_log_location = NULL;
event_log_last_entry = NULL;
*event_log_truncated = false;
@@ -389,7 +388,7 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64 
efi_size,
void *new_efi = NULL;
u8 hash[TPM2_SHA512_DIGEST_SIZE];
struct udevice *dev;
-   efi_status_t ret;
+   efi_status_t ret = EFI_SUCCESS;
u32 active;
int i;

@@ -404,12 +403,13 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64 
efi_size,
goto out;
}

-   ret = tcg2_platform_get_tpm2();
-   if (ret != EFI_SUCCESS)
+   if (tcg2_platform_get_tpm2()) {
+   ret = EFI_DEVICE_ERROR;
goto out;
+   }

-   ret = tcg2_get_active_pcr_banks(dev, );
-   if (ret != EFI_SUCCESS) {
+   if (tcg2_get_active_pcr_banks(dev, )) {
+   ret = EFI_DEVICE_ERROR;
goto out;
}

@@ -473,12 +473,12 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 
efi_size,
IMAGE_DOS_HEADER *dos;
IMAGE_NT_HEADERS32 *nt;
struct efi_handler *handler;
+   int rc;

if (!is_tcg2_protocol_installed())
return EFI_SUCCESS;

-   ret = tcg2_platform_get_tpm2();
-   if (ret != EFI_SUCCESS)
+   if (tcg2_platform_get_tpm2())
return EFI_SECURITY_VIOLATION;

switch (handle->image_type) {
@@ -502,9 +502,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
if (ret != EFI_SUCCESS)
return ret;

-   ret = tcg2_pcr_extend(dev, pcr_index, _list);
-   if (ret != EFI_SUCCESS)
-   return ret;
+   rc = tcg2_pcr_extend(dev, pcr_index, _list);
+   if (rc)
+   return EFI_DEVICE_ERROR;

ret = efi_search_protocol(>header,
  _guid_loaded_image_device_path, );
@@ -574,9 +574,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol 
*this, u64 flags,
   struct efi_tcg2_event *efi_tcg_event)
  {
struct udevice *dev;
-   efi_status_t ret;
+   efi_status_t ret = EFI_SUCCESS;
u32 event_type, pcr_index, event_size;
struct tpml_digest_values digest_list;
+   int rc = 0;

EFI_ENTRY("%p, %llu, %llu, %llu, %p", this, flags, data_to_hash,
  data_to_hash_len, efi_tcg_event);
@@ -586,9 +587,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol 
*this, u64 flags,
goto out;
}

-   ret = tcg2_platform_get_tpm2();
-   if (ret != EFI_SUCCESS)
+   if (tcg2_platform_get_tpm2()) {
+   ret = EFI_DEVICE_ERROR;
goto out;
+   }

if (efi_tcg_event->size < efi_tcg_event->header.header_size +
sizeof(u32)) {
@@ -621,8 +623,10 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol 
*this, u64 flags,
ret = tcg2_hash_pe_image((void *)(uintptr_t)data_to_hash,
 data_to_hash_len, _list);
} else {
-   ret = tcg2_create_digest(dev, (u8 *)(uintptr_t)data_to_hash,
-

Re: [PATCH 5/7] efi_loader: remove unneeded header files

2024-06-22 Thread Heinrich Schuchardt

On 22.06.24 16:35, Ilias Apalodimas wrote:

efi_tcg2.h already includes tpm-v2.h. Remove it

Signed-off-by: Ilias Apalodimas 


Reviewed by: Heinrich Schuchardt 


---
  lib/efi_loader/efi_tcg2.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 10c09caac35a..c654d2cbd704 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -16,7 +16,6 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 
  #include 




Re: [PATCH 4/7] tpm: Move TCG into a separate library

2024-06-22 Thread Heinrich Schuchardt

On 22.06.24 16:35, Ilias Apalodimas wrote:

commit 97707f12fdab ("tpm: Support boot measurements") moved out code
from the EFI subsystem into the TPM one to support measurements when
booting with !EFI.

Those were moved directly into the TPM subsystem and in the tpm-v2.c
library. In hindsight, it would have been better to move it in new
files since the TCG2 is governed by its own spec and it's cleaner
when we want to enable certian parts of the TPM functionality.


Nits:

%s/certian/certain/



So let's create a header file and another library and move the TCG
specific bits there.

Signed-off-by: Ilias Apalodimas 
---
  boot/bootm.c   |   1 +
  include/efi_tcg2.h |   1 +
  include/tpm-v2.h   | 474 +-
  include/tpm_tcg2.h | 336 ++
  lib/Makefile   |   2 +
  lib/tpm-v2.c   | 676 +--
  lib/tpm_tcg2.c | 696 +


The patch series were easier to review if moving header definitions were
separated from moving implementations.

This patch contains changes that are not described in the commit
message, e.g.

   if (elog->log_size) {
   if (log.found) {
   if (elog->log_size < log.log_position)
-  return -ENOBUFS;
+  return -ENOSPC;

I guess you wanted to put this into patch 1.

Please, separate the patches adequately.

+ * Copyright (c) 2020 Linaro
+ * Copyright (c) 2023 Linaro Limited

The copyright lines look inconsistent. Linaro Limited exists under this
name since April 13th, 2010. Is the 2020 copyright for a different company?


  7 files changed, 1114 insertions(+), 1072 deletions(-)
  create mode 100644 include/tpm_tcg2.h
  create mode 100644 lib/tpm_tcg2.c

diff --git a/boot/bootm.c b/boot/bootm.c
index 9879e1bba4eb..395b42cccd88 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -25,6 +25,7 @@
  #include 
  #include 
  #include 
+#include 
  #if defined(CONFIG_CMD_USB)
  #include 
  #endif
diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index 54490969b2d1..8dfb1bc9527b 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -18,6 +18,7 @@

  #include 
  #include 
+#include 

  /* TPMV2 only */
  #define TCG2_EVENT_LOG_FORMAT_TCG_2 0x0002
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index c9d5cb6d3e5a..c176e04c9952 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -26,14 +26,13 @@ struct udevice;
  #define TPM2_SHA512_DIGEST_SIZE   64
  #define TPM2_SM3_256_DIGEST_SIZE 32

+#define TPM2_HDR_LEN   10
+
  #define TPM2_MAX_PCRS 32
  #define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
  #define TPM2_MAX_CAP_BUFFER 1024
  #define TPM2_MAX_TPM_PROPERTIES ((TPM2_MAX_CAP_BUFFER - sizeof(u32) /* 
TPM2_CAP */ - \
 sizeof(u32)) / sizeof(struct 
tpms_tagged_property))
-
-#define TPM2_HDR_LEN   10
-
  /*
   *  We deviate from this draft of the specification by increasing the value of
   *  TPM2_NUM_PCR_BANKS from 3 to 16 to ensure compatibility with TPM2
@@ -55,211 +54,6 @@ struct udevice;
  #define TPM2_PT_MAX_COMMAND_SIZE  (u32)(TPM2_PT_FIXED + 30)
  #define TPM2_PT_MAX_RESPONSE_SIZE (u32)(TPM2_PT_FIXED + 31)

-/*
- * event types, cf.
- * "TCG Server Management Domain Firmware Profile Specification",
- * rev 1.00, 2020-05-01
- */
-#define EV_POST_CODE   ((u32)0x0001)
-#define EV_NO_ACTION   ((u32)0x0003)
-#define EV_SEPARATOR   ((u32)0x0004)
-#define EV_ACTION  ((u32)0x0005)
-#define EV_TAG ((u32)0x0006)
-#define EV_S_CRTM_CONTENTS ((u32)0x0007)
-#define EV_S_CRTM_VERSION  ((u32)0x0008)
-#define EV_CPU_MICROCODE   ((u32)0x0009)
-#define EV_PLATFORM_CONFIG_FLAGS   ((u32)0x000A)
-#define EV_TABLE_OF_DEVICES((u32)0x000B)
-#define EV_COMPACT_HASH((u32)0x000C)
-
-/*
- * event types, cf.
- * "TCG PC Client Platform Firmware Profile Specification", Family "2.0"
- * Level 00 Version 1.05 Revision 23, May 7, 2021
- */
-#define EV_EFI_EVENT_BASE  ((u32)0x8000)
-#define EV_EFI_VARIABLE_DRIVER_CONFIG  ((u32)0x8001)
-#define EV_EFI_VARIABLE_BOOT   ((u32)0x8002)
-#define EV_EFI_BOOT_SERVICES_APPLICATION   ((u32)0x8003)
-#define EV_EFI_BOOT_SERVICES_DRIVER((u32)0x8004)
-#define EV_EFI_RUNTIME_SERVICES_DRIVER ((u32)0x8005)
-#define EV_EFI_GPT_EVENT   ((u32)0x8006)
-#define EV_EFI_ACTION  ((u32)0x8007)
-#define EV_EFI_PLATFORM_FIRMWARE_BLOB  ((u32)0x8008)
-#define EV_EFI_HANDOFF_TABLES  ((u32)0x8009)
-#define EV_EFI_PLATFORM_FIRMWARE_BLOB2 ((u32)0x800A)
-#define EV_EFI_HANDOFF_TABLES2 

Re: [PATCH 1/1] efi_selftest: can't have measured device-tree with kaslr-seed

2024-06-22 Thread Heinrich Schuchardt

On 18.06.24 17:54, Ilias Apalodimas wrote:

On Tue, 18 Jun 2024 at 15:24, Heinrich Schuchardt
 wrote:


Test that we don't have a /chosen/kaslr-seed property if we measure the
device-tree.

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

diff --git a/lib/efi_selftest/efi_selftest_fdt.c 
b/lib/efi_selftest/efi_selftest_fdt.c
index aa3b13ae3ab..066d9581432 100644
--- a/lib/efi_selftest/efi_selftest_fdt.c
+++ b/lib/efi_selftest/efi_selftest_fdt.c
@@ -227,6 +227,13 @@ static int execute(void)
 return EFI_ST_FAILURE;
 }
 }
+   if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
+   str = get_property(u"kaslr-seed", u"chosen");
+   if (str) {
+   efi_st_error("kaslr-seed with measured fdt\n");
+   return EFI_ST_FAILURE;


When does this run? efi_try_purge_kaslr_seed() tries to remove the
kaslr-seed before measuring a DT. Are we safe enavbling the check
here?


do_efi_selftest() is called after efi_install_fdt(). efi_install_fdt() 
invokes efi_try_purge_kaslr_seed().


We would get an error here if efi_try_purge_kaslr_seed() were removed 
and measuring the DTB enabled.


Best regards

Heinrich



Thanks
/Ilias

+   }
+   }
 if (IS_ENABLED(CONFIG_RISCV)) {
 u32 fdt_hartid;

--
2.45.1





Re: [PATCH v5] cmd: bootm: add ELF file support

2024-06-21 Thread Heinrich Schuchardt

On 6/21/24 16:16, Максим Москалец wrote:

пт, 21 июн. 2024 г. в 15:51, Heinrich Schuchardt :


On 21.06.24 13:42, Maxim Moskalets wrote:

Some operating systems (e.g. seL4) and embedded applications are ELF
images. It is convenient to use FIT-images to implement trusted boot.
Added "elf" image type for booting using bootm command.

Signed-off-by: Maxim Moskalets 


I guess a test could be added to test/py/tests/test_zynqmp_rpu.py which
already uses the bootelf command?

Is it necessary to use specific hardware to run this test?




In Gitlab CI xilinx_zynq_virt_defconfig is used with QEMU parameters
from
https://source.denx.de/u-boot/u-boot-test-hooks/-/blob/master/bin/travis-ci/conf.xilinx_zynq_virt_qemu:

qemu-system-arm \
-machine xilinx-zynq-a9 \
-display none \
-m 1G \
-nographic \
-serial /dev/null \
-serial mon:stdio \
-monitor null \
-device loader,file=u-boot-dtb.bin,addr=0x400,cpu-num=0

See for instance this build:
https://source.denx.de/u-boot/custodians/u-boot-efi/-/jobs/852062

Best regards

Heinrich


Heinrich


---
v5:
   used func from lib/elf.c to avoid dependency form CLI
---
   boot/bootm_os.c  | 18 ++
   boot/image-fit.c |  3 ++-
   boot/image.c |  5 -
   cmd/Kconfig  |  7 +++
   include/image.h  |  1 +
   5 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/boot/bootm_os.c b/boot/bootm_os.c
index 15297ddb53..6a6621706f 100644
--- a/boot/bootm_os.c
+++ b/boot/bootm_os.c
@@ -8,6 +8,7 @@
   #include 
   #include 
   #include 
+#include 
   #include 
   #include 
   #include 
@@ -394,6 +395,20 @@ static int do_bootm_qnxelf(int flag, struct

bootm_info *bmi)

   }
   #endif

+#if defined(CONFIG_BOOTM_ELF)
+static int do_bootm_elf(int flag, struct bootm_info *bmi)
+{
+ Bootelf_flags flags = { .autostart = 1 };
+
+ if (flag != BOOTM_STATE_OS_GO)
+ return 0;
+
+ bootelf(bmi->images->ep, flags, 0, NULL);
+
+ return 1;
+}
+#endif
+
   #ifdef CONFIG_INTEGRITY
   static int do_bootm_integrity(int flag, struct bootm_info *bmi)
   {
@@ -535,6 +550,9 @@ static boot_os_fn *boot_os[] = {
   #ifdef CONFIG_BOOTM_EFI
   [IH_OS_EFI] = do_bootm_efi,
   #endif
+#if defined(CONFIG_BOOTM_ELF)
+ [IH_OS_ELF] = do_bootm_elf,
+#endif
   };

   /* Allow for arch specific config before we boot */
diff --git a/boot/image-fit.c b/boot/image-fit.c
index f6464bcf62..9253f81fff 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -2175,7 +2175,8 @@ int fit_image_load(struct bootm_headers *images,

ulong addr,

   fit_image_check_os(fit, noffset, IH_OS_TEE) ||
   fit_image_check_os(fit, noffset, IH_OS_OPENRTOS) ||
   fit_image_check_os(fit, noffset, IH_OS_EFI) ||
- fit_image_check_os(fit, noffset, IH_OS_VXWORKS);
+ fit_image_check_os(fit, noffset, IH_OS_VXWORKS) ||
+ fit_image_check_os(fit, noffset, IH_OS_ELF);

   /*
* If either of the checks fail, we should report an error, but
diff --git a/boot/image.c b/boot/image.c
index fc774d605d..abac254e02 100644
--- a/boot/image.c
+++ b/boot/image.c
@@ -130,7 +130,10 @@ static const table_entry_t uimage_os[] = {
   {   IH_OS_OPENRTOS, "openrtos", "OpenRTOS", },
   #endif
   {   IH_OS_OPENSBI,  "opensbi",  "RISC-V OpenSBI",   },
- {   IH_OS_EFI,  "efi",  "EFI Firmware" },
+ {   IH_OS_EFI,  "efi",  "EFI Firmware"  },
+#ifdef CONFIG_BOOTM_ELF
+ {   IH_OS_ELF,  "elf",  "ELF Image" },
+#endif

   {   -1, "", "", },
   };
diff --git a/cmd/Kconfig b/cmd/Kconfig
index a2dee34689..613baa106e 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -321,6 +321,13 @@ config BOOTM_EFI
   help
 Support booting UEFI FIT images via the bootm command.

+config BOOTM_ELF
+ bool "Support booting ELF images"
+ depends on CMD_BOOTM && LIB_ELF
+ default n
+ help
+   Support booting ELF images via the bootm command.
+
   config CMD_BOOTZ
   bool "bootz"
   help
diff --git a/include/image.h b/include/image.h
index c5b288f62b..9daaee15cd 100644
--- a/include/image.h
+++ b/include/image.h
@@ -100,6 +100,7 @@ enum {
   IH_OS_TEE,  /* Trusted Execution Environment */
   IH_OS_OPENSBI,  /* RISC-V OpenSBI */
   IH_OS_EFI,  /* EFI Firmware (e.g. GRUB2) */
+ IH_OS_ELF,  /* ELF Image (e.g. seL4) */

   IH_OS_COUNT,
   };







Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs

2024-06-21 Thread Heinrich Schuchardt

On 21.06.24 13:25, Ilias Apalodimas wrote:

On Fri, 21 Jun 2024 at 14:01, Ilias Apalodimas
 wrote:


Hi Vincent,

[...]


   $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
   encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
   SIV: 195894493536133784175416063449172723213
   decode: variant: reserved (Microsoft GUID)
   version: 4 (random data based)
   content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
(no semantics: random data only)

A reserved Microsoft GUID variant does not look right.


This seems like an existing bug. RFC4122 defines the MS reserved GUIDs
in the variant as
1 1 0Reserved, Microsoft Corporation backward
compatibility and the existing UUID_VARIANT_MASK is defined as 0xc0...


I think the variant mask 0xc0 is correct:

- The variant field is in the top three bits of the "clock seq high and
   reserved" byte, but...
- The variant we want is 1 0 x (do not care for bit 5, a.k.a. "Msb2").

With the mask 0xc0 we can clear the top two bits as we set the top most bit just
after anyway.

...the mask needs to be used correctly, though; see below.


Ah yes, the current code is using it in clrsetbits_8, which inverts it
internally, so it's indeed correct.





The patch below should work for you (on top of Calebs')

diff --git a/include/uuid.h b/include/uuid.h
index b38b20d957ef..78ed5839d2d6 100644
--- a/include/uuid.h
+++ b/include/uuid.h
@@ -81,7 +81,7 @@ struct uuid {
  #define UUID_VERSION_SHIFT 12
  #define UUID_VERSION   0x4

-#define UUID_VARIANT_MASK  0xc0
+#define UUID_VARIANT_MASK  0xb0
  #define UUID_VARIANT_SHIFT 7
  #define UUID_VARIANT   0x1

diff --git a/lib/uuid.c b/lib/uuid.c
index 89911b06ccc0..73251eaa397e 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -411,9 +411,9 @@ void gen_uuid_v5(const struct uuid *namespace,
struct uuid *uuid, ...)
 memcpy(uuid, hash, sizeof(*uuid));

 /* Configure variant/version bits */
-   tmp = be32_to_cpu(uuid->time_hi_and_version);
+   tmp = uuid->time_hi_and_version;


Not caring for the endianness at all does not look right.
Indeed, while this does work on my side in little-endian, this does not work on
on big-endian simulators.


Yes, we need the conversions


Also, this is a 16b quantity, not 32b; we need to redefine tmp to uint16_t and
use the be16 functions.



Yep I already pointed that out earlier.


 tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
-   uuid->time_hi_and_version = cpu_to_be32(tmp);
+   uuid->time_hi_and_version = tmp;

 uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;


We need to mask with ~UUID_VARIANT_MASK, I think.


 uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;

Can you give it a shot?


This does indeed work on my little-endian machines, but not on big-endian
simulators.
For testing on big-endian, I suggest using only genguid as the sandbox will not
help there:

   $ make sandbox_defconfig
   $ make tools-only
   $ ./tools/genguid 2a5aa852-b856-4d97-baa9-5c5f4421551f \
 -c "qcom,qrb4210-rb2" \
 QUALCOMM-UBOOT

...and feed the resulting UUID to `uuid -d'.
(The genguid command is the online help example.)


What I am afraid of is breaking existing use cases using a different
variant mask
If that's the case we might need to keep the buggy existing
UUID_VARIANT_MASK and use the new one only on v5 and newer code


I tried to debug further and I suspect that:

- Operations on 8b clock_seq_hi_and_reserved might need further casts.

- My understanding is that we are generating the v5 UUID as big-endian in
   memory; if this is indeed the case, genguid should not print it with the GUID
   byte order.


RFC4122 says that
"put name space ID in network byte order so it hashes the same no
matter what endian machine we're on "
the EFI spec says
"It should also be noted that TimeLow, TimeMid, TimeHighAndVersion
fields in the EFI are encoded as little endian. The following table
defines the format of an EFI GUID (128 bits)."

Which is lovely



But this brings up something that Heinrich also mentioned. Since the
efi spec and RFC4122 interpret the endianess differently, how do you
expect uuid -d to work?


The binary format does depend on endianness:

$ echo -n -e
"\\xF8\\x1D\\x4F\\xAE\\x7D\\xEC\\x51\\xD0\\xA7\\x65\\x00\\xA0\\xC9\\x1E\\x6B\\xF6"
\
> | uuid -d -F BIN -
encode: STR: f81d4fae-7dec-51d0-a765-00a0c91e6bf6
SIV: 329800735698586931527096882168799849462
decode: variant: DCE 1.1, ISO/IEC 11578:1996
version: 5 (name based, SHA-1)
content: F8:1D:4F:AE:7D:EC:01:D0:27:65:00:A0:C9:1E:6B:F6
 (not decipherable: truncated SHA-1 message digest only)

Best regards

Heinrich



Thanks
/Ilias

I'll send a patch with the changes

Regards
/Ilias


For the moment I am unable to make the code work in all the following cases:

- genguid on little-endian
- genguid on big-endian
- 

Re: [PATCH v5] cmd: bootm: add ELF file support

2024-06-21 Thread Heinrich Schuchardt

On 21.06.24 13:42, Maxim Moskalets wrote:

Some operating systems (e.g. seL4) and embedded applications are ELF
images. It is convenient to use FIT-images to implement trusted boot.
Added "elf" image type for booting using bootm command.

Signed-off-by: Maxim Moskalets 


I guess a test could be added to test/py/tests/test_zynqmp_rpu.py which
already uses the bootelf command?

Best regards

Heinrich


---
v5:
used func from lib/elf.c to avoid dependency form CLI
---
  boot/bootm_os.c  | 18 ++
  boot/image-fit.c |  3 ++-
  boot/image.c |  5 -
  cmd/Kconfig  |  7 +++
  include/image.h  |  1 +
  5 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/boot/bootm_os.c b/boot/bootm_os.c
index 15297ddb53..6a6621706f 100644
--- a/boot/bootm_os.c
+++ b/boot/bootm_os.c
@@ -8,6 +8,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -394,6 +395,20 @@ static int do_bootm_qnxelf(int flag, struct bootm_info 
*bmi)
  }
  #endif

+#if defined(CONFIG_BOOTM_ELF)
+static int do_bootm_elf(int flag, struct bootm_info *bmi)
+{
+   Bootelf_flags flags = { .autostart = 1 };
+
+   if (flag != BOOTM_STATE_OS_GO)
+   return 0;
+
+   bootelf(bmi->images->ep, flags, 0, NULL);
+
+   return 1;
+}
+#endif
+
  #ifdef CONFIG_INTEGRITY
  static int do_bootm_integrity(int flag, struct bootm_info *bmi)
  {
@@ -535,6 +550,9 @@ static boot_os_fn *boot_os[] = {
  #ifdef CONFIG_BOOTM_EFI
[IH_OS_EFI] = do_bootm_efi,
  #endif
+#if defined(CONFIG_BOOTM_ELF)
+   [IH_OS_ELF] = do_bootm_elf,
+#endif
  };

  /* Allow for arch specific config before we boot */
diff --git a/boot/image-fit.c b/boot/image-fit.c
index f6464bcf62..9253f81fff 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -2175,7 +2175,8 @@ int fit_image_load(struct bootm_headers *images, ulong 
addr,
fit_image_check_os(fit, noffset, IH_OS_TEE) ||
fit_image_check_os(fit, noffset, IH_OS_OPENRTOS) ||
fit_image_check_os(fit, noffset, IH_OS_EFI) ||
-   fit_image_check_os(fit, noffset, IH_OS_VXWORKS);
+   fit_image_check_os(fit, noffset, IH_OS_VXWORKS) ||
+   fit_image_check_os(fit, noffset, IH_OS_ELF);

/*
 * If either of the checks fail, we should report an error, but
diff --git a/boot/image.c b/boot/image.c
index fc774d605d..abac254e02 100644
--- a/boot/image.c
+++ b/boot/image.c
@@ -130,7 +130,10 @@ static const table_entry_t uimage_os[] = {
{   IH_OS_OPENRTOS, "openrtos",   "OpenRTOS",   },
  #endif
{   IH_OS_OPENSBI,  "opensbi","RISC-V OpenSBI", },
-   {   IH_OS_EFI,  "efi","EFI Firmware" },
+   {   IH_OS_EFI,  "efi","EFI Firmware"
},
+#ifdef CONFIG_BOOTM_ELF
+   {   IH_OS_ELF,  "elf","ELF Image"   },
+#endif

{   -1, "",   "",   },
  };
diff --git a/cmd/Kconfig b/cmd/Kconfig
index a2dee34689..613baa106e 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -321,6 +321,13 @@ config BOOTM_EFI
help
  Support booting UEFI FIT images via the bootm command.

+config BOOTM_ELF
+   bool "Support booting ELF images"
+   depends on CMD_BOOTM && LIB_ELF
+   default n
+   help
+ Support booting ELF images via the bootm command.
+
  config CMD_BOOTZ
bool "bootz"
help
diff --git a/include/image.h b/include/image.h
index c5b288f62b..9daaee15cd 100644
--- a/include/image.h
+++ b/include/image.h
@@ -100,6 +100,7 @@ enum {
IH_OS_TEE,  /* Trusted Execution Environment */
IH_OS_OPENSBI,  /* RISC-V OpenSBI */
IH_OS_EFI,  /* EFI Firmware (e.g. GRUB2) */
+   IH_OS_ELF,  /* ELF Image (e.g. seL4) */

IH_OS_COUNT,
  };




Re: [PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs

2024-06-21 Thread Heinrich Schuchardt

On 21.06.24 11:12, Vincent Stehlé wrote:

On Wed, Jun 19, 2024 at 10:15:32PM +0300, Ilias Apalodimas wrote:

Allô Vincent,


Hi Ilias :)



Thanks for testing!

On Wed, 19 Jun 2024 at 17:02, Vincent Stehlé  wrote:


On Fri, May 31, 2024 at 03:50:34PM +0200, Caleb Connolly wrote:

As more boards adopt support for the EFI CapsuleUpdate mechanism, there
is a growing issue of being able to target updates to them properly. The
current mechanism of hardcoding UUIDs for each board at compile time is
unsustainable, and maintaining lists of GUIDs is similarly cumbersome.

In this series, I propose that we adopt v5 GUIDs, these are generated
by using a well-known salt GUID as well as board specific information
(like the model/revision), these are hashed together and the result is
truncated to form a new UUID.


Dear Caleb,

Thank you for working on this proposal, this looks very useful.
Indeed, we found out during SystemReady certifications that it is easy for
unique ids to be inadvertently re-used, making them thus non-unique.

I have a doubt regarding the format of the generated UUIDs, which end up in the
ESRT, though.

Here is a quick experiment on the sandbox booting with a DTB using the efidebug
command.

With the patch series, rebased on the master branch:

   $ make sandbox_defconfig
   $ make
   $ ./u-boot --default_fdt
   ...
   U-Boot 2024.07-rc4-00028-g1c96225b0b3 (Jun 19 2024 - 15:29:04 +0200)
   ...
   Model: sandbox
   ...
   Hit any key to stop autoboot:  0
   => efidebug capsule esrt
   ...
   
   ESRT: fw_resource_count=2
   ESRT: fw_resource_count_max=2
   ESRT: fw_resource_version=1
   [entry 0]==
   ESRT: fw_class=FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
   ...
   [entry 1]==
   ESRT: fw_class=935FE837-FAC8-4394-C008-737D8852C60D
   ...
   

   $ uuid -d FD5DB83C-12F3-A46B-80A9-E3007C7FF56E
   encode: STR: fd5db83c-12f3-a46b-80a9-e3007c7ff56e
   SIV: 336781303264349553179461347850802165102
   decode: variant: DCE 1.1, ISO/IEC 11578:1996
   version: 10 (unknown)
   content: FD:5D:B8:3C:12:F3:04:6B:00:A9:E3:00:7C:7F:F5:6E
(not decipherable: unknown UUID version)

Version 10 does not look right.


So, this seems to be an endianess problem.
Looking at RFC4122 only the space ID needs to be in BE.



   $ uuid -d 935FE837-FAC8-4394-C008-737D8852C60D
   encode: STR: 935fe837-fac8-4394-c008-737d8852c60d
   SIV: 195894493536133784175416063449172723213
   decode: variant: reserved (Microsoft GUID)
   version: 4 (random data based)
   content: 93:5F:E8:37:FA:C8:03:94:00:08:73:7D:88:52:C6:0D
(no semantics: random data only)

A reserved Microsoft GUID variant does not look right.


This seems like an existing bug. RFC4122 defines the MS reserved GUIDs
in the variant as
1 1 0Reserved, Microsoft Corporation backward
compatibility and the existing UUID_VARIANT_MASK is defined as 0xc0...


I think the variant mask 0xc0 is correct:

- The variant field is in the top three bits of the "clock seq high and
   reserved" byte, but...
- The variant we want is 1 0 x (do not care for bit 5, a.k.a. "Msb2").

With the mask 0xc0 we can clear the top two bits as we set the top most bit just
after anyway.

...the mask needs to be used correctly, though; see below.



The patch below should work for you (on top of Calebs')

diff --git a/include/uuid.h b/include/uuid.h
index b38b20d957ef..78ed5839d2d6 100644
--- a/include/uuid.h
+++ b/include/uuid.h
@@ -81,7 +81,7 @@ struct uuid {
  #define UUID_VERSION_SHIFT 12
  #define UUID_VERSION   0x4

-#define UUID_VARIANT_MASK  0xc0
+#define UUID_VARIANT_MASK  0xb0
  #define UUID_VARIANT_SHIFT 7
  #define UUID_VARIANT   0x1

diff --git a/lib/uuid.c b/lib/uuid.c
index 89911b06ccc0..73251eaa397e 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -411,9 +411,9 @@ void gen_uuid_v5(const struct uuid *namespace,
struct uuid *uuid, ...)
 memcpy(uuid, hash, sizeof(*uuid));

 /* Configure variant/version bits */
-   tmp = be32_to_cpu(uuid->time_hi_and_version);
+   tmp = uuid->time_hi_and_version;


Not caring for the endianness at all does not look right.
Indeed, while this does work on my side in little-endian, this does not work on
on big-endian simulators.
Also, this is a 16b quantity, not 32b; we need to redefine tmp to uint16_t and
use the be16 functions.


 tmp = (tmp & ~UUID_VERSION_MASK) | (5 << UUID_VERSION_SHIFT);
-   uuid->time_hi_and_version = cpu_to_be32(tmp);
+   uuid->time_hi_and_version = tmp;

 uuid->clock_seq_hi_and_reserved &= UUID_VARIANT_MASK;


We need to mask with ~UUID_VARIANT_MASK, I think.


 uuid->clock_seq_hi_and_reserved |= UUID_VARIANT << UUID_VARIANT_SHIFT;

Can you give it a shot?


This does indeed work on my little-endian machines, but 

Re: [PATCH v4 9/9] doc: convert README.LED to .rst documentation

2024-06-20 Thread Heinrich Schuchardt

On 6/20/24 06:37, Christian Marangi wrote:

On Thu, Jun 20, 2024 at 08:13:34AM +0200, Heinrich Schuchardt wrote:

On 6/20/24 01:03, Christian Marangi wrote:

Convert README.LED to .rst documentation and include all the relevant
documentation in the status_led.h.

Signed-off-by: Christian Marangi 
---
   doc/README.LED |  77 --
   doc/api/index.rst  |   1 +
   doc/api/status_led.rst |  35 +++
   include/status_led.h   | 224 -
   4 files changed, 256 insertions(+), 81 deletions(-)
   delete mode 100644 doc/README.LED
   create mode 100644 doc/api/status_led.rst

diff --git a/doc/README.LED b/doc/README.LED
deleted file mode 100644
index c21c9d53ec3..000
--- a/doc/README.LED
+++ /dev/null
@@ -1,77 +0,0 @@
-Status LED
-
-
-This README describes the status LED API.
-
-The API is defined by the include file include/status_led.h
-
-The first step is to enable CONFIG_LED_STATUS in menuconfig:
-> Device Drivers > LED Support.
-
-If the LED support is only for specific board, enable
-CONFIG_LED_STATUS_BOARD_SPECIFIC in the menuconfig.
-
-Status LEDS 0 to 5 are enabled by the following configurations at menuconfig:
-CONFIG_STATUS_LED0, CONFIG_STATUS_LED1, ... CONFIG_STATUS_LED5
-
-The following should be configured for each of the enabled LEDs:
-CONFIG_STATUS_LED_BIT
-CONFIG_STATUS_LED_STATE
-CONFIG_STATUS_LED_FREQ
-Where  is an integer 1 through 5 (empty for 0).
-
-CONFIG_STATUS_LED_BIT is passed into the __led_* functions to identify which 
LED
-is being acted on. As such, the value choose must be unique with with respect 
to
-the other CONFIG_STATUS_LED_BIT's. Mapping the value to a physical LED is the
-reponsiblity of the __led_* function.
-
-CONFIG_STATUS_LED_STATE is the initial state of the LED. It should be set to 
one
-of these values: CONFIG_LED_STATUS_OFF or CONFIG_LED_STATUS_ON.
-
-CONFIG_STATUS_LED_FREQ determines the LED blink frequency.
-Values range from 2 to 10.
-
-Some other LED macros
--
-
-CONFIG_STATUS_LED_BOOT is the LED to light when the board is booting.
-This must be a valid LED number (0-5).
-
-CONFIG_STATUS_LED_RED is the red LED. It is used to signal errors. This must be
-a valid LED number (0-5). Other similar color LED's macros are
-CONFIG_STATUS_LED_GREEN, CONFIG_STATUS_LED_YELLOW and CONFIG_STATUS_LED_BLUE.
-
-General LED functions
--
-The following functions should be defined:
-
-__led_init is called once to initialize the LED to CONFIG_STATUS_LED_STATE.
-One time start up code should be placed here.
-
-__led_set is called to change the state of the LED.
-
-__led_toggle is called to toggle the current state of the LED.
-
-Colour LED
-
-
-Colour LED's are at present only used by ARM.
-
-The functions names explain their purpose.
-
-coloured_LED_init
-red_LED_on
-red_LED_off
-green_LED_on
-green_LED_off
-yellow_LED_on
-yellow_LED_off
-blue_LED_on
-blue_LED_off
-
-These are weakly defined in arch/arm/lib/board.c to noops. Where applicable, 
define
-these functions in the board specific source.
-
-TBD : Describe older board dependent macros similar to what is done for
-
-TBD : Describe general support via asm/status_led.h
diff --git a/doc/api/index.rst b/doc/api/index.rst
index 51b2013af36..d40e90801d1 100644
--- a/doc/api/index.rst
+++ b/doc/api/index.rst
@@ -22,6 +22,7 @@ U-Boot API documentation
  rng
  sandbox
  serial
+   status_led
  sysreset
  timer
  unicode
diff --git a/doc/api/status_led.rst b/doc/api/status_led.rst
new file mode 100644
index 000..44bbea47157
--- /dev/null
+++ b/doc/api/status_led.rst
@@ -0,0 +1,35 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Status LED
+==
+
+.. kernel-doc:: include/status_led.h
+   :doc: Overview
+
+CONFIG_STATUS_LED Description
+-
+
+.. kernel-doc:: include/status_led.h
+   :doc: CONFIG Description
+
+Special Status LED Configs
+--
+.. kernel-doc:: include/status_led.h
+   :doc: LED Status Config
+
+Colour Status LED Configs
+-
+.. kernel-doc:: include/status_led.h
+   :doc: LED Colour Config
+
+Required API
+
+
+.. kernel-doc:: include/status_led.h
+   :doc: Required API
+
+Status LED API
+--
+
+.. kernel-doc:: include/status_led.h
+   :internal:
diff --git a/include/status_led.h b/include/status_led.h
index 7de40551621..6893d1d68e0 100644
--- a/include/status_led.h
+++ b/include/status_led.h
@@ -4,18 +4,102 @@
* Wolfgang Denk, DENX Software Engineering, w...@denx.de.
*/

-/*
- * The purpose of this code is to signal the operational status of a
+/**
+ * DOC: Overview
+ *
+ * Status LED is a Low-Level way to handle LEDs to signal state of the


Status LEDs can be used to signal the operational status of a


+ * bootloader, for example boot progress, file transfer fail, activity
+ * of some sort lik

Re: [PATCH] efi_loader: adjust config options for capsule updates

2024-06-20 Thread Heinrich Schuchardt

On 18.06.24 17:49, Ilias Apalodimas wrote:

EFI_IGNORE_OSINDICATIONS is used to ignore OsIndications if setvariable
at runtime is not supported and allow the platform to perform capsule
updates on disk. With the recent changes boards can conditionally enable
setvariable at runtime using EFI_RT_VOLATILE_STORE.

So let's make the options depend on each other and clarify their
functionality. When EFI_RT_VOLATILE_STORE, setvariable at runtime is
supported and EFI_IGNORE_OSINDICATIONS, which also breaks the EFI spec, is
not needed anymore.

Signed-off-by: Ilias Apalodimas 
---
  lib/efi_loader/Kconfig | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 430bb7f0f7dc..c84064de1366 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -220,6 +220,8 @@ config EFI_CAPSULE_ON_DISK
  config EFI_IGNORE_OSINDICATIONS
bool "Ignore OsIndications for CapsuleUpdate on-disk"
depends on EFI_CAPSULE_ON_DISK
+   depends on !EFI_RT_VOLATILE_STORE


EFI_RT_VOLATILE_STORE does not imply that the OS that you are running
supports writing variables to ubootefi.var or eMMC.

How about

default y if !EFI_RT_VOLATILE_STORE

Best regards

Heinrich


+   default y
help
  There are boards where U-Boot does not support SetVariable at runtime.
  Select this option if you want to use the capsule-on-disk feature




Re: [PATCH 3/5] buildman: Support building within a Python venv

2024-06-20 Thread Heinrich Schuchardt

On 20.06.24 15:19, Simon Glass wrote:

The Python virtualenv tool sets up a few things in the envronment,


Nits

%s/envronment/environment/


putting its path first in the PATH environment variable and setting up
a sys.prefix different from the sys.base_prefix value.

At present buildman puts the toolchain path first in PATH so that it can
be found easily during the build. For sandbox this causes problems since
/usr/bin/gcc (for example) results in '/usr/bin' being prepended to the
PATH variable. As a result, the venv is partially disabled.


If you want to remember a PATH, why don't you use a differnet variable
like U_BOOT_TOOLPATH to convey this information instead of manipulating
the PATH variable?



The result is that sandbox builds within a venv ignore the venv, e.g.
when looking for packages.

Correct this by detecting the venv and adding the toolchain path after
the venv path.

Signed-off-by: Simon Glass 
---

  tools/buildman/bsettings.py |  3 ++
  tools/buildman/test.py  | 83 +
  tools/buildman/toolchain.py | 31 --
  3 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py
index e225ac2ca0f..1be1d45e0fa 100644
--- a/tools/buildman/bsettings.py
+++ b/tools/buildman/bsettings.py
@@ -31,6 +31,9 @@ def setup(fname=''):
  def add_file(data):
  settings.readfp(io.StringIO(data))

+def add_section(name):
+settings.add_section(name)
+
  def get_items(section):
  """Get the items from a section of the config.

diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index f92add7a7c5..83219182fe0 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -146,6 +146,7 @@ class TestBuild(unittest.TestCase):
  self.toolchains.Add('arm-linux-gcc', test=False)
  self.toolchains.Add('sparc-linux-gcc', test=False)
  self.toolchains.Add('powerpc-linux-gcc', test=False)
+self.toolchains.Add('/path/to/aarch64-linux-gcc', test=False)
  self.toolchains.Add('gcc', test=False)

  # Avoid sending any output
@@ -747,6 +748,88 @@ class TestBuild(unittest.TestCase):
  self.assertEqual([
  ['MARY="mary"', 'Missing expected line: CONFIG_MARY="mary"']], 
result)

+def call_make_environment(self, tchn, full_path, in_env=None):
+"""Call Toolchain.MakeEnvironment() and process the result
+
+Args:
+tchn (Toolchain): Toolchain to use
+full_path (bool): True to return the full path in CROSS_COMPILE
+rather than adding it to the PATH variable
+in_env (dict): Input environment to use, None to use current env
+
+Returns:
+tuple:
+dict: Changes that MakeEnvironment has made to the environment
+key: Environment variable that was changed
+value: New value (for PATH this only includes components
+which were added)
+str: Full value of the new PATH variable
+"""
+env = tchn.MakeEnvironment(full_path, env=in_env)
+
+# Get the original environment
+orig_env = dict(os.environb if in_env is None else in_env)
+orig_path = orig_env[b'PATH'].split(b':')
+
+# Find new variables
+diff = dict((k, env[k]) for k in env if orig_env.get(k) != env[k])
+
+# Find new / different path components
+diff_path = None
+new_path = None
+if b'PATH' in diff:
+new_path = diff[b'PATH'].split(b':')
+diff_paths = [p for p in new_path if p not in orig_path]
+diff_path = b':'.join(p for p in new_path if p not in orig_path)
+if diff_path:
+diff[b'PATH'] = diff_path
+else:
+del diff[b'PATH']
+return diff, new_path
+
+def test_toolchain_env(self):
+"""Test PATH and other environment settings for toolchains"""
+# Use a toolchain which has a path, so that full_path makes a 
difference
+tchn = self.toolchains.Select('aarch64')
+
+# Normal cases
+diff = self.call_make_environment(tchn, full_path=False)[0]
+self.assertEqual(
+{b'CROSS_COMPILE': b'aarch64-linux-', b'LC_ALL': b'C',
+ b'PATH': b'/path/to'}, diff)
+
+diff = self.call_make_environment(tchn, full_path=True)[0]
+self.assertEqual(
+{b'CROSS_COMPILE': b'/path/to/aarch64-linux-', b'LC_ALL': b'C'},
+diff)
+
+# When overriding the toolchain, only LC_ALL should be set
+tchn.override_toolchain = True
+diff = self.call_make_environment(tchn, full_path=True)[0]
+self.assertEqual({b'LC_ALL': b'C'}, diff)
+
+# Test that virtualenv is handled correctly
+tchn.override_toolchain = False
+sys.prefix = '/some/venv'
+env = dict(os.environb)
+env[b'PATH'] = 

Re: [PATCH v4 0/9] misc: introduce STATUS LED activity function

2024-06-20 Thread Heinrich Schuchardt

On 6/20/24 01:03, Christian Marangi wrote:

This series expand the STATUS LED framework with a new color
and a big new feature. One thing that many device need is a way
to communicate to the user that the device is actually doing
something.

This is especially useful for recovery steps where an
user (for example) insert an USB drive, keep a button pressed
and the device autorecover.

There is currently no way to signal the user externally that
the bootloader is processing/recoverying aside from setting
a LED on.

A solid LED on is not enough and won't actually signal any
kind of progress.
Solution is the good old blinking LED but uboot doesn't
suggest (and support) interrupts and almost all the LED
are usually GPIO LED that doesn't support HW blink.

To fix this and handle the problem of device not supporting
HW blink, expand the STATUS LED framework with new API.

We introduce a new config LED_STATUS_ACTIVITY, that similar
to the RED, GREEN and others, takes the LED ID set in
the LED_STATUS config and is used as the global LED for activity
operations.

We add status_led_activity_start/stop() used to turn the
activity LED on and register a cyclic to make it blink.
LED will continue to blink until _stop() is called.

Function that will start some kind of action will first call
_start() and then at the end will call stop().
If (on the broken implementation) a _start() is called before
calling a _stop(), the Cyclic is first unregister and is
registered again.

Support for this is initially added to the tftp, mtd and ubi
command.

This also contains a big fixup for the gpio_led driver that
currently deviates from the Documentation and make the
coloured status led feature unusable.

(world tested with the azure pipeline)


Hello Christian,

What is the relationship between CONFIG_LED and CONFIG_LED_STATUS?
Can they be used at the same time or should they be exclusive?
Should CONFIG_LED_STATUS depend on CONFIG_LED?

This should be clarified both in Kconfig and in the documentation.

At least cmd/led.c and cmd/legacy_led.c are conflicting as both provide
a command 'led'.

Can we unify the too?

Best regards

Heinrich



Changes v4:
- Rebase on top of next
- Rework for cyclic changes on next
- Fix compilation error
- Fix compilation warning due to missing header
Changes v3:
- Add log on Status LED error conditions
Changes v2:
- Add Documentation conversion
- Rework to Cyclic and simplify implementation

Christian Marangi (9):
   misc: gpio_led: fix broken coloured LED status functions
   led: status_led: add support for white LED colour
   led: status_led: add function to toggle a status LED
   led: status_led: add warning when an invalid Status LED ID is used
   led: status_led: add new activity LED config and functions
   tftp: implement support for LED status activity
   mtd: implement support for LED status activity
   ubi: implement support for LED status activity
   doc: convert README.LED to .rst documentation

  cmd/legacy_led.c  |   6 +
  cmd/mtd.c |  19 
  cmd/ubi.c |  15 ++-
  common/board_f.c  |   2 +
  doc/README.LED|  77 -
  doc/api/index.rst |   1 +
  doc/api/status_led.rst|  35 ++
  drivers/led/Kconfig   |  30 +
  drivers/misc/gpio_led.c   |  41 +--
  drivers/misc/status_led.c |  75 -
  include/status_led.h  | 231 +-
  net/tftp.c|   7 ++
  12 files changed, 440 insertions(+), 99 deletions(-)
  delete mode 100644 doc/README.LED
  create mode 100644 doc/api/status_led.rst





Re: [PATCH v4 9/9] doc: convert README.LED to .rst documentation

2024-06-20 Thread Heinrich Schuchardt

On 6/20/24 01:03, Christian Marangi wrote:

Convert README.LED to .rst documentation and include all the relevant
documentation in the status_led.h.

Signed-off-by: Christian Marangi 
---
  doc/README.LED |  77 --
  doc/api/index.rst  |   1 +
  doc/api/status_led.rst |  35 +++
  include/status_led.h   | 224 -
  4 files changed, 256 insertions(+), 81 deletions(-)
  delete mode 100644 doc/README.LED
  create mode 100644 doc/api/status_led.rst

diff --git a/doc/README.LED b/doc/README.LED
deleted file mode 100644
index c21c9d53ec3..000
--- a/doc/README.LED
+++ /dev/null
@@ -1,77 +0,0 @@
-Status LED
-
-
-This README describes the status LED API.
-
-The API is defined by the include file include/status_led.h
-
-The first step is to enable CONFIG_LED_STATUS in menuconfig:
-> Device Drivers > LED Support.
-
-If the LED support is only for specific board, enable
-CONFIG_LED_STATUS_BOARD_SPECIFIC in the menuconfig.
-
-Status LEDS 0 to 5 are enabled by the following configurations at menuconfig:
-CONFIG_STATUS_LED0, CONFIG_STATUS_LED1, ... CONFIG_STATUS_LED5
-
-The following should be configured for each of the enabled LEDs:
-CONFIG_STATUS_LED_BIT
-CONFIG_STATUS_LED_STATE
-CONFIG_STATUS_LED_FREQ
-Where  is an integer 1 through 5 (empty for 0).
-
-CONFIG_STATUS_LED_BIT is passed into the __led_* functions to identify which 
LED
-is being acted on. As such, the value choose must be unique with with respect 
to
-the other CONFIG_STATUS_LED_BIT's. Mapping the value to a physical LED is the
-reponsiblity of the __led_* function.
-
-CONFIG_STATUS_LED_STATE is the initial state of the LED. It should be set to 
one
-of these values: CONFIG_LED_STATUS_OFF or CONFIG_LED_STATUS_ON.
-
-CONFIG_STATUS_LED_FREQ determines the LED blink frequency.
-Values range from 2 to 10.
-
-Some other LED macros
--
-
-CONFIG_STATUS_LED_BOOT is the LED to light when the board is booting.
-This must be a valid LED number (0-5).
-
-CONFIG_STATUS_LED_RED is the red LED. It is used to signal errors. This must be
-a valid LED number (0-5). Other similar color LED's macros are
-CONFIG_STATUS_LED_GREEN, CONFIG_STATUS_LED_YELLOW and CONFIG_STATUS_LED_BLUE.
-
-General LED functions
--
-The following functions should be defined:
-
-__led_init is called once to initialize the LED to CONFIG_STATUS_LED_STATE.
-One time start up code should be placed here.
-
-__led_set is called to change the state of the LED.
-
-__led_toggle is called to toggle the current state of the LED.
-
-Colour LED
-
-
-Colour LED's are at present only used by ARM.
-
-The functions names explain their purpose.
-
-coloured_LED_init
-red_LED_on
-red_LED_off
-green_LED_on
-green_LED_off
-yellow_LED_on
-yellow_LED_off
-blue_LED_on
-blue_LED_off
-
-These are weakly defined in arch/arm/lib/board.c to noops. Where applicable, 
define
-these functions in the board specific source.
-
-TBD : Describe older board dependent macros similar to what is done for
-
-TBD : Describe general support via asm/status_led.h
diff --git a/doc/api/index.rst b/doc/api/index.rst
index 51b2013af36..d40e90801d1 100644
--- a/doc/api/index.rst
+++ b/doc/api/index.rst
@@ -22,6 +22,7 @@ U-Boot API documentation
 rng
 sandbox
 serial
+   status_led
 sysreset
 timer
 unicode
diff --git a/doc/api/status_led.rst b/doc/api/status_led.rst
new file mode 100644
index 000..44bbea47157
--- /dev/null
+++ b/doc/api/status_led.rst
@@ -0,0 +1,35 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Status LED
+==
+
+.. kernel-doc:: include/status_led.h
+   :doc: Overview
+
+CONFIG_STATUS_LED Description
+-
+
+.. kernel-doc:: include/status_led.h
+   :doc: CONFIG Description
+
+Special Status LED Configs
+--
+.. kernel-doc:: include/status_led.h
+   :doc: LED Status Config
+
+Colour Status LED Configs
+-
+.. kernel-doc:: include/status_led.h
+   :doc: LED Colour Config
+
+Required API
+
+
+.. kernel-doc:: include/status_led.h
+   :doc: Required API
+
+Status LED API
+--
+
+.. kernel-doc:: include/status_led.h
+   :internal:
diff --git a/include/status_led.h b/include/status_led.h
index 7de40551621..6893d1d68e0 100644
--- a/include/status_led.h
+++ b/include/status_led.h
@@ -4,18 +4,102 @@
   * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
   */

-/*
- * The purpose of this code is to signal the operational status of a
+/**
+ * DOC: Overview
+ *
+ * Status LED is a Low-Level way to handle LEDs to signal state of the


Status LEDs can be used to signal the operational status of a


+ * bootloader, for example boot progress, file transfer fail, activity
+ * of some sort like tftp transfer, mtd write/erase.


for example boot progress, file transfer failure, or ongoing activity
like tftp transfer or mtd 

[PATCH 1/1] riscv: semihosting: correct alignment

2024-06-19 Thread Heinrich Schuchardt
Commit 7400d34ba992 ("riscv: semihosting: replace inline assembly with
assembly file") reduced the alignment of function smh_trap().

As described in the "RISC-V Semihosting" specification [1] the ssli,
ebreak, and srai statements must all reside in the same memory page.

[1] RISC-V Semihosting, Version 0.4, 12th June 2024
https://github.com/riscv-non-isa/riscv-semihosting

Fixes: 7400d34ba992 ("riscv: semihosting: replace inline assembly with assembly 
file")
Signed-off-by: Heinrich Schuchardt 
---
 arch/riscv/lib/semihosting.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/lib/semihosting.S b/arch/riscv/lib/semihosting.S
index c0c571bce9b..49bb419a962 100644
--- a/arch/riscv/lib/semihosting.S
+++ b/arch/riscv/lib/semihosting.S
@@ -8,7 +8,7 @@
 
 .pushsection .text.smh_trap, "ax"
 ENTRY(smh_trap)
-   .align  2
+   .align  4   /* keep slli, ebreak, srai in same page */
.option push
.option norvc   /* semihosting sequence must be 32-bit wide */
 
-- 
2.45.1



Re: [PATCH 1/3] cmd: avoid duplicate weak flush_dcache_all()

2024-06-19 Thread Heinrich Schuchardt

On 19.06.24 15:19, Ilias Apalodimas wrote:

On Wed, 19 Jun 2024 at 16:05, Ilias Apalodimas
 wrote:


On Wed, 19 Jun 2024 at 15:36, Heinrich Schuchardt
 wrote:


On 19.06.24 14:23, Ilias Apalodimas wrote:

On Sun, 16 Jun 2024 at 20:31, Heinrich Schuchardt
 wrote:


If we have multiple weak implementations of functions, the linker might
choose any of these. ARM and RISC-V already provide a weak implementation
of flush_dcache_all().

Signed-off-by: Heinrich Schuchardt 
---
   cmd/cache.c | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/cmd/cache.c b/cmd/cache.c
index 0254ff17f9b..16fa0f7c652 100644
--- a/cmd/cache.c
+++ b/cmd/cache.c
@@ -52,11 +52,14 @@ static int do_icache(struct cmd_tbl *cmdtp, int flag, int 
argc,
  return 0;
   }

+/* ARM and RISC-V define a weak flush_dcache_all() themselves. */
+#if !defined(CONFIG_ARM) && !defined(CONFIG_RISCV)
   void __weak flush_dcache_all(void)
   {
  puts("No arch specific flush_dcache_all available!\n");
  /* please define arch specific flush_dcache_all */
   }


Aren't we supposed to add a single __weak function so the linker can
replace it? IOW why is the declaration for Arm/riscv a weak one?


Some sub-architectures override the architecture specific weak
implementation, e.g.

arch/riscv/cpu/andes/cache.c:44:
void flush_dcache_all(void)

arch/arm/cpu/arm926ejs/cache.c:17:
void flush_dcache_all(void)


Ok, this does fix a problem, but afaict this is a band-aid and the
cache management is a mess overall -- e.g invalidate_icache_all() will
suffer from the same issue if a sub-architecture decides to define its
own in the future.

Would it be less bad to define
static inline __weak flush_dcache_all(void)
{
}
static inline __weak flush_icache_all(void)
{
}


ugh, this but without the inline!

Thanks
/Ilias


GCC does not allow both a weak and a strong implementation in the same 
module:


  CC  board/sandbox/sandbox.o
arch/sandbox/cpu/cache.c:15:6: error: redefinition of 
‘invalidate_icache_all’

   15 | void invalidate_icache_all(void)
  |  ^
In file included from arch/sandbox/cpu/cache.c:6:
include/cpu_func.h:74:13: note: previous definition of 
‘invalidate_icache_all’ with type ‘void(void)’

   74 | void __weak invalidate_icache_all(void)
  | ^

Best regards

Heinrich


in include/cpu_func.h instead of a random cmd file?
We can only define those if !CONFIG_ARM && !!CONFIG_RISCV and add a
comment on why.
It's kind of putting lipstick on a pig, but at least we'll have them
gathered in a single header file and know what we have to fix.

Cheers
/Ilias


Best regards

Heinrich



Thanks
/Ilias

+#endif

   static int do_dcache(struct cmd_tbl *cmdtp, int flag, int argc,
   char *const argv[])
--
2.43.0







Re: [PATCH 2/3] arm: implement invalidate_icache_all on ARM11

2024-06-19 Thread Heinrich Schuchardt

On 19.06.24 14:22, Ilias Apalodimas wrote:

Hi Heinrich,

On Sun, 16 Jun 2024 at 20:31, Heinrich Schuchardt
 wrote:


In EFI sub-system we rely on invalidate_icache_all() to invalidate the
instruction cache after loading binaries. Add the missing implementation on
ARM1136, ARM1176.

Signed-off-by: Heinrich Schuchardt 
---
  arch/arm/cpu/arm11/cpu.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c
index 01d2e1a125d..4bf0446b543 100644
--- a/arch/arm/cpu/arm11/cpu.c
+++ b/arch/arm/cpu/arm11/cpu.c
@@ -116,3 +116,15 @@ void enable_caches(void)
  #endif
  }
  #endif
+
+#if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
+/* Invalidate entire I-cache */
+void invalidate_icache_all(void)
+{
+   unsigned long i = 0;
+
+   asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i));


This looks correct, but can't we define it as
__asm__("mcr p15, 0, %0, c7, c5, 0" : : "r" (0)); ?


Both compile to the same code. So we should simplify the expression.

arch/arm/cpu/armv7/cache_v7.c and other modules always uses asm and not 
__asm__.


@Tom:
Can we specify what is preferred in doc/develop/codingstyle.rst?

https://gcc.gnu.org/onlinedocs/gcc/Alternate-Keywords.html gives some 
background.


Best regards

Heinrich



Thanks
/Ilias

+}
+#else
+void invalidate_icache_all(void) {}
+#endif
--
2.43.0





Re: [PATCH 1/3] cmd: avoid duplicate weak flush_dcache_all()

2024-06-19 Thread Heinrich Schuchardt

On 19.06.24 14:23, Ilias Apalodimas wrote:

On Sun, 16 Jun 2024 at 20:31, Heinrich Schuchardt
 wrote:


If we have multiple weak implementations of functions, the linker might
choose any of these. ARM and RISC-V already provide a weak implementation
of flush_dcache_all().

Signed-off-by: Heinrich Schuchardt 
---
  cmd/cache.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/cmd/cache.c b/cmd/cache.c
index 0254ff17f9b..16fa0f7c652 100644
--- a/cmd/cache.c
+++ b/cmd/cache.c
@@ -52,11 +52,14 @@ static int do_icache(struct cmd_tbl *cmdtp, int flag, int 
argc,
 return 0;
  }

+/* ARM and RISC-V define a weak flush_dcache_all() themselves. */
+#if !defined(CONFIG_ARM) && !defined(CONFIG_RISCV)
  void __weak flush_dcache_all(void)
  {
 puts("No arch specific flush_dcache_all available!\n");
 /* please define arch specific flush_dcache_all */
  }


Aren't we supposed to add a single __weak function so the linker can
replace it? IOW why is the declaration for Arm/riscv a weak one?


Some sub-architectures override the architecture specific weak 
implementation, e.g.


arch/riscv/cpu/andes/cache.c:44:
void flush_dcache_all(void)

arch/arm/cpu/arm926ejs/cache.c:17:
void flush_dcache_all(void)

Best regards

Heinrich



Thanks
/Ilias

+#endif

  static int do_dcache(struct cmd_tbl *cmdtp, int flag, int argc,
  char *const argv[])
--
2.43.0





[PATCH v4 1/1] usb: informative message if no controller

2024-06-18 Thread Heinrich Schuchardt
The message 'No working controllers found' provides no clue that this
refers to USB controllers.

Provide a message that refers to USB.

Signed-off-by: Heinrich Schuchardt 
Reviewed-by: Dragan Simic 
Reviewed-by: Caleb Connolly 
Reviewed-by: Marek Vasut 
Reviewed-by: Mattijs Korpershoek 
---
v4:
correct commit message (remove log_info() reference)
v3:
plural controllers
v2:
add 'found' at end of message
keep printf
---
 drivers/usb/host/usb-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index cd3a07e4c37..bfec303e7af 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -387,7 +387,7 @@ int usb_init(void)
 
/* if we were not able to find at least one working bus, bail out */
if (controllers_initialized == 0)
-   printf("No working controllers found\n");
+   printf("No USB controllers found\n");
 
return usb_started ? 0 : -ENOENT;
 }
-- 
2.45.1



Re: [PATCH] cmd: make 'booti -h' not crash the board

2024-06-18 Thread Heinrich Schuchardt



Am 18. Juni 2024 16:51:56 MESZ schrieb Caleb Connolly 
:
>Check the result of hextoul() when parsing the first argument to booti,
>and add specific handling for "-h" to print usage rather than causing a
>null pointer exception.
>
>Fixes: 5db28905c952 ("cmd: Split 'bootz' and 'booti' out from 'bootm'")
>Signed-off-by: Caleb Connolly 
>---
> cmd/booti.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
>diff --git a/cmd/booti.c b/cmd/booti.c
>index 62b19e834366..c4029a84e7a7 100644
>--- a/cmd/booti.c
>+++ b/cmd/booti.c
>@@ -31,8 +31,9 @@ static int booti_start(struct bootm_info *bmi)
>   ulong dest_end;
>   unsigned long comp_len;
>   unsigned long decomp_len;
>   int ctype;
>+  char *endp;
> 
>   ret = bootm_run_states(bmi, BOOTM_STATE_START);
> 
>   /* Setup Linux kernel Image entry point */
>@@ -40,9 +41,14 @@ static int booti_start(struct bootm_info *bmi)
>   ld = image_load_addr;
>   debug("*  kernel: default image load address = 0x%08lx\n",
>   image_load_addr);
>   } else {
>-  ld = hextoul(bmi->addr_img, NULL);
>+  ld = hextoul(bmi->addr_img, );
>+  if (*endp != '\0') {
>+  printf("## Invalid kernel image address: %s\n",
>+ bmi->addr_img);
>+  return CMD_RET_USAGE;
>+  }
>   debug("*  kernel: cmdline image address = 0x%08lx\n", ld);
>   }
> 
>   temp = map_sysmem(ld, 0);
>@@ -108,8 +114,11 @@ int do_booti(struct cmd_tbl *cmdtp, int flag, int argc, 
>char *const argv[])
> 
>   /* Consume 'booti' */
>   argc--; argv++;
> 
>+  if (argc && strcmp(argv[0], "-h") == 0)
>+  return CMD_RET_USAGE;

We have the help command which works on all commands. Please, avoid duplicating 
this functionality.

Best regards

Heinrich


>+
>   bootm_init();
>   if (argc)
>   bmi.addr_img = argv[0];
>   if (argc > 1)


Re: [PATCH v6 4/4] test: cmd: fdt: fix chosen test for DM_RNG

2024-06-18 Thread Heinrich Schuchardt



Am 18. Juni 2024 18:45:53 MESZ schrieb Tim Harvey :
>On Tue, Jun 18, 2024 at 8:48 AM Tim Harvey  wrote:
>>
>> On Tue, Jun 18, 2024 at 4:51 AM Heinrich Schuchardt  
>> wrote:
>> >
>> > On 17.06.24 21:14, Tim Harvey wrote:
>> > > Now that kaslr-seed is automatically added to the chosen node if DM_RNG
>> > > is enabled, adjust the test to expect this.
>> >
>> > We need to check that if CONFIG_EFI_TCG2_PROTOCOL=y no kaslr-seed node
>> > is passed to EFI binaries.
>> >
>> > The right location for such a test is lib/efi_selftest/efi_selftest_tcg2.c.
>> >
>>
>> Hi Heinrich,
>>
>> I see you sent a patch for that but I'm not understanding how that
>> fits into the ut framework.
>>
>> > We need as similar check for CONFIG_MEASURED_BOOT=y.
>>
>> Can you explain more please?

The idea of measured boot is that you check if the hash has the same value 
every time you boot. If you measure the device-tree and it contains a random 
value, you get a random hash.

Best regards

Heinrich

>>
>> Does this explain the CI failures I see here:
>> https://dev.azure.com/u-boot/u-boot/_build/results?buildId=8721=logs=6ebe5bb0-481f-5026-b4e6-2d4192a94e80=66c5926e-2461-580f-927d-c0d0a6120549=539
>> https://dev.azure.com/u-boot/u-boot/_build/results?buildId=8721=logs=a1270dec-081b-5c65-5cd5-5e915a842596=69f6cf72-86f3-551a-807d-f28f62a1426f=541
>>
>> I'm still trying to make sense of those.
>>
>> In order to test this locally I built for sandbox64_defconfig and ran
>> "./u-boot -Dc 'ut fdt'" but it seems that doesn't cover enough cases.
>>
>
>I believe I understand the Azure pipeline failures now. I don't see
>exactly where they pick a defconfig but the failed test cases are
>under 'test.py for sandbox sandbox' and 'test.py for sandbox
>sandbox_clang' which I've come to understand means they use
>'sandbox_defconfig' which defines CONFIG_MEASURED_BOOT where
>sandbox64_defconfig which I tested does not.
>
>So I need to update the test to:
>+   if (IS_ENABLED(CONFIG_DM_RNG) &&
>+   !IS_ENABLED(CONFIG_MEASURED_BOOT) &&
>+   !IS_ENABLED(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT))
>+   ut_assert_nextlinen("\tkaslr-seed = ");
>
>I've issued a PR for v7 to test via CI and will submit if all is well.
>
>Best Regards,
>
>Tim
>
>> Best regards,
>>
>> Tim
>>
>> >
>> > Best regards
>> >
>> > Heinrich
>> >
>> > >
>> > > Signed-off-by: Tim Harvey 
>> > > Cc: Michal Simek 
>> > > Cc: Andy Yan 
>> > > Cc: Akash Gajjar 
>> > > Cc: Ilias Apalodimas 
>> > > Cc: Simon Glass 
>> > > Cc: Patrick Delaunay 
>> > > Cc: Patrice Chotard 
>> > > Cc: Devarsh Thakkar 
>> > > Cc: Heinrich Schuchardt 
>> > > Cc: Hugo Villeneuve 
>> > > Cc: Marek Vasut 
>> > > Cc: Tom Rini 
>> > > Cc: Chris Morgan 
>> > > ---
>> > > v6: new patch
>> > > ---
>> > >   test/cmd/fdt.c | 4 
>> > >   1 file changed, 4 insertions(+)
>> > >
>> > > diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
>> > > index 547085521758..537d8a338bbf 100644
>> > > --- a/test/cmd/fdt.c
>> > > +++ b/test/cmd/fdt.c
>> > > @@ -1347,6 +1347,8 @@ static int fdt_test_chosen(struct unit_test_state 
>> > > *uts)
>> > >   ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version 
>> > > string */
>> > >   if (env_bootargs)
>> > >   ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
>> > > + if (CONFIG_IS_ENABLED(DM_RNG))
>> > > + ut_assert_nextlinen("\tkaslr-seed = ");
>> > >   ut_assert_nextline("};");
>> > >   ut_assertok(ut_check_console_end(uts));
>> > >
>> > > @@ -1363,6 +1365,8 @@ static int fdt_test_chosen(struct unit_test_state 
>> > > *uts)
>> > >   ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version 
>> > > string */
>> > >   if (env_bootargs)
>> > >   ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
>> > > + if (CONFIG_IS_ENABLED(DM_RNG))
>> > > + ut_assert_nextlinen("\tkaslr-seed = ");
>> > >   ut_assert_nextline("};");
>> > >   ut_assertok(ut_check_console_end(uts));
>> > >
>> >


Re: [PATCH v3] doc: describe UEFI measured boot

2024-06-18 Thread Heinrich Schuchardt

On 18.06.24 17:23, Ilias Apalodimas wrote:

We currently only describe the process to enable measured boot using
bootm. Describe the UEFI requirements as well which predate bootm.

Signed-off-by: Ilias Apalodimas 


Please, rebase on 00cac7456125 ("doc: describe UEFI measured boot")

Best regards

Heinrich


---
Changes since v2:
- add all bootX commands in the description instead of just bootm
- Remove and extra _ from the header
Changes since v1:
- fixed remarks from Heinrich on titling and DTB measured PCR
  doc/usage/measured_boot.rst | 31 +++
  1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/doc/usage/measured_boot.rst b/doc/usage/measured_boot.rst
index 9691904a9d8a..d31cb05226cd 100644
--- a/doc/usage/measured_boot.rst
+++ b/doc/usage/measured_boot.rst
@@ -7,19 +7,42 @@ U-Boot can perform a measured boot, the process of hashing 
various components
  of the boot process, extending the results in the TPM and logging the
  component's measurement in memory for the operating system to consume.

+The functionality is available when booting via the EFI subsystem or 'bootm'
+command.
+
+UEFI measured boot
+--
+The EFI subsystem implements the `EFI TCG protocol
+`_
+and the `TCG PC Client Specific Platform Firmware Profile Specification
+`_
+which defines the binaries to be measured and the corresponding PCRs to be 
used.
+
+Requirements
+
+* A hardware TPM 2.0 supported by an enabled U-Boot driver
+* CONFIG_EFI_TCG2_PROTOCOL=y
+* CONFIG_EFI_TCG2_PROTOCOL_EVENTLOG_SIZE=y
+* optional CONFIG_EFI_TCG2_PROTOCOL_MEASURE_DTB=y will measure the loaded DTB 
in PCR 1
+
+Measured legacy boot with bootX command
+---
+The commands booti, bootm, and bootz can be used for measured boot
+using the legacy entry point of the Linux kernel.
+
  By default, U-Boot will measure the operating system (linux) image, the
  initrd image, and the "bootargs" environment variable. By enabling
-CONFIG_MEASURE_DEVICETREE, U-Boot will also measure the devicetree image.
+CONFIG_MEASURE_DEVICETREE, U-Boot will also measure the devicetree image in 
PCR1.

  The operating system typically would verify that the hashes found in the
  TPM PCRs match the contents of the event log. This can further be checked
  against the hash results of previous boots.

  Requirements
-
+

-* A hardware TPM 2.0 supported by the U-Boot drivers
-* CONFIG_TPM=y
+* A hardware TPM 2.0 supported by an enabled U-Boot driver
+* CONFIG_TPMv2=y
  * CONFIG_MEASURED_BOOT=y
  * Device-tree configuration of the TPM device to specify the memory area
for event logging. The TPM device node must either contain a phandle to
--
2.45.2





Re: [PATCH 1/1] spl: correct link to FIT specification

2024-06-18 Thread Heinrich Schuchardt

On 18.06.24 13:54, Marek Vasut wrote:

On 6/18/24 8:32 AM, Heinrich Schuchardt wrote:

Replace the invalid link to the FIT file format specification.

Signed-off-by: Heinrich Schuchardt 
---
SPL is tightly size constrained.

Shouldn't we remove the message with the link which is only of interest
to developers.
---
  common/spl/spl_fit.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 988125be008..2a097f4464c 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -587,7 +587,7 @@ __weak void *spl_load_simple_fit_fix_load(const 
void *fit)

  static void warn_deprecated(const char *msg)
  {
  printf("DEPRECATED: %s\n", msg);
-    printf("\tSee doc/uImage.FIT/source_file_format.txt\n");
+    printf("\tSee https://fitspec.osfw.foundation/\n;);


Why not fix the link to local documentation in the source tree instead ?

I would much prefer that than random links to random websites .


The internal page is going away, cf.

[RFC 1/1] doc: Remove FIT documentation that is elsewhere
https://lore.kernel.org/u-boot/20240617202041.3740154-2-sam.povi...@amd.com/

Best regards

Heinrich


[PATCH 1/1] cmd: correct kaslrseed description

2024-06-18 Thread Heinrich Schuchardt
The number of random bytes generated is hard coded as 8.
The command takes no argument.

Signed-off-by: Heinrich Schuchardt 
---
 cmd/kaslrseed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c
index e0d3c7fe748..e9b0e3d5985 100644
--- a/cmd/kaslrseed.c
+++ b/cmd/kaslrseed.c
@@ -68,7 +68,7 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const
 }
 
 U_BOOT_LONGHELP(kaslrseed,
-   "[n]\n"
+   "\n"
"  - append random bytes to chosen kaslr-seed node\n");
 
 U_BOOT_CMD(
-- 
2.45.1



[PATCH 1/1] efi_selftest: can't have measured device-tree with kaslr-seed

2024-06-18 Thread Heinrich Schuchardt
Test that we don't have a /chosen/kaslr-seed property if we measure the
device-tree.

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

diff --git a/lib/efi_selftest/efi_selftest_fdt.c 
b/lib/efi_selftest/efi_selftest_fdt.c
index aa3b13ae3ab..066d9581432 100644
--- a/lib/efi_selftest/efi_selftest_fdt.c
+++ b/lib/efi_selftest/efi_selftest_fdt.c
@@ -227,6 +227,13 @@ static int execute(void)
return EFI_ST_FAILURE;
}
}
+   if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
+   str = get_property(u"kaslr-seed", u"chosen");
+   if (str) {
+   efi_st_error("kaslr-seed with measured fdt\n");
+   return EFI_ST_FAILURE;
+   }
+   }
if (IS_ENABLED(CONFIG_RISCV)) {
u32 fdt_hartid;
 
-- 
2.45.1



Re: [PATCH v6 4/4] test: cmd: fdt: fix chosen test for DM_RNG

2024-06-18 Thread Heinrich Schuchardt

On 17.06.24 21:14, Tim Harvey wrote:

Now that kaslr-seed is automatically added to the chosen node if DM_RNG
is enabled, adjust the test to expect this.


We need to check that if CONFIG_EFI_TCG2_PROTOCOL=y no kaslr-seed node
is passed to EFI binaries.

The right location for such a test is lib/efi_selftest/efi_selftest_tcg2.c.

We need as similar check for CONFIG_MEASURED_BOOT=y.

Best regards

Heinrich



Signed-off-by: Tim Harvey 
Cc: Michal Simek 
Cc: Andy Yan 
Cc: Akash Gajjar 
Cc: Ilias Apalodimas 
Cc: Simon Glass 
Cc: Patrick Delaunay 
Cc: Patrice Chotard 
Cc: Devarsh Thakkar 
Cc: Heinrich Schuchardt 
Cc: Hugo Villeneuve 
Cc: Marek Vasut 
Cc: Tom Rini 
Cc: Chris Morgan 
---
v6: new patch
---
  test/cmd/fdt.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index 547085521758..537d8a338bbf 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -1347,6 +1347,8 @@ static int fdt_test_chosen(struct unit_test_state *uts)
ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version 
string */
if (env_bootargs)
ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
+   if (CONFIG_IS_ENABLED(DM_RNG))
+   ut_assert_nextlinen("\tkaslr-seed = ");
ut_assert_nextline("};");
ut_assertok(ut_check_console_end(uts));

@@ -1363,6 +1365,8 @@ static int fdt_test_chosen(struct unit_test_state *uts)
ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version 
string */
if (env_bootargs)
ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
+   if (CONFIG_IS_ENABLED(DM_RNG))
+   ut_assert_nextlinen("\tkaslr-seed = ");
ut_assert_nextline("};");
ut_assertok(ut_check_console_end(uts));





[PATCH v3 1/1] usb: informative message if no controller

2024-06-18 Thread Heinrich Schuchardt
The message 'No working controllers found' provides no clue that this
refers to USB controllers.

Provide a message that refers to USB. Use log_info().

Signed-off-by: Heinrich Schuchardt 
---
v3:
plural controllers
v2:
add 'found' at end of message
keep printf
---
 drivers/usb/host/usb-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index a1cd0ad2d66..e16432a1516 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -388,7 +388,7 @@ int usb_init(void)
 
/* if we were not able to find at least one working bus, bail out */
if (controllers_initialized == 0)
-   printf("No working controllers found\n");
+   printf("No USB controllers found\n");
 
return usb_started ? 0 : -ENOENT;
 }
-- 
2.43.0



[PATCH v2 1/1] usb: informative message if no controller

2024-06-18 Thread Heinrich Schuchardt
The message 'No working controllers found' provides no clue that this
refers to USB controllers.

Provide a message that refers to USB.

Signed-off-by: Heinrich Schuchardt 
---
v2:
add 'found' at end of message
keep printf
---
 drivers/usb/host/usb-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index a1cd0ad2d66..bd861270af3 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -388,7 +388,7 @@ int usb_init(void)
 
/* if we were not able to find at least one working bus, bail out */
if (controllers_initialized == 0)
-   printf("No working controllers found\n");
+   printf("No USB controller found\n");
 
return usb_started ? 0 : -ENOENT;
 }
-- 
2.43.0



[PATCH 1/1] spl: correct link to FIT specification

2024-06-18 Thread Heinrich Schuchardt
Replace the invalid link to the FIT file format specification.

Signed-off-by: Heinrich Schuchardt 
---
SPL is tightly size constrained.

Shouldn't we remove the message with the link which is only of interest
to developers.
---
 common/spl/spl_fit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 988125be008..2a097f4464c 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -587,7 +587,7 @@ __weak void *spl_load_simple_fit_fix_load(const void *fit)
 static void warn_deprecated(const char *msg)
 {
printf("DEPRECATED: %s\n", msg);
-   printf("\tSee doc/uImage.FIT/source_file_format.txt\n");
+   printf("\tSee https://fitspec.osfw.foundation/\n;);
 }
 
 static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
-- 
2.43.0



[PATCH] boot: links to FIT documentation in Kconfig

2024-06-18 Thread Heinrich Schuchardt
Correct the links to the FIT documentation in boot/Kconfig.

Signed-off-by: Heinrich Schuchardt 
---
 boot/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index 6f3096c15a6..36bdc83b957 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -85,7 +85,7 @@ config FIT_SIGNATURE
  using a hash signed and verified using RSA. If
  CONFIG_SHA_PROG_HW_ACCEL is defined, i.e support for progressive
  hashing is available using hardware, then the RSA library will use
- it. See doc/uImage.FIT/signature.txt for more details.
+ it. See doc/usage/fit/signature.rst for more details.
 
  WARNING: When relying on signed FIT images with a required signature
  check the legacy image format is disabled by default, so that
@@ -223,8 +223,8 @@ config SPL_LOAD_FIT
  1. "loadables" images, other than FDTs, which do not have a "load"
 property will not be loaded. This limitation also applies to FPGA
 images with the correct "compatible" string.
- 2. For FPGA images, the supported "compatible" list is in the
-doc/uImage.FIT/source_file_format.txt.
+ 2. For FPGA images, the supported "compatible" list may be found in
+https://fitspec.osfw.foundation/.
  3. FDTs are only loaded for images with an "os" property of "u-boot".
 "linux" images are also supported with Falcon boot mode.
 
-- 
2.43.0



[PATCH 1/1] doc: FIT links in develop/uefi/uefi.rst

2024-06-18 Thread Heinrich Schuchardt
Correct the links to the FIT documentation.

Signed-off-by: Heinrich Schuchardt 
---
 doc/develop/uefi/uefi.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index 0389b269c01..ea70dcbda86 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -72,7 +72,7 @@ bootm command. This feature is available if U-Boot is 
configured with::
 
 CONFIG_BOOTM_EFI=y
 
-A sample configuration is provided as file doc/uImage.FIT/uefi.its.
+A sample configuration is provided in :doc:`../../usage/fit/uefi`.
 
 Below you find the output of an example session starting GRUB::
 
@@ -96,7 +96,7 @@ Below you find the output of an example session starting 
GRUB::
 ## Transferring control to EFI (at address 404000d0) ...
 Welcome to GRUB!
 
-See doc/uImage.FIT/howto.txt for an introduction to FIT images.
+See :doc:`../../usage/fit/howto` for an introduction to FIT images.
 
 Configuring UEFI secure boot
 
-- 
2.43.0



[PATCH 1/1] cmd: link to doc/usage/fit/x86-fit-boot.rst

2024-06-18 Thread Heinrich Schuchardt
Replace the outdated link.

Signed-off-by: Heinrich Schuchardt 
---
 cmd/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index ff0f5941ecc..9e4245a9702 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -622,7 +622,7 @@ config CMD_ZBOOT
 
  Consider using FIT in preference to this since it supports directly
  booting both 32- and 64-bit kernels, as well as secure boot.
- Documentation is available in doc/uImage.FIT/x86-fit-boot.txt
+ Documentation is available in doc/usage/fit/x86-fit-boot.rst.
 
 endmenu
 
-- 
2.43.0



Re: [RFC 1/1] doc: Remove FIT documentation that is elsewhere

2024-06-18 Thread Heinrich Schuchardt

On 6/17/24 22:20, Sam Povilus wrote:

FIT documentation is now a separate project, instead of having a
duplicate, we should point at the other project.

Signed-off-by: Sam Povilus 
---
  doc/usage/fit/index.rst  |  14 +-
  doc/usage/fit/source_file_format.rst | 682 +--
  2 files changed, 5 insertions(+), 691 deletions(-)

diff --git a/doc/usage/fit/index.rst b/doc/usage/fit/index.rst
index bd25bd30b2..12a8c6fb16 100644
--- a/doc/usage/fit/index.rst
+++ b/doc/usage/fit/index.rst
@@ -4,16 +4,6 @@ Flat Image Tree (FIT)
  =

  U-Boot uses Flat Image Tree (FIT) as a standard file format for packaging
-images that it it reads and boots. Documentation about FIT is available at
-doc/uImage.FIT
+images that it it reads and boots. Documentation about FIT is available in
+`the Flattened Image Tree project `_.

-.. toctree::
-:maxdepth: 1
-
-source_file_format


This is the only entry to be replaced by the external link.


-howto
-x86-fit-boot
-signature
-verified-boot
-beaglebone_vboot
-overlay-fdt-boot


All of these files are staying around and must be kept linked.

Many links are missing here. The complete list should be:

beaglebone_vboot
howto
kernel_fdt
kernel_fdts_compressed
kernel
multi
multi_spl
multi-with-fpga
multi-with-loadables
overlay-fdt-boot
sec_firmware_ppa
signature
sign-configs
sign-images
uefi
update3
update_uboot
verified-boot
x86-fit-boot

These other documents should be carefully redacted. E.g. howto.rst
refers to the FIT file format as "the new uImage format".

Best regards

Heinrich


diff --git a/doc/usage/fit/source_file_format.rst 
b/doc/usage/fit/source_file_format.rst
index b2b1e42bd7..0c44329a82 100644
--- a/doc/usage/fit/source_file_format.rst
+++ b/doc/usage/fit/source_file_format.rst
@@ -3,682 +3,6 @@
  Flattened Image Tree (FIT) Format
  =

-Introduction
-
-
-The number of elements playing a role in the kernel booting process has
-increased over time and now typically includes the devicetree, kernel image and
-possibly a ramdisk image. Generally, all must be placed in the system memory 
and
-booted together.
-
-For firmware images a similar process has taken place, with various binaries
-loaded at different addresses, such as ARM's ATF, OpenSBI, FPGA and U-Boot
-itself.
-
-FIT provides a flexible and extensible format to deal with this complexity. It
-provides support for multiple components. It also supports multiple
-configurations, so that the same FIT can be used to boot multiple boards, with
-some components in common (e.g. kernel) and some specific to that board (e.g.
-devicetree).
-
-Terminology
-~~~
-
-This document defines FIT by providing FDT (Flat Device Tree) bindings. These
-describe the final form of the FIT at the moment when it is used. The user
-perspective may be simpler, as some of the properties (like timestamps and
-hashes) are filled in automatically by the U-Boot mkimage tool.
-
-To avoid confusion with the kernel FDT the following naming convention is used:
-
-FIT
-Flattened Image Tree
-
-FIT is formally a flattened devicetree (in the libfdt meaning), which conforms
-to bindings defined in this document.
-
-.its
-image tree source
-
-.itb
-flattened image tree blob
-
-Image-building procedure
-
-
-The following picture shows how the FIT is prepared. Input consists of
-image source file (.its) and a set of data files. Image is created with the
-help of standard U-Boot mkimage tool which in turn uses dtc (device tree
-compiler) to produce image tree blob (.itb). The resulting .itb file is the
-actual binary of a new FIT::
-
-tqm5200.its
-+
-vmlinux.bin.gz mkimage + dtc   xfer to target
-eldk-4.2-ramdisk  --> tqm5200.itb --> boot
-tqm5200.dtb  /|\
-  |
- 'new FIT'
-
-Steps:
-
-#. Create .its file, automatically filled-in properties are omitted
-
-#. Call mkimage tool on a .its file
-
-#. mkimage calls dtc to create .itb image and assures that
-   missing properties are added
-
-#. .itb (new FIT) is uploaded onto the target and used therein
-
-
-Unique identifiers
-~~
-
-To identify FIT sub-nodes representing images, hashes, configurations (which
-are defined in the following sections), the "unit name" of the given sub-node
-is used as it's identifier as it assures uniqueness without additional
-checking required.
-
-
-External data
-~
-
-FIT is normally built initially with image data in the 'data' property of each
-image node. It is also possible for this data to reside outside the FIT itself.
-This allows the 'FDT' part of the FIT to be quite small, so that it can be
-loaded and scanned 

Re: [PATCH v4 05/14] net-lwip: add DHCP support and dhcp commmand

2024-06-17 Thread Heinrich Schuchardt

On 17.06.24 17:32, Jerome Forissier wrote:

Add what it takes to enable NETDEVICES with NET_LWIP and enable DHCP as
well as the dhcp command. CMD_TFTPBOOT is selected by BOOTMETH_EFI due
to this code having an implicit dependency on do_tftpb().

Signed-off-by: Jerome Forissier 
---
  Makefile|   6 +
  boot/Kconfig|   3 +-
  cmd/Kconfig |  26 
  cmd/Makefile|   4 +
  cmd/net-lwip.c  |  13 ++
  common/board_r.c|   4 +-
  drivers/net/Kconfig |   2 +-
  include/net-lwip.h  |   2 +
  net-lwip/Makefile   |  15 +++
  net-lwip/dhcp.c |  99 +++
  net-lwip/eth_internal.h |  35 ++
  net-lwip/net-lwip.c | 270 
  net-lwip/tftp.c |  11 ++
  13 files changed, 486 insertions(+), 4 deletions(-)
  create mode 100644 cmd/net-lwip.c
  create mode 100644 net-lwip/Makefile
  create mode 100644 net-lwip/dhcp.c
  create mode 100644 net-lwip/eth_internal.h
  create mode 100644 net-lwip/net-lwip.c
  create mode 100644 net-lwip/tftp.c

diff --git a/Makefile b/Makefile
index 0fe1623c550..92a0ab770bb 100644
--- a/Makefile
+++ b/Makefile
@@ -862,6 +862,7 @@ libs-y += env/
  libs-y += lib/
  libs-y += fs/
  libs-$(CONFIG_NET) += net/
+libs-$(CONFIG_NET_LWIP) += net-lwip/
  libs-y += disk/
  libs-y += drivers/
  libs-$(CONFIG_SYS_FSL_DDR) += drivers/ddr/fsl/
@@ -2132,6 +2133,11 @@ etags:
  cscope:
$(FIND) $(FINDFLAGS) $(TAG_SUBDIRS) -name '*.[chS]' -print > \
cscope.files
+ifdef CONFIG_NET_LWIP
+   echo net/eth-uclass.c net/eth_common.c net/eth_bootdev.c \
+net/mdio-uclass.c net/mdio-mux-uclass.c >> \
+   cscope.files
+endif
@find $(TAG_SUBDIRS) -name '*.[chS]' -type l -print | \
grep -xvf - cscope.files > cscope.files.no-symlinks; \
mv cscope.files.no-symlinks cscope.files
diff --git a/boot/Kconfig b/boot/Kconfig
index 6f3096c15a6..004e69dd92a 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -378,7 +378,7 @@ config BOOT_DEFAULTS_CMDS
select CMD_FAT
select CMD_FS_GENERIC
select CMD_PART if PARTITIONS
-   select CMD_DHCP if CMD_NET
+   select CMD_DHCP if CMD_NET || CMD_NET_LWIP
select CMD_PING if CMD_NET
select CMD_PXE if CMD_NET
select CMD_BOOTI if ARM64
@@ -540,6 +540,7 @@ config BOOTMETH_EXTLINUX_PXE
  config BOOTMETH_EFILOADER
bool "Bootdev support for EFI boot"
depends on EFI_BINARY_EXEC
+   select CMD_TFTPBOOT if CMD_NET_LWIP
default y
help
  Enables support for EFI boot using bootdevs. This makes the
diff --git a/cmd/Kconfig b/cmd/Kconfig
index b026439c773..1bfa528e945 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2084,6 +2084,32 @@ config CMD_WOL

  endif

+if NET_LWIP
+
+menuconfig CMD_NET_LWIP
+   bool "Network commands (lwIP)"
+   default y
+
+if CMD_NET_LWIP
+
+config CMD_DHCP
+   bool "dhcp"
+   select PROT_DHCP_LWIP
+   help
+ Boot image via network using DHCP/TFTP protocol
+
+config CMD_TFTPBOOT
+   bool "tftp"
+   select PROT_UDP_LWIP
+   default n
+   help
+ tftpboot - load file via network using TFTP protocol
+ Currently a placeholder (not implemented)
+
+endif
+
+endif
+
  menu "Misc commands"

  config CMD_2048
diff --git a/cmd/Makefile b/cmd/Makefile
index 87133cc27a8..535b6838ca5 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -128,6 +128,10 @@ endif
  obj-$(CONFIG_CMD_MUX) += mux.o
  obj-$(CONFIG_CMD_NAND) += nand.o
  obj-$(CONFIG_CMD_NET) += net.o
+obj-$(CONFIG_CMD_NET_LWIP) += net-lwip.o
+ifdef CONFIG_CMD_NET_LWIP
+CFLAGS_net-lwip.o := -I$(srctree)/lib/lwip/lwip/src/include 
-I$(srctree)/lib/lwip/u-boot
+endif
  obj-$(CONFIG_ENV_SUPPORT) += nvedit.o
  obj-$(CONFIG_CMD_NVEDIT_EFI) += nvedit_efi.o
  obj-$(CONFIG_CMD_ONENAND) += onenand.o
diff --git a/cmd/net-lwip.c b/cmd/net-lwip.c
new file mode 100644
index 000..82edb5fd2e6
--- /dev/null
+++ b/cmd/net-lwip.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2024 Linaro Ltd. */
+
+#include 
+#include 
+
+#if defined(CONFIG_CMD_DHCP)
+U_BOOT_CMD(
+dhcp,   3,  1,  do_dhcp,
+"boot image via network using DHCP/TFTP protocol",
+"[loadAddress] [[hostIPaddr:]bootfilename]"
+);
+#endif
diff --git a/common/board_r.c b/common/board_r.c
index da0b80f24ff..6548eb8fdd5 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -472,7 +472,7 @@ static int initr_status_led(void)
  }
  #endif

-#ifdef CONFIG_CMD_NET
+#if defined(CONFIG_CMD_NET) || defined(CONFIG_CMD_NET_LWIP)
  static int initr_net(void)
  {
puts("Net:   ");
@@ -727,7 +727,7 @@ static init_fnc_t init_sequence_r[] = {
  #ifdef CONFIG_PCI_ENDPOINT
pci_ep_init,
  #endif
-#ifdef CONFIG_CMD_NET
+#if defined(CONFIG_CMD_NET) || 

Re: [PATCH 1/1] doc: update requirements.txt

2024-06-17 Thread Heinrich Schuchardt

On 17.06.24 15:53, Simon Glass wrote:

Hi Heinrich,

On Sun, 16 Jun 2024 at 02:59, Heinrich Schuchardt
 wrote:


Update all required Python packages to current release.

Signed-off-by: Heinrich Schuchardt 
---
  doc/sphinx/requirements.txt | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)


Reviewed-by: Simon Glass 

Does this fix / change anything? It is good to keep up-to-date to some
extent, but it will break documentation for existing checkouts, until
they refresh.

Regards,
SImon


Thanks for reviewing.

There was no bug but for instance an outdated certificates list was used.

The new requirements.txt is only used when you run `pip install -r 
doc/sphinx/requirements.txt`.


Best regards

Heinrich


Re: [PATCH v2 2/2] bootstd: Replace bootmethod(s) -> bootmeth(s)

2024-06-17 Thread Heinrich Schuchardt

On 17.06.24 11:00, Quentin Schulz wrote:

Hi all,

On 6/17/24 8:31 AM, Mattijs Korpershoek wrote:

Hi Heinrich,

Thank you for your review.

On dim., juin 16, 2024 at 09:38, Heinrich Schuchardt
 wrote:


On 6/4/24 17:15, Mattijs Korpershoek wrote:

According to [1], we should use bootmeth when describing the
struct bootmeth:

"""
For version 2, a new naming scheme is used as above:

  - bootdev is used instead of bootdevice, because 'device' is
overused,
  is everywhere in U-Boot, can be confused with udevice


Boot devices are udevices though they don't relate to hardware but to an
abstract concept.

bootdev is just an abbreviation. This does not make the meaning any
clearer.


Per my understanding, the name for this concept is "bootdev", not
"boot device", see:

https://docs.u-boot.org/en/latest/develop/bootstd.html#introduction




  - bootmeth - because 'method' is too vanilla, appears 1300
times in
  U-Boot
"""


Avoiding abbreviations like bootdev and bootmeth improved readability.


The above paragraph is quoted from email [1].
In this email, Simon made the choice to use bootmeth and bootdev
when pushing the initial implementation.

This patch just corrects the places where the older terminology
(bootmethod, bootdevice) was still used.



The current wording is just incorrect, so it needs to be fixed. We have
two choices: use the struct/abbreviated name (bootdevice -> bootdev;
bootmethod -> bootmeth) or the full name (bootdevice -> boot device;
bootmethod -> boot method).


The English languages has three types of compound words: solid,
hyphenated, open. bootmethod, boot-method, boot method all mean the same.

According to https://www.merriam-webster.com/help/faq-compound-words:
"Compound nouns are usually written as one word."

See also "U.S. Government Publishing Office Style Manual", chapter 6,
"COMPOUNDING RULES",
https://www.govinfo.gov/content/pkg/GPO-STYLEMANUAL-2000/pdf/GPO-STYLEMANUAL-2000.pdf

We should avoid unnecessary abbreviations.

Best regards

Heinrich



Heinrich are you suggesting we go for full name instead?

board/sandbox/sandbox.env should be using bootmeth instead as that's the
name of the feature?

Cheers,
Quentin




[PATCH 1/1] usb: informative message if no controller

2024-06-17 Thread Heinrich Schuchardt
The message 'No working controllers found' provides no clue that this
refers to USB controllers.

Provide a message that refers to USB. Use log_info().

Signed-off-by: Heinrich Schuchardt 
---
 drivers/usb/host/usb-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index a1cd0ad2d66..e16432a1516 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -388,7 +388,7 @@ int usb_init(void)
 
/* if we were not able to find at least one working bus, bail out */
if (controllers_initialized == 0)
-   printf("No working controllers found\n");
+   log_info("No USB controller\n");
 
return usb_started ? 0 : -ENOENT;
 }
-- 
2.43.0



[PATCH 3/3] efi_loader: avoid duplicate weak invalidate_icache_all()

2024-06-16 Thread Heinrich Schuchardt
If multiple weak implementations of a weak function exist, it is unclear
which one the linker should chose. cmd/cache.c already defines a weak
invalidate_icache_all().

We don't need a call to invalidate_icache_all() on x86.
ARM, RISC-V, and Sandbox provide an implementation.

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/efi_image_loader.c | 13 +++--
 lib/efi_loader/efi_runtime.c  |  7 ++-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c 
b/lib/efi_loader/efi_image_loader.c
index 60424360328..45dc5b6b244 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -173,11 +173,6 @@ static efi_status_t efi_loader_relocate(const 
IMAGE_BASE_RELOCATION *rel,
return EFI_SUCCESS;
 }
 
-void __weak invalidate_icache_all(void)
-{
-   /* If the system doesn't support icache_all flush, cross our fingers */
-}
-
 /**
  * efi_set_code_and_data_type() - determine the memory types to be used for 
code
  *   and data.
@@ -986,7 +981,13 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj 
*handle,
/* Flush cache */
flush_cache((ulong)efi_reloc,
ALIGN(virt_size, EFI_CACHELINE_SIZE));
-   invalidate_icache_all();
+
+   /*
+* If on x86 a write affects a prefetched instruction,
+* the prefetch queue is invalidated.
+*/
+   if (!CONFIG_IS_ENABLED(X86))
+   invalidate_icache_all();
 
/* Populate the loaded image interface bits */
loaded_image_info->image_base = efi_reloc;
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 011bcd04836..05369c47b01 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -783,7 +783,12 @@ void efi_runtime_relocate(ulong offset, struct 
efi_mem_desc *map)
lastoff = offset;
 #endif
 
-invalidate_icache_all();
+   /*
+* If on x86 a write affects a prefetched instruction,
+* the prefetch queue is invalidated.
+*/
+   if (!CONFIG_IS_ENABLED(X86))
+   invalidate_icache_all();
 }
 
 /**
-- 
2.43.0



[PATCH 2/3] arm: implement invalidate_icache_all on ARM11

2024-06-16 Thread Heinrich Schuchardt
In EFI sub-system we rely on invalidate_icache_all() to invalidate the
instruction cache after loading binaries. Add the missing implementation on
ARM1136, ARM1176.

Signed-off-by: Heinrich Schuchardt 
---
 arch/arm/cpu/arm11/cpu.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c
index 01d2e1a125d..4bf0446b543 100644
--- a/arch/arm/cpu/arm11/cpu.c
+++ b/arch/arm/cpu/arm11/cpu.c
@@ -116,3 +116,15 @@ void enable_caches(void)
 #endif
 }
 #endif
+
+#if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
+/* Invalidate entire I-cache */
+void invalidate_icache_all(void)
+{
+   unsigned long i = 0;
+
+   asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i));
+}
+#else
+void invalidate_icache_all(void) {}
+#endif
-- 
2.43.0



[PATCH 1/3] cmd: avoid duplicate weak flush_dcache_all()

2024-06-16 Thread Heinrich Schuchardt
If we have multiple weak implementations of functions, the linker might
choose any of these. ARM and RISC-V already provide a weak implementation
of flush_dcache_all().

Signed-off-by: Heinrich Schuchardt 
---
 cmd/cache.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/cmd/cache.c b/cmd/cache.c
index 0254ff17f9b..16fa0f7c652 100644
--- a/cmd/cache.c
+++ b/cmd/cache.c
@@ -52,11 +52,14 @@ static int do_icache(struct cmd_tbl *cmdtp, int flag, int 
argc,
return 0;
 }
 
+/* ARM and RISC-V define a weak flush_dcache_all() themselves. */
+#if !defined(CONFIG_ARM) && !defined(CONFIG_RISCV)
 void __weak flush_dcache_all(void)
 {
puts("No arch specific flush_dcache_all available!\n");
/* please define arch specific flush_dcache_all */
 }
+#endif
 
 static int do_dcache(struct cmd_tbl *cmdtp, int flag, int argc,
 char *const argv[])
-- 
2.43.0



  1   2   3   4   5   6   7   8   9   10   >