| Message ID | 1319829706-6459-1-git-send-email-swarren@nvidia.com |
|---|---|
| State | Accepted |
| Commit | 4715a81136049167160230efd28eb9d48eeded1f |
| Delegated to: | Stefano Babic |
| Headers | show |
Acked-by: Doug Anderson <dianders@chromium.org> ...to be fair though, the regression appears to be caused by a mid-air collision of Anton's change (f75dd584cdfe29dfdcfd424bb237b9238cfb8fe4) with my change. His patch was committed on Oct 25th (though authored earlier). When I submitted my patch on the Oct 19th against denx/master, I'm pretty sure it was correct. Thank you for finding / fixing! :) -Doug --- On Fri, Oct 28, 2011 at 12:21 PM, Stephen Warren <swarren@nvidia.com> wrote: > Commit deb5ca80275e8cfa74d5680b41204e08a095eca5 "disk: part_efi: fix > **pgpt_pte == NULL" modified the code to pass "&gpt_head" to > is_gpt_valid() rather than the previous "gpt_head". However, gpt_head > is a pointer to the buffer, not the actual buffer, since it was allocated > using ALLOC_CACHE_ALIGN_BUFFER. This caused is_gpt_valid() to read the > disk block onto the stack rather than into the buffer, causing the > code to fail. > > This change reverts that portion of the commit mentioned above. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > disk/part_efi.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/disk/part_efi.c b/disk/part_efi.c > index e7f2714..ddf80a7 100644 > --- a/disk/part_efi.c > +++ b/disk/part_efi.c > @@ -130,7 +130,7 @@ void print_part_efi(block_dev_desc_t * dev_desc) > } > /* 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) { > + gpt_head, &gpt_pte) != 1) { > printf("%s: *** ERROR: Invalid GPT ***\n", __func__); > return; > } > @@ -169,7 +169,7 @@ int get_partition_info_efi(block_dev_desc_t * 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) { > + gpt_head, &gpt_pte) != 1) { > printf("%s: *** ERROR: Invalid GPT ***\n", __func__); > return -1; > } > -- > 1.7.0.4 > >
Thanks Stephen, sorry Doug. :) -Anton On Fri, Oct 28, 2011 at 12:43 PM, Doug Anderson <dianders@chromium.org> wrote: > Acked-by: Doug Anderson <dianders@chromium.org> > > ...to be fair though, the regression appears to be caused by a mid-air > collision of Anton's change (f75dd584cdfe29dfdcfd424bb237b9238cfb8fe4) with > my change. His patch was committed on Oct 25th (though authored earlier). > When I submitted my patch on the Oct 19th against denx/master, I'm pretty > sure it was correct. > > Thank you for finding / fixing! :) > > -Doug > > --- > > On Fri, Oct 28, 2011 at 12:21 PM, Stephen Warren <swarren@nvidia.com> wrote: > >> Commit deb5ca80275e8cfa74d5680b41204e08a095eca5 "disk: part_efi: fix >> **pgpt_pte == NULL" modified the code to pass "&gpt_head" to >> is_gpt_valid() rather than the previous "gpt_head". However, gpt_head >> is a pointer to the buffer, not the actual buffer, since it was allocated >> using ALLOC_CACHE_ALIGN_BUFFER. This caused is_gpt_valid() to read the >> disk block onto the stack rather than into the buffer, causing the >> code to fail. >> >> This change reverts that portion of the commit mentioned above. >> >> Signed-off-by: Stephen Warren <swarren@nvidia.com> >> --- >> disk/part_efi.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/disk/part_efi.c b/disk/part_efi.c >> index e7f2714..ddf80a7 100644 >> --- a/disk/part_efi.c >> +++ b/disk/part_efi.c >> @@ -130,7 +130,7 @@ void print_part_efi(block_dev_desc_t * dev_desc) >> } >> /* 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) { >> + gpt_head, &gpt_pte) != 1) { >> printf("%s: *** ERROR: Invalid GPT ***\n", __func__); >> return; >> } >> @@ -169,7 +169,7 @@ int get_partition_info_efi(block_dev_desc_t * 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) { >> + gpt_head, &gpt_pte) != 1) { >> printf("%s: *** ERROR: Invalid GPT ***\n", __func__); >> return -1; >> } >> -- >> 1.7.0.4 >> >> > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > >
On Fri, Oct 28, 2011 at 3:47 PM, Anton Staaf <robotboy@chromium.org> wrote: > Thanks Stephen, sorry Doug. :) > > -Anton > > On Fri, Oct 28, 2011 at 12:43 PM, Doug Anderson <dianders@chromium.org> wrote: >> Acked-by: Doug Anderson <dianders@chromium.org> Tested-by: Simon Glass <sjg@chromium.org> >> >> ...to be fair though, the regression appears to be caused by a mid-air >> collision of Anton's change (f75dd584cdfe29dfdcfd424bb237b9238cfb8fe4) with >> my change. His patch was committed on Oct 25th (though authored earlier). >> When I submitted my patch on the Oct 19th against denx/master, I'm pretty >> sure it was correct. >> >> Thank you for finding / fixing! :) >> >> -Doug >> >> --- >> >> On Fri, Oct 28, 2011 at 12:21 PM, Stephen Warren <swarren@nvidia.com> wrote: >> >>> Commit deb5ca80275e8cfa74d5680b41204e08a095eca5 "disk: part_efi: fix >>> **pgpt_pte == NULL" modified the code to pass "&gpt_head" to >>> is_gpt_valid() rather than the previous "gpt_head". However, gpt_head >>> is a pointer to the buffer, not the actual buffer, since it was allocated >>> using ALLOC_CACHE_ALIGN_BUFFER. This caused is_gpt_valid() to read the >>> disk block onto the stack rather than into the buffer, causing the >>> code to fail. >>> >>> This change reverts that portion of the commit mentioned above. >>> >>> Signed-off-by: Stephen Warren <swarren@nvidia.com> >>> --- >>> disk/part_efi.c | 4 ++-- >>> 1 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/disk/part_efi.c b/disk/part_efi.c >>> index e7f2714..ddf80a7 100644 >>> --- a/disk/part_efi.c >>> +++ b/disk/part_efi.c >>> @@ -130,7 +130,7 @@ void print_part_efi(block_dev_desc_t * dev_desc) >>> } >>> /* 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) { >>> + gpt_head, &gpt_pte) != 1) { >>> printf("%s: *** ERROR: Invalid GPT ***\n", __func__); >>> return; >>> } >>> @@ -169,7 +169,7 @@ int get_partition_info_efi(block_dev_desc_t * 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) { >>> + gpt_head, &gpt_pte) != 1) { >>> printf("%s: *** ERROR: Invalid GPT ***\n", __func__); >>> return -1; >>> } >>> -- >>> 1.7.0.4 >>> >>> >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot >> >> > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
Tested-by: Anton Staaf <robotboy@chromium.org> Mike, do you think you could pull this into staging as per Wolfgang's recent request for help? Thanks, Anton On Sun, Oct 30, 2011 at 8:44 AM, Mike Frysinger <vapier@gentoo.org> wrote: > Acked-by: Mike Frysinger <vapier@gentoo.org> > -mike > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > >
On Tue, Nov 15, 2011 at 10:25 AM, Anton Staaf <robotboy@chromium.org> wrote: > Tested-by: Anton Staaf <robotboy@chromium.org> > Mike, do you think you could pull this into staging as per Wolfgang's > recent request for help? > > Thanks, > Anton > Please can one of the maintainers grab this and put it in staging? Regards, Simon > On Sun, Oct 30, 2011 at 8:44 AM, Mike Frysinger <vapier@gentoo.org> wrote: >> Acked-by: Mike Frysinger <vapier@gentoo.org> >> -mike >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot >> >> > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
Hi Simon, On Tue, 22 Nov 2011 14:10:12 -0800 Simon Glass <sjg@chromium.org> wrote: > On Tue, Nov 15, 2011 at 10:25 AM, Anton Staaf <robotboy@chromium.org> wrote: > > Tested-by: Anton Staaf <robotboy@chromium.org> > > Mike, do you think you could pull this into staging as per Wolfgang's > > recent request for help? > > > > Thanks, > > Anton > > > > Please can one of the maintainers grab this and put it in staging? I've seen similar patch, it is already in arm tree: http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=81432275812e7d9b3b5eec9c54c6c58dfe506759 Anatolij
On Tue, Nov 22, 2011 at 2:38 PM, Anatolij Gustschin <agust@denx.de> wrote: > Hi Simon, > > On Tue, 22 Nov 2011 14:10:12 -0800 > Simon Glass <sjg@chromium.org> wrote: > >> On Tue, Nov 15, 2011 at 10:25 AM, Anton Staaf <robotboy@chromium.org> wrote: >> > Tested-by: Anton Staaf <robotboy@chromium.org> >> > Mike, do you think you could pull this into staging as per Wolfgang's >> > recent request for help? >> > >> > Thanks, >> > Anton >> > >> >> Please can one of the maintainers grab this and put it in staging? > > I've seen similar patch, it is already in arm tree: > > http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=81432275812e7d9b3b5eec9c54c6c58dfe506759 OK, that's a funny place for it, but OK. Thanks for pointing that out. Regards, Simon > > Anatolij >
On 22/11/2011 23:10, Simon Glass wrote: > On Tue, Nov 15, 2011 at 10:25 AM, Anton Staaf <robotboy@chromium.org> wrote: >> Tested-by: Anton Staaf <robotboy@chromium.org> >> Mike, do you think you could pull this into staging as per Wolfgang's >> recent request for help? >> >> Thanks, >> Anton >> > > Please can one of the maintainers grab this and put it in staging? Hi Simon, I grab it in my TODO list in patchwork, I will take a look at it. Stefano
On Wed, Nov 23, 2011 at 12:30 AM, Stefano Babic <sbabic@denx.de> wrote: > On 22/11/2011 23:10, Simon Glass wrote: >> On Tue, Nov 15, 2011 at 10:25 AM, Anton Staaf <robotboy@chromium.org> wrote: >>> Tested-by: Anton Staaf <robotboy@chromium.org> >>> Mike, do you think you could pull this into staging as per Wolfgang's >>> recent request for help? >>> >>> Thanks, >>> Anton >>> >> >> Please can one of the maintainers grab this and put it in staging? > > Hi Simon, > > I grab it in my TODO list in patchwork, I will take a look at it. Thanks Stefano - please also see Anatolij's email on this thread. It seems that Albert may have picked up a similar patch. Regards, Simon > > Stefano > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de > ===================================================================== >
Am 23/11/2011 20:18, schrieb Simon Glass: > > Thanks Stefano - please also see Anatolij's email on this thread. It > seems that Albert may have picked up a similar patch. In fact the patch "Fix errors noticed after enabling CONFIG_EFI_PARTITION for the OMAP3 EVM board" has already fixed this issue. There is no need to merge this patch. Best regards. Stefano Babic
diff --git a/disk/part_efi.c b/disk/part_efi.c index e7f2714..ddf80a7 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -130,7 +130,7 @@ void print_part_efi(block_dev_desc_t * dev_desc) } /* 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) { + gpt_head, &gpt_pte) != 1) { printf("%s: *** ERROR: Invalid GPT ***\n", __func__); return; } @@ -169,7 +169,7 @@ int get_partition_info_efi(block_dev_desc_t * 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) { + gpt_head, &gpt_pte) != 1) { printf("%s: *** ERROR: Invalid GPT ***\n", __func__); return -1; }
Commit deb5ca80275e8cfa74d5680b41204e08a095eca5 "disk: part_efi: fix **pgpt_pte == NULL" modified the code to pass "&gpt_head" to is_gpt_valid() rather than the previous "gpt_head". However, gpt_head is a pointer to the buffer, not the actual buffer, since it was allocated using ALLOC_CACHE_ALIGN_BUFFER. This caused is_gpt_valid() to read the disk block onto the stack rather than into the buffer, causing the code to fail. This change reverts that portion of the commit mentioned above. Signed-off-by: Stephen Warren <swarren@nvidia.com> --- disk/part_efi.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)