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

2 Dec 2011

      On Fri, Dec 2, 2011 at 1:22 PM, Graeme Russ graeme.russ@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@gmail.com
mailto:graeme.russ@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@chromium.org
<mailto:gabeblack@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.