Re: [PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP

[Henry Wang <Henry.Wang@xxxxxxx>]

Hi Michal,

> On Dec 8, 2023, at 16:39, Michal Orzel <michal.orzel@xxxxxxx> wrote:
> 
> Hi Henry,
> 
> On 08/12/2023 06:46, Henry Wang wrote:
>> 
>> Unlike the emulators that currently being used in the CI pipelines,
>> the FVP must start at EL3. Therefore we need the firmware, i.e. the
>> TrustedFirmware-A (TF-A), for corresponding functionality.
>> 
>> There is a dedicated board (vexpress_fvp) in U-Boot (serve as the
>> BL33 of the TF-A) for the FVP platform, so the U-Boot should also be
>> compiled for the FVP platform instead of reusing the U-Boot for the
>> existing emulators used in the CI pipelines.
>> 
>> To avoid compiling TF-A and U-Boot everytime in the job, adding a
>> Dockerfile to the test artifacts to build TF-A v2.9.0 and U-Boot
>> v2023.10 for FVP. The binaries for the TF-A and U-Boot, as well as
>> the device tree for the FVP platform, will be saved (and exported by
>> the CI job introduced by following commits). Note that, a patch for
>> the TF-A will be applied before building to enable the virtio-net
>> and the virtio-rng device on the FVP. The virtio-net device will
>> provide the networking service for FVP, and the virtio-rng device
>> will improve the speed of the FVP.
>> 
>> Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>
>> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> ---
>> v2:
>> - Add Stefano's Reviewed-by tag.
>> ---
>> +# Build U-Boot and TF-A
>> +RUN curl -fsSLO 
>> https://ftp.denx.de/pub/u-boot/u-boot-"$UBOOT_VERSION".tar.bz2 && \
>> +    tar xvf u-boot-"$UBOOT_VERSION".tar.bz2 && \
>> +    cd u-boot-"$UBOOT_VERSION" && \
>> +    make -j$(nproc) V=1 vexpress_fvp_defconfig && \
>> +    make -j$(nproc) V=1 all && \
> Do we need 'all'? Can't we just build target 'u-boot' for u-boot.bin?

I think your suggestion makes sense, and I can have a try, if changing all to 
u-boot works,
I will use that in v3.

>> +    cd .. && \
>> +    git clone --branch "$TFA_VERSION" --depth 1 
>> https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git && \
>> +    cd trusted-firmware-a && \
>> +    curl -fsSLO 
>> https://git.yoctoproject.org/meta-arm/plain/meta-arm-bsp/recipes-bsp/trusted-firmware-a/files/fvp-base/0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch
>>  \
>> +         --output 
>> 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
>> +    git config --global user.email "you@xxxxxxxxxxx" && \
>> +    git config --global user.name "Your Name" && \
> If this is needed for git am, you could get away using 'patch -p1'

Hmmm right, then probably we can even not install git and use the tarball 
instead of
git clone.


>> +    git am 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch 
>> && \
>> +    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 
>> FVP_DT_PREFIX=fvp-base-gicv3-psci-1t all && \
>> +    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 
>> FVP_DT_PREFIX=fvp-base-gicv3-psci-1t fip 
>> BL33=../u-boot-"$UBOOT_VERSION"/u-boot.bin && \
>> +    cp build/fvp/debug/bl1.bin / && \
>> +    cp build/fvp/debug/fip.bin / && \
>> +    cp build/fvp/debug/fdts/fvp-base-gicv3-psci-1t.dtb / && \
>> +    cd /build && \
>> +    rm -rf u-boot-"$UBOOT_VERSION" trusted-firmware-a
> You forgot to remove u-boot tar file

oops, nice catch, thanks. Will also remove that in v3.

> Other than that:
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

Thanks!

Stefano: Can I keep your Reviewed-by tag after addressing Michal’s comments 
above?

Kind regards,
Henry

> 
> ~Michal