[Buildroot] [PATCH v2 1/3] boot/xilinx-prebuilt: add segmented config support

[Buildroot] [PATCH v2 1/3] boot/xilinx-prebuilt: add segmented config support

[Neal Frager via buildroot]

AMD has created a new segmented configuration for Versal products.  It splits
the Vivado hardware design into two PDI files, one containing the minimal
required configuration for the DDR and booting the processors, and a second PDI
file which contains the rest of the FPGA design and can be loaded at run-time
via U-boot or Linux.

The file names generated when using the Vivado Segmented Configuration are as
follows:

<design>_boot.pdi - Minimal DDR and PS config for booting
<design>_pld.pdi - Remainder of FPGA design to be loaded at run-time

Since two PDI files will be included in the XSA file when using Segmented
Configuration, the xilinx-prebuilt package needs to be updated to support
this feature.

For Buildroot purposes, the <design>_boot.pdi is the file that needs to be
included in the boot.bin for booting the processors, so this patch checks
for a file named *boot*.pdi which will indicate that Segmented Configuration
is being used and will make sure to use the <design>_boot.pdi file and not
the <design>_pld.pdi file when generating the boot.bin image.

If no pdi files contain the word "boot" in the filename, it can be assumed
that Segmented Configuration is not being used, so the single file *.pdi
method is the appropriate one for this case, and this patch is thus
backwards compatible with prior designs not using Segmented Configuration.

Also, Segmented Configuration is going to become the default mode for Versal
products, so the xilinx-prebuilt github location will also soon have two PDI
files for each board.  For this reason, this patch is also handling Segmented
Configuration for files downloaded from the xilinx-prebuilt repo.

For further information about the AMD Segmented Configuration, please see the
github tutorial below.

https://github.com/Xilinx/Vivado-Design-Tutorials/tree/2024.2/Versal/Boot_and_Config/Segmented_Configuration

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
V1->V2:
- $(@D) is not available in time for wildcard evaluation outside of shell
  script, so moved the wildcard functions into the install scripts
---
 boot/xilinx-prebuilt/xilinx-prebuilt.mk | 36 ++++++++++++++++++-------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/boot/xilinx-prebuilt/xilinx-prebuilt.mk b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
index d0817d3cc7..448c71a3fb 100644
--- a/boot/xilinx-prebuilt/xilinx-prebuilt.mk
+++ b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
@@ -28,9 +28,6 @@ XILINX_PREBUILT_BOARD = $(call qstrip,$(BR2_TARGET_XILINX_PREBUILT_BOARD))
 XILINX_PREBUILT_BOARD_DIR = $(@D)/$(XILINX_PREBUILT_BOARD)-$(XILINX_PREBUILT_FAMILY)
 
 ifeq ($(BR2_TARGET_XILINX_PREBUILT_VERSAL),y)
-# We need the *.pdi glob, because the file has different names for the
-# different boards, but there is only one, and it has to be named
-# vpl_gen_fixed.pdi when installed.
 ifeq ($(BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA),y)
 XILINX_PREBUILT_PLM = $(@D)/pdi_files/gen_files/plm.elf
 # Unlike the psmfw.elf file for Xilinx development boards,
@@ -39,11 +36,33 @@ XILINX_PREBUILT_PLM = $(@D)/pdi_files/gen_files/plm.elf
 # so to support current and future AMD Vivado versions, the filename
 # psm*fw.elf is used.
 XILINX_PREBUILT_PSMFW = $(@D)/pdi_files/static_files/psm*fw.elf
-XILINX_PREBUILT_PDI = $(@D)/*.pdi
+# We need the *.pdi glob, because the file has different names for the
+# different boards, and it has to be named vpl_gen_fixed.pdi when installed.
+# If Segmented Configuration is used, there will be two pdi files and we need
+# the file that has "boot" in the filename.
+define XILINX_PREBUILT_INSTALL_VERSAL_XSA_BOOT_PDI
+	$(if $(wildcard $(@D)/*boot*.pdi),
+		$(INSTALL) -D -m 0755 $(@D)/*boot*.pdi \
+			$(BINARIES_DIR)/vpl_gen_fixed.pdi,
+		$(INSTALL) -D -m 0755 $(@D)/*.pdi \
+			$(BINARIES_DIR)/vpl_gen_fixed.pdi
+	)
+endef
 else # BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA
 XILINX_PREBUILT_PLM = $(XILINX_PREBUILT_BOARD_DIR)/plm.elf
 XILINX_PREBUILT_PSMFW = $(XILINX_PREBUILT_BOARD_DIR)/psmfw.elf
-XILINX_PREBUILT_PDI = $(XILINX_PREBUILT_BOARD_DIR)/*.pdi
+# We need the *.pdi glob, because the file has different names for the
+# different boards, and it has to be named vpl_gen_fixed.pdi when installed.
+# If Segmented Configuration is used, there will be two pdi files and we need
+# the file that has "boot" in the filename.
+define XILINX_PREBUILT_INSTALL_VERSAL_BOOT_PDI
+	$(if $(wildcard $(XILINX_PREBUILT_BOARD_DIR)/*boot*.pdi),
+		$(INSTALL) -D -m 0755 $(XILINX_PREBUILT_BOARD_DIR)/*boot*.pdi \
+			$(BINARIES_DIR)/vpl_gen_fixed.pdi,
+		$(INSTALL) -D -m 0755 $(XILINX_PREBUILT_BOARD_DIR)/*.pdi \
+			$(BINARIES_DIR)/vpl_gen_fixed.pdi
+	)
+endef
 endif # BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA
 
 ifneq ($(BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PLM),y)
@@ -60,10 +79,6 @@ define XILINX_PREBUILT_INSTALL_VERSAL_PSMFW
 endef
 endif # !BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PSMFW
 
-define XILINX_PREBUILT_INSTALL_VERSAL_PDI
-	$(INSTALL) -D -m 0755 $(XILINX_PREBUILT_PDI) \
-		$(BINARIES_DIR)/vpl_gen_fixed.pdi
-endef
 else # BR2_TARGET_XILINX_PREBUILT_VERSAL
 ifneq ($(BR2_TARGET_XILINX_EMBEDDEDSW_ZYNQMP_PMUFW),y)
 define XILINX_PREBUILT_INSTALL_ZYNQMP_PMUFW
@@ -76,7 +91,8 @@ endif # BR2_TARGET_XILINX_PREBUILT_VERSAL
 define XILINX_PREBUILT_INSTALL_IMAGES_CMDS
 	$(XILINX_PREBUILT_INSTALL_VERSAL_PLM)
 	$(XILINX_PREBUILT_INSTALL_VERSAL_PSMFW)
-	$(XILINX_PREBUILT_INSTALL_VERSAL_PDI)
+	$(XILINX_PREBUILT_INSTALL_VERSAL_BOOT_PDI)
+	$(XILINX_PREBUILT_INSTALL_VERSAL_XSA_BOOT_PDI)
 	$(XILINX_PREBUILT_INSTALL_ZYNQMP_PMUFW)
 endef
 
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 2/3] board/versal: change pdi filename to boot.pdi

[Neal Frager via buildroot]

With the Segmented Configuration, the Versal PDI files will have new names:
<design>_boot.pdi - Minimal DDR and PS config for booting
<design>_pld.pdi - Remainder of FPGA design to be loaded at run-time

Since this will be the new default, this patch aligns the Buildroot file
naming by replacing the obscure vpl_gen_fixed.pdi filename with a simpler
boot.pdi filename.

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
V1->V2:
- rebased due to changes in first patch of series
---
 board/versal/post-image.sh              |  2 +-
 boot/xilinx-prebuilt/xilinx-prebuilt.mk | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/board/versal/post-image.sh b/board/versal/post-image.sh
index 22367d5028..b5ce2e3f0c 100755
--- a/board/versal/post-image.sh
+++ b/board/versal/post-image.sh
@@ -17,7 +17,7 @@ cat <<-__HEADER_EOF > "${BINARIES_DIR}/bootgen.bif"
 	the_ROM_image:
 	{
 	  image {
-	    { type=bootimage, file=${BINARIES_DIR}/vpl_gen_fixed.pdi }
+	    { type=bootimage, file=${BINARIES_DIR}/boot.pdi }
 	    { type=bootloader, file=${BINARIES_DIR}/plm.elf }
 	    { core=psm, file=${BINARIES_DIR}/psmfw.elf }
 	  }
diff --git a/boot/xilinx-prebuilt/xilinx-prebuilt.mk b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
index 448c71a3fb..e6817f4bd7 100644
--- a/boot/xilinx-prebuilt/xilinx-prebuilt.mk
+++ b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
@@ -37,30 +37,30 @@ XILINX_PREBUILT_PLM = $(@D)/pdi_files/gen_files/plm.elf
 # psm*fw.elf is used.
 XILINX_PREBUILT_PSMFW = $(@D)/pdi_files/static_files/psm*fw.elf
 # We need the *.pdi glob, because the file has different names for the
-# different boards, and it has to be named vpl_gen_fixed.pdi when installed.
+# different boards, and it has to be named boot.pdi when installed.
 # If Segmented Configuration is used, there will be two pdi files and we need
 # the file that has "boot" in the filename.
 define XILINX_PREBUILT_INSTALL_VERSAL_XSA_BOOT_PDI
 	$(if $(wildcard $(@D)/*boot*.pdi),
 		$(INSTALL) -D -m 0755 $(@D)/*boot*.pdi \
-			$(BINARIES_DIR)/vpl_gen_fixed.pdi,
+			$(BINARIES_DIR)/boot.pdi,
 		$(INSTALL) -D -m 0755 $(@D)/*.pdi \
-			$(BINARIES_DIR)/vpl_gen_fixed.pdi
+			$(BINARIES_DIR)/boot.pdi
 	)
 endef
 else # BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA
 XILINX_PREBUILT_PLM = $(XILINX_PREBUILT_BOARD_DIR)/plm.elf
 XILINX_PREBUILT_PSMFW = $(XILINX_PREBUILT_BOARD_DIR)/psmfw.elf
 # We need the *.pdi glob, because the file has different names for the
-# different boards, and it has to be named vpl_gen_fixed.pdi when installed.
+# different boards, and it has to be named boot.pdi when installed.
 # If Segmented Configuration is used, there will be two pdi files and we need
 # the file that has "boot" in the filename.
 define XILINX_PREBUILT_INSTALL_VERSAL_BOOT_PDI
 	$(if $(wildcard $(XILINX_PREBUILT_BOARD_DIR)/*boot*.pdi),
 		$(INSTALL) -D -m 0755 $(XILINX_PREBUILT_BOARD_DIR)/*boot*.pdi \
-			$(BINARIES_DIR)/vpl_gen_fixed.pdi,
+			$(BINARIES_DIR)/boot.pdi,
 		$(INSTALL) -D -m 0755 $(XILINX_PREBUILT_BOARD_DIR)/*.pdi \
-			$(BINARIES_DIR)/vpl_gen_fixed.pdi
+			$(BINARIES_DIR)/boot.pdi
 	)
 endef
 endif # BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 3/3] boot/xilinx-prebuilt: install pld.pdi to target

[Neal Frager via buildroot]

With the Versal Segmented Configuration, it is possible to load a PL bitstream
at run-time using the Linux fpgautil application.  Whenever Segmented
Configuration is used, both the boot.pdi and pld.pdi files will be available
to Buildroot.  For this reason, this patch improves the user experience by
installing the pld.pdi file to the target file system for run-time loading
whenever Versal Segemented Configuration is used.

Please note that the fpgautil application is required for loading the pld.pdi
file.  The source code for this application can be found below.  Users are
required to build it and include it in the target file system for loading the
pld.pdi file and thus programming the FPGA.

https://github.com/Xilinx/meta-xilinx/blob/master/meta-xilinx-core/recipes-bsp/fpga-manager-script/files/fpgautil.c

Signed-off-by: Neal Frager <neal.frager@amd.com>
---
V1->V2:
- new to patch series
---
 boot/xilinx-prebuilt/xilinx-prebuilt.mk | 28 ++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/boot/xilinx-prebuilt/xilinx-prebuilt.mk b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
index e6817f4bd7..212247d331 100644
--- a/boot/xilinx-prebuilt/xilinx-prebuilt.mk
+++ b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
@@ -19,7 +19,7 @@ XILINX_PREBUILT_LICENSE = MIT
 XILINX_PREBUILT_LICENSE_FILES = LICENSE
 endif # BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA
 
-XILINX_PREBUILT_INSTALL_TARGET = NO
+XILINX_PREBUILT_INSTALL_TARGET = YES
 XILINX_PREBUILT_INSTALL_IMAGES = YES
 
 XILINX_PREBUILT_FAMILY = $(call qstrip,$(BR2_TARGET_XILINX_PREBUILT_FAMILY))
@@ -48,6 +48,17 @@ define XILINX_PREBUILT_INSTALL_VERSAL_XSA_BOOT_PDI
 			$(BINARIES_DIR)/boot.pdi
 	)
 endef
+
+# Install pld.pdi in target file system for run-time loading when using
+# Segmented Configuration.
+define XILINX_PREBUILT_INSTALL_VERSAL_XSA_PLD_PDI
+	$(if $(wildcard $(@D)/*boot*.pdi),
+		mkdir -p $(TARGET_DIR)/lib/firmware && \
+		$(INSTALL) -D -m 0755 $(@D)/*pld*.pdi \
+			$(TARGET_DIR)/lib/firmware
+	)
+endef
+
 else # BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA
 XILINX_PREBUILT_PLM = $(XILINX_PREBUILT_BOARD_DIR)/plm.elf
 XILINX_PREBUILT_PSMFW = $(XILINX_PREBUILT_BOARD_DIR)/psmfw.elf
@@ -63,6 +74,16 @@ define XILINX_PREBUILT_INSTALL_VERSAL_BOOT_PDI
 			$(BINARIES_DIR)/boot.pdi
 	)
 endef
+
+# Install pld.pdi in target file system for run-time loading when using
+# Segmented Configuration.
+define XILINX_PREBUILT_INSTALL_VERSAL_PLD_PDI
+	$(if $(wildcard $(XILINX_PREBUILT_BOARD_DIR)/*boot*.pdi),
+		mkdir -p $(TARGET_DIR)/lib/firmware && \
+		$(INSTALL) -D -m 0755 $(XILINX_PREBUILT_BOARD_DIR)/*pld*.pdi \
+			$(TARGET_DIR)/lib/firmware
+	)
+endef
 endif # BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA
 
 ifneq ($(BR2_TARGET_XILINX_EMBEDDEDSW_VERSAL_PLM),y)
@@ -96,4 +117,9 @@ define XILINX_PREBUILT_INSTALL_IMAGES_CMDS
 	$(XILINX_PREBUILT_INSTALL_ZYNQMP_PMUFW)
 endef
 
+define XILINX_PREBUILT_INSTALL_TARGET_CMDS
+	$(XILINX_PREBUILT_INSTALL_VERSAL_PLD_PDI)
+	$(XILINX_PREBUILT_INSTALL_VERSAL_XSA_PLD_PDI)
+endef
+
 $(eval $(generic-package))
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/3] boot/xilinx-prebuilt: add segmented config support

[Luca Ceresoli via buildroot]

Hello Neal,

On Mon, 7 Apr 2025 16:29:09 +0100
Neal Frager <neal.frager@amd.com> wrote:

> AMD has created a new segmented configuration for Versal products.  It splits
> the Vivado hardware design into two PDI files, one containing the minimal
> required configuration for the DDR and booting the processors, and a second PDI
> file which contains the rest of the FPGA design and can be loaded at run-time
> via U-boot or Linux.
> 
> The file names generated when using the Vivado Segmented Configuration are as
> follows:
> 
> <design>_boot.pdi - Minimal DDR and PS config for booting
> <design>_pld.pdi - Remainder of FPGA design to be loaded at run-time
> 
> Since two PDI files will be included in the XSA file when using Segmented
> Configuration, the xilinx-prebuilt package needs to be updated to support
> this feature.
> 
> For Buildroot purposes, the <design>_boot.pdi is the file that needs to be
> included in the boot.bin for booting the processors, so this patch checks
> for a file named *boot*.pdi which will indicate that Segmented Configuration
> is being used and will make sure to use the <design>_boot.pdi file and not
> the <design>_pld.pdi file when generating the boot.bin image.
> 
> If no pdi files contain the word "boot" in the filename, it can be assumed
> that Segmented Configuration is not being used, so the single file *.pdi
> method is the appropriate one for this case, and this patch is thus
> backwards compatible with prior designs not using Segmented Configuration.
> 
> Also, Segmented Configuration is going to become the default mode for Versal
> products, so the xilinx-prebuilt github location will also soon have two PDI
> files for each board.  For this reason, this patch is also handling Segmented
> Configuration for files downloaded from the xilinx-prebuilt repo.
> 
> For further information about the AMD Segmented Configuration, please see the
> github tutorial below.
> 
> https://github.com/Xilinx/Vivado-Design-Tutorials/tree/2024.2/Versal/Boot_and_Config/Segmented_Configuration
> 
> Signed-off-by: Neal Frager <neal.frager@amd.com>

Thank you for the very good explanation, it's very helpful!

> ---
> V1->V2:
> - $(@D) is not available in time for wildcard evaluation outside of shell
>   script, so moved the wildcard functions into the install scripts
> ---
>  boot/xilinx-prebuilt/xilinx-prebuilt.mk | 36 ++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/boot/xilinx-prebuilt/xilinx-prebuilt.mk b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
> index d0817d3cc7..448c71a3fb 100644
> --- a/boot/xilinx-prebuilt/xilinx-prebuilt.mk
> +++ b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
> @@ -28,9 +28,6 @@ XILINX_PREBUILT_BOARD = $(call qstrip,$(BR2_TARGET_XILINX_PREBUILT_BOARD))
>  XILINX_PREBUILT_BOARD_DIR = $(@D)/$(XILINX_PREBUILT_BOARD)-$(XILINX_PREBUILT_FAMILY)
>  
>  ifeq ($(BR2_TARGET_XILINX_PREBUILT_VERSAL),y)
> -# We need the *.pdi glob, because the file has different names for the
> -# different boards, but there is only one, and it has to be named
> -# vpl_gen_fixed.pdi when installed.
>  ifeq ($(BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA),y)
>  XILINX_PREBUILT_PLM = $(@D)/pdi_files/gen_files/plm.elf
>  # Unlike the psmfw.elf file for Xilinx development boards,
> @@ -39,11 +36,33 @@ XILINX_PREBUILT_PLM = $(@D)/pdi_files/gen_files/plm.elf
>  # so to support current and future AMD Vivado versions, the filename
>  # psm*fw.elf is used.
>  XILINX_PREBUILT_PSMFW = $(@D)/pdi_files/static_files/psm*fw.elf
> -XILINX_PREBUILT_PDI = $(@D)/*.pdi
> +# We need the *.pdi glob, because the file has different names for the
> +# different boards, and it has to be named vpl_gen_fixed.pdi when installed.
> +# If Segmented Configuration is used, there will be two pdi files and we need
> +# the file that has "boot" in the filename.
> +define XILINX_PREBUILT_INSTALL_VERSAL_XSA_BOOT_PDI
> +	$(if $(wildcard $(@D)/*boot*.pdi),

According to your commit message, a more specific wildcard can be used:
"$(@D)/*_boot.pdi".

> +		$(INSTALL) -D -m 0755 $(@D)/*boot*.pdi \
> +			$(BINARIES_DIR)/vpl_gen_fixed.pdi,
> +		$(INSTALL) -D -m 0755 $(@D)/*.pdi \
> +			$(BINARIES_DIR)/vpl_gen_fixed.pdi
> +	)

It's very unfortunate that you are adding all this code, and in two
places (here and below). The code in v1 was looking a lot nicer, but
I'm not sure there is a way to make it work. If there is none, I think
you can at least improve by using the $(if) only for the filename, not
the whole command, as in this example (untested):

define XILINX_PREBUILT_INSTALL_VERSAL_XSA_BOOT_PDI
	$(INSTALL) -D -m 0755 \
		$(if $(wildcard $(@D)/*_boot.pdi), $(@D)/*_boot.pdi, $(@D)/*.pdi) \
		$(BINARIES_DIR)/vpl_gen_fixed.pdi
#endef

Also, do you need 0755 permissions for this file? Is that an
executable? If it isn't, that should be fixed as a preliminary patch.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/3] boot/xilinx-prebuilt: add segmented config support

[Frager, Neal via buildroot]

[AMD Official Use Only - AMD Internal Distribution Only]

Hello Luca,

> AMD has created a new segmented configuration for Versal products.  It splits
> the Vivado hardware design into two PDI files, one containing the minimal
> required configuration for the DDR and booting the processors, and a second PDI
> file which contains the rest of the FPGA design and can be loaded at run-time
> via U-boot or Linux.
>
> The file names generated when using the Vivado Segmented Configuration are as
> follows:
>
> <design>_boot.pdi - Minimal DDR and PS config for booting
> <design>_pld.pdi - Remainder of FPGA design to be loaded at run-time
>
> Since two PDI files will be included in the XSA file when using Segmented
> Configuration, the xilinx-prebuilt package needs to be updated to support
> this feature.
>
> For Buildroot purposes, the <design>_boot.pdi is the file that needs to be
> included in the boot.bin for booting the processors, so this patch checks
> for a file named *boot*.pdi which will indicate that Segmented Configuration
> is being used and will make sure to use the <design>_boot.pdi file and not
> the <design>_pld.pdi file when generating the boot.bin image.
>
> If no pdi files contain the word "boot" in the filename, it can be assumed
> that Segmented Configuration is not being used, so the single file *.pdi
> method is the appropriate one for this case, and this patch is thus
> backwards compatible with prior designs not using Segmented Configuration.
>
> Also, Segmented Configuration is going to become the default mode for Versal
> products, so the xilinx-prebuilt github location will also soon have two PDI
> files for each board.  For this reason, this patch is also handling Segmented
> Configuration for files downloaded from the xilinx-prebuilt repo.
>
> For further information about the AMD Segmented Configuration, please see the
> github tutorial below.
>
> https://github.com/Xilinx/Vivado-Design-Tutorials/tree/2024.2/Versal/Boot_and_Config/Segmented_Configuration
>
> Signed-off-by: Neal Frager <neal.frager@amd.com>

> Thank you for the very good explanation, it's very helpful!

And thank you (as always) for reviewing this patch set!

> ---
> V1->V2:
> - $(@D) is not available in time for wildcard evaluation outside of shell
>   script, so moved the wildcard functions into the install scripts
> ---
>  boot/xilinx-prebuilt/xilinx-prebuilt.mk | 36 ++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/boot/xilinx-prebuilt/xilinx-prebuilt.mk b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
> index d0817d3cc7..448c71a3fb 100644
> --- a/boot/xilinx-prebuilt/xilinx-prebuilt.mk
> +++ b/boot/xilinx-prebuilt/xilinx-prebuilt.mk
> @@ -28,9 +28,6 @@ XILINX_PREBUILT_BOARD = $(call qstrip,$(BR2_TARGET_XILINX_PREBUILT_BOARD))
>  XILINX_PREBUILT_BOARD_DIR = $(@D)/$(XILINX_PREBUILT_BOARD)-$(XILINX_PREBUILT_FAMILY)
>
>  ifeq ($(BR2_TARGET_XILINX_PREBUILT_VERSAL),y)
> -# We need the *.pdi glob, because the file has different names for the
> -# different boards, but there is only one, and it has to be named
> -# vpl_gen_fixed.pdi when installed.
>  ifeq ($(BR2_TARGET_XILINX_PREBUILT_VERSAL_XSA),y)
>  XILINX_PREBUILT_PLM = $(@D)/pdi_files/gen_files/plm.elf
>  # Unlike the psmfw.elf file for Xilinx development boards,
> @@ -39,11 +36,33 @@ XILINX_PREBUILT_PLM = $(@D)/pdi_files/gen_files/plm.elf
>  # so to support current and future AMD Vivado versions, the filename
>  # psm*fw.elf is used.
>  XILINX_PREBUILT_PSMFW = $(@D)/pdi_files/static_files/psm*fw.elf
> -XILINX_PREBUILT_PDI = $(@D)/*.pdi
> +# We need the *.pdi glob, because the file has different names for the
> +# different boards, and it has to be named vpl_gen_fixed.pdi when installed.
> +# If Segmented Configuration is used, there will be two pdi files and we need
> +# the file that has "boot" in the filename.
> +define XILINX_PREBUILT_INSTALL_VERSAL_XSA_BOOT_PDI
> +     $(if $(wildcard $(@D)/*boot*.pdi),

> According to your commit message, a more specific wildcard can be used:
> "$(@D)/*_boot.pdi".

Yes, you are correct.  It is probably best to be as specific as possible.

It will help with corner cases when users have the word "boot" in their Vivado
project design name.  With my current solution, if someone had the word "boot"
in their Vivado design name while using Segmented Configuration, both pdi
files would have the "*boot*.pdi" wild card, so the install command would
fail when trying to copy two files into a single file.

Your solution protects for this, leaving the only corner case being a non
Segmented Configuration case with "_boot" as the end of the Vivado project
design name.  This should not be an issue because there would only be one
pdi file in this case, and it would be the one we want to include in the
boot.bin.

> +             $(INSTALL) -D -m 0755 $(@D)/*boot*.pdi \
> +                     $(BINARIES_DIR)/vpl_gen_fixed.pdi,
> +             $(INSTALL) -D -m 0755 $(@D)/*.pdi \
> +                     $(BINARIES_DIR)/vpl_gen_fixed.pdi
> +     )

> It's very unfortunate that you are adding all this code, and in two
> places (here and below). The code in v1 was looking a lot nicer, but
> I'm not sure there is a way to make it work. If there is none, I think
> you can at least improve by using the $(if) only for the filename, not
> the whole command, as in this example (untested):

Yes, I was getting a false success with v1, but I caught the issue with the
$(@D) variable after doing more testing.  I agree having multiple install
shells is definitely not ideal, but I haven't thought of a better way yet.

> define XILINX_PREBUILT_INSTALL_VERSAL_XSA_BOOT_PDI
>       $(INSTALL) -D -m 0755 \
>               $(if $(wildcard $(@D)/*_boot.pdi), $(@D)/*_boot.pdi, $(@D)/*.pdi) \
>               $(BINARIES_DIR)/vpl_gen_fixed.pdi
> #endef

This is very good!  I will definitely adopt it.

> Also, do you need 0755 permissions for this file? Is that an
> executable? If it isn't, that should be fixed as a preliminary patch.

Good catch.  PDI files are indeed not ARM executable files.  This has been
an issue for a while as the current solution on master uses 0755 for the pdi.

I will make sure to fix it with v3.

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