Re: [PATCH 14/21] mach-snapdragon: dynamic load addresses

2023-11-21 Thread Caleb Connolly



On 21/11/2023 19:24, Tom Rini wrote:
> On Tue, Nov 21, 2023 at 05:09:37PM +, Caleb Connolly wrote:
> 
>> Heavily inspired by Apple board code. Use the LMB allocator to configure
>> load addresses at runtime, and implement a lookup table for selecting a
>> devicetree.
>>
>> As some Qualcomm RBx boards have different RAM capacities and base
>> addresses, it isn't possible to hardcode these regions.
>>
>> Signed-off-by: Caleb Connolly 
> [snip]
>> +#define KERNEL_COMP_SIZESZ_32M
>> +#define SZ_96M  (SZ_64M + SZ_32M)
>> +
>> +#define addr_alloc(lmb, size) lmb_alloc(lmb, size, SZ_2M)
>> +
>> +/* Stolen from arch/arm/mach-apple/board.c */
>> +int board_late_init(void)
>> +{
>> +struct lmb lmb;
>> +u32 status = 0;
>> +
>> +lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
>> +
>> +/* We need to be fairly conservative here as we support boards with 
>> just 1G of TOTAL RAM */
>> +status |= env_set_hex("kernel_addr_r", addr_alloc(&lmb, SZ_96M));
>> +status |= env_set_hex("loadaddr", addr_alloc(&lmb, SZ_64M));
>> +status |= env_set_hex("fdt_addr_r", addr_alloc(&lmb, SZ_2M));
>> +status |= env_set_hex("ramdisk_addr_r", addr_alloc(&lmb, SZ_96M));
>> +status |= env_set_hex("kernel_comp_addr_r", addr_alloc(&lmb, 
>> KERNEL_COMP_SIZE));
>> +status |= env_set_hex("kernel_comp_size", KERNEL_COMP_SIZE);
>> +status |= env_set_hex("scriptaddr", addr_alloc(&lmb, SZ_4M));
>> +status |= env_set_hex("pxefile_addr_r", addr_alloc(&lmb, SZ_4M));
> 
> First, with 1G total, we should I think be fine using SZ_128M and not
> needing to add SZ_96M. Second, having worked on
> https://patchwork.ozlabs.org/project/uboot/patch/20231119144338.630138-2-...@chromium.org/
> a bit I think your sizes are a bit funny. We're looking at where to
> decompress the provided Image file to and so 32M is too small. My
> generic v6.6 Image is 40MB. One of those "would be nice" things to see
> now that I've kicked things around a little was if all of our boot$foo
> decompression logic just asked the lmb to give us a new region of
> max-feasible-size for a Linux kernel (or we make it a choice between 64
> or 128MB max supported kernel image). And further looking at the code
> and matching it up with the function docs, it's even funkier than I had
> thought, sigh.

To your first point, yeah it looks like I can get away with 128M
uncompressed, it definitely feels a bit safer heh

To be honest, most of my testing has been booting via bootflow/bootefi
-> systemd-boot which I assume will be using the EFI allocator to load
everything. I still have a lot of local patches to push upstream,
otherwise I'd definitely be interested in finding a cleaner / more
generic solution here.
> 
> And to try and be clear, the second is an ask, but the first is a change
> request, those sizes are too small and inconsistent with supporting a
> large uncompressed kernel.
> 

-- 
// Caleb (they/them)


Re: [PATCH 14/21] mach-snapdragon: dynamic load addresses

2023-11-21 Thread Tom Rini
On Tue, Nov 21, 2023 at 05:09:37PM +, Caleb Connolly wrote:

> Heavily inspired by Apple board code. Use the LMB allocator to configure
> load addresses at runtime, and implement a lookup table for selecting a
> devicetree.
> 
> As some Qualcomm RBx boards have different RAM capacities and base
> addresses, it isn't possible to hardcode these regions.
> 
> Signed-off-by: Caleb Connolly 
[snip]
> +#define KERNEL_COMP_SIZE SZ_32M
> +#define SZ_96M   (SZ_64M + SZ_32M)
> +
> +#define addr_alloc(lmb, size) lmb_alloc(lmb, size, SZ_2M)
> +
> +/* Stolen from arch/arm/mach-apple/board.c */
> +int board_late_init(void)
> +{
> + struct lmb lmb;
> + u32 status = 0;
> +
> + lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
> +
> + /* We need to be fairly conservative here as we support boards with 
> just 1G of TOTAL RAM */
> + status |= env_set_hex("kernel_addr_r", addr_alloc(&lmb, SZ_96M));
> + status |= env_set_hex("loadaddr", addr_alloc(&lmb, SZ_64M));
> + status |= env_set_hex("fdt_addr_r", addr_alloc(&lmb, SZ_2M));
> + status |= env_set_hex("ramdisk_addr_r", addr_alloc(&lmb, SZ_96M));
> + status |= env_set_hex("kernel_comp_addr_r", addr_alloc(&lmb, 
> KERNEL_COMP_SIZE));
> + status |= env_set_hex("kernel_comp_size", KERNEL_COMP_SIZE);
> + status |= env_set_hex("scriptaddr", addr_alloc(&lmb, SZ_4M));
> + status |= env_set_hex("pxefile_addr_r", addr_alloc(&lmb, SZ_4M));

First, with 1G total, we should I think be fine using SZ_128M and not
needing to add SZ_96M. Second, having worked on
https://patchwork.ozlabs.org/project/uboot/patch/20231119144338.630138-2-...@chromium.org/
a bit I think your sizes are a bit funny. We're looking at where to
decompress the provided Image file to and so 32M is too small. My
generic v6.6 Image is 40MB. One of those "would be nice" things to see
now that I've kicked things around a little was if all of our boot$foo
decompression logic just asked the lmb to give us a new region of
max-feasible-size for a Linux kernel (or we make it a choice between 64
or 128MB max supported kernel image). And further looking at the code
and matching it up with the function docs, it's even funkier than I had
thought, sigh.

And to try and be clear, the second is an ask, but the first is a change
request, those sizes are too small and inconsistent with supporting a
large uncompressed kernel.

-- 
Tom


signature.asc
Description: PGP signature


[PATCH 14/21] mach-snapdragon: dynamic load addresses

2023-11-21 Thread Caleb Connolly
Heavily inspired by Apple board code. Use the LMB allocator to configure
load addresses at runtime, and implement a lookup table for selecting a
devicetree.

As some Qualcomm RBx boards have different RAM capacities and base
addresses, it isn't possible to hardcode these regions.

Signed-off-by: Caleb Connolly 
---
 arch/arm/Kconfig |  1 +
 arch/arm/mach-snapdragon/board.c | 36 
 board/qualcomm/dragonboard410c/dragonboard410c.c |  2 +-
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1faa70bb658d..ae44fabb1609 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1075,6 +1075,7 @@ config ARCH_SNAPDRAGON
select OF_SEPARATE
select SMEM
select SPMI
+   select BOARD_LATE_INIT
select OF_BOARD
select SAVE_PREV_BL_FDT_ADDR
imply CMD_DM
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
index 521390ed6eed..765a2c2a95fc 100644
--- a/arch/arm/mach-snapdragon/board.c
+++ b/arch/arm/mach-snapdragon/board.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -110,6 +111,41 @@ int board_init(void)
return 0;
 }
 
+void __weak qcom_late_init(void)
+{
+}
+
+#define KERNEL_COMP_SIZE   SZ_32M
+#define SZ_96M (SZ_64M + SZ_32M)
+
+#define addr_alloc(lmb, size) lmb_alloc(lmb, size, SZ_2M)
+
+/* Stolen from arch/arm/mach-apple/board.c */
+int board_late_init(void)
+{
+   struct lmb lmb;
+   u32 status = 0;
+
+   lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
+
+   /* We need to be fairly conservative here as we support boards with 
just 1G of TOTAL RAM */
+   status |= env_set_hex("kernel_addr_r", addr_alloc(&lmb, SZ_96M));
+   status |= env_set_hex("loadaddr", addr_alloc(&lmb, SZ_64M));
+   status |= env_set_hex("fdt_addr_r", addr_alloc(&lmb, SZ_2M));
+   status |= env_set_hex("ramdisk_addr_r", addr_alloc(&lmb, SZ_96M));
+   status |= env_set_hex("kernel_comp_addr_r", addr_alloc(&lmb, 
KERNEL_COMP_SIZE));
+   status |= env_set_hex("kernel_comp_size", KERNEL_COMP_SIZE);
+   status |= env_set_hex("scriptaddr", addr_alloc(&lmb, SZ_4M));
+   status |= env_set_hex("pxefile_addr_r", addr_alloc(&lmb, SZ_4M));
+
+   if (status)
+   log_warning("%s: Failed to set run time variables\n", __func__);
+
+   qcom_late_init();
+
+   return 0;
+}
+
 static void build_mem_map(void)
 {
int i;
diff --git a/board/qualcomm/dragonboard410c/dragonboard410c.c 
b/board/qualcomm/dragonboard410c/dragonboard410c.c
index bee8034ef92f..ab652e0a7c92 100644
--- a/board/qualcomm/dragonboard410c/dragonboard410c.c
+++ b/board/qualcomm/dragonboard410c/dragonboard410c.c
@@ -113,7 +113,7 @@ int misc_init_r(void)
return 0;
 }
 
-int board_late_init(void)
+int qcom_late_init(void)
 {
char serial[16];
 

-- 
2.42.1