Re: [PATCH 13/15] mach-snapdragon: support parsing memory map from SMEM

[Stephan Gerhold <stephan.gerhold@linaro.org>]

On Sun, Nov 24, 2024 at 08:17:55PM +0100, Caleb Connolly wrote:
> It is possible to derive the memory map for a Qualcomm platform from the
> SMEM shared memory region. The memory map is populated by the preloader.
> 
> Introduce support for parsing this data and using it to populate
> U-Boot's memory map. Since we aren't yet sure if this will work for
> every platform, it is not yet used in all cases, if U-Boot is booted
> with an internal FDT which has the memory map defined, this will
> be used instead.
> 
> If the FDT comes from ABL, or we're using an internal FDT with no memory
> map defined, then U-Boot will try to use SMEM. This should remove the
> need to define the memory map statically in a U-Boot overlay DT for most
> boards.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  arch/arm/mach-snapdragon/board.c     |   2 +-
>  arch/arm/mach-snapdragon/dram.c      | 106 +++++++++++++++++++++++++++++++++--
>  arch/arm/mach-snapdragon/qcom-priv.h |   4 +-
>  3 files changed, 106 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
> index 8d171957b852..269d39e4f6e1 100644
> --- a/arch/arm/mach-snapdragon/board.c
> +++ b/arch/arm/mach-snapdragon/board.c
> @@ -85,9 +85,9 @@ void *board_fdt_blob_setup(int *err)
>  	/*
>  	 * Parse the /memory node while we're here,
>  	 * this makes it easy to do other things early.
>  	 */
> -	qcom_parse_memory();
> +	qcom_parse_memory(internal_valid);
>  
>  	return (void *)gd->fdt_blob;
>  }
>  
> diff --git a/arch/arm/mach-snapdragon/dram.c b/arch/arm/mach-snapdragon/dram.c
> index c4c60039cb4c..ef226e000858 100644
> --- a/arch/arm/mach-snapdragon/dram.c
> +++ b/arch/arm/mach-snapdragon/dram.c
> @@ -9,14 +9,47 @@
>  #include <asm-generic/unaligned.h>
>  #include <dm.h>
>  #include <log.h>
>  #include <sort.h>
> +#include <soc/qcom/smem.h>
> +
> +#define SMEM_USABLE_RAM_PARTITION_TABLE 402
> +#define RAM_PART_NAME_LENGTH            16
> +#define RAM_NUM_PART_ENTRIES            32
> +#define CATEGORY_SDRAM 0x0E
> +#define TYPE_SYSMEM 0x01
>  
>  static struct {
>  	phys_addr_t start;
>  	phys_size_t size;
>  } prevbl_ddr_banks[CONFIG_NR_DRAM_BANKS] __section(".data") = { 0 };
>  
> +struct smem_ram_ptable_hdr {
> +	u32 magic[2];
> +	u32 version;
> +	u32 reserved;
> +	u32 len;
> +} __packed;
> +
> +struct smem_ram_ptn {
> +	char name[RAM_PART_NAME_LENGTH];
> +	u64 start;
> +	u64 size;
> +	u32 attr;
> +	u32 category;
> +	u32 domain;
> +	u32 type;
> +	u32 num_partitions;
> +	u32 reserved[3];
> +	u32 reserved2[2]; /* The struct grew by 8 bytes at some point */
> +} __packed;

This looks like smem_ram_ptn_v2, as defined in LK:
https://github.com/msm8916-mainline/lk2nd/blob/5e6f1adcb1c070e28a70dc1f3d807ce9538f61c5/platform/msm_shared/smem.h#L620-L660

We need to add a check for smem_ram_ptable_hdr->version and handle the
older structs as well. DB410c has version 1, which is missing the 8
bytes noted in the comment above. This makes it misbehave quite badly...

If I drop the u32 reserved2[2] above it works well though, and correctly
handles variants of DB410c with 2 GiB instead of 1 GiB RAM.

Perhaps it's enough to handle _v1 and _v2 for now. v0 has 32-bit
start/size fields on old SoCs, not sure if anyone is trying to run
U-Boot on these at the moment.

Thanks,
Stephan