Rebase onto edk2-stable202402 #129
Conversation
SergiiDmytruk
force-pushed
the
rebased
branch
2 times, most recently
from
d20efd4 to
3934a77
Compare
|
Looks pretty good already, boots on NovaCustom V540TU (with #136) Some issues I've noticed already:
|
|
It doesn't work on coreboot with edk2 as a payload on QEMU, failing very early: @SergiiDmytruk any idea what may be causing it? I haven't seen too much changes in such early code done by us, so maybe this is something that changed upstream. On a tangent note, there is |
|
@mkopec, thanks, will take a look whether these are caused by a missing commit, conflict resolution gone wrong or new base needs somewhat different changes.
Not really, seems like upstream changes. Weird thing is that I just tried using rebased EDK on APU6 to check whether RAM size is reported and saw exactly the same asserts. I'd expect this to either work or not work for
Don't we need PEI for TPM drivers of EDK? IIUC UefiPayloadPkg in upstream now has no PEI which is why it can't be used by us without reworking code somehow. |
Well, maybe indirectly... The code is old, but something must be different. The error occurs at |
|
It shouldn't fail there for OVMF which uses its own |
|
So After working around that APU is able to boot, although it seems to have issues with input/output (no boot menu, can't enter setup UI). Q35 seems to act similar to APU. Should probably find out what caused this new dependency in the first place to find a proper mitigation. |
There is probably no specific reason. Older version happened to work without trouble in the configuration used by coreboot and the new one just doesn't because code has changed. The workaround in 5ffa33a is probably the way to go. |
SergiiDmytruk
force-pushed
the
rebased
branch
2 times, most recently
from
d2f19a9 to
d471c3f
Compare
|
Looks like issue with serial on APU was really an issue with serial consoles in rebased version, I just used KVM and didn't notice it initially. Turned out that 35e5619 doesn't actually revert 4628519. The latter moves @krystian-hebel, Q35 seems to work now. @mkopec, I see RAM size on MSI Z690-A DDR5, Q35 and APU4. Was it displayed before the rebase?
I think this could have been fixed along with serial output, PS/2 keyboard seems to be registered in
No "Device Manager" -> "TCG2 Configuration" menu? Was present on MSI but not on APU4 due to TPM error: Turned out that f6ba45d commit was necessary, at least APU now detects TPM and probably NovaCustom V540TU.
Still couldn't find what was suppressing it from appearing on "One time boot" menu. |
Is it possible to add |
Doesn't work when I try:
Not sure how hard it would be to adjust coreboot to be more flexible, it must have trouble finding entrypoint. And in the end it might not help, because APRIORI PEI changes ordering in which PEIMs get processed and those asserts followed by segfault happen before PEIMs dispatcher even starts. The timer calls probably happen due to performance measurement code somewhere running as part of PEI core. |
|
Few remaining asserts can be removed by dropping It still fails for me on Q35, although much further: After removing I'm not sure how image lookup works, but according to logs 0x7039D18 isn't
|
|
Interestingly, after changing the amount of RAM I got different panic: |
Well, I assumed it was added for a purpose hence the workaround instead of removing performance measurements.
Weird given that it works for me on your Kconfig change, defconfig, bash script
One more change is diff --git a/src/vendorcode/intel/Kconfig b/src/vendorcode/intel/Kconfig
index b481aa0090b..f09aebea2cd 100644
--- a/src/vendorcode/intel/Kconfig
+++ b/src/vendorcode/intel/Kconfig
@@ -8,6 +8,7 @@ config UEFI_2_4_BINDING
select UDK_BASE
config UDK_2017_BINDING
+ def_bool y if BOARD_EMULATION_QEMU_X86_Q35
def_bool n
select UDK_BASE
defconfig: Script: #!/bin/bash
params=(
-M q35,smm=on,max-fw-size=16M
-enable-kvm
-m 4096
-drive if=pflash,format=raw,unit=0,file=build/coreboot.rom
-serial stdio
-action reboot=shutdown
)
exec qemu-system-x86_64 "${params[@]}" |
|
TPM and PS/2 KB issues are fixed, thanks.
I saw it on a different device with different DIMM modules, but when testing on the same unit, it's not displayed on old and new edk2. It's probably an issue with MTL and DDR5, not with EDK2 |
|
Interesting, it seems to work with
|
|
One more problem on QEMU, which may be connected to a problem that @miczyg1 was debugging on Protectli. By checking CR2 address against log of loaded EFI modules, I found out that this is
By comparing code from Linux's panic (in particular address modulo page size) it seems to be called from This code tries to use Boot Services after call to I haven't checked whether this is something limited to Dasharo or if it applies to upstream edk2 as well. |
|
Ubuntu 24.04 works slightly better (above was on 22.04), it can read UEFI variables, but not write to them nor create new ones. There is no oops in dmesg. I think it may use static copy of variables obtained on boot, or something else changed in the way efivars are used in newer kernel. |
Could be. I build with my system's 14.1.0.
Status = gBS->AllocatePages (AllocateAnyPages, MemoryType, Pages, &Memory);I'm not sure if page allocation should be done by runtime services though, maybe the invoking code is wrong. Also I don't see a more suitable memory library. |
|
I'm getting the same behavior on MTL-H laptop, with expected deviations caused by different memory layout. It worked fine with older edk2 before rebase. I've looked at the recent changes done to RuntimeDxe and didn't see anything obviously wrong with them. It may indeed be the calling code. Some of those allocations are to be done once, on first call, maybe this is what's missing. On the other hand, this tries to allocate BootServicesData... Maybe it was always broken, but worked by accident because allocated pool was on the same page as RuntimeServicesData, but IIRC UEFI allocates whole pages event for AllocatePool. There is also |
|
Was very puzzled on seeing the crash even after adding The failure occurs when Ubuntu updates OpenSSL version uses |
Actually, a copy of it exists and is used for MbedTLS as well, but it wasn't linked to PKCS7 implementation. This was easy to fix, however code of MbedTLS also invokes Updating that to unconditional |
|
That worked (no more crashes in |
Reused the same libraries. There are no more page faults, but EFI variables aren't writable as in 24.04 case. The reason is that SMI handler doesn't respond and EDK produces SMI handler doesn't work in runtime phase because Linux executes I tried to expose two tables from EDK to tell Linux to not relocate EFI runtime and deprecated So I attempted to make coreboot run in long mode by setting By the way, I know that it's virtual addressing because if I break To sum up:
|
This is a follow up for 6ca2060 and 784750e which provide more detailed information on the issue and how this addresses it. The files modified by this commit were chosen based on the list of sources in CryptoPkg/Library/BaseCryptLibMbedTls/RuntimeCryptLib.inf The only source permitted to request memory from boot services is SysCall/RuntimeMemAllocation.c which does it in constructor before ExitBootServices() is called. Trying to update minimal set of files because some of the API which does allocations get used outside of BaseCryptLibMbedTls and can do FreePool(). In the updated files, allocations are of two types: - temporary allocations within a function (they don't get returned or set to some output parameter) - paired alloc/free kind of functions which remain in control of how the memory is treated Signed-off-by: Sergii Dmytruk <[email protected]>
Yes, apologies for inaccurate wording. So we would want to proceed as described here: #129 (review) Can we do that? Let's gather confirmation from @mkopec and @krystian-hebel and proceed if there are no problems with that. |
Yes, current
Probably won't hurt to merge #163 first (if nobody else is going to test/review it) and then anyone who can modify protected branches can do the update. |
|
@miczyg1 I will leave this one to you then. |
What is 168? |
It's from the output of |
Not exactly a tag, but based on stable202002 + something/ I don't think 168 introduces anything meaningful to branch name. I think |
It's not a big deal either way. |
This is a port of upstream commits c248802 and bfefdc2 On modern platforms with TBT devices the coreboot resource allocator opens large PCI bridge MMIO windows above 4GiB to place hotplugable PCI BARs there as they won't fit below 4GiB. In addition modern GPGPU devices have very big PCI bars that doesn't fit below 4GiB. The PciHostBridgeLib made lots of assumptions about the coreboot resource allocator that were not verified at runtime and are no longer true. Remove all of the 'coreboot specific' code and implement the same logic as OvmfPkg's ScanForRootBridges. Fixes assertion "ASSERT [PciHostBridgeDxe] Bridge->Mem.Limit < 0x0000000100000000ULL". Co-authored-by: Patrick Rudolph <[email protected]> Signed-off-by: Michał Kopeć <[email protected]>
Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction writes to TPM2 physical presence PPI provided by coreboot (a memory region preserved across reboots). CPU caches must be explicitly flushed prior to platform reboot or request written to PPI will be lost. Signed-off-by: Artur Kowalski <[email protected]>
Flush cache not only when placing request in PPI, but also after clearing old request from PPI. Signed-off-by: Artur Kowalski <[email protected]>
|
Almost forgot to provide an observation regarding The takeaway is that we can reduce the difference between the two packages over time by gradually switching to libraries/DXEs of |
wiktormowinski
force-pushed
the
rebased
branch
from
cf57576 to
8eccede
Compare
It has been observed on MinnowBoard Turbot that the detected CPU count is lower than the number of all cores. In the tested unit, CPU is dual core, so only the BSP is detected. However, similar situation is observed on MTL laptop, where a total of 22 cores should be reported, but only 1 is detected. After EDK2 rebase a new PCD has been added, PcdFirstTimeWakeUpAPsBySipi which, when enabled (by default), sends only SIPI to APs. When disabled sends full INIT-SIPI-SIPI sequence. Only the latter case causes all the APs to wake up and be detected. Signed-off-by: Michał Żygowski <[email protected]> Signed-off-by: Sergii Dmytruk <[email protected]>
SergiiDmytruk
force-pushed
the
rebased
branch
from
8eccede to
6b645e4
Compare
Signed-off-by: Wiktor Mowinski <[email protected]>
This fixes "SecurityPkg: measure Dasharo variables before boot". gRT->GetNextVariableName() doesn't return variables in any fixed order. Seems like the order matches order in SMMSTORE. This means that measuring variables while enumerating them will produce different results depending on which variables were update last (setting a variable in SMMSTORE is marking old entry as deleted and appending of a new one). Sort list of variables that share the same GUID before measuring any of them to impose a fixed order. Also fix spacing in several places. Signed-off-by: Sergii Dmytruk <[email protected]>
Signed-off-by: Michał Żygowski <[email protected]>
…hout spaces Signed-off-by: Michał Żygowski <[email protected]>
Signed-off-by: Michał Żygowski <[email protected]>
|
@SergiiDmytruk @miczyg1 I am confused, is there any conclusion of this discussion and how to approach with finalizing the work here? |
|
I think the branches could have been updated long time ago. Even if something else from |
Nothing to finalize, except branch name changes. The rebased branch is already used in production and is a part of some releases... |
|
Changed branches:
Closing this PR and removing branches: |
Not a real PR, since this is a rebase (Dasharo/dasharo-issues#432), but could still be used for comments.
Things of note:
BaseDebugLibSerialPortnow causes build failures due to build cycles and its use in one case was meant to fix an issue with QubesOSrevert that commit: 4df31e5(technically it is reverted)PeiDxeDebugLibReportStatusCodeby making it do nothing in runtime phase 6390028dependency cycle: d13b2f3
PcdResetOnMemoryTypeInformationChangeneeds to be disabled for boot to workIssues discovered while testing that must be resolved:
HobList != ((void *) 0)assertions in coreboot+OVMF and at least on APU6 (MSI and NovaCustom V540TU somehow worked). Rebase onto edk2-stable202402 #129 (comment)