August 2023 - coreboot - mail.coreboot.org

[coreboot - Bug #505] (New) Intel Harcuvar CRB Denverton_NS С3000. Only 15 cores of a 16 core processor is defined in the operating system.
by Dmitry Ponamorev Sept. 15, 2023

Sept. 15, 2023

Issue #505 has been reported by Dmitry Ponamorev. ---------------------------------------- Bug #505: Intel Harcuvar CRB Denverton_NS С3000. Only 15 cores of a 16 core processor is defined in the operating system. https://ticket.coreboot.org/issues/505 * Author: Dmitry Ponamorev * Status: New * Priority: Normal * Assignee: Angel Pons * Category: board support * Target version: 4.15 * Start date: 2023-08-22 * Affected versions: 4.15 * Affected hardware: Intel Harcuvar CRB, Intel(R) Atom(TM) CPU C3958 * Affected OS: Debian 11, Ubunto 20.04 (and later) ---------------------------------------- Found a bug with the definition of not all processor cores of the Intel(R) Atom(TM) CPU C3958 processor in operating systems. The Debian or Ubuntu operating system defines only 15 cores. The problem concerns exactly the 16th core processor. For processors with fewer cores, the number of cores is determined correctly. The problem was found in coreboot version 4.14 and later. Version coreboot 4.11 did not have this problem. Most likely this is due to incorrect initialization of apic_id. (apic_id 0) An example of the initialization output of the processor cores is below: Will perform SMM setup. CPU: Intel(R) Atom(TM) CPU C3958 @ 2.00GHz. AP: slot 1 apic_id 6, MCU rev: 0x00000034 AP: slot 13 apic_id 1e, MCU rev: 0x00000034 AP: slot 6 apic_id a, MCU rev: 0x00000034 AP: slot 14 apic_id e, MCU rev: 0x00000034 AP: slot 5 apic_id 4, MCU rev: 0x00000034 AP: slot 2 apic_id 10, MCU rev: 0x00000034 AP: slot 4 apic_id 14, MCU rev: 0x00000034 AP: slot 11 apic_id 1c, MCU rev: 0x00000034 AP: slot 15 apic_id 8, MCU rev: 0x00000034 AP: slot 8 apic_id 16, MCU rev: 0x00000034 AP: slot 10 apic_id c, MCU rev: 0x00000034 AP: slot 3 apic_id 1a, MCU rev: 0x00000034 AP: slot 9 apic_id 2, MCU rev: 0x00000034 AP: slot 7 apic_id 12, MCU rev: 0x00000034 AP: slot 12 apic_id 18, MCU rev: 0x00000034 smm_place_entry_code: smbase 7ffe8400, stack_top 7fe08000 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffec000, cpu = 0 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffe9000, cpu = 12 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffe8800, cpu = 14 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffe9c00, cpu = 9 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffe8400, cpu = 15 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffeb800, cpu = 2 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffe9400, cpu = 11 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffe8c00, cpu = 13 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffea000, cpu = 8 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffeb400, cpu = 3 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffeb000, cpu = 4 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffea400, cpu = 7 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffe9800, cpu = 10 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffebc00, cpu = 1 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffea800, cpu = 6 smm_do_relocation : curr_smbase 0x30000 perm_smbase 0x7ffeac00, cpu = 5 Initializing CPU #0 Turbo is unavailable CPU #0 initialized Initializing CPU #8 Initializing CPU #12 Initializing CPU #14 Initializing CPU #4 Initializing CPU #9 Initializing CPU #10 Initializing CPU #1 Initializing CPU #15 Initializing CPU #7 Initializing CPU #11 Initializing CPU #3 Initializing CPU #2 Initializing CPU #13 Initializing CPU #6 CPU #7 initialized CPU #14 initialized CPU #15 initialized CPU #2 initialized CPU #6 initialized CPU #12 initialized Initializing CPU #5 CPU #3 initialized CPU #1 initialized CPU #10 initialized CPU #4 initialized CPU #11 initialized CPU #5 initialized CPU #9 initialized CPU #8 initialized CPU #13 initialized -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: https://ticket.coreboot.org/my/account

1 2

2023-08-23 - coreboot Leadership meeting minutes
by coreboot org Sept. 12, 2023

Sept. 12, 2023

# 2023-08-23 - coreboot Leadership # Attendees: * ChrisW, MartinR, SubrataB, WernerZ, JonathanH, PatrickG, PaulP, JeremyC, KapilP, AnilK, HannahW, JasonG, RajA, FelixH, SimonG, JayT, JuliusW, ShelleyC, Nico, DavidH, MaximilianB, PratikkumarP, JonM, VincentZ, StefanR, MarshallD, ArthurH, FelixS # Minutes: ## [Subrata] Intel AP FW team would like to present the uGOP **implementation** meant for early Sign of Life (SOL). * The support commitment about libgfxinit is not strong from Intel management hence, Intel wished to leverage existing GFX PEIM in a much smaller format to support early display init. I’m hoping to invite the Intel team to present the talk. * [Martin] I’m not sure that we want to replace an open source solution with a proprietary solution. If we can get the uGOP source code opened, that would be ideal, but I can’t imagine how difficult that would be. * Discussion was started on the mailing list _[RFC] Pre-Memory Sign-of-Life using Intel uGOP_: https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/4OS4… * [sjg] Has there been discussion of libgfxinit in C? * https://code.google.com/archive/p/i915tool/source/default/source * https://review.coreboot.org/c/coreboot/+/76762 * The root cause is long memory-training times on recent Intel devices. For AMD it is around 1 second per GB, but Intel takes longer * The uGOP driver is needed to show graphics during the long DDR5 Initialization. * Why can’t the uGOP driver be in the FSPM? * An API needs to be exposed. * Intel wants to have a unified interface between coreboot & UEFI firmware, which is why they want to use the uGOP driver. * So why can’t the uGOP driver be open source? * Because of intel. Maybe in a year the situation will have changed. * This isn’t coreboot’s problem. The Meteor Lake GPU VPU Linux driver was [open sourced last year](https://patchwork.kernel.org/project/dri-devel/patch/20220728131709.1…. * Release the information, and the community can help. * The uGOP is needed for early bringup of the chip, but Google needs the uGOP driver for the length of the program, because any other possibility won’t be tested. * The uGOP driver allows comparison with libgfxinit. * But we can already compare libgfxinit with the full GOP driver. * Again, why should the coreboot community care? * The only way we can get Intel to change and actually support open source is by putting our foot down now and refusing the uGOP driver. Intel has *no* incentive to change otherwise. * coreboot does *NOT* want another blob. Intel keeps telling coreboot that it’s just not possible or that it’s going to take too long, but if they had started on a previous project, they’d be ready now. * Intel: Not able to make changes for current platform but may consider them in future. * [Patrick] Same answer as in the last 13 years. Nothing ever happens (and I don’t blame the messengers, I fully believe that they try) ## [FelixS] Broken Matrix/IRC bridge * Bridge is disconnected from IRC and will not be coming back soon. What can we do for coreboot? * Can we set up our own matrix bridge. We can do this, but need to work out the details. * GDPR may be a concern (need to limit data collection). ## [Martin]How do we want to handle renaming I2C master/slave to controller/target. The I2C Spec has changed the names, but most datasheets have not. * Should we rename everything I2C across all of coreboot so that the naming is consistent inside coreboot, but register names are different from datasheets. * Should we leave the master/slave terminology for registers so they match the datasheets, and have the mismatch inside of coreboot. * https://review.coreboot.org/c/coreboot/+/77098 * Julius and Arthur vote to leave the register names using the terminology from their datasheets. * [Nic] This seems to be in line with what is published on https://doc.coreboot.org/community/language_style.html * Consensus during meeting: Keep drivers consistent with datasheets, however the higher-level APIs can use the newer language. ## [Martin] Does anyone know if the workaround for xeon_sp/spr: Enable BROKEN_FSP_NEEDS_STACK_ABOVE_BSS is still needed? * https://review.coreboot.org/c/coreboot/+/61164/3 * Arthur can look to see. ## [Martin] A while back, we discussed trying to help people with patches to keep things from getting stale. Over the past several weeks, I’ve been trying to review things that are starting to get a little old. While this can be cumbersome with long patch trains, I think it’s very useful overall. Here are the searches I’ve been using: * [cb_review_1_week_ignore_starred](https://review.coreboot.org/q/repo:coreboo… * [cb_review_1_week](https://review.coreboot.org/q/repo:coreboot+AND+status:op… * [cb_review_2_weeks](https://review.coreboot.org/q/repo:coreboot+AND+status:o… * The 1 week search filters out a lot of patches - anything with open comments, anything that isn’t verified, anything with open comments. * The 2 week patch just filters WIP and anything over 3 months old. * I’ve been trying to keep fresh patches from getting over 7 days old. ## [Martin] Within AMD, we’ve been discussing the licensing for the APCB (AMD PSP Customization Block) blobs. We believe that they should be licensed similar to how we license the vbt.bin files - as pure configuration data. Currently we’re using the CC-PDDC license for those files, and including them in the main coreboot repository. How would people feel about doing the same with the APCB files? * APCB configuration tooling isn’t public, so the data block itself is the best that can be offered * Still, it’s only data (like SPD or VBT) * Concern if this is data that ought to be changed more often than, say, SPD? * It makes sense to keep APCB with mainboard, but user should be aware that APCB may get reconfigured at runtime, thus it will not necessarily match the blob stored in the repo. ## [FelixS] [https://felixsinger.github.io/bootguard-status/](https://felixsinger.github… * People often ask on which boards coreboot can be ported on or if their boards can be supported by coreboot. * This tracks hardware which has BootGuard enabled or disabled. * What do people think of this? Would it be reasonable to host this at a coreboot.org domain? * Yes, but let’s make it more generic to support other companies. * Maybe merge with the board-status list? * This is autogenerated, so it would be difficult. * Let’s put it on coreboot.org as it is now, but update it for other platforms. ## [Paul] Please let’s use the mailing list more. Some topics on the agenda could be discussed there first. The agenda sometimes looks like a different/alternative forum. * It’s easier to discuss things in the meeting by speaking than having to write an email. * People should look at the agenda, and add arguments here before the meeting, such as the discussion about renaming I2c. This does help shorten the meetings. ## [Paul] Experienced reviewers from coreboot companies? Who replaced Aaron, Furqan and Tim? Currently my feeling is, only 9elements does “deep” review for the general code base. * It’s hard to get the experience to do the in-depth reviews - When experienced people leave the companies, it’s hard to replace them * [David] Need to recruit more people to develop coreboot professionally. * FelixH and MartinR review everything they merge pretty thoroughly. ## [Paul] Early MRC caching: https://review.coreboot.org/c/coreboot/+/77295 * New Kconfig option or should integration with all platforms be evaluated first * (Ran out of time, we’ll discuss on the mailing list.) ## [Paul] Status amount of funds and spending thereof. * (Ran out of time, we’ll discuss on the mailing list.) ## [Kyösti] Public document, or some other approach, needed to complete review of Intel Thunderbolt: https://review.coreboot.org/c/coreboot/+/75286 * Add JeremyC to the review, and he can try to help find someone. * Bring up on the mailing list so the people at intel have more visibility. # Notice Decisions shown here are not necessarily final, and are based on the current information available. If there are questions or comments about decisions made, or additional information to present, please put it on the leadership meeting agenda and show up if possible to discuss it. Of course items may also be discussed on the mailing list, but as it's difficult to interpret tone over email, controversial topics frequently do not have good progress in those discussions. For particularly difficult issues, it may be best to try to schedule another meeting. # coreboot leadership meeting minutes https://docs.google.com/document/d/1NRXqXcLBp5pFkHiJbrLdv3Spqh1Hu086HYkKrgK…

9 17

[RFC] x86: Add .data section support for XIP pre-RAM stages
by Compostella, Jeremy Sept. 6, 2023

Sept. 6, 2023

Dear coreboot community, I am looking for feedback on the following topic. x86 Pre-memory stages do not support the `.data' section and as a result developers are required to include runtime initialization code instead of relying on C global variable definition. To illustrate the impact of this lack of .data section support, here are two limitations I personally ran into: 1. The inclusion of libgfxinit in romstage last year has required some changes in libgfxinit to ensure data is initialized at runtime. In addition, we had to manually map some .data symbols in the _bss region. 2. CBFS cache is currently not supported in pre-memory stages and enabling it would require to add an initialization function and find a generic spot to call it. Instead of going though these workarounds and as it was suggested on [[RFC] VGA Text mode in romstage] last year, I believe we could add support for a `.data' section. I have been working on a solution for eXecute-In-Place (XIP) pre-memory stages (`bootblock', `verstage' and `romstage') which deliver good results (cf. <https://review.coreboot.org/c/coreboot/+/77289>). In short this patch: 1. creates a new ELF segment to hold the `.data' section 2. creates a `.data' section with its Virtual Memory Address (VMA) within Cache-as-RAM (CAR) boundaries and its Load Memory Address (LMA) following the .text section (at `_etext'). cbfstools is also updated: - To process this new segment and `.data' section - To place the .data section content right after the code (cf. `parse_elf_to_xip_stage' function) At the moment, this patch makes cbfstool detects the presence of the segment automatically and assume this is a "data" segment. But we could add a new parameter to make this new behavior less automatic if you think this would be better. This patch also adds a piece of assembly code (or C-code for bootblock) to copy the `.data' section content to Cache-As-RAM at runtime. Regards, -- *Jeremy* /One Emacs to rule them all/ [[RFC] VGA Text mode in romstage] <https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/2DYB…>

2 2

[PATCH v3 1/2] x86: coreboot: Update cbmem table
by Simon Glass Sept. 4, 2023

Sept. 4, 2023

This changed a few years ago to include an overflow flag. Bring in the new structure. This comes from coreboot commit: 6f5ead14b4 ("mb/google/nissa/var/joxer: Update eMMC DLL settings") Note: There are several implementations of this in coreboot. I have chosen to follow the one in src/lib/cbmem_console.c Signed-off-by: Simon Glass <sjg(a)chromium.org> --- Changes in v3: - Drop __packed as it does nothing useful arch/x86/include/asm/coreboot_tables.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index 4de137fbab9d..0dfb64babb96 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -299,11 +299,24 @@ struct cb_vdat { #define CB_TAG_TIMESTAMPS 0x0016 #define CB_TAG_CBMEM_CONSOLE 0x0017 +#define CBMC_CURSOR_MASK ((1 << 28) - 1) +#define CBMC_OVERFLOW BIT(31) + +/* + * struct cbmem_console - In-memory console buffer for coreboot + * + * Structure describing console buffer. It is overlaid on a flat memory area, + * with body covering the extent of the memory. Once the buffer is full, + * output will wrap back around to the start of the buffer. The high bit of the + * cursor field gets set to indicate that this happened. If the underlying + * storage allows this, the buffer will persist across multiple boots and append + * to the previous log. + */ struct cbmem_console { u32 size; u32 cursor; - char body[0]; -} __packed; + u8 body[0]; +}; #define CB_TAG_MRC_CACHE 0x0018 -- 2.41.0.694.ge786442a9b-goog

2 3

[coreboot - Bug #499] (New) coreboot will not boot edk2 on Lenovo T440p with CONFIG_RESOURCE_ALLOCATION_TOP_DOWN enabled, cannot disable this setting during build
by Oberon 4071 Aug. 31, 2023

Aug. 31, 2023

Issue #499 has been reported by Oberon 4071. ---------------------------------------- Bug #499: coreboot will not boot edk2 on Lenovo T440p with CONFIG_RESOURCE_ALLOCATION_TOP_DOWN enabled, cannot disable this setting during build https://ticket.coreboot.org/issues/499 * Author: Oberon 4071 * Status: New * Priority: Normal * Category: coreboot common code * Start date: 2023-06-29 * Affected versions: 4.21 * Needs backport to: none * Affected hardware: lenovo/t440p ---------------------------------------- coreboot revision in git: feb27dcbf3fc685b070c950a16e8adec958bc1ce coreboot revision (git describe --tags): 4.20-520-gfeb27dcbf3 Tested payloads: edk2 from MrChromebox revision uefipayload_202304 and uefipayload_202306 coreboot will not boot my Lenovo ThinkPad T440p with CONFIG_RESOURCE_ALLOCATION_TOP_DOWN enabled, when using the edk2 payload (MrChromebox version, either uefipayload_202304 or uefipayload_202306). The display sometimes turns on, indicating that some of the hardware initialization was successful, but the payload will not start. I tried to disable CONFIG_RESOURCE_ALLOCATION_TOP_DOWN in .config, but the build process insists on leaving this config enabled. This seems to be caused by the RESOURCE_ALLOCATION_TOP_DOWN setting changed to "def_bool y" in src/device/Kconfig in commit 5226301765ded70e0ef640e5252bbaca8cd14451 (allocator_v4: Treat above 4G resources more natively). The make target for building coreboot seems to automatically rerun olddefconfig, causing this setting to always remain enabled no matter what was previously saved in the .config file. Modifying src/device/Kconfig to change RESOURCE_ALLOCATION_TOP_DOWN to "def_bool n" appears to fix the problem on my machine. I have attached my .config file (with CONFIG_RESOURCE_ALLOCATION_TOP_DOWN enabled to reproduce the problem). ---Files-------------------------------- .config (20.7 KB) -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: https://ticket.coreboot.org/my/account

6 25

New on blogs.coreboot.org: coreboot version 4.21 released
by blogs.coreboot.org Aug. 31, 2023

Aug. 31, 2023

1 0

[coreboot - Bug #507] (New) Windows GPU driver fails to load on AMD based google/guybrush, google/skyrim boards
by Matt DeVillier Aug. 30, 2023

Aug. 30, 2023

Issue #507 has been reported by Matt DeVillier. ---------------------------------------- Bug #507: Windows GPU driver fails to load on AMD based google/guybrush, google/skyrim boards https://ticket.coreboot.org/issues/507 * Author: Matt DeVillier * Status: New * Priority: Normal * Target version: none * Start date: 2023-08-30 * Affected versions: 4.21, 4.15, 4.16, 4.17, 4.18, 4.19, master * Affected hardware: google/guybrush, google/skyrim ---------------------------------------- The Windows GPU drivers on AMD Cezanne and Mendocino platform ChromeOS devices (google guybrush and skyrim boards respectively) fail to load due to the PSP boot mode being `Development` vs `Production.` On these platforms, the PSP only boots in `Production` mode when ChromeOS verified boot mode is active. Possible solutions include having the PSP boot in `Production` mode when vboot is not used at all, when CONFIG_CHROMEOS is not set, or (always) when PSP signed verstage is used. Any of these require changes to the PSP firmware from AMD. -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: https://ticket.coreboot.org/my/account

1 0

[coreboot - Bug #506] (New) Apollolake/Geminilake boards fail to boot OS when CPU microcode included "from tree"
by Matt DeVillier Aug. 30, 2023

Aug. 30, 2023

Issue #506 has been reported by Matt DeVillier. ---------------------------------------- Bug #506: Apollolake/Geminilake boards fail to boot OS when CPU microcode included "from tree" https://ticket.coreboot.org/issues/506 * Author: Matt DeVillier * Status: New * Priority: Normal * Target version: none * Start date: 2023-08-30 * Affected versions: 4.21, 4.15, 4.16, 4.17, 4.18, 4.19, master * Affected hardware: all boards using Intel Apollolake/Geminilake SoCs * Affected OS: All ---------------------------------------- Apollolake/Geminilake (APL/GLK) based devices are unique in that they have a special firmware region called IFWI, which contains the coreboot bootbock and CPU microcode, among other things. As part of building an image for an APL/GLK-based device, an existing IFWI binary must be supplied in order to be modified with coreboot's bootblock. The CPU microcode however is not updated, as the region size is fixed, and current tooling (util/cbfstool/ifwitool) does not have the ability to resize it. When the default coreboot Kconfig option `CPU_MICROCODE_CBFS_DEFAULT_BINS` for CPU microcode is selected, coreboot will include a microcode update in CBFS which is (much) newer than that in the IFWI, and apply it in ramstage. This causes any OS to fail to boot: Linux hangs, and Windows BSODs with an `UNSUPPORTED CPU` error. The only workaround is to include an external CPU microcode binary which matches the one in IFWI (ie, use both the IFWI and microcode extracted from the original vendor firmware image). Not including any microcode update has not been tested, and may also be an option. There have been a few patches submitted to work around this (CB:65680, CB:64863), but the former requires external, non-public tooling, and the latter fails if the new microcode update is larger than the existing IFWI region. Possible areas to investigate are adding resizing capability to ifwitool, and constructing an IFWI from scratch (vs copying/modifying the existing one) -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: https://ticket.coreboot.org/my/account

1 0

[RFC] Pre-Memory Sign-of-Life using Intel uGOP
by Compostella, Jeremy Aug. 27, 2023

Aug. 27, 2023

Dear coreboot developers, With Raptor Lake, we introduced the Pre-Memory Sign-of-Life feature which displays an on-screen message while firmware components such as coreboot, Firmware Support Package Memory (FSP-M) or, CSME perform long time operations during pre-memory stages. We propose to take advantage of a proprietary driver Intel already supports, validates and includes in FSP silicon: the Intel Graphics PEIM (Pre-EFI Initialization Module) driver also known as the GOP (Graphical Output Protocol) driver. This driver is designed to run in post-memory initialization stages. Therefore, we derived a new version capable of running in pre-memory stages which we called microGOP. This version is specifically designed to perform graphics legacy VGA initialization. We intend to keep providing such a binary base solution on the long run as it addresses our software convergence goals and is compatible with early platform development stage constraints. [libgfxinit] supports can always be added later by the open-source community once the Graphics Programmer Reference Manuals are published. Below, we present the work we performed to run this microGOP driver from coreboot romstage. It allows to initialize graphics with a very similar flow compared to [libgfxinit] use. Our goal is to start collecting feedback. We will release all the patches on coreboot.org under the [ugop] topic soon. [libgfxinit] <https://github.com/coreboot/libgfxinit> [ugop] <https://review.coreboot.org/q/topic:ugop> 1 microGOP driver interface =========================== The uGOP PEIM provides the following PEIM-to-PEIM protocol under the `31a4622d-0e21-40a2-80db-c44208fce1b5' GUID. ,---- | #define PEI_PREMEM_GRAPHICS_PPI_GUID \ | { \ | 0x31a4622d, 0x0e21, 0x40a2, 0x80, 0xdb, 0xc4, 0x42, 0x08, 0xfc, 0xe1, 0xb5 \ | }; `---- The protocol is composed of three fields. ,---- | struct { | UINT32 Version; | PREMEM_PEI_GRAPHICS_INIT PreMemGraphicsPpiInit; | PREMEM_PEI_GRAPHICS_EXIT PreMemGraphicsPpiExit; | } PEI_MICRO_GRAPHICS_PPI; `---- 1. The current `Version' is `0x00010000'. Where the upper 16 bits represent the major (1) and the lower 16 bits represent the minor number. 2. `PreMemGraphicsPpiInit()' ,---- | typedef | EFI_STATUS | (EFIAPI *PREMEM_PEI_GRAPHICS_INIT) ( | IN VOID *Vbt | ); `---- The `PreMemGraphicsPpiInit()' should be supplied with a pointer to the Video BIOS Table. 3. `PreMemGraphicsPpiExit()' does not take any parameters. This function must be called to disable VGA graphics configuration once not necessary anymore. Not performing this operation may lead to undesirable behaviour when other graphics stack starts (GOP in FSP-S or Operating System driver). 2 Integration ============= As we intend to run microGOP driver in romstage, we want to keep the required coreboot code as small and efficient as possible. For this reason, we discarded re-using the EDK2 code which would have a major impact on the romstage binary size in addition to adding complication to the build scripts. Instead, we implemented a limited set of Pre-EFI Initialization services. The code is small and designed to accommodate a simple PEIM driver such as microGOP. 2.1 PEI services ~~~~~~~~~~~~~~~~ microGOP depends on a limited of PEI services: 1. `InstallPpi()' to install the PEIM Graphics PPI 2. `LocatePpi()' to access PEIM-to-PEIM Interface (PPI) Dependencies 3. `AllocatePool()' to dynamically allocate memory to handle internal data structure such as display information ... 4. `GetHobList()' and `CreateHob()' to access Hand Off Blocks (HOB) holding runtime data 5. `ReportStatusCode()' to report debug information which coreboot prints using `printk'. Those services implemented in coreboot are pretty straightforward and fit in less than 300 lines of code. 2.2 PEI services pointer ~~~~~~~~~~~~~~~~~~~~~~~~ microGOP expects to find the PEI services pointer in the architecture size word immediately preceding the Interrupt Descriptor Table (IDT) (cf. [Platform Initialization (PI) Specification] *5.4 PEI Services Table Retrieval*). Since [coreboot x86/exception] module already sets up the IDT and as we do not want to disrupt this configuration we create a copy of the IDT. But we include an extra architecture size word preceding the table to store the PEI services pointer. Note that FSP memory installs its own IDT but it backups and restores the one we have set up. Therefore, there is no risk of having PEI services pointer conflicts. [Platform Initialization (PI) Specification] <https://uefi.org/sites/default/files/resources/PI_Spec_1_6.pdf> [coreboot x86/exception] <https://github.com/coreboot/coreboot/blob/master/src/arch/x86/exception.c> 2.3 Portable Executable Relocation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ As we need to execute the microGOP binary in place, we need to perform a relocation operation of the Portable Executable binary. Since memory space is limited in the pre-memory stages, it is preferable to perform a static relocation operation during the firmware stitching operation. Fortunately, most of the logic and code is already available as this operation is performed on the FSP-M binary. We only have to add explicit support for EFI binaries (cf. [76762 cbfstool: Add relocation support for EFI binaries]). [76762 cbfstool: Add relocation support for EFI binaries] <https://review.coreboot.org/c/coreboot/+/76762> 2.4 Uncompressed VBT ~~~~~~~~~~~~~~~~~~~~ As microGOP requires the Video BIOS Table (VBT) and since memory space is limited in the pre-memory stages, it is preferable to keep VBT in uncompressed form in CBFS. We introduced the `CONFIG_VBT_CBFS_COMPRESSION' configuration entry to allow this (cf. [76816 drivers/intel/gma/Kconfig: Add VBT compression configuration entry]). [76816 drivers/intel/gma/Kconfig: Add VBT compression configuration entry] <https://review.coreboot.org/c/coreboot/+/76816> 3 Code flow =========== <file:./code-flow.svg> 4 Performances analysis ======================= The analysis below is based on a microGOP binary with eDP and HDMI support. 4.1 Size impact ~~~~~~~~~~~~~~~ When `CONFIG_UGOP_EARLY_GRAPHICS' is set 1. `ugop.efi' is included as a CBFS file 2. `romstage' includes extra code: `pei.c', `ugop.c' and `ux.c' 3. `vbt.bin' is stored uncompressed instead of lzma compressed CBFS File UGOP_EARLY_GRAPHICS=n (bytes) UGOP_EARLY_GRAPHICS=y (bytes) Delta (bytes) ---------------------------------------------------------------------------------------- microGOP 0 68448 68448 romstage 126128 136256 10128 vbt.bin 1264 9216 7952 ---------------------------------------------------------------------------------------- *Total* 127392 213920 *86528* The use of microGOP in coreboot represents a size increase of around 84 KB per region (`COREBOOT', `FW_MAIN_A' and `FW_MAIN_B'). 4.2 Regular Boot time impact (no Sign-of-Life) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ On a Meteor Lake Google Rex board, we performed 5 warm reset cycles (without and with `CONFIG_UGOP_EARLY_GRAPHICS' set) and we collected the `cbmem -t' outputs. We computed the median time of each duration (time between two timestamps) and then performed a comparison with a threshold of 0.5 ms. Start ID Start Description End ID End Description Delta (ms) ------------------------------------------------------------------------------------------------------------------------- 947 CSE received 'CPU Reset Done Ack sent' from PMC 991 Die Management Unit (DMU) load completed +1.0 507 starting to verify body (load+SHA2+RSA) 508 finished loading body +4.7 510 finished verifying body signature (RSA) 511 starting TPM PCR extend -0.8 1030 finished EC verification 1040 finished storage device initialization +1.4 The only relevant impact is the verification of the image (507 -> 508): +4.7 ms and it can be explained by the 84 KB size increase of the image. Overall the boot time impact is about 5 ms and concentrated on `verstage'. 4.3 Cache-As-Ram ~~~~~~~~~~~~~~~~ For microGOP to execute properly, we have to provide some memory allocation services (`allocate_pool' and `create_hob'). These services relies on cache-as-ram memory reservation (`.bss' section). We looked at the two new object files: Object file .bss section size (bytes) ---------------------------------------- pei.o 6730 ugop.o 9 ---------------------------------------- *6739* With microGOP sign-of-life, there is an extra 7 KB cache-as-ram use. 4.4 Conclusion ~~~~~~~~~~~~~~ The SPINOR and cache-as-RAM space use along with the boot performance penalty are limited and comparable to what it would be with libgfxinit. We also noticed that microGOP is faster to bring-up graphics than libgfxinit. Indeed, according to previously captured numbers on Raptor Lake compared to some number of microGOP on Meteor Lake, microGOP is three times faster to bring up graphics than libgfxinit on an eDP panel (119 ms vs 373 ms). 5 Summary ========= This Sign-of-Life microGOP driver based implementation presents the following advantages: - it needs a limited code addition - it has a limited impact on the performance - its flow and boot performance impact is comparable to libgfxinit solution - it is compatible with our software convergence goals - it can be available during new platform development early stages which help our partners to test the feature and stabilize the platform Regards, -- *Jeremy* /One Emacs to rule them all/

7 14

New Defects reported by Coverity Scan for coreboot
by scan-admin＠coverity.com Aug. 22, 2023

Aug. 22, 2023

Hi, Please find the latest report on new defect(s) introduced to coreboot found with Coverity Scan. 2 new defect(s) introduced to coreboot found with Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 2 of 2 defect(s) ** CID 1518657: (OVERRUN) ________________________________________________________________________________________________________ *** CID 1518657: (OVERRUN) /src/soc/mediatek/common/i2c.c: 514 in mtk_i2c_speed_init() 508 for (clk_div = 1; clk_div <= max_clk_div; clk_div++) { 509 clk_src = I2C_CLK_HZ / clk_div; 510 bus_ctrl->ac_timing.inter_clk_div = clk_div - 1; 511 512 if (speed > I2C_SPEED_FAST_PLUS) { 513 /* Set master code speed register */ >>> CID 1518657: (OVERRUN) >>> Overrunning callee's array of size 7 by passing argument "bus" (which evaluates to 7) in call to "mtk_i2c_calculate_speed". 514 if (mtk_i2c_calculate_speed(bus, clk_src, I2C_SPEED_FAST, 515 &l_step_cnt, &l_sample_cnt)) 516 continue; 517 518 /* Set the high speed mode register */ 519 if (mtk_i2c_calculate_speed(bus, clk_src, speed, /src/soc/mediatek/common/i2c.c: 525 in mtk_i2c_speed_init() 519 if (mtk_i2c_calculate_speed(bus, clk_src, speed, 520 &step_cnt, &sample_cnt)) 521 continue; 522 523 bus_ctrl->ac_timing.inter_clk_div = (clk_div - 1) << 8 | (clk_div - 1); 524 } else { >>> CID 1518657: (OVERRUN) >>> Overrunning callee's array of size 7 by passing argument "bus" (which evaluates to 7) in call to "mtk_i2c_calculate_speed". 525 if (mtk_i2c_calculate_speed(bus, clk_src, speed, 526 &l_step_cnt, &l_sample_cnt)) 527 continue; 528 529 /* Disable the high speed transaction */ 530 bus_ctrl->ac_timing.hs = I2C_TIME_CLR_VALUE; ** CID 1518656: Memory - corruptions (OVERRUN) /src/soc/mediatek/common/i2c.c: 510 in mtk_i2c_speed_init() ________________________________________________________________________________________________________ *** CID 1518656: Memory - corruptions (OVERRUN) /src/soc/mediatek/common/i2c.c: 510 in mtk_i2c_speed_init() 504 } 505 506 bus_ctrl = &mtk_i2c_bus_controller[bus]; 507 508 for (clk_div = 1; clk_div <= max_clk_div; clk_div++) { 509 clk_src = I2C_CLK_HZ / clk_div; >>> CID 1518656: Memory - corruptions (OVERRUN) >>> Overrunning array of 280 bytes at byte offset 280 by dereferencing pointer "bus_ctrl". 510 bus_ctrl->ac_timing.inter_clk_div = clk_div - 1; 511 512 if (speed > I2C_SPEED_FAST_PLUS) { 513 /* Set master code speed register */ 514 if (mtk_i2c_calculate_speed(bus, clk_src, I2C_SPEED_FAST, 515 &l_step_cnt, &l_sample_cnt)) ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P…

1 0