Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable
Hey Ard, On 03.01.23 10:59, Ard Biesheuvel wrote: On Thu, 29 Dec 2022 at 19:00, dann frazier wrote: On Mon, Nov 28, 2022 at 04:46:10PM +0100, Gerd Hoffmann wrote: On Mon, Sep 26, 2022 at 10:24:58AM +0200, Ard Biesheuvel wrote: When the memory protections were implemented and enabled on ArmVirtQemu 5+ years ago, we had to work around the fact that GRUB at the time expected EFI_LOADER_DATA to be executable, as that is the memory type it allocates when loading its modules. This has been fixed in GRUB in August 2017, so by now, we should be able to tighten this, and remove execute permissions from EFI_LOADER_DATA allocations. Data point: https://bugzilla.redhat.com/show_bug.cgi?id=2149020 tl;dr: fedora 37 grub.efi is still broken. This is also the case with existing Ubuntu releases, as well as AlmaLinux 9.1 and RHEL 8.7[*]. While it does appear to be fixed for the upcoming Ubuntu 23.04 (presumably via [**]), I plan to revert this patch in Debian/Ubuntu until it is more ubiquitous. Do you want to do the same upstream? I'm not sure at what point it would make sense to reintroduce it, given we can't force users to upgrade their bootloaders. Thanks for the report. You can override PCDs on the build command line, so I suggest you use that for building these images as long as it is needed. E.g,, append this to the build.sh command line --pcd PcdDxeNxMemoryProtectionPolicy=0xC0007FD1 to undo the effects of this patch. I do not intend to revert this patch - the trend under EFI is towards much stricter memory permissions, also on the MS side, and this is especially important under CC scenarios. And if 5+ years is not sufficient for out-of-tree GRUB to catch up, what is the point of waiting for it? I think we need to be smarter here. ArmVirtPkg is used by a lot of virtualization software - such as QEMU. Typically, users (and developers) expect that an update to a newer version will preserve compatibility. Let me contrive an example: In 1 year, QEMU updates to the latest AAVMF. It ships that as part of its pc-bios directory. Suddenly, we accidentally break old (immutable!) iso images that used to work. So someone that updates QEMU to a newer version will have a regression in running a Fedora 37 installation. Or RHEL 8.7 installation. Not good :). Outside of OSVs providing new iso images, there is also not much you can do about this. The grub binary here runs way before any updater code that could pull a fixed version from the internet. What alternatives do we have? Assuming grub is the only offender we have to worry about, is there a way we can identify that the allocation came from an unpatched version? Or have a database of hashes (with all known grub binaries that have this bug in existing isos) that would force disable NX protection for it if we hit it? Or disable NX when Secure Boot is disabled? I really think we need to be a bit more creative than make NX mandatory in all cases when we know the are immutable images out there that won't work with it, otherwise we break very legit use cases. Alex -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97890): https://edk2.groups.io/g/devel/message/97890 Mute This Topic: https://groups.io/mt/93922691/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable
On 04.01.23 10:35, Ard Biesheuvel wrote: On Tue, 3 Jan 2023 at 23:47, dann frazier wrote: On Tue, Jan 03, 2023 at 08:39:24PM +0100, Alexander Graf wrote: Hey Ard, On 03.01.23 10:59, Ard Biesheuvel wrote: On Thu, 29 Dec 2022 at 19:00, dann frazier wrote: On Mon, Nov 28, 2022 at 04:46:10PM +0100, Gerd Hoffmann wrote: On Mon, Sep 26, 2022 at 10:24:58AM +0200, Ard Biesheuvel wrote: When the memory protections were implemented and enabled on ArmVirtQemu 5+ years ago, we had to work around the fact that GRUB at the time expected EFI_LOADER_DATA to be executable, as that is the memory type it allocates when loading its modules. This has been fixed in GRUB in August 2017, so by now, we should be able to tighten this, and remove execute permissions from EFI_LOADER_DATA allocations. Data point: https://bugzilla.redhat.com/show_bug.cgi?id=2149020 tl;dr: fedora 37 grub.efi is still broken. This is also the case with existing Ubuntu releases, as well as AlmaLinux 9.1 and RHEL 8.7[*]. While it does appear to be fixed for the upcoming Ubuntu 23.04 (presumably via [**]), I plan to revert this patch in Debian/Ubuntu until it is more ubiquitous. Do you want to do the same upstream? I'm not sure at what point it would make sense to reintroduce it, given we can't force users to upgrade their bootloaders. Thanks for the report. You can override PCDs on the build command line, so I suggest you use that for building these images as long as it is needed. E.g,, append this to the build.sh command line --pcd PcdDxeNxMemoryProtectionPolicy=0xC0007FD1 to undo the effects of this patch. I do not intend to revert this patch - the trend under EFI is towards much stricter memory permissions, also on the MS side, and this is especially important under CC scenarios. And if 5+ years is not sufficient for out-of-tree GRUB to catch up, what is the point of waiting for it? I think we need to be smarter here. ArmVirtPkg is used by a lot of virtualization software - such as QEMU. Typically, users (and developers) expect that an update to a newer version will preserve compatibility. Let me contrive an example: In 1 year, QEMU updates to the latest AAVMF. It ships that as part of its pc-bios directory. Suddenly, we accidentally break old (immutable!) iso images that used to work. So someone that updates QEMU to a newer version will have a regression in running a Fedora 37 installation. Or RHEL 8.7 installation. Not good :). Outside of OSVs providing new iso images, there is also not much you can do about this. The grub binary here runs way before any updater code that could pull a fixed version from the internet. What alternatives do we have? Assuming grub is the only offender we have to worry about, is there a way we can identify that the allocation came from an unpatched version? Or have a database of hashes (with all known grub binaries that have this bug in existing isos) that would force disable NX protection for it if we hit it? Or disable NX when Secure Boot is disabled? I really think we need to be a bit more creative than make NX mandatory in all cases when we know the are immutable images out there that won't work with it, otherwise we break very legit use cases. While it has its own issues, I suppose another way to go about it is for distributors to provide multiple AAVMF_CODE images, and perhaps invent a "feature" flag for the json descriptor for management tools to select an appropriate one. I don't think having different versions of the image makes sense, tbh, but of course, this is up to the distros. Compatibility with ancient downstream GRUB builds is not a goal of the EDK2 upstream, so as long as distros can tweak the build to their needs, I don't see a reason to revert this change upstream. First of all, I don't think we should revert this change. We should augment it to give users the out-of-the-box experience they expect. On top of that, I don't think it's a problem of "distros". Every consumer of AAVMF will run into this problem as soon as their users will want to run any Red Hat OS (installer / image) all the way into 2022. That's very likely >90% of the user base. Because of that, I'm pretty sure no Cloud vendor will be able to enable NX in its current shape for example. I'm very happy to see NX proliferate through the stack, but let's please make sure we do it compatibly by default, otherwise the net result is that *everyone* who compiles AAVMF will disable NX by default again - and all you end up with is more frustration and more downstream code / forks. IMHO the most obvious approach would be a fingerprint based override. There should be a finite number of known broken grub binaries. If we just maintain a database with them and then apply some magic when we detect such a binary gets loaded, we'll have tightened security by default, without breaking backwards compat.
Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable
On 04.01.23 14:13, Alexander Graf wrote: On 04.01.23 10:35, Ard Biesheuvel wrote: On Tue, 3 Jan 2023 at 23:47, dann frazier wrote: On Tue, Jan 03, 2023 at 08:39:24PM +0100, Alexander Graf wrote: Hey Ard, On 03.01.23 10:59, Ard Biesheuvel wrote: On Thu, 29 Dec 2022 at 19:00, dann frazier wrote: On Mon, Nov 28, 2022 at 04:46:10PM +0100, Gerd Hoffmann wrote: On Mon, Sep 26, 2022 at 10:24:58AM +0200, Ard Biesheuvel wrote: When the memory protections were implemented and enabled on ArmVirtQemu 5+ years ago, we had to work around the fact that GRUB at the time expected EFI_LOADER_DATA to be executable, as that is the memory type it allocates when loading its modules. This has been fixed in GRUB in August 2017, so by now, we should be able to tighten this, and remove execute permissions from EFI_LOADER_DATA allocations. Data point: https://bugzilla.redhat.com/show_bug.cgi?id=2149020 tl;dr: fedora 37 grub.efi is still broken. This is also the case with existing Ubuntu releases, as well as AlmaLinux 9.1 and RHEL 8.7[*]. While it does appear to be fixed for the upcoming Ubuntu 23.04 (presumably via [**]), I plan to revert this patch in Debian/Ubuntu until it is more ubiquitous. Do you want to do the same upstream? I'm not sure at what point it would make sense to reintroduce it, given we can't force users to upgrade their bootloaders. Thanks for the report. You can override PCDs on the build command line, so I suggest you use that for building these images as long as it is needed. E.g,, append this to the build.sh command line --pcd PcdDxeNxMemoryProtectionPolicy=0xC0007FD1 to undo the effects of this patch. I do not intend to revert this patch - the trend under EFI is towards much stricter memory permissions, also on the MS side, and this is especially important under CC scenarios. And if 5+ years is not sufficient for out-of-tree GRUB to catch up, what is the point of waiting for it? I think we need to be smarter here. ArmVirtPkg is used by a lot of virtualization software - such as QEMU. Typically, users (and developers) expect that an update to a newer version will preserve compatibility. Let me contrive an example: In 1 year, QEMU updates to the latest AAVMF. It ships that as part of its pc-bios directory. Suddenly, we accidentally break old (immutable!) iso images that used to work. So someone that updates QEMU to a newer version will have a regression in running a Fedora 37 installation. Or RHEL 8.7 installation. Not good :). Outside of OSVs providing new iso images, there is also not much you can do about this. The grub binary here runs way before any updater code that could pull a fixed version from the internet. What alternatives do we have? Assuming grub is the only offender we have to worry about, is there a way we can identify that the allocation came from an unpatched version? Or have a database of hashes (with all known grub binaries that have this bug in existing isos) that would force disable NX protection for it if we hit it? Or disable NX when Secure Boot is disabled? I really think we need to be a bit more creative than make NX mandatory in all cases when we know the are immutable images out there that won't work with it, otherwise we break very legit use cases. While it has its own issues, I suppose another way to go about it is for distributors to provide multiple AAVMF_CODE images, and perhaps invent a "feature" flag for the json descriptor for management tools to select an appropriate one. I don't think having different versions of the image makes sense, tbh, but of course, this is up to the distros. Compatibility with ancient downstream GRUB builds is not a goal of the EDK2 upstream, so as long as distros can tweak the build to their needs, I don't see a reason to revert this change upstream. First of all, I don't think we should revert this change. We should augment it to give users the out-of-the-box experience they expect. On top of that, I don't think it's a problem of "distros". Every consumer of AAVMF will run into this problem as soon as their users will want to run any Red Hat OS (installer / image) all the way into 2022. That's very likely >90% of the user base. Because of that, I'm pretty sure no Cloud vendor will be able to enable NX in its current shape for example. I'm very happy to see NX proliferate through the stack, but let's please make sure we do it compatibly by default, otherwise the net result is that *everyone* who compiles AAVMF will disable NX by default again - and all you end up with is more frustration and more downstream code / forks. IMHO the most obvious approach would be a fingerprint based override. There should be a finite number of known broken grub binaries. If we just maintain a database with them and then apply some magic when we detect such a binary gets loaded, we'll ha
Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable
> Am 05.01.2023 um 09:11 schrieb Gerd Hoffmann : > > Hi, > >> To clarify, I mean something like the patch below, but with an additional >> callback notification similar to the Emu one in LoadImage(), so that we can >> make sure we only enable the quirk when we load a known-bad grub binary. >> That way we still force distros to ship fixed versions of their code, but >> enable old code to continue running. > >> + /* TODO: Only run this as part of a notify callback in ImageLoad() when >> we >> + load a grub binary with a known-broken hash */ >> + BOOLEAN is_broken_grub = TRUE; >> + if (is_broken_grub) { >> +RealAllocatePages = gBS->AllocatePages; >> +gBS->AllocatePages = AllocatePagesForceLoaderCode; >> + } > > You left out the hard part, which is the list of hashes. Yes, I'd crowd source that list. If someone has vested interest to keep their old grub binaries working, they can send an upstream patch to add their hash :). At least we'd have a path forward to make things work that is not "revert NX enablement". > And I suspect > you underestimate the number of broken grub binaries in the wild ... What number would you expect? I'd hope that we get to <100 realistically. I'm happy to hear about alternatives to this approach. I'm very confident that forcing NX on always is going to have the opposite effect of what we want: Everyone who ships AAVMF binaries will disable NX because they eventually get bug reports from users that their shiny update regressed some legit use case. The only alternative I can think of would be logic similar to the patch I sent without any grub hash check: Exclude AllocatePages for LoaderData from the NX logic. Keep NX for any other non-code memory type as well as LoaderData pool allocations. Alex -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97977): https://edk2.groups.io/g/devel/message/97977 Mute This Topic: https://groups.io/mt/93922691/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable
> Am 05.01.2023 um 12:44 schrieb Ard Biesheuvel : > > On Thu, 5 Jan 2023 at 12:19, Gerd Hoffmann wrote: >> >> Hi, >> What number would you expect? I'd hope that we get to <100 realistically. I'm happy to hear about alternatives to this approach. I'm very confident that forcing NX on always is going to have the opposite effect of what we want: Everyone who ships AAVMF binaries will disable NX because they eventually get bug reports from users that their shiny update regressed some legit use case. The only alternative I can think of would be logic similar to the patch I sent without any grub hash check: Exclude AllocatePages for LoaderData from the NX logic. Keep NX for any other non-code memory type as well as LoaderData pool allocations. >> >>> Another thing we might consider is trapping exec permission violations >>> and switching the pages in question from rw- to r-x. >> >> That sounds neat, especially as we can print a big'n'fat warning in that >> case, so the problem gets attention without actually breaking users. >> > > That, and a sleep(5) I like the direction this is moving :) > >> Looking at the efi calls it looks like edk2 doesn't track the owner of >> an allocation (say by image handle), so I suspect it is not possible to >> automatically figure who is to blame? >> >>> Does GRUB generally load/map executable modules at page granularity? >> >> I don't think so, at least the code handles modules not being page >> aligned. But I think it's not grub modules, that fix was actually >> picked up meanwhile. But there are downstream patches for image >> loader code which look suspicious to me ... >> > > OK, so the GRUB PE/COFF loader strikes again :-( > > So shim may be broken in the exact same way, and the things shim loads > may not adhere to page alignment for the sections. Loading the kernel > itself this way should be fine, though - we always had section > alignment in the EFI stub kernel. > > The downside of this approach is that it can only be done on a > page-by-page basis, given that the EFI_LOADER_DATA region in question > will cover both the .text/.rodata and the .data/.bss of the loaded > image. Does it have to be r-x instead of rwx? If we add the warning and sleep(5), that should hopefully give enough incentive already to OSVs to fix their grub binaries :). And then we don't even need to think about alignment. If I map a region as LoaderCode, I get rwx as well, no? So we effectively make LoaderData behave like LoaderCode plus warning plus sleep. Alex > > Could someone check/confirm whether shim builds need to be take into > account here? Thanks. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98038): https://edk2.groups.io/g/devel/message/98038 Mute This Topic: https://groups.io/mt/93922691/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] ArmPkg/ArmMmuLib AARCH64: avoid EL0 accessible mappings
On 22.09.21 18:19, Ard Biesheuvel wrote: > We never run any code at EL0, and so it would seem that any access > permissions set for EL0 (via the AP[1] attribute in the page tables) are > irrelevant. We currently set EL0 and EL1 permissions to the same value > arbitrarily. > > However, this causes problems on hardware like the Apple M1 running the > hypervisor framework, which enters EL1 with SCTLR_EL1.SPAN enabled, > which causes the Privileged Access Never (PAN) feature to be enabled on > any exception taken to EL1, including the IRQ exceptions that handle our > timer interrupt. When PAN is enabled, EL1 has no access to any mappings > that are also accessible to EL0, causing the firmware to crash if it > attempts to access such a mapping. > > Even though it is debatable whether or not SCTLR_EL1.SPAN should be > disabled at entry or whether the firmware should put all UNKNOWN bits in > all system registers in a consistent state (which it should), using EL0 > permissions serves no purpose whatsoever so let's fix that regardless. > > Signed-off-by: Ard Biesheuvel I can confirm that this unbreaks HVF guests running on M1 with SCTLR_EL1.SPAN=0 as reset state. Tested-by: Alexander Graf Alex -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#81007): https://edk2.groups.io/g/devel/message/81007 Mute This Topic: https://groups.io/mt/85793856/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 0/2] Improve hibernation safety
Operating Systems that get hibernated expect all non-boot-time allocations to be identical before and after hibernation. In edk2, we create pools and allocate pages starting from the highest allowed address for the allocation, usually 0x. Typically, that means we allocate a few pages of boot time data, then a few pages of runtime data, then another few pages of boot time data and again runtime data. Every allocation has direct impact on the following allocations. The problem with this scheme is that small code changes in boot time code already can have significant impact on runtime allocations, which then break hibernation. This patch set adds a mechanism to set an upper bound to dynamic memory allocations for different allocation types. This allows us to move data that has to stay at the same place across firmware changes at the same place. The patch set also enables this on OVMF by default. Alex Alexander Graf (2): MdeModulePkg/Core/Dxe: Allow to force runtime allocations at separate range OvmfPkg: Make hibernation critical allocations at own ranges MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +++ MdeModulePkg/Core/Dxe/Mem/Page.c | 70 +++ MdeModulePkg/MdeModulePkg.dec | 16 + MdeModulePkg/MdeModulePkg.uni | 12 +++ OvmfPkg/OvmfPkgIa32.dsc | 6 OvmfPkg/OvmfPkgIa32X64.dsc| 6 OvmfPkg/OvmfPkgX64.dsc| 6 7 files changed, 120 insertions(+) -- 2.16.4 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71790): https://edk2.groups.io/g/devel/message/71790 Mute This Topic: https://groups.io/mt/80739500/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/2] OvmfPkg: Make hibernation critical allocations at own ranges
Now that we have a framework available to set memory ranges for allocations that break hibernation if they move, let's push them to their own respective memory ranges. This way, they will be unaffected by boot time data allocation changes and we can thus still resume hibernated systems. Signed-off-by: Alexander Graf --- OvmfPkg/OvmfPkgIa32.dsc| 6 ++ OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++ OvmfPkg/OvmfPkgX64.dsc | 6 ++ 3 files changed, 18 insertions(+) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 1b8d34052b..afea65254d 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -575,6 +575,12 @@ # Point to the MdeModulePkg/Application/UiApp/UiApp.inf gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 } + # Simplify hibernation safety by putting relevant data into its own memory ranges + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIReclaimMemory|0x1900 + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIMemoryNVS|0x1910 + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesCode|0x1920 + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesData|0x1930 + # # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 9c1aee87e7..4d1334554a 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -581,6 +581,12 @@ # Point to the MdeModulePkg/Application/UiApp/UiApp.inf gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 } + # Simplify hibernation safety by putting relevant data into its own memory ranges + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIReclaimMemory|0x1900 + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIMemoryNVS|0x1910 + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesCode|0x1920 + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesData|0x1930 + # # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index fabb8b2f29..22cdf71f1e 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -581,6 +581,12 @@ # Point to the MdeModulePkg/Application/UiApp/UiApp.inf gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 } + # Simplify hibernation safety by putting relevant data into its own memory ranges + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIReclaimMemory|0x1900 + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIMemoryNVS|0x1910 + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesCode|0x1920 + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesData|0x1930 + # # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform -- 2.16.4 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71791): https://edk2.groups.io/g/devel/message/71791 Mute This Topic: https://groups.io/mt/80739507/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/2] MdeModulePkg/Core/Dxe: Allow to force runtime allocations at separate range
Operating Systems that get hibernated expect all non-boot-time allocations to be identical before and after hibernation. In edk2, we create pools and allocate pages starting from the highest allowed address for the allocation, usually 0x. Typically, that means we allocate a few pages of boot time data, then a few pages of runtime data, then another few pages of boot time data and again runtime data. Every allocation has direct impact on the following allocations. The problem with this scheme is that small code changes in boot time code already can have significant impact on runtime allocations, which then break hibernation. This patch adds a mechanism to override the MaxAddress for runtime allocations with a target defined Pcd value. With this feature enabled, we can have different allocation ranges for runtime and boot time allocations. This allows us to determine at boot time whether to load an old, hibernation compatible runtime allocation path or a new, hibernation unsafe runtime allocation. All within the same edk2 target binary. It also allows us to modify boot time behavior, such as modifying buffer allocation mechanisms without compromising on hibernation safety. Signed-off-by: Alexander Graf --- MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +++ MdeModulePkg/Core/Dxe/Mem/Page.c | 70 +++ MdeModulePkg/MdeModulePkg.dec | 16 + MdeModulePkg/MdeModulePkg.uni | 12 +++ 4 files changed, 102 insertions(+) diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index e4bca89577..0696246970 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -186,6 +186,10 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIReclaimMemory ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIMemoryNVS ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesCode## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesData## CONSUMES # [Hob] # RESOURCE_DESCRIPTOR ## CONSUMES diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c index 731bf08bc9..91599adccb 100644 --- a/MdeModulePkg/Core/Dxe/Mem/Page.c +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c @@ -1007,6 +1007,74 @@ CoreUpdateMemoryAttributes ( CoreReleaseMemoryLock (); } +UINT64 +EnforceMaxAddress ( + IN UINT64 MaxAddress, + IN EFI_MEMORY_TYPE NewType, + IN UINT64 NumberOfPages + ) +{ + UINT64 NumberOfBytes = LShiftU64 (NumberOfPages, EFI_PAGE_SHIFT); + UINT64 LowestPossible = MaxAddress; + UINT64 ForceMaxAddress; + LIST_ENTRY *Link; + MEMORY_MAP *Entry; + + switch (NewType) { + case EfiACPIReclaimMemory: +ForceMaxAddress = PcdGet64(PcdEnforceMaxACPIReclaimMemory); +break; + case EfiACPIMemoryNVS: +ForceMaxAddress = PcdGet64(PcdEnforceMaxACPIMemoryNVS); +break; + case EfiRuntimeServicesCode: +ForceMaxAddress = PcdGet64(PcdEnforceMaxEfiRuntimeServicesCode); +break; + case EfiRuntimeServicesData: +ForceMaxAddress = PcdGet64(PcdEnforceMaxEfiRuntimeServicesData); +break; + default: +ForceMaxAddress = MaxAddress; +break; + } + + // + // The currently requested address already fits our forced max constraint? + // Great, let's use that then. + // + if (ForceMaxAddress >= MaxAddress) { +return MaxAddress; + } + + // + // Check if the allocation would fit. If not, don't force it. + // + for (Link = gMemoryMap.ForwardLink; Link != &gMemoryMap; Link = Link->ForwardLink) { +Entry = CR (Link, MEMORY_MAP, Link, MEMORY_MAP_SIGNATURE); + +// +// If it's not a free entry, don't bother with it +// +if (Entry->Type != EfiConventionalMemory) { + continue; +} + +if ((Entry->Start < LowestPossible) && +((Entry->End - Entry->Start) >= NumberOfBytes)) { + LowestPossible = Entry->End; +} + } + DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "Force=%lx Lowest=%lx Max=%lx\n", ForceMaxAddress, LowestPossible, MaxAddress)); + + // + // We don't have free RAM available in the desired target area? Bail out! + // + if (ForceMaxAddress < LowestPossible) { +return MaxAddress; + } + + return ForceMaxAddress; +} /** Internal function. Finds a consecutive free page range below @@ -1041,6 +1109,8 @@ CoreFindFreePagesI ( LIST_ENTRY *Link; MEMORY_MAP *Entry; + MaxAddress = EnforceMaxAddress(MaxAddress, NewType, NumberOfPages); + if ((MaxAddress < EFI_PAGE_MASK) ||(NumberOfPages == 0)) { return 0; } diff --git a/MdeModuleP
Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/Core/Dxe: Allow to force runtime allocations at separate range
Hi Mike, Thanks a lot for the pointer! In my case, the defaults for the preallocated memory regions was just not big enough to prevent hibernation breakage. I increased them now as intended and everything works like a charm. Please ignore this patch set, the existing mechanism is definitely superior :) Thanks! Alex On 18.02.21 23:28, Kinney, Michael D wrote: Hi Alex, This feature is already available from the DXE Core using the MemoryTypeInformation and was specifically added to support hibernation use case. There is an optional HOB that is passed into DXE Core that can provide bin sizes for any supported memory types. Not just Runtime and ACPI. This feature can use a fixed HOB or use a dynamic HOB from a UEFI Variable, so that changes in the memory usage of the critical memory types for hibernation can be tracked and stored in the UEFI Variables and be used to populate the HOB. It is a platform choice to use fixed or dynamic HOB. https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Guid/MemoryTypeInformation.h https://github.com/tianocore/edk2/blob/978b9d511f5b9cb7bc5b09749f86c39bec51525d/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L2233 https://github.com/tianocore/edk2/blob/978b9d511f5b9cb7bc5b09749f86c39bec51525d/MdeModulePkg/Core/Dxe/Mem/Page.c#L45 The DXE Core use the HOB to pre-allocate bins for the memory types and do allocations from those bins. The UEFI Memory Map provides an entry for the entire bin (no just the allocated space), so small differences in allocations from boot to boot do not change the UEFI memory map. This also reduces the total number of memory map entries in the UEFI memory map. If dynamic HOB is used, then the Boot Manager compares the actual memory usage to the HOB. If the usage is larger or smaller by more than a threshold, then the UEFI Variable is updated. The platform has the choice to reboot or continue booting after this UEIF Variable update based on a PCD. The reboot can help make sure the first boot to the OS has the right bin sizes to support future hibernate boot flows. Mike -Original Message- From: devel@edk2.groups.io On Behalf Of Alexander Graf via groups.io Sent: Thursday, February 18, 2021 12:10 PM To: devel@edk2.groups.io Cc: Leif Lindholm ; Laszlo Ersek ; Ard Biesheuvel ; Justen, Jordan L ; Woodhouse, David ; Hendrik Borghorst Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/Core/Dxe: Allow to force runtime allocations at separate range Operating Systems that get hibernated expect all non-boot-time allocations to be identical before and after hibernation. In edk2, we create pools and allocate pages starting from the highest allowed address for the allocation, usually 0x. Typically, that means we allocate a few pages of boot time data, then a few pages of runtime data, then another few pages of boot time data and again runtime data. Every allocation has direct impact on the following allocations. The problem with this scheme is that small code changes in boot time code already can have significant impact on runtime allocations, which then break hibernation. This patch adds a mechanism to override the MaxAddress for runtime allocations with a target defined Pcd value. With this feature enabled, we can have different allocation ranges for runtime and boot time allocations. This allows us to determine at boot time whether to load an old, hibernation compatible runtime allocation path or a new, hibernation unsafe runtime allocation. All within the same edk2 target binary. It also allows us to modify boot time behavior, such as modifying buffer allocation mechanisms without compromising on hibernation safety. Signed-off-by: Alexander Graf --- MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +++ MdeModulePkg/Core/Dxe/Mem/Page.c | 70 +++ MdeModulePkg/MdeModulePkg.dec | 16 + MdeModulePkg/MdeModulePkg.uni | 12 +++ 4 files changed, 102 insertions(+) diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index e4bca89577..0696246970 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -186,6 +186,10 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIReclaimMemory ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxACPIMemoryNVS ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesCode## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdEnforceMaxEfiRuntimeServicesData## CONSUMES # [Hob] # RESOURCE_DESCRIPTOR ## CONSUMES diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c index 731bf08bc9
Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/Core/Dxe: Allow to force runtime allocations at separate range
Hey Laszlo, On 19.02.21 17:47, Laszlo Ersek wrote: Hello Alex, On 02/19/21 15:10, Alexander Graf wrote: Hi Mike, Thanks a lot for the pointer! In my case, the defaults for the preallocated memory regions was just not big enough to prevent hibernation breakage. I increased them now as intended and everything works like a charm. The most recent related OVMF commits are, for reference (merged on 2020-May-08): 1 2c06e76bba06 OvmfPkg/PlatformPei: don't track BS Code/Data in default MemTypeInfo HOB 2 356b96b3a2dc OvmfPkg/PlatformPei: rewrite MemTypeInfo HOB production logic 3 8db87f98357b OvmfPkg/PlatformPei: extract memory type info defaults to PCDs 4 7b6327ff03bb OvmfPkg/PlatformPei: increase memory type info defaults The commit messages hopefully explain how OVMF utilizes the pattern described by Mike, and what PCDs to tweak if you need larger bins. If you think your PCD updates are upstreamable, please feel free to submit those as a patch! In a nutshell, I was missing 7b6327ff03bb. So unfortunately nothing upstreamable this time around - you beat me to it and I was just suffering from running UDK2018 ;) Alex Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71850): https://edk2.groups.io/g/devel/message/71850 Mute This Topic: https://groups.io/mt/80739508/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] ArmVirtPkg/ArmVirtMemoryInitPeiLib: avoid redundant cache invalidation
On 29.01.22 16:13, Ard Biesheuvel wrote: Alex reports that the cache invalidation performed by ArmVirtMemoryInitPeiLib takes a non-negligible amount of time at boot. This cache invalidation used to be necessary to avoid inconsistencies between the CPU's cached and uncached views of the permanent PEI memory region, given that the PEI phase is where the MMU gets enabled. The only allocations done from permanent PEI memory with the MMU off are pages used for page tables, and since commit 748fea6279ef ("ArmPkg/ArmMmuLib AARCH64: invalidate page tables before populating them"), each of those is invalidated in the caches explicitly, for reasons described in the patch's commit log. All other allocations done in PEI are either from temporary PEI memory, which includes the stack, or from permanent PEI memory but after the MMU has been enabled. This means that the cache invalidation in ArmVirtMemoryInitPeiLib is no longer necessary, and can simply be dropped. Cc: Alexander Graf Signed-off-by: Ard Biesheuvel Reviewed-by: Alexander Graf Also, feel free to add Reported-by: Alexander Graf Thanks a bunch! Alex --- .../ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c| 9 - 1 file changed, 9 deletions(-) diff --git a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c index 022e13e762b6..98d90ad4200d 100644 --- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c +++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c @@ -91,15 +91,6 @@ MemoryPeim ( ); } - // - // When running under virtualization, the PI/UEFI memory region may be - // clean but not invalidated in system caches or in lower level caches - // on other CPUs. So invalidate the region by virtual address, to ensure - // that the contents we put there with the caches and MMU off will still - // be visible after turning them on. - // - InvalidateDataCacheRange ((VOID *)(UINTN)UefiMemoryBase, UefiMemorySize); - // Build Memory Allocation Hob InitMmu (); Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86251): https://edk2.groups.io/g/devel/message/86251 Mute This Topic: https://groups.io/mt/88767635/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 01/12] MdePkg: Add DebugBootlog header
We will shortly introduce a new bootlog framework. This commit contains the main library header that defines its data structure and public APIs. Signed-off-by: Alexander Graf --- MdePkg/Include/Library/DebugBootlog.h | 141 ++ 1 file changed, 141 insertions(+) create mode 100644 MdePkg/Include/Library/DebugBootlog.h diff --git a/MdePkg/Include/Library/DebugBootlog.h b/MdePkg/Include/Library/DebugBootlog.h new file mode 100644 index 00..5e6b05411c --- /dev/null +++ b/MdePkg/Include/Library/DebugBootlog.h @@ -0,0 +1,141 @@ +/** @file + Base Debug library for a RAM based boot log + It provides functions to store debug messages in RAM and make them available as + Bootlog Configuration Table. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. + Copyright (c) 2012, Red Hat, Inc. + Copyright (c) 2022, Amazon Development Center Germany GmbH. + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include + +/* + * The bootlog framework is a way to collect all boot logging streams from the + * system into a single, condensed and time correlated data format. Its purpose + * is to allow extraction and correlation of boot logs from within the Operating + * System. Unlike I/O based logging, it's very cheap to use and can be used to + * easily identify duration of boot time code paths. + * + * Its intended use in edk2 is as a print backend for DebugLib, however you can + * use it stand alone as well. To append an edk2 log line, call + * DebugBootlogAppend() with the UTF-8 encoded string to append. The framework + * will initialize the boot log and add a configuration table if not existent + * automatically. + * + * The bootlog entry point is the bootlog configuration table. That table + * contains a dynamic number of pointers to bootlog logs. Each of these logs + * belong to one component of the boot flow, such as edk2 or grub and contain a + * dynamic number of log entries, each with a time stamp and optional additional + * data. + * + * All fields are Little Endian. + * + * ___ + * | Bootlog Configuration Table | + * |---| + * | UINT32 Signature | "BTLG" | + * | UINT16 MaxLogs | Maximum number of logs this| + * | | configuration table can hold. | + * | | If you need more, create a | + * | | clone and reregister it. | + * | UINT16 NrLogs| Number of occupied Log slots | + * | BOOTLOG *Log[0] | ---+, + * | BOOTLOG *Log[..] | ---+| + * |___|| + * ___/ + * | Bootlog | <´ + * |---| + * | UINT32 Signature | "BTLH"| + * | CHAR8[4] Producer | "edk2" for edk2, boot binary | + * | | dependent.| + * | UINT16 ExtraHeaderType| 1 for edk2| + * | UINT8 ExtraHeaderSize| Bytes for ExtraHeader below | + * | UINT8 MsgExtraHeaderSize | Bytes for Extra header in | + * | | each log line | + * | UINT32 LastByte | Length of the log, starting | + * | | at &Signature | + * | UINT64 TicksPerSecond | Frequency of the timer for| + * | | each log line. Time at system | + * | | reset is 0 for all logs. | + * | ExtraHeader ExtraHeader | Depends on Type above ---+---, + * | BootlogMessage Msgs[] | Log lines ---+---|--, + * |___| | | + * _ / | + * | ExtraHeader (edk2) | <-´| + * |-|| + * | UINT32 AllocatedSize | Size of the allocation for the || + * | | current bootlog. Used internally || + * | | to resize on demand. || + * |_|| + * _/ + * | Bootlog Message (edk2) | <´ + * |-| + * | UINT32 ErrorLevel | edk2 defined ErrorLevel | + * | UINT64 Ticks | Ticks since power on| + * | C
[edk2-devel] [PATCH 02/12] MdePkg: Add Null BaseDebugBootlog
In some situations, we may not want to actually emit any boot log. This commit adds a null bootlog handler. We will use this in situations where we can not maintain a boot log, such as during the SEC phase. Signed-off-by: Alexander Graf --- .../BaseDebugBootlogNullLib.inf | 26 +++ .../BaseDebugBootlog/DebugBootlogNull.c | 32 +++ 2 files changed, 58 insertions(+) create mode 100644 MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogNullLib.inf create mode 100644 MdePkg/Library/BaseDebugBootlog/DebugBootlogNull.c diff --git a/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogNullLib.inf b/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogNullLib.inf new file mode 100644 index 00..0f650f6260 --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogNullLib.inf @@ -0,0 +1,26 @@ +## @file +# Null Debug library instance for a RAM based boot log. +# It provides function stubs for boot log that do nothing. +# +# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. +# Copyright (c) 2012, Red Hat, Inc. +# Copyright (c) 2022, Amazon Development Center Germany GmbH. All rights reserved. +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +# +## + +[Defines] + INF_VERSION= 0x00010005 + BASE_NAME = BaseDebugBootlogNull + FILE_GUID = DF934DA3-CD31-49FE-AF50-B3C87C79325F + MODULE_TYPE= BASE + VERSION_STRING = 1.0 + LIBRARY_CLASS = DebugBootlogLib + +[Sources] + DebugBootlogNull.c + +[Packages] + MdePkg/MdePkg.dec diff --git a/MdePkg/Library/BaseDebugBootlog/DebugBootlogNull.c b/MdePkg/Library/BaseDebugBootlog/DebugBootlogNull.c new file mode 100644 index 00..fbb304e6e2 --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/DebugBootlogNull.c @@ -0,0 +1,32 @@ +/** @file + Null Debug library instance for a RAM based boot log. + It provides function stubs for boot log that do nothing. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. + Copyright (c) 2012, Red Hat, Inc. + Copyright (c) 2022, Amazon Development Center Germany GmbH. + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include + +UINT32 +EFIAPI +GetDebugBootlogErrorLevel ( + VOID + ) +{ + return 0; +} + +RETURN_STATUS +EFIAPI +DebugBootlogAppend ( + IN CONST CHAR8 *String, + INUINTN Length, + INUINTN ErrorLevel + ) +{ + return RETURN_SUCCESS; +} -- 2.28.0.394.ge197136389 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90066): https://edk2.groups.io/g/devel/message/90066 Mute This Topic: https://groups.io/mt/91368901/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 00/12] Introduce Bootlog DEBUG() output
I recently looked at improving the bootup performance of virtual machines and was amazed by the fact that there is no logging / tracing framework available that would give me a full picture of the Pre-OS phase including time stamps and boot loaders (such as grub) without writing data to the serial port - which taints all measurements. The only place that I did find was PerformanceLib, which creates an FPDT. I had 2 problems with that though: 1) Discoverability - The best debugging tools give you all information you need as quickly as possible. The sub table FBPT which contains performance data is very condensed: It contains a modules GUID and up to 24 bytes of text per entry. Entries are limited to 255 bytes. It also only logs load events by default, nothing as fancy as FS reads. 2) Ease of use - The best kernel engineers I know still use printk() and trace_printk() to identify issues: Because it's easy. You can quickly extract data into text and analyze later. 3) Extensability to other payloads - The FPDT infrastructure considers everything as owned by PerformanceLib. I would like to create a one stop place for arbitrary UEFI applications to also put performance measurement data. So I wrote a small configuration table (very similar to FPDT) that contains references to boot logs, each with an array of strings as well as time stamp. This patch set contains everything needed to set this up and hook it into the 2 main DebugLib implementations in virtual machines: Serial and DebugIO. That way, I can see every debug message after boot with time stamp included, with very little impact on boot time. I tried to see if I can call PERF_EVENT() instead of my Bootlog infrastructure, but was unsuccessful. Let's use this patch set to open the conversation on how to best move forward. Eventually, I want to be able to log into Linux, fetch a sysfs file and get a human readable, synchronized, time stamped log file with enough information that allow me to create at least a bug report against the actual firmware owner. Github: https://github.com/agraf/edk2/tree/bootlog-v1 Alex Alexander Graf (12): MdePkg: Add DebugBootlog header MdePkg: Add Null BaseDebugBootlog MdePkg: Add Fallback timer support for BaseDebugBootlog MdePkg: Add X86 timer support for BaseDebugBootlog MdePkg: Add ARM timer support for BaseDebugBootlog MdePkg: Add Pei phase BaseDebugBootlog MdePkg: Add Dxe phase BaseDebugBootlog MdePkg: Add BaseDebugLibBootlog Scripts: Add bootlog decyphering script BaseDebugLibSerialPort: Include BaseDebugBootlog in all dscs BaseDebugLibSerialPort: Emit messages to boot log OvmfPkg/PlatformDebugLibIoPort: Add Bootlog support ArmVirtPkg/ArmVirt.dsc.inc| 5 + BaseTools/Scripts/ShowDebugLog.py | 88 + EmulatorPkg/EmulatorPkg.dsc | 3 + IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc | 1 + MdePkg/Include/Library/DebugBootlog.h | 141 +++ .../BaseDebugBootlog/BaseDebugBootlog.h | 43 +++ .../BaseDebugBootlog/BaseDebugBootlogLib.inf | 61 +++ .../BaseDebugBootlogLibPei.inf| 60 +++ .../BaseDebugBootlogNullLib.inf | 26 ++ .../Library/BaseDebugBootlog/DebugBootlog.c | 22 ++ .../BaseDebugBootlog/DebugBootlogArm.c| 32 ++ .../BaseDebugBootlog/DebugBootlogDxe.c| 349 ++ .../BaseDebugBootlog/DebugBootlogNotime.c | 31 ++ .../BaseDebugBootlog/DebugBootlogNull.c | 32 ++ .../BaseDebugBootlog/DebugBootlogPei.c| 36 ++ .../BaseDebugBootlog/DebugBootlogX86.c| 50 +++ .../BaseDebugLibBootlog.inf | 44 +++ .../BaseDebugLibBootlog.uni | 17 + MdePkg/Library/BaseDebugLibBootlog/DebugLib.c | 338 + .../BaseDebugLibSerialPort.inf| 1 + .../Library/BaseDebugLibSerialPort/DebugLib.c | 22 +- MdePkg/MdePkg.dec | 29 ++ OvmfPkg/AmdSev/AmdSevX64.dsc | 10 + OvmfPkg/Bhyve/BhyveX64.dsc| 10 + OvmfPkg/CloudHv/CloudHvX64.dsc| 10 + OvmfPkg/IntelTdx/IntelTdxX64.dsc | 8 + .../Library/PlatformDebugLibIoPort/DebugLib.c | 23 +- .../PlatformDebugLibIoPort.inf| 1 + .../PlatformRomDebugLibIoPort.inf | 1 + .../PlatformRomDebugLibIoPortNocheck.inf | 1 + OvmfPkg/Microvm/MicrovmX64.dsc| 10 + OvmfPkg/OvmfPkgIa32.dsc | 10 + OvmfPkg/OvmfPkgIa32X64.dsc| 10 + OvmfPkg/OvmfPkgX64.dsc| 7 + OvmfPkg/OvmfXen.dsc | 1 + SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 + UefiPayloadPkg/UefiPayloadPkg.dsc | 1 + 37 files changed, 1526 insertions(+), 9 deletions(-) create mode 100755 BaseTools/Scripts/ShowDebugLog.py create mode 100644 MdePkg/Inclu
[edk2-devel] [PATCH 03/12] MdePkg: Add Fallback timer support for BaseDebugBootlog
The bootlog infrastructure records time stamps of every message. However, not all platforms have readily available time sources. Add a fallback path to return 0 for every time stamp. Signed-off-by: Alexander Graf --- .../BaseDebugBootlog/DebugBootlogNotime.c | 31 +++ 1 file changed, 31 insertions(+) create mode 100644 MdePkg/Library/BaseDebugBootlog/DebugBootlogNotime.c diff --git a/MdePkg/Library/BaseDebugBootlog/DebugBootlogNotime.c b/MdePkg/Library/BaseDebugBootlog/DebugBootlogNotime.c new file mode 100644 index 00..e88c949cf2 --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/DebugBootlogNotime.c @@ -0,0 +1,31 @@ +/** @file + Base Debug library instance for a RAM based boot log + It provides functions to store debug messages in RAM and make them available as + Bootlog Configuration Table. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. + Copyright (c) 2022, Amazon Development Center Germany GmbH. + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include "BaseDebugBootlog.h" + +UINT64 +EFIAPI +BaseDebugLibBootlogTicksPerSecond ( + VOID + ) +{ + return 0; +} + +UINT64 +EFIAPI +BaseDebugLibBootlogTicks ( + VOID + ) +{ + return 0; +} + -- 2.28.0.394.ge197136389 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90069): https://edk2.groups.io/g/devel/message/90069 Mute This Topic: https://groups.io/mt/91368909/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 06/12] MdePkg: Add Pei phase BaseDebugBootlog
This patch adds all logic required to collect bootlog data during the PEI phase. During PEI, we create a HOB entry for every log line. Later, when we emit the first DXE bootlog line, we automatically collect all bootlog HOB entries into the actual bootlog. Signed-off-by: Alexander Graf --- .../BaseDebugBootlog/BaseDebugBootlog.h | 43 + .../BaseDebugBootlogLibPei.inf| 60 +++ .../Library/BaseDebugBootlog/DebugBootlog.c | 22 +++ .../BaseDebugBootlog/DebugBootlogPei.c| 36 +++ MdePkg/MdePkg.dec | 29 + 5 files changed, 190 insertions(+) create mode 100644 MdePkg/Library/BaseDebugBootlog/BaseDebugBootlog.h create mode 100644 MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLibPei.inf create mode 100644 MdePkg/Library/BaseDebugBootlog/DebugBootlog.c create mode 100644 MdePkg/Library/BaseDebugBootlog/DebugBootlogPei.c diff --git a/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlog.h b/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlog.h new file mode 100644 index 00..6bba4a5c30 --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlog.h @@ -0,0 +1,43 @@ +/** @file + Base Debug library instance for a RAM based boot log + It provides functions to store debug messages in RAM and make them available as + Bootlog Configuration Table. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. + Copyright (c) 2012, Red Hat, Inc. + Copyright (c) 2022, Amazon Development Center Germany GmbH. + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +// +// Starting size of the log buffer +// +#define BOOTLOG_MIN_SIZE 0x8 + +// +// Maximum number of parallel boot logs +// +#define MAX_LOGS 16 + +UINT64 +EFIAPI +BaseDebugLibBootlogTicksPerSecond ( + VOID + ); + +UINT64 +EFIAPI +BaseDebugLibBootlogTicks ( + VOID + ); diff --git a/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLibPei.inf b/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLibPei.inf new file mode 100644 index 00..abc66a953f --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLibPei.inf @@ -0,0 +1,60 @@ +## @file +# Base Debug library instance for a RAM based boot log +# It provides functions to store debug messages in RAM and make them available as +# Bootlog Configuration Table. +# +# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. +# Copyright (c) 2012, Red Hat, Inc. +# Copyright (c) 2022, Amazon Development Center Germany GmbH. All rights reserved. +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +# +## + +[Defines] + INF_VERSION= 0x00010005 + BASE_NAME = BaseDebugBootlog + FILE_GUID = DF934DA3-CD31-49FE-AF50-B3C87C79325D + MODULE_TYPE= PEI_CORE + VERSION_STRING = 1.0 + LIBRARY_CLASS = DebugBootlogLib|PEI_CORE PEIM + +[Sources] + DebugBootlog.c + DebugBootlogPei.c + +[Sources.IA32, Sources.X64] + DebugBootlogX86.c + +[Sources.ARM, Sources.AARCH64] + DebugBootlogArm.c + +[Sources.EBC, Sources.RISCV64] + DebugBootlogNotime.c + +[Packages] + MdePkg/MdePkg.dec + +[Packages.AARCH64] + ArmPkg/ArmPkg.dec + +[LibraryClasses] + BaseMemoryLib + PcdLib + PrintLib + BaseLib + DebugPrintErrorLevelLib + PeiServicesLib + +[LibraryClasses.AARCH64] + ArmGenericTimerCounterLib + +[LibraryClasses.ARM] + ArmGenericTimerCounterLib + +[Pcd] + gEfiMdePkgTokenSpaceGuid.PcdDebugBootlogErrorLevel ## CONSUMES + +[Guids] + gBootlogConfigTableGuid ## CONSUMES diff --git a/MdePkg/Library/BaseDebugBootlog/DebugBootlog.c b/MdePkg/Library/BaseDebugBootlog/DebugBootlog.c new file mode 100644 index 00..bceb1c96da --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/DebugBootlog.c @@ -0,0 +1,22 @@ +/** @file + Base Debug library instance for a RAM based boot log + It provides functions to store debug messages in RAM and make them available as + Bootlog Configuration Table. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. + Copyright (c) 2012, Red Hat, Inc. + Copyright (c) 2022, Amazon Development Center Germany GmbH. + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include + +UINT32 +EFIAPI +GetDebugBootlogErrorLevel ( + VOID + ) +{ + return PcdGet32 (PcdDebugBootlogErrorLevel); +} diff --git a/MdePkg/Library/BaseDebugBootlog/DebugBootlogPei.c b/MdePkg/Library/BaseDebugBootlog/DebugBootlogPei.c new file mode 100644 index 00..2106e8190a --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/DebugBootlogPei.c @@ -0,0 +1,36 @@ +/** @file + Base Debug library instance for a RAM based boot log + It provides functions to store debug messages in RAM and make them available as + Bootlog Configuration Table. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved
[edk2-devel] [PATCH 07/12] MdePkg: Add Dxe phase BaseDebugBootlog
This patch adds the main bootlog infrastructure to dynamically create the bootlog configuration table and edk2 bootlog at DXE phase. It attempts to do all this dynamically: The bootlog configuration table may first get created by a UEFI application. This code also collects all PEI phase log entries and assembles them into the final bootlog. Signed-off-by: Alexander Graf --- .../BaseDebugBootlog/BaseDebugBootlogLib.inf | 61 +++ .../BaseDebugBootlog/DebugBootlogDxe.c| 349 ++ 2 files changed, 410 insertions(+) create mode 100644 MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLib.inf create mode 100644 MdePkg/Library/BaseDebugBootlog/DebugBootlogDxe.c diff --git a/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLib.inf b/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLib.inf new file mode 100644 index 00..49cdd3f9ad --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLib.inf @@ -0,0 +1,61 @@ +## @file +# Base Debug library instance for a RAM based boot log +# It provides functions to store debug messages in RAM and make them available as +# Bootlog Configuration Table. +# +# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. +# Copyright (c) 2012, Red Hat, Inc. +# Copyright (c) 2022, Amazon Development Center Germany GmbH. All rights reserved. +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +# +## + +[Defines] + INF_VERSION= 0x00010005 + BASE_NAME = BaseDebugBootlog + FILE_GUID = DF934DA3-CD31-49FE-AF50-B3C87C79325C + MODULE_TYPE= DXE_CORE + VERSION_STRING = 1.0 + LIBRARY_CLASS = DebugBootlogLib|DXE_CORE DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION + CONSTRUCTOR= BaseDebugBootlogLibConstructor + +[Sources] + DebugBootlog.c + DebugBootlogDxe.c + +[Sources.IA32, Sources.X64] + DebugBootlogX86.c + +[Sources.ARM, Sources.AARCH64] + DebugBootlogArm.c + +[Sources.EBC, Sources.RISCV64] + DebugBootlogNotime.c + +[Packages] + MdePkg/MdePkg.dec + +[Packages.AARCH64] + ArmPkg/ArmPkg.dec + +[LibraryClasses] + BaseMemoryLib + PcdLib + PrintLib + BaseLib + DebugPrintErrorLevelLib + +[LibraryClasses.AARCH64] + ArmGenericTimerCounterLib + +[LibraryClasses.ARM] + ArmGenericTimerCounterLib + +[Pcd] + gEfiMdePkgTokenSpaceGuid.PcdDebugBootlogErrorLevel ## CONSUMES + +[Guids] + gBootlogConfigTableGuid ## CONSUMES + gEfiHobListGuid ## CONSUMES diff --git a/MdePkg/Library/BaseDebugBootlog/DebugBootlogDxe.c b/MdePkg/Library/BaseDebugBootlog/DebugBootlogDxe.c new file mode 100644 index 00..b95cb969f1 --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/DebugBootlogDxe.c @@ -0,0 +1,349 @@ +/** @file + Base Debug library instance for a RAM based boot log + It provides functions to store debug messages in RAM and make them available as + Bootlog Configuration Table. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. + Copyright (c) 2012, Red Hat, Inc. + Copyright (c) 2022, Amazon Development Center Germany GmbH. + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include "BaseDebugBootlog.h" + +// +// Cached pointer to the current bootlog structure +// +BOOTLOG_HEADER *gBootlog; + +// +// We can not link against UefiBootServicesTableLib as that itself references +// DebugLib, so instead we save our own copies of the System Table and Boot +// Services structs. +// +static EFI_SYSTEM_TABLE *mDebugST; + +/** + We can not link against UefiBootServicesTableLib as that itself references + DebugLib, so instead we save our own copies of the System Table and Boot + Services structs. + + @retval EFI_SUCCESS The constructor always returns RETURN_SUCCESS. + +**/ +EFI_STATUS +EFIAPI +BaseDebugBootlogLibConstructor ( + IN EFI_HANDLEImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +{ + mDebugST = SystemTable; + + return EFI_SUCCESS; +} + +EFI_STATUS +EFIAPI +BaseDebugBootlogAppendInternal ( + IN CONST CHAR8 *String, + INUINTN Length, + INUINTN ErrorLevel, + INUINT64 Ticks + ); + +static +EFI_STATUS +EFIAPI +UpdateEdk2Bootlog ( + IN BOOTLOG_CONFIG_TABLE *Table, + IN BOOTLOG_HEADER *OldBootlog, + IN BOOTLOG_HEADER *NewBootlog + ) +{ + UINTN Index; + + for (Index = 0; Index < Table->NrLogs; Index++) { +if (Table->LogAddress[Index] == (EFI_PHYSICAL_ADDRESS) OldBootlog) { + Table->LogAddress[Index] = (EFI_PHYSICAL_ADDRESS) NewBootlog; + return EFI_SUCCESS; +} + } + + return EFI_NOT_FOUND; +} + +static +EFI_STATUS +EFIAPI +FindEdk2Bootlog ( + IN BOOTLOG_CONFIG_TABLE *Table, + OUT BOOTLOG_HEADER **Bootlog + ) +{ + UINTN Index; + BOOTLOG_HEADER *Cur; + + for (Index = 0; Index < Table->NrLogs; Index++) { +Cur = (BOOTLOG_HEADER *)Table->LogAddress[Index]; +if (Cur->Signature == SIG_BOOTLOG_HEADER && +
[edk2-devel] [PATCH 08/12] MdePkg: Add BaseDebugLibBootlog
In some situations, we do not want to emit any debug output to serial, but still create a bootlog. In these situations, we can use BaseDebugLibBootlog instead of BaseDebugLibNull. It's a DebugLib that emits exclusively to the bootlog. Signed-off-by: Alexander Graf --- .../BaseDebugLibBootlog.inf | 44 +++ .../BaseDebugLibBootlog.uni | 17 + MdePkg/Library/BaseDebugLibBootlog/DebugLib.c | 338 ++ 3 files changed, 399 insertions(+) create mode 100644 MdePkg/Library/BaseDebugLibBootlog/BaseDebugLibBootlog.inf create mode 100644 MdePkg/Library/BaseDebugLibBootlog/BaseDebugLibBootlog.uni create mode 100644 MdePkg/Library/BaseDebugLibBootlog/DebugLib.c diff --git a/MdePkg/Library/BaseDebugLibBootlog/BaseDebugLibBootlog.inf b/MdePkg/Library/BaseDebugLibBootlog/BaseDebugLibBootlog.inf new file mode 100644 index 00..4e524e0859 --- /dev/null +++ b/MdePkg/Library/BaseDebugLibBootlog/BaseDebugLibBootlog.inf @@ -0,0 +1,44 @@ +## @file +# Instance of Debug library that only emits to the boot log +# It uses PrintLib to send debug messages to the boot log. +# +# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. +# Copyright (c) 2022, Amazon Development Center Germany GmbH. +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +# +## + +[Defines] + INF_VERSION= 0x00010005 + BASE_NAME = BaseDebugLibSerialPort + MODULE_UNI_FILE= BaseDebugLibSerialPort.uni + FILE_GUID = BB83F95F-EDBC-4884-A520-CD42AF388FAE + MODULE_TYPE= BASE + VERSION_STRING = 1.0 + LIBRARY_CLASS = DebugLib + +# +# VALID_ARCHITECTURES = IA32 X64 EBC +# + +[Sources] + DebugLib.c + +[Packages] + MdePkg/MdePkg.dec + +[LibraryClasses] + BaseMemoryLib + PcdLib + PrintLib + BaseLib + DebugPrintErrorLevelLib + DebugBootlogLib + +[Pcd] + gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue ## SOMETIMES_CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel ## CONSUMES + diff --git a/MdePkg/Library/BaseDebugLibBootlog/BaseDebugLibBootlog.uni b/MdePkg/Library/BaseDebugLibBootlog/BaseDebugLibBootlog.uni new file mode 100644 index 00..de131c1292 --- /dev/null +++ b/MdePkg/Library/BaseDebugLibBootlog/BaseDebugLibBootlog.uni @@ -0,0 +1,17 @@ +// /** @file +// Instance of Debug Library that only emits to the boot log. +// +// It uses Print Library to produce formatted output strings to the boot log. +// +// Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved. +// Copyright (c) 2022, Amazon Development Center Germany GmbH. +// +// SPDX-License-Identifier: BSD-2-Clause-Patent +// +// **/ + + +#string STR_MODULE_ABSTRACT #language en-US "Instance of Debug Library that only emits to the boot log." + +#string STR_MODULE_DESCRIPTION #language en-US "It uses Print Library to produce formatted output strings to the boot log configuration table." + diff --git a/MdePkg/Library/BaseDebugLibBootlog/DebugLib.c b/MdePkg/Library/BaseDebugLibBootlog/DebugLib.c new file mode 100644 index 00..ad95ba5d22 --- /dev/null +++ b/MdePkg/Library/BaseDebugLibBootlog/DebugLib.c @@ -0,0 +1,338 @@ +/** @file + Base Debug library instance that only emits to the boot log + It uses PrintLib to send debug messages to the boot log. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. + Copyright (c) 2022, Amazon Development Center Germany GmbH. + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include +#include +#include +#include +#include +#include +#include +#include + +// +// Define the maximum debug and assert message length that this library supports +// +#define MAX_DEBUG_MESSAGE_LENGTH 0x100 + +// +// VA_LIST can not initialize to NULL for all compiler, so we use this to +// indicate a null VA_LIST +// +VA_LIST mVaListNull; + +/** + Prints a debug message to the debug output device if the specified error level is enabled. + + If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib function + GetDebugPrintErrorLevel (), then print the message specified by Format and the + associated variable argument list to the debug output device. + + If Format is NULL, then ASSERT(). + + @param ErrorLevel The error level of the debug message. + @param Format Format string for the debug message to print. + @param ... Variable argument list whose contents are accessed + based on the format string specified by Format. + +**/ +VOID +EFIAPI +DebugPrint ( + IN UINTNErrorLevel, + IN CONST CHAR8 *Format, + ... + ) +{ + VA_LIST Marker; + + VA_START (Marker, Format); + DebugVPrint (ErrorLevel, Format, Marker); + VA_END (Marker); +} + +/** + Prints a debug message to the debug output device if the specified
[edk2-devel] [PATCH 09/12] Scripts: Add bootlog decyphering script
The bootlog itself is a binary data structure that is not immediately human readable. This commit adds a python script to generate a human readable form of it with Linux dmesg like time stamp information. The script can take multiple log sources and collate them into a single output, making it easier to correlate messages. Signed-off-by: Alexander Graf --- BaseTools/Scripts/ShowDebugLog.py | 88 +++ 1 file changed, 88 insertions(+) create mode 100755 BaseTools/Scripts/ShowDebugLog.py diff --git a/BaseTools/Scripts/ShowDebugLog.py b/BaseTools/Scripts/ShowDebugLog.py new file mode 100755 index 00..f99b4b02f7 --- /dev/null +++ b/BaseTools/Scripts/ShowDebugLog.py @@ -0,0 +1,88 @@ +#!/usr/bin/python3 +## @file +# Dump Bootlog files +# +# Copyright (c) 2022, Amazon Development Center Germany GmbH. All rights reserved. +# +# SPDX-License-Identifier: BSD-2-Clause-Patent +# + +from __future__ import print_function + +VersionNumber = '0.1' +__copyright__ = "Copyright (c) 2022, Amazon Development Center Germany GmbH. All rights reserved.." + +import argparse +import os +import sys +import struct + +class DebugLog: +"""Parses a Bootlog blob and provides a string representation +""" + +def __init__(self, log): +self.entries = [] +off = 0 + +signature, self.producer, extraheadertype, extraheadersize, msgextraheadersize, lastbyte, self.tickspersecond = struct.unpack_from('<4s4sHBBLQ', log) +if signature != b'BTLH': +raise Exception("File has incorrect signature. Expected b'BTLH'. Found %s." % signature) +if len(log) < lastbyte: +raise Exception("File smaller than total log contents (%d < %d). It was probably truncated." % (len(log), lastbyte)) +off = 4 + 4 + 2 + 1 + 1 + 4 + 8 + extraheadersize + +while off < lastbyte: +off = off + msgextraheadersize +time, = struct.unpack_from('https://edk2.groups.io/g/devel/message/90071 Mute This Topic: https://groups.io/mt/91368911/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 04/12] MdePkg: Add X86 timer support for BaseDebugBootlog
This patch adds time stamp infrastructure using the TSC. It attempts to determine TSC frequency inside virtual machines, but not on read hardware. Signed-off-by: Alexander Graf --- .../BaseDebugBootlog/DebugBootlogX86.c| 50 +++ 1 file changed, 50 insertions(+) create mode 100644 MdePkg/Library/BaseDebugBootlog/DebugBootlogX86.c diff --git a/MdePkg/Library/BaseDebugBootlog/DebugBootlogX86.c b/MdePkg/Library/BaseDebugBootlog/DebugBootlogX86.c new file mode 100644 index 00..1b6f677ce8 --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/DebugBootlogX86.c @@ -0,0 +1,50 @@ +/** @file + Base Debug library instance for a RAM based boot log + It provides functions to store debug messages in RAM and make them available as + Bootlog Configuration Table. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. + Copyright (c) 2022, Amazon Development Center Germany GmbH. + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include "BaseDebugBootlog.h" + +UINT64 +EFIAPI +BaseDebugLibBootlogTicksPerSecond ( + VOID + ) +{ + UINT32 MaxLeaf = 0; + UINT32 TscKhz = 0; + + /* Look up VMware's TSC leaf first */ + AsmCpuid (0x4000, &MaxLeaf, NULL, NULL, NULL); + if (MaxLeaf >= 0x4010 && MaxLeaf < 0x5000) { +/* We're in a VM that supports the TSC leaf. */ +AsmCpuid (0x4010, &TscKhz, NULL, NULL, NULL); + +return (UINT64)TscKhz * 1000; + } + + /* + * The natural fallback path here would be to leverage TimerLib's + * GetPerformanceCounterProperties(), but TimerLib uses DebugLib + * so we can not use it. Let's leave TSC frequency extraction to + * post processing for now. + */ + + return 0; +} + +UINT64 +EFIAPI +BaseDebugLibBootlogTicks ( + VOID + ) +{ + return AsmReadTsc(); +} + -- 2.28.0.394.ge197136389 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90072): https://edk2.groups.io/g/devel/message/90072 Mute This Topic: https://groups.io/mt/91368912/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 05/12] MdePkg: Add ARM timer support for BaseDebugBootlog
This patch adds bootlog time stamp infrastructure for ARM. It leverages the architected timer which is present on all supported platforms. Signed-off-by: Alexander Graf --- .../BaseDebugBootlog/DebugBootlogArm.c| 32 +++ 1 file changed, 32 insertions(+) create mode 100644 MdePkg/Library/BaseDebugBootlog/DebugBootlogArm.c diff --git a/MdePkg/Library/BaseDebugBootlog/DebugBootlogArm.c b/MdePkg/Library/BaseDebugBootlog/DebugBootlogArm.c new file mode 100644 index 00..5a2c346844 --- /dev/null +++ b/MdePkg/Library/BaseDebugBootlog/DebugBootlogArm.c @@ -0,0 +1,32 @@ +/** @file + Base Debug library instance for a RAM based boot log + It provides functions to store debug messages in RAM and make them available as + Bootlog Configuration Table. + + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. + Copyright (c) 2022, Amazon Development Center Germany GmbH. + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include "BaseDebugBootlog.h" +#include + +UINT64 +EFIAPI +BaseDebugLibBootlogTicksPerSecond ( + VOID + ) +{ + return ArmGenericTimerGetTimerFreq(); +} + +UINT64 +EFIAPI +BaseDebugLibBootlogTicks ( + VOID + ) +{ + return ArmGenericTimerGetSystemCount(); +} + -- 2.28.0.394.ge197136389 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90074): https://edk2.groups.io/g/devel/message/90074 Mute This Topic: https://groups.io/mt/91368915/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 11/12] BaseDebugLibSerialPort: Emit messages to boot log
Now that we have the bootlog infrastructure in place and link against it with all in tree consumers of BaseDebugLibSerialPort, let's emit log lines to the bootlog in addition to serial. The existing PcdDebugBootlogErrorLevel still defines which messages end up on serial. However, in addition the new PcdDebugBootlogErrorLevel defines which ones go into the bootlog. The latter may be more verbose. Signed-off-by: Alexander Graf --- .../BaseDebugLibSerialPort.inf| 1 + .../Library/BaseDebugLibSerialPort/DebugLib.c | 22 +++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf index 7504faee67..dd22fbeb4a 100644 --- a/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf +++ b/MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf @@ -36,6 +36,7 @@ PrintLib BaseLib DebugPrintErrorLevelLib + DebugBootlogLib [Pcd] gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue ## SOMETIMES_CONSUMES diff --git a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c index bd56869477..ea2611228d 100644 --- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c +++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c @@ -20,6 +20,7 @@ #include #include #include +#include // // Define the maximum debug and assert message length that this library supports @@ -103,16 +104,20 @@ DebugPrintMarker ( ) { CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH]; + UINT32 DebugPrintLevel, DebugBootlogLevel, Length; // // If Format is NULL, then ASSERT(). // ASSERT (Format != NULL); + DebugPrintLevel = GetDebugPrintErrorLevel (); + DebugBootlogLevel = GetDebugBootlogErrorLevel (); + // // Check driver debug mask value and global mask // - if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) { + if ((ErrorLevel & (DebugPrintLevel | DebugBootlogLevel)) == 0) { return; } @@ -120,15 +125,24 @@ DebugPrintMarker ( // Convert the DEBUG() message to an ASCII String // if (BaseListMarker == NULL) { -AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker); +Length = AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker); } else { -AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker); +Length = AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker); } // // Send the print string to a Serial Port // - SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer)); + if (ErrorLevel & DebugPrintLevel) { +SerialPortWrite ((UINT8 *)Buffer, Length); + } + + // + // Append the print string to the Boot Log + // + if (ErrorLevel & DebugBootlogLevel) { +DebugBootlogAppend (Buffer, Length, ErrorLevel); + } } /** -- 2.28.0.394.ge197136389 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90075): https://edk2.groups.io/g/devel/message/90075 Mute This Topic: https://groups.io/mt/91368917/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 10/12] BaseDebugLibSerialPort: Include BaseDebugBootlog in all dscs
In the next commit, we will make BaseDebugLibSerialPort call DebugBootlogLib to emit log lines. Make sure that every dsc that links against BaseDebugLibSerialPort also links against a DebugBootlogLib. Signed-off-by: Alexander Graf --- ArmVirtPkg/ArmVirt.dsc.inc | 5 + EmulatorPkg/EmulatorPkg.dsc | 3 +++ IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc | 1 + OvmfPkg/AmdSev/AmdSevX64.dsc| 10 ++ OvmfPkg/Bhyve/BhyveX64.dsc | 10 ++ OvmfPkg/CloudHv/CloudHvX64.dsc | 10 ++ OvmfPkg/IntelTdx/IntelTdxX64.dsc| 8 OvmfPkg/Microvm/MicrovmX64.dsc | 10 ++ OvmfPkg/OvmfPkgIa32.dsc | 10 ++ OvmfPkg/OvmfPkgIa32X64.dsc | 10 ++ OvmfPkg/OvmfPkgX64.dsc | 7 +++ SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 1 + UefiPayloadPkg/UefiPayloadPkg.dsc | 1 + 13 files changed, 86 insertions(+) diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc index f15a3f7f06..c63c441ff4 100644 --- a/ArmVirtPkg/ArmVirt.dsc.inc +++ b/ArmVirtPkg/ArmVirt.dsc.inc @@ -40,6 +40,7 @@ DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf !endif DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf + DebugBootlogLib|MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLib.inf BaseLib|MdePkg/Library/BaseLib/BaseLib.inf SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf @@ -184,6 +185,7 @@ ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf [LibraryClasses.common.SEC] + DebugBootlogLib|MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogNullLib.inf PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf @@ -195,6 +197,7 @@ MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf [LibraryClasses.common.PEI_CORE] + DebugBootlogLib|MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLibPei.inf PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf @@ -210,6 +213,7 @@ SerialPortLib|ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf [LibraryClasses.common.PEIM] + DebugBootlogLib|MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogLibPei.inf PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf @@ -247,6 +251,7 @@ MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf [LibraryClasses.common.DXE_RUNTIME_DRIVER] + DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf !if $(TARGET) != RELEASE diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc index 4cf886b9ea..b898622625 100644 --- a/EmulatorPkg/EmulatorPkg.dsc +++ b/EmulatorPkg/EmulatorPkg.dsc @@ -147,6 +147,7 @@ SerialPortLib|EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf PpiListLib|EmulatorPkg/Library/SecPpiListLib/SecPpiListLib.inf DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf + DebugBootlogLib|MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogNullLib.inf TimerLib|EmulatorPkg/Library/PeiTimerLib/PeiTimerLib.inf [LibraryClasses.common.USER_DEFINED, LibraryClasses.common.BASE] @@ -337,6 +338,7 @@ MdeModulePkg/Core/Dxe/DxeMain.inf { DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf + DebugBootlogLib|MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogNullLib.inf SerialPortLib|EmulatorPkg/Library/DxeEmuStdErrSerialPortLib/DxeEmuStdErrSerialPortLib.inf DxeEmuLib|EmulatorPkg/Library/DxeEmuLib/DxeEmuLib.inf NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf @@ -351,6 +353,7 @@ MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf { DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf + DebugBootlogLib|MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogNullLib.inf SerialPortLib|EmulatorPkg/Library/DxeEmuStdErrSerialPortLib/DxeEmuStdErrSerialPortLib.inf } diff --git a/IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc b/IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc index 961576c9a7..db083f8dfb 100644 --- a/IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc +++ b/IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc @@ -110,6 +110,7 @@ UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf !if $(TARGET) == DEBUG DebugLib|MdePkg/Library
[edk2-devel] [PATCH 12/12] OvmfPkg/PlatformDebugLibIoPort: Add Bootlog support
Now that we have Bootlog support for serial, let's also add it for the PV Debug Port. The only new platform we touch with this is Xen, where we just disable bootlogs for now. Signed-off-by: Alexander Graf --- .../Library/PlatformDebugLibIoPort/DebugLib.c | 23 +++ .../PlatformDebugLibIoPort.inf| 1 + .../PlatformRomDebugLibIoPort.inf | 1 + .../PlatformRomDebugLibIoPortNocheck.inf | 1 + OvmfPkg/OvmfXen.dsc | 1 + 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c index 4e25f198aa..80eb3cce0f 100644 --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include "DebugLibDetect.h" @@ -86,17 +87,20 @@ DebugPrintMarker ( { CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH]; UINTN Length; + UINT32 DebugPrintLevel, DebugBootlogLevel; // // If Format is NULL, then ASSERT(). // ASSERT (Format != NULL); + DebugPrintLevel = GetDebugPrintErrorLevel (); + DebugBootlogLevel = GetDebugBootlogErrorLevel (); + // - // Check if the global mask disables this message or the device is inactive + // Check if the global mask disables this message // - if (((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) || - !PlatformDebugLibIoPortFound ()) + if ((ErrorLevel & (DebugPrintLevel | DebugBootlogLevel)) == 0) { return; } @@ -111,9 +115,18 @@ DebugPrintMarker ( } // - // Send the print string to the debug I/O port + // Send the print string to the debug I/O port if it is active // - IoWriteFifo8 (PcdGet16 (PcdDebugIoPort), Length, Buffer); + if (PlatformDebugLibIoPortFound () && (ErrorLevel & DebugPrintLevel)) { +IoWriteFifo8 (PcdGet16 (PcdDebugIoPort), Length, Buffer); + } + + // + // Append the print string to the Boot Log + // + if (ErrorLevel & DebugBootlogLevel) { +DebugBootlogAppend (Buffer, Length, ErrorLevel); + } } /** diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf index 94ab910507..4a121a3b7b 100644 --- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf @@ -40,6 +40,7 @@ PrintLib BaseLib DebugPrintErrorLevelLib + DebugBootlogLib [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdDebugIoPort## CONSUMES diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf index 8f721d249d..ba2052f81d 100644 --- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf @@ -40,6 +40,7 @@ PrintLib BaseLib DebugPrintErrorLevelLib + DebugBootlogLib [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdDebugIoPort## CONSUMES diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf index 6a85b333ee..75cdfafd22 100644 --- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf @@ -39,6 +39,7 @@ PrintLib BaseLib DebugPrintErrorLevelLib + DebugBootlogLib [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdDebugIoPort## CONSUMES diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc index 6ba4bd729a..6f66d49855 100644 --- a/OvmfPkg/OvmfXen.dsc +++ b/OvmfPkg/OvmfXen.dsc @@ -226,6 +226,7 @@ !else DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf !endif + DebugBootlogLib|MdePkg/Library/BaseDebugBootlog/BaseDebugBootlogNullLib.inf [LibraryClasses.common] BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf -- 2.28.0.394.ge197136389 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90077): https://edk2.groups.io/g/devel/message/90077 Mute This Topic: https://groups.io/mt/91368920/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] PL031: Actually disable interrupts
> Am 11.07.2019 um 19:07 schrieb Laszlo Ersek : > >> On 07/10/19 19:13, Leif Lindholm wrote: >>> On Wed, Jul 10, 2019 at 04:53:11PM +0200, Alexander Graf via Groups.Io >>> wrote: >>> The PL031 interrupt mask register (IMSC) is not very clearly documented >>> in the PL031 specification. However, bit 0 (RTCIMSC) indicates whether >>> interrupts are enabled, not disabled. >> >> 3.3.5. Interrupt Mask Set or Clear register, RTCIMSC >> ... Writing 1 sets the mask. ... >> >> 3.6. Interrupts >> ... This interrupt is enabled or disabled by changing the mask bit in >> RTCIMSC. To enable the interrupt, set bit[0] HIGH. ... >> >> *boggle* > > Heh :) > > Alex, out of interest, what were the symptoms of this issue on your end? > Spurious interrupt confusing the firmware's exception handler, perhaps? No symptoms that I've encountered. I just stumbled over it while studying the device and its respective UEFI code :). But yes, you would see a spurious interrupt once the RTC wraps around to 0, so in 2038. Not that that would matter, as by that time you lost the only wall clock reference available on your ARM system anyway :). Alex -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43616): https://edk2.groups.io/g/devel/message/43616 Mute This Topic: https://groups.io/mt/32416951/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 4/4] OvmfPkg: don't assign PCI BARs above 4GiB when CSM enabled
Hi David and Laszlo, (with broken threading because gmane still mirrors the old ML ...) Mostly, this is only necessary for devices that the CSM might have native support for, such as VirtIO and NVMe; PciBusDxe will already degrade devices to 32-bit if they have an OpROM. However, there doesn't seem to be a generic way of requesting PciBusDxe to downgrade specific devices. There's IncompatiblePciDeviceSupportProtocol but that doesn't provide the PCI class information or a handle to the device itself, so there's no simple way to just match on all NVMe devices, for example. Just leave gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size set to zero for CSM builds, until/unless that can be fixed. Signed-off-by: David Woodhouse Reviewed-by: Laszlo Ersek --- OvmfPkg/OvmfPkgIa32X64.dsc | 4 OvmfPkg/OvmfPkgX64.dsc | 4 2 files changed, 8 insertions(+) diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 639e33cb285f..ad20531ceb8b 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -543,7 +543,11 @@ [PcdsDynamicDefault] gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base|0x0 gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size|0x0 gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base|0x0 +!ifdef $(CSM_ENABLE) + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x0 +!else gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x8 +!endif gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0 diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 69a3497c2c9e..0542ac2235b4 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -542,7 +542,11 @@ [PcdsDynamicDefault] gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base|0x0 gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size|0x0 gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base|0x0 +!ifdef $(CSM_ENABLE) + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x0 IIRC x86 Linux just takes firmware provided BAR maps as they are and doesn't map on its own. Or does it map if a BAR was previously unmapped? In the former case, wouldn't that mean that we're breaking GPU passthrough (*big* BARs) for OVMF if the OVMF version happens to support CSM? So if a distro decides to turn on CSM, that would be a very subtle regression. Would it be possible to change the PCI mapping logic to just simply *prefer* low BAR space if there's some available and the BAR is not big (<64MB for example)? That way we could have CSM enabled OVMF for everyone ;) Alex +!else gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x8 +!endif gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0 -- 2.21.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42943): https://edk2.groups.io/g/devel/message/42943 Mute This Topic: https://groups.io/mt/32213812/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] PL031: Actually disable interrupts
The PL031 interrupt mask register (IMSC) is not very clearly documented in the PL031 specification. However, bit 0 (RTCIMSC) indicates whether interrupts are enabled, not disabled. So before this commit, we were actually *enabling* interrupts for the RTC. This patch changes the logic to instead disable interrupts when they are not disabled already. Signed-off-by: Alexander Graf --- .../Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c index b630a5cfbf..75c95985d4 100644 --- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c +++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c @@ -80,8 +80,8 @@ InitializePL031 ( } // Ensure interrupts are masked. We do not want RTC interrupts in UEFI - if ((MmioRead32 (mPL031RtcBase + PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER) & PL031_SET_IRQ_MASK) != PL031_SET_IRQ_MASK) { -MmioOr32 (mPL031RtcBase + PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER, PL031_SET_IRQ_MASK); + if ((MmioRead32 (mPL031RtcBase + PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER) & PL031_SET_IRQ_MASK) != 0) { +MmioWrite32 (mPL031RtcBase + PL031_RTC_IMSC_IRQ_MASK_SET_CLEAR_REGISTER, 0); } // Clear any existing interrupts -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43483): https://edk2.groups.io/g/devel/message/43483 Mute This Topic: https://groups.io/mt/32416951/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-