Re: [ImageBuilder] [PATCH v2] uboot-script-gen: Add DOMU_STATIC_MEM

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 15 Jun 2022, Xenia Ragiadakou wrote:
> Add a new config parameter to configure a dom0less VM with static allocation.
> DOMU_STATIC_MEM[number]="baseaddr1 size1 ... baseaddrN sizeN"
> The parameter specifies the host physical address regions to be statically
> allocated to the VM. Each region is defined by its start address and size.
> 
> For instance,
> DOMU_STATIC_MEM[0]="0x30000000 0x10000000 0x50000000 0x20000000"
> indicates that the host memory regions [0x30000000, 0x40000000) and
> [0x50000000, 0x70000000) are statically allocated to the first dom0less VM.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>

Hi Xenia, thanks for the patch!

It looks fine as is, only two minor code style issues (tabs instead of
spaces for indentation.)

I think this would work. However, when static-mem is specified also the
total memory for the guest needs to match. So for instance:

  #xen,static-mem-address-cells = <0x1>;
  #xen,static-mem-size-cells = <0x1>;
  xen,static-mem = <0x30000000 0x20000000>;

In this case memory has to be:

  memory = <0x0 0x80000>;

memory is in kilobytes, so 0x20000000/1024=0x80000.

In ImageBuilder "memory" is normally set by the DOMU_MEM variable,
although that is in megabytes.

I think it would make sense to automatically calculate "memory" DOMU_MEM
based on the sizes passed via DOMU_STATIC_MEM when DOMU_STATIC_MEM is
specified: summing all the sizes together and dividing by 1024.

That could be done either with something like

    if test "${DOMU_STATIC_MEM[$i]}"
    then
        local memory=[calculate memory]
        dt_set "/chosen/domU$i" "memory" "int" "0 $memory"
        add_device_tree_static_mem "/chosen/domU$i" "${DOMU_STATIC_MEM[$i]}"

Or it could be done by changing DOMU_MEM to be in kilobytes and simply
setting DOMU_MEM based on the DOMU_STATIC_MEM values when
DOMU_STATIC_MEM is specified.

Would you be OK to add that to this patch? If not, that's OK. This patch
is also good to have as is.



> ---
> 
> Notes:
>     v2: in add_device_tree_static_mem(), replace i with val because variable i
>         is already in use as an index
> 
>  README.md                |  4 ++++
>  scripts/uboot-script-gen | 20 ++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/README.md b/README.md
> index 8ce13f0..876e46d 100644
> --- a/README.md
> +++ b/README.md
> @@ -154,6 +154,10 @@ Where:
>    automatically at boot as dom0-less guest. It can still be created
>    later from Dom0.
>  
> +- DOMU_STATIC_MEM[number]="baseaddr1 size1 ... baseaddrN sizeN"
> +  if specified, indicates the host physical address regions
> +  [baseaddr, baseaddr + size) to be reserved to the VM for static allocation.
> +
>  - LINUX is optional but specifies the Linux kernel for when Xen is NOT
>    used.  To enable this set any LINUX\_\* variables and do NOT set the
>    XEN variable.
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index 0adf523..3a5f720 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -108,6 +108,22 @@ function add_device_tree_passthrough()
>      dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" 
> $size)"
>  }
>  
> +function add_device_tree_static_mem()
> +{
> +    local path=$1
> +    local regions=$2
> +
> +    dt_set "$path" "#xen,static-mem-address-cells" "hex" "0x2"
> +    dt_set "$path" "#xen,static-mem-size-cells" "hex" "0x2"
> +
> +    for val in ${regions[@]}
> +    do
> +     cells+=("$(printf "0x%x 0x%x" $(($val >> 32)) $(($val & ((1 << 32) - 
> 1))))")
> +    done
> +
> +    dt_set "$path" "xen,static-mem" "hex" "${cells[*]}"
> +}
> +
>  function xen_device_tree_editing()
>  {
>      dt_set "/chosen" "#address-cells" "hex" "0x2"
> @@ -143,6 +159,10 @@ function xen_device_tree_editing()
>          dt_set "/chosen/domU$i" "#size-cells" "hex" "0x2"
>          dt_set "/chosen/domU$i" "memory" "int" "0 ${DOMU_MEM[$i]}"
>          dt_set "/chosen/domU$i" "cpus" "int" "${DOMU_VCPUS[$i]}"
> +     if test "${DOMU_STATIC_MEM[$i]}"
> +        then
> +         add_device_tree_static_mem "/chosen/domU$i" "${DOMU_STATIC_MEM[$i]}"
> +        fi
>          dt_set "/chosen/domU$i" "vpl011" "hex" "0x1"
>          add_device_tree_kernel "/chosen/domU$i" ${domU_kernel_addr[$i]} 
> ${domU_kernel_size[$i]} "${DOMU_CMD[$i]}"
>          if test "${domU_ramdisk_addr[$i]}"
> -- 
> 2.34.1
>