On 21.12.2021 20:34, Krystian Hebel wrote:
On 18.12.2021 18:07, Nico Huber wrote:
On 15.12.21 13:24, Krystian Hebel wrote:
Hi Nico,
On 15.12.2021 12:21, Nico Huber wrote:
Hi Krystian,
On 14.12.21 13:00, Krystian Hebel wrote:
For our work on POWER9 coreboot port we were using Skiboot [1] packed
into
FIT payload.
I might just miss it because I never worked with FIT, but maybe I'm
not
the only one: Can you elaborate why you chose FIT? Reading through
your
mail (and without further knowledge) it would just seem like the wrong
choice.
Good point, I spent so much time in this project that I treat too many
things as obvious. Don't hesitate to ask if I omit something important.
Skiboot must be supplied with information about hardware. Some of that
is generated by code based on current configuration (e.g. number of
cores, their IDs, memory amount and associativity), but a lot is always
the same for a given platform or even whole architecture (BMC sensors,
interrupt controller, register address space, LPC controller).
Isn't this all or mostly information that coreboot has already
available? It seems much like the situation on x86 with ACPI.
We also had static ACPI tables that redundantly stored infor-
mation at first. However, static ACPI tables were never treated
first-class, sometimes even like an alien, and were prone to rot.
Today, we try a good compromise between runtime generation and
static tables (which can be extended at runtime).
Not everything is defined in coreboot sources, AFAICT only XSCOM, LPC
and one out of 3 I2C buses are used, rest (almost 600 lines in dts) is
just static data. There is a lot of stuff that coreboot doesn't have to
worry about, but Skiboot and OS do.
This can
be passed either in HDAT structure (Hostboot data, proprietary and
mostly undocumented format in supposedly open firmware) or, as we
learned after asking on OpenPower-Firmware mailing list, using FDT:
https://lists.ozlabs.org/pipermail/openpower-firmware/2021-May/000641.html
In our current setup that second, constant part is supplied as FDT in
FIT image so it can be added as .dts file, which is easier to read and
understand than C code that would be used for creating these nodes.
Hmmm, I read above mailinglist thread. But I still miss the link to
FIT. coreboot would have a dtb (doesn't matter if it generates it at
runtime or a file in CBFS) and a payload (in CBFS or some other flash
partition), right? Why do you need FIT?
We don't _need_ FIT. We also don't need to implement new way of loading
the payload if existing one is working as expected, more or less. Do you
suggest removing FIT from coreboot completely? :)
FIT code already does the heavy lifting: it parses dtb to modifiable
format, appends firmware, compat and memory nodes (although the latter
are added in different format than Skiboot expects and we have to modify
it - both formats are compatible with spec, but not with each other...),
gives opportunity for fixups to DT before repacking it, sets entry point
and argument. Doing the same with dtb loaded from CBFS is possible, but
it would result in mostly duplicated code. The only pieces of FIT that
our code doesn't use are overlays and initramfs.
Another option could be to split fit_payload() into two halves, first
one would just load FIT from CBFS and choose appropriate config, second
would be given kernel, initramfs and dtb as 'struct region' and work
from there. That way we could mix various ways of obtaining pieces of
the puzzle without duplicating code. Note that this would still leave
bugs (normally I would called it "lack of implementation", but
documentation says explicitly that this is supported) in the FIT
decompression unresolved.
Just FYI, we decided to switch to ELF anyway. Some of the code will be
duplicated,
but it still is less than the glue code that would have to be used
otherwise.
Problem with FIT payload (de-)compression is thus not resolved.
--
Krystian Hebel
Firmware Engineer
https://3mdeb.com | @3mdeb_com
_______________________________________________
coreboot mailing list -- [email protected]
To unsubscribe send an email to [email protected]
