disk: part_efi: add support for the Alternate GPT

Message ID 1398810703-29467-1-git-send-email-srae@broadcom.com
State New
Headers show

Commit Message

Steve Rae April 29, 2014, 10:31 p.m.
Check the Alternate GPT table if the Primary GPT table is invalid.

Signed-off-by: Steve Rae <srae@broadcom.com>
---

 disk/part_efi.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Comments

Przemyslaw Marczak May 5, 2014, 10:50 a.m. | #1
Hello Steve,

On 04/30/2014 12:31 AM, Steve Rae wrote:
> Check the Alternate GPT table if the Primary GPT table is invalid.
>
> Signed-off-by: Steve Rae <srae@broadcom.com>
> ---
>
>   disk/part_efi.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index 216a292..ad899fa 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -93,7 +93,15 @@ void print_part_efi(block_dev_desc_t * dev_desc)
>   	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
>   			 gpt_head, &gpt_pte) != 1) {
>   		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
> -		return;
> +		if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
> +				 gpt_head, &gpt_pte) != 1) {
> +			printf("%s: *** ERROR: Invalid Alternate GPT ***\n",
> +			       __func__);
> +			return;
> +		} else {
> +			printf("%s: ***        Using Alternate GPT ***\n",
> +			       __func__);
> +		}
>   	}
>
>   	debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
> @@ -142,7 +150,15 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
>   	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
>   			gpt_head, &gpt_pte) != 1) {
>   		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
> -		return -1;
> +		if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
> +				 gpt_head, &gpt_pte) != 1) {
> +			printf("%s: *** ERROR: Invalid Alternate GPT ***\n",
> +			       __func__);
> +			return -1;
> +		} else {
> +			printf("%s: ***        Using Alternate GPT ***\n",
> +			       __func__);
> +		}
>   	}
>
>   	if (part > le32_to_cpu(gpt_head->num_partition_entries) ||
>

This looks ok, we can still use u-boot filesystem commands after gpt 
header block corruption, but the gpt is still...corrupted.

Maybe we need additional gpt subcommand for restore gpt in case when 
backup of gpt header is valid?

Actually we have only "gpt write" command but it needs proper 
environment variables. So I think that "gpt restore" is welcome.

Thank you,
Steve Rae May 5, 2014, 4:12 p.m. | #2
On 14-05-05 03:50 AM, Przemyslaw Marczak wrote:
> Hello Steve,
>
> On 04/30/2014 12:31 AM, Steve Rae wrote:
>> Check the Alternate GPT table if the Primary GPT table is invalid.
>>
>> Signed-off-by: Steve Rae <srae@broadcom.com>
>> ---
>>
>>   disk/part_efi.c | 20 ++++++++++++++++++--
>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>> index 216a292..ad899fa 100644
>> --- a/disk/part_efi.c
>> +++ b/disk/part_efi.c
>> @@ -93,7 +93,15 @@ void print_part_efi(block_dev_desc_t * dev_desc)
>>       if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
>>                gpt_head, &gpt_pte) != 1) {
>>           printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
>> -        return;
>> +        if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
>> +                 gpt_head, &gpt_pte) != 1) {
>> +            printf("%s: *** ERROR: Invalid Alternate GPT ***\n",
>> +                   __func__);
>> +            return;
>> +        } else {
>> +            printf("%s: ***        Using Alternate GPT ***\n",
>> +                   __func__);
>> +        }
>>       }
>>
>>       debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
>> @@ -142,7 +150,15 @@ int get_partition_info_efi(block_dev_desc_t * 
>> dev_desc, int part,
>>       if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
>>               gpt_head, &gpt_pte) != 1) {
>>           printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
>> -        return -1;
>> +        if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
>> +                 gpt_head, &gpt_pte) != 1) {
>> +            printf("%s: *** ERROR: Invalid Alternate GPT ***\n",
>> +                   __func__);
>> +            return -1;
>> +        } else {
>> +            printf("%s: ***        Using Alternate GPT ***\n",
>> +                   __func__);
>> +        }
>>       }
>>
>>       if (part > le32_to_cpu(gpt_head->num_partition_entries) ||
>>
>
> This looks ok, we can still use u-boot filesystem commands after gpt 
> header block corruption, but the gpt is still...corrupted.
>
> Maybe we need additional gpt subcommand for restore gpt in case when 
> backup of gpt header is valid?
>
> Actually we have only "gpt write" command but it needs proper 
> environment variables. So I think that "gpt restore" is welcome.
>
> Thank you,
Agree -- maybe I can find time to work on a "gpt restore" in the near 
future....
Thanks, Steve
Steve Rae May 5, 2014, 6:26 p.m. | #3
Hmmm - perhaps "Secondary" would be more accurate wording than "Alternate"
(v2 coming...)

On 14-05-05 09:12 AM, Steve Rae wrote:
>
> On 14-05-05 03:50 AM, Przemyslaw Marczak wrote:
>> Hello Steve,
>>
>> On 04/30/2014 12:31 AM, Steve Rae wrote:
>>> Check the Alternate GPT table if the Primary GPT table is invalid.
>>>
>>> Signed-off-by: Steve Rae <srae@broadcom.com>
>>> ---
>>>
>>>   disk/part_efi.c | 20 ++++++++++++++++++--
>>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>>> index 216a292..ad899fa 100644
>>> --- a/disk/part_efi.c
>>> +++ b/disk/part_efi.c
>>> @@ -93,7 +93,15 @@ void print_part_efi(block_dev_desc_t * dev_desc)
>>>       if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
>>>                gpt_head, &gpt_pte) != 1) {
>>>           printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
>>> -        return;
>>> +        if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
>>> +                 gpt_head, &gpt_pte) != 1) {
>>> +            printf("%s: *** ERROR: Invalid Alternate GPT ***\n",
>>> +                   __func__);
>>> +            return;
>>> +        } else {
>>> +            printf("%s: ***        Using Alternate GPT ***\n",
>>> +                   __func__);
>>> +        }
>>>       }
>>>
>>>       debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
>>> @@ -142,7 +150,15 @@ int get_partition_info_efi(block_dev_desc_t * 
>>> dev_desc, int part,
>>>       if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
>>>               gpt_head, &gpt_pte) != 1) {
>>>           printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
>>> -        return -1;
>>> +        if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
>>> +                 gpt_head, &gpt_pte) != 1) {
>>> +            printf("%s: *** ERROR: Invalid Alternate GPT ***\n",
>>> +                   __func__);
>>> +            return -1;
>>> +        } else {
>>> +            printf("%s: ***        Using Alternate GPT ***\n",
>>> +                   __func__);
>>> +        }
>>>       }
>>>
>>>       if (part > le32_to_cpu(gpt_head->num_partition_entries) ||
>>>
>>
>> This looks ok, we can still use u-boot filesystem commands after gpt 
>> header block corruption, but the gpt is still...corrupted.
>>
>> Maybe we need additional gpt subcommand for restore gpt in case when 
>> backup of gpt header is valid?
>>
>> Actually we have only "gpt write" command but it needs proper 
>> environment variables. So I think that "gpt restore" is welcome.
>>
>> Thank you,
> Agree -- maybe I can find time to work on a "gpt restore" in the near 
> future....
> Thanks, Steve
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
Tom Rini May 5, 2014, 6:43 p.m. | #4
On Mon, May 05, 2014 at 11:26:44AM -0700, Steve Rae wrote:

> Hmmm - perhaps "Secondary" would be more accurate wording than "Alternate"
> (v2 coming...)

Is there any "official" name for this in the GPT spec?
Steve Rae May 5, 2014, 7:22 p.m. | #5
On 14-05-05 11:43 AM, Tom Rini wrote:
> On Mon, May 05, 2014 at 11:26:44AM -0700, Steve Rae wrote:
>
>> Hmmm - perhaps "Secondary" would be more accurate wording than "Alternate"
>> (v2 coming...)
> Is there any "official" name for this in the GPT spec?
>
I proposed "Secondary" because it matched "doc/README.gpt"; and it is 
often called "secondary GPT" (for example: 
http://en.wikipedia.org/wiki/GUID_Partition_Table)
However, the UEFI Specifaction (Version 2.3.1, Errata A) calls it 
"backup GPT" 18 times, and "alternate GPT" once (and never calls it 
"secondary GPT")....
Tom Rini May 5, 2014, 7:41 p.m. | #6
On Mon, May 05, 2014 at 12:22:18PM -0700, Steve Rae wrote:
> 
> On 14-05-05 11:43 AM, Tom Rini wrote:
> >On Mon, May 05, 2014 at 11:26:44AM -0700, Steve Rae wrote:
> >
> >>Hmmm - perhaps "Secondary" would be more accurate wording than "Alternate"
> >>(v2 coming...)
> >Is there any "official" name for this in the GPT spec?
> >
> I proposed "Secondary" because it matched "doc/README.gpt"; and it
> is often called "secondary GPT" (for example:
> http://en.wikipedia.org/wiki/GUID_Partition_Table)
> However, the UEFI Specifaction (Version 2.3.1, Errata A) calls it
> "backup GPT" 18 times, and "alternate GPT" once (and never calls it
> "secondary GPT")....

I would go with backup GPT and update the README.gpt too please.
Steve Rae May 5, 2014, 7:54 p.m. | #7
On 14-05-05 12:41 PM, Tom Rini wrote:
> On Mon, May 05, 2014 at 12:22:18PM -0700, Steve Rae wrote:
>> On 14-05-05 11:43 AM, Tom Rini wrote:
>>> On Mon, May 05, 2014 at 11:26:44AM -0700, Steve Rae wrote:
>>>
>>>> Hmmm - perhaps "Secondary" would be more accurate wording than "Alternate"
>>>> (v2 coming...)
>>> Is there any "official" name for this in the GPT spec?
>>>
>> I proposed "Secondary" because it matched "doc/README.gpt"; and it
>> is often called "secondary GPT" (for example:
>> http://en.wikipedia.org/wiki/GUID_Partition_Table)
>> However, the UEFI Specifaction (Version 2.3.1, Errata A) calls it
>> "backup GPT" 18 times, and "alternate GPT" once (and never calls it
>> "secondary GPT")....
> I would go with backup GPT and update the README.gpt too please.
>
OK (v3 coming...)

Patch

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 216a292..ad899fa 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -93,7 +93,15 @@  void print_part_efi(block_dev_desc_t * dev_desc)
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
 			 gpt_head, &gpt_pte) != 1) {
 		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
-		return;
+		if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
+				 gpt_head, &gpt_pte) != 1) {
+			printf("%s: *** ERROR: Invalid Alternate GPT ***\n",
+			       __func__);
+			return;
+		} else {
+			printf("%s: ***        Using Alternate GPT ***\n",
+			       __func__);
+		}
 	}
 
 	debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
@@ -142,7 +150,15 @@  int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
 			gpt_head, &gpt_pte) != 1) {
 		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
-		return -1;
+		if (is_gpt_valid(dev_desc, (dev_desc->lba - 1),
+				 gpt_head, &gpt_pte) != 1) {
+			printf("%s: *** ERROR: Invalid Alternate GPT ***\n",
+			       __func__);
+			return -1;
+		} else {
+			printf("%s: ***        Using Alternate GPT ***\n",
+			       __func__);
+		}
 	}
 
 	if (part > le32_to_cpu(gpt_head->num_partition_entries) ||