[PATCH 2/2] image-host: increase path length when setting up the cipher.

2023-12-29 Thread Hugo Cornelis
This patch increases the maximum path length of the filename
containing the cipher key for the kernel from 128 to 256 characters.

Signed-off-by: Hugo Cornelis 
---

 tools/image-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/image-host.c b/tools/image-host.c
index 3719f36117..b0012a714d 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -376,7 +376,7 @@ static int fit_image_setup_cipher(struct image_cipher_info 
*info,
  int noffset)
 {
char *algo_name;
-   char filename[128];
+   char filename[256];
int ret = -1;
int snprintf_return;
 
-- 
2.34.1



[PATCH 1/2] image-host: add a check of the return value of snprintf.

2023-12-29 Thread Hugo Cornelis
This patch allows to generate a sensible error message when generating
binary images using very long filenames.

This can happen with Buildroot and Yocto builds.

Signed-off-by: Hugo Cornelis 
---

 tools/image-host.c | 29 +
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/tools/image-host.c b/tools/image-host.c
index ca4950312f..3719f36117 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -378,6 +378,7 @@ static int fit_image_setup_cipher(struct image_cipher_info 
*info,
char *algo_name;
char filename[128];
int ret = -1;
+   int snprintf_return;
 
if (fit_image_cipher_get_algo(fit, noffset, _name)) {
fprintf(stderr, "Can't get algo name for cipher in image 
'%s'\n",
@@ -414,8 +415,18 @@ static int fit_image_setup_cipher(struct image_cipher_info 
*info,
}
 
/* Read the key in the file */
-   snprintf(filename, sizeof(filename), "%s/%s%s",
-info->keydir, info->keyname, ".bin");
+   snprintf_return = snprintf(filename, sizeof(filename), "%s/%s%s",
+  info->keydir, info->keyname, ".bin");
+   if (snprintf_return >= sizeof(filename)) {
+   printf("Can't format the key filename when setting up the 
cipher: insufficient buffer space\n");
+   ret = -1;
+   goto out;
+   }
+   if (snprintf_return < 0) {
+   printf("Can't format the key filename when setting up the 
cipher: snprintf error\n");
+   ret = -1;
+   goto out;
+   }
info->key = malloc(info->cipher->key_len);
if (!info->key) {
fprintf(stderr, "Can't allocate memory for key\n");
@@ -436,8 +447,18 @@ static int fit_image_setup_cipher(struct image_cipher_info 
*info,
 
if (info->ivname) {
/* Read the IV in the file */
-   snprintf(filename, sizeof(filename), "%s/%s%s",
-info->keydir, info->ivname, ".bin");
+   snprintf_return = snprintf(filename, sizeof(filename), 
"%s/%s%s",
+  info->keydir, info->ivname, ".bin");
+   if (snprintf_return >= sizeof(filename)) {
+   printf("Can't format the IV filename when setting up 
the cipher: insufficient buffer space\n");
+   ret = -1;
+   goto out;
+   }
+   if (snprintf_return < 0) {
+   printf("Can't format the IV filename when setting up 
the cipher: snprintf error\n");
+   ret = -1;
+   goto out;
+   }
ret = fit_image_read_data(filename, (unsigned char *)info->iv,
  info->cipher->iv_len);
} else {
-- 
2.34.1



Re: [PATCH v13 00/24] Modernize U-Boot shell

2023-12-29 Thread Francis Laniel
Le jeudi 28 décembre 2023, 21:58:59 CET Tom Rini a écrit :
> On Fri, 22 Dec 2023 22:02:20 +0100, Francis Laniel wrote:
> > During 2021 summer, Sean Anderson wrote a contribution to add a new shell,
> > based on LIL, to U-Boot [1, 2].
> > While one of the goals of this contribution was to address the fact actual
> > U-Boot shell, which is based on Busybox hush, is old there was a
> > discussion
> > about adding a new shell versus updating the actual one [3, 4].
> > 
> > So, in this series, with Harald Seiler, we updated the actual U-Boot shell
> > to reflect what is currently in Busybox source code.
> > Basically, this contribution is about taking a snapshot of Busybox
> > shell/hush.c file (as it exists in commit 37460f5da) and adapt it to suit
> > U-Boot needs.
> > 
> > [...]
> 
> Applied to u-boot/next, thanks!

Thank you for the merge!
If there is any problem, do not hesitate to mail me and I will take care of 
it!




Re: [PATCH v13 15/24] cli: add modern hush as parser for run_command*()

2023-12-29 Thread Francis Laniel
Hi!

Le mardi 26 décembre 2023, 10:46:48 CET Simon Glass a écrit :
> Hi,
> 
> On Fri, Dec 22, 2023 at 9:23 PM Tom Rini  wrote:
> > On Fri, Dec 22, 2023 at 10:10:42PM +0100, Francis Laniel wrote:
> > > Hi!
> > > 
> > > Le vendredi 22 décembre 2023, 22:02:35 CET Francis Laniel a écrit :
> > > > Enables using, in code, modern hush as parser for run_command function
> > > > family. It also enables the command run to be used by CLI user of
> > > > modern
> > > > hush.
> > > > 
> > > > Reviewed-by: Simon Glass 
> > > > Signed-off-by: Francis Laniel 
> > 
> > [snip]
> > 
> > > > diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> > > > index a9b555c779..104f49deef 100644
> > > > --- a/test/boot/bootflow.c
> > > > +++ b/test/boot/bootflow.c
> > > > @@ -710,7 +710,21 @@ static int bootflow_scan_menu_boot(struct
> > > > unit_test_state *uts) ut_assert_skip_to_line("(2 bootflows, 2
> > > > valid)");
> > > > 
> > > > ut_assert_nextline("Selected: Armbian");
> > > > 
> > > > -   ut_assert_skip_to_line("Boot failed (err=-14)");
> > > > +
> > > > +   if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
> > > > +   /*
> > > > +* With old hush, despite booti failing to boot, i.e.
> > > > returning
> > > > +* CMD_RET_FAILURE, run_command() returns 0 which leads
> > > 
> > > bootflow_boot(),
> > > 
> > > > as + * we are using bootmeth_script here, to return
> > > > -EFAULT. +*/
> > > > +   ut_assert_skip_to_line("Boot failed (err=-14)");
> > > > +   } else if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
> > > > +   /*
> > > > +* While with modern one, run_command() propagates
> > > 
> > > CMD_RET_FAILURE
> > > 
> > > > returned +   * by booti, so we get 1 here.
> > > > +*/
> > > > +   ut_assert_skip_to_line("Boot failed (err=1)");
> > > > +   }
> > > 
> > > I would like to give a bit of context here.
> > > With the following patch:
> > > diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
> > > index c169c835e3..cc5adda0a3 100644
> > > --- a/test/py/tests/test_ut.py
> > > +++ b/test/py/tests/test_ut.py
> > > @@ -173,7 +173,7 @@ else
> > > 
> > > fi
> > >  
> > >  fi
> > >  booti ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r}
> > > 
> > > -
> > > +echo $?
> > > 
> > >  # Recompile with:
> > >  # mkimage -C none -A arm -T script -d /boot/boot.cmd /boot/boot.scr
> > >  ''' % (mmc_dev)
> > > 
> > > We can easily see that booti is failing while running the test:
> > > $ ./test/py/test.py -o log_cli=true -s --build -v -k
> > > 'test_ut[ut_bootstd_bootflow_scan_menu_boot'
> > > ...
> > > Aborting!
> > > Failed to load '/boot/dtb/rockchip/overlay/-fixup.scr'
> > > 1
> > > 
> > > The problem with old hush, is that the 1 returned here, which
> > > corresponds to CMD_RET_FAILURE, is not propagated as the return value
> > > of run_command(). So, this lead the -EFAULT branch here to be taken:
> > > int bootflow_boot(struct bootflow *bflow)
> > > {
> > > 
> > >   /* ... */
> > >   
> > >   ret = bootmeth_boot(bflow->method, bflow);
> > >   if (ret)
> > >   
> > >   return log_msg_ret("boot", ret);
> > >   
> > >   /*
> > >   
> > >* internal error, should not get here since we should have booted
> > >* something or returned an error
> > >*/
> > >   
> > >   return log_msg_ret("end", -EFAULT);
> > > 
> > > }
> > > 
> > > With modern hush, CMD_RET_FAILURE is propagated as the return value of
> > > run_command().
> > > As a consequence, we return with log_mst_ret("boot", 1), which leaded to
> > > this test to fail.
> > > The above modification consists in adapting the expected output to the
> > > current shell flavor.
> > > I think this is the good thing to do, as I find modern hush behavior
> > > better
> > > than the old one, i.e. it propagates CMD_RET_FAILURE as return of
> > > run_command().
> > 
> > Oh very nice, thanks for digging in to this and explaining!
> 
> Yes, thank you from me too!

You are welcome!
 
> 
> - Simon

Best regards and have a nice end of the year!




Re: Proposal: U-Boot memory management

2023-12-29 Thread Tom Rini
On Fri, Dec 29, 2023 at 06:44:15PM +0100, Mark Kettenis wrote:
> > Date: Fri, 29 Dec 2023 11:17:44 -0500
> > From: Tom Rini 
> > 
> > On Fri, Dec 29, 2023 at 05:05:17PM +0100, Heinrich Schuchardt wrote:
> > > 
> > > 
> > > Am 29. Dezember 2023 16:43:07 MEZ schrieb Tom Rini :
> > > >On Fri, Dec 29, 2023 at 05:36:09AM +, Simon Glass wrote:
> > > >> Hi,
> > > >> 
> > > >> On Sat, Dec 16, 2023 at 6:01 PM Simon Glass  wrote:
> > > >> >
> > > >> > Hi,
> > > >> >
> > > >> > This records my thoughts after a discussion with Ilias & Heinrich re
> > > >> > memory allocation in U-Boot.
> > > >> >
> > > >> > 1. malloc()
> > > >> >
> > > >> > malloc() is used for programmatic memory allocation. It allows memory
> > > >> > to be freed. It is not designed for very large allocations (e.g. a
> > > >> > 10MB kernel or 100MB ramdisk).
> > > >> >
> > > >> > 2. lmb
> > > >> >
> > > >> > lmb is used for large blocks of memory, such as those needed for a
> > > >> > kernel or ramdisk. Allocation is only transitory, for the purposes of
> > > >> > loading some images and booting. If the boot fails, then all lmb
> > > >> > allocations go away.
> > > >> >
> > > >> > lmb is set up by getting all available memory and then removing what
> > > >> > is used by U-Boot (code, data, malloc() space, etc.)
> > > >> >
> > > >> > lmb reservations have a few flags so that areas of memory can be
> > > >> > provided with attributes
> > > >> >
> > > >> > There are some corner cases...e.g. loading a file does an lmb
> > > >> > allocation but only for the purpose of avoiding a file being loaded
> > > >> > over U-Boot code/data. The allocation is dropped immediately after 
> > > >> > the
> > > >> > file is loaded. Within the bootm command, or when using standard 
> > > >> > boot,
> > > >> > this would be fairly easy to solve.
> > > >> >
> > > >> > Linux has renamed lmb to memblock. We should consider doing the same.
> > > >> >
> > > >> > 3. EFI
> > > >> >
> > > >> > EFI has its own memory-allocation tables.
> > > >> >
> > > >> > Like lmb, EFI is able to deal with large allocations. But via a 
> > > >> > 'pool'
> > > >> > function it can also do smaller allocations similar to malloc(),
> > > >> > although each one uses at least 4KB at present.
> > > >> >
> > > >> > EFI allocations do not go away when a boot fails.
> > > >> >
> > > >> > With EFI it is possible to add allocations post facto, in which case
> > > >> > they are added to the allocation table just as if the memory was
> > > >> > allocated with EFI to begin with.
> > > >> >
> > > >> > The EFI allocations and the lmb allocations use the same memory, so 
> > > >> > in
> > > >> > principle could conflict.
> > > >> >
> > > >> > EFI allocations are sometimes used to allocate internal U-Boot data 
> > > >> > as
> > > >> > well, if needed by the EFI app. For example, while efi_image_parse()
> > > >> > uses malloc(), efi_var_mem.c uses EFI allocations since the code runs
> > > >> > in the app context and may need to access the memory after U-Boot has
> > > >> > exited. Also efi_smbios.c uses allocate_pages() and then adds a new
> > > >> > mapping as well.
> > > >> >
> > > >> > EFI memory has attributes, including what the memory is used for (to
> > > >> > some degree of granularity). See enum efi_memory_type and struct
> > > >> > efi_mem_desc. In the latter there are also attribute flags - whether
> > > >> > memory is cacheable, etc.
> > > >> >
> > > >> > EFI also has the x86 idea of 'conventional' memory, meaning (I
> > > >> > believe) that below 4GB that isn't reserved for the hardware/system.
> > > >> > This is meaningless, or at least confusing, on ARM systems.
> > > >> >
> > > >> > 4. reservations
> > > >> >
> > > >> > It is perhaps worth mentioning a fourth method of memory management,
> > > >> > where U-Boot reserves chunks of memory before relocation (in
> > > >> > board_init_f.c), e.g. for the framebuffer, U-Boot code, the malloc()
> > > >> > region, etc.
> > > >> >
> > > >> >
> > > >> > Problems
> > > >> > —---
> > > >> >
> > > >> > There are no urgent problems, but here are some things that could be 
> > > >> > improved:
> > > >> >
> > > >> > 1. EFI should attach most of its data structures to driver model. 
> > > >> > This
> > > >> > work has started, with the partition support, but more effort would
> > > >> > help. This would make it easier to see which memory is related to
> > > >> > devices and which is separate.
> > > >> >
> > > >> > 2. Some drivers do EFI reservations today, whether EFI is used for
> > > >> > booting or not (e.g. rockchip video rk_vop_probe()).
> > > >> >
> > > >> > 3. U-Boot doesn't really map arch-specific memory attributes (e.g.
> > > >> > armv8's struct mm_region) to EFI ones.
> > > >> >
> > > >> > 4. EFI duplicates some code from bootm, some of which relates to
> > > >> > memory allocation (e.g. FDT fixup).
> > > >> >
> > > >> > 5. EFI code is used even if EFI is never used to boot
> > > >> >
> > > >> > 6. EFI allocations can result in the same memory 

Re: Proposal: U-Boot memory management

2023-12-29 Thread Tom Rini
On Fri, Dec 29, 2023 at 06:30:43PM +0100, Heinrich Schuchardt wrote:
> On 12/29/23 18:21, Tom Rini wrote:
> > On Fri, Dec 29, 2023 at 06:09:44PM +0100, Heinrich Schuchardt wrote:
> > > On 12/29/23 17:47, Tom Rini wrote:
> > > > On Fri, Dec 29, 2023 at 05:42:17PM +0100, Heinrich Schuchardt wrote:
> > > > > On 12/20/23 20:12, Tom Rini wrote:
> > > > > > On Tue, Dec 19, 2023 at 09:15:21PM -0700, Simon Glass wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > On Tue, 19 Dec 2023 at 05:46, Tom Rini  wrote:
> > > > > > > > 
> > > > > > > > On Tue, Dec 19, 2023 at 03:15:38AM +0100, Heinrich Schuchardt 
> > > > > > > > wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Am 19. Dezember 2023 02:26:00 MEZ schrieb Tom Rini 
> > > > > > > > > :
> > > > > > > > > > On Tue, Dec 19, 2023 at 01:01:51AM +0100, Heinrich 
> > > > > > > > > > Schuchardt wrote:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Am 19. Dezember 2023 00:31:30 MEZ schrieb Tom Rini 
> > > > > > > > > > > :
> > > > > > > > > > > > On Tue, Dec 19, 2023 at 12:29:19AM +0100, Heinrich 
> > > > > > > > > > > > Schuchardt wrote:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Am 19. Dezember 2023 00:16:40 MEZ schrieb Tom Rini 
> > > > > > > > > > > > > :
> > > > > > > > > > > > > > On Tue, Dec 19, 2023 at 12:08:31AM +0100, Heinrich 
> > > > > > > > > > > > > > Schuchardt wrote:
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Am 18. Dezember 2023 23:41:08 MEZ schrieb Tom 
> > > > > > > > > > > > > > > Rini :
> > > > > > > > > > > > > > > > On Mon, Dec 18, 2023 at 11:34:16PM +0100, 
> > > > > > > > > > > > > > > > Heinrich Schuchardt wrote:
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > [snip]
> > > > > > > > > > > > > > > > > Or take:
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > load host 0:1 $c kernel.efi
> > > > > > > > > > > > > > > > > load host 0:1 $d initrd.img
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > How could we ensure that initrd.img is not 
> > > > > > > > > > > > > > > > > overwriting a part of kernel.efi without 
> > > > > > > > > > > > > > > > > memory allocation?
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Today, invalid checksum as part of some part of 
> > > > > > > > > > > > > > > > the kernel fails. But
> > > > > > > > > > > > > > > > how do we do this tomorrow, are you suggesting 
> > > > > > > > > > > > > > > > that "load" perform
> > > > > > > > > > > > > > > > malloc() in some predefined size? If $c is 
> > > > > > > > > > > > > > > > below $d and $c + kernel.efi
> > > > > > > > > > > > > > > > is now above $d we can throw an error before 
> > > > > > > > > > > > > > > > trying to load, yes. But
> > > > > > > > > > > > > > > > what about:
> > > > > > > > > > > > > > > > load host 0:1 $d initrd.img
> > > > > > > > > > > > > > > > load host 0:1 $c kernel.efi
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > In that case (which is only marginally 
> > > > > > > > > > > > > > > > contrived, the more real case is
> > > > > > > > > > > > > > > > loading device tree in to unexpectedly large 
> > > > > > > > > > > > > > > > ramdisk because someone
> > > > > > > > > > > > > > > > didn't understand the general advice on why 
> > > > > > > > > > > > > > > > device tree is lower than
> > > > > > > > > > > > > > > > ramdisk address) I'm fine with an error that 
> > > > > > > > > > > > > > > > amounts to "you just
> > > > > > > > > > > > > > > > corrupted another allocation" and then "fail, 
> > > > > > > > > > > > > > > > reset the board" or so.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Our current malloc library cannot manage the 
> > > > > > > > > > > > > > > complete memory. We need a library like lmb which 
> > > > > > > > > > > > > > > should also cover the memory management that we 
> > > > > > > > > > > > > > > currently have in lib/efi/efi_memory.c. This must 
> > > > > > > > > > > > > > > include a memory type attribute for usage in the 
> > > > > > > > > > > > > > > GetMemoryMap() service. A management on page 
> > > > > > > > > > > > > > > level seems sufficient.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > The load command should permanently allocate 
> > > > > > > > > > > > > > > memory in that lmb+ library.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > We need an unload command to free the memory if 
> > > > > > > > > > > > > > > we want to reuse the memory or we might let the 
> > > > > > > > > > > > > > > load comand free the memory if exactly the same 
> > > > > > > > > > > > > > > start address is reused.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Our current way of loading things in to memory does 
> > > > > > > > > > > > > > not handle the case
> > > > > > > > > > > > > > I described, yes. How would what you're proposing 

[PATCH v2 05/26] arm: dts: k3-am654: copy bootph properties to a53 dts

2023-12-29 Thread Bryan Brattlof
In order to unify the R5 board dtb file with the Linux board dtb file,
we will need to copy all bootph-pre-ram properties to the *-u-boot.dtsi
overlay.

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi | 160 +++
 arch/arm/dts/k3-am654-r5-base-board.dts  |  85 +-
 2 files changed, 162 insertions(+), 83 deletions(-)

diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi 
b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
index f29cecf870bcd..4b1e8ce2c920c 100644
--- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
@@ -5,6 +5,166 @@
 
 #include "k3-am65x-binman.dtsi"
 
+_supply {
+   bootph-pre-ram;
+};
+
+_main {
+   bootph-pre-ram;
+};
+
+_navss {
+   bootph-pre-ram;
+};
+
+_mcu {
+   bootph-pre-ram;
+};
+
+_navss {
+   bootph-pre-ram;
+};
+
+_ringacc {
+   bootph-pre-ram;
+};
+
+_udmap {
+   bootph-pre-ram;
+};
+
+_gpio0 {
+   bootph-pre-ram;
+};
+
+_proxy_main {
+   bootph-pre-ram;
+};
+
+_wakeup {
+   bootph-pre-ram;
+
+   chipid@4314 {
+   bootph-pre-ram;
+   };
+};
+
+ {
+   bootph-pre-ram;
+};
+
+_pds {
+   bootph-pre-ram;
+};
+
+_clks {
+   bootph-pre-ram;
+};
+
+_reset {
+   bootph-pre-ram;
+};
+
+_uart0 {
+   bootph-pre-ram;
+};
+
+_vtm0 {
+   bootph-pre-ram;
+};
+
+_pmx0 {
+   bootph-pre-ram;
+};
+
+_uart0_pins_default {
+   bootph-pre-ram;
+};
+
+_vtt_pins_default {
+   bootph-pre-ram;
+};
+
+_uart0_pins_default {
+   bootph-pre-ram;
+};
+
+_i2c0_pins_default {
+   bootph-pre-ram;
+};
+
+_fss0_ospi0_pins_default {
+   bootph-pre-ram;
+};
+
+_pmx0 {
+   bootph-pre-ram;
+};
+
+_uart0_pins_default {
+   bootph-pre-ram;
+};
+
+_mmc0_pins_default {
+   bootph-pre-ram;
+};
+
+_mmc1_pins_default {
+   bootph-pre-ram;
+};
+
+_pins_default {
+   bootph-pre-ram;
+};
+
+_pmx1 {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+_i2c0 {
+   bootph-pre-ram;
+};
+
+_mpu {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+
+   flash@0 {
+   bootph-pre-ram;
+   };
+};
+
+_0 {
+   bootph-pre-ram;
+};
+
+_phy {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+_conf {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
 _0 {
remoteproc-name = "pru0_0";
 };
diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index d75c7bf3fe662..8f55dab508ee6 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -53,13 +53,10 @@
regulator-max-microvolt = <330>;
gpios = <_gpio0 28 GPIO_ACTIVE_HIGH>;
states = <0 0x0 330 0x1>;
-   bootph-pre-ram;
};
 };
 
 _main {
-   bootph-pre-ram;
-
timer1: timer@4040 {
compatible = "ti,omap5430-timer";
reg = <0x0 0x4040 0x0 0x80>;
@@ -67,15 +64,9 @@
clock-frequency = <2500>;
bootph-all;
};
-
-   main_navss: bus@3080 {
-   bootph-pre-ram;
-   };
 };
 
 _mcu {
-   bootph-pre-ram;
-
mcu_secproxy: secproxy@2838 {
compatible = "ti,am654-secure-proxy";
reg = <0x0 0x2a38 0x0 0x8>,
@@ -87,8 +78,6 @@
};
 
mcu_navss: bus@2838 {
-   bootph-pre-ram;
-
ringacc@2b80 {
reg =   <0x0 0x2b80 0x0 0x40>,
<0x0 0x2b00 0x0 0x40>,
@@ -96,7 +85,6 @@
<0x0 0x2a50 0x0 0x4>,
<0x0 0x2844 0x0 0x4>;
reg-names = "rt", "fifos", "proxy_gcfg", 
"proxy_target", "cfg";
-   bootph-pre-ram;
ti,dma-ring-reset-quirk;
};
 
@@ -109,34 +97,11 @@
<0x0 0x2840 0x0 0x2000>;
reg-names = "gcfg", "rchan", "rchanrt", "tchan",
"tchanrt", "rflow";
-   bootph-pre-ram;
};
};
 };
 
-_pds {
-   bootph-pre-ram;
-};
-
-_clks {
-   bootph-pre-ram;
-};
-
-_reset {
-   bootph-pre-ram;
-};
-
-_gpio0 {
-   bootph-pre-ram;
-};
-
-_proxy_main {
-   bootph-pre-ram;
-};
-
 _wakeup {
-   bootph-pre-ram;
-
sysctrler: sysctrler {
compatible = "ti,am654-system-controller";
mboxes= <_secproxy 4>, <_secproxy 5>;
@@ -150,40 +115,29 @@
clock-frequency = <2>;
bootph-pre-ram;
};
-
-   chipid@4314 {
-   bootph-pre-ram;
-   };
 };
 
  {
-   bootph-pre-ram;
-
mboxes= <_secproxy 8>, <_secproxy 

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

2023-12-29 Thread Bryan Brattlof
Hello Again Everyone!

This series gets the am65x booting again along with syncing the device
tree files with v6.7-rc1 Linux.

The bulk of these patches unify the WKUP SPL board file with the arm64
files to make future syncs from Linux much easier. In the end the DTBs
should look a lot like what the DTBs look like for the am64x which
is fairly similar to the am65x.

For those interested in what UART boot looks like:
   https://paste.sr.ht/~bryanb/7df8a645dc548912cd806abd5ecab967ef3287bc

Changes from v1: [0]
- fixed multi-line comment format
- moved wkup_uart0 and mcu_uart0 U-Boot overrides to the r5 board file
  as they are not needed for a53/main domain U-Boot builds
- corrected a bad wkup_i2c0_pins_default fixup in PATCH 6/26
- spelling fix (s/Libux/Linux/) in commit body for PATCH 6/26
- added trailers from Tom

Thanks for reviewing and happy holidays
~Bryan

[0] https://lore.kernel.org/u-boot/20231221174412.210807-1...@ti.com/

Bryan Brattlof (26):
  configs: am65x_evm_r5: enable driver for fixed regulators
  configs: am65x_evm_a53: disable CONSOLE_MUX
  arm: dts: k3-am654-r5: Merge board file and U-Boot overlay
  arm: dts: k3-am654: pull in dtb update from Linux
  arm: dts: k3-am654: copy bootph properties to a53 dts
  arm: dts: k3-am654: include a53 board dtb for r5 build
  arm: dts: k3-am654: remove duplicate vtt_supply
  arm: dts: k3-am654: remove duplicate wkup_uart0
  arm: dts: k3-am654: remove duplicate timer
  arm: dts: k3-am654: remove duplicate mcu_ringacc
  arm: dts: k3-am654: remove duplicate mcu_udmap
  arm: dts: k3-am654: add needed regs to udmap nodes
  arm: dts: k3-am654: remove duplicate mcu_uart0 node
  arm: dts: k3-am654: remove duplicate main_uart0
  arm: dts: k3-am654: remove duplicate sdhci0 pinmux node
  arm: dts: k3-am654: remove duplicate sdhci1 pinmux node
  arm: dts: k3-am654: remove duplicate wkup_i2c0
  arm: dts: k3-am654: remove duplicate ospi0 node
  arm: dts: k3-am654: remove usb0
  arm: dts: k3-am654: remove duplicate mdio
  arm: dts: k3-am654: remove duplicate vtt pinmux
  arm: dts: k3-am654: remove duplicate root properties
  arm: dts: k3-am654: remove un-needed aliases
  arm: dts: k3-am654: move dummy_clock to root node
  arm: dts: k3-am654: remove duplicate mcu secure proxy node
  arm: dts: k3-am654: convert bootph-pre-ram to bootph-all

 arch/arm/dts/k3-am65-main.dtsi| 342 +++---
 arch/arm/dts/k3-am65-mcu.dtsi | 156 +++-
 arch/arm/dts/k3-am65-wakeup.dtsi  |  10 +-
 arch/arm/dts/k3-am65.dtsi |  19 +-
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi  | 195 +-
 arch/arm/dts/k3-am654-base-board.dts  | 301 +--
 .../dts/k3-am654-r5-base-board-u-boot.dtsi| 208 ---
 arch/arm/dts/k3-am654-r5-base-board.dts   | 303 
 arch/arm/dts/k3-am654.dtsi|   7 +
 configs/am65x_evm_a53_defconfig   |   1 -
 configs/am65x_evm_r5_defconfig|   2 +
 11 files changed, 886 insertions(+), 658 deletions(-)
 delete mode 100644 arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi


base-commit: 0a0ceea2269b983e736b80104f03cc800d1a5e2a
-- 
2.43.0



[PATCH v2 13/26] arm: dts: k3-am654: remove duplicate mcu_uart0 node

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified we now have a
duplicate mcu_uart0 node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 12 
 1 file changed, 12 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 19806ec4e2309..bdee47802b047 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -18,7 +18,6 @@
ethernet0 = _port1;
remoteproc0 = 
remoteproc1 = _0;
-   serial1 = _uart0;
serial2 = _uart0;
spi0 = 
spi1 = 
@@ -98,8 +97,6 @@
 };
 
 _uart0 {
-   pinctrl-names = "default";
-   pinctrl-0 = <_uart0_pins_default>;
clock-frequency = <4800>;
/delete-property/ power-domains;
status = "okay";
@@ -126,15 +123,6 @@
>;
};
 
-   mcu_uart0_pins_default: mcu_uart0_pins_default {
-   pinctrl-single,pins = <
-   AM65X_WKUP_IOPAD(0x0044, PIN_INPUT, 4)  /* (P4) 
MCU_OSPI1_D1.MCU_UART0_RXD */
-   AM65X_WKUP_IOPAD(0x0048, PIN_OUTPUT, 4) /* (P5) 
MCU_OSPI1_D2.MCU_UART0_TXD */
-   AM65X_WKUP_IOPAD(0x004C, PIN_INPUT, 4)  /* (P1) 
MCU_OSPI1_D3.MCU_UART0_CTSn */
-   AM65X_WKUP_IOPAD(0x0054, PIN_OUTPUT, 4) /* (N3) 
MCU_OSPI1_CSn1.MCU_UART0_RTSn */
-   >;
-   };
-
wkup_i2c0_pins_default: wkup-i2c0-pins-default {
pinctrl-single,pins = <
AM65X_WKUP_IOPAD(0x00e0, PIN_INPUT, 0) /* (AC7) 
WKUP_I2C0_SCL */
-- 
2.43.0



[PATCH v2 24/26] arm: dts: k3-am654: move dummy_clock to root node

2023-12-29 Thread Bryan Brattlof
The dummy_clock node is used to help the drivers probe the IO needed to
setup consoles and boot media to load firmware into the SoC.

This dummy_clock isn't a device that exists nor does it exist in the
mcu domain. So move it from cbass_mcu to the root node to avoid any
confusion.

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 9926981c661a0..b27b2b05ad840 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -30,6 +30,13 @@
ti,sci-host-id = <10>;
bootph-pre-ram;
};
+
+   clk_200mhz: dummy_clock {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <2>;
+   bootph-pre-ram;
+   };
 };
 
 _mcu {
@@ -51,13 +58,6 @@
mbox-names = "tx", "rx";
bootph-pre-ram;
};
-
-   clk_200mhz: dummy_clock {
-   compatible = "fixed-clock";
-   #clock-cells = <0>;
-   clock-frequency = <2>;
-   bootph-pre-ram;
-   };
 };
 
 /*
-- 
2.43.0



[PATCH v2 25/26] arm: dts: k3-am654: remove duplicate mcu secure proxy node

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have a
duplicate mcu secure proxy node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index b27b2b05ad840..dea2ba85dcb31 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -39,22 +39,15 @@
};
 };
 
-_mcu {
-   mcu_secproxy: secproxy@2838 {
-   compatible = "ti,am654-secure-proxy";
-   reg = <0x0 0x2a38 0x0 0x8>,
- <0x0 0x2a40 0x0 0x8>,
- <0x0 0x2a48 0x0 0x8>;
-   reg-names = "rt", "scfg", "target_data";
-   #mbox-cells = <1>;
-   bootph-pre-ram;
-   };
+_proxy_mcu {
+   status = "okay";
+   bootph-pre-ram;
 };
 
 _wakeup {
sysctrler: sysctrler {
compatible = "ti,am654-system-controller";
-   mboxes= <_secproxy 4>, <_secproxy 5>;
+   mboxes= <_proxy_mcu 4>, <_proxy_mcu 5>;
mbox-names = "tx", "rx";
bootph-pre-ram;
};
@@ -76,7 +69,9 @@
 };
 
  {
-   mboxes= <_secproxy 8>, <_secproxy 6>, <_secproxy 5>;
+   mboxes = <_proxy_mcu 8>,
+<_proxy_mcu 6>,
+<_proxy_mcu 5>;
mbox-names = "tx", "rx", "notify";
ti,host-id = <4>;
ti,secure-host;
-- 
2.43.0



[PATCH v2 09/26] arm: dts: k3-am654: remove duplicate timer

2023-12-29 Thread Bryan Brattlof
timer1 is really just the mcu_timer0 node redefined for the WKUP SPL.
Remove the timer1 and replace it with the mcu_timer0 from the Linux
device tree we imported into U-Boot.

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi | 12 
 arch/arm/dts/k3-am654-r5-base-board.dts  | 30 ++--
 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi 
b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
index 4b1e8ce2c920c..a008af5b4a047 100644
--- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
@@ -5,6 +5,18 @@
 
 #include "k3-am65x-binman.dtsi"
 
+/ {
+   chosen {
+   tick-timer = _timer0;
+   };
+};
+
+_timer0 {
+   ti,timer-alwon;
+   clock-frequency = <2500>;
+   bootph-all;
+};
+
 _supply {
bootph-pre-ram;
 };
diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index fb13a17b1dc64..f462262b9aaac 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -26,11 +26,6 @@
usb1 = 
};
 
-   chosen {
-   stdout-path = "serial2:115200n8";
-   tick-timer = 
-   };
-
a53_0: a53@0 {
compatible = "ti,am654-rproc";
reg = <0x0 0x00a9 0x0 0x10>;
@@ -47,16 +42,6 @@
};
 };
 
-_main {
-   timer1: timer@4040 {
-   compatible = "ti,omap5430-timer";
-   reg = <0x0 0x4040 0x0 0x80>;
-   ti,timer-alwon;
-   clock-frequency = <2500>;
-   bootph-all;
-   };
-};
-
 _mcu {
mcu_secproxy: secproxy@2838 {
compatible = "ti,am654-secure-proxy";
@@ -108,6 +93,21 @@
};
 };
 
+/*
+ * timer init is called as part of rproc_start() while
+ * starting System Firmware, so any clock/power-domain
+ * operations will fail as SYSFW is not yet up and running.
+ * Delete all clock/power-domain properties to avoid
+ * timer init failure.
+ * This is an always on timer at 20MHz.
+ */
+_timer0 {
+   /delete-property/ clocks;
+   /delete-property/ assigned-clocks;
+   /delete-property/ assigned-clock-parents;
+   /delete-property/ power-domains;
+};
+
  {
mboxes= <_secproxy 8>, <_secproxy 6>, <_secproxy 5>;
mbox-names = "tx", "rx", "notify";
-- 
2.43.0



[PATCH v2 04/26] arm: dts: k3-am654: pull in dtb update from Linux

2023-12-29 Thread Bryan Brattlof
Pull in dtb updates for the am654 base board from v6.7-rc1 of Linux

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am65-main.dtsi   | 342 ++-
 arch/arm/dts/k3-am65-mcu.dtsi| 156 ++--
 arch/arm/dts/k3-am65-wakeup.dtsi |  10 +-
 arch/arm/dts/k3-am65.dtsi|  19 +-
 arch/arm/dts/k3-am654-base-board.dts | 301 ++-
 arch/arm/dts/k3-am654.dtsi   |   7 +
 6 files changed, 626 insertions(+), 209 deletions(-)

diff --git a/arch/arm/dts/k3-am65-main.dtsi b/arch/arm/dts/k3-am65-main.dtsi
index ba4e5d3e1ed7a..5ebb87f467de5 100644
--- a/arch/arm/dts/k3-am65-main.dtsi
+++ b/arch/arm/dts/k3-am65-main.dtsi
@@ -35,7 +35,10 @@
#interrupt-cells = <3>;
interrupt-controller;
reg = <0x00 0x0180 0x00 0x1>,   /* GICD */
- <0x00 0x0188 0x00 0x9>;   /* GICR */
+ <0x00 0x0188 0x00 0x9>,   /* GICR */
+ <0x00 0x6f00 0x00 0x2000>,/* GICC */
+ <0x00 0x6f01 0x00 0x1000>,/* GICH */
+ <0x00 0x6f02 0x00 0x2000>;/* GICV */
/*
 * vcpumntirq:
 * virtual CPU interface maintenance interrupt
@@ -88,6 +91,7 @@
clock-frequency = <4800>;
current-speed = <115200>;
power-domains = <_pds 146 TI_SCI_PD_EXCLUSIVE>;
+   status = "disabled";
};
 
main_uart1: serial@281 {
@@ -96,6 +100,7 @@
interrupts = ;
clock-frequency = <4800>;
power-domains = <_pds 147 TI_SCI_PD_EXCLUSIVE>;
+   status = "disabled";
};
 
main_uart2: serial@282 {
@@ -104,29 +109,47 @@
interrupts = ;
clock-frequency = <4800>;
power-domains = <_pds 148 TI_SCI_PD_EXCLUSIVE>;
+   status = "disabled";
};
 
crypto: crypto@4e0 {
compatible = "ti,am654-sa2ul";
reg = <0x0 0x4e0 0x0 0x1200>;
-   power-domains = <_pds 136 TI_SCI_PD_EXCLUSIVE>;
+   power-domains = <_pds 136 TI_SCI_PD_SHARED>;
#address-cells = <2>;
#size-cells = <2>;
ranges = <0x0 0x04e0 0x00 0x04e0 0x0 0x3>;
 
-   dmas = <_udmap 0xc000>, <_udmap 0x4000>,
-   <_udmap 0x4001>;
+   dmas = <_udmap 0xc001>, <_udmap 0x4002>,
+   <_udmap 0x4003>;
dma-names = "tx", "rx1", "rx2";
-   dma-coherent;
 
rng: rng@4e1 {
compatible = "inside-secure,safexcel-eip76";
reg = <0x0 0x4e1 0x0 0x7d>;
interrupts = ;
-   clocks = <_clks 136 1>;
+   status = "disabled"; /* Used by OP-TEE */
};
};
 
+   /* TIMERIO pad input CTRLMMR_TIMER*_CTRL registers */
+   main_timerio_input: pinctrl@104200 {
+   compatible = "pinctrl-single";
+   reg = <0x0 0x104200 0x0 0x30>;
+   #pinctrl-cells = <1>;
+   pinctrl-single,register-width = <32>;
+   pinctrl-single,function-mask = <0x001ff>;
+   };
+
+   /* TIMERIO pad output CTCTRLMMR_TIMERIO*_CTRL registers */
+   main_timerio_output: pinctrl@104280 {
+   compatible = "pinctrl-single";
+   reg = <0x0 0x104280 0x0 0x20>;
+   #pinctrl-cells = <1>;
+   pinctrl-single,register-width = <32>;
+   pinctrl-single,function-mask = <0x000f>;
+   };
+
main_pmx0: pinctrl@11c000 {
compatible = "pinctrl-single";
reg = <0x0 0x11c000 0x0 0x2e4>;
@@ -152,6 +175,7 @@
clock-names = "fck";
clocks = <_clks 110 1>;
power-domains = <_pds 110 TI_SCI_PD_EXCLUSIVE>;
+   status = "disabled";
};
 
main_i2c1: i2c@201 {
@@ -163,6 +187,7 @@
clock-names = "fck";
clocks = <_clks 111 1>;
power-domains = <_pds 111 TI_SCI_PD_EXCLUSIVE>;
+   status = "disabled";
};
 
main_i2c2: i2c@202 {
@@ -174,6 +199,7 @@
clock-names = "fck";
clocks = <_clks 112 1>;
power-domains = <_pds 112 TI_SCI_PD_EXCLUSIVE>;
+   status = "disabled";
};
 
main_i2c3: i2c@203 {
@@ -185,6 +211,7 @@
clock-names = "fck";
clocks = <_clks 113 1>;
power-domains = <_pds 113 TI_SCI_PD_EXCLUSIVE>;
+   status = "disabled";
};
 
ecap0: pwm@310 {
@@ -194,6 +221,7 @@
power-domains = <_pds 39 

[PATCH v2 18/26] arm: dts: k3-am654: remove duplicate ospi0 node

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have a
duplicate ospi0 node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 37 -
 1 file changed, 37 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 8c6cb147c821a..da41971a78f81 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -18,8 +18,6 @@
ethernet0 = _port1;
remoteproc0 = 
remoteproc1 = _0;
-   spi0 = 
-   spi1 = 
usb0 = 
usb1 = 
};
@@ -114,22 +112,6 @@
AM65X_WKUP_IOPAD(0x0040, PIN_OUTPUT_PULLUP, 7)  /* 
WKUP_GPIO0_28 */
>;
};
-
-   mcu_fss0_ospi0_pins_default: mcu-fss0-ospi0-pins_default {
-   pinctrl-single,pins = <
-   AM65X_WKUP_IOPAD(0x, PIN_OUTPUT, 0) /* (V1) 
MCU_OSPI0_CLK */
-   AM65X_WKUP_IOPAD(0x0008, PIN_INPUT, 0)   /* (U2) 
MCU_OSPI0_DQS */
-   AM65X_WKUP_IOPAD(0x000c, PIN_INPUT, 0)  /* (U4) 
MCU_OSPI0_D0 */
-   AM65X_WKUP_IOPAD(0x0010, PIN_INPUT, 0)  /* (U5) 
MCU_OSPI0_D1 */
-   AM65X_WKUP_IOPAD(0x0014, PIN_INPUT, 0)  /* (T2) 
MCU_OSPI0_D2 */
-   AM65X_WKUP_IOPAD(0x0018, PIN_INPUT, 0)  /* (T3) 
MCU_OSPI0_D3 */
-   AM65X_WKUP_IOPAD(0x001c, PIN_INPUT, 0)  /* (T4) 
MCU_OSPI0_D4 */
-   AM65X_WKUP_IOPAD(0x0020, PIN_INPUT, 0)  /* (T5) 
MCU_OSPI0_D5 */
-   AM65X_WKUP_IOPAD(0x0024, PIN_INPUT, 0)  /* (R2) 
MCU_OSPI0_D6 */
-   AM65X_WKUP_IOPAD(0x0028, PIN_INPUT, 0)  /* (R3) 
MCU_OSPI0_D7 */
-   AM65X_WKUP_IOPAD(0x002c, PIN_OUTPUT, 0) /* (R4) 
MCU_OSPI0_CSn0 */
-   >;
-   };
 };
 
 _pmx0 {
@@ -175,27 +157,8 @@
 };
 
  {
-   pinctrl-names = "default";
-   pinctrl-0 = <_fss0_ospi0_pins_default>;
-   bootph-pre-ram;
-
reg = <0x0 0x4704 0x0 0x100>,
  <0x0 0x5000 0x0 0x800>;
-
-   flash@0{
-   compatible = "jedec,spi-nor";
-   reg = <0x0>;
-   spi-tx-bus-width = <1>;
-   spi-rx-bus-width = <8>;
-   spi-max-frequency = <5000>;
-   cdns,tshsl-ns = <60>;
-   cdns,tsd2d-ns = <60>;
-   cdns,tchsh-ns = <60>;
-   cdns,tslch-ns = <60>;
-   cdns,read-delay = <0>;
-   #address-cells = <1>;
-   #size-cells = <1>;
-   };
 };
 
 _pmx0 {
-- 
2.43.0



[PATCH v2 14/26] arm: dts: k3-am654: remove duplicate main_uart0

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have a
duplicate main_uart0 node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 17 -
 1 file changed, 17 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index bdee47802b047..3d599a5413812 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -18,7 +18,6 @@
ethernet0 = _port1;
remoteproc0 = 
remoteproc1 = _0;
-   serial2 = _uart0;
spi0 = 
spi1 = 
usb0 = 
@@ -103,13 +102,6 @@
bootph-pre-ram;
 };
 
-_uart0 {
-   pinctrl-names = "default";
-   pinctrl-0 = <_uart0_pins_default>;
-   power-domains = <_pds 146 TI_SCI_PD_SHARED>;
-   status = "okay";
-};
-
 _vtm0 {
compatible = "ti,am654-vtm", "ti,am654-avs";
vdd-supply-3 = <_mpu>;
@@ -148,15 +140,6 @@
 };
 
 _pmx0 {
-   main_uart0_pins_default: main-uart0-pins-default {
-   pinctrl-single,pins = <
-   AM65X_IOPAD(0x01e4, PIN_INPUT, 0)   /* (AF11) 
UART0_RXD */
-   AM65X_IOPAD(0x01e8, PIN_OUTPUT, 0)  /* (AE11) 
UART0_TXD */
-   AM65X_IOPAD(0x01ec, PIN_INPUT, 0)   /* (AG11) 
UART0_CTSn */
-   AM65X_IOPAD(0x01f0, PIN_OUTPUT, 0)  /* (AD11) 
UART0_RTSn */
-   >;
-   };
-
main_mmc0_pins_default: main_mmc0_pins_default {
pinctrl-single,pins = <
AM65X_IOPAD(0x01a8, PIN_INPUT_PULLDOWN, 0)  /* 
(B25) MMC0_CLK */
-- 
2.43.0



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

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have a
duplicate sdhci1 pinmux node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index f28245b12b6f6..9d7467acd30c9 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -140,19 +140,6 @@
 };
 
 _pmx0 {
-   main_mmc1_pins_default: main_mmc1_pins_default {
-   pinctrl-single,pins = <
-   AM65X_IOPAD(0x02d4, PIN_INPUT_PULLDOWN, 0)  /* 
(C27) MMC1_CLK */
-   AM65X_IOPAD(0x02d8, PIN_INPUT_PULLUP, 0)/* 
(C28) MMC1_CMD */
-   AM65X_IOPAD(0x02d0, PIN_INPUT_PULLUP, 0)/* 
(D28) MMC1_DAT0 */
-   AM65X_IOPAD(0x02cc, PIN_INPUT_PULLUP, 0)/* 
(E27) MMC1_DAT1 */
-   AM65X_IOPAD(0x02c8, PIN_INPUT_PULLUP, 0)/* 
(D26) MMC1_DAT2 */
-   AM65X_IOPAD(0x02c4, PIN_INPUT_PULLUP, 0)/* 
(D27) MMC1_DAT3 */
-   AM65X_IOPAD(0x02dc, PIN_INPUT_PULLUP, 0)/* 
(B24) MMC1_SDCD */
-   AM65X_IOPAD(0x02e0, PIN_INPUT, 0)   /* 
(C24) MMC1_SDWP */
-   >;
-   };
-
usb0_pins_default: usb0_pins_default {
pinctrl-single,pins = <
AM65X_IOPAD(0x02bc, PIN_OUTPUT, 0) /* (AD9) 
USB0_DRVVBUS */
@@ -180,12 +167,18 @@
/delete-property/ power-domains;
 };
 
+/*
+ * MMC is probed to pull in firmware, so any clock
+ * or power-domain operation will fail as we do not
+ * have the firmware running at this point. Delete the
+ * power-domain properties to avoid making calls to
+ * SYSFW before it is loaded. Public ROM has already
+ * set it up for us anyway.
+ */
  {
clock-names = "clk_xin";
clocks = <_200mhz>;
-   pinctrl-0 = <_mmc1_pins_default>;
/delete-property/ power-domains;
-   ti,driver-strength-ohm = <50>;
 };
 
 _i2c0 {
-- 
2.43.0



[PATCH v2 19/26] arm: dts: k3-am654: remove usb0

2023-12-29 Thread Bryan Brattlof
The pinmux for usb0 is missing from the Linux board dtb file. Remove it
until we can introduce it in Linux

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi | 12 
 arch/arm/dts/k3-am654-r5-base-board.dts  | 29 
 2 files changed, 41 deletions(-)

diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi 
b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
index a24cb895e5578..3647088c29e4b 100644
--- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
@@ -125,10 +125,6 @@
bootph-pre-ram;
 };
 
-_pins_default {
-   bootph-pre-ram;
-};
-
 _pmx1 {
bootph-pre-ram;
 };
@@ -161,14 +157,6 @@
bootph-pre-ram;
 };
 
-_phy {
-   bootph-pre-ram;
-};
-
- {
-   bootph-pre-ram;
-};
-
 _conf {
bootph-pre-ram;
 };
diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index da41971a78f81..8e57c77cba923 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -18,8 +18,6 @@
ethernet0 = _port1;
remoteproc0 = 
remoteproc1 = _0;
-   usb0 = 
-   usb1 = 
};
 
a53_0: a53@0 {
@@ -114,14 +112,6 @@
};
 };
 
-_pmx0 {
-   usb0_pins_default: usb0_pins_default {
-   pinctrl-single,pins = <
-   AM65X_IOPAD(0x02bc, PIN_OUTPUT, 0) /* (AD9) 
USB0_DRVVBUS */
-   >;
-   };
-};
-
  {
vtt-supply = <_supply>;
pinctrl-names = "default";
@@ -161,14 +151,6 @@
  <0x0 0x5000 0x0 0x800>;
 };
 
-_pmx0 {
-   usb0_pins_default: usb0_pins_default {
-   pinctrl-single,pins = <
-   AM65X_IOPAD(0x02bc, PIN_OUTPUT, 0) /* (AD9) 
USB0_DRVVBUS */
-   >;
-   };
-};
-
 _0 {
status = "okay";
/delete-property/ clocks;
@@ -177,17 +159,6 @@
/delete-property/ assigned-clock-parents;
 };
 
-_phy {
-   status = "okay";
-   /delete-property/ clocks;
-};
-
- {
-   pinctrl-names = "default";
-   pinctrl-0 = <_pins_default>;
-   dr_mode = "peripheral";
-};
-
 _mdio {
phy0: ethernet-phy@0 {
reg = <0>;
-- 
2.43.0



[PATCH v2 06/26] arm: dts: k3-am654: include a53 board dtb for r5 build

2023-12-29 Thread Bryan Brattlof
To make things as organized as possible, start from the Linux board dtbs
and apply all properties needed for U-Boot in our *-u-boot.dtsi file for
the MAIN SPL and U-Boot builds.

We can then include these files for the WKUP SPL build making further
edits to the needed properties and nodes for the WKUP SPL bootloader's
view of the am65x.

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 8f55dab508ee6..79ca6387c22bb 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -5,7 +5,8 @@
 
 /dts-v1/;
 
-#include "k3-am654.dtsi"
+#include "k3-am654-base-board.dts"
+#include "k3-am654-base-board-u-boot.dtsi"
 #include "k3-am654-base-board-ddr4-1600MTs.dtsi"
 #include "k3-am654-ddr.dtsi"
 
-- 
2.43.0



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

2023-12-29 Thread Bryan Brattlof
Ethernet is one of a few IPs in U-Boot that depend on DMA to operate.
However there are a few missing registers ranges in the udmap nodes
need to properly setup DMA for the am65x.

A fix has been added to the Linux kernel[0] to add these ranges however
they have not made it to a Linux tag. To keep DMA operational until the
next DT sync from Linux, add these ranges to the *-u-boot.dtsi with a
note for our future selves.

[0] https://lore.kernel.org/r/20231213135138.929517-2-vigne...@ti.com

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi | 34 
 1 file changed, 34 insertions(+)

diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi 
b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
index a008af5b4a047..a24cb895e5578 100644
--- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
@@ -252,3 +252,37 @@
 _r5fss0 {
ti,cluster-mode = <0>;
 };
+
+/*
+ * The DMA driver requires a few extra register ranges
+ * which are missing for the am65x. A patch has been
+ * sent and will be synced after the v6.8-rc1 linux
+ * tag is published
+ */
+_udmap {
+   reg = <0x0 0x3115 0x0 0x100>,
+ <0x0 0x3400 0x0 0x10>,
+ <0x0 0x3500 0x0 0x10>,
+ <0x0 0x30b0 0x0 0x1>,
+ <0x0 0x30c0 0x0 0x1>,
+ <0x0 0x30d0 0x0 0x8000>;
+   reg-names = "gcfg", "rchanrt", "tchanrt",
+   "tchan", "rchan", "rflow";
+};
+
+/*
+ * The DMA driver requires a few extra register ranges
+ * which are missing for the am65x. A patch has been
+ * sent and will be synced after the v6.8-rc1 linux
+ * tag is published
+ */
+_udmap {
+   reg = <0x0 0x285c 0x0 0x100>,
+ <0x0 0x2a80 0x0 0x4>,
+ <0x0 0x2aa0 0x0 0x4>,
+ <0x0 0x284a 0x0 0x4000>,
+ <0x0 0x284c 0x0 0x4000>,
+ <0x0 0x2840 0x0 0x2000>;
+   reg-names = "gcfg", "rchanrt", "tchanrt",
+   "tchan", "rchan", "rflow";
+};
-- 
2.43.0



[PATCH v2 10/26] arm: dts: k3-am654: remove duplicate mcu_ringacc

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have a
duplicate mcu_ringacc node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 10 --
 1 file changed, 10 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index f462262b9aaac..c4fa84a3d0b05 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -54,16 +54,6 @@
};
 
mcu_navss: bus@2838 {
-   ringacc@2b80 {
-   reg =   <0x0 0x2b80 0x0 0x40>,
-   <0x0 0x2b00 0x0 0x40>,
-   <0x0 0x2859 0x0 0x100>,
-   <0x0 0x2a50 0x0 0x4>,
-   <0x0 0x2844 0x0 0x4>;
-   reg-names = "rt", "fifos", "proxy_gcfg", 
"proxy_target", "cfg";
-   ti,dma-ring-reset-quirk;
-   };
-
dma-controller@285c {
reg =   <0x0 0x285c 0x0 0x100>,
<0x0 0x284c 0x0 0x4000>,
-- 
2.43.0



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

2023-12-29 Thread Bryan Brattlof
Many nodes are reused between WKUP SPL, MAIN SPL, and U-Boot. Using
bootph-pre-ram is causing these nodes to be present in SPL builds but
pruned away during the U-Boot build. Convert these nodes to bootph-all
so they will remain no matter which dtb build is happening.

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi | 74 ++--
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi 
b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
index da6d4dffbf9d2..4fd188fa191b4 100644
--- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
@@ -18,151 +18,151 @@
 };
 
 _supply {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _main {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _navss {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _mcu {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _navss {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _ringacc {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _udmap {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _gpio0 {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _proxy_main {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _wakeup {
-   bootph-pre-ram;
+   bootph-all;
 
chipid@4314 {
-   bootph-pre-ram;
+   bootph-all;
};
 };
 
  {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _pds {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _clks {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _reset {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _uart0 {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _vtm0 {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _pmx0 {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _uart0_pins_default {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _vtt_pins_default {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _uart0_pins_default {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _i2c0_pins_default {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _fss0_ospi0_pins_default {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _pmx0 {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _uart0_pins_default {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _mmc0_pins_default {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _mmc1_pins_default {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _pmx1 {
-   bootph-pre-ram;
+   bootph-all;
 };
 
  {
-   bootph-pre-ram;
+   bootph-all;
 };
 
  {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _i2c0 {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _mpu {
-   bootph-pre-ram;
+   bootph-all;
 };
 
  {
-   bootph-pre-ram;
+   bootph-all;
 
flash@0 {
-   bootph-pre-ram;
+   bootph-all;
};
 };
 
 _0 {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _conf {
-   bootph-pre-ram;
+   bootph-all;
 };
 
  {
-   bootph-pre-ram;
+   bootph-all;
 };
 
 _0 {
-- 
2.43.0



[PATCH v2 08/26] arm: dts: k3-am654: remove duplicate wkup_uart0

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board files unified, we now have a duplicate
wkup_uart0 node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 12 
 1 file changed, 12 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 50e6be6118294..fb13a17b1dc64 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -18,7 +18,6 @@
ethernet0 = _port1;
remoteproc0 = 
remoteproc1 = _0;
-   serial0 = _uart0;
serial1 = _uart0;
serial2 = _uart0;
spi0 = 
@@ -117,8 +116,6 @@
 };
 
 _uart0 {
-   pinctrl-names = "default";
-   pinctrl-0 = <_uart0_pins_default>;
status = "okay";
bootph-pre-ram;
 };
@@ -146,15 +143,6 @@
 };
 
 _pmx0 {
-   wkup_uart0_pins_default: wkup_uart0_pins_default {
-   pinctrl-single,pins = <
-   AM65X_WKUP_IOPAD(0x00a0, PIN_INPUT, 0)  /* (AB1) 
WKUP_UART0_RXD */
-   AM65X_WKUP_IOPAD(0x00a4, PIN_OUTPUT, 0) /* (AB5) 
WKUP_UART0_TXD */
-   AM65X_WKUP_IOPAD(0x00c8, PIN_INPUT, 1)  /* (AC2) 
WKUP_GPIO0_6.WKUP_UART0_CTSn */
-   AM65X_WKUP_IOPAD(0x00cc, PIN_OUTPUT, 1) /* (AC1) 
WKUP_GPIO0_7.WKUP_UART0_RTSn */
-   >;
-   };
-
wkup_vtt_pins_default: wkup_vtt_pins_default {
pinctrl-single,pins = <
AM65X_WKUP_IOPAD(0x0040, PIN_OUTPUT_PULLUP, 7)  /* 
WKUP_GPIO0_28 */
-- 
2.43.0



[PATCH v2 20/26] arm: dts: k3-am654: remove duplicate mdio

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have a
duplicate mdio node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 9 -
 1 file changed, 9 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 8e57c77cba923..14bf1b4c8a462 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -159,15 +159,6 @@
/delete-property/ assigned-clock-parents;
 };
 
-_mdio {
-   phy0: ethernet-phy@0 {
-   reg = <0>;
-   /* TODO: phy reset: 
TCA9555RTWR(i2c:0x21)[p04].GPIO_MCU_RGMII_RSTN */
-   ti,rx-internal-delay = ;
-   ti,fifo-depth = ;
-   };
-};
-
 _cpsw {
reg = <0x0 0x4600 0x0 0x20>,
  <0x0 0x40f00200 0x0 0x2>;
-- 
2.43.0



[PATCH v2 11/26] arm: dts: k3-am654: remove duplicate mcu_udmap

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have a
duplicate mcu_udmap node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 13 -
 1 file changed, 13 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index c4fa84a3d0b05..19806ec4e2309 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -52,19 +52,6 @@
#mbox-cells = <1>;
bootph-pre-ram;
};
-
-   mcu_navss: bus@2838 {
-   dma-controller@285c {
-   reg =   <0x0 0x285c 0x0 0x100>,
-   <0x0 0x284c 0x0 0x4000>,
-   <0x0 0x2a80 0x0 0x4>,
-   <0x0 0x284a 0x0 0x4000>,
-   <0x0 0x2aa0 0x0 0x4>,
-   <0x0 0x2840 0x0 0x2000>;
-   reg-names = "gcfg", "rchan", "rchanrt", "tchan",
-   "tchanrt", "rflow";
-   };
-   };
 };
 
 _wakeup {
-- 
2.43.0



[PATCH v2 22/26] arm: dts: k3-am654: remove duplicate root properties

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we have duplicate
properties in the root node. Remove them

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index fb238adaea5b2..c039300c3cbb3 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -11,9 +11,6 @@
 #include "k3-am654-ddr.dtsi"
 
 / {
-   compatible =  "ti,am654-evm", "ti,am654";
-   model = "Texas Instruments AM654 R5 Base Board";
-
aliases {
ethernet0 = _port1;
remoteproc0 = 
-- 
2.43.0



[PATCH v2 23/26] arm: dts: k3-am654: remove un-needed aliases

2023-12-29 Thread Bryan Brattlof
These aliases are not needed in U-Boot. Remove them

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index c039300c3cbb3..9926981c661a0 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -12,7 +12,6 @@
 
 / {
aliases {
-   ethernet0 = _port1;
remoteproc0 = 
remoteproc1 = _0;
};
-- 
2.43.0



[PATCH v2 01/26] configs: am65x_evm_r5: enable driver for fixed regulators

2023-12-29 Thread Bryan Brattlof
Some of the regulators we need to successfully boot are fixed
regulators. Enable the driver to properly probe them.

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 configs/am65x_evm_r5_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/am65x_evm_r5_defconfig b/configs/am65x_evm_r5_defconfig
index b2f1e721b36d0..b892f2eb8c5f9 100644
--- a/configs/am65x_evm_r5_defconfig
+++ b/configs/am65x_evm_r5_defconfig
@@ -118,6 +118,8 @@ CONFIG_POWER_DOMAIN=y
 CONFIG_TI_SCI_POWER_DOMAIN=y
 CONFIG_DM_REGULATOR=y
 CONFIG_SPL_DM_REGULATOR=y
+CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_SPL_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_SPL_DM_REGULATOR_GPIO=y
 CONFIG_DM_REGULATOR_TPS62360=y
-- 
2.43.0



[PATCH v2 17/26] arm: dts: k3-am654: remove duplicate wkup_i2c0

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have a
duplicate wkup_i2c0 node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 25 -
 1 file changed, 25 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 9d7467acd30c9..8c6cb147c821a 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -115,13 +115,6 @@
>;
};
 
-   wkup_i2c0_pins_default: wkup-i2c0-pins-default {
-   pinctrl-single,pins = <
-   AM65X_WKUP_IOPAD(0x00e0, PIN_INPUT, 0) /* (AC7) 
WKUP_I2C0_SCL */
-   AM65X_WKUP_IOPAD(0x00e4, PIN_INPUT, 0) /* (AD6) 
WKUP_I2C0_SDA */
-   >;
-   };
-
mcu_fss0_ospi0_pins_default: mcu-fss0-ospi0-pins_default {
pinctrl-single,pins = <
AM65X_WKUP_IOPAD(0x, PIN_OUTPUT, 0) /* (V1) 
MCU_OSPI0_CLK */
@@ -181,24 +174,6 @@
/delete-property/ power-domains;
 };
 
-_i2c0 {
-   pinctrl-names = "default";
-   pinctrl-0 = <_i2c0_pins_default>;
-   clock-frequency = <40>;
-
-   vdd_mpu: tps62363@60 {
-   compatible = "ti,tps62363";
-   reg = <0x60>;
-   regulator-name = "VDD_MPU";
-   regulator-min-microvolt = <50>;
-   regulator-max-microvolt = <177>;
-   regulator-always-on;
-   regulator-boot-on;
-   ti,vsel0-state-high;
-   ti,vsel1-state-high;
-   };
-};
-
  {
pinctrl-names = "default";
pinctrl-0 = <_fss0_ospi0_pins_default>;
-- 
2.43.0



[PATCH v2 03/26] arm: dts: k3-am654-r5: Merge board file and U-Boot overlay

2023-12-29 Thread Bryan Brattlof
The R5 board file for U-Boot should be the same as the board file copied
from Linux with a few alterations to work with the R5's view of the SoC.

First we need to unify the R5 board file and it's U-Boot overlay before
we can unify the Linux board file with this one.

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi  |   1 -
 .../dts/k3-am654-r5-base-board-u-boot.dtsi| 208 --
 arch/arm/dts/k3-am654-r5-base-board.dts   | 124 ++-
 3 files changed, 119 insertions(+), 214 deletions(-)
 delete mode 100644 arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi

diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi 
b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
index 11d83927ac525..f29cecf870bcd 100644
--- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
@@ -3,7 +3,6 @@
  * Copyright (C) 2018-2021 Texas Instruments Incorporated - https://www.ti.com/
  */
 
-#include "k3-am654-r5-base-board-u-boot.dtsi"
 #include "k3-am65x-binman.dtsi"
 
 _0 {
diff --git a/arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi 
b/arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi
deleted file mode 100644
index 286604576e028..0
--- a/arch/arm/dts/k3-am654-r5-base-board-u-boot.dtsi
+++ /dev/null
@@ -1,208 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2018-2021 Texas Instruments Incorporated - https://www.ti.com/
- */
-
-#include 
-#include 
-#include "k3-am65x-binman.dtsi"
-
-/ {
-   chosen {
-   stdout-path = "serial2:115200n8";
-   };
-
-   aliases {
-   serial2 = _uart0;
-   ethernet0 = _port1;
-   usb0 = 
-   usb1 = 
-   spi0 = 
-   spi1 = 
-   };
-};
-
-_main{
-   bootph-pre-ram;
-   main_navss: bus@3080 {
-   bootph-pre-ram;
-   };
-};
-
-_mcu {
-   bootph-pre-ram;
-
-   mcu_navss: bus@2838 {
-   bootph-pre-ram;
-
-   ringacc@2b80 {
-   reg =   <0x0 0x2b80 0x0 0x40>,
-   <0x0 0x2b00 0x0 0x40>,
-   <0x0 0x2859 0x0 0x100>,
-   <0x0 0x2a50 0x0 0x4>,
-   <0x0 0x2844 0x0 0x4>;
-   reg-names = "rt", "fifos", "proxy_gcfg", 
"proxy_target", "cfg";
-   bootph-pre-ram;
-   ti,dma-ring-reset-quirk;
-   };
-
-   dma-controller@285c {
-   reg =   <0x0 0x285c 0x0 0x100>,
-   <0x0 0x284c 0x0 0x4000>,
-   <0x0 0x2a80 0x0 0x4>,
-   <0x0 0x284a 0x0 0x4000>,
-   <0x0 0x2aa0 0x0 0x4>,
-   <0x0 0x2840 0x0 0x2000>;
-   reg-names = "gcfg", "rchan", "rchanrt", "tchan",
-   "tchanrt", "rflow";
-   bootph-pre-ram;
-   };
-   };
-};
-
-_wakeup {
-   bootph-pre-ram;
-
-   chipid@4314 {
-   bootph-pre-ram;
-   };
-};
-
-_proxy_main {
-   bootph-pre-ram;
-};
-
- {
-   bootph-pre-ram;
-   k3_sysreset: sysreset-controller {
-   compatible = "ti,sci-sysreset";
-   bootph-pre-ram;
-   };
-};
-
-_pds {
-   bootph-pre-ram;
-};
-
-_clks {
-   bootph-pre-ram;
-};
-
-_reset {
-   bootph-pre-ram;
-};
-
-_pmx0 {
-   bootph-pre-ram;
-
-   wkup_i2c0_pins_default {
-   bootph-pre-ram;
-   };
-};
-
-_pmx0 {
-   bootph-pre-ram;
-   usb0_pins_default: usb0_pins_default {
-   pinctrl-single,pins = <
-   AM65X_IOPAD(0x02bc, PIN_OUTPUT, 0) /* (AD9) 
USB0_DRVVBUS */
-   >;
-   bootph-pre-ram;
-   };
-};
-
-_uart0_pins_default {
-   bootph-pre-ram;
-};
-
-_pmx1 {
-   bootph-pre-ram;
-};
-
-_pmx0 {
-   mcu-fss0-ospi0-pins-default {
-   bootph-pre-ram;
-   };
-};
-
-_uart0 {
-   bootph-pre-ram;
-};
-
-_mmc0_pins_default {
-   bootph-pre-ram;
-};
-
-_mmc1_pins_default {
-   bootph-pre-ram;
-};
-
- {
-   bootph-pre-ram;
-};
-
- {
-   bootph-pre-ram;
-};
-
-_mdio {
-   phy0: ethernet-phy@0 {
-   reg = <0>;
-   /* TODO: phy reset: 
TCA9555RTWR(i2c:0x21)[p04].GPIO_MCU_RGMII_RSTN */
-   ti,rx-internal-delay = ;
-   ti,fifo-depth = ;
-   };
-};
-
-_cpsw {
-   reg = <0x0 0x4600 0x0 0x20>,
- <0x0 0x40f00200 0x0 0x2>;
-   reg-names = "cpsw_nuss", "mac_efuse";
-   /delete-property/ ranges;
-
-   cpsw-phy-sel@40f04040 {
-   compatible = "ti,am654-cpsw-phy-sel";
-   reg= <0x0 0x40f04040 0x0 0x4>;
-   

[PATCH v2 21/26] arm: dts: k3-am654: remove duplicate vtt pinmux

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have a
duplicate vtt_pinmux node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-base-board-u-boot.dtsi |  2 +-
 arch/arm/dts/k3-am654-r5-base-board.dts  | 10 --
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi 
b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
index 3647088c29e4b..da6d4dffbf9d2 100644
--- a/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
+++ b/arch/arm/dts/k3-am654-base-board-u-boot.dtsi
@@ -93,7 +93,7 @@
bootph-pre-ram;
 };
 
-_vtt_pins_default {
+_vtt_pins_default {
bootph-pre-ram;
 };
 
diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 14bf1b4c8a462..fb238adaea5b2 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -104,18 +104,8 @@
vdd-supply-4 = <_mpu>;
 };
 
-_pmx0 {
-   wkup_vtt_pins_default: wkup_vtt_pins_default {
-   pinctrl-single,pins = <
-   AM65X_WKUP_IOPAD(0x0040, PIN_OUTPUT_PULLUP, 7)  /* 
WKUP_GPIO0_28 */
-   >;
-   };
-};
-
  {
vtt-supply = <_supply>;
-   pinctrl-names = "default";
-   pinctrl-0 = <_vtt_pins_default>;
 };
 
 /*
-- 
2.43.0



[PATCH v2 02/26] configs: am65x_evm_a53: disable CONSOLE_MUX

2023-12-29 Thread Bryan Brattlof
We do not have a need to share a single console with the evaluation
board and disabling this option reduces the complexity of configuring
the consoles. Disable CONSOLE_MUX

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 configs/am65x_evm_a53_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configs/am65x_evm_a53_defconfig b/configs/am65x_evm_a53_defconfig
index f4369865bf665..4d95e3bd29630 100644
--- a/configs/am65x_evm_a53_defconfig
+++ b/configs/am65x_evm_a53_defconfig
@@ -36,7 +36,6 @@ CONFIG_DISTRO_DEFAULTS=y
 CONFIG_OF_SYSTEM_SETUP=y
 CONFIG_BOOTCOMMAND="run findfdt; run distro_bootcmd; run init_${boot}; run 
boot_rprocs; if test ${boot_fit} -eq 1; then run get_fit_${boot}; run 
get_overlaystring; run run_fit; else; run get_kern_${boot}; run 
get_fdt_${boot}; run get_overlay_${boot}; run run_kern; fi;"
 CONFIG_LOGLEVEL=7
-CONFIG_CONSOLE_MUX=y
 CONFIG_SPL_MAX_SIZE=0x58000
 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
 CONFIG_SPL_BSS_START_ADDR=0x80a0
-- 
2.43.0



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

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified, we now have
a duplicate sdhci0 pinmux node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 26 -
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 3d599a5413812..f28245b12b6f6 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -140,22 +140,6 @@
 };
 
 _pmx0 {
-   main_mmc0_pins_default: main_mmc0_pins_default {
-   pinctrl-single,pins = <
-   AM65X_IOPAD(0x01a8, PIN_INPUT_PULLDOWN, 0)  /* 
(B25) MMC0_CLK */
-   AM65X_IOPAD(0x01aC, PIN_INPUT_PULLUP, 0)/* 
(B27) MMC0_CMD */
-   AM65X_IOPAD(0x01a4, PIN_INPUT_PULLUP, 0)/* 
(A26) MMC0_DAT0 */
-   AM65X_IOPAD(0x01a0, PIN_INPUT_PULLUP, 0)/* 
(E25) MMC0_DAT1 */
-   AM65X_IOPAD(0x019c, PIN_INPUT_PULLUP, 0)/* 
(C26) MMC0_DAT2 */
-   AM65X_IOPAD(0x0198, PIN_INPUT_PULLUP, 0)/* 
(A25) MMC0_DAT3 */
-   AM65X_IOPAD(0x0194, PIN_INPUT_PULLUP, 0)/* 
(E24) MMC0_DAT4 */
-   AM65X_IOPAD(0x0190, PIN_INPUT_PULLUP, 0)/* 
(A24) MMC0_DAT5 */
-   AM65X_IOPAD(0x018c, PIN_INPUT_PULLUP, 0)/* 
(B26) MMC0_DAT6 */
-   AM65X_IOPAD(0x0188, PIN_INPUT_PULLUP, 0)/* 
(D25) MMC0_DAT7 */
-   AM65X_IOPAD(0x01b0, PIN_INPUT, 0)   /* 
(C25) MMC0_DS */
-   >;
-   };
-
main_mmc1_pins_default: main_mmc1_pins_default {
pinctrl-single,pins = <
AM65X_IOPAD(0x02d4, PIN_INPUT_PULLDOWN, 0)  /* 
(C27) MMC1_CLK */
@@ -182,12 +166,18 @@
pinctrl-0 = <_vtt_pins_default>;
 };
 
+/*
+ * MMC is probed to pull in firmware, so any clock
+ * or power-domain operation will fail as we do not
+ * have the firmware running at this point. Delete the
+ * power-domain properties to avoid making calls to
+ * SYSFW before it is loaded. Public ROM has already
+ * set it up for us anyway.
+ */
  {
clock-names = "clk_xin";
clocks = <_200mhz>;
-   pinctrl-0 = <_mmc0_pins_default>;
/delete-property/ power-domains;
-   ti,driver-strength-ohm = <50>;
 };
 
  {
-- 
2.43.0



[PATCH v2 07/26] arm: dts: k3-am654: remove duplicate vtt_supply

2023-12-29 Thread Bryan Brattlof
With the Linux and U-Boot board dtb files unified we now have a
duplicate vtt_supply node. Remove it

Tested-by: Tom Rini 
Signed-off-by: Bryan Brattlof 
---
 arch/arm/dts/k3-am654-r5-base-board.dts | 9 -
 1 file changed, 9 deletions(-)

diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
b/arch/arm/dts/k3-am654-r5-base-board.dts
index 79ca6387c22bb..50e6be6118294 100644
--- a/arch/arm/dts/k3-am654-r5-base-board.dts
+++ b/arch/arm/dts/k3-am654-r5-base-board.dts
@@ -46,15 +46,6 @@
ti,sci-host-id = <10>;
bootph-pre-ram;
};
-
-   vtt_supply: vtt_supply {
-   compatible = "regulator-gpio";
-   regulator-name = "vtt";
-   regulator-min-microvolt = <0>;
-   regulator-max-microvolt = <330>;
-   gpios = <_gpio0 28 GPIO_ACTIVE_HIGH>;
-   states = <0 0x0 330 0x1>;
-   };
 };
 
 _main {
-- 
2.43.0



Re: Proposal: U-Boot memory management

2023-12-29 Thread Mark Kettenis
> Date: Fri, 29 Dec 2023 11:17:44 -0500
> From: Tom Rini 
> 
> On Fri, Dec 29, 2023 at 05:05:17PM +0100, Heinrich Schuchardt wrote:
> > 
> > 
> > Am 29. Dezember 2023 16:43:07 MEZ schrieb Tom Rini :
> > >On Fri, Dec 29, 2023 at 05:36:09AM +, Simon Glass wrote:
> > >> Hi,
> > >> 
> > >> On Sat, Dec 16, 2023 at 6:01 PM Simon Glass  wrote:
> > >> >
> > >> > Hi,
> > >> >
> > >> > This records my thoughts after a discussion with Ilias & Heinrich re
> > >> > memory allocation in U-Boot.
> > >> >
> > >> > 1. malloc()
> > >> >
> > >> > malloc() is used for programmatic memory allocation. It allows memory
> > >> > to be freed. It is not designed for very large allocations (e.g. a
> > >> > 10MB kernel or 100MB ramdisk).
> > >> >
> > >> > 2. lmb
> > >> >
> > >> > lmb is used for large blocks of memory, such as those needed for a
> > >> > kernel or ramdisk. Allocation is only transitory, for the purposes of
> > >> > loading some images and booting. If the boot fails, then all lmb
> > >> > allocations go away.
> > >> >
> > >> > lmb is set up by getting all available memory and then removing what
> > >> > is used by U-Boot (code, data, malloc() space, etc.)
> > >> >
> > >> > lmb reservations have a few flags so that areas of memory can be
> > >> > provided with attributes
> > >> >
> > >> > There are some corner cases...e.g. loading a file does an lmb
> > >> > allocation but only for the purpose of avoiding a file being loaded
> > >> > over U-Boot code/data. The allocation is dropped immediately after the
> > >> > file is loaded. Within the bootm command, or when using standard boot,
> > >> > this would be fairly easy to solve.
> > >> >
> > >> > Linux has renamed lmb to memblock. We should consider doing the same.
> > >> >
> > >> > 3. EFI
> > >> >
> > >> > EFI has its own memory-allocation tables.
> > >> >
> > >> > Like lmb, EFI is able to deal with large allocations. But via a 'pool'
> > >> > function it can also do smaller allocations similar to malloc(),
> > >> > although each one uses at least 4KB at present.
> > >> >
> > >> > EFI allocations do not go away when a boot fails.
> > >> >
> > >> > With EFI it is possible to add allocations post facto, in which case
> > >> > they are added to the allocation table just as if the memory was
> > >> > allocated with EFI to begin with.
> > >> >
> > >> > The EFI allocations and the lmb allocations use the same memory, so in
> > >> > principle could conflict.
> > >> >
> > >> > EFI allocations are sometimes used to allocate internal U-Boot data as
> > >> > well, if needed by the EFI app. For example, while efi_image_parse()
> > >> > uses malloc(), efi_var_mem.c uses EFI allocations since the code runs
> > >> > in the app context and may need to access the memory after U-Boot has
> > >> > exited. Also efi_smbios.c uses allocate_pages() and then adds a new
> > >> > mapping as well.
> > >> >
> > >> > EFI memory has attributes, including what the memory is used for (to
> > >> > some degree of granularity). See enum efi_memory_type and struct
> > >> > efi_mem_desc. In the latter there are also attribute flags - whether
> > >> > memory is cacheable, etc.
> > >> >
> > >> > EFI also has the x86 idea of 'conventional' memory, meaning (I
> > >> > believe) that below 4GB that isn't reserved for the hardware/system.
> > >> > This is meaningless, or at least confusing, on ARM systems.
> > >> >
> > >> > 4. reservations
> > >> >
> > >> > It is perhaps worth mentioning a fourth method of memory management,
> > >> > where U-Boot reserves chunks of memory before relocation (in
> > >> > board_init_f.c), e.g. for the framebuffer, U-Boot code, the malloc()
> > >> > region, etc.
> > >> >
> > >> >
> > >> > Problems
> > >> > —---
> > >> >
> > >> > There are no urgent problems, but here are some things that could be 
> > >> > improved:
> > >> >
> > >> > 1. EFI should attach most of its data structures to driver model. This
> > >> > work has started, with the partition support, but more effort would
> > >> > help. This would make it easier to see which memory is related to
> > >> > devices and which is separate.
> > >> >
> > >> > 2. Some drivers do EFI reservations today, whether EFI is used for
> > >> > booting or not (e.g. rockchip video rk_vop_probe()).
> > >> >
> > >> > 3. U-Boot doesn't really map arch-specific memory attributes (e.g.
> > >> > armv8's struct mm_region) to EFI ones.
> > >> >
> > >> > 4. EFI duplicates some code from bootm, some of which relates to
> > >> > memory allocation (e.g. FDT fixup).
> > >> >
> > >> > 5. EFI code is used even if EFI is never used to boot
> > >> >
> > >> > 6. EFI allocations can result in the same memory being used as has
> > >> > already been allocated by lmb. Users may load files which overwrite
> > >> > memory allocated by EFI.
> > >> 
> > >> 7. We need to support doing an allocation when a file is loaded (to
> > >> ensure files do not overlap), without making it too difficult to load
> > >> multiple files to the same place, if desired.
> > >> 
> > 

[PATCH v5 11/11] bloblist: Update documentation and header comment

2023-12-29 Thread Raymond Mao
From: Simon Glass 

Align the documentation with the v0.9 spec.

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

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



[PATCH v5 10/11] bloblist: Add alignment to bloblist_new()

2023-12-29 Thread Raymond Mao
From: Simon Glass 

Allow the alignment to be specified when creating a bloblist.

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

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

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

[PATCH v5 09/11] bloblist: Adjust the bloblist header

2023-12-29 Thread Raymond Mao
From: Simon Glass 

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

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

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

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

[PATCH v5 08/11] bloblist: Reduce blob-header size

2023-12-29 Thread Raymond Mao
From: Simon Glass 

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

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

 common/bloblist.c  | 24 +++-
 include/bloblist.h | 34 ++
 test/bloblist.c| 16 
 3 files changed, 45 insertions(+), 29 deletions(-)

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

[PATCH v5 07/11] bloblist: Handle alignment with a void entry

2023-12-29 Thread Raymond Mao
From: Simon Glass 

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

Signed-off-by: Simon Glass 
Co-developed-by: Raymond Mao 
Signed-off-by: Raymond Mao 
Reviewed-by: Simon Glass 
Reviewed-by: Ilias Apalodimas 
---
Changes in v5
- Optimize how to calculate the new allocated size.

 common/bloblist.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

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



[PATCH v5 06/11] bloblist: Checksum the entire bloblist

2023-12-29 Thread Raymond Mao
From: Simon Glass 

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

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

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

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



[PATCH v5 05/11] bloblist: Access record hdr_size and tag via a function

2023-12-29 Thread Raymond Mao
From: Simon Glass 

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

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

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

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



[PATCH v5 04/11] bloblist: Set version to 1

2023-12-29 Thread Raymond Mao
From: Simon Glass 

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

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

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



[PATCH v5 03/11] bloblist: Change the magic value

2023-12-29 Thread Raymond Mao
From: Simon Glass 

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

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

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

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



[PATCH v5 02/11] bloblist: Adjust API to align in powers of 2

2023-12-29 Thread Raymond Mao
From: Simon Glass 

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

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

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

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

[PATCH v5 01/11] bloblist: Update the tag numbering

2023-12-29 Thread Raymond Mao
From: Simon Glass 

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

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

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

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

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

[PATCH v5 00/11] Support Firmware Handoff spec via bloblist

2023-12-29 Thread Raymond Mao
Major changes:

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

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

v3: The implementation from boot arguments to bloblist and how to
load the FDT from the bloblist are moved to a forthcoming patch serie.

v4: Patch #7 and #14 from v3 are squashed to other patches.

v5: Patch #6 from v4 is squashed to #8

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

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

-- 
2.25.1



Re: Proposal: U-Boot memory management

2023-12-29 Thread Heinrich Schuchardt

On 12/29/23 18:21, Tom Rini wrote:

On Fri, Dec 29, 2023 at 06:09:44PM +0100, Heinrich Schuchardt wrote:

On 12/29/23 17:47, Tom Rini wrote:

On Fri, Dec 29, 2023 at 05:42:17PM +0100, Heinrich Schuchardt wrote:

On 12/20/23 20:12, Tom Rini wrote:

On Tue, Dec 19, 2023 at 09:15:21PM -0700, Simon Glass wrote:

Hi,

On Tue, 19 Dec 2023 at 05:46, Tom Rini  wrote:


On Tue, Dec 19, 2023 at 03:15:38AM +0100, Heinrich Schuchardt wrote:



Am 19. Dezember 2023 02:26:00 MEZ schrieb Tom Rini :

On Tue, Dec 19, 2023 at 01:01:51AM +0100, Heinrich Schuchardt wrote:



Am 19. Dezember 2023 00:31:30 MEZ schrieb Tom Rini :

On Tue, Dec 19, 2023 at 12:29:19AM +0100, Heinrich Schuchardt wrote:



Am 19. Dezember 2023 00:16:40 MEZ schrieb Tom Rini :

On Tue, Dec 19, 2023 at 12:08:31AM +0100, Heinrich Schuchardt wrote:



Am 18. Dezember 2023 23:41:08 MEZ schrieb Tom Rini :

On Mon, Dec 18, 2023 at 11:34:16PM +0100, Heinrich Schuchardt wrote:

[snip]

Or take:

load host 0:1 $c kernel.efi
load host 0:1 $d initrd.img

How could we ensure that initrd.img is not overwriting a part of kernel.efi 
without memory allocation?


Today, invalid checksum as part of some part of the kernel fails. But
how do we do this tomorrow, are you suggesting that "load" perform
malloc() in some predefined size? If $c is below $d and $c + kernel.efi
is now above $d we can throw an error before trying to load, yes. But
what about:
load host 0:1 $d initrd.img
load host 0:1 $c kernel.efi

In that case (which is only marginally contrived, the more real case is
loading device tree in to unexpectedly large ramdisk because someone
didn't understand the general advice on why device tree is lower than
ramdisk address) I'm fine with an error that amounts to "you just
corrupted another allocation" and then "fail, reset the board" or so.



Our current malloc library cannot manage the complete memory. We need a library 
like lmb which should also cover the memory management that we currently have 
in lib/efi/efi_memory.c. This must include a memory type attribute for usage in 
the GetMemoryMap() service. A management on page level seems sufficient.

The load command should permanently allocate memory in that lmb+ library.

We need an unload command to free the memory if we want to reuse the memory or 
we might let the load comand free the memory if exactly the same start address 
is reused.


Our current way of loading things in to memory does not handle the case
I described, yes. How would what you're proposing handle it?


If the load command has to allocate memory for the image and that allocation is 
kept, any attempt to allocate overlapping memory would fail.


So you're saying that the load command has to pre-allocate memory? Or as
it goes? If the latter, in what size chunks? This starts to get at what
Simon was talking about with respect to memory fragmentation. Which to
be clear is a problem we have today, we just let things overlap and hope
something later catches an incorrect checksum.



I don't want to replace the malloc library which handles large numbets of 
allocations.


I'm confused. The normal malloc library is not involved with current
image loading, it's direct to memory (with some attempts at sanity
checking by lmb).  Are you proposing a different allocator with
malloc/free like behavior? If so, please outline how it will determine
pool size, and how we'll use it to load thing to memory.


All memory below the stack needs to be managed. Malloc uses a small memory area 
(a few MiB) above the stack.


That's a rather huge change for how U-Boot works.


Closing the eyes when the user loads multiple files does not solve the 
fragmentation problem.


Yes. I'm only noting that today we just ignore the problem and sometimes
catch it via checksums.


Fragmentation only happens if we have many concurrent allocations.  In EFI we 
are allocating top down. The number of concurrent allocations is low. Typically 
a few dozen at most. After terminating an application these should be freed 
again.


OK, so are you saying that we would no longer be loading _to_ a location
in memory and instead just be saying "load this thing" and picking where
dynamically?


Both preassigned and allocator assigned adresses are compatible with memory 
management.

Architectures and binaries have different requirements. On riscv64 you can load 
Linux kernel, initrd, fdt anywhere. We don't need predefined addresses there. 
Other architectures have restrictions.


Yes, 64 bit architecture tend to only have alignment requirements while
32bit architectures have both alignment requirements and some memory
window requirement. Whatever we implement here needs to handle both
cases.


When loading a file from a file system we know the filesize beforehand. So 
allocation is trivial.

The loady command currently does not use the  offered size information but 
could do so.


We should be using that information to make sure we don't overwrite
U-Boot itself, but I don't recall how 

Re: Proposal: U-Boot memory management

2023-12-29 Thread Tom Rini
On Fri, Dec 29, 2023 at 06:09:44PM +0100, Heinrich Schuchardt wrote:
> On 12/29/23 17:47, Tom Rini wrote:
> > On Fri, Dec 29, 2023 at 05:42:17PM +0100, Heinrich Schuchardt wrote:
> > > On 12/20/23 20:12, Tom Rini wrote:
> > > > On Tue, Dec 19, 2023 at 09:15:21PM -0700, Simon Glass wrote:
> > > > > Hi,
> > > > > 
> > > > > On Tue, 19 Dec 2023 at 05:46, Tom Rini  wrote:
> > > > > > 
> > > > > > On Tue, Dec 19, 2023 at 03:15:38AM +0100, Heinrich Schuchardt wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > Am 19. Dezember 2023 02:26:00 MEZ schrieb Tom Rini 
> > > > > > > :
> > > > > > > > On Tue, Dec 19, 2023 at 01:01:51AM +0100, Heinrich Schuchardt 
> > > > > > > > wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Am 19. Dezember 2023 00:31:30 MEZ schrieb Tom Rini 
> > > > > > > > > :
> > > > > > > > > > On Tue, Dec 19, 2023 at 12:29:19AM +0100, Heinrich 
> > > > > > > > > > Schuchardt wrote:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Am 19. Dezember 2023 00:16:40 MEZ schrieb Tom Rini 
> > > > > > > > > > > :
> > > > > > > > > > > > On Tue, Dec 19, 2023 at 12:08:31AM +0100, Heinrich 
> > > > > > > > > > > > Schuchardt wrote:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Am 18. Dezember 2023 23:41:08 MEZ schrieb Tom Rini 
> > > > > > > > > > > > > :
> > > > > > > > > > > > > > On Mon, Dec 18, 2023 at 11:34:16PM +0100, Heinrich 
> > > > > > > > > > > > > > Schuchardt wrote:
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > [snip]
> > > > > > > > > > > > > > > Or take:
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > load host 0:1 $c kernel.efi
> > > > > > > > > > > > > > > load host 0:1 $d initrd.img
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > How could we ensure that initrd.img is not 
> > > > > > > > > > > > > > > overwriting a part of kernel.efi without memory 
> > > > > > > > > > > > > > > allocation?
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Today, invalid checksum as part of some part of the 
> > > > > > > > > > > > > > kernel fails. But
> > > > > > > > > > > > > > how do we do this tomorrow, are you suggesting that 
> > > > > > > > > > > > > > "load" perform
> > > > > > > > > > > > > > malloc() in some predefined size? If $c is below $d 
> > > > > > > > > > > > > > and $c + kernel.efi
> > > > > > > > > > > > > > is now above $d we can throw an error before trying 
> > > > > > > > > > > > > > to load, yes. But
> > > > > > > > > > > > > > what about:
> > > > > > > > > > > > > > load host 0:1 $d initrd.img
> > > > > > > > > > > > > > load host 0:1 $c kernel.efi
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > In that case (which is only marginally contrived, 
> > > > > > > > > > > > > > the more real case is
> > > > > > > > > > > > > > loading device tree in to unexpectedly large 
> > > > > > > > > > > > > > ramdisk because someone
> > > > > > > > > > > > > > didn't understand the general advice on why device 
> > > > > > > > > > > > > > tree is lower than
> > > > > > > > > > > > > > ramdisk address) I'm fine with an error that 
> > > > > > > > > > > > > > amounts to "you just
> > > > > > > > > > > > > > corrupted another allocation" and then "fail, reset 
> > > > > > > > > > > > > > the board" or so.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Our current malloc library cannot manage the complete 
> > > > > > > > > > > > > memory. We need a library like lmb which should also 
> > > > > > > > > > > > > cover the memory management that we currently have in 
> > > > > > > > > > > > > lib/efi/efi_memory.c. This must include a memory type 
> > > > > > > > > > > > > attribute for usage in the GetMemoryMap() service. A 
> > > > > > > > > > > > > management on page level seems sufficient.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > The load command should permanently allocate memory 
> > > > > > > > > > > > > in that lmb+ library.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > We need an unload command to free the memory if we 
> > > > > > > > > > > > > want to reuse the memory or we might let the load 
> > > > > > > > > > > > > comand free the memory if exactly the same start 
> > > > > > > > > > > > > address is reused.
> > > > > > > > > > > > 
> > > > > > > > > > > > Our current way of loading things in to memory does not 
> > > > > > > > > > > > handle the case
> > > > > > > > > > > > I described, yes. How would what you're proposing 
> > > > > > > > > > > > handle it?
> > > > > > > > > > > 
> > > > > > > > > > > If the load command has to allocate memory for the image 
> > > > > > > > > > > and that allocation is kept, any attempt to allocate 
> > > > > > > > > > > overlapping memory would fail.
> > > > > > > > > > 
> > > > > > > > > > So you're saying that the load command has to pre-allocate 
> > > > > > > > > > memory? Or as
> > > > > > > > > > it goes? If the latter, in what size chunks? This starts to 
> > > > > > > > > > 

Re: Proposal: U-Boot memory management

2023-12-29 Thread Heinrich Schuchardt

On 12/29/23 17:47, Tom Rini wrote:

On Fri, Dec 29, 2023 at 05:42:17PM +0100, Heinrich Schuchardt wrote:

On 12/20/23 20:12, Tom Rini wrote:

On Tue, Dec 19, 2023 at 09:15:21PM -0700, Simon Glass wrote:

Hi,

On Tue, 19 Dec 2023 at 05:46, Tom Rini  wrote:


On Tue, Dec 19, 2023 at 03:15:38AM +0100, Heinrich Schuchardt wrote:



Am 19. Dezember 2023 02:26:00 MEZ schrieb Tom Rini :

On Tue, Dec 19, 2023 at 01:01:51AM +0100, Heinrich Schuchardt wrote:



Am 19. Dezember 2023 00:31:30 MEZ schrieb Tom Rini :

On Tue, Dec 19, 2023 at 12:29:19AM +0100, Heinrich Schuchardt wrote:



Am 19. Dezember 2023 00:16:40 MEZ schrieb Tom Rini :

On Tue, Dec 19, 2023 at 12:08:31AM +0100, Heinrich Schuchardt wrote:



Am 18. Dezember 2023 23:41:08 MEZ schrieb Tom Rini :

On Mon, Dec 18, 2023 at 11:34:16PM +0100, Heinrich Schuchardt wrote:

[snip]

Or take:

load host 0:1 $c kernel.efi
load host 0:1 $d initrd.img

How could we ensure that initrd.img is not overwriting a part of kernel.efi 
without memory allocation?


Today, invalid checksum as part of some part of the kernel fails. But
how do we do this tomorrow, are you suggesting that "load" perform
malloc() in some predefined size? If $c is below $d and $c + kernel.efi
is now above $d we can throw an error before trying to load, yes. But
what about:
load host 0:1 $d initrd.img
load host 0:1 $c kernel.efi

In that case (which is only marginally contrived, the more real case is
loading device tree in to unexpectedly large ramdisk because someone
didn't understand the general advice on why device tree is lower than
ramdisk address) I'm fine with an error that amounts to "you just
corrupted another allocation" and then "fail, reset the board" or so.



Our current malloc library cannot manage the complete memory. We need a library 
like lmb which should also cover the memory management that we currently have 
in lib/efi/efi_memory.c. This must include a memory type attribute for usage in 
the GetMemoryMap() service. A management on page level seems sufficient.

The load command should permanently allocate memory in that lmb+ library.

We need an unload command to free the memory if we want to reuse the memory or 
we might let the load comand free the memory if exactly the same start address 
is reused.


Our current way of loading things in to memory does not handle the case
I described, yes. How would what you're proposing handle it?


If the load command has to allocate memory for the image and that allocation is 
kept, any attempt to allocate overlapping memory would fail.


So you're saying that the load command has to pre-allocate memory? Or as
it goes? If the latter, in what size chunks? This starts to get at what
Simon was talking about with respect to memory fragmentation. Which to
be clear is a problem we have today, we just let things overlap and hope
something later catches an incorrect checksum.



I don't want to replace the malloc library which handles large numbets of 
allocations.


I'm confused. The normal malloc library is not involved with current
image loading, it's direct to memory (with some attempts at sanity
checking by lmb).  Are you proposing a different allocator with
malloc/free like behavior? If so, please outline how it will determine
pool size, and how we'll use it to load thing to memory.


All memory below the stack needs to be managed. Malloc uses a small memory area 
(a few MiB) above the stack.


That's a rather huge change for how U-Boot works.


Closing the eyes when the user loads multiple files does not solve the 
fragmentation problem.


Yes. I'm only noting that today we just ignore the problem and sometimes
catch it via checksums.


Fragmentation only happens if we have many concurrent allocations.  In EFI we 
are allocating top down. The number of concurrent allocations is low. Typically 
a few dozen at most. After terminating an application these should be freed 
again.


OK, so are you saying that we would no longer be loading _to_ a location
in memory and instead just be saying "load this thing" and picking where
dynamically?


Both preassigned and allocator assigned adresses are compatible with memory 
management.

Architectures and binaries have different requirements. On riscv64 you can load 
Linux kernel, initrd, fdt anywhere. We don't need predefined addresses there. 
Other architectures have restrictions.


Yes, 64 bit architecture tend to only have alignment requirements while
32bit architectures have both alignment requirements and some memory
window requirement. Whatever we implement here needs to handle both
cases.


When loading a file from a file system we know the filesize beforehand. So 
allocation is trivial.

The loady command currently does not use the  offered size information but 
could do so.


We should be using that information to make sure we don't overwrite
U-Boot itself, but I don't recall how exactly we handle it today
off-hand.


If the user issues multiple load commands, he can overwrite previous 

Re: Proposal: U-Boot memory management

2023-12-29 Thread Tom Rini
On Fri, Dec 29, 2023 at 05:42:17PM +0100, Heinrich Schuchardt wrote:
> On 12/20/23 20:12, Tom Rini wrote:
> > On Tue, Dec 19, 2023 at 09:15:21PM -0700, Simon Glass wrote:
> > > Hi,
> > > 
> > > On Tue, 19 Dec 2023 at 05:46, Tom Rini  wrote:
> > > > 
> > > > On Tue, Dec 19, 2023 at 03:15:38AM +0100, Heinrich Schuchardt wrote:
> > > > > 
> > > > > 
> > > > > Am 19. Dezember 2023 02:26:00 MEZ schrieb Tom Rini 
> > > > > :
> > > > > > On Tue, Dec 19, 2023 at 01:01:51AM +0100, Heinrich Schuchardt wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > Am 19. Dezember 2023 00:31:30 MEZ schrieb Tom Rini 
> > > > > > > :
> > > > > > > > On Tue, Dec 19, 2023 at 12:29:19AM +0100, Heinrich Schuchardt 
> > > > > > > > wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Am 19. Dezember 2023 00:16:40 MEZ schrieb Tom Rini 
> > > > > > > > > :
> > > > > > > > > > On Tue, Dec 19, 2023 at 12:08:31AM +0100, Heinrich 
> > > > > > > > > > Schuchardt wrote:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Am 18. Dezember 2023 23:41:08 MEZ schrieb Tom Rini 
> > > > > > > > > > > :
> > > > > > > > > > > > On Mon, Dec 18, 2023 at 11:34:16PM +0100, Heinrich 
> > > > > > > > > > > > Schuchardt wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > [snip]
> > > > > > > > > > > > > Or take:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > load host 0:1 $c kernel.efi
> > > > > > > > > > > > > load host 0:1 $d initrd.img
> > > > > > > > > > > > > 
> > > > > > > > > > > > > How could we ensure that initrd.img is not 
> > > > > > > > > > > > > overwriting a part of kernel.efi without memory 
> > > > > > > > > > > > > allocation?
> > > > > > > > > > > > 
> > > > > > > > > > > > Today, invalid checksum as part of some part of the 
> > > > > > > > > > > > kernel fails. But
> > > > > > > > > > > > how do we do this tomorrow, are you suggesting that 
> > > > > > > > > > > > "load" perform
> > > > > > > > > > > > malloc() in some predefined size? If $c is below $d and 
> > > > > > > > > > > > $c + kernel.efi
> > > > > > > > > > > > is now above $d we can throw an error before trying to 
> > > > > > > > > > > > load, yes. But
> > > > > > > > > > > > what about:
> > > > > > > > > > > > load host 0:1 $d initrd.img
> > > > > > > > > > > > load host 0:1 $c kernel.efi
> > > > > > > > > > > > 
> > > > > > > > > > > > In that case (which is only marginally contrived, the 
> > > > > > > > > > > > more real case is
> > > > > > > > > > > > loading device tree in to unexpectedly large ramdisk 
> > > > > > > > > > > > because someone
> > > > > > > > > > > > didn't understand the general advice on why device tree 
> > > > > > > > > > > > is lower than
> > > > > > > > > > > > ramdisk address) I'm fine with an error that amounts to 
> > > > > > > > > > > > "you just
> > > > > > > > > > > > corrupted another allocation" and then "fail, reset the 
> > > > > > > > > > > > board" or so.
> > > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Our current malloc library cannot manage the complete 
> > > > > > > > > > > memory. We need a library like lmb which should also 
> > > > > > > > > > > cover the memory management that we currently have in 
> > > > > > > > > > > lib/efi/efi_memory.c. This must include a memory type 
> > > > > > > > > > > attribute for usage in the GetMemoryMap() service. A 
> > > > > > > > > > > management on page level seems sufficient.
> > > > > > > > > > > 
> > > > > > > > > > > The load command should permanently allocate memory in 
> > > > > > > > > > > that lmb+ library.
> > > > > > > > > > > 
> > > > > > > > > > > We need an unload command to free the memory if we want 
> > > > > > > > > > > to reuse the memory or we might let the load comand free 
> > > > > > > > > > > the memory if exactly the same start address is reused.
> > > > > > > > > > 
> > > > > > > > > > Our current way of loading things in to memory does not 
> > > > > > > > > > handle the case
> > > > > > > > > > I described, yes. How would what you're proposing handle it?
> > > > > > > > > 
> > > > > > > > > If the load command has to allocate memory for the image and 
> > > > > > > > > that allocation is kept, any attempt to allocate overlapping 
> > > > > > > > > memory would fail.
> > > > > > > > 
> > > > > > > > So you're saying that the load command has to pre-allocate 
> > > > > > > > memory? Or as
> > > > > > > > it goes? If the latter, in what size chunks? This starts to get 
> > > > > > > > at what
> > > > > > > > Simon was talking about with respect to memory fragmentation. 
> > > > > > > > Which to
> > > > > > > > be clear is a problem we have today, we just let things overlap 
> > > > > > > > and hope
> > > > > > > > something later catches an incorrect checksum.
> > > > > > > > 
> > > > > > > 
> > > > > > > I don't want to replace the malloc library which handles large 
> > > > > > > numbets of allocations.
> > > > > > 
> > > > > > I'm confused. The normal malloc library is not involved with current
> > > > > > 

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

2023-12-29 Thread Peter Robinson
On Fri, Dec 29, 2023 at 12:23 AM Tom Rini  wrote:
>
> On Thu, Dec 28, 2023 at 07:48:08PM +, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, Dec 28, 2023 at 3:40 PM Tom Rini  wrote:
> > >
> > > On Thu, Dec 28, 2023 at 03:09:40PM +, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, Dec 28, 2023 at 2:23 PM Tom Rini  wrote:
> > > > >
> > > > > On Thu, Dec 28, 2023 at 01:37:07PM +, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Wed, Dec 27, 2023 at 1:21 PM Tom Rini  wrote:
> > > > > > >
> > > > > > > On Wed, Dec 27, 2023 at 08:23:56AM +, Simon Glass wrote:
> > > > > > >
> > > > > > > > U-Boot builds devicetree binaries from its source tree. As part 
> > > > > > > > of the
> > > > > > > > Kconfig conversion, the Makefiles were updated to align with 
> > > > > > > > how this
> > > > > > > > is done in Linux: a single target for each SoC is used to build 
> > > > > > > > all the
> > > > > > > > .dtb files for that SoC.
> > > > > > > >
> > > > > > > > Since then, the Makefiles have devolved in some cases, 
> > > > > > > > resulting in
> > > > > > > > lots of target-specific build rules. Also Linux has moved to 
> > > > > > > > using
> > > > > > > > subdirectories for each vendor.
> > > > > > > >
> > > > > > > > Recent work aims to allow U-Boot to directly use devicetree 
> > > > > > > > files from
> > > > > > > > Linux. This would be easier if the directory structure were the 
> > > > > > > > same.
> > > > > > > > Another recent discussion involved dropping the build rules 
> > > > > > > > altogether.
> > > > > > > >
> > > > > > > > This series makes a start at cleaning up some of the build 
> > > > > > > > rules, to
> > > > > > > > reduce the amount of code and make it easier to add new boards 
> > > > > > > > for the
> > > > > > > > same SoC.
> > > > > > > >
> > > > > > > > One issue is that the ARCH_xxx Kconfig options between U-Boot 
> > > > > > > > and Linux
> > > > > > > > are not always the same. Given the large number of SoCs and 
> > > > > > > > boards
> > > > > > > > supported by U-Boot, it would be useful to align these where 
> > > > > > > > possible.
> > > > > > >
> > > > > > > I don't know why we should start with this now, and further not 
> > > > > > > being on
> > > > > > > top of Sumit's series to remove our duplicate dts files. And 
> > > > > > > that's
> > > > > > > where we can have the conversation about for which cases it even 
> > > > > > > makes
> > > > > > > sense to build all of the dts files for a given SoC.
> > > > > >
> > > > > > This is a completely different series - there are no conflicts with
> > > > > > Sumit's series so it can be applied before or after it.
> > > > > >
> > > > > > My goal here is to clean up our build rules, rather than just 
> > > > > > throwing
> > > > > > them all away, for reasons we have discussed previously. I filed [1]
> > > > > > to track that.
> > > > >
> > > > > Yes, I'm saying we shouldn't be doing anything this series does until
> > > > > after Sumit's series has landed. Along with the fact that I don't like
> > > > > what's going on in this series.
> > > >
> > > > This seems to again be the disagreement over whether a single DT
> > > > should be build for each board, or all the DTs for an SoC?
> > >
> > > It's about the disagreement on what we should be building, and what that
> > > infrastructure looks like. I do not like adding extra rules for
> > > "clarity" because they don't make things clearer, they lead to the
> > > unclear mess we have today getting worse. Most instructions are _not_
> > > "now take this dtb and this binary and .." but just "take this binary",
> > > because we already have the rules and logic to ensure we build the
> > > required dtbs. I also don't like the parts of this series that amount
> > > to deck shuffling when we should just be taking the chairs away. There's
> > > just not nor will there be a case for omap3/4/5 platforms of "change the
> > > dtb", so building more is pointless. For other SoCs, there may be some
> > > cases of "this could have been just a DT change", like
> > > rock5b-rk3588_defconfig / rock5a-rk3588s_defconfig could share a board
> > > dir, but the configs are different and the dts are different, so I don't
> > > know that you could really just swap the dtbs.
> >
> > It is true that we have a different defconfig for each board, but IMO
> > that is not good. It is an artifact of the way the build system works.
> > IMO Kconfig should be used to define sensible defaults so that the
> > defconfigs are nearly empty. Perhaps config fragments can be part of
> > the mix, if we can agree on [1].
> >
> > But if we let this genie out of the bottle, it will be impossible to
> > put back in. The devicetree should control the hardware in U-Boot and
> > it should be possible to use the same U-Boot proper on all boards
> > which use the same SoC.
>
> We've never been past the point where a few examples of closely related
> boards can re-use the same U-Boot build and just 

Re: Proposal: U-Boot memory management

2023-12-29 Thread Heinrich Schuchardt

On 12/20/23 20:12, Tom Rini wrote:

On Tue, Dec 19, 2023 at 09:15:21PM -0700, Simon Glass wrote:

Hi,

On Tue, 19 Dec 2023 at 05:46, Tom Rini  wrote:


On Tue, Dec 19, 2023 at 03:15:38AM +0100, Heinrich Schuchardt wrote:



Am 19. Dezember 2023 02:26:00 MEZ schrieb Tom Rini :

On Tue, Dec 19, 2023 at 01:01:51AM +0100, Heinrich Schuchardt wrote:



Am 19. Dezember 2023 00:31:30 MEZ schrieb Tom Rini :

On Tue, Dec 19, 2023 at 12:29:19AM +0100, Heinrich Schuchardt wrote:



Am 19. Dezember 2023 00:16:40 MEZ schrieb Tom Rini :

On Tue, Dec 19, 2023 at 12:08:31AM +0100, Heinrich Schuchardt wrote:



Am 18. Dezember 2023 23:41:08 MEZ schrieb Tom Rini :

On Mon, Dec 18, 2023 at 11:34:16PM +0100, Heinrich Schuchardt wrote:

[snip]

Or take:

load host 0:1 $c kernel.efi
load host 0:1 $d initrd.img

How could we ensure that initrd.img is not overwriting a part of kernel.efi 
without memory allocation?


Today, invalid checksum as part of some part of the kernel fails. But
how do we do this tomorrow, are you suggesting that "load" perform
malloc() in some predefined size? If $c is below $d and $c + kernel.efi
is now above $d we can throw an error before trying to load, yes. But
what about:
load host 0:1 $d initrd.img
load host 0:1 $c kernel.efi

In that case (which is only marginally contrived, the more real case is
loading device tree in to unexpectedly large ramdisk because someone
didn't understand the general advice on why device tree is lower than
ramdisk address) I'm fine with an error that amounts to "you just
corrupted another allocation" and then "fail, reset the board" or so.



Our current malloc library cannot manage the complete memory. We need a library 
like lmb which should also cover the memory management that we currently have 
in lib/efi/efi_memory.c. This must include a memory type attribute for usage in 
the GetMemoryMap() service. A management on page level seems sufficient.

The load command should permanently allocate memory in that lmb+ library.

We need an unload command to free the memory if we want to reuse the memory or 
we might let the load comand free the memory if exactly the same start address 
is reused.


Our current way of loading things in to memory does not handle the case
I described, yes. How would what you're proposing handle it?


If the load command has to allocate memory for the image and that allocation is 
kept, any attempt to allocate overlapping memory would fail.


So you're saying that the load command has to pre-allocate memory? Or as
it goes? If the latter, in what size chunks? This starts to get at what
Simon was talking about with respect to memory fragmentation. Which to
be clear is a problem we have today, we just let things overlap and hope
something later catches an incorrect checksum.



I don't want to replace the malloc library which handles large numbets of 
allocations.


I'm confused. The normal malloc library is not involved with current
image loading, it's direct to memory (with some attempts at sanity
checking by lmb).  Are you proposing a different allocator with
malloc/free like behavior? If so, please outline how it will determine
pool size, and how we'll use it to load thing to memory.


All memory below the stack needs to be managed. Malloc uses a small memory area 
(a few MiB) above the stack.


That's a rather huge change for how U-Boot works.


Closing the eyes when the user loads multiple files does not solve the 
fragmentation problem.


Yes. I'm only noting that today we just ignore the problem and sometimes
catch it via checksums.


Fragmentation only happens if we have many concurrent allocations.  In EFI we 
are allocating top down. The number of concurrent allocations is low. Typically 
a few dozen at most. After terminating an application these should be freed 
again.


OK, so are you saying that we would no longer be loading _to_ a location
in memory and instead just be saying "load this thing" and picking where
dynamically?


Both preassigned and allocator assigned adresses are compatible with memory 
management.

Architectures and binaries have different requirements. On riscv64 you can load 
Linux kernel, initrd, fdt anywhere. We don't need predefined addresses there. 
Other architectures have restrictions.


Yes, 64 bit architecture tend to only have alignment requirements while
32bit architectures have both alignment requirements and some memory
window requirement. Whatever we implement here needs to handle both
cases.


When loading a file from a file system we know the filesize beforehand. So 
allocation is trivial.

The loady command currently does not use the  offered size information but 
could do so.


We should be using that information to make sure we don't overwrite
U-Boot itself, but I don't recall how exactly we handle it today
off-hand.


If the user issues multiple load commands, he can overwrite previous files.


Then it sounds like we lost one benefit of all of this overhead.


During boot command execution I 

Re: Proposal: U-Boot memory management

2023-12-29 Thread Tom Rini
On Fri, Dec 29, 2023 at 05:05:17PM +0100, Heinrich Schuchardt wrote:
> 
> 
> Am 29. Dezember 2023 16:43:07 MEZ schrieb Tom Rini :
> >On Fri, Dec 29, 2023 at 05:36:09AM +, Simon Glass wrote:
> >> Hi,
> >> 
> >> On Sat, Dec 16, 2023 at 6:01 PM Simon Glass  wrote:
> >> >
> >> > Hi,
> >> >
> >> > This records my thoughts after a discussion with Ilias & Heinrich re
> >> > memory allocation in U-Boot.
> >> >
> >> > 1. malloc()
> >> >
> >> > malloc() is used for programmatic memory allocation. It allows memory
> >> > to be freed. It is not designed for very large allocations (e.g. a
> >> > 10MB kernel or 100MB ramdisk).
> >> >
> >> > 2. lmb
> >> >
> >> > lmb is used for large blocks of memory, such as those needed for a
> >> > kernel or ramdisk. Allocation is only transitory, for the purposes of
> >> > loading some images and booting. If the boot fails, then all lmb
> >> > allocations go away.
> >> >
> >> > lmb is set up by getting all available memory and then removing what
> >> > is used by U-Boot (code, data, malloc() space, etc.)
> >> >
> >> > lmb reservations have a few flags so that areas of memory can be
> >> > provided with attributes
> >> >
> >> > There are some corner cases...e.g. loading a file does an lmb
> >> > allocation but only for the purpose of avoiding a file being loaded
> >> > over U-Boot code/data. The allocation is dropped immediately after the
> >> > file is loaded. Within the bootm command, or when using standard boot,
> >> > this would be fairly easy to solve.
> >> >
> >> > Linux has renamed lmb to memblock. We should consider doing the same.
> >> >
> >> > 3. EFI
> >> >
> >> > EFI has its own memory-allocation tables.
> >> >
> >> > Like lmb, EFI is able to deal with large allocations. But via a 'pool'
> >> > function it can also do smaller allocations similar to malloc(),
> >> > although each one uses at least 4KB at present.
> >> >
> >> > EFI allocations do not go away when a boot fails.
> >> >
> >> > With EFI it is possible to add allocations post facto, in which case
> >> > they are added to the allocation table just as if the memory was
> >> > allocated with EFI to begin with.
> >> >
> >> > The EFI allocations and the lmb allocations use the same memory, so in
> >> > principle could conflict.
> >> >
> >> > EFI allocations are sometimes used to allocate internal U-Boot data as
> >> > well, if needed by the EFI app. For example, while efi_image_parse()
> >> > uses malloc(), efi_var_mem.c uses EFI allocations since the code runs
> >> > in the app context and may need to access the memory after U-Boot has
> >> > exited. Also efi_smbios.c uses allocate_pages() and then adds a new
> >> > mapping as well.
> >> >
> >> > EFI memory has attributes, including what the memory is used for (to
> >> > some degree of granularity). See enum efi_memory_type and struct
> >> > efi_mem_desc. In the latter there are also attribute flags - whether
> >> > memory is cacheable, etc.
> >> >
> >> > EFI also has the x86 idea of 'conventional' memory, meaning (I
> >> > believe) that below 4GB that isn't reserved for the hardware/system.
> >> > This is meaningless, or at least confusing, on ARM systems.
> >> >
> >> > 4. reservations
> >> >
> >> > It is perhaps worth mentioning a fourth method of memory management,
> >> > where U-Boot reserves chunks of memory before relocation (in
> >> > board_init_f.c), e.g. for the framebuffer, U-Boot code, the malloc()
> >> > region, etc.
> >> >
> >> >
> >> > Problems
> >> > —---
> >> >
> >> > There are no urgent problems, but here are some things that could be 
> >> > improved:
> >> >
> >> > 1. EFI should attach most of its data structures to driver model. This
> >> > work has started, with the partition support, but more effort would
> >> > help. This would make it easier to see which memory is related to
> >> > devices and which is separate.
> >> >
> >> > 2. Some drivers do EFI reservations today, whether EFI is used for
> >> > booting or not (e.g. rockchip video rk_vop_probe()).
> >> >
> >> > 3. U-Boot doesn't really map arch-specific memory attributes (e.g.
> >> > armv8's struct mm_region) to EFI ones.
> >> >
> >> > 4. EFI duplicates some code from bootm, some of which relates to
> >> > memory allocation (e.g. FDT fixup).
> >> >
> >> > 5. EFI code is used even if EFI is never used to boot
> >> >
> >> > 6. EFI allocations can result in the same memory being used as has
> >> > already been allocated by lmb. Users may load files which overwrite
> >> > memory allocated by EFI.
> >> 
> >> 7. We need to support doing an allocation when a file is loaded (to
> >> ensure files do not overlap), without making it too difficult to load
> >> multiple files to the same place, if desired.
> >> 
> >> >
> >> >
> >> > Lifetime
> >> > 
> >> >
> >> > We have three different memory allocators with different purposes. Can
> >> > we unify them a little?
> >> >
> >> > Within U-Boot:
> >> > - malloc() space lives forever
> >> > - lmb lives while setting out images for booting
> >> 

Re: Proposal: U-Boot memory management

2023-12-29 Thread Heinrich Schuchardt



Am 29. Dezember 2023 16:43:07 MEZ schrieb Tom Rini :
>On Fri, Dec 29, 2023 at 05:36:09AM +, Simon Glass wrote:
>> Hi,
>> 
>> On Sat, Dec 16, 2023 at 6:01 PM Simon Glass  wrote:
>> >
>> > Hi,
>> >
>> > This records my thoughts after a discussion with Ilias & Heinrich re
>> > memory allocation in U-Boot.
>> >
>> > 1. malloc()
>> >
>> > malloc() is used for programmatic memory allocation. It allows memory
>> > to be freed. It is not designed for very large allocations (e.g. a
>> > 10MB kernel or 100MB ramdisk).
>> >
>> > 2. lmb
>> >
>> > lmb is used for large blocks of memory, such as those needed for a
>> > kernel or ramdisk. Allocation is only transitory, for the purposes of
>> > loading some images and booting. If the boot fails, then all lmb
>> > allocations go away.
>> >
>> > lmb is set up by getting all available memory and then removing what
>> > is used by U-Boot (code, data, malloc() space, etc.)
>> >
>> > lmb reservations have a few flags so that areas of memory can be
>> > provided with attributes
>> >
>> > There are some corner cases...e.g. loading a file does an lmb
>> > allocation but only for the purpose of avoiding a file being loaded
>> > over U-Boot code/data. The allocation is dropped immediately after the
>> > file is loaded. Within the bootm command, or when using standard boot,
>> > this would be fairly easy to solve.
>> >
>> > Linux has renamed lmb to memblock. We should consider doing the same.
>> >
>> > 3. EFI
>> >
>> > EFI has its own memory-allocation tables.
>> >
>> > Like lmb, EFI is able to deal with large allocations. But via a 'pool'
>> > function it can also do smaller allocations similar to malloc(),
>> > although each one uses at least 4KB at present.
>> >
>> > EFI allocations do not go away when a boot fails.
>> >
>> > With EFI it is possible to add allocations post facto, in which case
>> > they are added to the allocation table just as if the memory was
>> > allocated with EFI to begin with.
>> >
>> > The EFI allocations and the lmb allocations use the same memory, so in
>> > principle could conflict.
>> >
>> > EFI allocations are sometimes used to allocate internal U-Boot data as
>> > well, if needed by the EFI app. For example, while efi_image_parse()
>> > uses malloc(), efi_var_mem.c uses EFI allocations since the code runs
>> > in the app context and may need to access the memory after U-Boot has
>> > exited. Also efi_smbios.c uses allocate_pages() and then adds a new
>> > mapping as well.
>> >
>> > EFI memory has attributes, including what the memory is used for (to
>> > some degree of granularity). See enum efi_memory_type and struct
>> > efi_mem_desc. In the latter there are also attribute flags - whether
>> > memory is cacheable, etc.
>> >
>> > EFI also has the x86 idea of 'conventional' memory, meaning (I
>> > believe) that below 4GB that isn't reserved for the hardware/system.
>> > This is meaningless, or at least confusing, on ARM systems.
>> >
>> > 4. reservations
>> >
>> > It is perhaps worth mentioning a fourth method of memory management,
>> > where U-Boot reserves chunks of memory before relocation (in
>> > board_init_f.c), e.g. for the framebuffer, U-Boot code, the malloc()
>> > region, etc.
>> >
>> >
>> > Problems
>> > —---
>> >
>> > There are no urgent problems, but here are some things that could be 
>> > improved:
>> >
>> > 1. EFI should attach most of its data structures to driver model. This
>> > work has started, with the partition support, but more effort would
>> > help. This would make it easier to see which memory is related to
>> > devices and which is separate.
>> >
>> > 2. Some drivers do EFI reservations today, whether EFI is used for
>> > booting or not (e.g. rockchip video rk_vop_probe()).
>> >
>> > 3. U-Boot doesn't really map arch-specific memory attributes (e.g.
>> > armv8's struct mm_region) to EFI ones.
>> >
>> > 4. EFI duplicates some code from bootm, some of which relates to
>> > memory allocation (e.g. FDT fixup).
>> >
>> > 5. EFI code is used even if EFI is never used to boot
>> >
>> > 6. EFI allocations can result in the same memory being used as has
>> > already been allocated by lmb. Users may load files which overwrite
>> > memory allocated by EFI.
>> 
>> 7. We need to support doing an allocation when a file is loaded (to
>> ensure files do not overlap), without making it too difficult to load
>> multiple files to the same place, if desired.
>> 
>> >
>> >
>> > Lifetime
>> > 
>> >
>> > We have three different memory allocators with different purposes. Can
>> > we unify them a little?
>> >
>> > Within U-Boot:
>> > - malloc() space lives forever
>> > - lmb lives while setting out images for booting
>> > - EFI (mostly) lives while booting an EFI app
>> >
>> > In practice, EFI is set up early in U-Boot. Some of this is necessary,
>> > some not. EFI allocations stay around forever. This works OK since
>> > large allocations are normally not done in EFI, so memory isn't really
>> > consumed to any great degree by 

Re: ACPI: usage of sandbox virtual addresses

2023-12-29 Thread Heinrich Schuchardt



Am 29. Dezember 2023 15:31:24 MEZ schrieb Simon Glass :
>Hi Heinrich,
>
>On Fri, Dec 29, 2023 at 11:14 AM Heinrich Schuchardt  
>wrote:
>>
>> On 12/29/23 09:26, Simon Glass wrote:
>> > Hi Heinrich,
>> >
>> > On Tue, Dec 26, 2023 at 10:03 AM Heinrich Schuchardt  
>> > wrote:
>> >>
>> >> On 12/26/23 10:50, Simon Glass wrote:
>> >>> Hi Heinrich,
>> >>>
>> >>> On Tue, Dec 26, 2023 at 8:56 AM Heinrich Schuchardt  
>> >>> wrote:
>> 
>>  Hello Simon,
>> 
>>  currently we use sandbox virtual addresses in all ACPI tables. This
>>  means that an application started by the U-Boot sandbox consuming these
>>  tables will crash due to accessing invalid addresses.
>> 
>>  Shouldn't the ACPI tables be migrated to use valid pointers?
>> >>>
>> >>> The confusion arises due to the difference between addresses and
>> >>> pointers. If we store addresses in the ACPI tables, then things should
>> >>> work.
>> >>>
>> >>> My approach has been to avoid using casts, but instead use
>> >>> map_sysmem() and mem_to_sysmem().
>> >>>
>> >>> Which particular piece of code is wrong in this case?
>> >>
>> >> Tables DSDT, XSDT, RSDP, FADT in the sandbox contain sandbox virtual
>> >> addresses instead of pointers to real host memory.
>> >>
>> >> All code referring to these tables should be changed.433: 
>> >>*gnvs = map_to_sysmem(ctx->current);
>> >
>> > I'm still not quite clear on this...can you point to functions or
>> > lines of code? When I look at acpi_add_table() it does use memmap, but
>> > perhaps some parts are wrong?
>> >
>> > Regards,
>> > Simon
>>
>> Here are some examples where the wrong values are set. We must get rid
>> of all map_to_sysmem() calls where writing ACPI tables.
>>
>> lib/acpi/acpi_table.c:
>> 170 rsdt->entry[i] = map_to_sysmem(table);
>> 188 xsdt->entry[i] = map_to_sysmem(table);
>>
>> lib/acpi/base.c:
>> 27: rsdp->rsdt_address = map_to_sysmem(rsdt);
>> 30: rsdp->xsdt_address = map_to_sysmem(xsdt);
>>
>> arch/x86/cpu/baytrail/acpi.c:
>> 81: fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs);
>> 82: fadt->x_dsdt = map_to_sysmem(ctx->dsdt);
>>
>> arch/x86/cpu/quark/acpi.c:
>> 76: fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs);
>> 77: fadt->x_dsdt = map_to_sysmem(ctx->dsdt);
>>
>> arch/x86/cpu/tangier/acpi.c:
>> 47: fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs);
>> 48: fadt->x_dsdt = map_to_sysmem(ctx->dsdt);
>>
>> arch/x86/lib/acpi_table.c:
>> 200:tcpa->lasa = map_to_sysmem(log);
>> 271:tpm2->lasa = map_to_sysmem(lasa);
>> 433:*gnvs = map_to_sysmem(ctx->current);
>> 575:fadt->x_firmware_ctrl = map_to_sysmem(facs);
>> 576:fadt->x_dsdt = map_to_sysmem(dsdt);
>
>OK, I see. But at least within sandbox, the address is what we want to
>store, not the pointer. Are you worried about what an EFI app will do
>in that case, when we run an app from sandbox? If so, then yes it is
>definitely a problem.
>
>Regards,
>Simon

The sandbox is an environment to run EFI binaries inside an OS. So the ACPI 
tables must contain pointers.

The acpi command should display sandbox virtual addresses. All conversions 
should be moved to the command.

Best regards

Heinrich


Re: Proposal: U-Boot memory management

2023-12-29 Thread Tom Rini
On Fri, Dec 29, 2023 at 05:36:09AM +, Simon Glass wrote:
> Hi,
> 
> On Sat, Dec 16, 2023 at 6:01 PM Simon Glass  wrote:
> >
> > Hi,
> >
> > This records my thoughts after a discussion with Ilias & Heinrich re
> > memory allocation in U-Boot.
> >
> > 1. malloc()
> >
> > malloc() is used for programmatic memory allocation. It allows memory
> > to be freed. It is not designed for very large allocations (e.g. a
> > 10MB kernel or 100MB ramdisk).
> >
> > 2. lmb
> >
> > lmb is used for large blocks of memory, such as those needed for a
> > kernel or ramdisk. Allocation is only transitory, for the purposes of
> > loading some images and booting. If the boot fails, then all lmb
> > allocations go away.
> >
> > lmb is set up by getting all available memory and then removing what
> > is used by U-Boot (code, data, malloc() space, etc.)
> >
> > lmb reservations have a few flags so that areas of memory can be
> > provided with attributes
> >
> > There are some corner cases...e.g. loading a file does an lmb
> > allocation but only for the purpose of avoiding a file being loaded
> > over U-Boot code/data. The allocation is dropped immediately after the
> > file is loaded. Within the bootm command, or when using standard boot,
> > this would be fairly easy to solve.
> >
> > Linux has renamed lmb to memblock. We should consider doing the same.
> >
> > 3. EFI
> >
> > EFI has its own memory-allocation tables.
> >
> > Like lmb, EFI is able to deal with large allocations. But via a 'pool'
> > function it can also do smaller allocations similar to malloc(),
> > although each one uses at least 4KB at present.
> >
> > EFI allocations do not go away when a boot fails.
> >
> > With EFI it is possible to add allocations post facto, in which case
> > they are added to the allocation table just as if the memory was
> > allocated with EFI to begin with.
> >
> > The EFI allocations and the lmb allocations use the same memory, so in
> > principle could conflict.
> >
> > EFI allocations are sometimes used to allocate internal U-Boot data as
> > well, if needed by the EFI app. For example, while efi_image_parse()
> > uses malloc(), efi_var_mem.c uses EFI allocations since the code runs
> > in the app context and may need to access the memory after U-Boot has
> > exited. Also efi_smbios.c uses allocate_pages() and then adds a new
> > mapping as well.
> >
> > EFI memory has attributes, including what the memory is used for (to
> > some degree of granularity). See enum efi_memory_type and struct
> > efi_mem_desc. In the latter there are also attribute flags - whether
> > memory is cacheable, etc.
> >
> > EFI also has the x86 idea of 'conventional' memory, meaning (I
> > believe) that below 4GB that isn't reserved for the hardware/system.
> > This is meaningless, or at least confusing, on ARM systems.
> >
> > 4. reservations
> >
> > It is perhaps worth mentioning a fourth method of memory management,
> > where U-Boot reserves chunks of memory before relocation (in
> > board_init_f.c), e.g. for the framebuffer, U-Boot code, the malloc()
> > region, etc.
> >
> >
> > Problems
> > —---
> >
> > There are no urgent problems, but here are some things that could be 
> > improved:
> >
> > 1. EFI should attach most of its data structures to driver model. This
> > work has started, with the partition support, but more effort would
> > help. This would make it easier to see which memory is related to
> > devices and which is separate.
> >
> > 2. Some drivers do EFI reservations today, whether EFI is used for
> > booting or not (e.g. rockchip video rk_vop_probe()).
> >
> > 3. U-Boot doesn't really map arch-specific memory attributes (e.g.
> > armv8's struct mm_region) to EFI ones.
> >
> > 4. EFI duplicates some code from bootm, some of which relates to
> > memory allocation (e.g. FDT fixup).
> >
> > 5. EFI code is used even if EFI is never used to boot
> >
> > 6. EFI allocations can result in the same memory being used as has
> > already been allocated by lmb. Users may load files which overwrite
> > memory allocated by EFI.
> 
> 7. We need to support doing an allocation when a file is loaded (to
> ensure files do not overlap), without making it too difficult to load
> multiple files to the same place, if desired.
> 
> >
> >
> > Lifetime
> > 
> >
> > We have three different memory allocators with different purposes. Can
> > we unify them a little?
> >
> > Within U-Boot:
> > - malloc() space lives forever
> > - lmb lives while setting out images for booting
> > - EFI (mostly) lives while booting an EFI app
> >
> > In practice, EFI is set up early in U-Boot. Some of this is necessary,
> > some not. EFI allocations stay around forever. This works OK since
> > large allocations are normally not done in EFI, so memory isn't really
> > consumed to any great degree by the boot process.
> >
> > What happens to EFI allocations if the app returns? They are still
> > present, in case another app is run. This seems fine.
> >
> > API
> > –--
> > Can 

Re: Proposal: U-Boot memory management

2023-12-29 Thread Heinrich Schuchardt



Am 29. Dezember 2023 06:36:09 MEZ schrieb Simon Glass :
>Hi,
>
>On Sat, Dec 16, 2023 at 6:01 PM Simon Glass  wrote:
>>
>> Hi,
>>
>> This records my thoughts after a discussion with Ilias & Heinrich re
>> memory allocation in U-Boot.
>>
>> 1. malloc()
>>
>> malloc() is used for programmatic memory allocation. It allows memory
>> to be freed. It is not designed for very large allocations (e.g. a
>> 10MB kernel or 100MB ramdisk).
>>
>> 2. lmb
>>
>> lmb is used for large blocks of memory, such as those needed for a
>> kernel or ramdisk. Allocation is only transitory, for the purposes of
>> loading some images and booting. If the boot fails, then all lmb
>> allocations go away.
>>
>> lmb is set up by getting all available memory and then removing what
>> is used by U-Boot (code, data, malloc() space, etc.)
>>
>> lmb reservations have a few flags so that areas of memory can be
>> provided with attributes
>>
>> There are some corner cases...e.g. loading a file does an lmb
>> allocation but only for the purpose of avoiding a file being loaded
>> over U-Boot code/data. The allocation is dropped immediately after the
>> file is loaded. Within the bootm command, or when using standard boot,
>> this would be fairly easy to solve.
>>
>> Linux has renamed lmb to memblock. We should consider doing the same.
>>
>> 3. EFI
>>
>> EFI has its own memory-allocation tables.
>>
>> Like lmb, EFI is able to deal with large allocations. But via a 'pool'
>> function it can also do smaller allocations similar to malloc(),
>> although each one uses at least 4KB at present.
>>
>> EFI allocations do not go away when a boot fails.
>>
>> With EFI it is possible to add allocations post facto, in which case
>> they are added to the allocation table just as if the memory was
>> allocated with EFI to begin with.
>>
>> The EFI allocations and the lmb allocations use the same memory, so in
>> principle could conflict.
>>
>> EFI allocations are sometimes used to allocate internal U-Boot data as
>> well, if needed by the EFI app. For example, while efi_image_parse()
>> uses malloc(), efi_var_mem.c uses EFI allocations since the code runs
>> in the app context and may need to access the memory after U-Boot has
>> exited. Also efi_smbios.c uses allocate_pages() and then adds a new
>> mapping as well.
>>
>> EFI memory has attributes, including what the memory is used for (to
>> some degree of granularity). See enum efi_memory_type and struct
>> efi_mem_desc. In the latter there are also attribute flags - whether
>> memory is cacheable, etc.
>>
>> EFI also has the x86 idea of 'conventional' memory, meaning (I
>> believe) that below 4GB that isn't reserved for the hardware/system.
>> This is meaningless, or at least confusing, on ARM systems.
>>
>> 4. reservations
>>
>> It is perhaps worth mentioning a fourth method of memory management,
>> where U-Boot reserves chunks of memory before relocation (in
>> board_init_f.c), e.g. for the framebuffer, U-Boot code, the malloc()
>> region, etc.
>>
>>
>> Problems
>> —---
>>
>> There are no urgent problems, but here are some things that could be 
>> improved:
>>
>> 1. EFI should attach most of its data structures to driver model. This
>> work has started, with the partition support, but more effort would
>> help. This would make it easier to see which memory is related to
>> devices and which is separate.
>>
>> 2. Some drivers do EFI reservations today, whether EFI is used for
>> booting or not (e.g. rockchip video rk_vop_probe()).
>>
>> 3. U-Boot doesn't really map arch-specific memory attributes (e.g.
>> armv8's struct mm_region) to EFI ones.
>>
>> 4. EFI duplicates some code from bootm, some of which relates to
>> memory allocation (e.g. FDT fixup).
>>
>> 5. EFI code is used even if EFI is never used to boot
>>
>> 6. EFI allocations can result in the same memory being used as has
>> already been allocated by lmb. Users may load files which overwrite
>> memory allocated by EFI.
>
>7. We need to support doing an allocation when a file is loaded (to
>ensure files do not overlap), without making it too difficult to load
>multiple files to the same place, if desired.
>
>>
>>
>> Lifetime
>> 
>>
>> We have three different memory allocators with different purposes. Can
>> we unify them a little?
>>
>> Within U-Boot:
>> - malloc() space lives forever
>> - lmb lives while setting out images for booting
>> - EFI (mostly) lives while booting an EFI app
>>
>> In practice, EFI is set up early in U-Boot. Some of this is necessary,
>> some not. EFI allocations stay around forever. This works OK since
>> large allocations are normally not done in EFI, so memory isn't really
>> consumed to any great degree by the boot process.
>>
>> What happens to EFI allocations if the app returns? They are still
>> present, in case another app is run. This seems fine.
>>
>> API
>> –--
>> Can we unify some APIs?
>>
>> It should be possible to use lmb for large EFI memory allocations, so
>> long as they are only needed for 

Re: [PATCH v3 4/8] dts: Add alternative location for upstream DTB builds

2023-12-29 Thread Sumit Garg
On Fri, 29 Dec 2023 at 01:18, Simon Glass  wrote:
>
> Hi Tom,
>
> On Thu, Dec 28, 2023 at 3:54 PM Tom Rini  wrote:
> >
> > On Thu, Dec 28, 2023 at 03:09:12PM +, Simon Glass wrote:
> > > Hi Tom, Sumit,
> > >
> > > On Thu, Dec 28, 2023 at 2:03 PM Tom Rini  wrote:
> > > >
> > > > On Thu, Dec 28, 2023 at 01:37:26PM +, Simon Glass wrote:
> > > > > Hi Sumit,
> > > > >
> > > > > On Thu, Dec 28, 2023 at 11:58 AM Sumit Garg  
> > > > > wrote:
> > > > > >
> > > > > > Allow platform owners to mirror devicetree files from 
> > > > > > devitree-rebasing
> > > > > > directory into dts/arch/$(ARCH) (special case for dts/arch/arm64). 
> > > > > > Then
> > > > > > build then along with any *-u-boot.dtsi file present in 
> > > > > > arch/$(ARCH)/dts
> > > > > > directory. Also add a new Makefile for arm64.
> > > > > >
> > > > > > This will help easy migration for platforms which currently are 
> > > > > > compliant
> > > > > > with upstream Linux kernel devicetree files.
> > > > > >
> > > > > > Signed-off-by: Sumit Garg 
> > > > > > ---
> > > > > >
> > > > > > Changes in v3:
> > > > > > --
> > > > > > - Minor commit message update
> > > > > >
> > > > > > Changes in v2:
> > > > > > --
> > > > > > - s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/
> > > > > >
> > > > > >  dts/Kconfig | 11 +++
> > > > > >  dts/Makefile| 17 ++---
> > > > > >  dts/arch/arm64/Makefile | 14 ++
> > > > > >  3 files changed, 39 insertions(+), 3 deletions(-)
> > > > > >  create mode 100644 dts/arch/arm64/Makefile
> > > > > >
> > > > > > diff --git a/dts/Kconfig b/dts/Kconfig
> > > > > > index 00c0aeff893..e58c1c6f2ab 100644
> > > > > > --- a/dts/Kconfig
> > > > > > +++ b/dts/Kconfig
> > > > > > @@ -85,6 +85,17 @@ config OF_LIVE
> > > > > >   enables a live tree which is available after relocation,
> > > > > >   and can be adjusted as needed.
> > > > > >
> > > > > > +config OF_UPSTREAM
> > > > > > +   bool "Enable use of devicetree imported from Linux kernel 
> > > > > > release"
> > > > > > +   help
> > > > > > + Traditionally, U-Boot platforms used to have their custom 
> > > > > > devicetree
> > > > > > + files or copy devicetree files from Linux kernel which 
> > > > > > are hard to
> > > > > > + maintain and can usually get out-of-sync from Linux 
> > > > > > kernel. This
> > > > > > + option enables platforms to migrate to 
> > > > > > devicetree-rebasing repo where
> > > > > > + a regular sync will be maintained every major Linux 
> > > > > > kernel release
> > > > > > + cycle. However, platforms can still have some custom 
> > > > > > u-boot specific
> > > > > > + bits maintained as part of *-u-boot.dtsi files.
> > > > >
> > > > > My only other suggestion here is to mention that this should be set in
> > > > > Kconfig, for the SoC as a whole. So I believe that means that it
> > > > > should be hidden, with no string for the 'bool':
> > > > >
> > > > >   bool  # Enable use of devicetree imported from Linux kernel 
> > > > > release
> > > >
> > > > I think we can just keep prompting for it now, to make the transition
> > > > easier, before this option just goes away in time, hopefully.
> > > >
> > > > > Also, this doesn't seem to work for me. Before this series I get these
> > > > > files when building firefly-rk3399:
> > > > >
> > > > > rk3399-eaidk-610.dtbrk3399-khadas-edge-v.dtb
> > > > > rk3399-orangepi.dtbrk3399-rock-pi-4a.dtb
> > > > > rk3399-evb.dtb  rk3399-leez-p710.dtb
> > > > > rk3399-pinebook-pro.dtbrk3399-rock-pi-4c.dtb
> > > > > rk3399-ficus.dtbrk3399-nanopc-t4.dtb
> > > > > rk3399-pinephone-pro.dtb   rk3399-rockpro64.dtb
> > > > > rk3399-firefly.dtb  rk3399-nanopi-m4-2gb.dtb
> > > > > rk3399pro-rock-pi-n10.dtb  rk3399-roc-pc.dtb
> > > > > rk3399-gru-bob.dtb  rk3399-nanopi-m4b.dtb
> > > > > rk3399-puma-haikou.dtb rk3399-roc-pc-mezzanine.dtb
> > > > > rk3399-gru-kevin.dtbrk3399-nanopi-m4.dtb
> > > > > rk3399-rock-4c-plus.dtb
> > > > > rk3399-khadas-edge-captain.dtb  rk3399-nanopi-neo4.dtb
> > > > > rk3399-rock-4se.dtb
> > > > > rk3399-khadas-edge.dtb  rk3399-nanopi-r4s.dtb 
> > > > > rk3399-rock960.dtb
> > > > >
> > > > > Afterwards I get this:
> > > > >
> > > > > make[3]: *** No rule to make target
> > > > > 'dts/arch/arm64/rk3399-firefly.dtb', needed by 'dtbs'.  Stop.
> > > > >
> > > > > So I set this manually for that one board:
> > > > >
> > > > > CONFIG_DEFAULT_DEVICE_TREE="rockchip/rk3399-firefly"
> > > > >
> > > > > and get:
> > > > >
> > > > > make[3]: *** No rule to make target
> > > > > 'dts/arch/arm64/rockchip/rk3399-firefly.dtb', needed by 'dtbs'.  Stop.
> > > > >
> > > > > I am not sure how to fix this, nor how this can be made to build all
> > > > > the DTs for rk3399, as it does today.
> > > >
> > > > Looking at the patch for amlogic boards, 

Re: ACPI: usage of sandbox virtual addresses

2023-12-29 Thread Simon Glass
Hi Heinrich,

On Fri, Dec 29, 2023 at 11:14 AM Heinrich Schuchardt  wrote:
>
> On 12/29/23 09:26, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Tue, Dec 26, 2023 at 10:03 AM Heinrich Schuchardt  
> > wrote:
> >>
> >> On 12/26/23 10:50, Simon Glass wrote:
> >>> Hi Heinrich,
> >>>
> >>> On Tue, Dec 26, 2023 at 8:56 AM Heinrich Schuchardt  
> >>> wrote:
> 
>  Hello Simon,
> 
>  currently we use sandbox virtual addresses in all ACPI tables. This
>  means that an application started by the U-Boot sandbox consuming these
>  tables will crash due to accessing invalid addresses.
> 
>  Shouldn't the ACPI tables be migrated to use valid pointers?
> >>>
> >>> The confusion arises due to the difference between addresses and
> >>> pointers. If we store addresses in the ACPI tables, then things should
> >>> work.
> >>>
> >>> My approach has been to avoid using casts, but instead use
> >>> map_sysmem() and mem_to_sysmem().
> >>>
> >>> Which particular piece of code is wrong in this case?
> >>
> >> Tables DSDT, XSDT, RSDP, FADT in the sandbox contain sandbox virtual
> >> addresses instead of pointers to real host memory.
> >>
> >> All code referring to these tables should be changed.433:  
> >>   *gnvs = map_to_sysmem(ctx->current);
> >
> > I'm still not quite clear on this...can you point to functions or
> > lines of code? When I look at acpi_add_table() it does use memmap, but
> > perhaps some parts are wrong?
> >
> > Regards,
> > Simon
>
> Here are some examples where the wrong values are set. We must get rid
> of all map_to_sysmem() calls where writing ACPI tables.
>
> lib/acpi/acpi_table.c:
> 170 rsdt->entry[i] = map_to_sysmem(table);
> 188 xsdt->entry[i] = map_to_sysmem(table);
>
> lib/acpi/base.c:
> 27: rsdp->rsdt_address = map_to_sysmem(rsdt);
> 30: rsdp->xsdt_address = map_to_sysmem(xsdt);
>
> arch/x86/cpu/baytrail/acpi.c:
> 81: fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs);
> 82: fadt->x_dsdt = map_to_sysmem(ctx->dsdt);
>
> arch/x86/cpu/quark/acpi.c:
> 76: fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs);
> 77: fadt->x_dsdt = map_to_sysmem(ctx->dsdt);
>
> arch/x86/cpu/tangier/acpi.c:
> 47: fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs);
> 48: fadt->x_dsdt = map_to_sysmem(ctx->dsdt);
>
> arch/x86/lib/acpi_table.c:
> 200:tcpa->lasa = map_to_sysmem(log);
> 271:tpm2->lasa = map_to_sysmem(lasa);
> 433:*gnvs = map_to_sysmem(ctx->current);
> 575:fadt->x_firmware_ctrl = map_to_sysmem(facs);
> 576:fadt->x_dsdt = map_to_sysmem(dsdt);

OK, I see. But at least within sandbox, the address is what we want to
store, not the pointer. Are you worried about what an EFI app will do
in that case, when we run an app from sandbox? If so, then yes it is
definitely a problem.

Regards,
Simon


[PATCH v3 3/3] Add imx93-var-som support

2023-12-29 Thread Mathieu Othacehe
From: Mathieu Othacehe 

Add support for the Variscite VAR-SOM-IMX93 evaluation kit. The SoM
consists of an NXP iMX93 dual A55 CPU. The SoM is mounted on a Variscite
Symphony SBC.

Signed-off-by: Mathieu Othacehe 
---
 arch/arm/dts/Makefile |3 +-
 .../dts/imx93-var-som-symphony-u-boot.dtsi|  253 +++
 arch/arm/dts/imx93-var-som-symphony.dts   |  305 
 arch/arm/dts/imx93-var-som.dtsi   |  111 ++
 arch/arm/include/asm/arch-imx9/clock.h|1 +
 arch/arm/mach-imx/imx9/Kconfig|7 +
 board/variscite/common/eth.c  |   59 +
 board/variscite/common/eth.h  |   12 +
 board/variscite/common/imx9_eeprom.c  |  190 +++
 board/variscite/common/imx9_eeprom.h  |   83 +
 board/variscite/common/mmc.c  |   47 +
 board/variscite/imx93_var_som/Kconfig |   12 +
 board/variscite/imx93_var_som/MAINTAINERS |7 +
 board/variscite/imx93_var_som/Makefile|   17 +
 board/variscite/imx93_var_som/container.cfg   |   10 +
 board/variscite/imx93_var_som/imx93_var_som.c |  126 ++
 .../variscite/imx93_var_som/imx93_var_som.env |  104 ++
 board/variscite/imx93_var_som/imximage.cfg|   10 +
 .../variscite/imx93_var_som/lpddr4x_timing.c  | 1489 +
 board/variscite/imx93_var_som/spl.c   |  144 ++
 configs/imx93_var_som_defconfig   |  156 ++
 doc/board/variscite/imx93_var_som.rst |   68 +
 doc/board/variscite/index.rst |1 +
 include/configs/imx93_var_som.h   |   48 +
 24 files changed, 3262 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/dts/imx93-var-som-symphony-u-boot.dtsi
 create mode 100644 arch/arm/dts/imx93-var-som-symphony.dts
 create mode 100644 arch/arm/dts/imx93-var-som.dtsi
 create mode 100644 board/variscite/common/eth.c
 create mode 100644 board/variscite/common/eth.h
 create mode 100644 board/variscite/common/imx9_eeprom.c
 create mode 100644 board/variscite/common/imx9_eeprom.h
 create mode 100644 board/variscite/common/mmc.c
 create mode 100644 board/variscite/imx93_var_som/Kconfig
 create mode 100644 board/variscite/imx93_var_som/MAINTAINERS
 create mode 100644 board/variscite/imx93_var_som/Makefile
 create mode 100644 board/variscite/imx93_var_som/container.cfg
 create mode 100644 board/variscite/imx93_var_som/imx93_var_som.c
 create mode 100644 board/variscite/imx93_var_som/imx93_var_som.env
 create mode 100644 board/variscite/imx93_var_som/imximage.cfg
 create mode 100644 board/variscite/imx93_var_som/lpddr4x_timing.c
 create mode 100644 board/variscite/imx93_var_som/spl.c
 create mode 100644 configs/imx93_var_som_defconfig
 create mode 100644 doc/board/variscite/imx93_var_som.rst
 create mode 100644 include/configs/imx93_var_som.h

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 85fd5b1157b..e6b604a2232 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1057,7 +1057,8 @@ dtb-$(CONFIG_ARCH_IMX8M) += \
imx8mq-librem5-r4.dtb
 
 dtb-$(CONFIG_ARCH_IMX9) += \
-   imx93-11x11-evk.dtb
+   imx93-11x11-evk.dtb \
+   imx93-var-som-symphony.dtb
 
 dtb-$(CONFIG_ARCH_IMXRT) += imxrt1050-evk.dtb \
imxrt1020-evk.dtb \
diff --git a/arch/arm/dts/imx93-var-som-symphony-u-boot.dtsi 
b/arch/arm/dts/imx93-var-som-symphony-u-boot.dtsi
new file mode 100644
index 000..e98bafd461a
--- /dev/null
+++ b/arch/arm/dts/imx93-var-som-symphony-u-boot.dtsi
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2022 NXP
+ * Copyright 2023 Variscite Ltd.
+ */
+
+#include "imx93-u-boot.dtsi"
+
+/ {
+   wdt-reboot {
+   compatible = "wdt-reboot";
+   wdt = <>;
+   bootph-pre-ram;
+   };
+
+   aliases {
+   ethernet0 = 
+   ethernet1 = 
+   };
+
+   firmware {
+   optee {
+   compatible = "linaro,optee-tz";
+   method = "smc";
+   };
+   };
+};
+
+&{/soc@0} {
+   bootph-all;
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+   bootph-all;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+_usdhc2_vmmc {
+   u-boot,off-on-delay-us = <2>;
+   bootph-pre-ram;
+};
+
+_reg_usdhc2_vmmc {
+   bootph-pre-ram;
+};
+
+_uart1 {
+   bootph-pre-ram;
+};
+
+_usdhc2 {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+};
+
+ {
+   bootph-pre-ram;
+   fsl,signal-voltage-switch-extra-delay-ms = <8>;
+};
+
+ {
+   compatible = "fsl,imx-eqos";
+};
+
+ {
+   reset-gpios = < 7 GPIO_ACTIVE_LOW>;
+   reset-assert-us = <15000>;
+   reset-deassert-us = <10>;
+};
+
+ {
+   reset-gpios = < 5 GPIO_ACTIVE_LOW>;
+  

[PATCH v3 2/3] mach-imx: Add i.MX93 binman support.

2023-12-29 Thread Mathieu Othacehe
From: Mathieu Othacehe 

Add dedicated Makefile targets for the i.MX93 and a new imx93-u-boot.dtsi
device-tree to create binman images.

Signed-off-by: Mathieu Othacehe 
---
 arch/arm/dts/imx93-u-boot.dtsi | 88 ++
 arch/arm/mach-imx/Makefile | 22 -
 2 files changed, 109 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/dts/imx93-u-boot.dtsi

diff --git a/arch/arm/dts/imx93-u-boot.dtsi b/arch/arm/dts/imx93-u-boot.dtsi
new file mode 100644
index 000..40e17bbc5ae
--- /dev/null
+++ b/arch/arm/dts/imx93-u-boot.dtsi
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Mathieu Othacehe 
+ */
+
+/ {
+   binman: binman {
+   multiple-images;
+   };
+};
+
+ {
+   u-boot-spl-ddr {
+   align = <4>;
+   align-size = <4>;
+   filename = "u-boot-spl-ddr.bin";
+   pad-byte = <0xff>;
+
+   u-boot-spl {
+   align-end = <4>;
+   filename = "u-boot-spl.bin";
+   };
+
+   ddr-1d-imem-fw {
+   filename = "lpddr4_imem_1d_v202201.bin";
+   align-end = <4>;
+   type = "blob-ext";
+   };
+
+   ddr-1d-dmem-fw {
+   filename = "lpddr4_dmem_1d_v202201.bin";
+   align-end = <4>;
+   type = "blob-ext";
+   };
+
+   ddr-2d-imem-fw {
+   filename = "lpddr4_imem_2d_v202201.bin";
+   align-end = <4>;
+   type = "blob-ext";
+   };
+
+   ddr-2d-dmem-fw {
+   filename = "lpddr4_dmem_2d_v202201.bin";
+   align-end = <4>;
+   type = "blob-ext";
+   };
+   };
+
+   spl {
+   filename = "spl.bin";
+
+   mkimage {
+   args = "-n spl/u-boot-spl.cfgout -T imx8image -e 
0x2049A000";
+
+   blob {
+   filename = "u-boot-spl-ddr.bin";
+   };
+   };
+   };
+
+   u-boot-container {
+   filename = "u-boot-container.bin";
+
+   mkimage {
+   args = "-n u-boot-container.cfgout -T imx8image -e 0x0";
+
+   blob {
+   filename = "u-boot.bin";
+   };
+   };
+   };
+
+   imx-boot {
+   filename = "flash.bin";
+   pad-byte = <0x00>;
+
+   spl: blob-ext@1 {
+   filename = "spl.bin";
+   offset = <0x0>;
+   align-size = <0x400>;
+   align = <0x400>;
+   };
+
+   uboot: blob-ext@2 {
+   filename = "u-boot-container.bin";
+   };
+   };
+};
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index aebfa6517bd..2a16f9b9dfc 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -129,6 +129,9 @@ DEPFILE_EXISTS := $(shell $(CPP) $(cpp_flags) -x c -o 
u-boot-dtb.cfgout $(srctre
 else ifeq ($(CONFIG_ARCH_IMX8M), y)
 IMAGE_TYPE := imx8mimage
 DEPFILE_EXISTS := 0
+else ifeq ($(CONFIG_ARCH_IMX9), y)
+IMAGE_TYPE := imx8image
+DEPFILE_EXISTS := 0
 else
 IMAGE_TYPE := imximage
 DEPFILE_EXISTS := 0
@@ -211,7 +214,24 @@ endif
 endif
 
 ifeq ($(CONFIG_ARCH_IMX9), y)
-SPL:
+
+SPL: spl/u-boot-spl.bin spl/u-boot-spl.cfgout u-boot-container.cfgout FORCE
+
+MKIMAGEFLAGS_flash.bin = -n spl/u-boot-spl.cfgout -T $(IMAGE_TYPE) -e 
$(CONFIG_SPL_TEXT_BASE)
+flash.bin: MKIMAGEOUTPUT = flash.log
+
+spl/u-boot-spl.cfgout: $(IMX_CONFIG) FORCE
+   $(Q)mkdir -p $(dir $@)
+   $(call if_changed_dep,cpp_cfg)
+
+spl/u-boot-spl-ddr.bin: spl/u-boot-spl.bin spl/u-boot-spl.cfgout FORCE
+
+u-boot-container.cfgout: $(IMX_CONTAINER_CFG) FORCE
+   $(Q)mkdir -p $(dir $@)
+   $(call if_changed_dep,cpp_cfg)
+
+flash.bin: spl/u-boot-spl-ddr.bin container.cfgout FORCE
+   $(call if_changed,mkimage)
 endif
 
 else
-- 
2.25.1



[PATCH v3 1/3] spl: binman: Disable u_boot_any symbols for i.MX93 boards

2023-12-29 Thread Mathieu Othacehe
From: Mathieu Othacehe 

This is extending commit 6516c9b349b3 ("spl: binman: Disable u_boot_any
symbols for i.MX8M boards") to i.MX93 boards.

Signed-off-by: Mathieu Othacehe 
---
 common/spl/Kconfig | 2 +-
 common/spl/Kconfig.tpl | 2 +-
 common/spl/Kconfig.vpl | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index c5dd476db58..322e6fe882f 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -206,7 +206,7 @@ config SPL_BINMAN_SYMBOLS
 config SPL_BINMAN_UBOOT_SYMBOLS
bool "Declare binman symbols for U-Boot phases in SPL"
depends on SPL_BINMAN_SYMBOLS
-   default n if ARCH_IMX8M
+   default n if ARCH_IMX8M || ARCH_IMX9
default y
help
  This enables use of symbols in SPL which refer to U-Boot phases,
diff --git a/common/spl/Kconfig.tpl b/common/spl/Kconfig.tpl
index 3d6cf1e59f3..9bb57e6fbff 100644
--- a/common/spl/Kconfig.tpl
+++ b/common/spl/Kconfig.tpl
@@ -23,7 +23,7 @@ config TPL_BINMAN_SYMBOLS
 config TPL_BINMAN_UBOOT_SYMBOLS
bool "Declare binman symbols for U-Boot phases in TPL"
depends on TPL_BINMAN_SYMBOLS
-   default n if ARCH_IMX8M
+   default n if ARCH_IMX8M || ARCH_IMX9
default y
help
  This enables use of symbols in TPL which refer to U-Boot phases,
diff --git a/common/spl/Kconfig.vpl b/common/spl/Kconfig.vpl
index ae1a3c724f3..f1993026bba 100644
--- a/common/spl/Kconfig.vpl
+++ b/common/spl/Kconfig.vpl
@@ -243,7 +243,7 @@ config VPL_BINMAN_SYMBOLS
 config VPL_BINMAN_UBOOT_SYMBOLS
bool "Declare binman symbols for U-Boot phases in VPL"
depends on VPL_BINMAN_SYMBOLS
-   default n if ARCH_IMX8M
+   default n if ARCH_IMX8M || ARCH_IMX9
default y
help
  This enables use of symbols in VPL which refer to U-Boot phases,
-- 
2.25.1



[PATCH v3 0/3] Add imx93-var-som support

2023-12-29 Thread Mathieu Othacehe
From: Mathieu Othacehe 

Hello,

This v3 adds support for binman and documents how to build and flash a
bootloader for the i.MX93 Variscite Symphony evaluation board.

Thanks,

Mathieu

v2: https://lists.denx.de/pipermail/u-boot/2023-December/542001.html

Mathieu Othacehe (3):
  spl: binman: Disable u_boot_any symbols for i.MX93 boards
  mach-imx: Add i.MX93 binman support.
  Add imx93-var-som support

 arch/arm/dts/Makefile |3 +-
 arch/arm/dts/imx93-u-boot.dtsi|   88 +
 .../dts/imx93-var-som-symphony-u-boot.dtsi|  253 +++
 arch/arm/dts/imx93-var-som-symphony.dts   |  305 
 arch/arm/dts/imx93-var-som.dtsi   |  111 ++
 arch/arm/include/asm/arch-imx9/clock.h|1 +
 arch/arm/mach-imx/Makefile|   22 +-
 arch/arm/mach-imx/imx9/Kconfig|7 +
 board/variscite/common/eth.c  |   59 +
 board/variscite/common/eth.h  |   12 +
 board/variscite/common/imx9_eeprom.c  |  190 +++
 board/variscite/common/imx9_eeprom.h  |   83 +
 board/variscite/common/mmc.c  |   47 +
 board/variscite/imx93_var_som/Kconfig |   12 +
 board/variscite/imx93_var_som/MAINTAINERS |7 +
 board/variscite/imx93_var_som/Makefile|   17 +
 board/variscite/imx93_var_som/container.cfg   |   10 +
 board/variscite/imx93_var_som/imx93_var_som.c |  126 ++
 .../variscite/imx93_var_som/imx93_var_som.env |  104 ++
 board/variscite/imx93_var_som/imximage.cfg|   10 +
 .../variscite/imx93_var_som/lpddr4x_timing.c  | 1489 +
 board/variscite/imx93_var_som/spl.c   |  144 ++
 common/spl/Kconfig|2 +-
 common/spl/Kconfig.tpl|2 +-
 common/spl/Kconfig.vpl|2 +-
 configs/imx93_var_som_defconfig   |  156 ++
 doc/board/variscite/imx93_var_som.rst |   68 +
 doc/board/variscite/index.rst |1 +
 include/configs/imx93_var_som.h   |   48 +
 29 files changed, 3374 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/dts/imx93-u-boot.dtsi
 create mode 100644 arch/arm/dts/imx93-var-som-symphony-u-boot.dtsi
 create mode 100644 arch/arm/dts/imx93-var-som-symphony.dts
 create mode 100644 arch/arm/dts/imx93-var-som.dtsi
 create mode 100644 board/variscite/common/eth.c
 create mode 100644 board/variscite/common/eth.h
 create mode 100644 board/variscite/common/imx9_eeprom.c
 create mode 100644 board/variscite/common/imx9_eeprom.h
 create mode 100644 board/variscite/common/mmc.c
 create mode 100644 board/variscite/imx93_var_som/Kconfig
 create mode 100644 board/variscite/imx93_var_som/MAINTAINERS
 create mode 100644 board/variscite/imx93_var_som/Makefile
 create mode 100644 board/variscite/imx93_var_som/container.cfg
 create mode 100644 board/variscite/imx93_var_som/imx93_var_som.c
 create mode 100644 board/variscite/imx93_var_som/imx93_var_som.env
 create mode 100644 board/variscite/imx93_var_som/imximage.cfg
 create mode 100644 board/variscite/imx93_var_som/lpddr4x_timing.c
 create mode 100644 board/variscite/imx93_var_som/spl.c
 create mode 100644 configs/imx93_var_som_defconfig
 create mode 100644 doc/board/variscite/imx93_var_som.rst
 create mode 100644 include/configs/imx93_var_som.h

-- 
2.25.1



Re: [PATCH v3 0/3] Add imx93-var-som support

2023-12-29 Thread Fabio Estevam
Hi Mathieu,

On Fri, Dec 29, 2023 at 8:17 AM Mathieu Othacehe  wrote:
>
> From: Mathieu Othacehe 
>
> Hello,
>
> This v3 adds support for binman and documents how to build and flash a
> bootloader for the i.MX93 Variscite Symphony evaluation board.

Nice work! Thanks for adding i.MX93 binman support.

>From a glance, the whole series looks good. I will take a closer look soon.

Cheers


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

2023-12-29 Thread Heinrich Schuchardt

On 12/28/23 14:37, Simon Glass wrote:

Hi Heinrich,

On Thu, Dec 28, 2023 at 7:30 AM Heinrich Schuchardt
 wrote:


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

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



Reviewed-by: Simon Glass 


diff --git a/lib/smbios.c b/lib/smbios.c
index 45480b01af..550b2471f9 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -467,7 +467,8 @@ static void smbios_write_type4_dm(struct smbios_type4 *t,
 }
  #endif

-   t->processor_family = processor_family;
+   t->processor_family = 0xfe;
+   t->processor_family2 = processor_family;


Why not use 'family' if it fits?


Using family for values less then 0x100 is allowable.
But there would be no benefit in making our code more complicated.

Best regards

Heinrich




 t->processor_manufacturer = smbios_add_prop(ctx, NULL, vendor);
 t->processor_version = smbios_add_prop(ctx, NULL, name);
  }
@@ -489,7 +490,6 @@ static int smbios_write_type4(ulong *current, int handle,
 t->l1_cache_handle = 0x;
 t->l2_cache_handle = 0x;
 t->l3_cache_handle = 0x;
-   t->processor_family2 = t->processor_family;

 len = t->length + smbios_string_table_len(ctx);
 *current += len;
--
2.43.0



Regards,
Simon




Re: ACPI: usage of sandbox virtual addresses

2023-12-29 Thread Heinrich Schuchardt

On 12/29/23 09:26, Simon Glass wrote:

Hi Heinrich,

On Tue, Dec 26, 2023 at 10:03 AM Heinrich Schuchardt  wrote:


On 12/26/23 10:50, Simon Glass wrote:

Hi Heinrich,

On Tue, Dec 26, 2023 at 8:56 AM Heinrich Schuchardt  wrote:


Hello Simon,

currently we use sandbox virtual addresses in all ACPI tables. This
means that an application started by the U-Boot sandbox consuming these
tables will crash due to accessing invalid addresses.

Shouldn't the ACPI tables be migrated to use valid pointers?


The confusion arises due to the difference between addresses and
pointers. If we store addresses in the ACPI tables, then things should
work.

My approach has been to avoid using casts, but instead use
map_sysmem() and mem_to_sysmem().

Which particular piece of code is wrong in this case?


Tables DSDT, XSDT, RSDP, FADT in the sandbox contain sandbox virtual
addresses instead of pointers to real host memory.

All code referring to these tables should be changed.433:  
 *gnvs = map_to_sysmem(ctx->current);


I'm still not quite clear on this...can you point to functions or
lines of code? When I look at acpi_add_table() it does use memmap, but
perhaps some parts are wrong?

Regards,
Simon


Here are some examples where the wrong values are set. We must get rid
of all map_to_sysmem() calls where writing ACPI tables.

lib/acpi/acpi_table.c:
170 rsdt->entry[i] = map_to_sysmem(table);
188 xsdt->entry[i] = map_to_sysmem(table);

lib/acpi/base.c:
27: rsdp->rsdt_address = map_to_sysmem(rsdt);
30: rsdp->xsdt_address = map_to_sysmem(xsdt);

arch/x86/cpu/baytrail/acpi.c:
81: fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs);
82: fadt->x_dsdt = map_to_sysmem(ctx->dsdt);

arch/x86/cpu/quark/acpi.c:
76: fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs);
77: fadt->x_dsdt = map_to_sysmem(ctx->dsdt);

arch/x86/cpu/tangier/acpi.c:
47: fadt->x_firmware_ctrl = map_to_sysmem(ctx->facs);
48: fadt->x_dsdt = map_to_sysmem(ctx->dsdt);

arch/x86/lib/acpi_table.c:
200:tcpa->lasa = map_to_sysmem(log);
271:tpm2->lasa = map_to_sysmem(lasa);
433:*gnvs = map_to_sysmem(ctx->current);
575:fadt->x_firmware_ctrl = map_to_sysmem(facs);
576:fadt->x_dsdt = map_to_sysmem(dsdt);

Best regards

Heinrich

Best regards

Heinrich


Re: [PATCH v6 8/8] docs: k3: Add secure booting documentation

2023-12-29 Thread Manorit Chawdhry
Hi Andrew,

On 10:41-20231206, Andrew Davis wrote:
> On 12/6/23 3:51 AM, Manorit Chawdhry wrote:
> > This commit adds a general flow to explain the usage of firewalls and
> > the chain of trust in K3 devices.
> > 
> > Signed-off-by: Manorit Chawdhry 
> > ---
> >   doc/board/ti/k3.rst | 45 +
> >   1 file changed, 45 insertions(+)
> > 
> > diff --git a/doc/board/ti/k3.rst b/doc/board/ti/k3.rst
> > index 947b86ba8292..60d04c5708be 100644
> > --- a/doc/board/ti/k3.rst
> > +++ b/doc/board/ti/k3.rst
> > @@ -104,6 +104,51 @@ firmware can be loaded on the now free core in the 
> > wakeup domain.
> >   For more information on the bootup process of your SoC, consult the
> >   device specific boot flow documentation.
> > +Secure Boot
> > +^^^
> > +
> > +K3 HS-SE devices are used for authenticated boot flow with secure boot.
> 
> How about:
> 
> K3 HS-SE devices enforce an authenticated boot flow for secure boot.
> 
> > +HS-FS devices have optional authentication in the flow and doesn't 
> > "require"
> > +authentication unless converted to HS-SE devices.
> 
> HS-FS is the state of a K3 device before it has been eFused with
> customer security keys. In the HS-FS state the authentication still
> can function as in HS-SE but as there are no customer keys to verify
> the signatures against the authentication will pass for certificates
> signed with any key.
> 
> > +
> > +Chain of trust
> > +""
> > +
> > +1) SMS starts up and loads the authenticated ROM code in Wakeup Domain
> > +2) ROM code starts up and loads the authenticated tiboot3.bin in Wakeup
> > +   Domain
> > +3) Wakeup SPL (tiboot3.bin) would authenticate the next set of binaries
> > +   (ATF,OP-TEE,DM,SPL,etc.)
> > +4) After ATF and OP-TEE load, ARMV8 U-boot authenticates the next set of
> > +   binaries (Linux and DTBs) if using FIT Image authentication and having a
> > +   signature node in U-boot.
> 
> 
> This might be better said like this:
> 
> 1) Public ROM loads the tiboot3.bin (R5 SPL, TIFS)
> 2) R5 SPL loads tispl.bin (ATF, OP-TEE, DM, SPL)
> 3) SPL loads u-boot.img (U-Boot)
> 4) U-Boot loads fitImage (Linux and DTBs)
> 
> Each stage authenticating (and optionally decrypting) while loading is
> implied by the following sentences below, no need to repeat each time.
> 
> Plus the use of "ROM" is confusing, we have two ROMs in K3 (Secure ROM
> and Public ROM), you should always be specific.
> 
> > +
> > +Steps 1-3 are all authenticated by either the ROM code or TIFS as the
> 
> s/ROM code/Secure ROM
> 
> > +authenticating entity and step 4 uses U-boot standard mechanism for
> > +authenticating.
> > +
> > +All the authentication that are done for ROM/TIFS are done through x509
> > +certificates that are signed.
> > +
> > +Firewalls
> > +"
> > +
> > +1) ROM comes up and sets up firewalls that are needed by itself
> 
> Secure ROM
> 
> > +2) TIFS (in multicertificate will setup it's own firewalls)
> 
> TIFS sets up it's own firewalls in either case.
> 
> > +3) R5 SPL comes along and opens up other firewalls ( that are not owned by
> > +   anyone - essentially firewalls that were setup by ROM but are not needed
> > +   anymore)
> 
> How about:
> 
> 3) R5 SPL will remove any firewalls that are leftover from the Secure ROM 
> stage
> that are no longer required.
> 
> > +4) Each stage beyond this: such as tispl.bin containing TFA/OPTEE uses 
> > OIDs to
> > +   set up firewalls to protect themselves (enforced by TIFS)
> > +5) TFA/OP-TEE can configure other firewalls at runtime if required as they
> > +   are already authenticated and firewalled off from illegal access.
> > +6) A53 SPL and U-boot itself startups but has no ability to change the
> > +   protection firewalls enforced by x509 OIDs or any other firewalls
> > +   configured by ROM/TIFS in the beginning.
> 
> 6) All later stages can setup or remove firewalls that have not been already
> configured by previous stages, such as those created by TIFS, TFA, and OP-TEE.
> 
> > +
> > +Futhur, firewalls have a lockdown bit in hardware that enforces the setting
> > +(and cannot be over-ridden) till the full system is resetted.
> 
> s/till/until
> s/resetted/reset
> 

Have incorporated the suggested changes, Thanks!

Regards,
Manorit

> Andrew
> 
> > +
> >   Software Sources
> >   
> > 


[PATCH v7 9/9] docs: board: ti: k3: Add secure booting documentation

2023-12-29 Thread Manorit Chawdhry
This commit adds a general flow to explain the usage of firewalls and
the chain of trust in K3 devices.

Signed-off-by: Manorit Chawdhry 
---
 doc/board/ti/k3.rst | 43 +++
 1 file changed, 43 insertions(+)

diff --git a/doc/board/ti/k3.rst b/doc/board/ti/k3.rst
index 1064c21b5a1d..7dfe39c5fa57 100644
--- a/doc/board/ti/k3.rst
+++ b/doc/board/ti/k3.rst
@@ -104,6 +104,49 @@ firmware can be loaded on the now free core in the wakeup 
domain.
 For more information on the bootup process of your SoC, consult the
 device specific boot flow documentation.
 
+Secure Boot
+---
+
+K3 HS-SE (High Security - Security Enforced) devices enforce an
+authenticated boot flow for secure boot. HS-FS (High Security - Field
+Securable) is the state of a K3 device before it has been eFused with
+customer security keys.  In the HS-FS state the authentication still can
+function as in HS-SE but as there are no customer keys to verify the
+signatures against the authentication will pass for certificates signed
+with any key.
+
+Chain of trust
+^^
+
+1) Public ROM loads the tiboot3.bin (R5 SPL, TIFS)
+2) R5 SPL loads tispl.bin (ATF, OP-TEE, DM, SPL)
+3) SPL loads u-boot.img (U-Boot)
+4) U-Boot loads fitImage (Linux and DTBs)
+
+Steps 1-3 are all authenticated by either the Secure ROM or TIFS as the
+authenticating entity and step 4 uses U-boot standard mechanism for
+authenticating.
+
+All the authentication that are done for ROM/TIFS are done through x509
+certificates that are signed.
+
+Firewalls
+^
+
+1) Secure ROM comes up and sets up firewalls that are needed by itself
+2) TIFS will setup it's own firewalls to protect core system resources
+3) R5 SPL will remove any firewalls that are leftover from the Secure ROM stage
+   that are no longer required.
+4) Each stage beyond this: such as tispl.bin containing TFA/OPTEE uses OIDs to
+   set up firewalls to protect themselves (enforced by TIFS)
+5) TFA/OP-TEE can configure other firewalls at runtime if required as they
+   are already authenticated and firewalled off from illegal access.
+6) All later stages can setup or remove firewalls that have not been already
+   configured by previous stages, such as those created by TIFS, TFA, and 
OP-TEE.
+
+Futhur, firewalls have a lockdown bit in hardware that enforces the setting
+(and cannot be over-ridden) until the full system is reset.
+
 Software Sources
 
 

-- 
2.43.0



[PATCH v7 8/9] docs: board: ti: k3: Cleanup FIT signature documentation

2023-12-29 Thread Manorit Chawdhry
The previous documentation had been very crude so refactor it to make it
cleaner and concise.

Signed-off-by: Manorit Chawdhry 
---
 doc/board/ti/k3.rst | 270 +---
 1 file changed, 171 insertions(+), 99 deletions(-)

diff --git a/doc/board/ti/k3.rst b/doc/board/ti/k3.rst
index f19ee56f296e..1064c21b5a1d 100644
--- a/doc/board/ti/k3.rst
+++ b/doc/board/ti/k3.rst
@@ -248,6 +248,8 @@ Building tiboot3.bin
the final `tiboot3.bin` binary. (or the `sysfw.itb` if your device
uses the split binary flow)
 
+.. _k3_rst_include_start_build_steps_spl_r5:
+
 .. k3_rst_include_start_build_steps_spl_r5
 .. prompt:: bash $
 
@@ -312,6 +314,8 @@ use the `lite` option.
finished, we can jump back into U-Boot again, this time running on a
64bit core in the main domain.
 
+.. _k3_rst_include_start_build_steps_uboot:
+
 .. k3_rst_include_start_build_steps_uboot
 .. prompt:: bash $
 
@@ -337,144 +341,212 @@ wakeup and main domain and to boot to the U-Boot prompt
| `tispl.bin` for HS devices or `tispl.bin_unsigned` for GP devices
| `u-boot.img` for HS devices or `u-boot.img_unsigned` for GP devices
 
-Fit Signature Signing
+FIT signature signing
 -
 
-K3 Platforms have fit signature signing enabled by default on their primary
-platforms. Here we'll take an example for creating fit image for J721e platform
+K3 platforms have FIT signature signing enabled by default on their primary
+platforms. Here we'll take an example for creating FIT Image for J721E platform
 and the same can be extended to other platforms
 
-1. Describing FIT source
+Pre-requisites:
+
+* U-boot build (:ref:`U-boot build `)
+* Linux Image and Linux DTB prebuilt
+
+Describing FIT source
+^
 
-  .. code-block:: bash
+FIT Image is a packed structure containing binary blobs and configurations.
+The Kernel FIT Image that we have has Kernel Image, DTB and the DTBOs.  It
+supports packing multiple images and configurations that allow you to
+choose any configuration at runtime to boot from.
+
+.. code-block::
 
 /dts-v1/;
 
 / {
-description = "Kernel fitImage for j721e-hs-evm";
-#address-cells = <1>;
-
-images {
-kernel-1 {
-description = "Linux kernel";
-data = /incbin/("Image");
-type = "kernel";
-arch = "arm64";
-os = "linux";
-compression = "none";
-load = <0x8008>;
-entry = <0x8008>;
-hash-1 {
-algo = "sha512";
-};
-
-};
-fdt-ti_k3-j721e-common-proc-board.dtb {
-description = "Flattened Device Tree blob";
-data = /incbin/("k3-j721e-common-proc-board.dtb");
-type = "flat_dt";
-arch = "arm64";
-compression = "none";
-load = <0x8300>;
-hash-1 {
-algo = "sha512";
-};
-
-};
+description = "FIT Image description";
+#address-cells = <1>;
+
+images {
+[image-1]
+[image-2]
+[fdt-1]
+[fdt-2]
+}
+
+configurations {
+default = 
+[conf-1: image-1,fdt-1]
+[conf-2: image-2,fdt-1]
+}
+}
+
+* Sample Images
+
+.. code-block::
+
+kernel-1 {
+description = "Linux kernel";
+data = /incbin/("linux.bin");
+type = "kernel";
+arch = "arm64";
+os = "linux";
+compression = "gzip";
+load = <0x8100>;
+entry = <0x8100>;
+hash-1 {
+algo = "sha512";
 };
-
-configurations {
-default = "conf-ti_k3-j721e-common-proc-board.dtb";
-conf-ti_k3-j721e-common-proc-board.dtb {
-description = "Linux kernel, FDT blob";
-fdt = "fdt-ti_k3-j721e-common-proc-board.dtb";
-kernel = "kernel-1";
-signature-1 {
-algo = "sha512,rsa4096";
-key-name-hint = "custMpk";
-sign-images = "kernel", "fdt";
-};
-};
+};
+fdt-ti_k3-j721e-common-proc-board.dtb {
+description = "Flattened Device Tree blob";
+data = 
/incbin/("arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dtb");
+

[PATCH v7 7/9] arm: dts: k3-j7200-binman: Add firewall configurations

2023-12-29 Thread Manorit Chawdhry
The following commits adds the configuration of firewalls required to
protect ATF and OP-TEE memory region from non-secure reads and
writes using master and slave firewalls present in our K3 SOCs.

Signed-off-by: Manorit Chawdhry 
---
 arch/arm/dts/k3-j7200-binman.dtsi | 90 +++
 1 file changed, 90 insertions(+)

diff --git a/arch/arm/dts/k3-j7200-binman.dtsi 
b/arch/arm/dts/k3-j7200-binman.dtsi
index 38cccabaa71d..06db86598761 100644
--- a/arch/arm/dts/k3-j7200-binman.dtsi
+++ b/arch/arm/dts/k3-j7200-binman.dtsi
@@ -195,6 +195,96 @@
 
fit {
images {
+   atf {
+   ti-secure {
+   auth-in-place = <0xa02>;
+
+   firewall-257-0 {
+   /* cpu_0_cpu_0_msmc 
Background Firewall */
+   insert-template = 
<_bg_1>;
+   id = <257>;
+   region = <0>;
+   };
+
+   firewall-257-1 {
+   /* cpu_0_cpu_0_msmc 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <257>;
+   region = <1>;
+   };
+
+   /*  firewall-4760-0 {
+*  nb_slv0__mem0 
Background Firewall
+*  Already 
configured by the secure entity
+*  };
+*/
+
+   firewall-4760-1 {
+   /* nb_slv0__mem0 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <4760>;
+   region = <1>;
+   };
+
+   /*  firewall-4761-0 {
+*  nb_slv1__mem0 
Background Firewall
+*  Already 
configured by the secure entity
+*  };
+*/
+
+   firewall-4761-1 {
+   /* nb_slv1__mem0 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <4761>;
+   region = <1>;
+   };
+   };
+   };
+
+   tee {
+   ti-secure {
+   auth-in-place = <0xa02>;
+
+   /* cpu_0_cpu_0_msmc region 0 
and 1 configured
+* during ATF Firewalling
+*/
+
+   firewall-257-2 {
+   /* cpu_0_cpu_0_msmc 
Foreground Firewall */
+   insert-template = 
<_armv8_optee_fg>;
+   id = <257>;
+   region = <2>;
+   };
+
+   firewall-4762-0 {
+   /* nb_slv2__mem0 
Background Firewall - 0 */
+   insert-template = 
<_bg_3>;
+   id = <4762>;
+   region = <0>;
+   };
+
+   firewall-4762-1 {
+   /* nb_slv2__mem0 
Foreground Firewall */
+   insert-template = 
<_armv8_optee_fg>;
+ 

[PATCH v7 6/9] arm: dts: k3-j721s2-binman: Add firewall configurations

2023-12-29 Thread Manorit Chawdhry
The following commits adds the configuration of firewalls required to
protect ATF and OP-TEE memory region from non-secure reads and
writes using master and slave firewalls present in our K3 SOCs.

Signed-off-by: Manorit Chawdhry 
---
 arch/arm/dts/k3-j721s2-binman.dtsi | 123 +
 1 file changed, 123 insertions(+)

diff --git a/arch/arm/dts/k3-j721s2-binman.dtsi 
b/arch/arm/dts/k3-j721s2-binman.dtsi
index f17dd8e04cfa..7efb135bdff9 100644
--- a/arch/arm/dts/k3-j721s2-binman.dtsi
+++ b/arch/arm/dts/k3-j721s2-binman.dtsi
@@ -159,6 +159,129 @@
 
fit {
images {
+   atf {
+   ti-secure {
+   auth-in-place = <0xa02>;
+
+   firewall-257-0 {
+   /* cpu_0_cpu_0_msmc 
Background Firewall */
+   insert-template = 
<_bg_1>;
+   id = <257>;
+   region = <0>;
+   };
+
+   firewall-257-1 {
+   /* cpu_0_cpu_0_msmc 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <257>;
+   region = <1>;
+   };
+
+   firewall-284-0 {
+   /* dru_0_msmc 
Background Firewall */
+   insert-template = 
<_bg_3>;
+   id = <284>;
+   region = <0>;
+   };
+
+   firewall-284-1 {
+   /* dru_0_msmc 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <284>;
+   region = <1>;
+   };
+
+   /*  firewall-5140-0 {
+*  nb_slv0__mem0 
Background Firewall
+*  Already 
configured by the secure entity
+*  };
+*/
+
+   firewall-5140-1 {
+   /* nb_slv0__mem0 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <5140>;
+   region = <1>;
+   };
+
+   /*  firewall-5140-0 {
+*  nb_slv1__mem0 
Background Firewall
+*  Already 
configured by the secure entity
+*  };
+*/
+
+   firewall-5141-1 {
+   /* nb_slv1__mem0 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <5141>;
+   region = <1>;
+   };
+
+   };
+   };
+
+   tee {
+   ti-secure {
+   auth-in-place = <0xa02>;
+
+   firewall-257-2 {
+   /* cpu_0_cpu_0_msmc 
Foreground Firewall */
+   insert-template = 
<_armv8_optee_fg>;
+   id = <257>;
+   region = <2>;
+   };
+
+   

[PATCH v7 5/9] arm: dts: k3-j721e-binman: Add firewall configurations

2023-12-29 Thread Manorit Chawdhry
The following commits adds the configuration of firewalls required to
protect ATF and OP-TEE memory region from non-secure reads and
writes using master and slave firewalls present in our K3 SOCs.

Signed-off-by: Manorit Chawdhry 
---
 arch/arm/dts/k3-j721e-binman.dtsi | 116 ++
 1 file changed, 116 insertions(+)

diff --git a/arch/arm/dts/k3-j721e-binman.dtsi 
b/arch/arm/dts/k3-j721e-binman.dtsi
index dbc385a852de..1bd9f96a58e9 100644
--- a/arch/arm/dts/k3-j721e-binman.dtsi
+++ b/arch/arm/dts/k3-j721e-binman.dtsi
@@ -146,6 +146,122 @@
 
fit {
images {
+   atf {
+   ti-secure {
+   auth-in-place = <0xa02>;
+
+   firewall-257-0 {
+   /* cpu_0_cpu_0_msmc 
Background Firewall */
+   insert-template = 
<_bg_1>;
+   id = <257>;
+   region = <0>;
+   };
+
+   firewall-257-1 {
+   /* cpu_0_cpu_0_msmc 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <257>;
+   region = <1>;
+   };
+
+   firewall-284-0 {
+   /* dru_0_msmc 
Background Firewall */
+   insert-template = 
<_bg_3>;
+   id = <284>;
+   region = <0>;
+   };
+
+   firewall-284-1 {
+   /* dru_0_msmc 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <284>;
+   region = <1>;
+   };
+
+   /*  firewall-4760-0 {
+*  nb_slv0__mem0 
Background Firewall
+*  Already 
configured by the secure entity
+*  };
+*/
+
+   firewall-4760-1 {
+   /* nb_slv0__mem0 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <4760>;
+   region = <1>;
+   };
+
+   /*  firewall-4761-0 {
+*  nb_slv1__mem0 
Background Firewall
+*  Already 
configured by the secure entity
+*  };
+*/
+
+   firewall-4761-1 {
+   /* nb_slv1__mem0 
Foreground Firewall */
+   insert-template = 
<_armv8_atf_fg>;
+   id = <4761>;
+   region = <1>;
+   };
+
+   };
+   };
+
+   tee {
+   ti-secure {
+   auth-in-place = <0xa02>;
+
+   /* cpu_0_cpu_0_msmc region 0 
and 1 configured
+* during ATF Firewalling
+*/
+
+   firewall-257-2 {
+   /* cpu_0_cpu_0_msmc 
Foreground Firewall */
+   insert-template = 
<_armv8_optee_fg>;
+

[PATCH v7 4/9] arm: dts: k3-binman: Add k3-security.h and include it in k3-binman.dtsi

2023-12-29 Thread Manorit Chawdhry
For readability during configuring firewalls, adding k3-security.h file
and including it in k3-binman.dtsi to be accessible across K3 SoCs

Reviewed-by: Simon Glass 
Signed-off-by: Manorit Chawdhry 
---
 arch/arm/dts/k3-binman.dtsi | 49 ++
 arch/arm/dts/k3-security.h  | 58 +
 2 files changed, 107 insertions(+)

diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
index cd9926a01696..758c8bf6ea16 100644
--- a/arch/arm/dts/k3-binman.dtsi
+++ b/arch/arm/dts/k3-binman.dtsi
@@ -3,6 +3,8 @@
  * Copyright (C) 2022-2023 Texas Instruments Incorporated - https://www.ti.com/
  */
 
+#include "k3-security.h"
+
 / {
binman: binman {
multiple-images;
@@ -437,6 +439,53 @@
};
};
};
+   firewall_bg_1: template-5 {
+   control = <(FWCTRL_EN | FWCTRL_LOCK |
+   FWCTRL_BG | FWCTRL_CACHE)>;
+   permissions = <((FWPRIVID_ALL << FWPRIVID_SHIFT) |
+   FWPERM_SECURE_PRIV_RWCD |
+   FWPERM_SECURE_USER_RWCD |
+   FWPERM_NON_SECURE_PRIV_RWCD |
+   FWPERM_NON_SECURE_USER_RWCD)>;
+   start_address = <0x0 0x0>;
+   end_address = <0xff 0x>;
+   };
+   firewall_bg_3: template-6 {
+   insert-template = <_bg_1>;
+   permissions = <((FWPRIVID_ALL << FWPRIVID_SHIFT) |
+   FWPERM_SECURE_PRIV_RWCD |
+   FWPERM_SECURE_USER_RWCD |
+   FWPERM_NON_SECURE_PRIV_RWCD |
+   FWPERM_NON_SECURE_USER_RWCD)>,
+ <((FWPRIVID_ALL << FWPRIVID_SHIFT) |
+   FWPERM_SECURE_PRIV_RWCD |
+   FWPERM_SECURE_USER_RWCD |
+   FWPERM_NON_SECURE_PRIV_RWCD |
+   FWPERM_NON_SECURE_USER_RWCD)>,
+ <((FWPRIVID_ALL << FWPRIVID_SHIFT) |
+   FWPERM_SECURE_PRIV_RWCD |
+   FWPERM_SECURE_USER_RWCD |
+   FWPERM_NON_SECURE_PRIV_RWCD |
+   FWPERM_NON_SECURE_USER_RWCD)>;
+   };
+   firewall_armv8_atf_fg: template-7 {
+   control = <(FWCTRL_EN | FWCTRL_LOCK |
+   FWCTRL_CACHE)>;
+   permissions = <((FWPRIVID_ARMV8 << FWPRIVID_SHIFT) |
+   FWPERM_SECURE_PRIV_RWCD |
+   FWPERM_SECURE_USER_RWCD)>;
+   start_address = <0x0 0x7000>;
+   end_address = <0x0 0x7001>;
+   };
+   firewall_armv8_optee_fg: template-8 {
+   control = <(FWCTRL_EN | FWCTRL_LOCK |
+   FWCTRL_CACHE)>;
+   permissions = <((FWPRIVID_ARMV8 << FWPRIVID_SHIFT) |
+   FWPERM_SECURE_PRIV_RWCD |
+   FWPERM_SECURE_USER_RWCD)>;
+   start_address = <0x0 0x9e80>;
+   end_address = <0x0 0x9fff>;
+   };
 
 };
 
diff --git a/arch/arm/dts/k3-security.h b/arch/arm/dts/k3-security.h
new file mode 100644
index ..33609caa8fb5
--- /dev/null
+++ b/arch/arm/dts/k3-security.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+#ifndef DTS_ARM64_TI_K3_FIREWALL_H
+#define DTS_ARM64_TI_K3_FIREWALL_H
+
+#define FWPRIVID_ALL0xc3
+#define FWPRIVID_ARMV8  1
+#define FWPRIVID_SHIFT  16
+
+#define FWCTRL_EN 0xA
+#define FWCTRL_LOCK   (1 << 4)
+#define FWCTRL_BG (1 << 8)
+#define FWCTRL_CACHE  (1 << 9)
+
+#define FWPERM_SECURE_PRIV_WRITE  (1 << 0)
+#define FWPERM_SECURE_PRIV_READ   (1 << 1)
+#define FWPERM_SECURE_PRIV_CACHEABLE  (1 << 2)
+#define FWPERM_SECURE_PRIV_DEBUG  (1 << 3)
+
+#define FWPERM_SECURE_PRIV_RWCD   (FWPERM_SECURE_PRIV_READ | \
+  
FWPERM_SECURE_PRIV_WRITE | \
+  
FWPERM_SECURE_PRIV_CACHEABLE | \
+  
FWPERM_SECURE_PRIV_DEBUG)
+
+#define FWPERM_SECURE_USER_WRITE  (1 << 4)
+#define FWPERM_SECURE_USER_READ   (1 << 5)
+#define FWPERM_SECURE_USER_CACHEABLE  (1 

[PATCH v7 3/9] binman: ftest: Add test for ti-secure firewall node

2023-12-29 Thread Manorit Chawdhry
Add test for TI firewalling node in ti-secure.

Reviewed-by: Simon Glass 
Signed-off-by: Manorit Chawdhry 
---
 tools/binman/ftest.py  | 23 ++
 tools/binman/test/324_ti_secure_firewall.dts   | 28 ++
 .../325_ti_secure_firewall_missing_property.dts| 28 ++
 3 files changed, 79 insertions(+)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index a273120d9f9b..be6adcdd8b7c 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -7042,6 +7042,29 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
entry_args=entry_args)[0]
 self.assertGreater(len(data), len(TI_UNSECURE_DATA))
 
+def testPackTiSecureFirewall(self):
+"""Test that an image with a TI secured binary can be created"""
+keyfile = self.TestFile('key.key')
+entry_args = {
+'keyfile': keyfile,
+}
+data_no_firewall = self._DoReadFileDtb('296_ti_secure.dts',
+   entry_args=entry_args)[0]
+data_firewall = self._DoReadFileDtb('324_ti_secure_firewall.dts',
+   entry_args=entry_args)[0]
+self.assertGreater(len(data_firewall),len(data_no_firewall))
+
+def testPackTiSecureFirewallMissingProperty(self):
+"""Test that an image with a TI secured binary can be created"""
+keyfile = self.TestFile('key.key')
+entry_args = {
+'keyfile': keyfile,
+}
+with self.assertRaises(ValueError) as e:
+data_firewall = 
self._DoReadFileDtb('325_ti_secure_firewall_missing_property.dts',
+   entry_args=entry_args)[0]
+self.assertRegex(str(e.exception), "Node '/binman/ti-secure': Subnode 
'firewall-0-2' is missing properties: id,region")
+
 def testPackTiSecureMissingTool(self):
 """Test that an image with a TI secured binary (non-functional) can be 
created
 when openssl is missing"""
diff --git a/tools/binman/test/324_ti_secure_firewall.dts 
b/tools/binman/test/324_ti_secure_firewall.dts
new file mode 100644
index ..7ec407fa67ba
--- /dev/null
+++ b/tools/binman/test/324_ti_secure_firewall.dts
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   ti-secure {
+   content = <_binary>;
+   auth-in-place = <0xa02>;
+
+   firewall-0-2 {
+   id = <0>;
+   region = <2>;
+   control = <0x31a>;
+   permissions = <0xc3>;
+   start_address = <0x0 0x9e80>;
+   end_address = <0x0 0x9fff>;
+   };
+
+   };
+   unsecure_binary: blob-ext {
+   filename = "ti_unsecure.bin";
+   };
+   };
+};
diff --git a/tools/binman/test/325_ti_secure_firewall_missing_property.dts 
b/tools/binman/test/325_ti_secure_firewall_missing_property.dts
new file mode 100644
index ..24a0a9962503
--- /dev/null
+++ b/tools/binman/test/325_ti_secure_firewall_missing_property.dts
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   ti-secure {
+   content = <_binary>;
+   auth-in-place = <0xa02>;
+
+   firewall-0-2 {
+   // id = <0>;
+   // region = <2>;
+   control = <0x31a>;
+   permissions = <0xc3>;
+   start_address = <0x0 0x9e80>;
+   end_address = <0x0 0x9fff>;
+   };
+
+   };
+   unsecure_binary: blob-ext {
+   filename = "ti_unsecure.bin";
+   };
+   };
+};

-- 
2.43.0



[PATCH v7 2/9] binman: ti-secure: Add support for firewalling entities

2023-12-29 Thread Manorit Chawdhry
We can now firewall entities while loading them through our secure
entity TIFS, the required information should be present in the
certificate that is being parsed by TIFS.

The following commit adds the support to enable the certificates to be
generated if the firewall configurations are present in the binman dtsi
nodes.

Reviewed-by: Simon Glass 
Signed-off-by: Manorit Chawdhry 
---
 tools/binman/btool/openssl.py   | 16 ++-
 tools/binman/etype/ti_secure.py | 95 +
 tools/binman/etype/x509_cert.py |  4 +-
 3 files changed, 112 insertions(+), 3 deletions(-)

diff --git a/tools/binman/btool/openssl.py b/tools/binman/btool/openssl.py
index 7ee2683ab236..fe81a1f51b1e 100644
--- a/tools/binman/btool/openssl.py
+++ b/tools/binman/btool/openssl.py
@@ -82,7 +82,7 @@ imageSize  = INTEGER:{len(indata)}
 return self.run_cmd(*args)
 
 def x509_cert_sysfw(self, cert_fname, input_fname, key_fname, sw_rev,
-  config_fname, req_dist_name_dict):
+  config_fname, req_dist_name_dict, firewall_cert_data):
 """Create a certificate to be booted by system firmware
 
 Args:
@@ -94,6 +94,13 @@ imageSize  = INTEGER:{len(indata)}
 req_dist_name_dict (dict): Dictionary containing key-value pairs of
 req_distinguished_name section extensions, must contain extensions 
for
 C, ST, L, O, OU, CN and emailAddress
+firewall_cert_data (dict):
+  - auth_in_place (int): The Priv ID for copying as the
+specific host in firewall protected region
+  - num_firewalls (int): The number of firewalls in the
+extended certificate
+  - certificate (str): Extended firewall certificate with
+the information for the firewall configurations.
 
 Returns:
 str: Tool output
@@ -121,6 +128,7 @@ basicConstraints   = CA:true
 1.3.6.1.4.1.294.1.3= ASN1:SEQUENCE:swrv
 1.3.6.1.4.1.294.1.34   = ASN1:SEQUENCE:sysfw_image_integrity
 1.3.6.1.4.1.294.1.35   = ASN1:SEQUENCE:sysfw_image_load
+1.3.6.1.4.1.294.1.37   = ASN1:SEQUENCE:firewall
 
 [ swrv ]
 swrv = INTEGER:{sw_rev}
@@ -132,7 +140,11 @@ imageSize  = INTEGER:{len(indata)}
 
 [ sysfw_image_load ]
 destAddr = FORMAT:HEX,OCT:
-authInPlace = INTEGER:2
+authInPlace = INTEGER:{hex(firewall_cert_data['auth_in_place'])}
+
+[ firewall ]
+numFirewallRegions = INTEGER:{firewall_cert_data['num_firewalls']}
+{firewall_cert_data['certificate']}
 ''', file=outf)
 args = ['req', '-new', '-x509', '-key', key_fname, '-nodes',
 '-outform', 'DER', '-out', cert_fname, '-config', config_fname,
diff --git a/tools/binman/etype/ti_secure.py b/tools/binman/etype/ti_secure.py
index d939dce57139..704dcf8a3811 100644
--- a/tools/binman/etype/ti_secure.py
+++ b/tools/binman/etype/ti_secure.py
@@ -7,9 +7,44 @@
 
 from binman.entry import EntryArg
 from binman.etype.x509_cert import Entry_x509_cert
+from dataclasses import dataclass
 
 from dtoc import fdt_util
 
+@dataclass
+class Firewall():
+id: int
+region: int
+control : int
+permissions: list
+start_address: str
+end_address: str
+
+def ensure_props(self, etype, name):
+missing_props = []
+for key, val in self.__dict__.items():
+if val is None:
+missing_props += [key]
+
+if len(missing_props):
+etype.Raise(f"Subnode '{name}' is missing properties: 
{','.join(missing_props)}")
+
+def get_certificate(self) -> str:
+unique_identifier = f"{self.id}{self.region}"
+cert = f"""
+firewallID{unique_identifier} = INTEGER:{self.id}
+region{unique_identifier} = INTEGER:{self.region}
+control{unique_identifier} = INTEGER:{hex(self.control)}
+nPermissionRegs{unique_identifier} = INTEGER:{len(self.permissions)}
+"""
+for index, permission in enumerate(self.permissions):
+cert += f"""permissions{unique_identifier}{index} = 
INTEGER:{hex(permission)}
+"""
+cert += f"""startAddress{unique_identifier} = 
FORMAT:HEX,OCT:{self.start_address:02x}
+endAddress{unique_identifier} = FORMAT:HEX,OCT:{self.end_address:02x}
+"""
+return cert
+
 class Entry_ti_secure(Entry_x509_cert):
 """Entry containing a TI x509 certificate binary
 
@@ -17,6 +52,11 @@ class Entry_ti_secure(Entry_x509_cert):
 - content: List of phandles to entries to sign
 - keyfile: Filename of file containing key to sign binary with
 - sha: Hash function to be used for signing
+- auth-in-place: This is an integer field that contains two pieces
+  of information
+Lower Byte - Remains 0x02 as per our use case
+( 0x02: Move the authenticated binary back to the header )
+Upper Byte - The Host ID of the core owning the firewall
 
 Output files:
 - input. - input file passed to openssl
@@ -25,6 

[PATCH v7 1/9] dtoc: Change dst to self in debug message

2023-12-29 Thread Manorit Chawdhry
Fix the error message to not use dst and use self as it is copying the
properties to self.

While using templating if there are no subnodes defined, we end up in
this situation where "dst" isn't defined and it tries to print the error
message and fails.

'UnboundLocalError: local variable 'dst' referenced before assignment'

Fixes: 55e1278d5eca ("dtoc: Allow inserting a list of nodes into another")

Signed-off-by: Manorit Chawdhry 
---
 tools/dtoc/fdt.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index 5963925146a5..991a36b98796 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -782,7 +782,7 @@ class Node:
 for node in parent.subnodes.__reversed__():
 dst = self.copy_node(node)
 
-tout.debug(f'merge props from {parent.path} to {dst.path}')
+tout.debug(f'merge props from {parent.path} to {self.path}')
 self.merge_props(parent, False)
 
 

-- 
2.43.0



[PATCH v7 0/9] ATF and OP-TEE Firewalling for K3 devices.

2023-12-29 Thread Manorit Chawdhry
K3 devices have firewalls that are used to prevent illegal accesses to
memory regions that are deemed secure. The series prevents the illegal
accesses to ATF and OP-TEE regions that are present in different K3
devices. 

AM62X, AM62AX and AM64X are currently in hold due to some firewall
configurations that our System Controller (TIFS) needs to handle. 
The devices that are not configured with the firewalling nodes will not
be affected and can continue to work fine until the firewall nodes are
added so will be a non-blocking merge. 

Test Logs: https://gist.github.com/manorit2001/4cead2fb3a19eb5d19005b3f54682627
CICD Run: https://github.com/u-boot/u-boot/pull/442

Signed-off-by: Manorit Chawdhry 
---
Changes in v7:

* Andrew
- Update documentation
- Incorporate templating 

* Simon
- Change the prefix for -binman.dtsi files

* Jon
- Remove the unintentional dependency on python3.9+
  
(https://lore.kernel.org/all/CADL8D3ZWoZpMidBTy+iSs-KOB6+LRAFVcDa-n_fVqvd00Z0=n...@mail.gmail.com/)

- Add another patch to fix templating inclusion log
- Change headings level for secure boot documentation
- Populate 3 priv id slots for the background firewalls that require it
- Link to v6: 
https://lore.kernel.org/r/20231206-binman-firewalling-v6-0-e7fce13a6...@ti.com

---
Manorit Chawdhry (9):
  dtoc: Change dst to self in debug message
  binman: ti-secure: Add support for firewalling entities
  binman: ftest: Add test for ti-secure firewall node
  arm: dts: k3-binman: Add k3-security.h and include it in k3-binman.dtsi
  arm: dts: k3-j721e-binman: Add firewall configurations
  arm: dts: k3-j721s2-binman: Add firewall configurations
  arm: dts: k3-j7200-binman: Add firewall configurations
  docs: board: ti: k3: Cleanup FIT signature documentation
  docs: board: ti: k3: Add secure booting documentation

 arch/arm/dts/k3-binman.dtsi|  49 
 arch/arm/dts/k3-j7200-binman.dtsi  |  90 ++
 arch/arm/dts/k3-j721e-binman.dtsi  | 116 
 arch/arm/dts/k3-j721s2-binman.dtsi | 123 
 arch/arm/dts/k3-security.h |  58 
 doc/board/ti/k3.rst| 313 ++---
 tools/binman/btool/openssl.py  |  16 +-
 tools/binman/etype/ti_secure.py|  95 +++
 tools/binman/etype/x509_cert.py|   4 +-
 tools/binman/ftest.py  |  23 ++
 tools/binman/test/324_ti_secure_firewall.dts   |  28 ++
 .../325_ti_secure_firewall_missing_property.dts|  28 ++
 tools/dtoc/fdt.py  |   2 +-
 13 files changed, 842 insertions(+), 103 deletions(-)
---
base-commit: 2b28c3b871cd5d55b19f0a86cef970139f8ab952
change-id: 20230724-binman-firewalling-65ecdb23ec0a

Best regards,
-- 
Manorit Chawdhry 



[PATCH V4 3/3] configs: andes: add the fdt blob copy address for SPL

2023-12-29 Thread Randolph
Add the address to which the FDT blob is to be moved.

Signed-off-by: Randolph 
---
 configs/ae350_rv32_falcon_defconfig | 1 +
 configs/ae350_rv32_falcon_xip_defconfig | 1 +
 configs/ae350_rv64_falcon_defconfig | 1 +
 configs/ae350_rv64_falcon_xip_defconfig | 1 +
 4 files changed, 4 insertions(+)

diff --git a/configs/ae350_rv32_falcon_defconfig 
b/configs/ae350_rv32_falcon_defconfig
index 3f2993e371..c0837322bb 100644
--- a/configs/ae350_rv32_falcon_defconfig
+++ b/configs/ae350_rv32_falcon_defconfig
@@ -14,6 +14,7 @@ CONFIG_TARGET_ANDES_AE350=y
 CONFIG_RISCV_SMODE=y
 # CONFIG_AVAILABLE_HARTS is not set
 CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT=y
+CONFIG_SPL_PAYLOAD_ARGS_ADDR=0x0170
 CONFIG_SYS_MONITOR_BASE=0x8800
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x1000
diff --git a/configs/ae350_rv32_falcon_xip_defconfig 
b/configs/ae350_rv32_falcon_xip_defconfig
index e4f4c7807c..d401e4963b 100644
--- a/configs/ae350_rv32_falcon_xip_defconfig
+++ b/configs/ae350_rv32_falcon_xip_defconfig
@@ -15,6 +15,7 @@ CONFIG_TARGET_ANDES_AE350=y
 CONFIG_RISCV_SMODE=y
 CONFIG_SPL_XIP=y
 CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT=y
+CONFIG_SPL_PAYLOAD_ARGS_ADDR=0x0170
 CONFIG_SYS_MONITOR_BASE=0x8800
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x8001
diff --git a/configs/ae350_rv64_falcon_defconfig 
b/configs/ae350_rv64_falcon_defconfig
index 4fb83d8240..b4ae5f9848 100644
--- a/configs/ae350_rv64_falcon_defconfig
+++ b/configs/ae350_rv64_falcon_defconfig
@@ -14,6 +14,7 @@ CONFIG_ARCH_RV64I=y
 CONFIG_RISCV_SMODE=y
 # CONFIG_AVAILABLE_HARTS is not set
 CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT=y
+CONFIG_SPL_PAYLOAD_ARGS_ADDR=0x0170
 CONFIG_SYS_MONITOR_BASE=0x8800
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x1000
diff --git a/configs/ae350_rv64_falcon_xip_defconfig 
b/configs/ae350_rv64_falcon_xip_defconfig
index 45464260ee..c7c8324d30 100644
--- a/configs/ae350_rv64_falcon_xip_defconfig
+++ b/configs/ae350_rv64_falcon_xip_defconfig
@@ -15,6 +15,7 @@ CONFIG_ARCH_RV64I=y
 CONFIG_RISCV_SMODE=y
 CONFIG_SPL_XIP=y
 CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT=y
+CONFIG_SPL_PAYLOAD_ARGS_ADDR=0x0170
 CONFIG_SYS_MONITOR_BASE=0x8800
 CONFIG_FIT=y
 CONFIG_SPL_LOAD_FIT_ADDRESS=0x8001
-- 
2.34.1



[PATCH V4 2/3] spl: riscv: falcon: move fdt blob to specified address

2023-12-29 Thread Randolph
In Falcon Boot mode, the fdt blob should be move to the RAM from
kernel BSS section. To avoid being cleared by BSS initialisation.
SPL_PAYLOAD_ARGS_ADDR is the address where SPL copies.

Signed-off-by: Randolph 
---
 board/AndesTech/ae350/ae350.c | 25 -
 common/spl/Kconfig|  2 +-
 common/spl/spl_opensbi.c  | 15 +++
 3 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/board/AndesTech/ae350/ae350.c b/board/AndesTech/ae350/ae350.c
index 772c6bf1ee..36375d9def 100644
--- a/board/AndesTech/ae350/ae350.c
+++ b/board/AndesTech/ae350/ae350.c
@@ -19,8 +19,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -28,29 +26,6 @@ DECLARE_GLOBAL_DATA_PTR;
  * Miscellaneous platform dependent initializations
  */
 
-#if CONFIG_IS_ENABLED(LOAD_FIT) || CONFIG_IS_ENABLED(LOAD_FIT_FULL)
-#define ANDES_SPL_FDT_ADDR (CONFIG_TEXT_BASE - 0x10)
-void spl_perform_fixups(struct spl_image_info *spl_image)
-{
-   /*
-* Originally, u-boot-spl will place DTB directly after the kernel,
-* but the size of the kernel did not include the BSS section, which
-* means u-boot-spl will place the DTB in the kernel BSS section
-* causing the DTB to be cleared by kernel BSS initializtion.
-* Moving DTB in front of the kernel can avoid the error.
-*/
-   if (ANDES_SPL_FDT_ADDR < 0) {
-   printf("%s: CONFIG_TEXT_BASE needs to be larger than 
0x10\n",
-  __func__);
-   hang();
-   }
-
-   memcpy((void *)ANDES_SPL_FDT_ADDR, spl_image->fdt_addr,
-  fdt_totalsize(spl_image->fdt_addr));
-   spl_image->fdt_addr = map_sysmem(ANDES_SPL_FDT_ADDR, 0);
-}
-#endif
-
 int board_init(void)
 {
gd->bd->bi_boot_params = PHYS_SDRAM_0 + 0x400;
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index c521b02f4a..bb283d823e 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -1081,7 +1081,7 @@ config SPL_OS_BOOT
 
 config SPL_PAYLOAD_ARGS_ADDR
hex "Address in memory to load 'args' file for Falcon Mode to"
-   depends on SPL_OS_BOOT
+   depends on SPL_OS_BOOT || SPL_LOAD_FIT_OPENSBI_OS_BOOT
default 0x8800 if ARCH_OMAP2PLUS
help
  Address in memory where the 'args' file, typically a device tree
diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
index 9801d38c0b..8127ebc946 100644
--- a/common/spl/spl_opensbi.c
+++ b/common/spl/spl_opensbi.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -57,6 +58,20 @@ void __noreturn spl_invoke_opensbi(struct spl_image_info 
*spl_image)
hang();
}
 
+   /*
+* Originally, u-boot-spl will place DTB directly after the kernel,
+* but the size of the kernel did not include the BSS section, which
+* means u-boot-spl will place the DTB in the kernel BSS section
+* causing the DTB to be cleared by kernel BSS initializtion.
+* Moving DTB in front of the kernel can avoid the error.
+*/
+#if CONFIG_IS_ENABLED(LOAD_FIT_OPENSBI_OS_BOOT) && \
+CONFIG_IS_ENABLED(PAYLOAD_ARGS_ADDR)
+   memcpy((void *)CONFIG_SPL_PAYLOAD_ARGS_ADDR, spl_image->fdt_addr,
+  fdt_totalsize(spl_image->fdt_addr));
+   spl_image->fdt_addr = map_sysmem(CONFIG_SPL_PAYLOAD_ARGS_ADDR, 0);
+#endif
+
/*
 * Find next os image in /fit-images
 * The next os image default is u-boot proper, once enable
-- 
2.34.1



[PATCH V4 1/3] doc: falcon: riscv: Falcon Mode boot on RISC-V

2023-12-29 Thread Randolph
Add documentation to introduce the Falcon Mode on RISC-V.
In this mode, the boot sequence is SPL -> OpenSBI -> Linux kernel.

Signed-off-by: Randolph 
---
 doc/develop/falcon.rst | 158 +
 1 file changed, 158 insertions(+)

diff --git a/doc/develop/falcon.rst b/doc/develop/falcon.rst
index 8a46c0efa1..244b4ccb5c 100644
--- a/doc/develop/falcon.rst
+++ b/doc/develop/falcon.rst
@@ -256,3 +256,161 @@ the following command:
 Falcon Mode was presented at the RMLL 2012. Slides are available at:
 
 http://schedule2012.rmll.info/IMG/pdf/LSM2012_UbootFalconMode_Babic.pdf
+
+Falcon Mode Boot on RISC-V
+--
+
+Introduction
+
+
+In the RISC-V environment, OpenSBI is required to enable a supervisor mode
+binary to execute certain privileged operations. The typical boot sequence on
+RISC-V is SPL -> OpenSBI -> U-Boot -> Linux kernel. SPL will load and start
+the OpenSBI initializations, then OpenSBI will bring up the next image, U-Boot
+proper. The OpenSBI binary must be prepared in advance of the U-Boot build
+process and it will be packed together with U-Boot into a file called
+u-boot.itb.
+
+The Falcon Mode on RISC-V platforms is a distinct boot sequence. Borrowing
+ideas from the U-Boot Falcon Mode on ARM, it skips the U-Boot proper phase
+in the normal boot process and allows OpenSBI to load and start the Linux
+kernel. Its boot sequence is SPL -> OpenSBI -> Linux kernel. The OpenSBI
+binary and Linux kernel binary must be prepared prior to the U-Boot build
+process and they will be packed together as a FIT image named linux.itb in
+this process.
+
+CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT enables the Falcon Mode boot on RISC-V.
+This configuration setting tells OpenSBI that Linux kernel is its next OS
+image and makes it load and start the kernel afterwards.
+
+Note that the Falcon Mode boot bypasses a lot of initializations by U-Boot.
+If the Linux kernel expects hardware initializations by U-Boot, make sure to
+port the relevant code to the SPL build process.
+
+Configuration
+~
+
+CONFIG_SPL_LOAD_FIT_ADDRESS
+Specifies the address to load u-boot.itb in a normal boot. When the Falcon
+Mode boot is enabled, it specifies the load address of linux.itb.
+
+CONFIG_SYS_TEXT_BASE
+Specifies the address of the text section for a u-boot proper in a normal
+boot. When the Falcon Mode boot is enabled, it specifies the text section
+address for the Linux kernel image.
+
+CONFIG_SPL_PAYLOAD_ARGS_ADDR
+The address in the RAM to which the FDT blob is to be moved by the SPL.
+SPL places the FDT blob right after the kernel. As the kernel does not
+include the BSS section in its size calculation, SPL ends up placing
+the FDT blob within the BSS section of the kernel. This may cause the
+FDT blob to be cleared during kernel BSS initialization. To avoid the
+issue, be sure to move the FDT blob out of the kernel first.
+
+CONFIG_SPL_LOAD_FIT_OPENSBI_OS_BOOT
+Activates the Falcon Mode boot on RISC-V.
+
+Example for Andes AE350 Board
+~
+
+A FDT blob is required to boot the Linux kernel from the SPL. Andes AE350
+platforms generally come with a builtin dtb. To load a custom DTB, follow
+these steps:
+
+1. Load the custom DTB to SDRAM::
+
+=> fatload mmc 0:1 0x2000 user_custom.dtb
+
+2. Set the SPI speed::
+
+=> sf probe 0:0 5000 0
+
+3. Erase sectors from the SPI Flash::
+
+=> sf erase 0xf 0x1
+
+4. Write the FDT blob to the erased sectors of the Flash::
+
+=> sf write 0x2000 0xf 0x1
+
+Console Log of AE350 Falcon Mode Boot
+~
+
+::
+
+U-Boot SPL 2023.01-00031-g777ecdea66 (Oct 31 2023 - 18:41:36 +0800)
+Trying to boot from RAM
+
+OpenSBI v1.2-51-g7304e42
+   _  _
+  / __ \  / |  _ \_   _|
+ | |  | |_ __   ___ _ __ | (___ | |_) || |
+ | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
+ | |__| | |_) |  __/ | | |) | |_) || |_
+  \/| .__/ \___|_| |_|_/|/_|
+| |
+|_|
+
+Platform Name : andestech,ax25
+Platform Features : medeleg
+Platform HART Count   : 1
+Platform IPI Device   : andes_plicsw
+Platform Timer Device : andes_plmt @ 6000Hz
+Platform Console Device   : uart8250
+Platform HSM Device   : andes_smu
+Platform PMU Device   : andes_pmu
+Platform Reboot Device: atcwdt200
+Platform Shutdown Device  : ---
+Firmware Base : 0x0
+Firmware Size : 196 KB
+Runtime SBI Version   : 1.0
+
+Domain0 Name  : root
+Domain0 Boot HART : 0
+Domain0 HARTs : 0*
+Domain0 Region00  : 

[PATCH V4 0/3] doc: falcon: riscv: Falcon Mode boot on RISC-V

2023-12-29 Thread Randolph
Changes in v4:
- remove "Function that a board must implement" section in
  falcon.rst
- fix compile error when config LOAD_FIT_OPENSBI_OS_BOOT
  not enabled.

Changes in v3:
- Change by suggestions in falcon.rst 
- Move the board-related code to arch-specific code,
  its the issue when enabling LOAD_FIT_OPENSBI_OS_BOOT
- Add SPL_PAYLOAD_ARGS_ADDR to defconfig.
  This is the address that SPL copies into defconfig.

Randolph (3):
  doc: falcon: riscv: Falcon Mode boot on RISC-V
  spl: riscv: falcon: move fdt blob to specified address
  configs: andes: add the fdt blob copy address for SPL

 board/AndesTech/ae350/ae350.c   |  25 
 common/spl/Kconfig  |   2 +-
 common/spl/spl_opensbi.c|  15 +++
 configs/ae350_rv32_falcon_defconfig |   1 +
 configs/ae350_rv32_falcon_xip_defconfig |   1 +
 configs/ae350_rv64_falcon_defconfig |   1 +
 configs/ae350_rv64_falcon_xip_defconfig |   1 +
 doc/develop/falcon.rst  | 158 
 8 files changed, 178 insertions(+), 26 deletions(-)

-- 
2.34.1



Re: ACPI: usage of sandbox virtual addresses

2023-12-29 Thread Simon Glass
Hi Heinrich,

On Tue, Dec 26, 2023 at 10:03 AM Heinrich Schuchardt  wrote:
>
> On 12/26/23 10:50, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Tue, Dec 26, 2023 at 8:56 AM Heinrich Schuchardt  
> > wrote:
> >>
> >> Hello Simon,
> >>
> >> currently we use sandbox virtual addresses in all ACPI tables. This
> >> means that an application started by the U-Boot sandbox consuming these
> >> tables will crash due to accessing invalid addresses.
> >>
> >> Shouldn't the ACPI tables be migrated to use valid pointers?
> >
> > The confusion arises due to the difference between addresses and
> > pointers. If we store addresses in the ACPI tables, then things should
> > work.
> >
> > My approach has been to avoid using casts, but instead use
> > map_sysmem() and mem_to_sysmem().
> >
> > Which particular piece of code is wrong in this case?
>
> Tables DSDT, XSDT, RSDP, FADT in the sandbox contain sandbox virtual
> addresses instead of pointers to real host memory.
>
> All code referring to these tables should be changed.

I'm still not quite clear on this...can you point to functions or
lines of code? When I look at acpi_add_table() it does use memmap, but
perhaps some parts are wrong?

Regards,
Simon