| Message ID | MRZP264MB17365A55C00DF63EF052F388FA39A@MRZP264MB1736.FRAP264.PROD.OUTLOOK.COM |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | Add support for SPL MMC load redundant U-Boot | expand |
Hi Ayrton, On 19.07.23 11:38, Ayrton Leyssens wrote: > From 99512830410551132e8f7577615b3af4f5f1c137 Mon Sep 17 00:00:00 2001 > From: Ayrton Leyssens <ayrton.leyssens@scioteq.com> > Date: Wed, 19 Jul 2023 11:27:19 +0200 > Subject: [PATCH Add support for SPL mmc load redundant U-Boot > > This patch adds support for loading a redundant / second U-Boot from a MMC. > In some cases you want to update U-Boot, to have a "selectable" bootloader and keep the "old" and "new" one. > In this patch we use the environment for a user-defined variable. > Maybe this could be extended to some other ways as well. > I find the selection based on an environment variable quite weak. A wrong value could lead to unbootable device. IMHO the selection should be done in a way similar as what we have for the environment itself, that means the information if a u-boot image is valid or not should be inside the image itself. For example this is true in case u-boot is packed inside a fitImage. A simple selection is not sufficient, there should be a mechanism like for the environmant to fallback to the other partition if the chosen is not working or at least, integrity check is broken. Best regards, Stefano > Signed-off-by: Ayrton Leyssens <ayrton.leyssens@scioteq.com> > --- > > common/spl/Kconfig | 29 +++++++++++++++++++++++++++++ > common/spl/spl_mmc.c | 10 ++++++++++ > 2 files changed, 39 insertions(+) > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > index bee231b583..2c0ffd4363 100644 > --- a/common/spl/Kconfig > +++ b/common/spl/Kconfig > @@ -824,6 +824,25 @@ config SYS_MMCSD_FS_BOOT > Enable MMC FS Boot mode. Partition is selected by option > SYS_MMCSD_FS_BOOT_PARTITION. > +config SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE > + bool "Use MMC Boot Partition Failsafe" > + depends on SYS_MMCSD_FS_BOOT > + depends on SPL_ENV_SUPPORT > + default false > + help > + Use a redundant partition to load U-Boot from. > + > +config SYS_MMCSD_FS_BOOT_PARTITION_SELECTOR > + string "MMC Boot Partition Selector" > + depends on SYS_MMCSD_FS_BOOT > + depends on SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE > + default "" > + help > + Environment variable in U-Boot to use for the partition selection > + to boot U-Boot from. > + A 1 selects the "normal" partition, any other value selects the > + redundant partition. > + > config SYS_MMCSD_FS_BOOT_PARTITION > int "MMC Boot Partition" > depends on SYS_MMCSD_FS_BOOT > @@ -833,6 +852,16 @@ config SYS_MMCSD_FS_BOOT_PARTITION > used in fs mode. > Use -1 as a special value to use the first bootable partition. > +config SYS_MMCSD_FS_BOOT_PARTITION_REDUNDANT > + int "MMC Boot Partition Redundant" > + depends on SYS_MMCSD_FS_BOOT > + depends on SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE > + default 2 > + help > + Redundant Partition on the MMC to load U-Boot from when the MMC is being > + used in fs mode. > + Use -1 as a special value to use the first bootable partition. > + > config SPL_MMC_TINY > bool "Tiny MMC framework in SPL" > depends on SPL_MMC > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c > index a665091b00..bf804fabcc 100644 > --- a/common/spl/spl_mmc.c > +++ b/common/spl/spl_mmc.c > @@ -10,6 +10,7 @@ > #include <log.h> > #include <part.h> > #include <spl.h> > +#include <env.h> > #include <linux/compiler.h> > #include <errno.h> > #include <asm/u-boot.h> > @@ -281,6 +282,7 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, > int err = -ENOSYS; > __maybe_unused int partition = CONFIG_SYS_MMCSD_FS_BOOT_PARTITION; > + __maybe_unused int env_partition_selector = 0; > #if CONFIG_SYS_MMCSD_FS_BOOT_PARTITION == -1 > { > @@ -303,7 +305,15 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, > } > } > #endif > +#ifdef CONFIG_SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE > + env_init(); > + env_relocate(); > + env_partition_selector = env_get_ulong(CONFIG_SYS_MMCSD_FS_BOOT_PARTITION_SELECTOR, 10, 0); > + > + if (env_partition_selector != 1) > + partition = CONFIG_SYS_MMCSD_FS_BOOT_PARTITION_REDUNDANT; > +#endif > #ifdef CONFIG_SPL_FS_FAT > if (!spl_start_uboot()) { > err = spl_load_image_fat_os(spl_image, bootdev, mmc_get_blk_desc(mmc), > -- > 2.25.1
Hi Stefano > -----Original Message----- > From: Stefano Babic <sbabic@denx.de> > Sent: Wednesday, July 19, 2023 11:57 AM > To: Ayrton Leyssens <ayrton.leyssens@scioteq.com>; u-boot@lists.denx.de > Subject: Re: [PATCH] Add support for SPL MMC load redundant U-Boot > > Hi Ayrton, > > On 19.07.23 11:38, Ayrton Leyssens wrote: > > From 99512830410551132e8f7577615b3af4f5f1c137 Mon Sep 17 00:00:00 > > 2001 > > From: Ayrton Leyssens <ayrton.leyssens@scioteq.com> > > Date: Wed, 19 Jul 2023 11:27:19 +0200 > > Subject: [PATCH Add support for SPL mmc load redundant U-Boot > > > > This patch adds support for loading a redundant / second U-Boot from a > MMC. > > In some cases you want to update U-Boot, to have a "selectable" > bootloader and keep the "old" and "new" one. > > In this patch we use the environment for a user-defined variable. > > Maybe this could be extended to some other ways as well. > > > > I find the selection based on an environment variable quite weak. A wrong > value could lead to unbootable device. IMHO the selection should be done in > a way similar as what we have for the environment itself, that means the > information if a u-boot image is valid or not should be inside the image itself. > For example this is true in case u-boot is packed inside a fitImage. Maybe my word choice is not that clear. I do not mean it as a redundant U-Boot in case the first choice fails. More as like, a selectable U-Boot. The U-Boot could be selected by the user within U-Boot, or a higher level OS by setting the environment variable. This in case of a complete redundant system (with exception of the MLO) in which the user makes sure a software update goes well. If the user verifies the U-Boot is written well, he will toggle the environment variable and as such boot the whole 'B' side of the software chain. Next update writes over the 'A' side and toggles to boot the 'A' side next. > > A simple selection is not sufficient, there should be a mechanism like for the > environmant to fallback to the other partition if the chosen is not working or > at least, integrity check is broken. I agree, for a complete redundant U-Boot, this is not sufficient. At least, this could be the first step to such system. > Best regards, > Stefano > > > Signed-off-by: Ayrton Leyssens <ayrton.leyssens@scioteq.com> > > --- > > > > common/spl/Kconfig | 29 +++++++++++++++++++++++++++++ > > common/spl/spl_mmc.c | 10 ++++++++++ > > 2 files changed, 39 insertions(+) > > > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig index > > bee231b583..2c0ffd4363 100644 > > --- a/common/spl/Kconfig > > +++ b/common/spl/Kconfig > > @@ -824,6 +824,25 @@ config SYS_MMCSD_FS_BOOT > > Enable MMC FS Boot mode. Partition is selected by option > > SYS_MMCSD_FS_BOOT_PARTITION. > > +config SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE > > + bool "Use MMC Boot Partition Failsafe" > > + depends on SYS_MMCSD_FS_BOOT > > + depends on SPL_ENV_SUPPORT > > + default false > > + help > > + Use a redundant partition to load U-Boot from. > > + > > +config SYS_MMCSD_FS_BOOT_PARTITION_SELECTOR > > + string "MMC Boot Partition Selector" > > + depends on SYS_MMCSD_FS_BOOT > > + depends on SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE > > + default "" > > + help > > + Environment variable in U-Boot to use for the partition selection > > + to boot U-Boot from. > > + A 1 selects the "normal" partition, any other value selects the > > + redundant partition. > > + > > config SYS_MMCSD_FS_BOOT_PARTITION > > int "MMC Boot Partition" > > depends on SYS_MMCSD_FS_BOOT @@ -833,6 +852,16 @@ > > config SYS_MMCSD_FS_BOOT_PARTITION > > used in fs mode. > > Use -1 as a special value to use the first bootable partition. > > +config SYS_MMCSD_FS_BOOT_PARTITION_REDUNDANT > > + int "MMC Boot Partition Redundant" > > + depends on SYS_MMCSD_FS_BOOT > > + depends on SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE > > + default 2 > > + help > > + Redundant Partition on the MMC to load U-Boot from when the > MMC is being > > + used in fs mode. > > + Use -1 as a special value to use the first bootable partition. > > + > > config SPL_MMC_TINY > > bool "Tiny MMC framework in SPL" > > depends on SPL_MMC > > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index > > a665091b00..bf804fabcc 100644 > > --- a/common/spl/spl_mmc.c > > +++ b/common/spl/spl_mmc.c > > @@ -10,6 +10,7 @@ > > #include <log.h> > > #include <part.h> > > #include <spl.h> > > +#include <env.h> > > #include <linux/compiler.h> > > #include <errno.h> > > #include <asm/u-boot.h> > > @@ -281,6 +282,7 @@ static int spl_mmc_do_fs_boot(struct > spl_image_info *spl_image, > > int err = -ENOSYS; > > __maybe_unused int partition = > > CONFIG_SYS_MMCSD_FS_BOOT_PARTITION; > > + __maybe_unused int env_partition_selector = 0; > > #if CONFIG_SYS_MMCSD_FS_BOOT_PARTITION == -1 > > { > > @@ -303,7 +305,15 @@ static int spl_mmc_do_fs_boot(struct > spl_image_info *spl_image, > > } > > } > > #endif > > +#ifdef CONFIG_SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE > > + env_init(); > > + env_relocate(); > > + env_partition_selector = > > +env_get_ulong(CONFIG_SYS_MMCSD_FS_BOOT_PARTITION_SELECTOR, > 10, 0); > > + > > + if (env_partition_selector != 1) > > + partition = > > +CONFIG_SYS_MMCSD_FS_BOOT_PARTITION_REDUNDANT; > > +#endif > > #ifdef CONFIG_SPL_FS_FAT > > if (!spl_start_uboot()) { > > err = spl_load_image_fat_os(spl_image, > > bootdev, mmc_get_blk_desc(mmc), > > -- > > 2.25.1 > > -- > ========================================================== > =========== > DENX Software Engineering GmbH, Managing Director: Erika Unter > HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > ========================================================== > ===========
Hi Ayrton, On 19.07.23 15:38, Ayrton Leyssens wrote: > Hi Stefano > >> -----Original Message----- >> From: Stefano Babic <sbabic@denx.de> >> Sent: Wednesday, July 19, 2023 11:57 AM >> To: Ayrton Leyssens <ayrton.leyssens@scioteq.com>; u-boot@lists.denx.de >> Subject: Re: [PATCH] Add support for SPL MMC load redundant U-Boot >> >> Hi Ayrton, >> >> On 19.07.23 11:38, Ayrton Leyssens wrote: >>> From 99512830410551132e8f7577615b3af4f5f1c137 Mon Sep 17 00:00:00 >>> 2001 >>> From: Ayrton Leyssens <ayrton.leyssens@scioteq.com> >>> Date: Wed, 19 Jul 2023 11:27:19 +0200 >>> Subject: [PATCH Add support for SPL mmc load redundant U-Boot >>> >>> This patch adds support for loading a redundant / second U-Boot from a >> MMC. >>> In some cases you want to update U-Boot, to have a "selectable" >> bootloader and keep the "old" and "new" one. >>> In this patch we use the environment for a user-defined variable. >>> Maybe this could be extended to some other ways as well. >>> >> >> I find the selection based on an environment variable quite weak. A wrong >> value could lead to unbootable device. IMHO the selection should be done in >> a way similar as what we have for the environment itself, that means the >> information if a u-boot image is valid or not should be inside the image itself. >> For example this is true in case u-boot is packed inside a fitImage. > > Maybe my word choice is not that clear. > I do not mean it as a redundant U-Boot in case the first choice fails. > More as like, a selectable U-Boot. > It was clear :-) > The U-Boot could be selected by the user within U-Boot, or a higher level OS by setting the environment variable. > This in case of a complete redundant system (with exception of the MLO) in which the user makes sure a software update goes well. > If the user verifies the U-Boot is written well, Rather experience learns that if an image was well written, it does not mean that all devices in fields will boot. On upper layers (kernel, rootfs,app) it is usual to have a fallback in case something is going wrong. In this case, it does not happen, and the usage of an environment can be misuesed. It is true, this is done for kernel and rootfs, too, but in those cases we still have the bootloader making a decision and running a fallback, if something is going wrong. The feature in the patch is already available if u-boot is written on the mmcblkbootX partition, and after writing successfully the new u-boot image, user space will toggle the hardware partitions via the extCSD register in the eMMC. Even in that case, there is no fallback. > he will toggle the environment variable and as such boot the whole 'B' side of the software chain. > Next update writes over the 'A' side and toggles to boot the 'A' side next. > >> >> A simple selection is not sufficient, there should be a mechanism like for the >> environmant to fallback to the other partition if the chosen is not working or >> at least, integrity check is broken. > > I agree, for a complete redundant U-Boot, this is not sufficient. > At least, this could be the first step to such system. My idea for a full redundant u-boot is quite different. If u-boot is put into a fit, SPL can check the integrity of image and via bootcounter in SPL can even switch to the old one if something is wrong, of course by enabling watchdog in SPL. This feature is in my opinion already available on eMMC by switching the hardware boot partitions. Best regards, Stefano > >> Best regards, >> Stefano >> >>> Signed-off-by: Ayrton Leyssens <ayrton.leyssens@scioteq.com> >>> --- >>> >>> common/spl/Kconfig | 29 +++++++++++++++++++++++++++++ >>> common/spl/spl_mmc.c | 10 ++++++++++ >>> 2 files changed, 39 insertions(+) >>> >>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig index >>> bee231b583..2c0ffd4363 100644 >>> --- a/common/spl/Kconfig >>> +++ b/common/spl/Kconfig >>> @@ -824,6 +824,25 @@ config SYS_MMCSD_FS_BOOT >>> Enable MMC FS Boot mode. Partition is selected by option >>> SYS_MMCSD_FS_BOOT_PARTITION. >>> +config SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE >>> + bool "Use MMC Boot Partition Failsafe" >>> + depends on SYS_MMCSD_FS_BOOT >>> + depends on SPL_ENV_SUPPORT >>> + default false >>> + help >>> + Use a redundant partition to load U-Boot from. >>> + >>> +config SYS_MMCSD_FS_BOOT_PARTITION_SELECTOR >>> + string "MMC Boot Partition Selector" >>> + depends on SYS_MMCSD_FS_BOOT >>> + depends on SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE >>> + default "" >>> + help >>> + Environment variable in U-Boot to use for the partition selection >>> + to boot U-Boot from. >>> + A 1 selects the "normal" partition, any other value selects the >>> + redundant partition. >>> + >>> config SYS_MMCSD_FS_BOOT_PARTITION >>> int "MMC Boot Partition" >>> depends on SYS_MMCSD_FS_BOOT @@ -833,6 +852,16 @@ >>> config SYS_MMCSD_FS_BOOT_PARTITION >>> used in fs mode. >>> Use -1 as a special value to use the first bootable partition. >>> +config SYS_MMCSD_FS_BOOT_PARTITION_REDUNDANT >>> + int "MMC Boot Partition Redundant" >>> + depends on SYS_MMCSD_FS_BOOT >>> + depends on SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE >>> + default 2 >>> + help >>> + Redundant Partition on the MMC to load U-Boot from when the >> MMC is being >>> + used in fs mode. >>> + Use -1 as a special value to use the first bootable partition. >>> + >>> config SPL_MMC_TINY >>> bool "Tiny MMC framework in SPL" >>> depends on SPL_MMC >>> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index >>> a665091b00..bf804fabcc 100644 >>> --- a/common/spl/spl_mmc.c >>> +++ b/common/spl/spl_mmc.c >>> @@ -10,6 +10,7 @@ >>> #include <log.h> >>> #include <part.h> >>> #include <spl.h> >>> +#include <env.h> >>> #include <linux/compiler.h> >>> #include <errno.h> >>> #include <asm/u-boot.h> >>> @@ -281,6 +282,7 @@ static int spl_mmc_do_fs_boot(struct >> spl_image_info *spl_image, >>> int err = -ENOSYS; >>> __maybe_unused int partition = >>> CONFIG_SYS_MMCSD_FS_BOOT_PARTITION; >>> + __maybe_unused int env_partition_selector = 0; >>> #if CONFIG_SYS_MMCSD_FS_BOOT_PARTITION == -1 >>> { >>> @@ -303,7 +305,15 @@ static int spl_mmc_do_fs_boot(struct >> spl_image_info *spl_image, >>> } >>> } >>> #endif >>> +#ifdef CONFIG_SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE >>> + env_init(); >>> + env_relocate(); >>> + env_partition_selector = >>> +env_get_ulong(CONFIG_SYS_MMCSD_FS_BOOT_PARTITION_SELECTOR, >> 10, 0); >>> + >>> + if (env_partition_selector != 1) >>> + partition = >>> +CONFIG_SYS_MMCSD_FS_BOOT_PARTITION_REDUNDANT; >>> +#endif >>> #ifdef CONFIG_SPL_FS_FAT >>> if (!spl_start_uboot()) { >>> err = spl_load_image_fat_os(spl_image, >>> bootdev, mmc_get_blk_desc(mmc), >>> -- >>> 2.25.1 >> >> -- >> ========================================================== >> =========== >> DENX Software Engineering GmbH, Managing Director: Erika Unter >> HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany >> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de >> ========================================================== >> =========== >
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index bee231b583..2c0ffd4363 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -824,6 +824,25 @@ config SYS_MMCSD_FS_BOOT Enable MMC FS Boot mode. Partition is selected by option SYS_MMCSD_FS_BOOT_PARTITION. +config SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE + bool "Use MMC Boot Partition Failsafe" + depends on SYS_MMCSD_FS_BOOT + depends on SPL_ENV_SUPPORT + default false + help + Use a redundant partition to load U-Boot from. + +config SYS_MMCSD_FS_BOOT_PARTITION_SELECTOR + string "MMC Boot Partition Selector" + depends on SYS_MMCSD_FS_BOOT + depends on SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE + default "" + help + Environment variable in U-Boot to use for the partition selection + to boot U-Boot from. + A 1 selects the "normal" partition, any other value selects the + redundant partition. + config SYS_MMCSD_FS_BOOT_PARTITION int "MMC Boot Partition" depends on SYS_MMCSD_FS_BOOT @@ -833,6 +852,16 @@ config SYS_MMCSD_FS_BOOT_PARTITION used in fs mode. Use -1 as a special value to use the first bootable partition. +config SYS_MMCSD_FS_BOOT_PARTITION_REDUNDANT + int "MMC Boot Partition Redundant" + depends on SYS_MMCSD_FS_BOOT + depends on SYS_MMCSD_FS_BOOT_PARTITION_FAILSAFE + default 2 + help + Redundant Partition on the MMC to load U-Boot from when the MMC is being + used in fs mode. + Use -1 as a special value to use the first bootable partition. + config SPL_MMC_TINY bool "Tiny MMC framework in SPL" depends on SPL_MMC diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index a665091b00..bf804fabcc 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -10,6 +10,7 @@ #include <log.h> #include <part.h> #include <spl.h> +#include <env.h> #include <linux/compiler.h> #include <errno.h> #include <asm/u-boot.h> @@ -281,6 +282,7 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, int err = -ENOSYS; __maybe_unused int partition = CONFIG_SYS_MMCSD_FS_BOOT_PARTITION; + __maybe_unused int env_partition_selector = 0; #if CONFIG_SYS_MMCSD_FS_BOOT_PARTITION == -1 { @@ -303,7 +305,15 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image, }