Hi Stefano,
On 07/10/2022 00:34, Stefano Stabellini wrote:
>
>
> +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 <[email protected]>
>>
>> ---
>> 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)))"
> }
That is ok for me.
>
>
>> +
>> # 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?
The reason why we need these extra variables is to add them into respective
cells under different nodes. In dt_set we need to put the variable names
for dynamic assignment and variable values for static assignment. We cannot
do this having a single pair data_addr_var,data_addr. These evals corresponds
to variables from xen_file_loading. dt_set and add_size are two different
functions. The former is used to create the nodes and the latter is used to
set values for the environment variables.
Example:
dt_set "/chosen/dom0" "reg" "var" "dom0_linux"
- this will create a reg property for dom0 kernel. We need to insert the upper
and lower halves into this property (so we need separate variables for that)
e.g.
reg <0x${dom0_linux_addr1} 0x${dom0_linux_addr2} 0x${dom0_linux_size1}
0x${dom0_linux_size2}>
load_file $DOM0_KERNEL "dom0_linux" calling add_size
- this will set values for upper and lower halves into u-boot env variables
that corresponds to variables we placed previously in reg property,
e.g.
setenv dom0_linux_addr1 ${memaddr1}
setenv dom0_linux_addr2 ${memaddr2}
setenv dom0_linux_size1 ${filesize1}
setenv dom0_linux_size2 ${filesize2}
FWICS, we cannot achieve this using a single pair.
~Michal
