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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 2 Mar 2023, Michal Orzel wrote:
> Hi Jiamei,
> 
> Patch looks good apart from minor comments down below.

Just wanted to add that the patch looks OK to me too and don't have any
further comments beyond the ones Michal's already made


> 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
>