Re: [PATCH v3 4/9] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled
hi Masami, On Thu, 31 Mar 2022 at 08:15, Masami Hiramatsu wrote: > > Hi Sughosh, > > If you remove the DEFAULT_DFU_ALT_INFO definition but introduce > set_dfu_alt_info(), this also must the CONFIG_SET_DFU_ALT_INFO for > each platform configuration too. > Unless that, some platform will not lose the dfu_alt_info until next > patch ([5/9]) is applied. I believe all the platforms are setting dfu_alt_info in the environment primarily for the capsule update. So I will squash this patch with patch 5/9. If you or any other board maintainer gives feedback for adding CONFIG_SET_DFU_ALT_INFO in the board's defconfig file, I will make that change in my next version. Thanks. -sughosh > > Thank you, > > 2022年3月30日(水) 23:51 Sughosh Ganu : > > > > Currently, there are a bunch of boards which enable the UEFI capsule > > update feature. The actual update of the firmware images is done > > through the dfu framework which uses the dfu_alt_info environment > > variable for getting information on the update, like device, partition > > number/address etc. Currently, these boards define the dfu_alt_info > > variable in the board config header, as an environment variable. With > > this, the variable can be modified from the u-boot command line and > > this can cause an incorrect update. > > > > To prevent this from happening, define the set_dfu_alt_info function > > in the board file, and use the function for populating the > > variable. With the function defined, the dfu framework populates the > > dfu_alt_info variable through the board file, instead of fetching the > > variable from the environment, thus making the update more robust. > > > > Signed-off-by: Sughosh Ganu > > --- > > > > Changes since V2: New Patch > > > > > > .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 24 + > > .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 24 + > > board/emulation/common/qemu_dfu.c | 6 ++--- > > board/kontron/pitx_imx8m/pitx_imx8m.c | 24 + > > board/kontron/sl-mx8mm/sl-mx8mm.c | 24 + > > board/kontron/sl28/sl28.c | 25 ++ > > board/sandbox/sandbox.c | 26 +++ > > board/socionext/developerbox/developerbox.c | 26 +++ > > board/xilinx/zynq/board.c | 5 ++-- > > board/xilinx/zynqmp/zynqmp.c | 5 ++-- > > include/configs/imx8mm-cl-iot-gate.h | 1 - > > include/configs/imx8mp_rsb3720.h | 1 - > > include/configs/kontron-sl-mx8mm.h| 1 - > > include/configs/kontron_pitx_imx8m.h | 1 - > > include/configs/kontron_sl28.h| 2 -- > > include/configs/synquacer.h | 6 - > > 16 files changed, 182 insertions(+), 19 deletions(-) > > > > diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > > b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > > index 1c953ba195..41154ca9f3 100644 > > --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > > +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > > @@ -5,10 +5,12 @@ > > */ > > > > #include > > +#include > > #include > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -24,6 +26,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > DECLARE_GLOBAL_DATA_PTR; > > @@ -231,3 +234,24 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc > > *mmc) > > } > > } > > #endif /* CONFIG_SPL_MMC_SUPPORT */ > > + > > +#if defined(CONFIG_SET_DFU_ALT_INFO) > > + > > +#define DFU_ALT_BUF_LENSZ_1K > > + > > +void set_dfu_alt_info(char *interface, char *devstr) > > +{ > > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN); > > + > > + if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) && > > + env_get("dfu_alt_info")) > > + return; > > + > > + memset(buf, 0, DFU_ALT_BUF_LEN); > > + > > + snprintf(buf, DFU_ALT_BUF_LEN, > > +"mmc 2=flash-bin raw 0 0x1B00 mmcpart 1"); > > + > > + env_set("dfu_alt_info", buf); > > +} > > +#endif /* CONFIG_SET_DFU_ALT_INFO */ > > diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > > b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > > index f5b89a5ddc..1880dd9c55 100644 > > --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > > +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > > @@ -5,6 +5,7 @@ > > */ > > > > #include > > +#include > > #include > > #include > > #include > > @@ -12,6 +13,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > > > @@ -24,6 +26,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "ddr/ddr.h" > > > > @@ -446,3 +449,24 @@ int board_late_init(void) > > > > return 0; > > } > > + > >
Re: [PATCH v3 4/9] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled
Hi Sughosh, If you remove the DEFAULT_DFU_ALT_INFO definition but introduce set_dfu_alt_info(), this also must the CONFIG_SET_DFU_ALT_INFO for each platform configuration too. Unless that, some platform will not lose the dfu_alt_info until next patch ([5/9]) is applied. Thank you, 2022年3月30日(水) 23:51 Sughosh Ganu : > > Currently, there are a bunch of boards which enable the UEFI capsule > update feature. The actual update of the firmware images is done > through the dfu framework which uses the dfu_alt_info environment > variable for getting information on the update, like device, partition > number/address etc. Currently, these boards define the dfu_alt_info > variable in the board config header, as an environment variable. With > this, the variable can be modified from the u-boot command line and > this can cause an incorrect update. > > To prevent this from happening, define the set_dfu_alt_info function > in the board file, and use the function for populating the > variable. With the function defined, the dfu framework populates the > dfu_alt_info variable through the board file, instead of fetching the > variable from the environment, thus making the update more robust. > > Signed-off-by: Sughosh Ganu > --- > > Changes since V2: New Patch > > > .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 24 + > .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 24 + > board/emulation/common/qemu_dfu.c | 6 ++--- > board/kontron/pitx_imx8m/pitx_imx8m.c | 24 + > board/kontron/sl-mx8mm/sl-mx8mm.c | 24 + > board/kontron/sl28/sl28.c | 25 ++ > board/sandbox/sandbox.c | 26 +++ > board/socionext/developerbox/developerbox.c | 26 +++ > board/xilinx/zynq/board.c | 5 ++-- > board/xilinx/zynqmp/zynqmp.c | 5 ++-- > include/configs/imx8mm-cl-iot-gate.h | 1 - > include/configs/imx8mp_rsb3720.h | 1 - > include/configs/kontron-sl-mx8mm.h| 1 - > include/configs/kontron_pitx_imx8m.h | 1 - > include/configs/kontron_sl28.h| 2 -- > include/configs/synquacer.h | 6 - > 16 files changed, 182 insertions(+), 19 deletions(-) > > diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > index 1c953ba195..41154ca9f3 100644 > --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > @@ -5,10 +5,12 @@ > */ > > #include > +#include > #include > #include > #include > #include > +#include > #include > #include > #include > @@ -24,6 +26,7 @@ > #include > #include > #include > +#include > #include > > DECLARE_GLOBAL_DATA_PTR; > @@ -231,3 +234,24 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc > *mmc) > } > } > #endif /* CONFIG_SPL_MMC_SUPPORT */ > + > +#if defined(CONFIG_SET_DFU_ALT_INFO) > + > +#define DFU_ALT_BUF_LENSZ_1K > + > +void set_dfu_alt_info(char *interface, char *devstr) > +{ > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN); > + > + if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) && > + env_get("dfu_alt_info")) > + return; > + > + memset(buf, 0, DFU_ALT_BUF_LEN); > + > + snprintf(buf, DFU_ALT_BUF_LEN, > +"mmc 2=flash-bin raw 0 0x1B00 mmcpart 1"); > + > + env_set("dfu_alt_info", buf); > +} > +#endif /* CONFIG_SET_DFU_ALT_INFO */ > diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > index f5b89a5ddc..1880dd9c55 100644 > --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > @@ -5,6 +5,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -12,6 +13,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -24,6 +26,7 @@ > #include > #include > #include > +#include > > #include "ddr/ddr.h" > > @@ -446,3 +449,24 @@ int board_late_init(void) > > return 0; > } > + > +#if defined(CONFIG_SET_DFU_ALT_INFO) > + > +#define DFU_ALT_BUF_LENSZ_1K > + > +void set_dfu_alt_info(char *interface, char *devstr) > +{ > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN); > + > + if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) && > + env_get("dfu_alt_info")) > + return; > + > + memset(buf, 0, DFU_ALT_BUF_LEN); > + > + snprintf(buf, DFU_ALT_BUF_LEN, > +"mmc 2=flash-bin raw 0x42 0x1D00 mmcpart 1"); > + > + env_set("dfu_alt_info", buf); > +} > +#endif /* CONFIG_SET_DFU_ALT_INFO */ > diff --git a/board/emulation/common/qemu_dfu.c >
[PATCH v3 4/9] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled
Currently, there are a bunch of boards which enable the UEFI capsule update feature. The actual update of the firmware images is done through the dfu framework which uses the dfu_alt_info environment variable for getting information on the update, like device, partition number/address etc. Currently, these boards define the dfu_alt_info variable in the board config header, as an environment variable. With this, the variable can be modified from the u-boot command line and this can cause an incorrect update. To prevent this from happening, define the set_dfu_alt_info function in the board file, and use the function for populating the variable. With the function defined, the dfu framework populates the dfu_alt_info variable through the board file, instead of fetching the variable from the environment, thus making the update more robust. Signed-off-by: Sughosh Ganu --- Changes since V2: New Patch .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 24 + .../imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 24 + board/emulation/common/qemu_dfu.c | 6 ++--- board/kontron/pitx_imx8m/pitx_imx8m.c | 24 + board/kontron/sl-mx8mm/sl-mx8mm.c | 24 + board/kontron/sl28/sl28.c | 25 ++ board/sandbox/sandbox.c | 26 +++ board/socionext/developerbox/developerbox.c | 26 +++ board/xilinx/zynq/board.c | 5 ++-- board/xilinx/zynqmp/zynqmp.c | 5 ++-- include/configs/imx8mm-cl-iot-gate.h | 1 - include/configs/imx8mp_rsb3720.h | 1 - include/configs/kontron-sl-mx8mm.h| 1 - include/configs/kontron_pitx_imx8m.h | 1 - include/configs/kontron_sl28.h| 2 -- include/configs/synquacer.h | 6 - 16 files changed, 182 insertions(+), 19 deletions(-) diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c index 1c953ba195..41154ca9f3 100644 --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c @@ -5,10 +5,12 @@ */ #include +#include #include #include #include #include +#include #include #include #include @@ -24,6 +26,7 @@ #include #include #include +#include #include DECLARE_GLOBAL_DATA_PTR; @@ -231,3 +234,24 @@ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc) } } #endif /* CONFIG_SPL_MMC_SUPPORT */ + +#if defined(CONFIG_SET_DFU_ALT_INFO) + +#define DFU_ALT_BUF_LENSZ_1K + +void set_dfu_alt_info(char *interface, char *devstr) +{ + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN); + + if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) && + env_get("dfu_alt_info")) + return; + + memset(buf, 0, DFU_ALT_BUF_LEN); + + snprintf(buf, DFU_ALT_BUF_LEN, +"mmc 2=flash-bin raw 0 0x1B00 mmcpart 1"); + + env_set("dfu_alt_info", buf); +} +#endif /* CONFIG_SET_DFU_ALT_INFO */ diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c index f5b89a5ddc..1880dd9c55 100644 --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -12,6 +13,7 @@ #include #include #include +#include #include #include @@ -24,6 +26,7 @@ #include #include #include +#include #include "ddr/ddr.h" @@ -446,3 +449,24 @@ int board_late_init(void) return 0; } + +#if defined(CONFIG_SET_DFU_ALT_INFO) + +#define DFU_ALT_BUF_LENSZ_1K + +void set_dfu_alt_info(char *interface, char *devstr) +{ + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN); + + if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) && + env_get("dfu_alt_info")) + return; + + memset(buf, 0, DFU_ALT_BUF_LEN); + + snprintf(buf, DFU_ALT_BUF_LEN, +"mmc 2=flash-bin raw 0x42 0x1D00 mmcpart 1"); + + env_set("dfu_alt_info", buf); +} +#endif /* CONFIG_SET_DFU_ALT_INFO */ diff --git a/board/emulation/common/qemu_dfu.c b/board/emulation/common/qemu_dfu.c index 62234a7647..85dff4373b 100644 --- a/board/emulation/common/qemu_dfu.c +++ b/board/emulation/common/qemu_dfu.c @@ -44,10 +44,11 @@ void set_dfu_alt_info(char *interface, char *devstr) ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN); - if (env_get("dfu_alt_info")) + if (!CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT) && + env_get("dfu_alt_info")) return; - memset(buf, 0, sizeof(buf)); + memset(buf, 0, DFU_ALT_BUF_LEN); /* * Currently dfu_alt_info is needed on Qemu ARM64 for @@ -64,5 +65,4 @@