[U-Boot] [PATCH V2 1/9] disk: part_efi: remove indent level from loop
From: Stephen Warren swarren@nvidia.com
Simplify the partition printing loop in print_part_efi() to bail out early when the first invalid partition is found, rather than indenting the whole body of the loop. This simplifies later patches.
Signed-off-by: Stephen Warren swarren@nvidia.com --- v2: Rebased series on top of the bug-fixes I sent for the release.
disk/part_efi.c | 17 ++++++++--------- 1 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 264ea9c..008177e 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -138,15 +138,14 @@ void print_part_efi(block_dev_desc_t * dev_desc)
printf("Part\tName\t\t\tStart LBA\tEnd LBA\n"); for (i = 0; i < le32_to_int(gpt_head->num_partition_entries); i++) { + /* Stop at the first non valid PTE */ + if (!is_pte_valid(&gpt_pte[i])) + break;
- if (is_pte_valid(&gpt_pte[i])) { - printf("%3d\t%-18s\t0x%08llX\t0x%08llX\n", (i + 1), - print_efiname(&gpt_pte[i]), - le64_to_int(gpt_pte[i].starting_lba), - le64_to_int(gpt_pte[i].ending_lba)); - } else { - break; /* Stop at the first non valid PTE */ - } + printf("%3d\t%-18s\t0x%08llX\t0x%08llX\n", (i + 1), + print_efiname(&gpt_pte[i]), + le64_to_int(gpt_pte[i].starting_lba), + le64_to_int(gpt_pte[i].ending_lba)); }
/* Remember to free pte */
From: Stephen Warren swarren@nvidia.com
The partition name is a long variable-length string. Move it last on the line to ensure consistent layout and that the entries align with the "header" line. Also, surround it in quotes, so if it's empty, it's obvious that something is still being printed.
Also, change the case of the LBA numbers; lower-case looks nicer in my opinion, and will be more consistent with the UUID printing that is added later in this series.
Signed-off-by: Stephen Warren swarren@nvidia.com --- disk/part_efi.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 008177e..b6b2bf5 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -136,16 +136,16 @@ void print_part_efi(block_dev_desc_t * dev_desc)
debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
- printf("Part\tName\t\t\tStart LBA\tEnd LBA\n"); + printf("Part\tStart LBA\tEnd LBA\t\tName\n"); for (i = 0; i < le32_to_int(gpt_head->num_partition_entries); i++) { /* Stop at the first non valid PTE */ if (!is_pte_valid(&gpt_pte[i])) break;
- printf("%3d\t%-18s\t0x%08llX\t0x%08llX\n", (i + 1), - print_efiname(&gpt_pte[i]), + printf("%3d\t0x%08llx\t0x%08llx\t"%s"\n", (i + 1), le64_to_int(gpt_pte[i].starting_lba), - le64_to_int(gpt_pte[i].ending_lba)); + le64_to_int(gpt_pte[i].ending_lba), + print_efiname(&gpt_pte[i])); }
/* Remember to free pte */
From: Stephen Warren swarren@nvidia.com
When printing the partition table, print the partition type UUID and the individual partition UUID. Do this unconditionally, since partition UUIDs are useful.
Signed-off-by: Stephen Warren swarren@nvidia.com --- disk/part_efi.c | 50 ++++++++++++++++++++++++++++---------------------- 1 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index b6b2bf5..6b80cd9 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -113,6 +113,26 @@ static char *print_efiname(gpt_entry *pte) return name; }
+static void uuid_string(unsigned char *uuid, char *str) +{ + static const u8 le[16] = {3, 2, 1, 0, 5, 4, 7, 6, 8, 9, 10, 11, + 12, 13, 14, 15}; + int i; + + for (i = 0; i < 16; i++) { + sprintf(str, "%02x", uuid[le[i]]); + str += 2; + switch (i) { + case 3: + case 5: + case 7: + case 9: + *str++ = '-'; + break; + } + } +} + /* * Public Functions (include/part.h) */ @@ -122,6 +142,7 @@ void print_part_efi(block_dev_desc_t * dev_desc) ALLOC_CACHE_ALIGN_BUFFER(gpt_header, gpt_head, 1); gpt_entry *gpt_pte = NULL; int i = 0; + char uuid[37];
if (!dev_desc) { printf("%s: Invalid Argument(s)\n", __func__); @@ -137,6 +158,9 @@ void print_part_efi(block_dev_desc_t * dev_desc) debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
printf("Part\tStart LBA\tEnd LBA\t\tName\n"); + printf("\tType UUID\n"); + printf("\tPartition UUID\n"); + for (i = 0; i < le32_to_int(gpt_head->num_partition_entries); i++) { /* Stop at the first non valid PTE */ if (!is_pte_valid(&gpt_pte[i])) @@ -146,6 +170,10 @@ void print_part_efi(block_dev_desc_t * dev_desc) le64_to_int(gpt_pte[i].starting_lba), le64_to_int(gpt_pte[i].ending_lba), print_efiname(&gpt_pte[i])); + uuid_string(gpt_pte[i].partition_type_guid.b, uuid); + printf("\ttype:\t%s\n", uuid); + uuid_string(gpt_pte[i].unique_partition_guid.b, uuid); + printf("\tuuid:\t%s\n", uuid); }
/* Remember to free pte */ @@ -153,28 +181,6 @@ void print_part_efi(block_dev_desc_t * dev_desc) return; }
-#ifdef CONFIG_PARTITION_UUIDS -static void uuid_string(unsigned char *uuid, char *str) -{ - static const u8 le[16] = {3, 2, 1, 0, 5, 4, 7, 6, 8, 9, 10, 11, - 12, 13, 14, 15}; - int i; - - for (i = 0; i < 16; i++) { - sprintf(str, "%02x", uuid[le[i]]); - str += 2; - switch (i) { - case 3: - case 5: - case 7: - case 9: - *str++ = '-'; - break; - } - } -} -#endif - int get_partition_info_efi(block_dev_desc_t * dev_desc, int part, disk_partition_t * info) {
From: Stephen Warren swarren@nvidia.com
Add no_block_io_protocol and legacy_bios_bootable attribute definitions. These are sourced from UEFI Spec 2.3, page 105, table 19. Credits to the libparted source for the specification pointer.
Signed-off-by: Stephen Warren swarren@nvidia.com --- disk/part_efi.h | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/disk/part_efi.h b/disk/part_efi.h index 5903e7c..1d0c67c 100644 --- a/disk/part_efi.h +++ b/disk/part_efi.h @@ -113,7 +113,9 @@ typedef struct _gpt_header {
typedef struct _gpt_entry_attributes { unsigned long long required_to_function:1; - unsigned long long reserved:47; + unsigned long long no_block_io_protocol:1; + unsigned long long legacy_bios_bootable:1; + unsigned long long reserved:45; unsigned long long type_guid_specific:16; } __attribute__ ((packed)) gpt_entry_attributes;
From: Stephen Warren swarren@nvidia.com
When printing the EFI partition table, print the raw attributes. Convert struct gpt_entry_attributes to a union to allow raw access.
Signed-off-by: Stephen Warren swarren@nvidia.com --- disk/part_efi.c | 2 ++ disk/part_efi.h | 15 +++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index 6b80cd9..d563509 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -158,6 +158,7 @@ void print_part_efi(block_dev_desc_t * dev_desc) debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
printf("Part\tStart LBA\tEnd LBA\t\tName\n"); + printf("\tAttributes\n"); printf("\tType UUID\n"); printf("\tPartition UUID\n");
@@ -170,6 +171,7 @@ void print_part_efi(block_dev_desc_t * dev_desc) le64_to_int(gpt_pte[i].starting_lba), le64_to_int(gpt_pte[i].ending_lba), print_efiname(&gpt_pte[i])); + printf("\tattrs:\t0x%016llx\n", gpt_pte[i].attributes.raw); uuid_string(gpt_pte[i].partition_type_guid.b, uuid); printf("\ttype:\t%s\n", uuid); uuid_string(gpt_pte[i].unique_partition_guid.b, uuid); diff --git a/disk/part_efi.h b/disk/part_efi.h index 1d0c67c..4e28d1d 100644 --- a/disk/part_efi.h +++ b/disk/part_efi.h @@ -111,12 +111,15 @@ typedef struct _gpt_header { unsigned char reserved2[GPT_BLOCK_SIZE - 92]; } __attribute__ ((packed)) gpt_header;
-typedef struct _gpt_entry_attributes { - unsigned long long required_to_function:1; - unsigned long long no_block_io_protocol:1; - unsigned long long legacy_bios_bootable:1; - unsigned long long reserved:45; - unsigned long long type_guid_specific:16; +typedef union _gpt_entry_attributes { + struct { + unsigned long long required_to_function:1; + unsigned long long no_block_io_protocol:1; + unsigned long long legacy_bios_bootable:1; + unsigned long long reserved:45; + unsigned long long type_guid_specific:16; + } fields; + unsigned long long raw; } __attribute__ ((packed)) gpt_entry_attributes;
#define PARTNAME_SZ (72 / sizeof(efi_char16_t))
From: Stephen Warren swarren@nvidia.com
A partition is considered bootable if it either has the "legacy BIOS bootable" flag set, or if the partition type UUID matches the standard "system" type.
Signed-off-by: Stephen Warren swarren@nvidia.com --- disk/part_efi.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c index d563509..7a39d52 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -133,6 +133,15 @@ static void uuid_string(unsigned char *uuid, char *str) } }
+static efi_guid_t system_guid = PARTITION_SYSTEM_GUID; + +static inline int is_bootable(gpt_entry *p) +{ + return p->attributes.fields.legacy_bios_bootable || + !memcmp(&(p->partition_type_guid), &system_guid, + sizeof(efi_guid_t)); +} + /* * Public Functions (include/part.h) */ @@ -219,6 +228,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part, sprintf((char *)info->name, "%s", print_efiname(&gpt_pte[part - 1])); sprintf((char *)info->type, "U-Boot"); + info->bootable = is_bootable(&gpt_pte[part - 1]); #ifdef CONFIG_PARTITION_UUIDS uuid_string(gpt_pte[part - 1].unique_partition_guid.b, info->uuid); #endif
From: Stephen Warren swarren@nvidia.com
Minor cleanups required so later patches don't trigger checkpatch.
Signed-off-by: Stephen Warren swarren@nvidia.com --- disk/part_dos.c | 21 +++++++++++---------- 1 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/disk/part_dos.c b/disk/part_dos.c index 5c454e6..513a54a 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -65,7 +65,8 @@ static inline int is_bootable(dos_partition_t *p) return p->boot_ind == 0x80; }
-static void print_one_part (dos_partition_t *p, int ext_part_sector, int part_num) +static void print_one_part(dos_partition_t *p, int ext_part_sector, + int part_num) { int lba_start = ext_part_sector + le32_to_int (p->start4); int lba_size = le32_to_int (p->size4); @@ -105,8 +106,9 @@ int test_part_dos (block_dev_desc_t *dev_desc)
/* Print a partition that is relative to its Extended partition table */ -static void print_partition_extended (block_dev_desc_t *dev_desc, int ext_part_sector, int relative, - int part_num) +static void print_partition_extended(block_dev_desc_t *dev_desc, + int ext_part_sector, int relative, + int part_num) { ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz); dos_partition_t *pt; @@ -135,7 +137,7 @@ static void print_partition_extended (block_dev_desc_t *dev_desc, int ext_part_s
if ((pt->sys_ind != 0) && (ext_part_sector == 0 || !is_extended (pt->sys_ind)) ) { - print_one_part (pt, ext_part_sector, part_num); + print_one_part(pt, ext_part_sector, part_num); }
/* Reverse engr the fdisk part# assignment rule! */ @@ -151,10 +153,9 @@ static void print_partition_extended (block_dev_desc_t *dev_desc, int ext_part_s if (is_extended (pt->sys_ind)) { int lba_start = le32_to_int (pt->start4) + relative;
- print_partition_extended (dev_desc, lba_start, - ext_part_sector == 0 ? lba_start - : relative, - part_num); + print_partition_extended(dev_desc, lba_start, + ext_part_sector == 0 ? lba_start : relative, + part_num); } }
@@ -261,8 +262,8 @@ static int get_partition_info_extended (block_dev_desc_t *dev_desc, int ext_part
void print_part_dos (block_dev_desc_t *dev_desc) { - printf ("Partition Start Sector Num Sectors Type\n"); - print_partition_extended (dev_desc, 0, 0, 1); + printf("Partition Start Sector Num Sectors Type\n"); + print_partition_extended(dev_desc, 0, 0, 1); }
int get_partition_info_dos (block_dev_desc_t *dev_desc, int part, disk_partition_t * info)
From: Stephen Warren swarren@nvidia.com
When printing the partition table, make sure data aligns with the column headers. Change format of partition number field to %3d to match part_efi.c.
Signed-off-by: Stephen Warren swarren@nvidia.com --- disk/part_dos.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/disk/part_dos.c b/disk/part_dos.c index 513a54a..732187a 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -71,7 +71,7 @@ static void print_one_part(dos_partition_t *p, int ext_part_sector, int lba_start = ext_part_sector + le32_to_int (p->start4); int lba_size = le32_to_int (p->size4);
- printf("%5d\t\t%10d\t%10d\t%2x%s%s\n", + printf("%3d\t%-10d\t%-10d\t%02x%s%s\n", part_num, lba_start, lba_size, p->sys_ind, (is_extended(p->sys_ind) ? " Extd" : ""), (is_bootable(p) ? " Boot" : "")); @@ -262,7 +262,7 @@ static int get_partition_info_extended (block_dev_desc_t *dev_desc, int ext_part
void print_part_dos (block_dev_desc_t *dev_desc) { - printf("Partition Start Sector Num Sectors Type\n"); + printf("Part\tStart Sector\tNum Sectors\tType\n"); print_partition_extended(dev_desc, 0, 0, 1); }
From: Stephen Warren swarren@nvidia.com
This information may be useful to compare against command "part uuid", or if you want to manually paste the information into the kernel command-line.
Signed-off-by: Stephen Warren swarren@nvidia.com --- disk/part_dos.c | 19 +++++++++++-------- 1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/disk/part_dos.c b/disk/part_dos.c index 732187a..3fe901b 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -66,13 +66,13 @@ static inline int is_bootable(dos_partition_t *p) }
static void print_one_part(dos_partition_t *p, int ext_part_sector, - int part_num) + int part_num, unsigned int disksig) { int lba_start = ext_part_sector + le32_to_int (p->start4); int lba_size = le32_to_int (p->size4);
- printf("%3d\t%-10d\t%-10d\t%02x%s%s\n", - part_num, lba_start, lba_size, p->sys_ind, + printf("%3d\t%-10d\t%-10d\t%08x-%02x\t%02x%s%s\n", + part_num, lba_start, lba_size, disksig, part_num, p->sys_ind, (is_extended(p->sys_ind) ? " Extd" : ""), (is_bootable(p) ? " Boot" : "")); } @@ -108,7 +108,7 @@ int test_part_dos (block_dev_desc_t *dev_desc) */ static void print_partition_extended(block_dev_desc_t *dev_desc, int ext_part_sector, int relative, - int part_num) + int part_num, unsigned int disksig) { ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz); dos_partition_t *pt; @@ -127,6 +127,9 @@ static void print_partition_extended(block_dev_desc_t *dev_desc, return; }
+ if (!ext_part_sector) + disksig = le32_to_int(&buffer[DOS_PART_DISKSIG_OFFSET]); + /* Print all primary/logical partitions */ pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET); for (i = 0; i < 4; i++, pt++) { @@ -137,7 +140,7 @@ static void print_partition_extended(block_dev_desc_t *dev_desc,
if ((pt->sys_ind != 0) && (ext_part_sector == 0 || !is_extended (pt->sys_ind)) ) { - print_one_part(pt, ext_part_sector, part_num); + print_one_part(pt, ext_part_sector, part_num, disksig); }
/* Reverse engr the fdisk part# assignment rule! */ @@ -155,7 +158,7 @@ static void print_partition_extended(block_dev_desc_t *dev_desc,
print_partition_extended(dev_desc, lba_start, ext_part_sector == 0 ? lba_start : relative, - part_num); + part_num, disksig); } }
@@ -262,8 +265,8 @@ static int get_partition_info_extended (block_dev_desc_t *dev_desc, int ext_part
void print_part_dos (block_dev_desc_t *dev_desc) { - printf("Part\tStart Sector\tNum Sectors\tType\n"); - print_partition_extended(dev_desc, 0, 0, 1); + printf("Part\tStart Sector\tNum Sectors\tUUID\t\tType\n"); + print_partition_extended(dev_desc, 0, 0, 1, 0); }
int get_partition_info_dos (block_dev_desc_t *dev_desc, int part, disk_partition_t * info)
On Mon, Oct 08, 2012 at 08:14:32AM -0000, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Simplify the partition printing loop in print_part_efi() to bail out early when the first invalid partition is found, rather than indenting the whole body of the loop. This simplifies later patches.
Signed-off-by: Stephen Warren swarren@nvidia.com
This, along with the rest of the series have been applied to u-boot/master, thanks! Note that for 9/9 I had to rework it slightly to take the new output formatting, can you please double check? Thanks!
On 10/17/2012 09:01 AM, Tom Rini wrote:
On Mon, Oct 08, 2012 at 08:14:32AM -0000, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Simplify the partition printing loop in print_part_efi() to bail out early when the first invalid partition is found, rather than indenting the whole body of the loop. This simplifies later patches.
Signed-off-by: Stephen Warren swarren@nvidia.com
This, along with the rest of the series have been applied to u-boot/master, thanks! Note that for 9/9 I had to rework it slightly to take the new output formatting, can you please double check? Thanks!
Thanks. The results looks fine; there is zero diff between my local branch and u-boot/master on the files that series touched.
I wonder if the rework you needed to do was because patch 8/9 wasn't applied? I did do some cleanup of the printf strings in that patch separately, but there isn't an equivalent commit in your tree; it's as if 8/9 and 9/9 were squashed into one.
BTW, I had also posted a minor fix "disk: initialize name/part fields when returning a whole disk" which may have been overlooked? Perhaps it's still in your queue.
Thanks.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 10/17/12 09:04, Stephen Warren wrote:
On 10/17/2012 09:01 AM, Tom Rini wrote:
On Mon, Oct 08, 2012 at 08:14:32AM -0000, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Simplify the partition printing loop in print_part_efi() to bail out early when the first invalid partition is found, rather than indenting the whole body of the loop. This simplifies later patches.
Signed-off-by: Stephen Warren swarren@nvidia.com
This, along with the rest of the series have been applied to u-boot/master, thanks! Note that for 9/9 I had to rework it slightly to take the new output formatting, can you please double check? Thanks!
Thanks. The results looks fine; there is zero diff between my local branch and u-boot/master on the files that series touched.
I wonder if the rework you needed to do was because patch 8/9 wasn't applied? I did do some cleanup of the printf strings in that patch separately, but there isn't an equivalent commit in your tree; it's as if 8/9 and 9/9 were squashed into one.
There we go, I just missed 8/9 (I see it in my patchwork TODO list now).
BTW, I had also posted a minor fix "disk: initialize name/part fields when returning a whole disk" which may have been overlooked? Perhaps it's still in your queue.
That is still in my queue. I'm trying to leave a buffer around when patches are posted vs when I pull them in to allow for more review by the folks doing U-Boot in their spare time. I should pick up that as well as the fat_set_blk_dev series soon.
- -- Tom
On 10/17/2012 10:12 AM, Tom Rini wrote:
On 10/17/12 09:04, Stephen Warren wrote:
...
BTW, I had also posted a minor fix "disk: initialize name/part fields when returning a whole disk" which may have been overlooked? Perhaps it's still in your queue.
That is still in my queue. I'm trying to leave a buffer around when patches are posted vs when I pull them in to allow for more review by the folks doing U-Boot in their spare time.
OK, makes sense.
I should pick up that as well as the fat_set_blk_dev series soon.
Pavel had some comments on that series; I might rework it to drop the first two patches and replace them with something that simply removes the partition number from the printf. Feel free to weigh in on that thread if you have an opinion which way to go.
participants (2)
-
Stephen Warren -
Tom Rini