diff mbox series

[U-Boot,v3,02/21] part: extract MBR signature from partitions

Message ID 20170913220546.19560-3-robdclark@gmail.com
State Accepted
Commit ff98cb90514d9b787ddc097c203ac8db2941efe1
Delegated to: Alexander Graf
Headers show
Series efi_loader: enough UEFI for standard distro boot | expand

Commit Message

Rob Clark Sept. 13, 2017, 10:05 p.m. UTC 
From: Peter Jones <pjones@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@redhat.com>
[separated from efi_loader part, and fixed build-errors for non-
 CONFIG_EFI_PARTITION case]
Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 disk/part_dos.c | 12 +++++++++---
 disk/part_efi.c | 20 ++++++++++++++++++++
 include/blk.h   | 18 ++++++++++++++++++
 3 files changed, 47 insertions(+), 3 deletions(-)

Comments

Alexander Graf Sept. 21, 2017, 7:05 a.m. UTC | #1 
> From: Peter Jones <pjones@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@redhat.com>
> [separated from efi_loader part, and fixed build-errors for non-
>  CONFIG_EFI_PARTITION case]
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Thanks, applied to efi-next

Alex
Fabio Estevam Oct. 2, 2017, 6:17 p.m. UTC | #2 
On Wed, Sep 13, 2017 at 7:05 PM, Rob Clark <robdclark@gmail.com> wrote:
> From: Peter Jones <pjones@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@redhat.com>
> [separated from efi_loader part, and fixed build-errors for non-
>  CONFIG_EFI_PARTITION case]
> Signed-off-by: Rob Clark <robdclark@gmail.com>

This is commit ff98cb90514d9b78 in mainline now and breaks all mx6 SPL boots:

U-Boot SPL 2017.09-00221-g0d6ab32 (Oct 02 2017 - 15:13:19)
Trying to boot from MMC1
(hangs here)

Does anyone know how to fix this problem?
Peter Robinson Oct. 2, 2017, 6:49 p.m. UTC | #3 
On Mon, Oct 2, 2017 at 7:17 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Sep 13, 2017 at 7:05 PM, Rob Clark <robdclark@gmail.com> wrote:
>> From: Peter Jones <pjones@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@redhat.com>
>> [separated from efi_loader part, and fixed build-errors for non-
>>  CONFIG_EFI_PARTITION case]
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>
> This is commit ff98cb90514d9b78 in mainline now and breaks all mx6 SPL boots:
>
> U-Boot SPL 2017.09-00221-g0d6ab32 (Oct 02 2017 - 15:13:19)
> Trying to boot from MMC1
> (hangs here)
>
> Does anyone know how to fix this problem?

That would explain what I'm seeing as we pull this patch set in to
2017.09 for EFI boot support on aarch64 platforms.

Peter
Fabio Estevam Oct. 2, 2017, 7:02 p.m. UTC | #4 
On Mon, Oct 2, 2017 at 3:49 PM, Peter Robinson <pbrobinson@gmail.com> wrote:

> That would explain what I'm seeing as we pull this patch set in to
> 2017.09 for EFI boot support on aarch64 platforms.

Locally I am applying the following hack, so that I can continue to
work on mx6 with SPL:
https://pastebin.com/MX8tK76X
Tom Rini Oct. 2, 2017, 7:32 p.m. UTC | #5 
On Mon, Oct 02, 2017 at 04:02:37PM -0300, Fabio Estevam wrote:
> On Mon, Oct 2, 2017 at 3:49 PM, Peter Robinson <pbrobinson@gmail.com> wrote:
> 
> > That would explain what I'm seeing as we pull this patch set in to
> > 2017.09 for EFI boot support on aarch64 platforms.
> 
> Locally I am applying the following hack, so that I can continue to
> work on mx6 with SPL:
> https://pastebin.com/MX8tK76X

FWIW, on cubox the current status is looping, and your kludge I just get
no output :(  But there may be some other problems here too as I'm
seeing some oddities as I try out SDP on it.
Rob Clark Oct. 2, 2017, 7:35 p.m. UTC | #6 
On Mon, Oct 2, 2017 at 2:17 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Sep 13, 2017 at 7:05 PM, Rob Clark <robdclark@gmail.com> wrote:
>> From: Peter Jones <pjones@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@redhat.com>
>> [separated from efi_loader part, and fixed build-errors for non-
>>  CONFIG_EFI_PARTITION case]
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>
> This is commit ff98cb90514d9b78 in mainline now and breaks all mx6 SPL boots:
>
> U-Boot SPL 2017.09-00221-g0d6ab32 (Oct 02 2017 - 15:13:19)
> Trying to boot from MMC1
> (hangs here)
>
> Does anyone know how to fix this problem?

not sure to what extent debug()/printf() works from SPL, but if it
does maybe adding this patch with and without that commit and
comparing logs would give some hint:

--------------
diff --git a/disk/part.c b/disk/part.c
index aa9183d696..8e0bf54b3e 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -5,6 +5,8 @@
  * SPDX-License-Identifier:    GPL-2.0+
  */

+#define DEBUG
+
 #include <common.h>
 #include <command.h>
 #include <errno.h>
@@ -13,7 +15,7 @@
 #include <part.h>
 #include <ubifs_uboot.h>

-#undef    PART_DEBUG
+#define    PART_DEBUG

 #ifdef    PART_DEBUG
 #define    PRINTF(fmt,args...)    printf (fmt ,##args)
--------------

I don't really see why this commit would cause problems, but I guess
either something unexpected going on w/ partitions or block device
driver on this platform..

BR,
-R
Fabio Estevam Oct. 3, 2017, 12:30 a.m. UTC | #7 
Hi Tom,

On Mon, Oct 2, 2017 at 4:32 PM, Tom Rini <trini@konsulko.com> wrote:

> FWIW, on cubox the current status is looping, and your kludge I just get
> no output :(  But there may be some other problems here too as I'm

Just tried it on 2017.11-rc1 plus the attached hack and it boots fine:

U-Boot SPL 2017.11-rc1-dirty (Oct 02 2017 - 21:27:10)
Trying to boot from MMC1


U-Boot 2017.11-rc1-dirty (Oct 02 2017 - 21:27:10 -0300)

CPU:   Freescale i.MX6Q rev1.5 996 MHz (running at 792 MHz)
CPU:   Extended Commercial temperature grade (-20C to 105C) at 26C
Reset cause: POR
Board: MX6 Cubox-i
DRAM:  2 GiB
MMC:   FSL_SDHC: 0
No panel detected: default to HDMI
Display: HDMI (1024x768)
In:    serial
Out:   serial
Err:   serial
Net:   FEC
Hit any key to stop autoboot:  0
=>
From b334aa07bcf959fed9c68bfc8e801f9aabe9b93c Mon Sep 17 00:00:00 2001
From: Fabio Estevam <festevam@gmail.com>
Date: Mon, 2 Oct 2017 21:28:14 -0300
Subject: [PATCH] part_dos: hack

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 disk/part_dos.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index 1a36be0..a1cf553 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -89,20 +89,6 @@ static int test_block_type(unsigned char *buffer)
 
 static int part_test_dos(struct blk_desc *dev_desc)
 {
-	ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, dev_desc->blksz);
-
-	if (blk_dread(dev_desc, 0, 1, (ulong *)mbr) != 1)
-		return -1;
-
-	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;
 }
Fabio Estevam Oct. 3, 2017, 1:52 a.m. UTC | #8 
Hi Rob,

On Mon, Oct 2, 2017 at 4:35 PM, Rob Clark <robdclark@gmail.com> wrote:

> not sure to what extent debug()/printf() works from SPL, but if it
> does maybe adding this patch with and without that commit and
> comparing logs would give some hint:

Unfortunately I don't get any logs with this debug patch.

> I don't really see why this commit would cause problems, but I guess
> either something unexpected going on w/ partitions or block device
> driver on this platform..

I have just sent a RFC patch that fixes the boot here.
diff mbox series

Patch

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 1965812a9d..41b4d7efa8 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"
@@ -41,6 +43,17 @@  enum if_type {
 #define BLK_REV_SIZE		8
 
 /*
+ * Identifies the partition table type (ie. MBR vs GPT GUID) signature
+ */
+enum sig_type {
+	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)
  */
@@ -67,6 +80,11 @@  struct blk_desc {
 	char		vendor[BLK_VEN_SIZE + 1]; /* device vendor string */
 	char		product[BLK_PRD_SIZE + 1]; /* device product number */
 	char		revision[BLK_REV_SIZE + 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