| Message ID | 20230302044606.136130-2-jiamei.xie@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | uboot-script-gen: Add support for static heap and shared memory | expand |
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@arm.com> > > 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@arm.com> > --- > 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
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@arm.com> > > > > 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@arm.com> > > --- > > 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 >
Hi Stefano and Michal, > -----Original Message----- > From: Stefano Stabellini <sstabellini@kernel.org> > Sent: Friday, March 3, 2023 7:42 AM > To: Michal Orzel <michal.orzel@amd.com> > Cc: Jiamei Xie <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org; Wei > Chen <Wei.Chen@arm.com>; sstabellini@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 <jiamei.xie@arm.com> > > > > > > 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@arm.com> > > > --- > > > 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 > >
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()