[PATCH v2 1/1] spl: allow loading via partition type GUID
Simon Glass
sjg at chromium.org
Tue Feb 21 20:41:45 CET 2023
Hi Heinrich,
On Thu, 16 Feb 2023 at 23:56, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
>
>
> On 2/17/23 03:55, Simon Glass wrote:
> > " properHi Heinrich,
> >
> > On Thu, 16 Feb 2023 at 14:31, Heinrich Schuchardt
> > <heinrich.schuchardt at canonical.com> wrote:
> >>
> >>
> >>
> >> On 2/16/23 21:17, Simon Glass wrote:
> >>> Hi Heinrich,
> >>>
> >>> On Thu, 16 Feb 2023 at 08:30, Heinrich Schuchardt
> >>> <heinrich.schuchardt at canonical.com> wrote:
> >>>>
> >>>> Some boards provide main U-Boot as a dedicated partition to SPL.
> >>>> Currently we can define either a fixed partition number or an MBR
> >>>> partition type to define which partition is to be used.
> >>>>
> >>>> Partition numbers tend to conflict with established partitioning schemes
> >>>> of Linux distros. MBR partitioning is more and more replaced by GPT
> >>>> partitioning.
> >>>>
> >>>> Allow defining a partition type GUID identifying the partition to load
> >>>> main U-Boot from.
> >>>>
> >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >>>> ---
> >>>> v2:
> >>>> avoid if/endif in Kconfig
> >>>> ---
> >>>> common/spl/Kconfig | 27 ++++++++++++++++++++++-----
> >>>> common/spl/spl_mmc.c | 13 +++++++++++++
> >>>> 2 files changed, 35 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> >>>> index 3c2af453ab..9d12b48297 100644
> >>>> --- a/common/spl/Kconfig
> >>>> +++ b/common/spl/Kconfig
> >>>> @@ -514,19 +514,36 @@ config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION
> >>>> used in raw mode
> >>>>
> >>>> config SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
> >>>> - bool "MMC raw mode: by partition type"
> >>>> + bool "MMC raw mode: by MBR partition type"
> >>>> depends on DOS_PARTITION && SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
> >>>> help
> >>>> - Use partition type for specifying U-Boot partition on MMC/SD in
> >>>> + Use MBR partition type for specifying U-Boot partition on MMC/SD in
> >>>> raw mode. U-Boot will be loaded from the first partition of this
> >>>> type to be found.
> >>>>
> >>>> config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE
> >>>> - hex "Partition Type on the MMC to load U-Boot from"
> >>>> + hex "MBR Partition Type on the MMC to load U-Boot from"
> >>>> depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
> >>>> help
> >>>> - Partition Type on the MMC to load U-Boot from, when the MMC is being
> >>>> - used in raw mode.
> >>>> + MBR Partition Type on the MMC to load U-Boot from, when the MMC is
> >>>> + being used in raw mode.
> >>>> +
> >>>> +config SYS_MMCSD_RAW_MODE_U_BOOT_USE_GPT_PARTITION_TYPE
> >>>> + bool "MMC raw mode: GPT by partition type"
> >>>> + depends on PARTITION_TYPE_GUID && SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
> >>>> + help
> >>>> + Use GPT partition type for specifying U-Boot partition on MMC/SD in
> >>>> + raw mode. U-Boot will be loaded from the first partition of this
> >>>> + type to be found.
> >>>> +
> >>>> +config SYS_MMCSD_RAW_MODE_U_BOOT_GPT_PARTITION_TYPE
> >>>> + string "GPT Partition Type on the MMC to load U-Boot from"
> >>>> + depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_GPT_PARTITION_TYPE
> >>>> + default d2f002f8-e4e7-4269-b8ac-3bb6fabeaff6
> >>>
> >>> What is this? Can we have a register of these hideous things and call
> >>> them by name?
> >
> > Further, I don't see any documentation on this in U-Boot. Could you at
> > least add a list of these things?
> >
> >>>
> >>>> + help
> >>>> + GPT Partition Type on the MMC to load U-Boot from, when the MMC is
> >>>> + being used in raw mode. The GUID must be lower case, low endian,
> >>>> + and formatted like d2f002f8-e4e7-4269-b8ac-3bb6fabeaff6.
> >>>>
> >>>> config SUPPORT_EMMC_BOOT_OVERRIDE_PART_CONFIG
> >>>> bool "Override eMMC EXT_CSC_PART_CONFIG by user defined partition"
> >>>> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> >>>> index e4135b2048..69bf1d6e98 100644
> >>>> --- a/common/spl/spl_mmc.c
> >>>> +++ b/common/spl/spl_mmc.c
> >>>> @@ -191,6 +191,19 @@ static int mmc_load_image_raw_partition(struct spl_image_info *spl_image,
> >>>> struct disk_partition info;
> >>>> int err;
> >>>>
> >>>> +#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_GPT_PARTITION_TYPE
> >>>> + for (int i = 1; i <= MAX_SEARCH_PARTITIONS; ++i) {
> >>>> + err = part_get_info(mmc_get_blk_desc(mmc), i, &info);
> >>>> + if (err)
> >>>> + continue;
> >>>> + if (!strncmp(info.type_guid,
> >>>> + CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_GPT_PARTITION_TYPE,
> >>>> + UUID_STR_LEN)) {
> >>>> + partition = i;
> >>>> + break;
> >>>> + }
> >>>> + }
> >>>> +#endif
> >>>> #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
> >>>> int type_part;
> >>>> /* Only support MBR so DOS_ENTRY_NUMBERS */
> >>>> --
> >>>> 2.38.1
> >>>>
> >>>
> >>> Is it possible to avoid using #ifdef here?
> >>
> >> Unfortunately not. Field 'type_guid' is restricted by an #ifdef. So
> >> unconditional compilation would fail.
> >
> > Do you think it is worth adding an accessor as we have done with some
> > global_data things?
>
> There are other places like disk/part_efi.c where we could trade #ifdef
> for if with such an accessor. The accessor itself would require an #ifdef.
>
> We should be careful about the value returned for
> CONFIG_PARTITION_TYPE_GUID=n. Returning NULL would probably lead to
> warnings from GCC and Coverity. We would better return a dummy string
> like "00000000-0000-0000-0000-000000000000".
>
> >
> >>
> >>>
> >>> Longer term, I wonder if we can add a DT schema for all of
> >>> this...these CONFIG options for boot selection seem to be getting out
> >>> of hand!
> >>
> >> Tom just moved a lot of constants hard coded in C code to Kconfig with a
> >> big effort. Now you want to move Kconfig values to hard coded constants
> >> in device-tree.
> >>
> >> Running in circles does not sound like a winning strategy.
> >
> > The values seem to be common across certain boards of the same type, SoC, etc.
>
> As I described above using the same GUID value for different boards only
> makes sense if they are supported by the very same U-Boot binary.
>
> On boards with the same SoC (even from different vendors) I would very
> much prefer a single U-Boot SPL selecting a board specific device-tree
> to having multiple binaries. We should push developers into this direction.
>
> >
> > I'm not sure, but at some point this is all going to get out of hand.
> > Already we have these options:
> >
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_EMMC_BOOT_PARTITION
> > common/spl/Kconfig:config SYS_MMCSD_FS_BOOT_PARTITION
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_KERNEL_SECTOR
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_ARGS_SECTOR
> > common/spl/Kconfig:config SYS_MMCSD_RAW_MODE_ARGS_SECTORS
> >
> > That is just for MMC raw mode.
> >
> > For environment we have SYS_MMC_ENV_DEV and _PART. If you look around
> > you'll see loads of these options.
> >
> > I see that rockchip uses u-boot,spl-boot-order as a way to determine
> > the boot order. This makes it configurable without rebuilding
> > U-Boot...although I don't think we need to make the MMC stuff
> > configurable, since I am assuming that the boot ROM determines at
> > least some of it...?
>
> This patch is about SPL loading main U-Boot. So the boot ROM is not
> involved.
>
> >
> > It seems that the whole thing is crying out for a bit of organisation
> > and a proper schema.
>
> The discussion was about hard-coding the values vs configuration.
Actually my intent was to use names instead of UUIDs, which I consider
to be obfuscation. Then there would be a conversion table somewhere.
>
> OS distributions should have enough flexibility to deliver an
> installation image with U-Boot for multiple boards on the same medium.
> For the build process it is preferable to use different configurations
> instead of patching source code per U-Boot which might be required if
> hard-coded values for partition GUIDs in the device-trees are used.
>
> I think Tom's approach is right. The U-Boot documentation should give
> guidance on how new boards should find U-Boot SPL and main U-Boot.
For my understanding, why is U-Boot SPL in one partition and U-Boot
proper in another?
Regards,
Simon
More information about the U-Boot
mailing list