[PATCH 0/5] efi_loader: disk: Add EFI_PARTITION_INFO_PROTOCOL support for MBR

[PATCH 0/5] efi_loader: disk: Add EFI_PARTITION_INFO_PROTOCOL support for MBR

[Javier Martinez Canillas]

This patch series extends the EFI_PARTITION_INFO_PROTOCOL implementation
to also support MBR partition tables schemes.

When EFI_PARTITION_INFO_PROTOCOL was implented, only GPT support was
added but this can cause compatibility issues with platforms whose boot
ROM only supports MBR. This series add support for MBR partition tables
to the protocol, making U-Boot compatible with systems that require a
legacy MBR table.

Patches #1 to #3 are preparatory changes to remove duplicated definitions
of data structures to store the MBR records.

Finally patch #4 adds the MBR support to the EFI_PARTITION_INFO_PROTOCOL
implementation and patch #5 enhances the protocol selftest, to check that
the partition record data is correctly filled.


Javier Martinez Canillas (5):
  disk: part_dos: Move header to the main include directory
  disk: part_dos: Align dos_partition_t with struct partition
  disk: part_efi: Remove redundant struct partition definition
  efi_loader: disk: Extend EFI_PARTITION_INFO_PROTOCOL to support MBR
  efi_selftest: Enhance MBR test for PARTITION_INFO_PROTOCOL

 disk/part_dos.c                              | 77 ++++++++++++--------
 disk/part_efi.c                              |  4 +-
 include/part.h                               | 14 ++++
 {disk => include}/part_dos.h                 |  6 +-
 include/part_efi.h                           | 19 +----
 lib/efi_loader/efi_disk.c                    |  9 ++-
 lib/efi_selftest/efi_selftest_block_device.c | 12 +++
 7 files changed, 86 insertions(+), 55 deletions(-)
 rename {disk => include}/part_dos.h (87%)

-- 
2.52.0

base-commit: 6cdd7597a2fbfc1572c1b0af23d3daf1cefa2de7
branch: add-mbr-efi-part-info-proto

[PATCH 1/5] disk: part_dos: Move header to the main include directory

[Javier Martinez Canillas]

There are two different struct definitions for MBR partition table
entries: one in part_dos.h and a nearly identical one in part_efi.h.

To enable future consolidation of these two structures, move part_dos.h
to the main include directory. This makes it accessible from other parts
of the codebase, such as part_efi.h, and is the first step toward removing
the redundant definition.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 disk/part_dos.c              | 2 +-
 {disk => include}/part_dos.h | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename {disk => include}/part_dos.h (100%)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index 18dd35c9b98e..2545cc6bf5d2 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -19,7 +19,7 @@
 #include <vsprintf.h>
 #include <asm/unaligned.h>
 #include <linux/compiler.h>
-#include "part_dos.h"
+#include <part_dos.h>
 #include <part.h>
 
 #define DOS_PART_DEFAULT_SECTOR 512
diff --git a/disk/part_dos.h b/include/part_dos.h
similarity index 100%
rename from disk/part_dos.h
rename to include/part_dos.h
-- 
2.52.0

Re: [PATCH 1/5] disk: part_dos: Move header to the main include directory

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

On Tue, Jan 20, 2026 at 02:36:10PM +0100, Javier Martinez Canillas wrote:

> There are two different struct definitions for MBR partition table
> entries: one in part_dos.h and a nearly identical one in part_efi.h.
> 
> To enable future consolidation of these two structures, move part_dos.h
> to the main include directory. This makes it accessible from other parts
> of the codebase, such as part_efi.h, and is the first step toward removing
> the redundant definition.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/5] disk: part_dos: Align dos_partition_t with struct partition

[Javier Martinez Canillas]

The dos_partition_t struct defined in part_dos.h is nearly identical to
the struct partition defined in part_efi.h. They differ primarily in how
define their starting sector and number of sectors fields.

The former uses unsigned char arrays while the latter uses __le32 types.
Using __le32 is preferable, as it removes the ambiguity and potential
misuse of a raw byte array. This also aligns the structure with how the
Linux kernel defines it nowadays, which is the original source of it.

To prepare for future consolidation where one of the data structures can
be removed, this change aligns both definitions and updates all accessors
for dos_partition_t.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 disk/part_dos.c    | 16 ++++++++--------
 include/part_dos.h |  6 +++---
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index 2545cc6bf5d2..60c3d6773696 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -49,8 +49,8 @@ static int get_bootable(dos_partition_t *p)
 static void print_one_part(dos_partition_t *p, lbaint_t ext_part_sector,
 			   int part_num, unsigned int disksig)
 {
-	lbaint_t lba_start = ext_part_sector + get_unaligned_le32(p->start4);
-	lbaint_t lba_size  = get_unaligned_le32(p->size4);
+	lbaint_t lba_start = ext_part_sector + get_unaligned_le32(&p->start_sect);
+	lbaint_t lba_size  = get_unaligned_le32(&p->nr_sects);
 
 	printf("%3d\t%-10" LBAFlength "u\t%-10" LBAFlength
 		"u\t%08x-%02x\t%02x%s%s\n",
@@ -185,7 +185,7 @@ static void print_partition_extended(struct blk_desc *desc,
 	for (i = 0; i < 4; i++, pt++) {
 		if (is_extended (pt->sys_ind)) {
 			lbaint_t lba_start
-				= get_unaligned_le32 (pt->start4) + relative;
+				= get_unaligned_le32 (&pt->start_sect) + relative;
 
 			print_partition_extended(desc, lba_start,
 						 !ext_part_sector ? lba_start :
@@ -252,8 +252,8 @@ static int part_get_info_extended(struct blk_desc *desc,
 			else
 				info->blksz = DOS_PART_DEFAULT_SECTOR;
 			info->start = (lbaint_t)(ext_part_sector +
-					get_unaligned_le32(pt->start4));
-			info->size  = (lbaint_t)get_unaligned_le32(pt->size4);
+					get_unaligned_le32(&pt->start_sect));
+			info->size  = (lbaint_t)get_unaligned_le32(&pt->nr_sects);
 			part_set_generic_name(desc, part_num,
 					      (char *)info->name);
 			/* sprintf(info->type, "%d, pt->sys_ind); */
@@ -281,7 +281,7 @@ static int part_get_info_extended(struct blk_desc *desc,
 	for (i = 0; i < 4; i++, pt++) {
 		if (is_extended (pt->sys_ind)) {
 			lbaint_t lba_start
-				= get_unaligned_le32 (pt->start4) + relative;
+				= get_unaligned_le32 (&pt->start_sect) + relative;
 
 			return part_get_info_extended(desc, lba_start,
 				 ext_part_sector == 0 ? lba_start : relative,
@@ -356,8 +356,8 @@ static void mbr_fill_pt_entry(dos_partition_t *pt, lbaint_t start,
 	pt->sys_ind = sys_ind;
 	lba_to_chs(start, &pt->cyl, &pt->head, &pt->sector);
 	lba_to_chs(start + size - 1, &pt->end_cyl, &pt->end_head, &pt->end_sector);
-	put_unaligned_le32(relative, &pt->start4);
-	put_unaligned_le32(size, &pt->size4);
+	put_unaligned_le32(relative, &pt->start_sect);
+	put_unaligned_le32(size, &pt->nr_sects);
 }
 
 int write_mbr_partitions(struct blk_desc *dev,
diff --git a/include/part_dos.h b/include/part_dos.h
index 505582242281..92956d530637 100644
--- a/include/part_dos.h
+++ b/include/part_dos.h
@@ -30,8 +30,8 @@ typedef struct dos_partition {
 	unsigned char end_head;		/* end head				*/
 	unsigned char end_sector;	/* end sector				*/
 	unsigned char end_cyl;		/* end cylinder				*/
-	unsigned char start4[4];	/* starting sector counting from 0	*/
-	unsigned char size4[4];		/* nr of sectors in partition		*/
-} dos_partition_t;
+	__le32 start_sect;		/* starting sector counting from 0	*/
+	__le32 nr_sects;		/* nr of sectors in partition		*/
+} __packed dos_partition_t;
 
 #endif	/* _DISK_PART_DOS_H */
-- 
2.52.0

Re: [PATCH 2/5] disk: part_dos: Align dos_partition_t with struct partition

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]

On Tue, Jan 20, 2026 at 02:36:11PM +0100, Javier Martinez Canillas wrote:

> The dos_partition_t struct defined in part_dos.h is nearly identical to
> the struct partition defined in part_efi.h. They differ primarily in how
> define their starting sector and number of sectors fields.
> 
> The former uses unsigned char arrays while the latter uses __le32 types.
> Using __le32 is preferable, as it removes the ambiguity and potential
> misuse of a raw byte array. This also aligns the structure with how the
> Linux kernel defines it nowadays, which is the original source of it.
> 
> To prepare for future consolidation where one of the data structures can
> be removed, this change aligns both definitions and updates all accessors
> for dos_partition_t.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 3/5] disk: part_efi: Remove redundant struct partition definition

[Javier Martinez Canillas]

Now that dos_partition_t and struct partition are identical the duplicated
data structure definition in the part_efi.h header can just be removed.

This results in a single, shared definition for MBR partition table
entries, instead of having the same definition in two different places.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 disk/part_efi.c    |  4 ++--
 include/part_efi.h | 19 +++----------------
 2 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index fb1ed534f861..d8b17ec2e91a 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -51,7 +51,7 @@ static inline u32 efi_crc32(const void *buf, u32 len)
  * Private function prototypes
  */
 
-static int pmbr_part_valid(struct partition *part);
+static int pmbr_part_valid(dos_partition_t *part);
 static int is_pmbr_valid(legacy_mbr * mbr);
 static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head,
 			gpt_entry **pgpt_pte);
@@ -990,7 +990,7 @@ int write_mbr_and_gpt_partitions(struct blk_desc *desc, void *buf)
  *
  * Returns: 1 if EFI GPT partition type is found.
  */
-static int pmbr_part_valid(struct partition *part)
+static int pmbr_part_valid(dos_partition_t *part)
 {
 	if (part->sys_ind == EFI_PMBR_OSTYPE_EFI_GPT &&
 		get_unaligned_le32(&part->start_sect) == 1UL) {
diff --git a/include/part_efi.h b/include/part_efi.h
index fb402df6f13e..2cea50880465 100644
--- a/include/part_efi.h
+++ b/include/part_efi.h
@@ -18,6 +18,7 @@
 #define _DISK_PART_EFI_H
 
 #include <efi.h>
+#include <part_dos.h>
 
 #define MSDOS_MBR_SIGNATURE 0xAA55
 #define MSDOS_MBR_BOOT_CODE_SIZE 440
@@ -77,20 +78,6 @@
 /* linux/include/efi.h */
 typedef u16 efi_char16_t;
 
-/* based on linux/include/genhd.h */
-struct partition {
-	u8 boot_ind;		/* 0x80 - active */
-	u8 head;		/* starting head */
-	u8 sector;		/* starting sector */
-	u8 cyl;			/* starting cylinder */
-	u8 sys_ind;		/* What partition type */
-	u8 end_head;		/* end head */
-	u8 end_sector;		/* end sector */
-	u8 end_cyl;		/* end cylinder */
-	__le32 start_sect;	/* starting sector counting from 0 */
-	__le32 nr_sects;	/* nr of sectors in partition */
-} __packed;
-
 /* based on linux/fs/partitions/efi.h */
 typedef struct _gpt_header {
 	__le64 signature;
@@ -134,7 +121,7 @@ typedef struct _legacy_mbr {
 	u8 boot_code[MSDOS_MBR_BOOT_CODE_SIZE];
 	__le32 unique_mbr_signature;
 	__le16 unknown;
-	struct partition partition_record[4];
+	dos_partition_t partition_record[4];
 	__le16 signature;
 } __packed legacy_mbr;
 
@@ -153,7 +140,7 @@ struct efi_partition_info {
 	u8 system;
 	u8 reserved[7];
 	union {
-		struct partition mbr;
+		dos_partition_t mbr;
 		gpt_entry gpt;
 	} info;
 } __packed;
-- 
2.52.0

Re: [PATCH 3/5] disk: part_efi: Remove redundant struct partition definition

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

On Tue, Jan 20, 2026 at 02:36:12PM +0100, Javier Martinez Canillas wrote:

> Now that dos_partition_t and struct partition are identical the duplicated
> data structure definition in the part_efi.h header can just be removed.
> 
> This results in a single, shared definition for MBR partition table
> entries, instead of having the same definition in two different places.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 4/5] efi_loader: disk: Extend EFI_PARTITION_INFO_PROTOCOL to support MBR

[Javier Martinez Canillas]

The EFI_PARTITION_INFO_PROTOCOL provides detailed information about
partitions. The UEFI specification mentions that both GPT and MBR
partition schemes are supported, but the U-Boot implementation only
supports the former.

This can cause compatibility issues for platforms whose boot ROM only
supports MBR. This change adds support for MBR partition tables to
the protocol, making U-Boot compatible with systems that require a
legacy MBR table.

To implement this, the existing part_get_info_extended() function,
which already traverses DOS partitions, is refactored to optionally
retrieve the raw MBR partition record. This provides the necessary
data for the EFI subsystem.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 disk/part_dos.c           | 63 ++++++++++++++++++++++++---------------
 include/part.h            | 14 +++++++++
 lib/efi_loader/efi_disk.c |  9 ++++--
 3 files changed, 59 insertions(+), 27 deletions(-)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index 60c3d6773696..5b508483bae7 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -201,7 +201,9 @@ static void print_partition_extended(struct blk_desc *desc,
 static int part_get_info_extended(struct blk_desc *desc,
 				  lbaint_t ext_part_sector, lbaint_t relative,
 				  int part_num, int which_part,
-				  struct disk_partition *info, uint disksig)
+				  struct disk_partition *info,
+				  dos_partition_t *mbr,
+				  uint disksig)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, desc->blksz);
 	struct disk_partition wdinfo = { 0 };
@@ -232,9 +234,11 @@ static int part_get_info_extended(struct blk_desc *desc,
 	if (CONFIG_IS_ENABLED(PARTITION_UUIDS) && !ext_part_sector)
 		disksig = get_unaligned_le32(&buffer[DOS_PART_DISKSIG_OFFSET]);
 
-	ret = part_get_info_whole_disk(desc, &wdinfo);
-	if (ret)
-		return ret;
+	if (info) {
+		ret = part_get_info_whole_disk(desc, &wdinfo);
+		if (ret)
+			return ret;
+	}
 
 	/* Print all primary/logical partitions */
 	pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET);
@@ -247,25 +251,29 @@ static int part_get_info_extended(struct blk_desc *desc,
 		    (pt->sys_ind != 0) &&
 		    (part_num == which_part) &&
 		    (ext_part_sector == 0 || is_extended(pt->sys_ind) == 0)) {
-			if (wdinfo.blksz > DOS_PART_DEFAULT_SECTOR)
-				info->blksz = wdinfo.blksz;
-			else
-				info->blksz = DOS_PART_DEFAULT_SECTOR;
-			info->start = (lbaint_t)(ext_part_sector +
-					get_unaligned_le32(&pt->start_sect));
-			info->size  = (lbaint_t)get_unaligned_le32(&pt->nr_sects);
-			part_set_generic_name(desc, part_num,
-					      (char *)info->name);
-			/* sprintf(info->type, "%d, pt->sys_ind); */
-			strcpy((char *)info->type, "U-Boot");
-			info->bootable = get_bootable(pt);
-			if (CONFIG_IS_ENABLED(PARTITION_UUIDS)) {
-				char str[12];
-
-				sprintf(str, "%08x-%02x", disksig, part_num);
-				disk_partition_set_uuid(info, str);
+			if (info) {
+				if (wdinfo.blksz > DOS_PART_DEFAULT_SECTOR)
+					info->blksz = wdinfo.blksz;
+				else
+					info->blksz = DOS_PART_DEFAULT_SECTOR;
+				info->start = (lbaint_t)(ext_part_sector +
+							 get_unaligned_le32(&pt->start_sect));
+				info->size  = (lbaint_t)get_unaligned_le32(&pt->nr_sects);
+				part_set_generic_name(desc, part_num,
+						      (char *)info->name);
+				/* sprintf(info->type, "%d, pt->sys_ind); */
+				strcpy((char *)info->type, "U-Boot");
+				info->bootable = get_bootable(pt);
+				if (CONFIG_IS_ENABLED(PARTITION_UUIDS)) {
+					char str[12];
+
+					sprintf(str, "%08x-%02x", disksig, part_num);
+					disk_partition_set_uuid(info, str);
+				}
+				info->sys_ind = pt->sys_ind;
 			}
-			info->sys_ind = pt->sys_ind;
+			if (mbr)
+				memcpy(mbr, pt, sizeof(*mbr));
 			return 0;
 		}
 
@@ -285,7 +293,8 @@ static int part_get_info_extended(struct blk_desc *desc,
 
 			return part_get_info_extended(desc, lba_start,
 				 ext_part_sector == 0 ? lba_start : relative,
-				 part_num, which_part, info, disksig);
+						      part_num, which_part, info,
+						      mbr, disksig);
 		}
 	}
 
@@ -317,7 +326,13 @@ static void __maybe_unused part_print_dos(struct blk_desc *desc)
 static int __maybe_unused part_get_info_dos(struct blk_desc *desc, int part,
 					    struct disk_partition *info)
 {
-	return part_get_info_extended(desc, 0, 0, 1, part, info, 0);
+	return part_get_info_extended(desc, 0, 0, 1, part, info, NULL, 0);
+}
+
+int __maybe_unused part_get_mbr(struct blk_desc *desc, int part,
+				dos_partition_t *mbr)
+{
+	return part_get_info_extended(desc, 0, 0, 1, part, NULL, mbr, 0);
 }
 
 int is_valid_dos_buf(void *buf)
diff --git a/include/part.h b/include/part.h
index daebbbc2e68f..84dbdbbd1494 100644
--- a/include/part.h
+++ b/include/part.h
@@ -704,6 +704,20 @@ int write_mbr_partitions(struct blk_desc *dev,
 int layout_mbr_partitions(struct disk_partition *p, int count,
 			  lbaint_t total_sectors);
 
+/**
+ * part_get_mbr() - Get the MBR partition record of a partition
+ *
+ * This function reads the MBR partition record for a given block
+ * device and partition number.
+ *
+ * @desc:	block device descriptor
+ * @part:	partition number for which to return the partition record
+ * @mbr:	MBR partition record
+ *
+ * Return:	0 on success, otherwise error
+ */
+int part_get_mbr(struct blk_desc *desc, int part, dos_partition_t *mbr);
+
 #endif
 
 #if CONFIG_IS_ENABLED(PARTITIONS)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 130c4db9606f..f8a57539ec61 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -475,9 +475,12 @@ static efi_status_t efi_disk_add_dev(
 #if CONFIG_IS_ENABLED(DOS_PARTITION)
 		case PART_TYPE_DOS:
 			info->type = PARTITION_TYPE_MBR;
-
-			/* TODO: implement support for MBR partition types */
-			log_debug("EFI_PARTITION_INFO_PROTOCOL doesn't support MBR\n");
+			ret = part_get_mbr(desc, part, &info->info.mbr);
+			if (ret) {
+				log_debug("get MBR for part %d failed %ld\n",
+					  part, ret);
+				goto error;
+			}
 			break;
 #endif
 		default:
-- 
2.52.0

Re: [PATCH 4/5] efi_loader: disk: Extend EFI_PARTITION_INFO_PROTOCOL to support MBR

[Heinrich Schuchardt]

On 1/20/26 14:36, Javier Martinez Canillas wrote:
> The EFI_PARTITION_INFO_PROTOCOL provides detailed information about
> partitions. The UEFI specification mentions that both GPT and MBR
> partition schemes are supported, but the U-Boot implementation only
> supports the former.
> 
> This can cause compatibility issues for platforms whose boot ROM only
> supports MBR. This change adds support for MBR partition tables to
> the protocol, making U-Boot compatible with systems that require a
> legacy MBR table.
> 
> To implement this, the existing part_get_info_extended() function,
> which already traverses DOS partitions, is refactored to optionally
> retrieve the raw MBR partition record. This provides the necessary
> data for the EFI subsystem.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
>   disk/part_dos.c           | 63 ++++++++++++++++++++++++---------------
>   include/part.h            | 14 +++++++++
>   lib/efi_loader/efi_disk.c |  9 ++++--
>   3 files changed, 59 insertions(+), 27 deletions(-)
> 
> diff --git a/disk/part_dos.c b/disk/part_dos.c
> index 60c3d6773696..5b508483bae7 100644
> --- a/disk/part_dos.c
> +++ b/disk/part_dos.c
> @@ -201,7 +201,9 @@ static void print_partition_extended(struct blk_desc *desc,

Please add a Sphinx style function description describing the usage of 
the parameters.

>   static int part_get_info_extended(struct blk_desc *desc,
>   				  lbaint_t ext_part_sector, lbaint_t relative,
>   				  int part_num, int which_part,
> -				  struct disk_partition *info, uint disksig)
> +				  struct disk_partition *info,
> +				  dos_partition_t *mbr,
> +				  uint disksig)
>   {
>   	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, desc->blksz);
>   	struct disk_partition wdinfo = { 0 };
> @@ -232,9 +234,11 @@ static int part_get_info_extended(struct blk_desc *desc,
>   	if (CONFIG_IS_ENABLED(PARTITION_UUIDS) && !ext_part_sector)
>   		disksig = get_unaligned_le32(&buffer[DOS_PART_DISKSIG_OFFSET]);
>   
> -	ret = part_get_info_whole_disk(desc, &wdinfo);
> -	if (ret)
> -		return ret;
> +	if (info) {
> +		ret = part_get_info_whole_disk(desc, &wdinfo);
> +		if (ret)
> +			return ret;
> +	}
>   
>   	/* Print all primary/logical partitions */
>   	pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET);
> @@ -247,25 +251,29 @@ static int part_get_info_extended(struct blk_desc *desc,
>   		    (pt->sys_ind != 0) &&
>   		    (part_num == which_part) &&
>   		    (ext_part_sector == 0 || is_extended(pt->sys_ind) == 0)) {
> -			if (wdinfo.blksz > DOS_PART_DEFAULT_SECTOR)
> -				info->blksz = wdinfo.blksz;
> -			else
> -				info->blksz = DOS_PART_DEFAULT_SECTOR;
> -			info->start = (lbaint_t)(ext_part_sector +
> -					get_unaligned_le32(&pt->start_sect));
> -			info->size  = (lbaint_t)get_unaligned_le32(&pt->nr_sects);
> -			part_set_generic_name(desc, part_num,
> -					      (char *)info->name);
> -			/* sprintf(info->type, "%d, pt->sys_ind); */
> -			strcpy((char *)info->type, "U-Boot");
> -			info->bootable = get_bootable(pt);
> -			if (CONFIG_IS_ENABLED(PARTITION_UUIDS)) {
> -				char str[12];
> -
> -				sprintf(str, "%08x-%02x", disksig, part_num);
> -				disk_partition_set_uuid(info, str);
> +			if (info) {
> +				if (wdinfo.blksz > DOS_PART_DEFAULT_SECTOR)
> +					info->blksz = wdinfo.blksz;
> +				else
> +					info->blksz = DOS_PART_DEFAULT_SECTOR;
> +				info->start = (lbaint_t)(ext_part_sector +
> +							 get_unaligned_le32(&pt->start_sect));
> +				info->size  = (lbaint_t)get_unaligned_le32(&pt->nr_sects);
> +				part_set_generic_name(desc, part_num,
> +						      (char *)info->name);
> +				/* sprintf(info->type, "%d, pt->sys_ind); */
> +				strcpy((char *)info->type, "U-Boot");
> +				info->bootable = get_bootable(pt);
> +				if (CONFIG_IS_ENABLED(PARTITION_UUIDS)) {
> +					char str[12];
> +
> +					sprintf(str, "%08x-%02x", disksig, part_num);
> +					disk_partition_set_uuid(info, str);
> +				}
> +				info->sys_ind = pt->sys_ind;
>   			}
> -			info->sys_ind = pt->sys_ind;
> +			if (mbr)
> +				memcpy(mbr, pt, sizeof(*mbr));
>   			return 0;
>   		}
>   
> @@ -285,7 +293,8 @@ static int part_get_info_extended(struct blk_desc *desc,
>   
>   			return part_get_info_extended(desc, lba_start,
>   				 ext_part_sector == 0 ? lba_start : relative,
> -				 part_num, which_part, info, disksig);
> +						      part_num, which_part, info,
> +						      mbr, disksig);
>   		}
>   	}
>   
> @@ -317,7 +326,13 @@ static void __maybe_unused part_print_dos(struct blk_desc *desc)
>   static int __maybe_unused part_get_info_dos(struct blk_desc *desc, int part,
>   					    struct disk_partition *info)
>   {
> -	return part_get_info_extended(desc, 0, 0, 1, part, info, 0);
> +	return part_get_info_extended(desc, 0, 0, 1, part, info, NULL, 0);
> +}
> +
> +int __maybe_unused part_get_mbr(struct blk_desc *desc, int part,
> +				dos_partition_t *mbr)
> +{
> +	return part_get_info_extended(desc, 0, 0, 1, part, NULL, mbr, 0);
>   }
>   
>   int is_valid_dos_buf(void *buf)
> diff --git a/include/part.h b/include/part.h
> index daebbbc2e68f..84dbdbbd1494 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -704,6 +704,20 @@ int write_mbr_partitions(struct blk_desc *dev,
>   int layout_mbr_partitions(struct disk_partition *p, int count,
>   			  lbaint_t total_sectors);
>   
> +/**
> + * part_get_mbr() - Get the MBR partition record of a partition
> + *
> + * This function reads the MBR partition record for a given block
> + * device and partition number.
> + *
> + * @desc:	block device descriptor
> + * @part:	partition number for which to return the partition record
> + * @mbr:	MBR partition record
> + *
> + * Return:	0 on success, otherwise error
> + */
> +int part_get_mbr(struct blk_desc *desc, int part, dos_partition_t *mbr);
> +
>   #endif
>   

Please, put the partition library changes into one patch and the EFI 
changes into another.

Best regards

Heinrich

>   #if CONFIG_IS_ENABLED(PARTITIONS)
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 130c4db9606f..f8a57539ec61 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -475,9 +475,12 @@ static efi_status_t efi_disk_add_dev(
>   #if CONFIG_IS_ENABLED(DOS_PARTITION)
>   		case PART_TYPE_DOS:
>   			info->type = PARTITION_TYPE_MBR;
> -
> -			/* TODO: implement support for MBR partition types */
> -			log_debug("EFI_PARTITION_INFO_PROTOCOL doesn't support MBR\n");
> +			ret = part_get_mbr(desc, part, &info->info.mbr);
> +			if (ret) {
> +				log_debug("get MBR for part %d failed %ld\n",
> +					  part, ret);
> +				goto error;
> +			}
>   			break;
>   #endif
>   		default:

Re: [PATCH 4/5] efi_loader: disk: Extend EFI_PARTITION_INFO_PROTOCOL to support MBR

[Javier Martinez Canillas]

Heinrich Schuchardt <xypron.glpk@gmx.de> writes:

Hello Heinrich,

Thanks for your feedback.

> On 1/20/26 14:36, Javier Martinez Canillas wrote:
>> The EFI_PARTITION_INFO_PROTOCOL provides detailed information about
>> partitions. The UEFI specification mentions that both GPT and MBR
>> partition schemes are supported, but the U-Boot implementation only
>> supports the former.
>> 
>> This can cause compatibility issues for platforms whose boot ROM only
>> supports MBR. This change adds support for MBR partition tables to
>> the protocol, making U-Boot compatible with systems that require a
>> legacy MBR table.
>> 
>> To implement this, the existing part_get_info_extended() function,
>> which already traverses DOS partitions, is refactored to optionally
>> retrieve the raw MBR partition record. This provides the necessary
>> data for the EFI subsystem.
>> 
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>> 
>>   disk/part_dos.c           | 63 ++++++++++++++++++++++++---------------
>>   include/part.h            | 14 +++++++++
>>   lib/efi_loader/efi_disk.c |  9 ++++--
>>   3 files changed, 59 insertions(+), 27 deletions(-)
>> 
>> diff --git a/disk/part_dos.c b/disk/part_dos.c
>> index 60c3d6773696..5b508483bae7 100644
>> --- a/disk/part_dos.c
>> +++ b/disk/part_dos.c
>> @@ -201,7 +201,9 @@ static void print_partition_extended(struct blk_desc *desc,
>
> Please add a Sphinx style function description describing the usage of 
> the parameters.
>

Sure, I didn't add for the new parameter since the function didn't have
a description, but I'll add a preparatory patch in v2 to add that.

[...]
   
>> +/**
>> + * part_get_mbr() - Get the MBR partition record of a partition
>> + *
>> + * This function reads the MBR partition record for a given block
>> + * device and partition number.
>> + *
>> + * @desc:	block device descriptor
>> + * @part:	partition number for which to return the partition record
>> + * @mbr:	MBR partition record
>> + *
>> + * Return:	0 on success, otherwise error
>> + */
>> +int part_get_mbr(struct blk_desc *desc, int part, dos_partition_t *mbr);
>> +
>>   #endif
>>   
>
> Please, put the partition library changes into one patch and the EFI 
> changes into another.
>

Ok, I'll split these in v2.

> Best regards
>
> Heinrich
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 4/5] efi_loader: disk: Extend EFI_PARTITION_INFO_PROTOCOL to support MBR

[Jan Kiszka]

On 20.01.26 14:36, Javier Martinez Canillas wrote:
> The EFI_PARTITION_INFO_PROTOCOL provides detailed information about
> partitions. The UEFI specification mentions that both GPT and MBR
> partition schemes are supported, but the U-Boot implementation only
> supports the former.
> 
> This can cause compatibility issues for platforms whose boot ROM only
> supports MBR. This change adds support for MBR partition tables to
> the protocol, making U-Boot compatible with systems that require a
> legacy MBR table.

Normally, this is simple to address via hybrid partition tables. I've
done that already for two different SoCs (TI AM62 and Altera Cyclone V),
and this allows to use all the nifty features of the modern GPT without
upsetting the ROM loaders. Do you have a SoC where this does not work?

That said, adding this feature may still be valuable.

Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center

Re: [PATCH 4/5] efi_loader: disk: Extend EFI_PARTITION_INFO_PROTOCOL to support MBR

[Javier Martinez Canillas]

Jan Kiszka <jan.kiszka@siemens.com> writes:

Hello Jan,

> On 20.01.26 14:36, Javier Martinez Canillas wrote:
>> The EFI_PARTITION_INFO_PROTOCOL provides detailed information about
>> partitions. The UEFI specification mentions that both GPT and MBR
>> partition schemes are supported, but the U-Boot implementation only
>> supports the former.
>> 
>> This can cause compatibility issues for platforms whose boot ROM only
>> supports MBR. This change adds support for MBR partition tables to
>> the protocol, making U-Boot compatible with systems that require a
>> legacy MBR table.
>
> Normally, this is simple to address via hybrid partition tables. I've
> done that already for two different SoCs (TI AM62 and Altera Cyclone V),
> and this allows to use all the nifty features of the modern GPT without
> upsetting the ROM loaders. Do you have a SoC where this does not work?
>

Yes, I know that could be worked around by using hybrid partition tables
but IMO using that could be fragile.

> That said, adding this feature may still be valuable.
>

Indeed and as mentioned in the commit message it is something supported by
the EFI partition info protocol according to the spec, so it makes sense
for U-boot to also implement support for MBR.

> Jan
>
> -- 
> Siemens AG, Foundational Technologies
> Linux Expert Center
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH 4/5] efi_loader: disk: Extend EFI_PARTITION_INFO_PROTOCOL to support MBR

[Jan Kiszka]

On 21.01.26 10:28, Javier Martinez Canillas wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
> Hello Jan,
> 
>> On 20.01.26 14:36, Javier Martinez Canillas wrote:
>>> The EFI_PARTITION_INFO_PROTOCOL provides detailed information about
>>> partitions. The UEFI specification mentions that both GPT and MBR
>>> partition schemes are supported, but the U-Boot implementation only
>>> supports the former.
>>>
>>> This can cause compatibility issues for platforms whose boot ROM only
>>> supports MBR. This change adds support for MBR partition tables to
>>> the protocol, making U-Boot compatible with systems that require a
>>> legacy MBR table.
>>
>> Normally, this is simple to address via hybrid partition tables. I've
>> done that already for two different SoCs (TI AM62 and Altera Cyclone V),
>> and this allows to use all the nifty features of the modern GPT without
>> upsetting the ROM loaders. Do you have a SoC where this does not work?
>>
> 
> Yes, I know that could be worked around by using hybrid partition tables
> but IMO using that could be fragile.

Try it, it's not in my experience. Again, you gain modern GPT partitions
this way, for anything but the boot partition the ROM loader needs.

Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center

[PATCH 5/5] efi_selftest: Enhance MBR test for PARTITION_INFO_PROTOCOL

[Javier Martinez Canillas]

The EFI_PARTITION_INFO_PROTOCOL test was added before the protocol fully
supported MBR partitions. As a result, it lacked specific checks for the
content of the raw MBR partition record.

Now that MBR support has been implemented, enhance the selftest to provide
coverage for the MBR entries too.

This verifies that the protocol correctly reads and exposes MBR partition
records and prevents this functionality to regress due future changes.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 lib/efi_selftest/efi_selftest_block_device.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c
index f145e58a267a..50d1fb29e1ef 100644
--- a/lib/efi_selftest/efi_selftest_block_device.c
+++ b/lib/efi_selftest/efi_selftest_block_device.c
@@ -404,6 +404,18 @@ static int execute(void)
 			     part_info->system);
 		return EFI_ST_FAILURE;
 	}
+	/* The first partition starts at LBA 1 on the test image */
+	if (part_info->info.mbr.start_sect != 1) {
+		efi_st_error("MBR start sector %d, expected 1\n",
+			     part_info->info.mbr.start_sect);
+		return EFI_ST_FAILURE;
+	}
+	/* The partition type is FAT12 (0x01) in the test image */
+	if (part_info->info.mbr.sys_ind != 0x01) {
+		efi_st_error("MBR system indicator %d, expected 1\n",
+			     part_info->info.mbr.sys_ind);
+		return EFI_ST_FAILURE;
+	}
 
 	/* Open the simple file system protocol */
 	ret = boottime->open_protocol(handle_partition,
-- 
2.52.0

Re: [PATCH 5/5] efi_selftest: Enhance MBR test for PARTITION_INFO_PROTOCOL

[Heinrich Schuchardt]

On 1/20/26 14:36, Javier Martinez Canillas wrote:
> The EFI_PARTITION_INFO_PROTOCOL test was added before the protocol fully
> supported MBR partitions. As a result, it lacked specific checks for the
> content of the raw MBR partition record.
> 
> Now that MBR support has been implemented, enhance the selftest to provide
> coverage for the MBR entries too.
> 
> This verifies that the protocol correctly reads and exposes MBR partition
> records and prevents this functionality to regress due future changes.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
>   lib/efi_selftest/efi_selftest_block_device.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c
> index f145e58a267a..50d1fb29e1ef 100644
> --- a/lib/efi_selftest/efi_selftest_block_device.c
> +++ b/lib/efi_selftest/efi_selftest_block_device.c
> @@ -404,6 +404,18 @@ static int execute(void)
>   			     part_info->system);
>   		return EFI_ST_FAILURE;
>   	}
> +	/* The first partition starts at LBA 1 on the test image */
> +	if (part_info->info.mbr.start_sect != 1) {
> +		efi_st_error("MBR start sector %d, expected 1\n",
> +			     part_info->info.mbr.start_sect);
> +		return EFI_ST_FAILURE;
> +	}
> +	/* The partition type is FAT12 (0x01) in the test image */
> +	if (part_info->info.mbr.sys_ind != 0x01) {
> +		efi_st_error("MBR system indicator %d, expected 1\n",
> +			     part_info->info.mbr.sys_ind);
> +		return EFI_ST_FAILURE;
> +	}

How about defining a static variable with all fields and using memcmp() 
to check them all?

Best regards

Heinrich

>   
>   	/* Open the simple file system protocol */
>   	ret = boottime->open_protocol(handle_partition,

Re: [PATCH 5/5] efi_selftest: Enhance MBR test for PARTITION_INFO_PROTOCOL

[Javier Martinez Canillas]

Heinrich Schuchardt <xypron.glpk@gmx.de> writes:

> On 1/20/26 14:36, Javier Martinez Canillas wrote:
>> The EFI_PARTITION_INFO_PROTOCOL test was added before the protocol fully
>> supported MBR partitions. As a result, it lacked specific checks for the
>> content of the raw MBR partition record.
>> 
>> Now that MBR support has been implemented, enhance the selftest to provide
>> coverage for the MBR entries too.
>> 
>> This verifies that the protocol correctly reads and exposes MBR partition
>> records and prevents this functionality to regress due future changes.
>> 
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>> 
>>   lib/efi_selftest/efi_selftest_block_device.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>> 
>> diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c
>> index f145e58a267a..50d1fb29e1ef 100644
>> --- a/lib/efi_selftest/efi_selftest_block_device.c
>> +++ b/lib/efi_selftest/efi_selftest_block_device.c
>> @@ -404,6 +404,18 @@ static int execute(void)
>>   			     part_info->system);
>>   		return EFI_ST_FAILURE;
>>   	}
>> +	/* The first partition starts at LBA 1 on the test image */
>> +	if (part_info->info.mbr.start_sect != 1) {
>> +		efi_st_error("MBR start sector %d, expected 1\n",
>> +			     part_info->info.mbr.start_sect);
>> +		return EFI_ST_FAILURE;
>> +	}
>> +	/* The partition type is FAT12 (0x01) in the test image */
>> +	if (part_info->info.mbr.sys_ind != 0x01) {
>> +		efi_st_error("MBR system indicator %d, expected 1\n",
>> +			     part_info->info.mbr.sys_ind);
>> +		return EFI_ST_FAILURE;
>> +	}
>
> How about defining a static variable with all fields and using memcmp() 
> to check them all?
>

Makes sense. I'll do that in v2 as well. Thanks!

> Best regards
>
> Heinrich
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat