[U-Boot] [PATCH 3/4] x86: Force the lib_sysinfo structure to be in the .data section

[Gabe Black]

On Fri, Dec 2, 2011 at 1:22 PM, Graeme Russ <graeme.russ at gmail.com> wrote:

> Hi Gabe,
>
> On 03/12/11 08:16, Gabe Black wrote:
> >
> >
> > On Fri, Dec 2, 2011 at 1:10 PM, Graeme Russ <graeme.russ at gmail.com
> > <mailto:graeme.russ at gmail.com>> wrote:
> >
> >     Hi Gabe,
> >
> >     On 30/11/11 17:07, Gabe Black wrote:
> >     > Otherwise it ends up in the .bss section. U-boot assumes that it
> doesn't
> >     > need to copy it over during relocation, and instead fills that
> whole
> >     > section with zeroes. If we really were booting from ROM that would
> be
> >     > appropriate, but we need some information from the coreboot tables
> >     (memory
> >     > size) before then and have to fill that structure before
> relocation. We
> >     > skirt u-boot's assumption by putting this in .data where it
> assumes there
> >     > is still read only but non-zero data.
> >     >
> >     > Signed-off-by: Gabe Black <gabeblack at chromium.org
> >     <mailto:gabeblack at chromium.org>>
> >     > ---
> >     >  arch/x86/cpu/coreboot/sysinfo.c |    8 +++++++-
> >     >  1 files changed, 7 insertions(+), 1 deletions(-)
> >     >
> >     > diff --git a/arch/x86/cpu/coreboot/sysinfo.c
> >     b/arch/x86/cpu/coreboot/sysinfo.c
> >     > index 464f8a1..e74fe0a 100644
> >     > --- a/arch/x86/cpu/coreboot/sysinfo.c
> >     > +++ b/arch/x86/cpu/coreboot/sysinfo.c
> >     > @@ -30,4 +30,10 @@
> >     >
> >     >  #include <asm/ic/coreboot/sysinfo.h>
> >     >
> >     > -struct sysinfo_t lib_sysinfo;
> >     > +/*
> >     > + * This needs to be in the .data section so that it's copied over
> during
> >     > + * relocation. By default it's put in the .bss section which is
> >     simply filled
> >     > + * with zeroes when transitioning from "ROM", which is really RAM,
> >     to other
> >     > + * RAM.
> >     > + */
> >     > +struct sysinfo_t lib_sysinfo __attribute__((section(".data")));
> >
> >     I think this can be logically folded into the first patch
> >
> >     Regards,
> >
> >     Graeme
> >
> >
> > I would rather not do that since the first patch is, modulo checkpatch
> and
> > build fixes, just importing that code from coreboot's libpayload. This
> > change is making an important, non-obvious change which makes it work
> > properly within u-boot. These are two different things, and one way or
> the
> > other this one is important enough to be singled out as its own change
> and
> > not lost in the midst of the other.
>
> I agree that is a logical position to take, however I think that the fact
> that you introduce a commit which is known to be 'broken' is the overriding
> factor - Include to fact that the modification is made in the commit
> message and the comment like:
>
> /*
>  * Note: U-Boot needs this to be in the .data section so....
>  * ...
>  */
>
> Also, can you supply any specific commit id(s) for the originating source -
> We typically try to include as much canonical information to identify the
> source to a particular commit
>
> Regards,
>
> Graeme
>


These commits were originally put together many months ago, so the exact
version of the original source is lost.