[1/1] arm: apple: Switch to fully dynamic mem layout

Support for Apple M1 Pro and Max will allow using a single binary for
all M1 SoCs. The M1 Pro/Max have a different memory layout. The RAM
start address is 0x100_0000_0000 instead of 0x8_0000_0000.
Replace the hardcoded memory layout with dynamic initialized
environment variables in board_late_init().

Tested on Mac Mini (2020) and Macbook Pro 14-inch (2021).

Signed-off-by: Janne Grunau <j@jannau.net>
---
 arch/arm/mach-apple/board.c | 24 ++++++++++++++++++++++++
 configs/apple_m1_defconfig  |  1 +
 include/configs/apple.h     |  5 -----
 3 files changed, 25 insertions(+), 5 deletions(-)

On 2022-02-06 18:02:43 -0500, Tom Rini wrote:
> On Sun, Feb 06, 2022 at 10:07:04PM +0100, Janne Grunau wrote:
> 
> > Support for Apple M1 Pro and Max will allow using a single binary for
> > all M1 SoCs. The M1 Pro/Max have a different memory layout. The RAM
> > start address is 0x100_0000_0000 instead of 0x8_0000_0000.
> > Replace the hardcoded memory layout with dynamic initialized
> > environment variables in board_late_init().
> > 
> > Tested on Mac Mini (2020) and Macbook Pro 14-inch (2021).
> > 
> > Signed-off-by: Janne Grunau <j@jannau.net>
> [snip]
> > +int board_late_init(void)
> > +{
> > +        ulong max_size;
> > +        u32 status = 0;
> > +
> > +        max_size = gd->start_addr_sp - CONFIG_STACK_SIZE;
> > +        max_size = round_down(max_size, SZ_16M);
> > +
> > +        status |= env_set_hex("scriptaddr", max_size + SZ_2M);
> > +
> > +        status |= env_set_hex("pxefile_addr_r", max_size + SZ_1M);
> > +
> > +        status |= env_set_hex("fdt_addr_r", gd->ram_base + SZ_32M - SZ_1M);
> > +
> > +        status |= env_set_hex("kernel_addr_r", gd->ram_base + SZ_32M);
> > +
> > +        status |= env_set_hex("ramdisk_addr_r", gd->ram_base + SZ_64M);
> > +
> > +        if (status)
> > +                printf("%s: Saving run time variables FAILED\n", __func__);
> 
> This should be "Setting" rather than saving, since we don't (and
> shouldn't) save the environment here.

yes, sorry I forgot I wanted to change that. I suppose log_warning() 
would be more appropiate than printf. Changed locally in addition to 
using tabs for indentation. I'll wait a little while before I send a 
fixed v2.

Thanks,

Janne

> From: Janne Grunau <j@jannau.net>
> Date: Sun,  6 Feb 2022 22:07:04 +0100
> 
> Support for Apple M1 Pro and Max will allow using a single binary for
> all M1 SoCs. The M1 Pro/Max have a different memory layout. The RAM
> start address is 0x100_0000_0000 instead of 0x8_0000_0000.
> Replace the hardcoded memory layout with dynamic initialized
> environment variables in board_late_init().

Thanks,

That's one of the things on my todo list done ;).

A few questions/issues/nits though:

- This doesn't set "loadaddr" in a dynamic way.  That is used
  internally by some of the u-boot commands, so it probably should be
  addes as well.  Currently "loadaddr" is set to the value of
  CONFIG_SYS_LOAD_ADDR which is a required option.  That should
  probably be fixed, but that's a diff for another day.  We may want
  to set it to 0 to make it obvious that it needs to ve overridden.

- Since gd->ram_base isn't necessarily aligned to more than a page
  boundary, the generated addresses aren't aligned very much.  I'm not
  sure what the requirements are exactly, but this could be
  problematic for "kernel_addr_r" and "ramdisk_addr_r".

- Whitespace in board_late_init() is a bit excessive.  Personaly I
  would group the env_set_hex() calls together, perhaps in two groups
  based on whether they are defined in terms of max_size or in terms
  of gd->ram_base.

- Is the way you construct max_size safe?  If the lowest address of
  the stack is already happens to be aligned on a 16MB boundary
  loading something into "scriptaddr" or "pxefile_addr_r" may
  overwrite the stack isn't it?

- We may need to revise the addresses in the future when support for
  PCIe is added.  This depends on how we're going to handle the DART
  since these addresses need to be DMA-reachable.

> 
> Tested on Mac Mini (2020) and Macbook Pro 14-inch (2021).
> 
> Signed-off-by: Janne Grunau <j@jannau.net>
> ---
>  arch/arm/mach-apple/board.c | 24 ++++++++++++++++++++++++
>  configs/apple_m1_defconfig  |  1 +
>  include/configs/apple.h     |  5 -----
>  3 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c
> index b7e8d212f1..09a75a6e4e 100644
> --- a/arch/arm/mach-apple/board.c
> +++ b/arch/arm/mach-apple/board.c
> @@ -136,3 +136,27 @@ ulong board_get_usable_ram_top(ulong total_size)
>  	 */
>  	return 0x980000000;
>  }
> +
> +int board_late_init(void)
> +{
> +        ulong max_size;
> +        u32 status = 0;
> +
> +        max_size = gd->start_addr_sp - CONFIG_STACK_SIZE;
> +        max_size = round_down(max_size, SZ_16M);
> +
> +        status |= env_set_hex("scriptaddr", max_size + SZ_2M);
> +
> +        status |= env_set_hex("pxefile_addr_r", max_size + SZ_1M);
> +
> +        status |= env_set_hex("fdt_addr_r", gd->ram_base + SZ_32M - SZ_1M);
> +
> +        status |= env_set_hex("kernel_addr_r", gd->ram_base + SZ_32M);
> +
> +        status |= env_set_hex("ramdisk_addr_r", gd->ram_base + SZ_64M);
> +
> +        if (status)
> +                printf("%s: Saving run time variables FAILED\n", __func__);
> +
> +	return 0;
> +}
> diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
> index cb235e4e7d..ad49e2be65 100644
> --- a/configs/apple_m1_defconfig
> +++ b/configs/apple_m1_defconfig
> @@ -8,6 +8,7 @@ CONFIG_SYS_LOAD_ADDR=0x880000000
>  CONFIG_USE_PREBOOT=y
>  # CONFIG_DISPLAY_CPUINFO is not set
>  CONFIG_DISPLAY_BOARDINFO_LATE=y
> +CONFIG_BOARD_LATE_INIT=y
>  # CONFIG_NET is not set
>  # CONFIG_MMC is not set
>  CONFIG_DEBUG_UART_ANNOUNCE=y
> diff --git a/include/configs/apple.h b/include/configs/apple.h
> index 3e5fb495f1..f73f4f047b 100644
> --- a/include/configs/apple.h
> +++ b/include/configs/apple.h
> @@ -9,10 +9,6 @@
>  	"stdout=serial,vidconsole\0" \
>  	"stderr=serial,vidconsole\0"
>  
> -#define ENV_MEM_LAYOUT_SETTINGS \
> -	"fdt_addr_r=0x960100000\0" \
> -	"kernel_addr_r=0x960200000\0"
> -
>  #if CONFIG_IS_ENABLED(CMD_USB)
>  	#define BOOT_TARGET_USB(func) func(USB, usb, 0)
>  #else
> @@ -26,7 +22,6 @@
>  
>  #define CONFIG_EXTRA_ENV_SETTINGS \
>  	ENV_DEVICE_SETTINGS \
> -	ENV_MEM_LAYOUT_SETTINGS \
>  	BOOTENV
>  
>  #endif
> -- 
> 2.35.1
> 
>