Re: [edk2-devel] UPL: UefiPayload depends on FIT FDT, but Platform Init has no reason to load it

["Chiu, Chasel via groups.io" <chasel.chiu=intel.com@groups.io>]

[-- Attachment #1: Type: text/plain, Size: 5563 bytes --]


Hi Benjamin,

Thanks for clarifying and fixing the implementation issues. It might be good to split single huge PR into small PRs so we could merge some of the small fixes without blocking by other bigger changes. Like this load base address fix, we can merge it once we verified the edk2 use case still working. What do you think?

Thanks,
Chasel


From: Benjamin Doron <benjamin.doron00@gmail.com>
Sent: Tuesday, December 10, 2024 9:58 PM
To: Liu, Linus <linus.liu@intel.com>; devel@edk2.groups.io
Cc: Dong, Guo <guo.dong@intel.com>; Rhodes, Sean <sean@starlabs.systems>; Lu, James <james.lu@intel.com>; Guo, Gua <gua.guo@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
Subject: RE: UPL: UefiPayload depends on FIT FDT, but Platform Init has no reason to load it

Ah, coreboot wants to load the images individually. And if you check the PE image base and entrypoint, there's a mismatch of 0x1000, so loading the tianocore image to where the "load" property says it should go won't work. In my email, you can see that the module is rebased to 8M + 4K, but "load" is set to 8M.

I've fixed this issue now, see https://github.com/tianocore/edk2/pull/6382/commits/90b718c9ba43100ff4dc045d4806683942934d8d. This will keep working for EDK2-based Platform Init, and allow us to support coreboot too.

The current plan, as discussed in the UPL meeting today, is to implement support for "/options/upl-images@<addr>/image" as defined in the spec in EDK2, which will tell us where the FVs are without parsing the FIT header. I've implemented and tested that on coreboot, and I'll push that patch in the morning.

Best regards,
Benjamin

On December 10, 2024 9:38:28 p.m. EST, "Liu, Linus" <linus.liu@intel.com<mailto:linus.liu@intel.com>> wrote:
Hi Benjamin
May I know what the problem in coreboot is?

Thanks


From: Liu, Linus
Sent: Tuesday, December 10, 2024 1:42 PM
To: Benjamin Doron <benjamin.doron00@gmail.com<mailto:benjamin.doron00@gmail.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Lu, James <james.lu@intel.com<mailto:james.lu@intel.com>>; Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; Chiu, Chasel <chasel.chiu@intel.com<mailto:chasel.chiu@intel.com>>
Subject: RE: UPL: UefiPayload depends on FIT FDT, but Platform Init has no reason to load it

Add Chasel.


From: Benjamin Doron <benjamin.doron00@gmail.com<mailto:benjamin.doron00@gmail.com>>
Sent: Tuesday, December 10, 2024 11:28 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>>; Rhodes, Sean <sean@starlabs.systems<mailto:sean@starlabs.systems>>; Lu, James <james.lu@intel.com<mailto:james.lu@intel.com>>; Liu, Linus <linus.liu@intel.com<mailto:linus.liu@intel.com>>; Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>
Subject: UPL: UefiPayload depends on FIT FDT, but Platform Init has no reason to load it

Hi,

In https://github.com/tianocore/edk2/blob/a0ac7cf/UefiPayloadPkg/UefiPayloadEntry/FitUniversalPayloadEntry.c#L220, UefiPayload parses its own UPL FIT's FDT to determine where the other FVs can be found (if you follow the references backwards, you'll see that gUniversalPayloadBaseGuid HOBs come from `upl-images@<addr>`. I don't love this design, but the alternative is to have Platform Init copy in the `images` node from the FIT FDT to the UPL FDT, which may not be any better.

The problem is that currently, Platform Init needs to have hardcoded logic to load the entire .FIT file to where the entrypoint image points. See below:

FDT dump of UPL FIT:
images {
    tianocore {
        description = "Uefi Universal Payload";
        project = "tianocore";
        arch = "x86";
        type = "flat-binary";
        producer = "intel";
        data-offset = <0x00001000>;
        data-size = <0x00016000>;
        reloc-start = <0x00012000>;
        entry-start = <0x00000000 0x00807c98>;
        load = <0x00000000 0x00800000>;
    };
    // more images here
};

UefiPayloadPkg/UniversalPayloadBuild.py:

        RunCommand (
            "GenFw --rebase 0x{:02X} -o {} {} ".format (
              fit_image_info_header.LoadAddr + fit_image_info_header.DataOffset,
              TargetRebaseFile,
              TargetRebaseFile,
            ))

So, the entrypoint is supposed to be located 0x800000 + 0x1000, not 0x800000. The first 0x1000 is the FDT, but a Platform Init simply complying with the FIT spec does not know that. We would have this problem in coreboot.

To fix this issue, I propose we emit another 'image' into the FIT's FDT, called "upl-fit-fdt". Then, we can shift the entrypoint's load value back where it should be (0x1000 greater than it is now), and platform init will copy it without needing hardcoded logic.
(I'm aware that a patch would be needed somewhere in UefiPayload - I believe FitUniversalPayloadEntry but it might be FitParserLib - to make sure that this image or this classification of images are not turned into FV HOBs inside EDK2)

What do you think?

Best regards,
Benjamin


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120895): https://edk2.groups.io/g/devel/message/120895
Mute This Topic: https://groups.io/mt/110019777/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 12841 bytes --]