* [PATCH v2 1/1] spl: allow loading via partition type GUID @ 2023-02-16 15:29 Heinrich Schuchardt 2023-02-16 17:01 ` Tom Rini 2023-02-16 20:17 ` Simon Glass 0 siblings, 2 replies; 15+ messages in thread From: Heinrich Schuchardt @ 2023-02-16 15:29 UTC (permalink / raw) To: Tom Rini Cc: Yanhong Wang, Simon Glass, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot, Heinrich Schuchardt 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@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 + 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-16 15:29 [PATCH v2 1/1] spl: allow loading via partition type GUID Heinrich Schuchardt @ 2023-02-16 17:01 ` Tom Rini 2023-02-16 17:19 ` Heinrich Schuchardt 2023-02-16 20:17 ` Simon Glass 1 sibling, 1 reply; 15+ messages in thread From: Tom Rini @ 2023-02-16 17:01 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Yanhong Wang, Simon Glass, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot [-- Attachment #1: Type: text/plain, Size: 1078 bytes --] On Thu, Feb 16, 2023 at 04:29:56PM +0100, Heinrich Schuchardt 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@canonical.com> [snip] > +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 Is there a reason to use any other GUID value here? If not, just define, or use it directly with a comment. If there is a reason for other values to be used: Reviewed-by: Tom Rini <trini@konsulko.com> -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-16 17:01 ` Tom Rini @ 2023-02-16 17:19 ` Heinrich Schuchardt 2023-02-16 17:22 ` Tom Rini 0 siblings, 1 reply; 15+ messages in thread From: Heinrich Schuchardt @ 2023-02-16 17:19 UTC (permalink / raw) To: Tom Rini Cc: Yanhong Wang, Simon Glass, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot On 2/16/23 18:01, Tom Rini wrote: > On Thu, Feb 16, 2023 at 04:29:56PM +0100, Heinrich Schuchardt 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@canonical.com> > [snip] >> +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 > > Is there a reason to use any other GUID value here? If not, just define, > or use it directly with a comment. If there is a reason for other > values to be used: Board vendors already differ in the partition GUIDs they use, e.g. SiFive: SPL: 5B193300-FC78-40CD-8002-E86C45580B47 Main U-Boot: 2E54B353-1271-4842-806F-E436D6AF6985 StarFive SPL: 2E54B353-1271-4842-806F-E436D6AF6985 (sic) Microchip Icicle Board: SPL: 21686148-6449-6E6F-744E-656564454649 Using unique IDs per board allow to create SD card images where U-Boot for multiple boards is installed in parallel: The boot ROM will select the board specific SPL via a unique ID and then SPL will use another unique ID to load the matching main U-Boot. I only added a default value here to provide a formatting sample. Best regards Heinrich > > Reviewed-by: Tom Rini <trini@konsulko.com> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-16 17:19 ` Heinrich Schuchardt @ 2023-02-16 17:22 ` Tom Rini 0 siblings, 0 replies; 15+ messages in thread From: Tom Rini @ 2023-02-16 17:22 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Yanhong Wang, Simon Glass, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot [-- Attachment #1: Type: text/plain, Size: 2037 bytes --] On Thu, Feb 16, 2023 at 06:19:11PM +0100, Heinrich Schuchardt wrote: > > > On 2/16/23 18:01, Tom Rini wrote: > > On Thu, Feb 16, 2023 at 04:29:56PM +0100, Heinrich Schuchardt 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@canonical.com> > > [snip] > > > +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 > > > > Is there a reason to use any other GUID value here? If not, just define, > > or use it directly with a comment. If there is a reason for other > > values to be used: > > Board vendors already differ in the partition GUIDs they use, e.g. > > SiFive: > SPL: 5B193300-FC78-40CD-8002-E86C45580B47 > Main U-Boot: 2E54B353-1271-4842-806F-E436D6AF6985 > > StarFive > SPL: 2E54B353-1271-4842-806F-E436D6AF6985 (sic) > > Microchip Icicle Board: > SPL: 21686148-6449-6E6F-744E-656564454649 > > Using unique IDs per board allow to create SD card images where U-Boot for > multiple boards is installed in parallel: > > The boot ROM will select the board specific SPL via a unique ID and then SPL > will use another unique ID to load the matching main U-Boot. > > I only added a default value here to provide a formatting sample. Thanks for elaborating. It might be helpful to put something in a follow-up patch about these use cases under doc/ -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-16 15:29 [PATCH v2 1/1] spl: allow loading via partition type GUID Heinrich Schuchardt 2023-02-16 17:01 ` Tom Rini @ 2023-02-16 20:17 ` Simon Glass 2023-02-16 21:31 ` Heinrich Schuchardt 1 sibling, 1 reply; 15+ messages in thread From: Simon Glass @ 2023-02-16 20:17 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Tom Rini, Yanhong Wang, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot Hi Heinrich, On Thu, 16 Feb 2023 at 08:30, Heinrich Schuchardt <heinrich.schuchardt@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@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? > + 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? 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! Regards, Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-16 20:17 ` Simon Glass @ 2023-02-16 21:31 ` Heinrich Schuchardt 2023-02-17 2:55 ` Simon Glass 0 siblings, 1 reply; 15+ messages in thread From: Heinrich Schuchardt @ 2023-02-16 21:31 UTC (permalink / raw) To: Simon Glass Cc: Tom Rini, Yanhong Wang, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot On 2/16/23 21:17, Simon Glass wrote: > Hi Heinrich, > > On Thu, 16 Feb 2023 at 08:30, Heinrich Schuchardt > <heinrich.schuchardt@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@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? > >> + 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. > > 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. Best regards Heinrich > > Regards, > Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-16 21:31 ` Heinrich Schuchardt @ 2023-02-17 2:55 ` Simon Glass 2023-02-17 6:55 ` Heinrich Schuchardt 0 siblings, 1 reply; 15+ messages in thread From: Simon Glass @ 2023-02-17 2:55 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Tom Rini, Yanhong Wang, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot " properHi Heinrich, On Thu, 16 Feb 2023 at 14:31, Heinrich Schuchardt <heinrich.schuchardt@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@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@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? > > > > > 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. 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...? It seems that the whole thing is crying out for a bit of organisation and a proper schema. Regards, Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-17 2:55 ` Simon Glass @ 2023-02-17 6:55 ` Heinrich Schuchardt 2023-02-17 10:34 ` Mark Kettenis 2023-02-21 19:41 ` Simon Glass 0 siblings, 2 replies; 15+ messages in thread From: Heinrich Schuchardt @ 2023-02-17 6:55 UTC (permalink / raw) To: Simon Glass Cc: Tom Rini, Yanhong Wang, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot On 2/17/23 03:55, Simon Glass wrote: > " properHi Heinrich, > > On Thu, 16 Feb 2023 at 14:31, Heinrich Schuchardt > <heinrich.schuchardt@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@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@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. 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. Best regards Heinrich ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-17 6:55 ` Heinrich Schuchardt @ 2023-02-17 10:34 ` Mark Kettenis 2023-02-17 11:06 ` Heinrich Schuchardt 2023-02-21 19:41 ` Simon Glass 1 sibling, 1 reply; 15+ messages in thread From: Mark Kettenis @ 2023-02-17 10:34 UTC (permalink / raw) To: Heinrich Schuchardt Cc: sjg, trini, yanhong.wang, afd, alpernebiyasak, sr, andre.przywara, cJ-uboot, hws, u-boot > Date: Fri, 17 Feb 2023 07:55:58 +0100 > From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > 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. But in that case we surely want to have a single board and SoC independent partition GUID? > > 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. > > 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. Well, yes, but it would be even more helpful to have a single well-known partition GUID such that the OS partitioning tools can recognize the U-Boot partition. If you make it configurable and every contributed board uses a different GUID that will be impractical. > 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. > > Best regards > > Heinrich > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-17 10:34 ` Mark Kettenis @ 2023-02-17 11:06 ` Heinrich Schuchardt 2023-02-17 15:03 ` Tom Rini 2023-02-19 19:52 ` Mark Kettenis 0 siblings, 2 replies; 15+ messages in thread From: Heinrich Schuchardt @ 2023-02-17 11:06 UTC (permalink / raw) To: Mark Kettenis Cc: sjg, trini, yanhong.wang, afd, alpernebiyasak, sr, andre.przywara, cJ-uboot, hws, u-boot On 2/17/23 11:34, Mark Kettenis wrote: >> Date: Fri, 17 Feb 2023 07:55:58 +0100 >> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >> >>> 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. > > But in that case we surely want to have a single board and SoC > independent partition GUID? > No. I want to create one installation image which runs on multiple boards, e.g. part 1, GUID 8300, /boot part 2, GUID 8300, / part 15, GUID EF00, /boot/efi part 20, GUID SPL 1, SPL for board 1 part 21, GUID U-Boot 2, U-Boot for board 1 ... part 127, GUID SPL 54, SPL for board 54 part 128, GUID U-Boot 54, U-Boot for board 54 >>> 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. >> >> 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. > > Well, yes, but it would be even more helpful to have a single > well-known partition GUID such that the OS partitioning tools can > recognize the U-Boot partition. If you make it configurable and every > contributed board uses a different GUID that will be impractical. The OS partitioning tools simply shouldn't touch partitions with GUIDs that they don't know. Best regards Heinrich > >> 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. >> >> Best regards >> >> Heinrich >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-17 11:06 ` Heinrich Schuchardt @ 2023-02-17 15:03 ` Tom Rini 2023-02-19 19:52 ` Mark Kettenis 1 sibling, 0 replies; 15+ messages in thread From: Tom Rini @ 2023-02-17 15:03 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Mark Kettenis, sjg, yanhong.wang, afd, alpernebiyasak, sr, andre.przywara, cJ-uboot, hws, u-boot [-- Attachment #1: Type: text/plain, Size: 2417 bytes --] On Fri, Feb 17, 2023 at 12:06:40PM +0100, Heinrich Schuchardt wrote: > On 2/17/23 11:34, Mark Kettenis wrote: > > > Date: Fri, 17 Feb 2023 07:55:58 +0100 > > > From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > > > > > 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. > > > > But in that case we surely want to have a single board and SoC > > independent partition GUID? > > > > No. I want to create one installation image which runs on multiple boards, > e.g. > > part 1, GUID 8300, /boot > part 2, GUID 8300, / > part 15, GUID EF00, /boot/efi > part 20, GUID SPL 1, SPL for board 1 > part 21, GUID U-Boot 2, U-Boot for board 1 > ... > part 127, GUID SPL 54, SPL for board 54 > part 128, GUID U-Boot 54, U-Boot for board 54 Is this concept proposed somewhere that OS / distro people might further comment on? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-17 11:06 ` Heinrich Schuchardt 2023-02-17 15:03 ` Tom Rini @ 2023-02-19 19:52 ` Mark Kettenis 2023-02-19 21:21 ` Heinrich Schuchardt 1 sibling, 1 reply; 15+ messages in thread From: Mark Kettenis @ 2023-02-19 19:52 UTC (permalink / raw) To: Heinrich Schuchardt Cc: sjg, trini, yanhong.wang, afd, alpernebiyasak, sr, andre.przywara, cJ-uboot, hws, u-boot > Date: Fri, 17 Feb 2023 12:06:40 +0100 > From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > On 2/17/23 11:34, Mark Kettenis wrote: > >> Date: Fri, 17 Feb 2023 07:55:58 +0100 > >> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > >> > >>> 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. > > > > But in that case we surely want to have a single board and SoC > > independent partition GUID? > > > > No. I want to create one installation image which runs on multiple > boards, e.g. > > part 1, GUID 8300, /boot > part 2, GUID 8300, / > part 15, GUID EF00, /boot/efi > part 20, GUID SPL 1, SPL for board 1 > part 21, GUID U-Boot 2, U-Boot for board 1 > ... > part 127, GUID SPL 54, SPL for board 54 > part 128, GUID U-Boot 54, U-Boot for board 54 Interesting idea. However, if you rely on the SoC bootrom to boot from a partition with a specific GUID, you probably can't have separate SPL partitions for each board; you'd just have one for each SoC. And that in turn means that you can't really have a separate U-Boot partition for each board unless you have board-detection code in SPL. But in that case you'd probably be better off with putting that board detection code in U-Boot itself and bundle U-Boot together with the supported board device trees in a FIT. > >>> 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. > >> > >> 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. > > > > Well, yes, but it would be even more helpful to have a single > > well-known partition GUID such that the OS partitioning tools can > > recognize the U-Boot partition. If you make it configurable and every > > contributed board uses a different GUID that will be impractical. > > The OS partitioning tools simply shouldn't touch partitions with GUIDs > that they don't know. Well, that makes it hard to "recycle" disks that have been partitioned before. Partition tools will have the ability to delete partitions to make that possible. In that case it is useful for users to be able to determine the purpose of a partition. By the way, you probably should set Bit 0 in the partition table entry Attributes field for these partitions to indicate that a partition should not be deleted. > >> 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. > >> > >> Best regards > >> > >> Heinrich > >> > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-19 19:52 ` Mark Kettenis @ 2023-02-19 21:21 ` Heinrich Schuchardt 0 siblings, 0 replies; 15+ messages in thread From: Heinrich Schuchardt @ 2023-02-19 21:21 UTC (permalink / raw) To: Mark Kettenis Cc: sjg, trini, yanhong.wang, afd, alpernebiyasak, sr, andre.przywara, cJ-uboot, hws, u-boot On 2/19/23 20:52, Mark Kettenis wrote: >> Date: Fri, 17 Feb 2023 12:06:40 +0100 >> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >> >> On 2/17/23 11:34, Mark Kettenis wrote: >>>> Date: Fri, 17 Feb 2023 07:55:58 +0100 >>>> From: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >>>> >>>>> 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. >>> >>> But in that case we surely want to have a single board and SoC >>> independent partition GUID? >>> >> >> No. I want to create one installation image which runs on multiple >> boards, e.g. >> >> part 1, GUID 8300, /boot >> part 2, GUID 8300, / >> part 15, GUID EF00, /boot/efi >> part 20, GUID SPL 1, SPL for board 1 >> part 21, GUID U-Boot 2, U-Boot for board 1 >> ... >> part 127, GUID SPL 54, SPL for board 54 >> part 128, GUID U-Boot 54, U-Boot for board 54 > > Interesting idea. However, if you rely on the SoC bootrom to boot > from a partition with a specific GUID, you probably can't have > separate SPL partitions for each board; you'd just have one for each > SoC. And that in turn means that you can't really have a separate > U-Boot partition for each board unless you have board-detection code > in SPL. But in that case you'd probably be better off with putting > that board detection code in U-Boot itself and bundle U-Boot together > with the supported board device trees in a FIT. You are right in that GUID based SPL selection and board detection code will probably have to be combined. For the same SoC selecting a board specific device-tree may work in many cases. > >>>>> 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. >>>> >>>> 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. >>> >>> Well, yes, but it would be even more helpful to have a single >>> well-known partition GUID such that the OS partitioning tools can >>> recognize the U-Boot partition. If you make it configurable and every >>> contributed board uses a different GUID that will be impractical. >> >> The OS partitioning tools simply shouldn't touch partitions with GUIDs >> that they don't know. > > Well, that makes it hard to "recycle" disks that have been partitioned > before. Partition tools will have the ability to delete partitions to > make that possible. In that case it is useful for users to be able to > determine the purpose of a partition. > > By the way, you probably should set Bit 0 in the partition table entry > Attributes field for these partitions to indicate that a partition > should not be deleted. It was you who suggested to hard code a GUID for firmware that would get special treatment. My suggestion is simply that that special treatment can be given to all unknown GUIDs. Best regards Heinrich > >>>> 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. >>>> >>>> Best regards >>>> >>>> Heinrich >>>> >> >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-17 6:55 ` Heinrich Schuchardt 2023-02-17 10:34 ` Mark Kettenis @ 2023-02-21 19:41 ` Simon Glass 2023-02-26 14:56 ` Simon Glass 1 sibling, 1 reply; 15+ messages in thread From: Simon Glass @ 2023-02-21 19:41 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Tom Rini, Yanhong Wang, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot Hi Heinrich, On Thu, 16 Feb 2023 at 23:56, Heinrich Schuchardt <heinrich.schuchardt@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@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@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@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] spl: allow loading via partition type GUID 2023-02-21 19:41 ` Simon Glass @ 2023-02-26 14:56 ` Simon Glass 0 siblings, 0 replies; 15+ messages in thread From: Simon Glass @ 2023-02-26 14:56 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Tom Rini, Yanhong Wang, Andrew Davis, Alper Nebi Yasak, Stefan Roese, Andre Przywara, Jérôme Carretero, Harald Seiler, u-boot Hi Heinrich, On Tue, 21 Feb 2023 at 12:41, Simon Glass <sjg@chromium.org> wrote: > > Hi Heinrich, > > On Thu, 16 Feb 2023 at 23:56, Heinrich Schuchardt > <heinrich.schuchardt@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@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@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@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. Actually there already is one in uuid.c so we could use the strings in that. > > > > > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-02-26 14:57 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-16 15:29 [PATCH v2 1/1] spl: allow loading via partition type GUID Heinrich Schuchardt 2023-02-16 17:01 ` Tom Rini 2023-02-16 17:19 ` Heinrich Schuchardt 2023-02-16 17:22 ` Tom Rini 2023-02-16 20:17 ` Simon Glass 2023-02-16 21:31 ` Heinrich Schuchardt 2023-02-17 2:55 ` Simon Glass 2023-02-17 6:55 ` Heinrich Schuchardt 2023-02-17 10:34 ` Mark Kettenis 2023-02-17 11:06 ` Heinrich Schuchardt 2023-02-17 15:03 ` Tom Rini 2023-02-19 19:52 ` Mark Kettenis 2023-02-19 21:21 ` Heinrich Schuchardt 2023-02-21 19:41 ` Simon Glass 2023-02-26 14:56 ` Simon Glass
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox