1. Patchew
  2. Xen
  3. View series

[ImageBuilder] uboot-script-gen: add xen xsm policy loading support

Grygorii Strashko posted 1 patch 6 months, 2 weeks ago
Failed in applying to current master (apply log)
There is a newer version of this series
README.md                |  2 ++
scripts/uboot-script-gen | 33 +++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
[ImageBuilder] uboot-script-gen: add xen xsm policy loading support
Posted by Grygorii Strashko 6 months, 2 weeks ago 
From: Grygorii Strashko <grygorii_strashko@epam.com>

This patch adds Xen XSM policy loading support.

The configuration file XEN_POLICY specifies Xen hypervisor
XSM policy binary to load.

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
 README.md                |  2 ++
 scripts/uboot-script-gen | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/README.md b/README.md
index 137abef153ce..9106d2a07302 100644
--- a/README.md
+++ b/README.md
@@ -91,6 +91,8 @@ Where:
 - XEN specifies the Xen hypervisor binary to load. Note that it has to
   be a regular Xen binary, not a u-boot binary.
 
+- XEN_POLICY specifies the Xen hypervisor XSM policy binary to load.
+
 - XEN_COLORS specifies the colors (cache coloring) to be used for Xen
   and is in the format startcolor-endcolor
 
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index c4d26caf5e0e..343eba20e4d9 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -315,6 +315,15 @@ function xen_device_tree_editing()
     dt_set "/chosen" "#size-cells" "hex" "0x2"
     dt_set "/chosen" "xen,xen-bootargs" "str" "$XEN_CMD"
 
+    if test "$XEN_POLICY" && test $xen_policy_addr != "-"
+    then
+        local node_name="xen-policy@${xen_policy_addr#0x}"
+
+        dt_mknode "/chosen" "$node_name"
+        dt_set "/chosen/$node_name" "compatible" "str_a" "xen,xsm-policy xen,multiboot-module multiboot,module"
+        dt_set "/chosen/$node_name" "reg" "hex" "$(split_addr_size $xen_policy_addr $xen_policy_size)"
+    fi
+
     if test "$DOM0_KERNEL"
     then
         local node_name="dom0@${dom0_kernel_addr#0x}"
@@ -900,6 +909,14 @@ xen_file_loading()
     kernel_addr=$memaddr
     kernel_path=$XEN
     load_file "$XEN" "host_kernel"
+
+    xen_policy_addr=="-"
+    if test "$XEN_POLICY"
+    then
+        xen_policy_addr=$memaddr
+        load_file "$XEN_POLICY" "xen_policy"
+        xen_policy_size=$filesize
+    fi
 }
 
 linux_file_loading()
@@ -939,6 +956,22 @@ bitstream_load_and_config()
 
 create_its_file_xen()
 {
+    if test "$XEN_POLICY" && test $xen_policy_addr != "-"
+    then
+        cat >> "$its_file" <<- EOF
+        xen_policy {
+            description = "Xen XSM policy binary";
+            data = /incbin/("$XEN_POLICY");
+            type = "kernel";
+            arch = "arm64";
+            os = "linux";
+            compression = "none";
+            load = <$xen_policy_addr>;
+            $fit_algo
+        };
+	EOF
+    fi
+
     if test "$DOM0_KERNEL"
     then
         if test "$ramdisk_addr" != "-"
-- 
2.34.1
Re: [ImageBuilder] uboot-script-gen: add xen xsm policy loading support
Posted by Alejandro Vallejo 6 months, 2 weeks ago 
As a general question, why using "test" so much, rather than

   if [ -n "$FOO" -a "FOO" == ABC ]

Using test seems far harder to read than it needs to be, and single
brackets are perfectly valid POSIX shell.

On Mon Apr 14, 2025 at 9:14 AM BST, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@epam.com>
>
> This patch adds Xen XSM policy loading support.
>
> The configuration file XEN_POLICY specifies Xen hypervisor
> XSM policy binary to load.
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
>  README.md                |  2 ++
>  scripts/uboot-script-gen | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/README.md b/README.md
> index 137abef153ce..9106d2a07302 100644
> --- a/README.md
> +++ b/README.md
> @@ -91,6 +91,8 @@ Where:
>  - XEN specifies the Xen hypervisor binary to load. Note that it has to
>    be a regular Xen binary, not a u-boot binary.
>  
> +- XEN_POLICY specifies the Xen hypervisor XSM policy binary to load.
> +
>  - XEN_COLORS specifies the colors (cache coloring) to be used for Xen
>    and is in the format startcolor-endcolor
>  
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index c4d26caf5e0e..343eba20e4d9 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -315,6 +315,15 @@ function xen_device_tree_editing()
>      dt_set "/chosen" "#size-cells" "hex" "0x2"
>      dt_set "/chosen" "xen,xen-bootargs" "str" "$XEN_CMD"
>  
> +    if test "$XEN_POLICY" && test $xen_policy_addr != "-"

If XEN_POLICY is a binary, shouldn't it be "test -f" ? Same later on.

Also, missing quotes around $xen_policy_addr

> +    then
> +        local node_name="xen-policy@${xen_policy_addr#0x}"
> +
> +        dt_mknode "/chosen" "$node_name"
> +        dt_set "/chosen/$node_name" "compatible" "str_a" "xen,xsm-policy xen,multiboot-module multiboot,module"
> +        dt_set "/chosen/$node_name" "reg" "hex" "$(split_addr_size $xen_policy_addr $xen_policy_size)"
> +    fi
> +
>      if test "$DOM0_KERNEL"

test -f?
g
>      then
>          local node_name="dom0@${dom0_kernel_addr#0x}"
> @@ -900,6 +909,14 @@ xen_file_loading()
>      kernel_addr=$memaddr
>      kernel_path=$XEN
>      load_file "$XEN" "host_kernel"
> +
> +    xen_policy_addr=="-"

Do you mean = ?

> +    if test "$XEN_POLICY"
> +    then
> +        xen_policy_addr=$memaddr
> +        load_file "$XEN_POLICY" "xen_policy"
> +        xen_policy_size=$filesize
> +    fi
>  }
>  
>  linux_file_loading()
> @@ -939,6 +956,22 @@ bitstream_load_and_config()
>  
>  create_its_file_xen()
>  {
> +    if test "$XEN_POLICY" && test $xen_policy_addr != "-"
> +    then
> +        cat >> "$its_file" <<- EOF
> +        xen_policy {
> +            description = "Xen XSM policy binary";
> +            data = /incbin/("$XEN_POLICY");
> +            type = "kernel";
> +            arch = "arm64";
> +            os = "linux";
> +            compression = "none";
> +            load = <$xen_policy_addr>;
> +            $fit_algo
> +        };
> +	EOF
> +    fi
> +
>      if test "$DOM0_KERNEL"
>      then
>          if test "$ramdisk_addr" != "-"

Cheers,
Alejandro
Re: [ImageBuilder] uboot-script-gen: add xen xsm policy loading support
Posted by Grygorii Strashko 6 months, 2 weeks ago 
Hi Alejandro,

On 14.04.25 14:57, Alejandro Vallejo wrote:
> As a general question, why using "test" so much, rather than
> 
>     if [ -n "$FOO" -a "FOO" == ABC ]
> 
> Using test seems far harder to read than it needs to be, and single
> brackets are perfectly valid POSIX shell.
> 
> On Mon Apr 14, 2025 at 9:14 AM BST, Grygorii Strashko wrote:
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> This patch adds Xen XSM policy loading support.
>>
>> The configuration file XEN_POLICY specifies Xen hypervisor
>> XSM policy binary to load.
>>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> ---
>>   README.md                |  2 ++
>>   scripts/uboot-script-gen | 33 +++++++++++++++++++++++++++++++++
>>   2 files changed, 35 insertions(+)
>>
>> diff --git a/README.md b/README.md
>> index 137abef153ce..9106d2a07302 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -91,6 +91,8 @@ Where:
>>   - XEN specifies the Xen hypervisor binary to load. Note that it has to
>>     be a regular Xen binary, not a u-boot binary.
>>   
>> +- XEN_POLICY specifies the Xen hypervisor XSM policy binary to load.
>> +
>>   - XEN_COLORS specifies the colors (cache coloring) to be used for Xen
>>     and is in the format startcolor-endcolor
>>   
>> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
>> index c4d26caf5e0e..343eba20e4d9 100755
>> --- a/scripts/uboot-script-gen
>> +++ b/scripts/uboot-script-gen
>> @@ -315,6 +315,15 @@ function xen_device_tree_editing()
>>       dt_set "/chosen" "#size-cells" "hex" "0x2"
>>       dt_set "/chosen" "xen,xen-bootargs" "str" "$XEN_CMD"
>>   
>> +    if test "$XEN_POLICY" && test $xen_policy_addr != "-"
> 
> If XEN_POLICY is a binary, shouldn't it be "test -f" ? Same later on.

You're right. I'll update it as below...

> 
> Also, missing quotes around $xen_policy_addr
> 
>> +    then
>> +        local node_name="xen-policy@${xen_policy_addr#0x}"
>> +
>> +        dt_mknode "/chosen" "$node_name"
>> +        dt_set "/chosen/$node_name" "compatible" "str_a" "xen,xsm-policy xen,multiboot-module multiboot,module"
>> +        dt_set "/chosen/$node_name" "reg" "hex" "$(split_addr_size $xen_policy_addr $xen_policy_size)"
>> +    fi
>> +
>>       if test "$DOM0_KERNEL"
> 
> test -f?
> g
>>       then
>>           local node_name="dom0@${dom0_kernel_addr#0x}"
>> @@ -900,6 +909,14 @@ xen_file_loading()
>>       kernel_addr=$memaddr
>>       kernel_path=$XEN
>>       load_file "$XEN" "host_kernel"
>> +
>> +    xen_policy_addr=="-"
> 
> Do you mean = ?
> 
>> +    if test "$XEN_POLICY"
>> +    then

I'll add here:

             check_file_type "${XEN_POLICY}" "SE Linux policy"

and fix other comments.

>> +        xen_policy_addr=$memaddr
>> +        load_file "$XEN_POLICY" "xen_policy"
>> +        xen_policy_size=$filesize
>> +    fi
>>   }
>>   
>>   linux_file_loading()
>> @@ -939,6 +956,22 @@ bitstream_load_and_config()
>>   
>>   create_its_file_xen()
>>   {
>> +    if test "$XEN_POLICY" && test $xen_policy_addr != "-"
>> +    then
>> +        cat >> "$its_file" <<- EOF
>> +        xen_policy {
>> +            description = "Xen XSM policy binary";
>> +            data = /incbin/("$XEN_POLICY");
>> +            type = "kernel";
>> +            arch = "arm64";
>> +            os = "linux";
>> +            compression = "none";
>> +            load = <$xen_policy_addr>;
>> +            $fit_algo
>> +        };
>> +	EOF
>> +    fi
>> +
>>       if test "$DOM0_KERNEL"
>>       then
>>           if test "$ramdisk_addr" != "-"

Thanks for your review.

-- 
Best regards,
-grygorii
Re: [ImageBuilder] uboot-script-gen: add xen xsm policy loading support
Posted by Alejandro Vallejo 6 months, 2 weeks ago 
On Mon Apr 14, 2025 at 12:57 PM BST, Alejandro Vallejo wrote:
> As a general question, why using "test" so much, rather than
>
>    if [ -n "$FOO" -a "FOO" == ABC ]
>
> Using test seems far harder to read than it needs to be, and single
> brackets are perfectly valid POSIX shell.
>
> On Mon Apr 14, 2025 at 9:14 AM BST, Grygorii Strashko wrote:
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> This patch adds Xen XSM policy loading support.
>>
>> The configuration file XEN_POLICY specifies Xen hypervisor
>> XSM policy binary to load.
>>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> ---
>>  README.md                |  2 ++
>>  scripts/uboot-script-gen | 33 +++++++++++++++++++++++++++++++++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/README.md b/README.md
>> index 137abef153ce..9106d2a07302 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -91,6 +91,8 @@ Where:
>>  - XEN specifies the Xen hypervisor binary to load. Note that it has to
>>    be a regular Xen binary, not a u-boot binary.
>>  
>> +- XEN_POLICY specifies the Xen hypervisor XSM policy binary to load.
>> +
>>  - XEN_COLORS specifies the colors (cache coloring) to be used for Xen
>>    and is in the format startcolor-endcolor
>>  
>> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
>> index c4d26caf5e0e..343eba20e4d9 100755
>> --- a/scripts/uboot-script-gen
>> +++ b/scripts/uboot-script-gen
>> @@ -315,6 +315,15 @@ function xen_device_tree_editing()
>>      dt_set "/chosen" "#size-cells" "hex" "0x2"
>>      dt_set "/chosen" "xen,xen-bootargs" "str" "$XEN_CMD"
>>  
>> +    if test "$XEN_POLICY" && test $xen_policy_addr != "-"
>
> If XEN_POLICY is a binary, shouldn't it be "test -f" ? Same later on.

On the absent flag:

Nevermind, I see you're using "-n" implicitly instead. I'd rather it be
less opaque, but it does function. I do think most uses of test "$FOO"
would be better off with an explicit -f or -n. But that's a separate
matter.

Cheers,
Alejandro

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.