[U-Boot] [PATCH 1/2] disk: efi: unify code for finding a valid gpt
There were 3 copies of the same sequence, make it into a function.
Signed-off-by: Urja Rannikko urjaman@gmail.com --- disk/part_efi.c | 73 +++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 39 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 239455b816..5f935da4c2 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -51,6 +51,8 @@ static int is_gpt_valid(struct blk_desc *dev_desc, u64 lba, static gpt_entry *alloc_read_gpt_entries(struct blk_desc *dev_desc, gpt_header *pgpt_head); static int is_pte_valid(gpt_entry * pte); +static int find_valid_gpt(struct blk_desc *dev_desc, gpt_header *gpt_head, + gpt_entry **pgpt_pte);
static char *print_efiname(gpt_entry *pte) { @@ -192,19 +194,8 @@ int get_disk_guid(struct blk_desc * dev_desc, char *guid) unsigned char *guid_bin;
/* This function validates AND fills in the GPT header and PTE */ - if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA, - gpt_head, &gpt_pte) != 1) { - printf("%s: *** ERROR: Invalid GPT ***\n", __func__); - if (is_gpt_valid(dev_desc, dev_desc->lba - 1, - gpt_head, &gpt_pte) != 1) { - printf("%s: *** ERROR: Invalid Backup GPT ***\n", - __func__); - return -EINVAL; - } else { - printf("%s: *** Using Backup GPT ***\n", - __func__); - } - } + if (find_valid_gpt(dev_desc, gpt_head, &gpt_pte) != 1) + return -EINVAL;
guid_bin = gpt_head->disk_guid.b; uuid_bin_to_str(guid_bin, guid, UUID_STR_FORMAT_GUID); @@ -221,19 +212,8 @@ void part_print_efi(struct blk_desc *dev_desc) unsigned char *uuid_bin;
/* This function validates AND fills in the GPT header and PTE */ - if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA, - gpt_head, &gpt_pte) != 1) { - printf("%s: *** ERROR: Invalid GPT ***\n", __func__); - if (is_gpt_valid(dev_desc, (dev_desc->lba - 1), - gpt_head, &gpt_pte) != 1) { - printf("%s: *** ERROR: Invalid Backup GPT ***\n", - __func__); - return; - } else { - printf("%s: *** Using Backup GPT ***\n", - __func__); - } - } + if (find_valid_gpt(dev_desc, gpt_head, &gpt_pte) != 1) + return;
debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
@@ -282,19 +262,8 @@ int part_get_info_efi(struct blk_desc *dev_desc, int part, }
/* This function validates AND fills in the GPT header and PTE */ - if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA, - gpt_head, &gpt_pte) != 1) { - printf("%s: *** ERROR: Invalid GPT ***\n", __func__); - if (is_gpt_valid(dev_desc, (dev_desc->lba - 1), - gpt_head, &gpt_pte) != 1) { - printf("%s: *** ERROR: Invalid Backup GPT ***\n", - __func__); - return -1; - } else { - printf("%s: *** Using Backup GPT ***\n", - __func__); - } - } + if (find_valid_gpt(dev_desc, gpt_head, &gpt_pte) != 1) + return -1;
if (part > le32_to_cpu(gpt_head->num_partition_entries) || !is_pte_valid(&gpt_pte[part - 1])) { @@ -981,6 +950,32 @@ static int is_gpt_valid(struct blk_desc *dev_desc, u64 lba, return 1; }
+/** + * find_valid_gpt() - finds a valid GPT header and PTEs + * + * gpt is a GPT header ptr, filled on return. + * ptes is a PTEs ptr, filled on return. + * + * Description: returns 1 if found a valid gpt, 0 on error. + * If valid, returns pointers to PTEs. + */ +static int find_valid_gpt(struct blk_desc *dev_desc, gpt_header *gpt_head, + gpt_entry **pgpt_pte) +{ + if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA, + gpt_head, pgpt_pte) != 1) { + printf("%s: *** ERROR: Invalid GPT ***\n", __func__); + if (is_gpt_valid(dev_desc, (dev_desc->lba - 1), + gpt_head, pgpt_pte) != 1) { + printf("%s: *** ERROR: Invalid Backup GPT ***\n", + __func__); + return 0; + } + printf("%s: *** Using Backup GPT ***\n", __func__); + } + return 1; +} + /** * alloc_read_gpt_entries(): reads partition entries from disk * @dev_desc
Some ChromeOS devices (atleast veyron speedy) have the first 8MiB of the eMMC write protected and equipped with a dummy 'IGNOREME' GPT header - instead of spewing error messages about it, just silently try the backup GPT.
Note: this does not touch the gpt cmd writing/verifying functions, those will still complain.
Signed-off-by: Urja Rannikko urjaman@gmail.com --- disk/part_efi.c | 28 +++++++++++++++++++++------- include/part_efi.h | 2 ++ 2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 5f935da4c2..83ba7db665 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -893,7 +893,7 @@ static int is_pmbr_valid(legacy_mbr * mbr) * gpt is a GPT header ptr, filled on return. * ptes is a PTEs ptr, filled on return. * - * Description: returns 1 if valid, 0 on error. + * Description: returns 1 if valid, 0 on error, 2 if ignored header * If valid, returns pointers to PTEs. */ static int is_gpt_valid(struct blk_desc *dev_desc, u64 lba, @@ -919,6 +919,12 @@ static int is_gpt_valid(struct blk_desc *dev_desc, u64 lba, return 0; }
+ /* Invalid but nothing to yell about. */ + if (le64_to_cpu(pgpt_head->signature) == GPT_HEADER_CHROMEOS_IGNORE) { + debug("ChromeOS 'IGNOREME' GPT header found and ignored\n"); + return 2; + } + if (validate_gpt_header(pgpt_head, (lbaint_t)lba, dev_desc->lba)) return 0;
@@ -962,16 +968,24 @@ static int is_gpt_valid(struct blk_desc *dev_desc, u64 lba, static int find_valid_gpt(struct blk_desc *dev_desc, gpt_header *gpt_head, gpt_entry **pgpt_pte) { - if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA, - gpt_head, pgpt_pte) != 1) { - printf("%s: *** ERROR: Invalid GPT ***\n", __func__); - if (is_gpt_valid(dev_desc, (dev_desc->lba - 1), - gpt_head, pgpt_pte) != 1) { + int r; + + r = is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA, gpt_head, + pgpt_pte); + + if (r != 1) { + if (r != 2) + printf("%s: *** ERROR: Invalid GPT ***\n", __func__); + + if (is_gpt_valid(dev_desc, (dev_desc->lba - 1), gpt_head, + pgpt_pte) != 1) { printf("%s: *** ERROR: Invalid Backup GPT ***\n", __func__); return 0; } - printf("%s: *** Using Backup GPT ***\n", __func__); + if (r != 2) + printf("%s: *** Using Backup GPT ***\n", + __func__); } return 1; } diff --git a/include/part_efi.h b/include/part_efi.h index 7170b61c95..eb5797af74 100644 --- a/include/part_efi.h +++ b/include/part_efi.h @@ -25,6 +25,8 @@ #define EFI_PMBR_OSTYPE_EFI_GPT 0xEE
#define GPT_HEADER_SIGNATURE_UBOOT 0x5452415020494645ULL +#define GPT_HEADER_CHROMEOS_IGNORE 0x454d45524f4e4749ULL // 'IGNOREME' + #define GPT_HEADER_REVISION_V1 0x00010000 #define GPT_PRIMARY_PARTITION_TABLE_LBA 1ULL #define GPT_ENTRY_NUMBERS CONFIG_EFI_PARTITION_ENTRIES_NUMBERS
Hi Urja,
On Wed, 3 Apr 2019 at 06:25, Urja Rannikko urjaman@gmail.com wrote:
Some ChromeOS devices (atleast veyron speedy) have the first 8MiB of the eMMC write protected and equipped with a dummy 'IGNOREME' GPT header - instead of spewing error messages about it, just silently try the backup GPT.
Note: this does not touch the gpt cmd writing/verifying functions, those will still complain.
Signed-off-by: Urja Rannikko urjaman@gmail.com
disk/part_efi.c | 28 +++++++++++++++++++++------- include/part_efi.h | 2 ++ 2 files changed, 23 insertions(+), 7 deletions(-)
Could we add a Kconfig to control this? Other devices shouldn't have this code.
Regards, SImon
Hi Simon,
On Wed, Apr 24, 2019 at 3:54 AM Simon Glass sjg@chromium.org wrote:
Hi Urja,
<snip>
Could we add a Kconfig to control this? Other devices shouldn't have this code.
I suppose i can do that... but I will need to know all the devices that do this - I'd assume atleast jerry, minnie and speedy but maybe also rk3399 chromeos devices?
I'm thinking basically an ifdef around the check in is_gpt_valid and let gcc optimize the rest of the code (test for 2) out, and then select the Kconfig (ummm maybe CONFIG_IGNORE_CROS_IGNOREME_GPT or do you have better ideas?) for the appropriate boards.
Hi Urja,
On Fri, 26 Apr 2019 at 04:26, Urja Rannikko urjaman@gmail.com wrote:
Hi Simon,
On Wed, Apr 24, 2019 at 3:54 AM Simon Glass sjg@chromium.org wrote:
Hi Urja,
<snip> > > Could we add a Kconfig to control this? Other devices shouldn't have this code. I suppose i can do that... but I will need to know all the devices that do this - I'd assume atleast jerry, minnie and speedy but maybe also rk3399 chromeos devices?
I'm thinking basically an ifdef around the check in is_gpt_valid and let gcc optimize the rest of the code (test for 2) out, and then
How about a new 'SUPPORT_CHROMEOS' Kconfig which implies your new CONFIG below? You could enable it on configs/*chrome*
See if you can use
if (IS_ENABLED(CONFIG_xx))
instead of #ifdef.
select the Kconfig (ummm maybe CONFIG_IGNORE_CROS_IGNOREME_GPT or do
Maybe just CONFIG_CROS_GPT.
you have better ideas?) for the appropriate boards.
-- Urja Rannikko
Regards, Simon
On Wed, 3 Apr 2019 at 06:25, Urja Rannikko urjaman@gmail.com wrote:
There were 3 copies of the same sequence, make it into a function.
Signed-off-by: Urja Rannikko urjaman@gmail.com
disk/part_efi.c | 73 +++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 39 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Hi,
On Wed, Apr 24, 2019 at 3:55 AM Simon Glass sjg@chromium.org wrote:
On Wed, 3 Apr 2019 at 06:25, Urja Rannikko urjaman@gmail.com wrote:
There were 3 copies of the same sequence, make it into a function.
Signed-off-by: Urja Rannikko urjaman@gmail.com
disk/part_efi.c | 73 +++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 39 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Thank you, I'd just like to point out that you replied to the v1 of this series, though v2 is identical in content (just rebased).
The actual patch that should apply is this one: http://patchwork.ozlabs.org/patch/1084276/
participants (2)
-
Simon Glass -
Urja Rannikko