Missing important FSP Hob structs header files for EagleStreamFspBinPkg

[johnnylinwiwynn]

EagleStreamFspBinPkg/Include only provides UPD header files for FSP input interface for the bootloader, but doesn't provide any important Hob struct header files for the FSP output interface. Running
"find -iname hob.h" I can see there are quite many Hob header files for Client FSP, such as
/AlderLakeFspBinPkg/Client/AlderLakeS/Include/FirmwareVersionInfoHob.h
./AlderLakeFspBinPkg/Client/AlderLakeS/Include/MemInfoHob.h
./AlderLakeFspBinPkg/Client/AlderLakeS/Include/FspInfoHob.h
./AlderLakeFspBinPkg/Client/AlderLakeS/Include/SmbiosCacheInfoHob.h

Can Intel please also release the needed Hob header files for EagleStreamFspBinPkg? For example, these are the Hob header files for WW43 2022 EGS FSP:
https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/main/src/vendorcode/intel/fsp/fsp2_0/sapphirerapids_sp/

Thanks.

[65a]

Trying the current FSP with coreboot HEAD is completely broken on the board I am porting (Tyan S5652) and I think this is part of the reason. Please release updated headers, currently I'm seeing a number of strange issues that may be related to misinterpreted HOB blocks, including the inability to run option ROMs or boot a Linux payload, despite making it to EOP.

[johnnylinwiwynn]

Some discussion on the change you can reference from
https://review.coreboot.org/c/coreboot/+/80360
At least with UEFI payload I can boot up OS.

https://github.com/johnnylinwiwynn/Intel_AC_OSF
FYI

[65a]

@johnnylinwiwynn two questions (I actually had read some of that gerrit change's comments trying to get things working):

1. What is a TOCTOU mitigated FSP? I don't think this board has bootguard enforcement (I don't think I would get this far), but ME has been placed in the MFG-mode.
2. How did you bring up graphics (or did you let OS bring up graphics)?
   I'll give it a try, thanks!

[65a]

Looking at my romstage setup and changing some things to match Archer city seems to have fixed things, I believe this was either using XAPIC by default, or configuring CXL, I was able to boot a bzImage. Thanks for that!

[LeanSheng]

@65a great to hear that it got resolved on your side!
I am also working on EGS and next generation Intel server platforms with coreboot.
Actually there is a slack group for Open Source Firmware, feel free to join:
https://join.slack.com/t/osfw/shared_invite/zt-2eyud9b4k-EulKQ3MwjTIAn0VAi_J_uA

You can then go to the coreboot channel and start discussions. There are many experienced and helpful folks there from all over the world :)

If not you could also drop me an email if you need something related:
sheng.tan@9elements.com

[johnnylinwiwynn]

@johnnylinwiwynn two questions (I actually had read some of that gerrit change's comments trying to get things working):

1. What is a TOCTOU mitigated FSP? I don't think this board has bootguard enforcement (I don't think I would get this far), but ME has been placed in the MFG-mode.

2. How did you bring up graphics (or did you let OS bring up graphics)?

I'll give it a try, thanks!

For some unknown reasons FSP needs to revert a commit related to a TOCTOU security pcd configuration otherwise Linux payload will reset and cannot boot, but UEFI payload is not affected by this. The FSP pcd is Intel NDA. I can share more if you have Intel FSP source code NDA.

I haven't tried to enable graphics since we use headless server only, but the community and firmware vender like 9elements, SysPro Consulting could help.

[65a]

I do not have an NDA, but thanks to your help I was able to boot a linux (also SeaBIOS but no keyboard, and u-boot with some issues) payload. I had success with XAPIC and late X2APIC (Called "XAPIC then X2APIC") mode, originally I was getting reboots in the default mode, but it may have also been some changes to try to configure CXL. I'll close my other issue , but it would still be nice if the FSP headers and Coreboot matched.

[65a]

As far as I can tell, Linux payload, and perhaps others, will only boot if coreboot is set to wipe memory. This results in one logged null pointer exception, but no other issues.

[65a]

TME is not working, as the policy part of the MSR is not programmed correctly. This may be a HOB/UPD issue, as copying Alderlake's coreboot init for TME doesn't produce any change.

[LeanSheng]

This is bad, let me try to reach out to my intel contact to fix this.

[nate-desimone]

For some unknown reasons FSP needs to revert a commit related to a TOCTOU security pcd configuration otherwise Linux payload will reset and cannot boot, but UEFI payload is not affected by this. The FSP pcd is Intel NDA. I can share more if you have Intel FSP source code NDA.

Intel has root caused the issue with the TOCTOU mitigation & coreboot. A fix is currently under development and will be included in an upcoming FSP release.

[65a]

Great news! For what it's worth, I think the headers coreboot uses today do not match, especially the memory hob. For instance, the binary dump of the hob data shows things like the module serial numbers, but coreboot doesn't see this, and most DIMMs have the present boolean set to false. Another example is that SNC is apparently enabled, even though it shouldn't be (wasn't requested) and TME is configured improperly. This seems like the structs on the coreboot side do not match the actual hob, so there's a memory safety issue. Publishing the actual HOB structs as returned by the FSP would help in these cases, but glad you are also looking at the reset/TOCTOU issue.

[felixsinger]

For some unknown reasons FSP needs to revert a commit related to a TOCTOU security pcd configuration otherwise Linux payload will reset and cannot boot, but UEFI payload is not affected by this. The FSP pcd is Intel NDA. I can share more if you have Intel FSP source code NDA.

Intel has root caused the issue with the TOCTOU mitigation & coreboot. A fix is currently under development and will be included in an upcoming FSP release.

Amazing. Thank you!

[LeanSheng]

For some unknown reasons FSP needs to revert a commit related to a TOCTOU security pcd configuration otherwise Linux payload will reset and cannot boot, but UEFI payload is not affected by this. The FSP pcd is Intel NDA. I can share more if you have Intel FSP source code NDA.

Intel has root caused the issue with the TOCTOU mitigation & coreboot. A fix is currently under development and will be included in an upcoming FSP release.

@nate-desimone It has been 4 months now per your last update. Any update? :)

[65a]

+1, still very interested in updated headers and any fixed FSP. This thread seems to be now a mixture of TOCTOU and Headers discussion, I want to focus on the headers (For TOCTOU mitigation mitigation, I had success enabling Coreboot's CONFIG_SECURITY_CLEAR_DRAM_ON_REGULAR_BOOT, which is kinda weird).

Public headers are old problem:

The current headers available publicly are subtly wrong and create a memory safety risk when used with the public FSP. What I mean by that is:

Coreboot code:

printk(BIOS_DEBUG,
           "^^^  MEMMAP_SOCKET: %ld, ChannelDevice: %ld, MEMMAP_DIMM_DEVICE_INFO_STRUCT: %ld\n",
           sizeof(MEMMAP_SOCKET), sizeof(struct ChannelDevice),
           sizeof(MEMMAP_DIMM_DEVICE_INFO_STRUCT));
    printk(BIOS_DEBUG, "^^^  Element Offset: %ld\n",
           offsetof(SYSTEM_MEMORY_MAP_HOB, Element));
    printk(BIOS_DEBUG, "^^^  Socket Offset: %ld\n",
           offsetof(SYSTEM_MEMORY_MAP_HOB, Socket));
    printk(BIOS_DEBUG, "^^^  ChannelInfo Offset: %ld\n",
           offsetof(MEMMAP_SOCKET, ChannelInfo));
    printk(BIOS_DEBUG, "^^^  DimmInfo Offset: %ld\n",
           offsetof(struct ChannelDevice, DimmInfo));
    printk(BIOS_DEBUG, "^^^  DimmSize Offset: %ld\n",
           offsetof(struct DimmDevice, DimmSize));

    for (int s = 0; s < MAX_SOCKET; ++s) {
        if (!hob->Socket[s].SocketEnabled)
            continue;
        for (int ch = 0; ch < MAX_CH; ++ch) {
            if (!hob->Socket[s].ChannelInfo[ch].Enabled)
                continue;
            for (int dimm = 0; dimm < MAX_DIMM; ++dimm) {
                if (!hob->Socket[s].ChannelInfo[ch].DimmInfo[dimm].Present)
                    continue;
                printk(BIOS_DEBUG,
                       "\tsocket: %d, ch: %d, dimm: %d, enabled: %d, DimmSize: 0x%x\n",
                       s, ch, dimm,
                       hob->Socket[s].ChannelInfo[ch].DimmInfo[dimm].Enabled,
                       hob->Socket[s].ChannelInfo[ch].DimmInfo[dimm].DimmSize);
            }
        }
    }

    printk(BIOS_DEBUG, "\tBiosFisVersion: 0x%x\n", hob->BiosFisVersion);
    printk(BIOS_DEBUG, "\tMmiohBase: 0x%x\n", hob->MmiohBase);

Prints the hob data based on the structs in the current publicly available header. On a (thankfully) booting system with 16 dimms and an HBM Xeon Max, this code prints (during last HOB handoff, earlier are no better):

================== MEMORY MAP HOB DATA ==================
hob: 0x777f5000, structure size: 0x7c2d
        lowMemBase: 0x0, lowMemSize: 0x20, highMemBase: 0x40, highMemSize: 0x17e0
        memSize: 0x1800, memFreq: 0x12c0
        NumChPerMC: 2
        SystemMemoryMapElement Entries: 5, entry size: 19
                memory_map 0, SocketId: 0x0, BaseAddress: 0x0, ElementSize: 0x20, Type: 0x1
                memory_map 1, SocketId: 0x0, BaseAddress: 0x40, ElementSize: 0x7e0, Type: 0x1
                memory_map 2, SocketId: 0x0, BaseAddress: 0x820, ElementSize: 0x800, Type: 0x1
                memory_map 3, SocketId: 0x0, BaseAddress: 0x1020, ElementSize: 0x400, Type: 0x1
                memory_map 4, SocketId: 0x0, BaseAddress: 0x1420, ElementSize: 0x400, Type: 0x5
^^^  MEMMAP_SOCKET: 5845, ChannelDevice: 270, MEMMAP_DIMM_DEVICE_INFO_STRUCT: 126
^^^  Element Offset: 142
^^^  Socket Offset: 7239
^^^  ChannelInfo Offset: 3685
^^^  DimmInfo Offset: 18
^^^  DimmSize Offset: 14
        BiosFisVersion: 0x0
        MmiohBase: 0x0

Whatever bit happens to be at hob->Socket[s].SocketEnabled happens to be 0, I guess thankfully as it prevents further reading of the HOB incorrectly. Happy to provide raw hexdumps of the HOB as well, but it's quite clear to me the struct sizes/offsets are wrong, and the fact it can even boot is a bit of a matter of luck.

Really hopeful that Intel can release a matching FSP and header set, and thank you!

[LeanSheng]

+1, still very interested in updated headers and any fixed FSP. This thread seems to be now a mixture of TOCTOU and Headers discussion, I want to focus on the headers (For TOCTOU mitigation mitigation, I had success enabling Coreboot's CONFIG_SECURITY_CLEAR_DRAM_ON_REGULAR_BOOT, which is kinda weird).

Public headers are old problem:

The current headers available publicly are subtly wrong and create a memory safety risk when used with the public FSP. What I mean by that is:

Coreboot code:

printk(BIOS_DEBUG,

           "^^^  MEMMAP_SOCKET: %ld, ChannelDevice: %ld, MEMMAP_DIMM_DEVICE_INFO_STRUCT: %ld\n",

           sizeof(MEMMAP_SOCKET), sizeof(struct ChannelDevice),

           sizeof(MEMMAP_DIMM_DEVICE_INFO_STRUCT));

    printk(BIOS_DEBUG, "^^^  Element Offset: %ld\n",

           offsetof(SYSTEM_MEMORY_MAP_HOB, Element));

    printk(BIOS_DEBUG, "^^^  Socket Offset: %ld\n",

           offsetof(SYSTEM_MEMORY_MAP_HOB, Socket));

    printk(BIOS_DEBUG, "^^^  ChannelInfo Offset: %ld\n",

           offsetof(MEMMAP_SOCKET, ChannelInfo));

    printk(BIOS_DEBUG, "^^^  DimmInfo Offset: %ld\n",

           offsetof(struct ChannelDevice, DimmInfo));

    printk(BIOS_DEBUG, "^^^  DimmSize Offset: %ld\n",

           offsetof(struct DimmDevice, DimmSize));



    for (int s = 0; s < MAX_SOCKET; ++s) {

        if (!hob->Socket[s].SocketEnabled)

            continue;

        for (int ch = 0; ch < MAX_CH; ++ch) {

            if (!hob->Socket[s].ChannelInfo[ch].Enabled)

                continue;

            for (int dimm = 0; dimm < MAX_DIMM; ++dimm) {

                if (!hob->Socket[s].ChannelInfo[ch].DimmInfo[dimm].Present)

                    continue;

                printk(BIOS_DEBUG,

                       "\tsocket: %d, ch: %d, dimm: %d, enabled: %d, DimmSize: 0x%x\n",

                       s, ch, dimm,

                       hob->Socket[s].ChannelInfo[ch].DimmInfo[dimm].Enabled,

                       hob->Socket[s].ChannelInfo[ch].DimmInfo[dimm].DimmSize);

            }

        }

    }



    printk(BIOS_DEBUG, "\tBiosFisVersion: 0x%x\n", hob->BiosFisVersion);

    printk(BIOS_DEBUG, "\tMmiohBase: 0x%x\n", hob->MmiohBase);

Prints the hob data based on the structs in the current publicly available header. On a (thankfully) booting system with 16 dimms and an HBM Xeon Max, this code prints (during last HOB handoff, earlier are no better):

================== MEMORY MAP HOB DATA ==================

hob: 0x777f5000, structure size: 0x7c2d

        lowMemBase: 0x0, lowMemSize: 0x20, highMemBase: 0x40, highMemSize: 0x17e0

        memSize: 0x1800, memFreq: 0x12c0

        NumChPerMC: 2

        SystemMemoryMapElement Entries: 5, entry size: 19

                memory_map 0, SocketId: 0x0, BaseAddress: 0x0, ElementSize: 0x20, Type: 0x1

                memory_map 1, SocketId: 0x0, BaseAddress: 0x40, ElementSize: 0x7e0, Type: 0x1

                memory_map 2, SocketId: 0x0, BaseAddress: 0x820, ElementSize: 0x800, Type: 0x1

                memory_map 3, SocketId: 0x0, BaseAddress: 0x1020, ElementSize: 0x400, Type: 0x1

                memory_map 4, SocketId: 0x0, BaseAddress: 0x1420, ElementSize: 0x400, Type: 0x5

^^^  MEMMAP_SOCKET: 5845, ChannelDevice: 270, MEMMAP_DIMM_DEVICE_INFO_STRUCT: 126

^^^  Element Offset: 142

^^^  Socket Offset: 7239

^^^  ChannelInfo Offset: 3685

^^^  DimmInfo Offset: 18

^^^  DimmSize Offset: 14

        BiosFisVersion: 0x0

        MmiohBase: 0x0

Whatever bit happens to be at hob->Socket[s].SocketEnabled happens to be 0, I guess thankfully as it prevents further reading of the HOB incorrectly. Happy to provide raw hexdumps of the HOB as well, but it's quite clear to me the struct sizes/offsets are wrong, and the fact it can even boot is a bit of a matter of luck.

Really hopeful that Intel can release a matching FSP and header set, and thank you!

@65a
We 9elements have been following this issue every month and work closely with Intel on it. Intel engineer has been working very hard internally for the past few month to resolve the process hurdle in order to get it resolved asap. Let me follow up with Intel on this again. If you urgently need this, reach out to my email sheng.tan@9elements.com, we could share the required files after signing NDA. We also got some fixes in FSP and trying to update fixes back to Intel.

[65a]

@LeanSheng Thank you for your kind offer. I will wait for the public release, as I am not interested in the NDA. Looking forward to when this is updated! Glad to hear Intel is working on it :)

[65a]

For other folks who are also waiting patiently, I've experimentally confirmed the current coreboot hobs are incorrect. To fix the DIMM detection issue,
set #define FSP_RESERVED3_LEN 45 in MemoryMapDataHob.h

This will fix the dimm detection and return the correct DIMM info. This also confirms that coreboot with the current FSP and headers combo is exhibiting undefined behavior, which is a memory safety issue. Note that this value is experimentally determined, and it's probably unwise to trust that is the correct distribution of the additional bytes, but it works better than the current state...ymmv.

There are some other issues, recommended to set NO_FSP_TEMP_RAM_EXIT in the Kconfig as well. This is relevant if you're getting a hang or null pointer dereference at postcar with specific options or payloads. The other xeon_sp chips do set this, probably spr should too.

[shuoliu0]

We tentatively made fixes with below config (not merged yet) and look forward to evaluations.

coreboot baseline - commit c52ffcede36658e7efd2b8d01d92e7e2eaaa12e7 + https://review.coreboot.org/c/coreboot/+/80360 3rdparty/fsp baseline - commit 15c0f7b + #115 3rdparty/intel-microcode baseline - commit 8ac9378a84879e81c503e09f344560b3dd7f72df

[65a]

DIMM info (and other HOBs) are working correctly as far as I can tell now. 2LM and 1LM memory both work as well. Didn't try TXT or SGX. TME doesn't seem to work still (thought maybe Upd was wrong, but maybe it is a precondition issue, as the MSR is locked to disabled state by FSP). As far as the title of this bug, seems fixed from here. Thanks for the update.