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

2023-03-02 Thread Jiamei Xie 
Hi Stefano and Michal,

> -Original Message-
> From: Stefano Stabellini 
> Sent: Friday, March 3, 2023 7:42 AM
> To: Michal Orzel 
> Cc: Jiamei Xie ; xen-devel@lists.xenproject.org; Wei
> Chen ; sstabell...@kernel.org
> 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 
> > >
> > > 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="0x5000 0x3000", if specified,
> > > indicates the host memory region starting from paddr 0x5000
> > > with a size of 0x3000 to be reserved as static heap.
> > >
> > > Signed-off-by: jiamei Xie 
> > > ---
> > >  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
> >

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

2023-03-02 Thread Stefano Stabellini 
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 
> > 
> > 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="0x5000 0x3000", if specified,
> > indicates the host memory region starting from paddr 0x5000
> > with a size of 0x3000 to be reserved as static heap.
> > 
> > Signed-off-by: jiamei Xie 
> > ---
> >  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
>

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

2023-03-02 Thread Michal Orzel 
Hi Jiamei,

Patch looks good apart from minor comments down below.

On 02/03/2023 05:46, jiamei.xie wrote:
> 
> 
> From: jiamei Xie 
> 
> 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="0x5000 0x3000", if specified,
> indicates the host memory region starting from paddr 0x5000
> with a size of 0x3000 to be reserved as static heap.
> 
> Signed-off-by: jiamei Xie 
> ---
>  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

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

2023-03-01 Thread jiamei.xie 
From: jiamei Xie 

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="0x5000 0x3000", if specified,
indicates the host memory region starting from paddr 0x5000
with a size of 0x3000 to be reserved as static heap.

Signed-off-by: jiamei Xie 
---
 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.
+
 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")
+done
+
+dt_set "$path" "xen,static-heap" "hex" "${cells[*]}"
+}
+
 function add_device_tree_cpupools()
 {
 local cpu
@@ -344,6 +359,11 @@ function xen_device_tree_editing()
 then
 add_device_tree_cpupools
 fi
+
+if test "${STATIC_HEAP}"
+then
+add_device_tree_static_heap "/chosen" "${STATIC_HEAP}"
+fi
 }
 
 function linux_device_tree_editing()
-- 
2.25.1