[ImageBuilder][PATCH v2 0/2] uboot-script-gen: Add support for static heap and shared memory

[ImageBuilder][PATCH v2 0/2] uboot-script-gen: Add support for static heap and shared memory

[jiamei.xie]

This patch series is to enable to set the device tree for static heap and static
shared memory by uboot-script-gen

Changes from v1:
 - Rename STATIC_HEAP to XEN_STATIC_HEAP and move it right after
   XEN_CMD documentation.
 - Rather than two separate properties and just use one like follows:
   Change
     DOMU_SHARED_MEM[0]="0x50000000 0x6000000 0x10000000"
     DOMU_SHARED_MEM_ID[0]="my-shared-mem-0"
   to
     DOMU_SHARED_MEM[0]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
- Use split_value function instead of opencoding it.

jiamei Xie (1):
  uboot-script-gen: Add XEN_STATIC_HEAP

jiamei.xie (1):
  uboot-script-gen: add support for static shared memory

 README.md                | 21 ++++++++++++++++++
 scripts/uboot-script-gen | 47 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

-- 
2.25.1

[ImageBuilder][PATCH v2 1/2] uboot-script-gen: Add XEN_STATIC_HEAP

[jiamei.xie]

From: jiamei Xie <jiamei.xie@arm.com>

Add a new config parameter to configure Xen static heap.
XEN_STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
if specified, indicates the host physical address regions
[baseaddr, baseaddr + size) to be reserved as Xen static heap.

For instance, XEN_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>
---
Changes from v1:
 - Rename STATIC_HEAP to XEN_STATIC_HEAP and move it right after
   XEN_CMD documentation.
 - Use split_value function instead of opencoding it.
---
 README.md                |  4 ++++
 scripts/uboot-script-gen | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/README.md b/README.md
index 814a004..78b83f1 100644
--- a/README.md
+++ b/README.md
@@ -97,6 +97,10 @@ Where:
 - XEN_CMD specifies the command line arguments used for Xen.  If not
   set, the default one will be used.
 
+- XEN_STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
+  if specified, indicates the host physical address regions
+  [baseaddr, baseaddr + size) to be reserved as Xen static heap.
+
 - PASSTHROUGH_DTS_REPO specifies the git repository and/or the directory
   which contains the partial device trees. This is optional. However, if
   this is specified, then DOMU_PASSTHROUGH_PATHS[number] need to be specified.
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index f07e334..cca3e59 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_xen_static_heap()
+{
+    local path=$1
+    local regions=$2
+    local cells=()
+    local val
+
+    for val in ${regions[@]}
+    do
+        cells+=("$(split_value $val)")
+    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 "${XEN_STATIC_HEAP}"
+    then
+        add_device_tree_xen_static_heap "/chosen" "${XEN_STATIC_HEAP}"
+    fi
 }
 
 function linux_device_tree_editing()
-- 
2.25.1

Re: [ImageBuilder][PATCH v2 1/2] uboot-script-gen: Add XEN_STATIC_HEAP

[Michal Orzel]

On 16/03/2023 10:09, jiamei.xie wrote:
> 
> 
> From: jiamei Xie <jiamei.xie@arm.com>
> 
> Add a new config parameter to configure Xen static heap.
> XEN_STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
> if specified, indicates the host physical address regions
> [baseaddr, baseaddr + size) to be reserved as Xen static heap.
> 
> For instance, XEN_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>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [ImageBuilder][PATCH v2 1/2] uboot-script-gen: Add XEN_STATIC_HEAP

[Stefano Stabellini]

On Thu, 16 Mar 2023, Michal Orzel wrote:
> On 16/03/2023 10:09, jiamei.xie wrote:
> > From: jiamei Xie <jiamei.xie@arm.com>
> > 
> > Add a new config parameter to configure Xen static heap.
> > XEN_STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
> > if specified, indicates the host physical address regions
> > [baseaddr, baseaddr + size) to be reserved as Xen static heap.
> > 
> > For instance, XEN_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>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

[ImageBuilder][PATCH v2 2/2] uboot-script-gen: add support for static shared memory

[jiamei.xie]

Introduce support for creating shared-mem node for dom0less domUs in
the device tree. Add the following option:
- DOMU_SHARED_MEM[number]="SHM-ID HPA GPA size"
  if specified, indicate the unique identifier of the shared memory
  region is SHM-ID, the host physical address HPA will get mapped at
  guest address GPA in domU and the memory of size will be reserved to
  be shared memory.

The static shared memory is used between two dom0less domUs.

Below is an example:
NUM_DOMUS=2
DOMU_SHARED_MEM[0]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
DOMU_SHARED_MEM[1]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"

This static shared memory region is identified as "my-shared-mem-0",
host physical address starting at 0x50000000 of 256MB will be reserved
to be shared between two domUs. It will get mapped at 0x6000000 in both
guest physical address space. Both DomUs are the borrower domain, the
owner domain is the default owner domain DOMID_IO.

Signed-off-by: jiamei.xie <jiamei.xie@arm.com>
---
Changes from v1:
 - Rather than two separate properties and just use one like follows:
   Change
     DOMU_SHARED_MEM[0]="0x50000000 0x6000000 0x10000000"
     DOMU_SHARED_MEM_ID[0]="my-shared-mem-0"
   to
     DOMU_SHARED_MEM[0]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
- Use split_value function instead of opencoding it.
---
 README.md                | 17 +++++++++++++++++
 scripts/uboot-script-gen | 27 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/README.md b/README.md
index 78b83f1..fe5d205 100644
--- a/README.md
+++ b/README.md
@@ -196,6 +196,23 @@ Where:
   if specified, indicates the host physical address regions
   [baseaddr, baseaddr + size) to be reserved to the VM for static allocation.
 
+- DOMU_SHARED_MEM[number]="SHM-ID HPA GPA size"
+  if specified, indicate SHM-ID represents the unique identifier of the shared
+  memory region, the host physical address HPA will get mapped at guest
+  address GPA in domU and the memory of size will be reserved to be shared
+  memory. The shared memory is used between two dom0less domUs.
+
+  Below is an example:
+  NUM_DOMUS=2
+  DOMU_SHARED_MEM[0]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
+  DOMU_SHARED_MEM[1]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
+
+  This static shared memory region is identified as "my-shared-mem-0", host
+  physical address starting at 0x50000000 of 256MB will be reserved to be
+  shared between two domUs. It will get mapped at 0x6000000 in both guest
+  physical address space. Both DomUs are the borrower domain, the owner
+  domain is the default owner domain DOMID_IO.
+
 - DOMU_DIRECT_MAP[number] can be set to 1 or 0.
   If set to 1, the VM is direct mapped. The default is 1.
   This is only applicable when DOMU_STATIC_MEM is specified.
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index cca3e59..46ea7e5 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -204,6 +204,28 @@ function add_device_tree_xen_static_heap()
     dt_set "$path" "xen,static-heap" "hex" "${cells[*]}"
 }
 
+function add_device_tree_static_shared_mem()
+{
+    local path=$1
+    local domid=$2
+    local shared_mem=$3
+    local SHARED_MEM_ID=${shared_mem%% *}
+    local regions="${shared_mem#* }"
+    local cells=()
+    local SHARED_MEM_HOST=${regions%% *}
+
+    dt_mknode "${path}" "domU${domid}-shared-mem@${SHARED_MEM_HOST}"
+
+    for val in ${regions[@]}
+    do
+        cells+=("$(split_value $val)")
+    done
+
+    dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "compatible" "str" "xen,domain-shared-memory-v1"
+    dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "xen,shm-id" "str" "${SHARED_MEM_ID}"
+    dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "xen,shared-mem" "hex" "${cells[*]}"
+}
+
 function add_device_tree_cpupools()
 {
     local cpu
@@ -329,6 +351,11 @@ function xen_device_tree_editing()
             dt_set "/chosen/domU$i" "xen,enhanced" "str" "enabled"
         fi
 
+        if test -n "${DOMU_SHARED_MEM[i]}"
+        then
+            add_device_tree_static_shared_mem "/chosen/domU${i}" "${i}" "${DOMU_SHARED_MEM[i]}"
+        fi
+
         if test "${DOMU_COLORS[$i]}"
         then
             local startcolor=$(echo "${DOMU_COLORS[$i]}"  | cut -d "-" -f 1)
-- 
2.25.1

Re: [ImageBuilder][PATCH v2 2/2] uboot-script-gen: add support for static shared memory

[Stefano Stabellini]

On Thu, 16 Mar 2023, jiamei.xie wrote:
> Introduce support for creating shared-mem node for dom0less domUs in
> the device tree. Add the following option:
> - DOMU_SHARED_MEM[number]="SHM-ID HPA GPA size"
>   if specified, indicate the unique identifier of the shared memory
>   region is SHM-ID, the host physical address HPA will get mapped at
>   guest address GPA in domU and the memory of size will be reserved to
>   be shared memory.
> 
> The static shared memory is used between two dom0less domUs.
> 
> Below is an example:
> NUM_DOMUS=2
> DOMU_SHARED_MEM[0]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
> DOMU_SHARED_MEM[1]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
> 
> This static shared memory region is identified as "my-shared-mem-0",
> host physical address starting at 0x50000000 of 256MB will be reserved
> to be shared between two domUs. It will get mapped at 0x6000000 in both
> guest physical address space. Both DomUs are the borrower domain, the
> owner domain is the default owner domain DOMID_IO.
> 
> Signed-off-by: jiamei.xie <jiamei.xie@arm.com>

The patch is good just a couple of minor comments


> ---
> Changes from v1:
>  - Rather than two separate properties and just use one like follows:
>    Change
>      DOMU_SHARED_MEM[0]="0x50000000 0x6000000 0x10000000"
>      DOMU_SHARED_MEM_ID[0]="my-shared-mem-0"
>    to
>      DOMU_SHARED_MEM[0]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
> - Use split_value function instead of opencoding it.
> ---
>  README.md                | 17 +++++++++++++++++
>  scripts/uboot-script-gen | 27 +++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/README.md b/README.md
> index 78b83f1..fe5d205 100644
> --- a/README.md
> +++ b/README.md
> @@ -196,6 +196,23 @@ Where:
>    if specified, indicates the host physical address regions
>    [baseaddr, baseaddr + size) to be reserved to the VM for static allocation.
>  
> +- DOMU_SHARED_MEM[number]="SHM-ID HPA GPA size"
> +  if specified, indicate SHM-ID represents the unique identifier of the shared
> +  memory region, the host physical address HPA will get mapped at guest
> +  address GPA in domU and the memory of size will be reserved to be shared
> +  memory. The shared memory is used between two dom0less domUs.
> +
> +  Below is an example:
> +  NUM_DOMUS=2
> +  DOMU_SHARED_MEM[0]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
> +  DOMU_SHARED_MEM[1]="my-shared-mem-0 0x50000000 0x6000000 0x10000000"
> +
> +  This static shared memory region is identified as "my-shared-mem-0", host
> +  physical address starting at 0x50000000 of 256MB will be reserved to be
> +  shared between two domUs. It will get mapped at 0x6000000 in both guest
> +  physical address space. Both DomUs are the borrower domain, the owner
> +  domain is the default owner domain DOMID_IO.
> +
>  - DOMU_DIRECT_MAP[number] can be set to 1 or 0.
>    If set to 1, the VM is direct mapped. The default is 1.
>    This is only applicable when DOMU_STATIC_MEM is specified.
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index cca3e59..46ea7e5 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -204,6 +204,28 @@ function add_device_tree_xen_static_heap()
>      dt_set "$path" "xen,static-heap" "hex" "${cells[*]}"
>  }
>  
> +function add_device_tree_static_shared_mem()
> +{
> +    local path=$1
> +    local domid=$2

I don't think we need a "domid" parameter, do we? Given that the node
name has the address in it, it cannot conflict anyway with other similar
nodes. So, the following should work?

dt_set "${path}/shared-mem@${shared_mem_host}" "compatible" "str" "xen,domain-shared-memory-v1"


> +    local shared_mem=$3
> +    local SHARED_MEM_ID=${shared_mem%% *}
> +    local regions="${shared_mem#* }"
> +    local cells=()
> +    local SHARED_MEM_HOST=${regions%% *}

please use lower capital letters for local variables, so shared_mem_host
instead of SHARED_MEM_HOST


> +    dt_mknode "${path}" "domU${domid}-shared-mem@${SHARED_MEM_HOST}"
> +
> +    for val in ${regions[@]}
> +    do
> +        cells+=("$(split_value $val)")
> +    done
> +
> +    dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "compatible" "str" "xen,domain-shared-memory-v1"
> +    dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "xen,shm-id" "str" "${SHARED_MEM_ID}"
> +    dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "xen,shared-mem" "hex" "${cells[*]}"
> +}
> +
>  function add_device_tree_cpupools()
>  {
>      local cpu
> @@ -329,6 +351,11 @@ function xen_device_tree_editing()
>              dt_set "/chosen/domU$i" "xen,enhanced" "str" "enabled"
>          fi
>  
> +        if test -n "${DOMU_SHARED_MEM[i]}"
> +        then
> +            add_device_tree_static_shared_mem "/chosen/domU${i}" "${i}" "${DOMU_SHARED_MEM[i]}"
> +        fi

The reason why I suggested to remove the "domid" parameter above is
that ${i} here is not actually the domid, it is just the numeric order
of the domain in the ImageBuilder config file. It happens often in my
tests that Xen assigns different domids to the domains compared to the
order they appear in the ImageBuilder config file and in device tree.


>          if test "${DOMU_COLORS[$i]}"
>          then
>              local startcolor=$(echo "${DOMU_COLORS[$i]}"  | cut -d "-" -f 1)
> -- 
> 2.25.1
>