Re: [coreboot] FSP 2.0 headers in coreboot
On Tue, May 22, 2018 at 2:25 PM, Youness Alaoui wrote: > On Fri, May 18, 2018 at 2:59 PM, Nico Huber wrote: >> >> I have to admit, I don't like your patch. While it gets the job done, >> it brings `MemInfoHob.h` and `FspsUpd.h` out of sync, so the state in >> coreboot as a whole would match neither version. >> > Good point. It is a Frankenstein, but it was either that or have > #ifdefs in the fsp vendorcode itself to determine if attribute X is > available or not, etc.. > >>> - or abandon my patch (which means I can never send a working >>> board-status for the librems without having a dirty tree or a version >>> commit hash that doesn't match upstream) >>> - or have the possibility to choose between the two (either via >>> #ifdefs or via a variants method). >> >> If we can't get a new, fitting binary out of Intel, I would prefer this, >> or, more bluntly, just copy the GitHub version and revert the changes >> that need the additional UPD. >> >> In other words, whatever happens, I think we should have the headers >> of the latest public release in the tree. >> > > I agree, that's probably the cleanest solution, but I didn't suggest > it because I figured people would be yelling about removing > functionality that might need to be re-added eventually if Intel end > up doing an updated release. > >>> - or, if Intel people are reading this right now (or someone here can >>> poke them directly), have the public release updated so this matter >>> can be dropped entirely (the public update would need to be released >>> *very* soon though). >> >> Even if you (Purism) poke them about it, that might help. But I would >> like to see that Google does that too (IMHO, they profit most from the >> mess). Any of you should have more leverage than the customer of a >> customer of a customer of a potential customer of Intel has, that I >> work for ;) > I know no-one at Intel to poke them about. I'll ask if Todd has > contacts that might be able to help. I'm hoping that some of the > people that are working for Intel are following this email thread but > if they are, they haven't raised their hands. Anyone knows or can > suggest the name of someone that we might poke ? I'm guessing those > working for Google who actually received the newer FSP from Intel > might know who to ask, and since they are the ones that would be > affected by their code/features being removed if we revert to github > version of header, it might help put some pressure on this issue (so > far, I haven't seen any incentive to make them do that). I thought I said something somewhere -- maybe on a code review? Anyway, I've been pushing on this from my end. I don't have an answer yet, though. > >> >>> Should we put this to a vote now, or should we discuss the >>> possibilities/alternatives some more, if anyone has ideas on how to >>> tackle this specific issue ? >> >> Well, my vote, in order of preference: >> >> 1. Poke Intel. >> 2. Get a verbatim copy of the GitHub headers in (in a way of [least] effort >> for >> the community). Maybe in a month from now? no matter the outcome >> from 1. >> >> Nico > > I agree, and I give my 100% vote for that as well. It will be much > better than an #ifdef mess and would be a good incentive for the > people from Intel not saying "no, too much hassle to update the file, > we got nothing to lose anyway if we just ignore this". > So let's see who can poke who from Intel, let's give a deadline (1 or > 2 months? if bureaucracy means it will take longer, we might revisit > if we at least got a confirmation that the issue is being looked at on > Intel's side), then once deadline is reached, we revert to public > headers and remove functionality that prevents building (access to > nonexistent fields in UPD) and in the future, reject patches on gerrit > for non public blobs. > > Thanks! -- coreboot mailing list: coreboot@coreboot.org https://mail.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] FSP 2.0 headers in coreboot
On Fri, May 18, 2018 at 2:59 PM, Nico Huber wrote: > > I have to admit, I don't like your patch. While it gets the job done, > it brings `MemInfoHob.h` and `FspsUpd.h` out of sync, so the state in > coreboot as a whole would match neither version. > Good point. It is a Frankenstein, but it was either that or have #ifdefs in the fsp vendorcode itself to determine if attribute X is available or not, etc.. >> - or abandon my patch (which means I can never send a working >> board-status for the librems without having a dirty tree or a version >> commit hash that doesn't match upstream) >> - or have the possibility to choose between the two (either via >> #ifdefs or via a variants method). > > If we can't get a new, fitting binary out of Intel, I would prefer this, > or, more bluntly, just copy the GitHub version and revert the changes > that need the additional UPD. > > In other words, whatever happens, I think we should have the headers > of the latest public release in the tree. > I agree, that's probably the cleanest solution, but I didn't suggest it because I figured people would be yelling about removing functionality that might need to be re-added eventually if Intel end up doing an updated release. >> - or, if Intel people are reading this right now (or someone here can >> poke them directly), have the public release updated so this matter >> can be dropped entirely (the public update would need to be released >> *very* soon though). > > Even if you (Purism) poke them about it, that might help. But I would > like to see that Google does that too (IMHO, they profit most from the > mess). Any of you should have more leverage than the customer of a > customer of a customer of a potential customer of Intel has, that I > work for ;) I know no-one at Intel to poke them about. I'll ask if Todd has contacts that might be able to help. I'm hoping that some of the people that are working for Intel are following this email thread but if they are, they haven't raised their hands. Anyone knows or can suggest the name of someone that we might poke ? I'm guessing those working for Google who actually received the newer FSP from Intel might know who to ask, and since they are the ones that would be affected by their code/features being removed if we revert to github version of header, it might help put some pressure on this issue (so far, I haven't seen any incentive to make them do that). > >> Should we put this to a vote now, or should we discuss the >> possibilities/alternatives some more, if anyone has ideas on how to >> tackle this specific issue ? > > Well, my vote, in order of preference: > > 1. Poke Intel. > 2. Get a verbatim copy of the GitHub headers in (in a way of [least] effort > for > the community). Maybe in a month from now? no matter the outcome > from 1. > > Nico I agree, and I give my 100% vote for that as well. It will be much better than an #ifdef mess and would be a good incentive for the people from Intel not saying "no, too much hassle to update the file, we got nothing to lose anyway if we just ignore this". So let's see who can poke who from Intel, let's give a deadline (1 or 2 months? if bureaucracy means it will take longer, we might revisit if we at least got a confirmation that the issue is being looked at on Intel's side), then once deadline is reached, we revert to public headers and remove functionality that prevents building (access to nonexistent fields in UPD) and in the future, reject patches on gerrit for non public blobs. Thanks! -- coreboot mailing list: coreboot@coreboot.org https://mail.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] Building coreboot for Apollo Lake: missing 'IFWI' region
Hi Nico and Julius,
Thank you for the helpful tips. I will look into the fmd files in the tree
and see if I can come up with something sensible for this board.
I'll update you on any progress (or lack thereof).
Cheers,
Hal
On Sat, May 19, 2018 at 4:57 PM, Nico Huber wrote:
> Hi,
>
> On 19.05.2018 02:27, Julius Werner wrote:
> > Apollolake looks quite different, and I don't really know all the
> details,
> > but somehow the reset vector is interwoven with that IFWI thing near the
> > front of the ROM. You can see an example Chrome OS Apollolake FMAP in
> > src/mainboard/google/reef/chromeos.fmd, and compare it to a normal
> Chrome
> > OS x86 FMAP like src/mainboard/google/glados/chromeos.fmd . The
> Apollolake
> > support was added by Chrome OS folks, so maybe they didn't bother
> adapting
> > the default-x86.fmd mechanism to work with it because all the Chrome OS
> > boards have their own FMAP anyway. You may be able to get further in your
> > build if you copy the reef/chromeos.fmd file, adapt it to your mainboard
> if
> > necessary, and then point to it with CONFIG_FMDFILE.
>
> I'm not an expert for Apollo Lake either, but I know other Intel plat-
> forms and my way around their documentation.
>
> I too believe that adding an .fmd file like Julius described, will
> fix your problem. Though, there are already some non-CrOS boards in
> the tree. Might be more worth looking at their (simpler) .fmd (e.g.
> `src/mainboard/siemens/mc_apl1/mc_apl1.fmd`).
>
> Regarding the details, I've digged around a little. Here is what I found
> out so far (stop reading here unless you really want to know, it might
> confuse you even more):
>
> Apollo Lake uses a Firmware Descriptor (IFD) like other Intel platforms,
> but the IFD's layout is used much less. One region amidst this layout
> is the IFWI. The IFWI is partitioned by other means and contains code
> for some integrated controllers and also what used to be the BIOS. The
> latter is divided into IBB and OBB. The IBB partition is used for core-
> boot's bootblock. The OBB partition for everything else in coreboot. The
> OBB is the last part of the IFWI.
>
> To make the situation even more confusing, somebody had the great idea
> to use the same term, IFWI, for something slightly different. I guess to
> separate the read-only from updateable parts better, IFWI in the notion
> of coreboot's .fmd files is the actual IFWI minus the OBB. I will refer
> to it as the RO_CORE_IFWI (might not be the worst idea to rename it like
> this in coreboot). So what happens during build (cf. `soc/intel/
> apollolake/Makefile.inc:125`):
>
> - coreboot is provided an IFWI image (that has to contain all the
> coreboot-unrelated code for the extra chipset controllers, those
> are just passed through by our build system).
>
> - Our build system crafts the RO_CORE_IFWI with the provided IFWI:
> o The OBB is stripped off.
> o The IBB is replaced with coreboot's bootblock.
>
> - The final image is build with
> o the RO_CORE_IFWI in place of IFWI in the .fmd and
> o all other coreboot parts in their respective place in the .fmd.
>
> Looking at the `mc_apl1.fmd` again:
>
> - SI_DESC is like the Intel Firmware Descriptor.
> - IFWI is the RO_CORE_IFWI (IFWI minus OBB with replaced IBB).
> - Everything else before DEVICE_EXTENSION is the OBB.
> - DEVICE_EXTENSIONS is another partition in the IFD, that I've just
> learned about.
>
> Nico
>
> PS. This is how I would have written such .fmd (currently not compatible
> to `apollolake/Makefile.inc`):
>
> FLASH 16M {
> SI_DESC@0x0 0x1000
> IFWI@0x1000 0xefe000 {
> RO_CORE_IFWI@0x0 0x2ff000
> OBB@0x2ff000 0xbff000 {
> FMAP@0x 0x800
> COREBOOT(CBFS)@0x800 0xb9d800
> UNIFIED_MRC_CACHE@0xb9e000 0x21000 {
> RECOVERY_MRC_CACHE@0x0 0x1
> RW_MRC_CACHE@0x1 0x1
> RW_VAR_MRC_CACHE@0x2 0x1000
> }
> BIOS_UNUSABLE@0xbbf000 0x4
> }
> }
> DEVICE_EXTENSION@0xeff000 0x10
> UNUSED_HOLE@0xfff000 0x1000
> }
>
--
coreboot mailing list: coreboot@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot
