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 |
> 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
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?
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
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
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.
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
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; }
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 --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