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

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

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@xxxxxxxx>
> >
> > 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@xxxxxxxx>
> > ---
> >   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