[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] ImageBuilder: Add support for 64-bit addresses



+xen-devel

On Thu, 6 Oct 2022, Michal Orzel wrote:
> At the moment, ImageBuilder assumes that all addresses/sizes are
> 32-bit max. It sets #{address,size}-cells to 0x2 and puts 0x0 as the
> value for the first cell. Because of that, we cannot specify MEMORY_START
> and MEMORY_END to be above 32-bits (e.g. to place the images in the
> upper memory bank).
> 
> Add support to properly handle 64-bit addresses:
>  - add function split_into_halves to split the value passed as a first
>    argument into upper and lower halves. These are then set as values for
>    variables passed respetively as the second and third argument,
>  - whenever there is a variable storing the full 64-bit number with
>    "addr" or "size" in name, introduce two additional variables with
>    "addr1,addr2"/"size1,size2" in name to store the halves. These are
>    then used to properly set cells.
> 
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
>
> ---
>  scripts/uboot-script-gen | 60 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 53 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index 16269f02f1e7..4c6525a910f3 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -25,6 +25,14 @@ function dt_mknode()
>      fi
>  }
>  
> +# Usage:
> +# split_into_halves <value> <variable_to_store_upper> 
> <variable_to_store_lower>
> +function split_into_halves()
> +{
> +    eval "$2=$(printf "0x%X\n" $(($1 >> 32)))"
> +    eval "$3=$(printf "0x%X\n" $(($1 & 0xFFFFFFFF)))"
> +}

I know it is the same thing, but I would prefer the following version
because it makes it easier to read:

# Usage:
# split_into_halves <value> <variable_to_store_upper> <variable_to_store_lower>
function split_into_halves()
{
    local value=$1
    local upper=$2
    local lower=$3

    eval "$upper=$(printf "0x%X\n" $(($value >> 32)))"
    eval "$lower=$(printf "0x%X\n" $(($value & 0xFFFFFFFF)))"
}


> +
>  # data_type is either
>  #   int
>  #   hex
> @@ -41,10 +49,14 @@ function dt_set()
>  
>      if test $data_type = "var"
>      then
> -        eval data_addr_var="$data"_addr
> -        eval data_addr=\$"$data_addr_var"
> -        eval data_size_var="$data"_size
> -        eval data_size=\$"$data_size_var"
> +        eval data_addr1_var="$data"_addr1
> +        eval data_addr2_var="$data"_addr2
> +        eval data_addr1=\$"$data_addr1_var"
> +        eval data_addr2=\$"$data_addr2_var"
> +        eval data_size1_var="$data"_size1
> +        eval data_size2_var="$data"_size2
> +        eval data_size1=\$"$data_size1_var"
> +        eval data_size2=\$"$data_size2_var"

To avoid making the code more complex, is it possible to stick with just
a single data_addr variable in u-boot and calculate the upper and lower
32-bit using u-boot commands?

That way, we can keep the current scheme of address and size variable
storage, which I think is already complex enough, and just add the
calculation when needed.

So we could do for the dynamic case:

setexpr data_addr1 \${"$data_addr_var"} / 0x100000000
setexpr data_addr2 \${"$data_addr_var"} \& 0xffffffff

And it could be moved into split_into_halves, so that split_into_halves
can be called for both the dynamic case and the non-dynamic case maybe.

This should reduce the amount of changes significantly.


>      fi
>  
>      if test "$UBOOT_SOURCE" && test ! "$FIT"
> @@ -54,9 +66,9 @@ function dt_set()
>          then
>              if test $dynamic_loading_opt
>              then
> -                echo "fdt set $path $var <0x0 0x\${"$data_addr_var"} 0x0 
> 0x\${"$data_size_var"}>" >> $UBOOT_SOURCE
> +                echo "fdt set $path $var <0x\${"$data_addr1_var"} 
> 0x\${"$data_addr2_var"} 0x\${"$data_size1_var"} 0x\${"$data_size2_var"}>" >> 
> $UBOOT_SOURCE
>              else
> -                echo "fdt set $path $var <0x0 $data_addr 0x0 $data_size>" >> 
> $UBOOT_SOURCE
> +                echo "fdt set $path $var <$data_addr1 $data_addr2 
> $data_size1 $data_size2>" >> $UBOOT_SOURCE
>              fi
>          elif test $data_type = "hex" || test $data_type = "int"
>          then
> @@ -84,7 +96,7 @@ function dt_set()
>      then
>          if test $data_type = "var"
>          then
> -            fdtput $FDTEDIT -p -t x $path $var 0x0 "$data_addr" 0x0 
> "$data_size"
> +            fdtput $FDTEDIT -p -t x $path $var "$data_addr1" "$data_addr2" 
> "$data_size1" "$data_size2"
>          elif test $data_type = "hex"
>          then
>              fdtput $FDTEDIT -p -t x $path $var $data
> @@ -396,18 +408,38 @@ function add_size()
>      local filename=$1
>      local fit_scr_name=$2
>      local binary_name_addr="${fit_scr_name}_addr"
> +    local binary_name_addr1="${fit_scr_name}_addr1"
> +    local binary_name_addr2="${fit_scr_name}_addr2"
>      local binary_name_size="${fit_scr_name}_size"
> +    local binary_name_size1="${fit_scr_name}_size1"
> +    local binary_name_size2="${fit_scr_name}_size2"
> +
> +    split_into_halves $memaddr memaddr1 memaddr2
>      eval "$fit_scr_name"_addr=$memaddr
> +    eval "$fit_scr_name"_addr1=$memaddr1
> +    eval "$fit_scr_name"_addr2=$memaddr2
>  
>      local size=`stat -L --printf="%s" $filename`
>      filesize=$size
> +    split_into_halves $size size1 size2
>      eval "$fit_scr_name"_size=`printf "0x%X\n" $size`
> +    eval "$fit_scr_name"_size1=$size1
> +    eval "$fit_scr_name"_size2=$size2
>      eval binary_name_size_value=\$"$binary_name_size"
> +    split_into_halves $binary_name_size_value binary_name_size_value1 
> binary_name_size_value2
>      
>      if test $dynamic_loading_opt
>      then
> +        echo "setexpr memaddr1 \${memaddr} \/ 0x100000000" >> $UBOOT_SOURCE
> +        echo "setexpr memaddr2 \${memaddr} \& 0xFFFFFFFF" >> $UBOOT_SOURCE
> +        echo "setexpr filesize1 \${filesize} \/ 0x100000000" >> $UBOOT_SOURCE
> +        echo "setexpr filesize2 \${filesize} \& 0xFFFFFFFF" >> $UBOOT_SOURCE
>          echo "setenv $binary_name_addr \${memaddr}" >> $UBOOT_SOURCE
> +        echo "setenv $binary_name_addr1 \${memaddr1}" >> $UBOOT_SOURCE
> +        echo "setenv $binary_name_addr2 \${memaddr2}" >> $UBOOT_SOURCE
>          echo "setenv $binary_name_size \${filesize}" >> $UBOOT_SOURCE
> +        echo "setenv $binary_name_size1 \${filesize1}" >> $UBOOT_SOURCE
> +        echo "setenv $binary_name_size2 \${filesize2}" >> $UBOOT_SOURCE
>          # Compute load addr for next binary dynamically
>          echo "setexpr memaddr \${memaddr} \+ \${filesize}" >> $UBOOT_SOURCE
>          echo "setexpr memaddr \${memaddr} \+ $padding_mask" >> $UBOOT_SOURCE
> @@ -415,7 +447,11 @@ function add_size()
>      else
>          # Store load addr and size as literals
>          echo "setenv $binary_name_addr $memaddr" >> $UBOOT_SOURCE
> +        echo "setenv $binary_name_addr1 $memaddr1" >> $UBOOT_SOURCE
> +        echo "setenv $binary_name_addr2 $memaddr2" >> $UBOOT_SOURCE
>          echo "setenv $binary_name_size $binary_name_size_value" >> 
> $UBOOT_SOURCE
> +        echo "setenv $binary_name_size1 $binary_name_size_value1" >> 
> $UBOOT_SOURCE
> +        echo "setenv $binary_name_size2 $binary_name_size_value2" >> 
> $UBOOT_SOURCE
>      fi
>  
>      memaddr=$(( $memaddr + $size + $offset - 1))
> @@ -666,16 +702,20 @@ xen_file_loading()
>      then
>          check_compressed_file_type $DOM0_KERNEL "executable"
>          dom0_kernel_addr=$memaddr
> +        split_into_halves $dom0_kernel_addr dom0_kernel_addr1 
> dom0_kernel_addr2
>          load_file $DOM0_KERNEL "dom0_linux"
>          dom0_kernel_size=$filesize
> +        split_into_halves $dom0_kernel_size dom0_kernel_size1 
> dom0_kernel_size2
>      fi
>      if test "$DOM0_RAMDISK"
>      then
>          check_compressed_file_type $DOM0_RAMDISK "cpio archive"
>          ramdisk_addr=$memaddr
> +        split_into_halves $ramdisk_addr ramdisk_addr1 ramdisk_addr2
>          ramdisk_path=$DOM0_RAMDISK
>          load_file "$DOM0_RAMDISK" "dom0_ramdisk"
>          ramdisk_size=$filesize
> +        split_into_halves $ramdisk_size ramdisk_size1 ramdisk_size2
>      else
>          ramdisk_addr="-"
>      fi
> @@ -701,21 +741,27 @@ xen_file_loading()
>  
>          check_compressed_file_type ${DOMU_KERNEL[$i]} "executable"
>          domU_kernel_addr[$i]=$memaddr
> +        split_into_halves ${domU_kernel_addr[$i]} domU_kernel_addr1[$i] 
> domU_kernel_addr2[$i]
>          load_file ${DOMU_KERNEL[$i]} "domU${i}_kernel"
>          domU_kernel_size[$i]=$filesize
> +        split_into_halves ${domU_kernel_size[$i]} domU_kernel_size1[$i] 
> domU_kernel_size2[$i]
>          if test "${DOMU_RAMDISK[$i]}"
>          then
>              check_compressed_file_type ${DOMU_RAMDISK[$i]} "cpio archive"
>              domU_ramdisk_addr[$i]=$memaddr
> +            split_into_halves ${domU_ramdisk_addr[$i]} 
> domU_ramdisk_addr1[$i] domU_ramdisk_addr2[$i]
>              load_file ${DOMU_RAMDISK[$i]} "domU${i}_ramdisk"
>              domU_ramdisk_size[$i]=$filesize
> +            split_into_halves ${domU_ramdisk_size[$i]} 
> domU_ramdisk_size1[$i] domU_ramdisk_size2[$i]
>          fi
>          if test "${DOMU_PASSTHROUGH_DTB[$i]}"
>          then
>              check_compressed_file_type ${DOMU_PASSTHROUGH_DTB[$i]} "Device 
> Tree Blob"
>              domU_passthrough_dtb_addr[$i]=$memaddr
> +            split_into_halves ${domU_passthrough_dtb_addr[$i]} 
> domU_passthrough_dtb_addr1[$i] domU_passthrough_dtb_addr2[$i]
>              load_file ${DOMU_PASSTHROUGH_DTB[$i]} "domU${i}_fdt"
>              domU_passthrough_dtb_size[$i]=$filesize
> +            split_into_halves ${domU_passthrough_dtb_size[$i]} 
> domU_passthrough_dtb_size1[$i] domU_passthrough_dtb_size2[$i]
>          fi
>          i=$(( $i + 1 ))
>      done
> -- 
> 2.25.1
> 
> 



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.