| Message ID | 20221124102144.16833-1-neal.frager@amd.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v6,1/2] package/versal-firmware: new package | expand |
On Thu, 24 Nov 2022 03:21:43 -0700 Neal Frager via buildroot <buildroot@buildroot.org> wrote: > This patch adds support for downloading versal microblaze firmware binaries. > These are necessary for booting Xilinx versal devices. > > The location of these binaries is temporary, and will soon be added to the > Xilinx firmware repository. The temporary location is using the same free > distribution license as the Xilinx firmware repository. > > Once these files are available on the Xilinx repository, this package will > be updated to the new location. > > Signed-off-by: Neal Frager <neal.frager@amd.com> I wanted to apply this... but there is a serious licensing issue I believe, see below. > diff --git a/package/versal-firmware/Config.in b/package/versal-firmware/Config.in > new file mode 100644 > index 0000000000..122ef02175 > --- /dev/null > +++ b/package/versal-firmware/Config.in > @@ -0,0 +1,24 @@ > +config BR2_PACKAGE_VERSAL_FIRMWARE > + bool "versal-firmware" > + depends on BR2_aarch64 > + help > + Pre-built firmware files for Xilinx Versal boards. > + > + https://github.com/nealfrager/buildroot-firmware > + > +if BR2_PACKAGE_VERSAL_FIRMWARE > + > +config BR2_PACKAGE_VERSAL_FIRMWARE_VERSION > + string "versal firmware version" Drop "versal" here: this option appears indented under "versal-firmware", so repeating "versal" is not needed. > + default v2022.2 Quotes around v2022.2 > +config BR2_PACKAGE_VERSAL_FIRMWARE_BOARD > + string "versal board name" > + default vck190 Same comments here. > + help > + Name of Versal target board. > + Used for installing the appropriate firmware boot.bin. > + > +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..cb36d1e045 > --- /dev/null > +++ b/package/versal-firmware/versal-firmware.mk > @@ -0,0 +1,23 @@ > +################################################################################ > +# > +# versal-firmware > +# > +################################################################################ > + > +VERSAL_FIRMWARE_VERSION = $(call qstrip,$(BR2_PACKAGE_VERSAL_FIRMWARE_VERSION)) > +VERSAL_FIRMWARE_SITE = $(call github,nealfrager,buildroot-firmware,$(BR2_PACKAGE_VERSAL_FIRMWARE_VERSION)) Use $(VERSAL_FIRMWARE_VERSION) here > +VERSAL_FIRMWARE_LICENSE = "Xilinx-Binary-Only or GPL-2.0-or-later" > + > +VERSAL_FIRMWARE_LICENSE_FILES = LICENSE I really don't understand how this repository code can be under GPL-2.0-or-later. The GPL implies that source code is available as the GPL explicitly states that the software should be provided under its "preferred form of modification", which a closed source binary definitely is not. So when the LICENSE file says: COPYRIGHT TEXT: --------------- The files distributed with this project are provided in binary form under the following free and permissive binary-only license; source files are not provided. While the following free and permissive license is similar to the MIT open source license, this it is NOT the MIT open source license or any other OSI-approved open-source license. Copyright (c) 2022 Xilinx, Inc Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. Files: kv260/kv260_pmufw.elf zcu102/zcu102_pmufw.elf zcu106/zcu106_pmufw.elf vck190/vck190_plm.elf vck190/vck190_psmfw.elf vck190/vck190_vpl_gen_fixed.pdi --------------------------------------------------------------------------------------------------- COPYRIGHT TEXT: --------------- Copyright (C) 2010 - 2022 <Xilinx, Inc> This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program; if not, see <http://www.gnu.org/licenses/> SPDX-License-Identifier: GPL-2.0 / Files: kv260/kv260_pmufw.elf zcu102/zcu102_pmufw.elf zcu106/zcu106_pmufw.elf vck190/vck190_plm.elf vck190/vck190_psmfw.elf vck190/vck190_vpl_gen_fixed.pdi The first part somewhat makes sense, but the second part certainly does not make sense. If those binary files contain GPLv2 code, providing only the binaries is a violation of the GPL. So I'm sorry, but this needs to be fixed up before we can consider applying this package. Best regards, Thomas
Hi Thomas, > This patch adds support for downloading versal microblaze firmware binaries. > These are necessary for booting Xilinx versal devices. > > The location of these binaries is temporary, and will soon be added to > the Xilinx firmware repository. The temporary location is using the > same free distribution license as the Xilinx firmware repository. > > Once these files are available on the Xilinx repository, this package > will be updated to the new location. > > Signed-off-by: Neal Frager <neal.frager@amd.com> > I wanted to apply this... but there is a serious licensing issue I believe, see below. Thank you for your review and feedback. Licensing is still something that I do not understand as well as I probably should. > diff --git a/package/versal-firmware/Config.in > b/package/versal-firmware/Config.in > new file mode 100644 > index 0000000000..122ef02175 > --- /dev/null > +++ b/package/versal-firmware/Config.in > @@ -0,0 +1,24 @@ > +config BR2_PACKAGE_VERSAL_FIRMWARE > + bool "versal-firmware" > + depends on BR2_aarch64 > + help > + Pre-built firmware files for Xilinx Versal boards. > + > + https://github.com/nealfrager/buildroot-firmware > + > +if BR2_PACKAGE_VERSAL_FIRMWARE > + > +config BR2_PACKAGE_VERSAL_FIRMWARE_VERSION > + string "versal firmware version" > Drop "versal" here: this option appears indented under "versal-firmware", so repeating "versal" is not needed. Ok, no problem. > + default v2022.2 > Quotes around v2022.2 Ok. I will fix this. > +config BR2_PACKAGE_VERSAL_FIRMWARE_BOARD > + string "versal board name" > + default vck190 > Same comments here. Understood. I will fix this too. > + help > + Name of Versal target board. > + Used for installing the appropriate firmware boot.bin. > + > +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..cb36d1e045 > --- /dev/null > +++ b/package/versal-firmware/versal-firmware.mk > @@ -0,0 +1,23 @@ > +##################################################################### > +########### > +# > +# versal-firmware > +# > +##################################################################### > +########### > + > +VERSAL_FIRMWARE_VERSION = $(call > +qstrip,$(BR2_PACKAGE_VERSAL_FIRMWARE_VERSION)) > +VERSAL_FIRMWARE_SITE = $(call > +github,nealfrager,buildroot-firmware,$(BR2_PACKAGE_VERSAL_FIRMWARE_VE > +RSION)) > Use $(VERSAL_FIRMWARE_VERSION) here No problem. Thanks for catching this. > +VERSAL_FIRMWARE_LICENSE = "Xilinx-Binary-Only or GPL-2.0-or-later" > + > +VERSAL_FIRMWARE_LICENSE_FILES = LICENSE > I really don't understand how this repository code can be under GPL-2.0-or-later. The GPL implies that source code is available as the GPL explicitly states that the software should be provided under its "preferred form of modification", which a closed source binary definitely is not. > The first part somewhat makes sense, but the second part certainly does not make sense. If those binary files contain GPLv2 code, providing only the binaries is a violation of the GPL. > So I'm sorry, but this needs to be fixed up before we can consider applying this package. I copied the LICENSE file from the Xilinx/ubuntu-firmware repo below. Looking more closely at it, I believe you are right that the license should just be Xilinx-Binary-Only. I will update the LICENSE file to only include the binary portion as described in the below license: https://github.com/Xilinx/ubuntu-firmware/blob/main/LICENSE The source code for the versal_plm and versal_psmfw applications comes from the same Xilinx/embeddedsw repository as the zynqmp_pmufw, so the source and binary licenses for all of these files should be the same. https://github.com/Xilinx/embeddedsw/tree/master/lib/sw_apps If I modify the LICENSE file and remove the commentary about GPL, do you think this could be acceptable for inclusion in buildroot? If there is anything else I need to add, please let me know. Thanks again for your help. Best regards, Neal Frager AMD
Hello, On Fri, 25 Nov 2022 09:29:05 +0000 "Frager, Neal" <neal.frager@amd.com> wrote: > I copied the LICENSE file from the Xilinx/ubuntu-firmware repo below. > Looking more closely at it, I believe you are right that the license > should just be Xilinx-Binary-Only. > > I will update the LICENSE file to only include the binary portion as > described in the below license: > https://github.com/Xilinx/ubuntu-firmware/blob/main/LICENSE > > The source code for the versal_plm and versal_psmfw applications > comes from the same Xilinx/embeddedsw repository as the zynqmp_pmufw, > so the source and binary licenses for all of these files should be > the same. https://github.com/Xilinx/embeddedsw/tree/master/lib/sw_apps Is this the full source code needed to build the versal-firmware? If so, why do we use a precompiled binary? We should instead build it from source from this repository. > If I modify the LICENSE file and remove the commentary about GPL, do > you think this could be acceptable for inclusion in buildroot? If > there is anything else I need to add, please let me know. Yes, if the pre-compiled binaries are distributed under the Xilinx-Binary-Only license, then that's OK. Thomas
Hi Thomas, > Le 25 nov. 2022 à 06:02, Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit : > > Hello, > >> On Fri, 25 Nov 2022 09:29:05 +0000 >> "Frager, Neal" <neal.frager@amd.com> wrote: >> >> I copied the LICENSE file from the Xilinx/ubuntu-firmware repo below. >> Looking more closely at it, I believe you are right that the license >> should just be Xilinx-Binary-Only. >> >> I will update the LICENSE file to only include the binary portion as >> described in the below license: >> https://github.com/Xilinx/ubuntu-firmware/blob/main/LICENSE >> >> The source code for the versal_plm and versal_psmfw applications >> comes from the same Xilinx/embeddedsw repository as the zynqmp_pmufw, >> so the source and binary licenses for all of these files should be >> the same. https://github.com/Xilinx/embeddedsw/tree/master/lib/sw_apps > > Is this the full source code needed to build the versal-firmware? If > so, why do we use a precompiled binary? We should instead build it from > source from this repository. Yes, it is the full source code, just like there is the full source code for the zynqmp pmufw. The problem is that all 3 of these applications are running on a microblaze. So to build them, we need to be able to work with two compilers. A microblaze compiler for all the firmware applications and an ARM compiler for the rest. Normally, it would be much better if we could build the pmufw for zynqmp and the plm and psmfw applications for versal. We have discussed this before, but do you have any new ideas for how we could configure buildroot to download and build two compilers for two different architectures? > >> If I modify the LICENSE file and remove the commentary about GPL, do >> you think this could be acceptable for inclusion in buildroot? If >> there is anything else I need to add, please let me know. > > Yes, if the pre-compiled binaries are distributed under the > Xilinx-Binary-Only license, then that's OK. > Great, I have modified the license file accordingly. > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering and training > https://bootlin.com Best regards, Neal Frager AMD
Hello Neal, On Fri, 25 Nov 2022 11:18:43 +0000 "Frager, Neal" <neal.frager@amd.com> wrote: > Yes, it is the full source code, just like there is the full source > code for the zynqmp pmufw. The problem is that all 3 of these > applications are running on a microblaze. > > So to build them, we need to be able to work with two compilers. A > microblaze compiler for all the firmware applications and an ARM > compiler for the rest. > > Normally, it would be much better if we could build the pmufw for > zynqmp and the plm and psmfw applications for versal. We have > discussed this before, but do you have any new ideas for how we could > configure buildroot to download and build two compilers for two > different architectures? Sure: we are already doing that for some ARM64 platforms that need to build ARM32 firmware: this needs a different compiler. It is for this reason that we have the package/arm-gnu-toolchain/ package, which downloads and installs a pre-compiled bare-metal ARM32 toolchain. Then this toolchain is for example used by boot/arm-trusted-firmware/ in some situations: ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_NEEDS_ARM32_TOOLCHAIN),y) ARM_TRUSTED_FIRMWARE_DEPENDENCIES += host-arm-gnu-toolchain endif So if you have a Microblaze bare metal toolchain available somewhere, we could create a package for it, and build the firmware from source. Best regards, Thomas
Hi Thomas, > Yes, it is the full source code, just like there is the full source > code for the zynqmp pmufw. The problem is that all 3 of these > applications are running on a microblaze. > > So to build them, we need to be able to work with two compilers. A > microblaze compiler for all the firmware applications and an ARM > compiler for the rest. > > Normally, it would be much better if we could build the pmufw for > zynqmp and the plm and psmfw applications for versal. We have > discussed this before, but do you have any new ideas for how we could > configure buildroot to download and build two compilers for two > different architectures? > Sure: we are already doing that for some ARM64 platforms that need to build ARM32 firmware: this needs a different compiler. > It is for this reason that we have the package/arm-gnu-toolchain/ package, which downloads and installs a pre-compiled bare-metal ARM32 toolchain. Then this toolchain is for example used by boot/arm-trusted-firmware/ in some situations: > ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_NEEDS_ARM32_TOOLCHAIN),y) > ARM_TRUSTED_FIRMWARE_DEPENDENCIES += host-arm-gnu-toolchain endif > So if you have a Microblaze bare metal toolchain available somewhere, we could create a package for it, and build the firmware from source. This is a good idea! It seems adding a microblaze compiler should be easier than I thought. I am not aware of any pre-built microblaze compiler that we could just download, but perhaps using a package, we could download and build a microblaze gcc compiler from sources? The Xilinx 2022.2 tools use a gcc 11.2.0 compiler with the following patch set for microblaze. It would be great if we could develop a package that builds the exact same compiler for buildroot. https://github.com/Xilinx/meta-xilinx/tree/xlnx-rel-v2022.2/meta-microblaze/recipes-devtools/gcc/gcc-11 Once we have a microblaze compiler available in buildroot, we could then build the zynqmp pmufw as well as the versal plm and psmfw applications from the Xilinx/embeddedsw sources. Then we would only have the vck190_vpl_gen_fixed.pdi file for versal that would need to be downloaded from a repository since it requires Vivado for generating it. Best regards, Neal Frager AMD
Hello Neal, On Fri, 25 Nov 2022 13:18:38 +0000 "Frager, Neal" <neal.frager@amd.com> wrote: > This is a good idea! It seems adding a microblaze compiler should be > easier than I thought. > > I am not aware of any pre-built microblaze compiler that we could > just download, but perhaps using a package, we could download and > build a microblaze gcc compiler from sources? This is much more difficult, we really don't have the infrastructure to do that in Buildroot right now. It requires building another binutils and another gcc than the target gcc, and Buildroot is clearly not prepared/designed for this. Thomas
> Yes, it is the full source code, just like there is the full source > code for the zynqmp pmufw. The problem is that all 3 of these > applications are running on a microblaze. > > So to build them, we need to be able to work with two compilers. A > microblaze compiler for all the firmware applications and an ARM > compiler for the rest. > > Normally, it would be much better if we could build the pmufw for > zynqmp and the plm and psmfw applications for versal. We have > discussed this before, but do you have any new ideas for how we could > configure buildroot to download and build two compilers for two > different architectures? > Sure: we are already doing that for some ARM64 platforms that need to build ARM32 firmware: this needs a different compiler. > It is for this reason that we have the package/arm-gnu-toolchain/ package, which downloads and installs a pre-compiled bare-metal ARM32 toolchain. Then this toolchain is for example used by boot/arm-trusted-firmware/ in some situations: > ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_NEEDS_ARM32_TOOLCHAIN),y) > ARM_TRUSTED_FIRMWARE_DEPENDENCIES += host-arm-gnu-toolchain endif > So if you have a Microblaze bare metal toolchain available somewhere, we could create a package for it, and build the firmware from source. I have gone ahead and made a v7 of the versal-firmware package anyway with the fixes you have requested. Even if we add a microblaze compiler package as a next step, the vck190_vpl_gen_fixed.pdi will still need to be downloaded from a binary repo, so the versal-firmware repo is needed in any case. Please let me know if any further changes are needed for v7. I will look into how to best implement a microblaze compiler package as a next step, so that the versal plm and psmfw elf files can be built instead of just downloaded and installed. Thanks again for your help and feedback! Best regards, Neal Frager AMD
diff --git a/DEVELOPERS b/DEVELOPERS index 2aecdb1c8f..4689453d54 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -2154,6 +2154,7 @@ F: configs/zynqmp_zcu102_defconfig F: configs/zynqmp_zcu106_defconfig F: configs/zynqmp_kria_kv260_defconfig F: package/bootgen/ +F: package/versal-firmware/ N: Nicola Di Lieto <nicola.dilieto@gmail.com> F: package/uacme/ diff --git a/package/Config.in b/package/Config.in index aef80f9ab0..ebc7df10ec 100644 --- a/package/Config.in +++ b/package/Config.in @@ -443,6 +443,7 @@ menu "Firmware" source "package/sunxi-boards/Config.in" source "package/ts4900-fpga/Config.in" source "package/ux500-firmware/Config.in" + source "package/versal-firmware/Config.in" source "package/wilc-firmware/Config.in" source "package/wilink-bt-firmware/Config.in" source "package/zd1211-firmware/Config.in" diff --git a/package/versal-firmware/Config.in b/package/versal-firmware/Config.in new file mode 100644 index 0000000000..122ef02175 --- /dev/null +++ b/package/versal-firmware/Config.in @@ -0,0 +1,24 @@ +config BR2_PACKAGE_VERSAL_FIRMWARE + bool "versal-firmware" + depends on BR2_aarch64 + help + Pre-built firmware files for Xilinx Versal boards. + + https://github.com/nealfrager/buildroot-firmware + +if BR2_PACKAGE_VERSAL_FIRMWARE + +config BR2_PACKAGE_VERSAL_FIRMWARE_VERSION + string "versal firmware version" + default v2022.2 + help + Release version of Versal firmware. + +config BR2_PACKAGE_VERSAL_FIRMWARE_BOARD + string "versal board name" + default vck190 + help + Name of Versal target board. + Used for installing the appropriate firmware boot.bin. + +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..cb36d1e045 --- /dev/null +++ b/package/versal-firmware/versal-firmware.mk @@ -0,0 +1,23 @@ +################################################################################ +# +# versal-firmware +# +################################################################################ + +VERSAL_FIRMWARE_VERSION = $(call qstrip,$(BR2_PACKAGE_VERSAL_FIRMWARE_VERSION)) +VERSAL_FIRMWARE_SITE = $(call github,nealfrager,buildroot-firmware,$(BR2_PACKAGE_VERSAL_FIRMWARE_VERSION)) +VERSAL_FIRMWARE_LICENSE = "Xilinx-Binary-Only or GPL-2.0-or-later" + +VERSAL_FIRMWARE_LICENSE_FILES = LICENSE + +VERSAL_FIRMWARE_INSTALL_TARGET = NO +VERSAL_FIRMWARE_INSTALL_IMAGES = YES + +define VERSAL_FIRMWARE_INSTALL_IMAGES_CMDS + $(foreach f,plm.elf psmfw.elf vpl_gen_fixed.pdi,\ + $(INSTALL) -D -m 0755 $(@D)/$(BR2_PACKAGE_VERSAL_FIRMWARE_BOARD)/$(BR2_PACKAGE_VERSAL_FIRMWARE_BOARD)_$(f) \ + $(BINARIES_DIR)/$(BR2_PACKAGE_VERSAL_FIRMWARE_BOARD)_$(f) + ) +endef + +$(eval $(generic-package))
This patch adds support for downloading versal microblaze firmware binaries. These are necessary for booting Xilinx versal devices. The location of these binaries is temporary, and will soon be added to the Xilinx firmware repository. The temporary location is using the same free distribution license as the Xilinx firmware repository. Once these files are available on the Xilinx repository, this package will be updated to the new location. Signed-off-by: Neal Frager <neal.frager@amd.com> --- V1->V2: - set defaults for version and board name - improved format of install commands V2->V3: - no changes V3->V4: - no changes V4->V5: - no changes V5->V6: - no changes --- DEVELOPERS | 1 + package/Config.in | 1 + package/versal-firmware/Config.in | 24 ++++++++++++++++++++++ package/versal-firmware/versal-firmware.mk | 23 +++++++++++++++++++++ 4 files changed, 49 insertions(+) create mode 100644 package/versal-firmware/Config.in create mode 100644 package/versal-firmware/versal-firmware.mk