| Message ID | 20230904100443.1613306-1-neal.frager@amd.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v3,1/6] package/binutils-bare-metal: new package | expand |
Hello Neal, Ibai, thank you for your persistence in working on this! The overall patch set appears pretty clean, except for a few remarks as you can read in this and the other replies. I had a small hiccup while trying to apply your patches using 'git am' from my inbox: error: cannot convert from y to UTF-8 This is probably due to this weird header value: Content-Type: text/plain; charset="y" Probably some dirt in your git config. However the mbox file as downloaded from patchwork did apply without issues. I don't think you need to resend the series just for this. On Mon, 4 Sep 2023 11:04:38 +0100 Neal Frager <neal.frager@amd.com> wrote: > This patch adds a new package for building binutils for a bare-metal toolchain. > The cpu architecture is defined by a toolchain-bare-metal virtual package. > While any cpu architecture could be used, the default configuration will be a > Xilinx microblaze little endian architecture, so that buildroot will be able > to build the microblaze firmware applications for zynqmp and versal. > > When configured for the Xilinx microblaze architecture, all of the binutils > patches that are applied to the Xilinx distributed toolchain will be applied > in order to generate a toolchain that is equivalent to what Xilinx distributes. > > Signed-off-by: Ibai Erkiaga-Elorza <ibai.erkiaga-elorza@amd.com> > Signed-off-by: Neal Frager <neal.frager@amd.com> ... > diff --git a/package/binutils-bare-metal/Config.in.host b/package/binutils-bare-metal/Config.in.host > new file mode 100644 > index 0000000000..036698d418 > --- /dev/null > +++ b/package/binutils-bare-metal/Config.in.host > @@ -0,0 +1,19 @@ > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > + bool "host binutils-bare-metal" > + help > + binutils-bare-metal is a host utility for a > + bare-metal toolchain "a host utility seems a bit of an understatement, I'd rather say "Build to GNU binutils for a bare-metal toolchain" to clarify this is building no less than the GNU binutils. > + > +if BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > + > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION > + string > + default "2.39" > + > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_EXTRA_CONFIG_OPTIONS > + string "Additional binutils options" > + default "" > + help > + Any additional binutils options you may want to include Do we really want this without a valid use case? The same question could apply to the _VERSION setting, however I feel it's reasonable to keep it... This applies to patches 2 and 3 as well. > diff --git a/package/binutils-bare-metal/binutils-bare-metal.mk b/package/binutils-bare-metal/binutils-bare-metal.mk > new file mode 100644 > index 0000000000..fd983abc93 > --- /dev/null > +++ b/package/binutils-bare-metal/binutils-bare-metal.mk > @@ -0,0 +1,56 @@ > +################################################################################ > +# > +# binutils-bare-metal > +# > +################################################################################ > + > +HOST_BINUTILS_BARE_METAL_VERSION = $(call qstrip,$(BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION)) > +ifeq ($(HOST_BINUTILS_BARE_METAL_VERSION),) > +HOST_BINUTILS_BARE_METAL_VERSION = 2.39 > +endif # BINUTILS_VERSION > + > +HOST_BINUTILS_BARE_METAL_SITE ?= $(BR2_GNU_MIRROR)/binutils > +HOST_BINUTILS_BARE_METAL_SOURCE ?= binutils-$(HOST_BINUTILS_BARE_METAL_VERSION).tar.xz > + > +HOST_BINUTILS_BARE_METAL_LICENSE = GPL-3.0+, libiberty LGPL-2.1+ > +HOST_BINUTILS_BARE_METAL_LICENSE_FILES = COPYING3 COPYING.LIB > +HOST_BINUTILS_BARE_METAL_CPE_ID_VENDOR = gnu > + > +HOST_BINUTILS_BARE_METAL_DEPENDENCIES = host-zlib > + > +# if toolchain is for microblazeel-xilinx, apply Xilinx patch set > +ifeq ($(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH),"microblazeel-xilinx") The toolchain-bare-metal is added in patch 4, thus this series as-is is not bisectable. I don't see any obvious solutions as toolchain-bare-metal selects binutils-bare-metal so I guess some maintainer can provide some hints on how to handle this. The same issue applies to patches 2 and 3. > +HOST_BINUTILS_BARE_METAL_EXTRA_DOWNLOADS = https://github.com/Xilinx/meta-xilinx/archive/refs/tags/xlnx-rel-v2023.1.tar.gz This is not a huge download (500 kB compressed, 5.6 MB uncompressed), but we use the binutils patches only, which account for 10% of the whole archive. I wonder whether there is a way to download only a subdirectory from github. And looking at the patches themselves, I wonder how many are actually needed. At a cursory look, some don't really look like production code. Are those changes being mainlined? > +define HOST_BINUTILS_BARE_METAL_EXTRACT_PATCHES > + mkdir -p $(@D)/patches > + tar -xf $(HOST_BINUTILS_BARE_METAL_DL_DIR)/xlnx-rel-v2023.1.tar.gz --strip-components=5 -C $(@D)/patches meta-xilinx-xlnx-rel-v2023.1/meta-microblaze/recipes-devtools/binutils/binutils Please split this very long line. e.g.: tar -xf $(HOST_BINUTILS_BARE_METAL_DL_DIR)/xlnx-rel-v2023.1.tar.gz \ --strip-components=5 \ -C $(@D)/patches meta-xilinx-xlnx-rel-v2023.1/meta-microblaze/recipes-devtools/binutils/binutils Luca
Hi Luca, > Hello Neal, Ibai, > thank you for your persistence in working on this! > The overall patch set appears pretty clean, except for a few remarks as you can read in this and the other replies. Thank you for the kind words. > I had a small hiccup while trying to apply your patches using 'git am' from my inbox: > error: cannot convert from y to UTF-8 > This is probably due to this weird header value: > Content-Type: text/plain; charset="y" > Probably some dirt in your git config. However the mbox file as downloaded from patchwork did apply without issues. I don't think you need to resend the series just for this. I will create a clean patch set with v4, so this should hopefully disappear. > This patch adds a new package for building binutils for a bare-metal toolchain. > The cpu architecture is defined by a toolchain-bare-metal virtual package. > While any cpu architecture could be used, the default configuration > will be a Xilinx microblaze little endian architecture, so that > buildroot will be able to build the microblaze firmware applications for zynqmp and versal. > > When configured for the Xilinx microblaze architecture, all of the > binutils patches that are applied to the Xilinx distributed toolchain > will be applied in order to generate a toolchain that is equivalent to what Xilinx distributes. > > Signed-off-by: Ibai Erkiaga-Elorza <ibai.erkiaga-elorza@amd.com> > Signed-off-by: Neal Frager <neal.frager@amd.com> ... > diff --git a/package/binutils-bare-metal/Config.in.host > b/package/binutils-bare-metal/Config.in.host > new file mode 100644 > index 0000000000..036698d418 > --- /dev/null > +++ b/package/binutils-bare-metal/Config.in.host > @@ -0,0 +1,19 @@ > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > + bool "host binutils-bare-metal" > + help > + binutils-bare-metal is a host utility for a > + bare-metal toolchain > "a host utility seems a bit of an understatement, I'd rather say "Build to GNU binutils for a bare-metal toolchain" to clarify this is building no less than the GNU binutils. I agree. I will change this in v4. > + > +if BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > + > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION > + string > + default "2.39" > + > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_EXTRA_CONFIG_OPTIONS > + string "Additional binutils options" > + default "" > + help > + Any additional binutils options you may want to include > Do we really want this without a valid use case? > The same question could apply to the _VERSION setting, however I feel it's reasonable to keep it... > This applies to patches 2 and 3 as well. My thought was that someone might want to add additional bare-metal options aside from the Xilinx microblaze target. If not building binutils, gcc or newlib for microblaze, users might want to change the version and the config options used. > diff --git a/package/binutils-bare-metal/binutils-bare-metal.mk > b/package/binutils-bare-metal/binutils-bare-metal.mk > new file mode 100644 > index 0000000000..fd983abc93 > --- /dev/null > +++ b/package/binutils-bare-metal/binutils-bare-metal.mk > @@ -0,0 +1,56 @@ > +##################################################################### > +########### > +# > +# binutils-bare-metal > +# > +##################################################################### > +########### > + > +HOST_BINUTILS_BARE_METAL_VERSION = $(call > +qstrip,$(BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION)) > +ifeq ($(HOST_BINUTILS_BARE_METAL_VERSION),) > +HOST_BINUTILS_BARE_METAL_VERSION = 2.39 endif # BINUTILS_VERSION > + > +HOST_BINUTILS_BARE_METAL_SITE ?= $(BR2_GNU_MIRROR)/binutils > +HOST_BINUTILS_BARE_METAL_SOURCE ?= > +binutils-$(HOST_BINUTILS_BARE_METAL_VERSION).tar.xz > + > +HOST_BINUTILS_BARE_METAL_LICENSE = GPL-3.0+, libiberty LGPL-2.1+ > +HOST_BINUTILS_BARE_METAL_LICENSE_FILES = COPYING3 COPYING.LIB > +HOST_BINUTILS_BARE_METAL_CPE_ID_VENDOR = gnu > + > +HOST_BINUTILS_BARE_METAL_DEPENDENCIES = host-zlib > + > +# if toolchain is for microblazeel-xilinx, apply Xilinx patch set > +ifeq > +($(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH),"microblazeel-xilinx") > The toolchain-bare-metal is added in patch 4, thus this series as-is is not bisectable. I don't see any obvious solutions as toolchain-bare-metal selects binutils-bare-metal so I guess some maintainer can provide some hints on how to handle this. > The same issue applies to patches 2 and 3. Ok, I am not sure what to do with this feedback. > +HOST_BINUTILS_BARE_METAL_EXTRA_DOWNLOADS = > +https://github.com/Xilinx/meta-xilinx/archive/refs/tags/xlnx-rel-v202 > +3.1.tar.gz > This is not a huge download (500 kB compressed, 5.6 MB uncompressed), but we use the binutils patches only, which account for 10% of the whole archive. I wonder whether there is a way to download only a subdirectory from github. > And looking at the patches themselves, I wonder how many are actually needed. At a cursory look, some don't really look like production code. > Are those changes being mainlined? Our initial plan was to make binutils and gcc versions that match the Xilinx software release as closely as possible. This is why we just took the patch set blindly. From my view, it is easier for me to maintain this if we either take all of the patches as is or none at all. > +define HOST_BINUTILS_BARE_METAL_EXTRACT_PATCHES > + mkdir -p $(@D)/patches > + tar -xf $(HOST_BINUTILS_BARE_METAL_DL_DIR)/xlnx-rel-v2023.1.tar.gz > +--strip-components=5 -C $(@D)/patches > +meta-xilinx-xlnx-rel-v2023.1/meta-microblaze/recipes-devtools/binutil > +s/binutils > Please split this very long line. e.g.: > tar -xf $(HOST_BINUTILS_BARE_METAL_DL_DIR)/xlnx-rel-v2023.1.tar.gz \ > --strip-components=5 \ > -C $(@D)/patches > meta-xilinx-xlnx-rel-v2023.1/meta-microblaze/recipes-devtools/binutils/binutils Yes, no problem for v4. Best regards, Neal Frager AMD
Hi Neal, On Fri, 22 Sep 2023 13:34:58 +0000 "Frager, Neal" <neal.frager@amd.com> wrote: > Hi Luca, > > > Hello Neal, Ibai, > > > thank you for your persistence in working on this! > > > The overall patch set appears pretty clean, except for a few remarks as you can read in this and the other replies. > > Thank you for the kind words. > > > I had a small hiccup while trying to apply your patches using 'git am' from my inbox: > > > error: cannot convert from y to UTF-8 > > > This is probably due to this weird header value: > > > Content-Type: text/plain; charset="y" > > > Probably some dirt in your git config. However the mbox file as downloaded from patchwork did apply without issues. I don't think you need to resend the series just for this. > > I will create a clean patch set with v4, so this should hopefully disappear. > > > > This patch adds a new package for building binutils for a bare-metal toolchain. > > The cpu architecture is defined by a toolchain-bare-metal virtual package. > > While any cpu architecture could be used, the default configuration > > will be a Xilinx microblaze little endian architecture, so that > > buildroot will be able to build the microblaze firmware applications for zynqmp and versal. > > > > When configured for the Xilinx microblaze architecture, all of the > > binutils patches that are applied to the Xilinx distributed toolchain > > will be applied in order to generate a toolchain that is equivalent to what Xilinx distributes. > > > > Signed-off-by: Ibai Erkiaga-Elorza <ibai.erkiaga-elorza@amd.com> > > Signed-off-by: Neal Frager <neal.frager@amd.com> > > ... > > > diff --git a/package/binutils-bare-metal/Config.in.host > > b/package/binutils-bare-metal/Config.in.host > > new file mode 100644 > > index 0000000000..036698d418 > > --- /dev/null > > +++ b/package/binutils-bare-metal/Config.in.host > > @@ -0,0 +1,19 @@ > > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > > + bool "host binutils-bare-metal" > > + help > > + binutils-bare-metal is a host utility for a > > + bare-metal toolchain > > > "a host utility seems a bit of an understatement, I'd rather say "Build to GNU binutils for a bare-metal toolchain" to clarify this is building no less than the GNU binutils. > > I agree. I will change this in v4. > > > + > > +if BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > > + > > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION > > + string > > + default "2.39" > > + > > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_EXTRA_CONFIG_OPTIONS > > + string "Additional binutils options" > > + default "" > > + help > > + Any additional binutils options you may want to include > > > Do we really want this without a valid use case? > > > The same question could apply to the _VERSION setting, however I feel it's reasonable to keep it... > > > This applies to patches 2 and 3 as well. > > My thought was that someone might want to add additional bare-metal options aside from the Xilinx microblaze target. > > If not building binutils, gcc or newlib for microblaze, users might want to change the version and the config options used. > > > diff --git a/package/binutils-bare-metal/binutils-bare-metal.mk > > b/package/binutils-bare-metal/binutils-bare-metal.mk > > new file mode 100644 > > index 0000000000..fd983abc93 > > --- /dev/null > > +++ b/package/binutils-bare-metal/binutils-bare-metal.mk > > @@ -0,0 +1,56 @@ > > +##################################################################### > > +########### > > +# > > +# binutils-bare-metal > > +# > > +##################################################################### > > +########### > > + > > +HOST_BINUTILS_BARE_METAL_VERSION = $(call > > +qstrip,$(BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION)) > > +ifeq ($(HOST_BINUTILS_BARE_METAL_VERSION),) > > +HOST_BINUTILS_BARE_METAL_VERSION = 2.39 endif # BINUTILS_VERSION > > + > > +HOST_BINUTILS_BARE_METAL_SITE ?= $(BR2_GNU_MIRROR)/binutils > > +HOST_BINUTILS_BARE_METAL_SOURCE ?= > > +binutils-$(HOST_BINUTILS_BARE_METAL_VERSION).tar.xz > > + > > +HOST_BINUTILS_BARE_METAL_LICENSE = GPL-3.0+, libiberty LGPL-2.1+ > > +HOST_BINUTILS_BARE_METAL_LICENSE_FILES = COPYING3 COPYING.LIB > > +HOST_BINUTILS_BARE_METAL_CPE_ID_VENDOR = gnu > > + > > +HOST_BINUTILS_BARE_METAL_DEPENDENCIES = host-zlib > > + > > +# if toolchain is for microblazeel-xilinx, apply Xilinx patch set > > +ifeq > > +($(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH),"microblazeel-xilinx") > > > The toolchain-bare-metal is added in patch 4, thus this series as-is is not bisectable. I don't see any obvious solutions as toolchain-bare-metal selects binutils-bare-metal so I guess some maintainer can provide some hints on how to handle this. > > > The same issue applies to patches 2 and 3. > > Ok, I am not sure what to do with this feedback. I'd say waiting for Buildroot maintainers feedback. > > +HOST_BINUTILS_BARE_METAL_EXTRA_DOWNLOADS = > > +https://github.com/Xilinx/meta-xilinx/archive/refs/tags/xlnx-rel-v202 > > +3.1.tar.gz > > > This is not a huge download (500 kB compressed, 5.6 MB uncompressed), but we use the binutils patches only, which account for 10% of the whole archive. I wonder whether there is a way to download only a subdirectory from github. > > > And looking at the patches themselves, I wonder how many are actually needed. At a cursory look, some don't really look like production code. > > Are those changes being mainlined? > > Our initial plan was to make binutils and gcc versions that match the Xilinx software release as closely as possible. This is why we just took the patch set blindly. > > From my view, it is easier for me to maintain this if we either take all of the patches as is or none at all. Agreed it is simpler to maintain, so this boils down to the question: are those patches being actively mainlined? I have a sad feeling about it as the number and overall quality of those patches seems the same I saw more than an year ago. Luca
Hi Luca, > Hi Luca, > > > Hello Neal, Ibai, > > > thank you for your persistence in working on this! > > > The overall patch set appears pretty clean, except for a few remarks as you can read in this and the other replies. > > Thank you for the kind words. > > > I had a small hiccup while trying to apply your patches using 'git am' from my inbox: > > > error: cannot convert from y to UTF-8 > > > This is probably due to this weird header value: > > > Content-Type: text/plain; charset="y" > > > Probably some dirt in your git config. However the mbox file as downloaded from patchwork did apply without issues. I don't think you need to resend the series just for this. > > I will create a clean patch set with v4, so this should hopefully disappear. > > > > This patch adds a new package for building binutils for a bare-metal toolchain. > > The cpu architecture is defined by a toolchain-bare-metal virtual package. > > While any cpu architecture could be used, the default configuration > > will be a Xilinx microblaze little endian architecture, so that > > buildroot will be able to build the microblaze firmware applications for zynqmp and versal. > > > > When configured for the Xilinx microblaze architecture, all of the > > binutils patches that are applied to the Xilinx distributed > > toolchain will be applied in order to generate a toolchain that is equivalent to what Xilinx distributes. > > > > Signed-off-by: Ibai Erkiaga-Elorza <ibai.erkiaga-elorza@amd.com> > > Signed-off-by: Neal Frager <neal.frager@amd.com> > > ... > > > diff --git a/package/binutils-bare-metal/Config.in.host > > b/package/binutils-bare-metal/Config.in.host > > new file mode 100644 > > index 0000000000..036698d418 > > --- /dev/null > > +++ b/package/binutils-bare-metal/Config.in.host > > @@ -0,0 +1,19 @@ > > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > > + bool "host binutils-bare-metal" > > + help > > + binutils-bare-metal is a host utility for a > > + bare-metal toolchain > > > "a host utility seems a bit of an understatement, I'd rather say "Build to GNU binutils for a bare-metal toolchain" to clarify this is building no less than the GNU binutils. > > I agree. I will change this in v4. > > > + > > +if BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > > + > > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION > > + string > > + default "2.39" > > + > > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_EXTRA_CONFIG_OPTIONS > > + string "Additional binutils options" > > + default "" > > + help > > + Any additional binutils options you may want to include > > > Do we really want this without a valid use case? > > > The same question could apply to the _VERSION setting, however I feel it's reasonable to keep it... > > > This applies to patches 2 and 3 as well. > > My thought was that someone might want to add additional bare-metal options aside from the Xilinx microblaze target. > > If not building binutils, gcc or newlib for microblaze, users might want to change the version and the config options used. > > > diff --git a/package/binutils-bare-metal/binutils-bare-metal.mk > > b/package/binutils-bare-metal/binutils-bare-metal.mk > > new file mode 100644 > > index 0000000000..fd983abc93 > > --- /dev/null > > +++ b/package/binutils-bare-metal/binutils-bare-metal.mk > > @@ -0,0 +1,56 @@ > > +################################################################### > > +## > > +########### > > +# > > +# binutils-bare-metal > > +# > > +################################################################### > > +## > > +########### > > + > > +HOST_BINUTILS_BARE_METAL_VERSION = $(call > > +qstrip,$(BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION)) > > +ifeq ($(HOST_BINUTILS_BARE_METAL_VERSION),) > > +HOST_BINUTILS_BARE_METAL_VERSION = 2.39 endif # BINUTILS_VERSION > > + > > +HOST_BINUTILS_BARE_METAL_SITE ?= $(BR2_GNU_MIRROR)/binutils > > +HOST_BINUTILS_BARE_METAL_SOURCE ?= > > +binutils-$(HOST_BINUTILS_BARE_METAL_VERSION).tar.xz > > + > > +HOST_BINUTILS_BARE_METAL_LICENSE = GPL-3.0+, libiberty LGPL-2.1+ > > +HOST_BINUTILS_BARE_METAL_LICENSE_FILES = COPYING3 COPYING.LIB > > +HOST_BINUTILS_BARE_METAL_CPE_ID_VENDOR = gnu > > + > > +HOST_BINUTILS_BARE_METAL_DEPENDENCIES = host-zlib > > + > > +# if toolchain is for microblazeel-xilinx, apply Xilinx patch set > > +ifeq > > +($(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH),"microblazeel-xilinx > > +") > > > The toolchain-bare-metal is added in patch 4, thus this series as-is is not bisectable. I don't see any obvious solutions as toolchain-bare-metal selects binutils-bare-metal so I guess some maintainer can provide some hints on how to handle this. > > > The same issue applies to patches 2 and 3. > > Ok, I am not sure what to do with this feedback. > I'd say waiting for Buildroot maintainers feedback. > > +HOST_BINUTILS_BARE_METAL_EXTRA_DOWNLOADS = > > +https://github.com/Xilinx/meta-xilinx/archive/refs/tags/xlnx-rel-v2 > > +02 > > +3.1.tar.gz > > > This is not a huge download (500 kB compressed, 5.6 MB uncompressed), but we use the binutils patches only, which account for 10% of the whole archive. I wonder whether there is a way to download only a subdirectory from github. > > > And looking at the patches themselves, I wonder how many are actually needed. At a cursory look, some don't really look like production code. > > Are those changes being mainlined? > > Our initial plan was to make binutils and gcc versions that match the Xilinx software release as closely as possible. This is why we just took the patch set blindly. > > From my view, it is easier for me to maintain this if we either take all of the patches as is or none at all. > Agreed it is simpler to maintain, so this boils down to the question: > are those patches being actively mainlined? I have a sad feeling about it as the number and overall quality of those patches seems the same I saw more than an year ago. As a quick test, I tried removing the Xilinx patch set to see if the build would still work. I ended up getting the following build error with the zynqmp pmufw: microblaze_sleep.c:81:9: note: '#pragma message: For the sleep routines, assembly instructions are used' 81 | #pragma message ("For the sleep routines, assembly instructions are used") | ^~~~~~~ microblaze_selftest.S: Assembler messages: microblaze_selftest.S:554: Error: unknown opcode "bsifi" microblaze_selftest.S:555: Error: unknown opcode "bsefi" This is the same error we see when using the zynqmp-pmufw-builder. I will go through the gcc patch set. Perhaps instead of applying all patches, I just look for the one that fixes the above issue and apply that. Best regards, Neal Frager AMD
[AMD Official Use Only - General] Hi Ibai, Luca, > > > Hi Luca, > > > > > Hello Neal, Ibai, > > > > > thank you for your persistence in working on this! > > > > > The overall patch set appears pretty clean, except for a few > > > remarks as you > can read in this and the other replies. > > > > Thank you for the kind words. > > > > > I had a small hiccup while trying to apply your patches using 'git > > > am' from > my inbox: > > > > > error: cannot convert from y to UTF-8 > > > > > This is probably due to this weird header value: > > > > > Content-Type: text/plain; charset="y" > > > > > Probably some dirt in your git config. However the mbox file as > downloaded from patchwork did apply without issues. I don't think you > need to resend the series just for this. > > > > I will create a clean patch set with v4, so this should hopefully disappear. > > > > > > > This patch adds a new package for building binutils for a > > > bare-metal > toolchain. > > > The cpu architecture is defined by a toolchain-bare-metal virtual package. > > > While any cpu architecture could be used, the default > > > configuration will be a Xilinx microblaze little endian > > > architecture, so that buildroot will be able to build the > > > microblaze firmware applications for > zynqmp and versal. > > > > > > When configured for the Xilinx microblaze architecture, all of the > > > binutils patches that are applied to the Xilinx distributed > > > toolchain will be applied in order to generate a toolchain that is > > > equivalent > to what Xilinx distributes. > > > > > > Signed-off-by: Ibai Erkiaga-Elorza <ibai.erkiaga-elorza@amd.com> > > > Signed-off-by: Neal Frager <neal.frager@amd.com> > > > > ... > > > > > diff --git a/package/binutils-bare-metal/Config.in.host > > > b/package/binutils-bare-metal/Config.in.host > > > new file mode 100644 > > > index 0000000000..036698d418 > > > --- /dev/null > > > +++ b/package/binutils-bare-metal/Config.in.host > > > @@ -0,0 +1,19 @@ > > > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > > > + bool "host binutils-bare-metal" > > > + help > > > + binutils-bare-metal is a host utility for a > > > + bare-metal toolchain > > > > > "a host utility seems a bit of an understatement, I'd rather say > > > "Build to > GNU binutils for a bare-metal toolchain" to clarify this is building > no less than the GNU binutils. > > > > I agree. I will change this in v4. > > > > > + > > > +if BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > > > + > > > +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION > > > + string > > > + default "2.39" > > > + > > > +config > BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_EXTRA_CONFIG_OPTIONS > > > + string "Additional binutils options" > > > + default "" > > > + help > > > + Any additional binutils options you may want to include > > > > > Do we really want this without a valid use case? > > > > > The same question could apply to the _VERSION setting, however I > > > feel it's > reasonable to keep it... > > > > > This applies to patches 2 and 3 as well. > > > > My thought was that someone might want to add additional bare-metal > options aside from the Xilinx microblaze target. > > > > If not building binutils, gcc or newlib for microblaze, users might > > want to > change the version and the config options used. > > > > > diff --git a/package/binutils-bare-metal/binutils-bare-metal.mk > > > b/package/binutils-bare-metal/binutils-bare-metal.mk > > > new file mode 100644 > > > index 0000000000..fd983abc93 > > > --- /dev/null > > > +++ b/package/binutils-bare-metal/binutils-bare-metal.mk > > > @@ -0,0 +1,56 @@ > > > > +################################################################## > # > > > +## > > > +########### > > > +# > > > +# binutils-bare-metal > > > +# > > > > +################################################################## > # > > > +## > > > +########### > > > + > > > +HOST_BINUTILS_BARE_METAL_VERSION = $(call > > > +qstrip,$(BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION)) > > > +ifeq ($(HOST_BINUTILS_BARE_METAL_VERSION),) > > > +HOST_BINUTILS_BARE_METAL_VERSION = 2.39 endif # > BINUTILS_VERSION > > > + > > > +HOST_BINUTILS_BARE_METAL_SITE ?= $(BR2_GNU_MIRROR)/binutils > > > +HOST_BINUTILS_BARE_METAL_SOURCE ?= > > > +binutils-$(HOST_BINUTILS_BARE_METAL_VERSION).tar.xz > > > + > > > +HOST_BINUTILS_BARE_METAL_LICENSE = GPL-3.0+, libiberty LGPL-2.1+ > > > +HOST_BINUTILS_BARE_METAL_LICENSE_FILES = COPYING3 COPYING.LIB > > > +HOST_BINUTILS_BARE_METAL_CPE_ID_VENDOR = gnu > > > + > > > +HOST_BINUTILS_BARE_METAL_DEPENDENCIES = host-zlib > > > + > > > +# if toolchain is for microblazeel-xilinx, apply Xilinx patch set > > > +ifeq > > > > +($(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH),"microblazeel- > xilinx > > > +") > > > > > The toolchain-bare-metal is added in patch 4, thus this series > > > as-is is not > bisectable. I don't see any obvious solutions as toolchain-bare-metal > selects binutils-bare-metal so I guess some maintainer can provide > some hints on how to handle this. > > > > > The same issue applies to patches 2 and 3. > > > > Ok, I am not sure what to do with this feedback. > > > I'd say waiting for Buildroot maintainers feedback. > > > > +HOST_BINUTILS_BARE_METAL_EXTRA_DOWNLOADS = > > > +https://github.com/Xilinx/meta-xilinx/archive/refs/tags/xlnx-rel- > > > +v2 > > > +02 > > > +3.1.tar.gz > > > > > This is not a huge download (500 kB compressed, 5.6 MB > > > uncompressed), > but we use the binutils patches only, which account for 10% of the > whole archive. I wonder whether there is a way to download only a > subdirectory from github. > > > > > And looking at the patches themselves, I wonder how many are > > > actually > needed. At a cursory look, some don't really look like production code. > > > Are those changes being mainlined? > > > > Our initial plan was to make binutils and gcc versions that match > > the Xilinx > software release as closely as possible. This is why we just took the > patch set blindly. > > > > From my view, it is easier for me to maintain this if we either take > > all of the > patches as is or none at all. > > > Agreed it is simpler to maintain, so this boils down to the question: > > are those patches being actively mainlined? I have a sad feeling > > about it as > the number and overall quality of those patches seems the same I saw > more than an year ago. > > As a quick test, I tried removing the Xilinx patch set to see if the > build would still work. > > I ended up getting the following build error with the zynqmp pmufw: > microblaze_sleep.c:81:9: note: '#pragma message: For the sleep > routines, assembly instructions are used' > 81 | #pragma message ("For the sleep routines, assembly > instructions are > used") > | ^~~~~~~ > microblaze_selftest.S: Assembler messages: > microblaze_selftest.S:554: Error: unknown opcode "bsifi" > microblaze_selftest.S:555: Error: unknown opcode "bsefi" > > This is the same error we see when using the zynqmp-pmufw-builder. > > I will go through the gcc patch set. Perhaps instead of applying all > patches, I just look for the one that fixes the above issue and apply that. > > Note that the microblaze toolchain build in Yocto where these patches are coming from, is a multilib configuration to support different microblaze configurations. As the goal of this package is to build just a toolchain for the PMU (and PMC in Versal) there is no need for multilib configuration and neither for some of these patches (i.e. 64bit architecture). The tricky point is to figure out which ones are really needed for this use case 😊 I tried cherry-picking the patches needed for the bsifi and bsefi instructions. Unfortunately, both the binutils and gcc patch sets include many modifications of the same source files. For example, if I only apply the gcc patch 0024-Patch-MicroBlaze-this-patch-has-1.Fixed-the-bug-in.patch that adds the bsifi and bsefi functionality, it cannot apply unless I have several proceeding patches included too. And then when I only include the patches needed for applying this update, gcc does not build properly unless I include even more patches. The only way I can easily find for adding the patches for the bsifi and bsefi instructions is to just include the full set of 54 patches. When I do this, I can build gcc and the zynqmp pmufw without errors. Since patches are required in both the binutils set and the gcc set, I think the easiest solution for us is to just take the full patch set for each that yocto is using. I do not believe there is currently any organized effort to upstream any of these patches, but every AMD Xilinx software release will include a microblaze compiler with a patch set that we can apply. To keep maintenance of all this as easy as possible, I believe the best solution is to just apply the full patch set for binutils and gcc every time. Is this ok for both of you? Best regards, Neal Frager AMD
Hi Neal, Ibai, On Sat, 23 Sep 2023 09:50:34 +0000 "Frager, Neal" <neal.frager@amd.com> wrote: > > > From my view, it is easier for me to maintain this if we either take > > > all of the > > patches as is or none at all. > > > > > Agreed it is simpler to maintain, so this boils down to the question: > > > are those patches being actively mainlined? I have a sad feeling > > > about it as > > the number and overall quality of those patches seems the same I saw > > more than an year ago. > > > > As a quick test, I tried removing the Xilinx patch set to see if the > > build would still work. > > > > I ended up getting the following build error with the zynqmp pmufw: > > microblaze_sleep.c:81:9: note: '#pragma message: For the sleep > > routines, assembly instructions are used' > > 81 | #pragma message ("For the sleep routines, assembly > > instructions are > > used") > > | ^~~~~~~ > > microblaze_selftest.S: Assembler messages: > > microblaze_selftest.S:554: Error: unknown opcode "bsifi" > > microblaze_selftest.S:555: Error: unknown opcode "bsefi" > > > > This is the same error we see when using the zynqmp-pmufw-builder. > > > > I will go through the gcc patch set. Perhaps instead of applying all > > patches, I just look for the one that fixes the above issue and apply that. > > > > > Note that the microblaze toolchain build in Yocto where these patches are coming from, is a multilib configuration to support different microblaze configurations. As the goal of this package is to build just a toolchain for the PMU (and PMC in Versal) there is no need for multilib configuration and neither for some of these patches (i.e. 64bit architecture). The tricky point is to figure out which ones are really needed for this use case 😊 > > I tried cherry-picking the patches needed for the bsifi and bsefi instructions. Unfortunately, both the binutils and gcc patch sets include many modifications of the same source files. > > For example, if I only apply the gcc patch 0024-Patch-MicroBlaze-this-patch-has-1.Fixed-the-bug-in.patch that adds the bsifi and bsefi functionality, it cannot apply unless I have several proceeding patches included too. And then when I only include the patches needed for applying this update, gcc does not build properly unless I include even more patches. > > The only way I can easily find for adding the patches for the bsifi and bsefi instructions is to just include the full set of 54 patches. When I do this, I can build gcc and the zynqmp pmufw without errors. > > Since patches are required in both the binutils set and the gcc set, I think the easiest solution for us is to just take the full patch set for each that yocto is using. > > I do not believe there is currently any organized effort to upstream any of these patches, but every AMD Xilinx software release will include a microblaze compiler with a patch set that we can apply. To keep maintenance of all this as easy as possible, I believe the best solution is to just apply the full patch set for binutils and gcc every time. > > Is this ok for both of you? I'm OK with the whole approach, except for the sentence "I do not believe there is currently any organized effort to upstream any of these patches"... which is probably already clear to the recipients of this message, and thus is not going to be solved in this thread, however I just want to be sure my position is clear. I'd also like to stress that I appreciate a lot the work you are doing to properly support the pmufw in Buildroot. Thanks! Luca
Hi Luca, > Le 25 sept. 2023 à 04:59, Luca Ceresoli <luca.ceresoli@bootlin.com> a écrit : > > Hi Neal, Ibai, > > On Sat, 23 Sep 2023 09:50:34 +0000 > "Frager, Neal" <neal.frager@amd.com> wrote: > >>>> From my view, it is easier for me to maintain this if we either take >>>> all of the >>> patches as is or none at all. >>> >>>> Agreed it is simpler to maintain, so this boils down to the question: >>>> are those patches being actively mainlined? I have a sad feeling >>>> about it as >>> the number and overall quality of those patches seems the same I saw >>> more than an year ago. >>> >>> As a quick test, I tried removing the Xilinx patch set to see if the >>> build would still work. >>> >>> I ended up getting the following build error with the zynqmp pmufw: >>> microblaze_sleep.c:81:9: note: '#pragma message: For the sleep >>> routines, assembly instructions are used' >>> 81 | #pragma message ("For the sleep routines, assembly >>> instructions are >>> used") >>> | ^~~~~~~ >>> microblaze_selftest.S: Assembler messages: >>> microblaze_selftest.S:554: Error: unknown opcode "bsifi" >>> microblaze_selftest.S:555: Error: unknown opcode "bsefi" >>> >>> This is the same error we see when using the zynqmp-pmufw-builder. >>> >>> I will go through the gcc patch set. Perhaps instead of applying all >>> patches, I just look for the one that fixes the above issue and apply that. >>> >> >>> Note that the microblaze toolchain build in Yocto where these patches are coming from, is a multilib configuration to support different microblaze configurations. As the goal of this package is to build just a toolchain for the PMU (and PMC in Versal) there is no need for multilib configuration and neither for some of these patches (i.e. 64bit architecture). The tricky point is to figure out which ones are really needed for this use case 😊 >> >> I tried cherry-picking the patches needed for the bsifi and bsefi instructions. Unfortunately, both the binutils and gcc patch sets include many modifications of the same source files. >> >> For example, if I only apply the gcc patch 0024-Patch-MicroBlaze-this-patch-has-1.Fixed-the-bug-in.patch that adds the bsifi and bsefi functionality, it cannot apply unless I have several proceeding patches included too. And then when I only include the patches needed for applying this update, gcc does not build properly unless I include even more patches. >> >> The only way I can easily find for adding the patches for the bsifi and bsefi instructions is to just include the full set of 54 patches. When I do this, I can build gcc and the zynqmp pmufw without errors. >> >> Since patches are required in both the binutils set and the gcc set, I think the easiest solution for us is to just take the full patch set for each that yocto is using. >> >> I do not believe there is currently any organized effort to upstream any of these patches, but every AMD Xilinx software release will include a microblaze compiler with a patch set that we can apply. To keep maintenance of all this as easy as possible, I believe the best solution is to just apply the full patch set for binutils and gcc every time. >> >> Is this ok for both of you? > > I'm OK with the whole approach, except for the sentence "I do not > believe there is currently any organized effort to upstream any of > these patches"... which is probably already clear to the recipients > of this message, and thus is not going to be solved in this thread, > however I just want to be sure my position is clear. I'd also like to > stress that I appreciate a lot the work you are doing to properly > support the pmufw in Buildroot. Thanks! > > Luca Your position is very clear. And I can assure you that both Ibai and I agree with it. It would be much better if all of these binutils and gcc patches for microblaze go upstream, and both Ibai and I have pushed for it internally at AMD / Xilinx. The only thing I can say is that change is always possible. Yesterday, we could not build a zynqmp pmufw, versal plm or versal psmfw in buildroot. Today, we have submitted a solution to change that. Tomorrow (figurative meaning the future), we hope to get all these binutils and gcc patches upstream, so the upstream toolchain matches the AMD Xilinx distributed toolchain. One step at a time. > > -- > Luca Ceresoli, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com Best regards, Neal Frager AMD
>>>>> "Frager," == Frager, Neal <neal.frager@amd.com> writes: Hello, >>> Is this ok for both of you? >> >> I'm OK with the whole approach, except for the sentence "I do not >> believe there is currently any organized effort to upstream any of >> these patches"... which is probably already clear to the recipients >> of this message, and thus is not going to be solved in this thread, >> however I just want to be sure my position is clear. I'd also like to >> stress that I appreciate a lot the work you are doing to properly >> support the pmufw in Buildroot. Thanks! >> >> Luca > Your position is very clear. And I can assure you that both Ibai and I agree with it. > It would be much better if all of these binutils and gcc patches for > microblaze go upstream, and both Ibai and I have pushed for it > internally at AMD / Xilinx. > The only thing I can say is that change is always possible. > Yesterday, we could not build a zynqmp pmufw, versal plm or versal > psmfw in buildroot. Today, we have submitted a solution to change > that. > Tomorrow (figurative meaning the future), we hope to get all these > binutils and gcc patches upstream, so the upstream toolchain matches > the AMD Xilinx distributed toolchain. > One step at a time. Sure! Sorry, I am somewhat late to the review game here. I wonder how this fits with Luca's zynqmp-pmufw-builder? E.G. today the setup is that the pmufw is built outside Buildroot and we just point the u-boot package to where it can fetch the prebuilt firmware binary - This is nice in the sense that it is fast and simple, but makes is somewhat annoying to make modifications to the firmware. This series instead goes to the other extreme, E.G. we build the entire microblaze toolchain from scratch and then use it to build the firmware and use it in the u-boot package - This is nice because it is all in Buildroot and we have it all under control, but also brings quite some build time overhead for building the toolchain before building the (small) toolchain. You can naturally "solve" it by using two defconfigs, E.G. one that builds the pmufw and another that uses the prebuilt one, but it isn't very handy either. Would an in between option not be more interesting, E.G. use (or download) a prebuilt microblaze toolchain and use that to build the firmware? That would still give the flexibility to easily tweak the firmware, but not the overhead of building the toolchain every time? I guess the problems with that are what to do about the meta-xilinx patches and where/who wants to host a prebuilt one?
Hello Peter, >>> Is this ok for both of you? >> >> I'm OK with the whole approach, except for the sentence "I do not >> believe there is currently any organized effort to upstream any of >> these patches"... which is probably already clear to the recipients >> of this message, and thus is not going to be solved in this thread, >> however I just want to be sure my position is clear. I'd also like to >> stress that I appreciate a lot the work you are doing to properly >> support the pmufw in Buildroot. Thanks! >> >> Luca > Your position is very clear. And I can assure you that both Ibai and I agree with it. > It would be much better if all of these binutils and gcc patches for > microblaze go upstream, and both Ibai and I have pushed for it > internally at AMD / Xilinx. > The only thing I can say is that change is always possible. > Yesterday, we could not build a zynqmp pmufw, versal plm or versal > psmfw in buildroot. Today, we have submitted a solution to change > that. > Tomorrow (figurative meaning the future), we hope to get all these > binutils and gcc patches upstream, so the upstream toolchain matches > the AMD Xilinx distributed toolchain. > One step at a time. > Sure! Sorry, I am somewhat late to the review game here. I wonder how this fits with Luca's zynqmp-pmufw-builder? From my view, we can continue maintaining Luca's zynqmp-pmufw-builder in parallel. Buildroot will still offer the option to accept a pre-built pmufw binary without needing to build a microblaze toolchain, so if users prefer to build the pmufw outside of buildroot (to have a faster buildroot build perhaps), this capability will still be available. > E.G. today the setup is that the pmufw is built outside Buildroot and we just point the u-boot package to where it can fetch the prebuilt firmware binary - This is nice in the sense that it is fast and simple, but makes is somewhat annoying to make modifications to the firmware. > This series instead goes to the other extreme, E.G. we build the entire microblaze toolchain from scratch and then use it to build the firmware and use it in the u-boot package - This is nice because it is all in Buildroot and we have it all under control, but also brings quite some build time overhead for building the toolchain before building the (small) toolchain. You can naturally "solve" it by using two defconfigs, E.G. one that builds the pmufw and another that uses the prebuilt one, but it isn't very handy either. > Would an in between option not be more interesting, E.G. use (or download) a prebuilt microblaze toolchain and use that to build the firmware? That would still give the flexibility to easily tweak the firmware, but not the overhead of building the toolchain every time? > I guess the problems with that are what to do about the meta-xilinx patches and where/who wants to host a prebuilt one? This is a good point. As for the meta-xilinx patches, Ibai and I have started going through them and have even started upstreaming a few. The majority of the meta-xilinx patches are actually for enabling 64-bit microblaze support, which is not available in the upstream gnu binutils or gcc. Since we do not need 64-bit microblaze for the zynqmp pmufw or the versal plm and psmfw applications, we can easily skip these patches. Our current objective is to get all microblaze 32-bit bug fix and feature support patches (such as the barrel shift instructions used by the pmufw) upstreamed. Since this appears to be a small subset of the overall meta-xilinx patches, we hope to be able to enable the use of the upstream gnu binutils and gcc, so that the meta-xilinx patches will no longer be needed for our use case of building the zynqmp pmufw, versal plm and versal psmfw applications. However, in order to achieve what you are asking for, we would still need someone to host a pre-built microblaze compiler somewhere, if we would want to go this route. At the moment, it is not in the AMD Xilinx plan to host the toolchains somewhere outside of a Petalinux or Vitis download. If the upstream toolchain included all the necessary meta-xilinx patches, could bootlin potentially host a pre-built toolchain somewhere? Even if we add the option to use a pre-built microblaze toolchain, I would still like to include the option we have developed in buildroot to build and control everything within buildroot itself. It is true that the build time is longer, but many users like the option of building the toolchains they use and would be willing to pay the build time price to have this option. From my view, adding a pre-built toolchain option should not be "instead of" offering the option to build a toolchain, but should instead be an option in "addition to" building a toolchain. Basically, following the same concept as using the buildroot internal toolchain or supplying an external one for the main Linux toolchain. What are your thoughts? Best regards, Neal Frager AMD
Hi Neal, Peter, On Sun, 1 Oct 2023 16:11:23 +0000 "Frager, Neal" <neal.frager@amd.com> wrote: > Hello Peter, > > >>> Is this ok for both of you? > >> > >> I'm OK with the whole approach, except for the sentence "I do not >> believe there is currently any organized effort to upstream any of >> these patches"... which is probably already clear to the recipients >> of this message, and thus is not going to be solved in this thread, >> however I just want to be sure my position is clear. I'd also like to >> stress that I appreciate a lot the work you are doing to properly >> support the pmufw in Buildroot. Thanks! > >> > >> Luca > > > Your position is very clear. And I can assure you that both Ibai and I agree with it. > > > It would be much better if all of these binutils and gcc patches for > microblaze go upstream, and both Ibai and I have pushed for it > internally at AMD / Xilinx. > > > The only thing I can say is that change is always possible. > > Yesterday, we could not build a zynqmp pmufw, versal plm or versal > psmfw in buildroot. Today, we have submitted a solution to change > that. > > > Tomorrow (figurative meaning the future), we hope to get all these > binutils and gcc patches upstream, so the upstream toolchain matches > the AMD Xilinx distributed toolchain. > > > One step at a time. > > > Sure! Sorry, I am somewhat late to the review game here. I wonder how this fits with Luca's zynqmp-pmufw-builder? > > From my view, we can continue maintaining Luca's zynqmp-pmufw-builder in parallel. Buildroot will still offer the option to accept a pre-built pmufw binary without needing to build a microblaze toolchain, so if users prefer to build the pmufw outside of buildroot (to have a faster buildroot build perhaps), this capability will still be available. As Neal wrote, if/when Buildroot will be able to build a pmufw on its own (with the proposed approach, or a downloaded toolchain, or both), zynqmp-pmufw-builder can continue existing even though the use cases for it will be reduced. I have no plan to stop maintaining it for the foreseeable future. > > E.G. today the setup is that the pmufw is built outside Buildroot and we just point the u-boot package to where it can fetch the prebuilt firmware binary - This is nice in the sense that it is fast and simple, but makes is somewhat annoying to make modifications to the firmware. > > > This series instead goes to the other extreme, E.G. we build the entire microblaze toolchain from scratch and then use it to build the firmware and use it in the u-boot package - This is nice because it is all in Buildroot and we have it all under control, but also brings quite some build time overhead for building the toolchain before building the > (small) toolchain. You can naturally "solve" it by using two defconfigs, E.G. one that builds the pmufw and another that uses the prebuilt one, but it isn't very handy either. I think the approach taken here by Neal and Ibai is valuable, especially as it would allow Buildroot defconfigs to be self-standing. Additionally it is already proving useful as it prompted "the community" (mostly Neal -- thanks about that) to upstream patches needed to support Microblaze in binutils and gcc, that are currently downstream. > > Would an in between option not be more interesting, E.G. use (or > download) a prebuilt microblaze toolchain and use that to build the firmware? That would still give the flexibility to easily tweak the firmware, but not the overhead of building the toolchain every time? > > > I guess the problems with that are what to do about the meta-xilinx patches and where/who wants to host a prebuilt one? > > This is a good point. As for the meta-xilinx patches, Ibai and I have started going through them and have even started upstreaming a few. The majority of the meta-xilinx patches are actually for enabling 64-bit microblaze support, which is not available in the upstream gnu binutils or gcc. Since we do not need 64-bit microblaze for the zynqmp pmufw or the versal plm and psmfw applications, we can easily skip these patches. > > Our current objective is to get all microblaze 32-bit bug fix and feature support patches (such as the barrel shift instructions used by the pmufw) upstreamed. Since this appears to be a small subset of the overall meta-xilinx patches, we hope to be able to enable the use of the upstream gnu binutils and gcc, so that the meta-xilinx patches will no longer be needed for our use case of building the zynqmp pmufw, versal plm and versal psmfw applications. > > However, in order to achieve what you are asking for, we would still need someone to host a pre-built microblaze compiler somewhere, if we would want to go this route. At the moment, it is not in the AMD Xilinx plan to host the toolchains somewhere outside of a Petalinux or Vitis download. > > If the upstream toolchain included all the necessary meta-xilinx patches, could bootlin potentially host a pre-built toolchain somewhere? I fyou are thinking about toolchains.bootlin.com, I am not the maintainer of those toolchains but I think the idea is to only have Linux toolchains there, not bare metal ones, thus newlib is not supported. Also I'm pretty sure downstream feature patches are absolutely not welcome there. Luca
Hello Luca, On Tue, 3 Oct 2023 09:15:39 +0200 Luca Ceresoli <luca.ceresoli@bootlin.com> wrote: > > > This series instead goes to the other extreme, E.G. we build the > > > entire microblaze toolchain from scratch and then use it to build > > > the firmware and use it in the u-boot package - This is nice > > > because it is all in Buildroot and we have it all under control, > > > but also brings quite some build time overhead for building the > > > toolchain before building the > > (small) toolchain. You can naturally "solve" it by using two > > defconfigs, E.G. one that builds the pmufw and another that uses > > the prebuilt one, but it isn't very handy either. > > I think the approach taken here by Neal and Ibai is valuable, > especially as it would allow Buildroot defconfigs to be self-standing. > Additionally it is already proving useful as it prompted "the > community" (mostly Neal -- thanks about that) to upstream patches > needed to support Microblaze in binutils and gcc, that are currently > downstream. I agree. > I fyou are thinking about toolchains.bootlin.com, I am not the > maintainer of those toolchains but I think the idea is to only have > Linux toolchains there, not bare metal ones, thus newlib is not > supported. Also I'm pretty sure downstream feature patches are > absolutely not welcome there. Regarding providing a bare-metal toolchain on toolchains.bootlin.com, we have a chicken-and-egg problem: the toolchains on toolchains.bootlin.com are built using... Buildroot. So if we want toolchains.bootlin.com to offer pre-built bare-metal toolchains, we need Buildroot to be able to generate such bare-metal toolchains in the first place :-) Thomas
Hi Thomas, Peter, > > > This series instead goes to the other extreme, E.G. we build the > > > entire microblaze toolchain from scratch and then use it to build > > > the firmware and use it in the u-boot package - This is nice > > > because it is all in Buildroot and we have it all under control, > > > but also brings quite some build time overhead for building the > > > toolchain before building the > > (small) toolchain. You can naturally "solve" it by using two > > defconfigs, E.G. one that builds the pmufw and another that uses the > > prebuilt one, but it isn't very handy either. > > I think the approach taken here by Neal and Ibai is valuable, > especially as it would allow Buildroot defconfigs to be self-standing. > Additionally it is already proving useful as it prompted "the > community" (mostly Neal -- thanks about that) to upstream patches > needed to support Microblaze in binutils and gcc, that are currently > downstream. > I agree. If we agree that being able to build everything (including the microblaze bare-metal) toolchain is valuable, then should we go ahead and commit v4 of my patch set? We can always figure out how to add a pre-built microblaze toolchain later. And if users wish to have a faster build time, they can always use the option of a pre-built pmufw in their defconfig. This will skip the build of the microblaze bare-metal toolchain. > I fyou are thinking about toolchains.bootlin.com, I am not the > maintainer of those toolchains but I think the idea is to only have > Linux toolchains there, not bare metal ones, thus newlib is not > supported. Also I'm pretty sure downstream feature patches are > absolutely not welcome there. >Regarding providing a bare-metal toolchain on toolchains.bootlin.com, we have a chicken-and-egg problem: the toolchains on toolchains.bootlin.com are built using... Buildroot. So if we want toolchains.bootlin.com to offer pre-built bare-metal toolchains, we need Buildroot to be able to generate such bare-metal toolchains in the first place :-) Understood. We will have to think of a place to host a pre-built toolchain. By the way, Ibai and I have successfully identified the minimum patch set required for correctly building the zynqmp pmufw, versal plm and versal psmfw including the barrel shift instructions. We are in the process of upstreaming all the necessary patches to gnu binutils. Once upstreamed, we can move the toolchain-bare-metal packages to the upstream version without needing the meta-xilinx patch set. I will update the toolchain-bare-metal packages when this is ready. Basically, we are skipping the majority of the patches which are just for enabling 64-bit microblaze support which we do not require for zynqmp or versal. The upstream binutils and gcc are actually in very good shape for 32-bit microblaze support. That is why the zynqmp-pmufw-builder works already today. Best regards, Neal Frager AMD
diff --git a/DEVELOPERS b/DEVELOPERS index 26d0a0c223..1ebdf0a2c9 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -1327,6 +1327,9 @@ F: package/mrp/ N: Ian Haylock <haylocki@yahoo.co.uk> F: package/python-rpi-gpio/ +N: Ibai Erkiaga-Elorza <ibai.erkiaga-elorza@amd.com> +F: package/binutils-bare-metal/ + N: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr> F: package/angularjs/ @@ -2191,6 +2194,7 @@ F: configs/zynq_zc706_defconfig F: configs/zynqmp_kria_kv260_defconfig F: configs/zynqmp_zcu102_defconfig F: configs/zynqmp_zcu106_defconfig +F: package/binutils-bare-metal/ F: package/bootgen/ F: package/versal-firmware/ diff --git a/package/binutils-bare-metal/Config.in.host b/package/binutils-bare-metal/Config.in.host new file mode 100644 index 0000000000..036698d418 --- /dev/null +++ b/package/binutils-bare-metal/Config.in.host @@ -0,0 +1,19 @@ +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL + bool "host binutils-bare-metal" + help + binutils-bare-metal is a host utility for a + bare-metal toolchain + +if BR2_PACKAGE_HOST_BINUTILS_BARE_METAL + +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION + string + default "2.39" + +config BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_EXTRA_CONFIG_OPTIONS + string "Additional binutils options" + default "" + help + Any additional binutils options you may want to include + +endif #BR2_PACKAGE_HOST_BINUTILS_BARE_METAL diff --git a/package/binutils-bare-metal/binutils-bare-metal.hash b/package/binutils-bare-metal/binutils-bare-metal.hash new file mode 100644 index 0000000000..3402b5f2a9 --- /dev/null +++ b/package/binutils-bare-metal/binutils-bare-metal.hash @@ -0,0 +1,7 @@ +# From https://gcc.gnu.org/pub/binutils/releases/sha512.sum +sha512 68e038f339a8c21faa19a57bbc447a51c817f47c2e06d740847c6e9cc3396c025d35d5369fa8c3f8b70414757c89f0e577939ddc0d70f283182504920f53b0a3 binutils-2.39.tar.xz + +# locally computed +sha512 bf3561c3495dd112b269a2c21dd758c1e5e7a73f959052f63511313e44222ce85b8db81e8de3b60b2c0bb8668ee834ac85036517fb6970e06fe352765dd127d0 xlnx-rel-v2023.1.tar.gz +sha256 8ceb4b9ee5adedde47b31e975c1d90c73ad27b6b165a1dcd80c7c545eb65b903 COPYING3 +sha256 56bdea73b6145ef6ac5259b3da390b981d840c24cb03b8e1cbc678de7ecfa18d COPYING.LIB diff --git a/package/binutils-bare-metal/binutils-bare-metal.mk b/package/binutils-bare-metal/binutils-bare-metal.mk new file mode 100644 index 0000000000..fd983abc93 --- /dev/null +++ b/package/binutils-bare-metal/binutils-bare-metal.mk @@ -0,0 +1,56 @@ +################################################################################ +# +# binutils-bare-metal +# +################################################################################ + +HOST_BINUTILS_BARE_METAL_VERSION = $(call qstrip,$(BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_VERSION)) +ifeq ($(HOST_BINUTILS_BARE_METAL_VERSION),) +HOST_BINUTILS_BARE_METAL_VERSION = 2.39 +endif # BINUTILS_VERSION + +HOST_BINUTILS_BARE_METAL_SITE ?= $(BR2_GNU_MIRROR)/binutils +HOST_BINUTILS_BARE_METAL_SOURCE ?= binutils-$(HOST_BINUTILS_BARE_METAL_VERSION).tar.xz + +HOST_BINUTILS_BARE_METAL_LICENSE = GPL-3.0+, libiberty LGPL-2.1+ +HOST_BINUTILS_BARE_METAL_LICENSE_FILES = COPYING3 COPYING.LIB +HOST_BINUTILS_BARE_METAL_CPE_ID_VENDOR = gnu + +HOST_BINUTILS_BARE_METAL_DEPENDENCIES = host-zlib + +# if toolchain is for microblazeel-xilinx, apply Xilinx patch set +ifeq ($(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH),"microblazeel-xilinx") +HOST_BINUTILS_BARE_METAL_EXTRA_DOWNLOADS = https://github.com/Xilinx/meta-xilinx/archive/refs/tags/xlnx-rel-v2023.1.tar.gz + +define HOST_BINUTILS_BARE_METAL_EXTRACT_PATCHES + mkdir -p $(@D)/patches + tar -xf $(HOST_BINUTILS_BARE_METAL_DL_DIR)/xlnx-rel-v2023.1.tar.gz --strip-components=5 -C $(@D)/patches meta-xilinx-xlnx-rel-v2023.1/meta-microblaze/recipes-devtools/binutils/binutils +endef +HOST_BINUTILS_BARE_METAL_POST_EXTRACT_HOOKS += HOST_BINUTILS_BARE_METAL_EXTRACT_PATCHES + +define HOST_BINUTILS_BARE_METAL_APPLY_LOCAL_PATCHES + $(APPLY_PATCHES) $(@D) $(@D)/patches *.patch; +endef +HOST_BINUTILS_BARE_METAL_POST_PATCH_HOOKS += HOST_BINUTILS_BARE_METAL_APPLY_LOCAL_PATCHES +endif + +# Don't build documentation. It takes up extra space / build time, +# and sometimes needs specific makeinfo versions to work +HOST_BINUTILS_BARE_METAL_CONF_ENV += MAKEINFO=true +HOST_BINUTILS_BARE_METAL_MAKE_OPTS += MAKEINFO=true +HOST_BINUTILS_BARE_METAL_INSTALL_OPTS += MAKEINFO=true install + +HOST_BINUTILS_BARE_METAL_EXTRA_CONFIG_OPTIONS = $(call qstrip,$(BR2_PACKAGE_HOST_BINUTILS_BARE_METAL_EXTRA_CONFIG_OPTIONS)) + +HOST_BINUTILS_BARE_METAL_CONF_OPTS = \ + --target=$(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH)-elf \ + --disable-gprof \ + --disable-shared \ + --enable-lto \ + --enable-static \ + --disable-initfini-array \ + --disable-multilib \ + --disable-werror \ + $(HOST_BINUTILS_BARE_METAL_EXTRA_CONFIG_OPTIONS) + +$(eval $(host-autotools-package))