Re: [Buildroot] [PATCH v1 1/3] add package/versal-firmware

["Frager, Neal via buildroot" <buildroot@buildroot.org>]

Hi Yann,

> Neal, All,

> This patch adds support for downloading versal firmware binaries.
> These are necessary for booting Xilinx versal devices.
> diff --git a/package/versal-firmware/Config.in 
> b/package/versal-firmware/Config.in
> new file mode 100644
> index 0000000000..9ca46b1d16
> --- /dev/null
> +++ b/package/versal-firmware/Config.in
> @@ -0,0 +1,32 @@
> +config BR2_PACKAGE_VERSAL_FIRMWARE
> +     bool "versal-firmware"
> +     depends on BR2_aarch64
> +     help
> +       Versal Firmware
> +
> +       Pre-built binaries of the current bootloader firmware
> +
> +       https://github.com/nealfrager/versal_boot

> This looks like your personal git tree, not the official one. (Yeah, I noticed your email address, but I would still have expected something under https://github.com/Xilinx/).

> The official images are there:
>    https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18842316/Linux+Prebuilt+Images
>    https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/2347204609/2022.1+Release

> If one tries to download any resource from there, they are redirected to a login screen, e.g.:
>    https://login.xilinx.com/app/xilinxinc_f5awsprod_1/exknv8ms950lm0Ldh0x7/sso/saml

> The licensing information is not clearly detailed in the portal, and being low-level firmware files, I'm afraid there might be restrictions on the redistribution og those files...

Yes, I agree that the boot.bin should come from a freely available official AMD/Xilinx source.

I am working on this and am using my own personal github location in the meantime to figure out how this could work with buildroot.

I understand that this patch set cannot be accepted until the boot.bin is available freely from an official source location.

> +if BR2_PACKAGE_VERSAL_FIRMWARE
> +
> +config BR2_PACKAGE_VERSAL_FIRMWARE_LOCATION
> +     string "versal firmware location"

> The prompt should be "Custom git repository"

Could you clarify this a bit?  Are you speaking of the BR2 name or the string text when you say the prompt?

> +     help
> +       Location of a versal firmware boot.bin.
> +
> +       The value should be a git repository.

> So, there is not even a default location?

The official default solution (aside from petalinux) is to clone the embeddedsw github location and build the plm.elf and psmfw.elf from that with a microblaze compiler.
And then use bootgen to generate a boot.bin.

While I can build u-boot and atf easily with buildroot, I do not have a clean solution for building the plm and psmfw images and generating the boot.bin.
This is why I am looking for a short term solution of just downloading a prebuilt boot.bin.

At the moment, distribution of pre-built binaries requires the click-wrap downloading procedure which does not lend itself well for an automated buildroot download.

> +config BR2_PACKAGE_VERSAL_FIRMWARE_BOARD
> +     string "versal board name"
> +     help
> +       Name of versal target board.
> +
> +       Used for installing the appropriate firmware boot.bin.
>
> +config BR2_PACKAGE_VERSAL_FIRMWARE_VERSION
> +     string "versal firmware version"
> +     help
> +       Release version of versal firmware.

> The version entry should be just after the git tree entry.

Ok.  This is my first time doing this, so thank you for this feedback.

> +endif # BR2_PACKAGE_VERSAL_FIRMWARE
> diff --git a/package/versal-firmware/versal-firmware.mk 
> b/package/versal-firmware/versal-firmware.mk
> new file mode 100644
> index 0000000000..b465b2bd83
> --- /dev/null
> +++ b/package/versal-firmware/versal-firmware.mk
> @@ -0,0 +1,25 @@
> +#####################################################################
> +###########
> +#
> +# versal-firmware
> +#
> +#####################################################################
> +###########
> +
> +VERSAL_FIRMWARE_VERSION = $(call 
> +qstrip,$(BR2_PACKAGE_VERSAL_FIRMWARE_VERSION))
> +VERSAL_FIRMWARE_SITE = $(BR2_PACKAGE_VERSAL_FIRMWARE_LOCATION)
> +
> +VERSAL_FIRMWARE_INSTALL_IMAGES = YES
> +
> +VERSAL_FIRMWARE_FILES = \
> +     $(if $(BR2_PACKAGE_VERSAL_FIRMWARE), boot.bin)

This does not make sense: if the versal-firmware package is not enabled, then it will not be installed, so there is no need to condition some variables/values to the package being enabled or not.

> +define VERSAL_FIRMWARE_INSTALL_BIN
> +     $(foreach f,$(VERSAL_FIRMWARE_FILES), \
> +             $(INSTALL) -D -m 0644 
> +$(@D)/$(BR2_PACKAGE_VERSAL_FIRMWARE_BOARD)/$(f) $(BINARIES_DIR)/$(f)

> VERSAL_FIRMWARE_FILES is only ever one item, boot.bin, the loop will always be on one item, so there is no need for the loop.

> Also, no need for a seaprate macro; just add this code to VERSAL_FIRMWARE_INSTALL_IMAGES_CMDS.

Same comment as above.  I was working from other examples of firmware downloading to create this patch set, and I appreciate your feedback for making it simpler.

> In the end, I don't think we should apply this package, as it looks like it tries to circumvent the official distrobution restrictions on these firmware files.

I understand, and I hope to be able to come up with a solution.
If you have any ideas for how buildroot might be able to generate the boot.bin, I am happy to receive any help.

> Regards,
> Yann E. MORIN.

> +     )
> +endef
> +
> +define VERSAL_FIRMWARE_INSTALL_IMAGES_CMDS
> +     $(VERSAL_FIRMWARE_INSTALL_BIN)
> +endef
> +
> +$(eval $(generic-package))
> --
> 2.17.1
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

Best regards,
Neal Frager
AMD
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot