| Message ID | 20200516153644.13748-5-digetx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce NVIDIA Tegra Partition Table | expand |
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.
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!
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.
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.
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 --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;
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(+)