Re: [ImageBuilder][PATCH 1/2] uboot-script-gen: Add support for static heap

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Jiamei,

Patch looks good apart from minor comments down below.

On 02/03/2023 05:46, jiamei.xie wrote:
> 
> 
> From: jiamei Xie <jiamei.xie@xxxxxxx>
> 
> Add a new config parameter to configure static heap.
> STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
> if specified, indicates the host physical address regions
> [baseaddr, baseaddr + size) to be reserved as static heap.
> 
> For instance, STATIC_HEAP="0x50000000 0x30000000", if specified,
> indicates the host memory region starting from paddr 0x50000000
> with a size of 0x30000000 to be reserved as static heap.
> 
> Signed-off-by: jiamei Xie <jiamei.xie@xxxxxxx>
> ---
>  README.md                |  4 ++++
>  scripts/uboot-script-gen | 20 ++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/README.md b/README.md
> index 814a004..787f413 100644
> --- a/README.md
> +++ b/README.md
> @@ -256,6 +256,10 @@ Where:
> 
>  - NUM_CPUPOOLS specifies the number of boot-time cpupools to create.
> 
> +- STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
> +  if specified, indicates the host physical address regions
> +  [baseaddr, baseaddr + size) to be reserved as static heap.
As this option impacts Xen and not domUs, please call it XEN_STATIC_HEAP and 
move
it right after XEN_CMD documentation.

> +
>  Then you can invoke uboot-script-gen as follows:
> 
>  ```
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index f07e334..4775293 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -189,6 +189,21 @@ function add_device_tree_static_mem()
>      dt_set "$path" "xen,static-mem" "hex" "${cells[*]}"
>  }
> 
> +function add_device_tree_static_heap()
> +{
> +    local path=$1
> +    local regions=$2
> +    local cells=()
> +    local val
> +
> +    for val in ${regions[@]}
> +    do
> +        cells+=("$(printf "0x%x 0x%x" $(($val >> 32)) $(($val & ((1 << 32) - 
> 1))))")
Please use split_value function instead of opencoding it.
It will then become:
cells+=("$(split_value $val)")

~Michal