Hi Sughosh, Some nots below
On Thu, Mar 31, 2022 at 06:57:46PM +0530, Sughosh Ganu wrote: > 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 select SET_DFU_ALT_INFO for all platforms which > enable the capsule update feature. 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 <[email protected]> > --- > > Changes since V3: > * Do not remove the existing dfu_alt_info definitions made by > platforms in the config files, as discussed with Masami. > * Squash the selection of the SET_DFU_ALT_INFO config symbol for > capsule update feature as part of this 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 ++-- > lib/efi_loader/Kconfig | 2 ++ > 11 files changed, 184 insertions(+), 7 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 <common.h> > +#include <dfu.h> > #include <dwc3-uboot.h> > #include <efi.h> > #include <efi_loader.h> > #include <errno.h> > +#include <memalign.h> > #include <miiphy.h> > #include <netdev.h> > #include <spl.h> > @@ -24,6 +26,7 @@ > #include <asm/mach-imx/dma.h> > #include <linux/delay.h> > #include <linux/kernel.h> > +#include <linux/sizes.h> > #include <power/pmic.h> > > 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_LEN SZ_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; Just add a helper function with this since we need to repeat it for every board. Something like 'bool needs_runtime_dfu_alt_info() ' > + > + memset(buf, 0, DFU_ALT_BUF_LEN); I'd prefer sizeof(buf) instead of explicitly calling the length. Otherwise LGTM, but I'd prefer if board maintainer had a look as well Thanks /Ilias > + > + 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 */ [...]
