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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 8 Dec 2023, Henry Wang wrote:
> 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?

Yes

Cheers,

Stefano