Hi Jeremy and all the rest of the Intel folk who took the time to come to the 
coreboot meeting today. I wasn't thrilled with the message you were there to 
present, but I was happy to see all who attended.


I may be incorrect, but I assume that Google has been talking with Intel about 
displaying a screen during memory training for roughly the same amount of time 
that it's been discussed with AMD. For us, I believe that discussion started 
well over a year ago.

For this to be brought up now, with the argument of "Well, we need it for this 
coming platform" seems like a problem brought on by not having this discussion 
with the coreboot community much earlier in the process. The urgency of getting 
a solution merged at this point is not coreboot's doing or responsibility, and 
it seems like coreboot is being pressured to accept it based on the timing, 
which doesn't feel great.



The argument of "we'd like one solution for both coreboot and UEFI firmware" 
seems reasonable to me, but there seems to have been little attention paid to 
what coreboot, an open source project, actually wants. Instead, the position 
seems to be one of forcing coreboot to use what the UEFI firmware uses.

I don't like that idea, but as was said in the meeting today, coreboot is 
willing to accept that, but on the condition that the source for the binary 
(and future similar binaries) is made open. To show good faith Intel could 
guarantee that the source will be opened when it can (supplying a date) and 
release the GOP driver source for platforms that do have released PRMs.


You say here that the uGOP functionality already exists in the FSP, but since 
it's a proprietary blob, I can't actually *tell* what's in there (not that I'm 
doubting your word). This argument is also one of "What's the harm in just one 
more blob?" which has no reason to ever end. At some point, we need to say "no 
more". This seems like a reasonable time.


I understand why companies feel like they need to keep some things proprietary, 
even if I don't like it. This blob just doesn't feel like one of those things. 
This feels like something that could be opened, Intel just can't be bothered to 
do the work to open it, at least not in a timely manner.

Someone inside Intel has the authority to open source this binary. Someone can 
get the programmer manual released. If this is an important feature for this 
Chromebook, surely that person will decide that it makes sense to open source 
it so that it can be used with coreboot. If they decide it's not important 
enough, that's fine too.



Apologies again to all the engineers who are stuck in the middle here. I'll buy 
a round of drinks if I see you at the Open Source Firmware Conference in 
October.


Martin


Aug 23, 2023, 16:25 by jeremy.composte...@intel.com:

>> Hi Jeremy,
>>
>> Thanks for posting this. I know that you're planning on doing a
>> presentation about this in this week's leadership meeting and look
>> forward to that. <https://coreboot.org/calendar.html>
>>
>> A few questions:
>> 1) How does the uGOP driver work with libgfxinit? Does using uGOP mean
>> that the full GOP driver then needs to be used, or can the system
>> transition back to libgfxinit after memory is initialized.
>>
>
> uGOP does not interact with libgfxinit. However, even though we could
> not test it, there is no reason to think libgfxinit would not work
> properly if run after uGOP.
>
>> 2) When is the Graphics Programmer Reference Manuals going to be
>> published so that the support can be added? Is this planned for next
>> month, next year, or not currently planned, but hoped for?
>>
>
> We do not have a clear commitment that the Meteor Lake Programmer
> Manual are going to published publicly. i915 public driver code can be
> used as a reference once it fully supports Meteor Lake graphics
> generation.
>
>> 3) Is there a reason that the uGOP driver can't be open sourced, at
>> least once the Graphics Programmer Reference Manuals are released?
>>
>
> We cannot open-source the code for platforms for which the PRMs are
> not publicly published and we currently do not have a commitment for
> having RPMs for Meteor Lake.
>
>> 4) When you talk about the differences in time between the uGOP driver
>> and libgfxinit, is that strictly due to when they are called, or is
>> there some further difference that the uGOP driver is able to
>> accomplish that libgfxinit wouldn't ever be able to do?
>>
>
> This is not due to the way they are being called. We have not
> investigated the reasons behind why uGOP and GOP also are faster to
> bring up graphics. This is just an observation.
>
>> 5) Is there a reason that Intel is unwilling to add (or help add) the
>> required code to libgfxinit, an open source solution that according to
>> your notes should be comparable to the uGOP binary solution? Would
>> Intel be willing to help once the reference manuals are released, or
>> is any cooperation between intel and the community on libgfxinit just
>> not able to happen?
>>
>
> libgfxinit presents some challenges for us:
> - This component is only used by coreboot which does fit our software
> platform convergence goal
> - it is open-source which prevent us from using it as a solution until
> PRMs are publicly published
> - It is written in SPARK making it very specific and also most Intel
> GFX engineers are not familiar with ADA.
>
>> 6) I assume that the uGOP driver is completely optional, and is only
>> needed to show early signs of life. Is that correct, or could the uGOP
>> driver become mandatory at some point?
>>
>
> uGOP is only an option we want to make available for pre-memory
> Sign-of-Life use-cases.  uGOP, even though it looks like yet another
> blob, is actually mostly just sharing what is already in FSP-S. We are
> offering to make it available outside of FSP-M to offer more
> flexibility in the use-cases for coreboot and chromebook devices.
>
>> Please understand that any unhappiness about this plan is not directed
>> at you personally (or *should* not be), but just the idea of 
>> addingmakmakinging
>> yet another binary blob to the coreboot boot flow. I've been in this
>> same spot and understand the frustration of just trying to get your
>> work done, while the community is unhappy about the direction of the
>> work being done.
>>
>> Thanks very much.
>> Martin
>>
>> Aug 14, 2023, 14:53 by jeremy.composte...@intel.com:
>>
>>>
>>> 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 µGOP. 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
>> <https://github.com/coreboot/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 µGOP driver from
>>>
>> coreboot romstage. It allows to initialize graphics with a very
>> similar flow compared to > libgfxinit
>> <https://github.com/coreboot/libgfxinit>> use.
>>
>>>
>>>
>>> Our goal is to start collecting feedback. We will release all the
>>>
>> patches on coreboot.org under the > ugop
>> <https://review.coreboot.org/q/topic:ugop>> topic soon.
>>
>>>
>>> 1.>  µGOP 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> ;
>>
>>> The current > Version> is > 0x00010000> . Where the upper 16 bits
>>>
>> represent the major (1) and the lower 16 bits represent the minor
>> number.
>>
>>>
>>> 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.
>>
>>>
>>> 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 µGOP 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 µGOP.
>>
>>>
>>> 2.1.>  PEI services
>>>
>>> µGOP depends on a limited of PEI services:
>>>
>>> InstallPpi()>  to install the PEIM Graphics PPI
>>> LocatePpi()>  to access PEIM-to-PEIM Interface (PPI) Dependencies
>>> AllocatePool()> to dynamically allocate memory to handle internal
>>>
>> data structure such as display information …
>>
>>> GetHobList()>  and > CreateHob()>  to access Hand Off Blocks (HOB) holding 
>>> runtime data
>>> 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
>>>
>>> µGOP expects to find the PEI services pointer in the architecture
>>>
>> size word immediately preceding the Interrupt Descriptor Table (IDT)
>> (cf. > Platform Initialization (PI) Specification
>> <https://uefi.org/sites/default/files/resources/PI_Spec_1_6.pdf>> >
>> 5.4 PEI Services Table Retrieval> ). Since > coreboot x86/exception
>> <https://github.com/coreboot/coreboot/blob/master/src/arch/x86/exception.c>>
>> 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.
>>
>>>
>>> 2.3.>  Portable Executable Relocation
>>>
>>> As we need to execute the µGOP 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
>> <https://review.coreboot.org/c/coreboot/+/76762>> ).
>>
>>>
>>> 2.4.>  Uncompressed VBT
>>>
>>> As µGOP 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 <https://review.coreboot.org/c/coreboot/+/76816>> ).
>>
>>>
>>> 3.>  Code flow
>>>
>>>
>>>
>>> 4.>  Performances analysis
>>>
>>> The analysis below is based on a µGOP binary with eDP and HDMI support.
>>>
>>> 4.1.>  Size impact
>>>
>>> When > CONFIG_UGOP_EARLY_GRAPHICS>  is set
>>>
>>> ugop.efi>  is included as a CBFS file
>>> romstage>  includes extra code: > pei.c> , > ugop.c>  and > ux.c
>>> vbt.bin>  is stored uncompressed instead of lzma compressed
>>> CBFS File
>>> UGOP_EARLY_GRAPHICS=n (bytes)
>>> UGOP_EARLY_GRAPHICS=y (bytes)
>>> Delta (bytes)
>>> µGOP
>>> 0
>>> 68448
>>> 68448
>>> romstage
>>> 126128
>>> 136256
>>> 10128
>>> vbt.bin
>>> 1264
>>> 9216
>>> 7952
>>> Total
>>> 127392
>>> 213920
>>> 86528
>>>
>>> The use of µGOP 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 µGOP 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 µGOP 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 µGOP is faster to bring-up graphics than
>>>
>> libgfxinit. Indeed, according to previously captured numbers on Raptor
>> Lake compared to some number of µGOP on Meteor Lake, µGOP 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 µGOP 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
>>>
>>>
> _______________________________________________
> coreboot mailing list -- coreboot@coreboot.org
> To unsubscribe send an email to coreboot-le...@coreboot.org
>

_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org

Reply via email to