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

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

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: