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

[Jiamei Xie <Jiamei.Xie@xxxxxxx>]

Hi Stefano and Michal,

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Sent: Friday, March 3, 2023 7:42 AM
> To: Michal Orzel <michal.orzel@xxxxxxx>
> Cc: Jiamei Xie <Jiamei.Xie@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei
> Chen <Wei.Chen@xxxxxxx>; sstabellini@xxxxxxxxxx
> Subject: Re: [ImageBuilder][PATCH 1/2] uboot-script-gen: Add support for
> static heap
> 
> 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.
Thanks for your comments . Ack
> >
> > > +
> > >  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)")

Thanks for your comments . Ack.
> >
> > ~Michal
> >