5 comments:
Patch Set #17, Line 20: Remove the postcar stage.
probably some remains from an older version of this patch. […]
Sorry, I wasn't clear. I just meant the wording. The postcar stage is
indeed disable in case of RESET_VECTOR_IN_RAM (by a depends on !...)
by this patch. So that should be mentioned here.
It's hard to overlook in the huge patch because it's implicit and just
one line... ;)
File src/arch/x86/early_ram.ld:
i don't particularly like this, since it just adds another line for no gain, but changed for consist […]
The other style option is /* and */ on the first and last line without preceding *s. i.e.
/* multi-
line
comment */
However, for top-level comments, I prefer the current style with wasted space.
Patch Set #17, Line 18: _PAD_FOR_ALIGNS = 0x20;
Nico, Arthur, here's what this is for and I would like a prettier solution, of course. […]
Thanks, that was clear to me already, though. What I'm asking for is the
specific number `0x20` aka. 32. I looked into `memlayout.h` and the only
region with an explicit ALIGN_COUNTER() is the new EARLYRAM_STACK(). It's
not using 32, though, but 16.
And now that I looked further into it, do we need the explicit ALIGN_
COUNTER() at all? Do we really expect any size to not be a multiple of 16?
Aren't the assertions enough?
Patch Set #17, Line 30: #endif
We can also assert here that `. <= CONFIG_X86_RESET_VECTOR`.
File src/arch/x86/memlayout.ld:
Patch Set #17, Line 57: #include <arch/x86/id.ld>
At the moment, it's not there and we're not attempting to use it for anything. It seems like Kyosti had suggested the ID was used by flashrom, but I couldn't find that, even after asking around a bit. Can you state the practical reasons for keeping it there?
Where did you ask? did you try the coreboot or flashrom mailing list? Took
me less than a minute to find it, cb_check_image() in `cbtables.c` (I'm not
a fan of that code, but it's there).
Another example is the flashupdate feature in FILO.
Well, I guess we can expect that every company that uses coreboot has built
some infrastructure around it. Having the `id` section at a specific location
was always an invariant for coreboot on x86.
I once had to deal with it because on Apollo Lake there is no CBFS at the
end of the `coreboot.rom`. It turned out that there were such assumptions
all over the place. I can't say how it looks like in other companies, but
I would be surprised if we are the only one.
I don't say it has to be in the bootblock. If, for instance, you have
nothing else that has to be at the end of CBFS, you could just put it
there as usual.
(marking as resolved because any solution wouldn't belong into this change)
To view, visit change 35035. To unsubscribe, or for help writing mail filters, visit settings.