[U-Boot] [PATCH v2 02/21] part: extract MBR signature from partitions

[Simon Glass]

Hi Rob,

On 10 September 2017 at 05:21, Rob Clark <robdclark at gmail.com> wrote:
> From: Peter Jones <pjones at redhat.com>
>
> EFI client programs need the signature information from the partition
> table to determine the disk a partition is on, so we need to fill that
> in here.
>
> Signed-off-by: Peter Jones <pjones at redhat.com>
> [separated from efi_loader part, and fixed build-errors for non-
>  CONFIG_EFI_PARTITION case]
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
>  disk/part_dos.c | 12 +++++++++---
>  disk/part_efi.c | 20 ++++++++++++++++++++
>  include/blk.h   | 15 +++++++++++++++
>  3 files changed, 44 insertions(+), 3 deletions(-)
>

Reviewed-by: Simon Glass <sjg at chromium.org>

Please see below.

> diff --git a/disk/part_dos.c b/disk/part_dos.c
> index 7ede15ec26..850a538e83 100644
> --- a/disk/part_dos.c
> +++ b/disk/part_dos.c
> @@ -89,14 +89,20 @@ static int test_block_type(unsigned char *buffer)
>
>  static int part_test_dos(struct blk_desc *dev_desc)
>  {
> -       ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
> +       ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, dev_desc->blksz);
>
> -       if (blk_dread(dev_desc, 0, 1, (ulong *)buffer) != 1)
> +       if (blk_dread(dev_desc, 0, 1, (ulong *)mbr) != 1)
>                 return -1;
>
> -       if (test_block_type(buffer) != DOS_MBR)
> +       if (test_block_type((unsigned char *)mbr) != DOS_MBR)
>                 return -1;
>
> +       if (dev_desc->sig_type == SIG_TYPE_NONE &&
> +           mbr->unique_mbr_signature != 0) {
> +               dev_desc->sig_type = SIG_TYPE_MBR;
> +               dev_desc->mbr_sig = mbr->unique_mbr_signature;
> +       }
> +
>         return 0;
>  }
>
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index 2973d52f6a..208bb14ee8 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -923,11 +923,19 @@ static int is_pmbr_valid(legacy_mbr * mbr)
>  static int is_gpt_valid(struct blk_desc *dev_desc, u64 lba,
>                         gpt_header *pgpt_head, gpt_entry **pgpt_pte)
>  {
> +       ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, dev_desc->blksz);
> +
>         if (!dev_desc || !pgpt_head) {
>                 printf("%s: Invalid Argument(s)\n", __func__);
>                 return 0;
>         }
>
> +       /* Read MBR Header from device */
> +       if (blk_dread(dev_desc, 0, 1, (ulong *)mbr) != 1) {
> +               printf("*** ERROR: Can't read MBR header ***\n");
> +               return 0;
> +       }
> +
>         /* Read GPT Header from device */
>         if (blk_dread(dev_desc, (lbaint_t)lba, 1, pgpt_head) != 1) {
>                 printf("*** ERROR: Can't read GPT header ***\n");
> @@ -937,6 +945,18 @@ static int is_gpt_valid(struct blk_desc *dev_desc, u64 lba,
>         if (validate_gpt_header(pgpt_head, (lbaint_t)lba, dev_desc->lba))
>                 return 0;
>
> +       if (dev_desc->sig_type == SIG_TYPE_NONE) {
> +               efi_guid_t empty = {};
> +               if (memcmp(&pgpt_head->disk_guid, &empty, sizeof(empty))) {
> +                       dev_desc->sig_type = SIG_TYPE_GUID;
> +                       memcpy(&dev_desc->guid_sig, &pgpt_head->disk_guid,
> +                             sizeof(empty));
> +               } else if (mbr->unique_mbr_signature != 0) {
> +                       dev_desc->sig_type = SIG_TYPE_MBR;
> +                       dev_desc->mbr_sig = mbr->unique_mbr_signature;
> +               }
> +       }
> +
>         /* Read and allocate Partition Table Entries */
>         *pgpt_pte = alloc_read_gpt_entries(dev_desc, pgpt_head);
>         if (*pgpt_pte == NULL) {
> diff --git a/include/blk.h b/include/blk.h
> index a106f9ca0e..3c59fdb3d4 100644
> --- a/include/blk.h
> +++ b/include/blk.h
> @@ -8,6 +8,8 @@
>  #ifndef BLK_H
>  #define BLK_H
>
> +#include <efi.h>
> +
>  #ifdef CONFIG_SYS_64BIT_LBA
>  typedef uint64_t lbaint_t;
>  #define LBAFlength "ll"
> @@ -36,6 +38,14 @@ enum if_type {
>         IF_TYPE_COUNT,                  /* Number of interface types */
>  };
>
> +enum sig_type {

Please add a comment about what this is

> +       SIG_TYPE_NONE,
> +       SIG_TYPE_MBR,
> +       SIG_TYPE_GUID,
> +
> +       SIG_TYPE_COUNT                  /* Number of signature types */
> +};
> +
>  /*
>   * With driver model (CONFIG_BLK) this is uclass platform data, accessible
>   * with dev_get_uclass_platdata(dev)
> @@ -63,6 +73,11 @@ struct blk_desc {
>         char            vendor[40+1];   /* IDE model, SCSI Vendor */
>         char            product[20+1];  /* IDE Serial no, SCSI product */
>         char            revision[8+1];  /* firmware revision */
> +       enum sig_type   sig_type;       /* Partition table signature type */
> +       union {
> +               uint32_t mbr_sig;       /* MBR integer signature */
> +               efi_guid_t guid_sig;    /* GPT GUID Signature */
> +       };
>  #if CONFIG_IS_ENABLED(BLK)
>         /*
>          * For now we have a few functions which take struct blk_desc as a
> --
> 2.13.5
>

Regards,
Simon