Re: [PATCH v3 4/9] board: Define set_dfu_alt_info() for boards with UEFI capsule update enabled

2022-03-31 Thread Sughosh Ganu
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

2022-03-30 Thread Masami Hiramatsu
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

2022-03-30 Thread 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 
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 @@