diff mbox series

[v5,4/6] partitions/efi: Support GPT entry lookup at a non-standard location

Message ID 20200516153644.13748-5-digetx@gmail.com
State New
Headers show
Series Introduce NVIDIA Tegra Partition Table | expand

Commit Message

Dmitry Osipenko May 16, 2020, 3:36 p.m. UTC
Most of consumer-grade NVIDIA Tegra devices use a proprietary bootloader
that can't be easily replaced because it's locked down using Secure Boot
cryptography signing and the crypto keys aren't given to a device owner.
These devices usually have eMMC storage that is partitioned using a custom
NVIDIA Tegra partition table format.  Of course bootloader and other
"special things" are stored on the eMMC storage, and thus, the partition
format can't be changed.

The Tegra partition format has been reverse-engineered, but NVIDIA uses
a virtually merged "boot" and "main" eMMC HW partitions in theirs
bootloader. This is not supported by Linux kernel and can't be easily
implemented. Hence partition table entry isn't accessible by kernel if
it's located at the "boot" eMMC partition. Luckily this is a rare case in
practice and even if it's the case, likely that the proprietary bootloader
will supply kernel with a non-standard gpt_sector= cmdline option.  This
gpt_sector= argument points at a GPT entry which resides at a non-standard
location on the eMMC storage.

This patch allows to support the non-standard cmdline option for NVIDIA
Tegra devices without bothering any other platforms.  The force_gpt_sector
parser-state struct member should be set before invoking efi_partition()
and it should be unset after the invocation completion.  This new parser
member instructs GPT parser to look up GPT entry at the given sector if
state->force_gpt_sector != 0.

This patch is based on the original work done by Colin Cross for the
downstream Android kernel.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 block/partitions/check.h | 1 +
 block/partitions/efi.c   | 8 ++++++++
 2 files changed, 9 insertions(+)

Comments

Randy Dunlap May 16, 2020, 3:51 p.m. UTC | #1
On 5/16/20 8:36 AM, Dmitry Osipenko wrote:
> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
> index b64bfdd4326c..3af4660bc11f 100644
> --- a/block/partitions/efi.c
> +++ b/block/partitions/efi.c
> @@ -621,6 +621,14 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>          if (!good_agpt && force_gpt)
>                  good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>  
> +	/* The force_gpt_sector is used by NVIDIA Tegra partition parser in
> +	 * order to convey a non-standard location of the GPT entry for lookup.
> +	 * By default force_gpt_sector is set to 0 and has no effect.
> +	 */

Please fix the multi-line comment format as described in
Documentation/process/coding-style.rst.

> +	if (!good_agpt && force_gpt && state->force_gpt_sector)
> +		good_agpt = is_gpt_valid(state, state->force_gpt_sector,
> +					 &agpt, &aptes);
> +
>          /* The obviously unsuccessful case */
>          if (!good_pgpt && !good_agpt)
>                  goto fail;

thanks.
Dmitry Osipenko May 16, 2020, 4:50 p.m. UTC | #2
16.05.2020 18:51, Randy Dunlap пишет:
> On 5/16/20 8:36 AM, Dmitry Osipenko wrote:
>> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
>> index b64bfdd4326c..3af4660bc11f 100644
>> --- a/block/partitions/efi.c
>> +++ b/block/partitions/efi.c
>> @@ -621,6 +621,14 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>>          if (!good_agpt && force_gpt)
>>                  good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>>  
>> +	/* The force_gpt_sector is used by NVIDIA Tegra partition parser in
>> +	 * order to convey a non-standard location of the GPT entry for lookup.
>> +	 * By default force_gpt_sector is set to 0 and has no effect.
>> +	 */
> 
> Please fix the multi-line comment format as described in
> Documentation/process/coding-style.rst.
> 
>> +	if (!good_agpt && force_gpt && state->force_gpt_sector)
>> +		good_agpt = is_gpt_valid(state, state->force_gpt_sector,
>> +					 &agpt, &aptes);
>> +
>>          /* The obviously unsuccessful case */
>>          if (!good_pgpt && !good_agpt)
>>                  goto fail;
> 
> thanks.
> 

Hello Randy,

I know that it's not a proper kernel-style formatting, but that's the
style used by the whole efi.c source code and I wanted to maintain the
same style, for consistency. Of course I can change to a proper style if
it's more desirable than the consistency. Thank you for the comment!
Randy Dunlap May 16, 2020, 4:58 p.m. UTC | #3
On 5/16/20 9:50 AM, Dmitry Osipenko wrote:
> 16.05.2020 18:51, Randy Dunlap пишет:
>> On 5/16/20 8:36 AM, Dmitry Osipenko wrote:
>>> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
>>> index b64bfdd4326c..3af4660bc11f 100644
>>> --- a/block/partitions/efi.c
>>> +++ b/block/partitions/efi.c
>>> @@ -621,6 +621,14 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>>>          if (!good_agpt && force_gpt)
>>>                  good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>>>  
>>> +	/* The force_gpt_sector is used by NVIDIA Tegra partition parser in
>>> +	 * order to convey a non-standard location of the GPT entry for lookup.
>>> +	 * By default force_gpt_sector is set to 0 and has no effect.
>>> +	 */
>>
>> Please fix the multi-line comment format as described in
>> Documentation/process/coding-style.rst.
>>
>>> +	if (!good_agpt && force_gpt && state->force_gpt_sector)
>>> +		good_agpt = is_gpt_valid(state, state->force_gpt_sector,
>>> +					 &agpt, &aptes);
>>> +
>>>          /* The obviously unsuccessful case */
>>>          if (!good_pgpt && !good_agpt)
>>>                  goto fail;
>>
>> thanks.
>>
> 
> Hello Randy,
> 
> I know that it's not a proper kernel-style formatting, but that's the
> style used by the whole efi.c source code and I wanted to maintain the
> same style, for consistency. Of course I can change to a proper style if
> it's more desirable than the consistency. Thank you for the comment!
> 

too bad. Sorry to hear that.
It should have been "fixed" much earlier.
It's probably too late now.
Dmitry Osipenko May 17, 2020, 12:11 a.m. UTC | #4
16.05.2020 19:58, Randy Dunlap пишет:
> On 5/16/20 9:50 AM, Dmitry Osipenko wrote:
>> 16.05.2020 18:51, Randy Dunlap пишет:
>>> On 5/16/20 8:36 AM, Dmitry Osipenko wrote:
>>>> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
>>>> index b64bfdd4326c..3af4660bc11f 100644
>>>> --- a/block/partitions/efi.c
>>>> +++ b/block/partitions/efi.c
>>>> @@ -621,6 +621,14 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>>>>          if (!good_agpt && force_gpt)
>>>>                  good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>>>>  
>>>> +	/* The force_gpt_sector is used by NVIDIA Tegra partition parser in
>>>> +	 * order to convey a non-standard location of the GPT entry for lookup.
>>>> +	 * By default force_gpt_sector is set to 0 and has no effect.
>>>> +	 */
>>>
>>> Please fix the multi-line comment format as described in
>>> Documentation/process/coding-style.rst.
>>>
>>>> +	if (!good_agpt && force_gpt && state->force_gpt_sector)
>>>> +		good_agpt = is_gpt_valid(state, state->force_gpt_sector,
>>>> +					 &agpt, &aptes);
>>>> +
>>>>          /* The obviously unsuccessful case */
>>>>          if (!good_pgpt && !good_agpt)
>>>>                  goto fail;
>>>
>>> thanks.
>>>
>>
>> Hello Randy,
>>
>> I know that it's not a proper kernel-style formatting, but that's the
>> style used by the whole efi.c source code and I wanted to maintain the
>> same style, for consistency. Of course I can change to a proper style if
>> it's more desirable than the consistency. Thank you for the comment!
>>
> 
> too bad. Sorry to hear that.
> It should have been "fixed" much earlier.
> It's probably too late now.

Actually, I now see that there is a mix of different comment styles in
the efi.c code. So it should be fine to use the proper style, I'll
change it in v6.

I don't think it's too late, it's never late to make a correction :)
There are some other coding style problems in the efi.c that won't hurt
to fix, I may take a look at fixing them later on.
Randy Dunlap May 17, 2020, 2:07 a.m. UTC | #5
On 5/16/20 5:11 PM, Dmitry Osipenko wrote:
> 16.05.2020 19:58, Randy Dunlap пишет:
>> On 5/16/20 9:50 AM, Dmitry Osipenko wrote:
>>> 16.05.2020 18:51, Randy Dunlap пишет:
>>>> On 5/16/20 8:36 AM, Dmitry Osipenko wrote:
>>>>> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
>>>>> index b64bfdd4326c..3af4660bc11f 100644
>>>>> --- a/block/partitions/efi.c
>>>>> +++ b/block/partitions/efi.c
>>>>> @@ -621,6 +621,14 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
>>>>>          if (!good_agpt && force_gpt)
>>>>>                  good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
>>>>>  
>>>>> +	/* The force_gpt_sector is used by NVIDIA Tegra partition parser in
>>>>> +	 * order to convey a non-standard location of the GPT entry for lookup.
>>>>> +	 * By default force_gpt_sector is set to 0 and has no effect.
>>>>> +	 */
>>>>
>>>> Please fix the multi-line comment format as described in
>>>> Documentation/process/coding-style.rst.
>>>>
>>>>> +	if (!good_agpt && force_gpt && state->force_gpt_sector)
>>>>> +		good_agpt = is_gpt_valid(state, state->force_gpt_sector,
>>>>> +					 &agpt, &aptes);
>>>>> +
>>>>>          /* The obviously unsuccessful case */
>>>>>          if (!good_pgpt && !good_agpt)
>>>>>                  goto fail;
>>>>
>>>> thanks.
>>>>
>>>
>>> Hello Randy,
>>>
>>> I know that it's not a proper kernel-style formatting, but that's the
>>> style used by the whole efi.c source code and I wanted to maintain the
>>> same style, for consistency. Of course I can change to a proper style if
>>> it's more desirable than the consistency. Thank you for the comment!
>>>
>>
>> too bad. Sorry to hear that.
>> It should have been "fixed" much earlier.
>> It's probably too late now.
> 
> Actually, I now see that there is a mix of different comment styles in
> the efi.c code. So it should be fine to use the proper style, I'll
> change it in v6.
> 
> I don't think it's too late, it's never late to make a correction :)
> There are some other coding style problems in the efi.c that won't hurt
> to fix, I may take a look at fixing them later on.
> 

OK, great. Thanks.
diff mbox series

Patch

diff --git a/block/partitions/check.h b/block/partitions/check.h
index ffa01cc4b0b0..55acf6340e5b 100644
--- a/block/partitions/check.h
+++ b/block/partitions/check.h
@@ -22,6 +22,7 @@  struct parsed_partitions {
 	int limit;
 	bool access_beyond_eod;
 	char *pp_buf;
+	sector_t force_gpt_sector;
 };
 
 typedef struct {
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index b64bfdd4326c..3af4660bc11f 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -621,6 +621,14 @@  static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
         if (!good_agpt && force_gpt)
                 good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
 
+	/* The force_gpt_sector is used by NVIDIA Tegra partition parser in
+	 * order to convey a non-standard location of the GPT entry for lookup.
+	 * By default force_gpt_sector is set to 0 and has no effect.
+	 */
+	if (!good_agpt && force_gpt && state->force_gpt_sector)
+		good_agpt = is_gpt_valid(state, state->force_gpt_sector,
+					 &agpt, &aptes);
+
         /* The obviously unsuccessful case */
         if (!good_pgpt && !good_agpt)
                 goto fail;