coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: [WIP]arch/x86: Tear down CAR in ramstage
by Aaron Durbin (Code Review) 14 May '21

14 May '21

Attention is currently required from: Jason Glenesk, Anjaneya "Reddy" Chagam, Raul Rangel, Marshall Dawson, Jonathan Zhang, Johnny Lin, Arthur Heymans, Morgan Jang, Kyösti Mälkki, Patrick Rudolph, Felix Held. Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54301 ) Change subject: [WIP]arch/x86: Tear down CAR in ramstage ...................................................................... Patch Set 2: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/54301/comment/f94721b2_d9098d0c PS2, Line 15: likely to be slow. Yes, and among many other things. Caching teardown in general will have the same problem as all the old ones -- just this time they are sitting in ramstage. Just so the background is captured. We used to teardown cache as ram in romstage then return back into C code to load ramstage. Variable juggling is hard there because much of the code is expecting state to be maintained. That's why we added the relocatable car variable stuff; it was complicated but necessary to reuse code from areas that had SRAM that was maintained or targeting ramstage environment. So we put in postcar to provide a clean boundary between romstage, ramstage, and semantics of dealing w/ cache as ram backing store disappearing. postcar provides smallish, though it seems ROM space is so tight one can't afford it (would be good to see the numbers. it can be compressed as well), environment where it can be loaded in uncached DRAM, clean up cache-as-ram, enable caching for DRAM, and start running all the fancy C code we have. Moving that to stuff to ramstage inherently means you are loading a bigger footprint into uncacheable space. And one needs to ensure that cache-as-ram teardown is done from assembly (which this CL does) and no fancy anything until caching is set up. Long story short, I think re-mixing things isn't the best direction because this area is pretty darn complicated to get right. I think we have a pretty good abstraction w/ postcar to where it makes many things straight forward. I would be curious to know numbers on the pressure people are seeing w.r.t. ROM footprints and if there's anything else we can do to help reduce it. File src/arch/x86/c_start.S: https://review.coreboot.org/c/coreboot/+/54301/comment/006b227d_1421dc0a PS2, Line 62: #if CONFIG(RAMSTAGE_CAR_TEARDOWN) What's the reasoning for not putting this before the _cbmem_top_ptr memory access? It might be advisable to put this sequence as close to the entry as possible so that it doesn't allow assumptions to break down over time. File src/cpu/x86/mtrr/earlymtrr.c: https://review.coreboot.org/c/coreboot/+/54301/comment/ca28b69f_a9b05257 PS2, Line 121: __attribute__((used, __section__(".module_parameters"))) static const volatile uint64_t post_car_mtrrs; static const is making a local object despite the section attribute. What is the toolchain doing to resolve these? https://review.coreboot.org/c/coreboot/+/54301/comment/e0bf18d9_2c9d6859 PS2, Line 125: post_car_mtrrs Where is this object initialized? It was previously created the by loader of postcar targeting the module parameter space. Now this object memory is within the ramstage address space. I'm not following how this works. Oh. I see there are other CLs. I think the direction of intermixing cache-as-ram w/ c code is ripe for problems -- just like before. -- To view, visit https://review.coreboot.org/c/coreboot/+/54301 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I02e5017ab2b6a6fd30b37edad19165c2531c8fd1 Gerrit-Change-Number: 54301 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 14 May 2021 16:06:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: tests: code coverage improvements
by Paul Fagerburg (Code Review) 14 May '21

14 May '21

Attention is currently required from: Martin Roth, Julius Werner. Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54072 ) Change subject: tests: code coverage improvements ...................................................................... Patch Set 3: (3 comments) File tests/Makefile.inc: https://review.coreboot.org/c/coreboot/+/54072/comment/dc2413c4_b4f72c57 PS3, Line 195: coverage-unit-tests: unit-tests coverage-report-tests > make could decide to build the dependencies in parallel. […] I didn't think of the parallel dependencies. Yeah, that's a problem. [Using `coverage-report` instead of `coverage-report-tests` in this example because I'm changing that target.] If I make coverage-report depend on unit-tests, the unit-tests target will build and run *all* of the unit tests. In some cases, you may only want a coverage report for the single unit test that we built, e.g. `COV=1 make tests/lib/uuid-test coverage-report` I think the safest thing is to just remove that target. If you want all of the unit tests and a coverage report, then `COV=1 make unit-tests coverage-report` https://review.coreboot.org/c/coreboot/+/54072/comment/4ada7737_04956e1d PS3, Line 199: coverage_rpt > nit: "coveratge_reports" might be better, but it's just my taste. I had to add -tests to every target to get the main Makefile to use tests/Makefile.inc. I've made changes in the main Makefile to use 'coverage' as well as -tests to get into unit tests. https://review.coreboot.org/c/coreboot/+/54072/comment/6f6d5cf3_edcc05a1 PS3, Line 258: @echo ' clean-coverage-report-tests' : @echo ' - Remove the code coverage report' > nit: You can reformat this whole section to make all target names and their first line of descriptio […] Fixed. -- To view, visit https://review.coreboot.org/c/coreboot/+/54072 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2bd2bfdedfab291aabeaa968c10b17e9b61c9c0a Gerrit-Change-Number: 54072 Gerrit-PatchSet: 3 Gerrit-Owner: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Fri, 14 May 2021 15:26:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: tests: code coverage improvements
by Paul Fagerburg (Code Review) 14 May '21

14 May '21

Attention is currently required from: Paul Fagerburg, Julius Werner. Hello build bot (Jenkins), Jakub Czapiga, Julius Werner, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/54072 to look at the new patch set (#4). Change subject: tests: code coverage improvements ...................................................................... tests: code coverage improvements Fix the exclusion path for lcov; it should exclude the directory with source code, not object files. Use the COV environment variable to * control whether we build for coverage or not * select the output directory Add a separate target for generating the report, so we can get a report for all of the tests together or just a single test. Add documentation. Signed-off-by: Paul Fagerburg <pfagerburg(a)google.com> Change-Id: I2bd2bfdedfab291aabeaa968c10b17e9b61c9c0a --- A Documentation/technotes/2021-05-code-coverage.md M Documentation/tutorial/part3.md M Makefile M tests/Makefile.inc 4 files changed, 103 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/54072/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/54072 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2bd2bfdedfab291aabeaa968c10b17e9b61c9c0a Gerrit-Change-Number: 54072 Gerrit-PatchSet: 4 Gerrit-Owner: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: [WIP]arch/x86: Tear down CAR in ramstage
by build bot (Jenkins) (Code Review) 14 May '21

14 May '21

Attention is currently required from: Jason Glenesk, Anjaneya "Reddy" Chagam, Raul Rangel, Marshall Dawson, Jonathan Zhang, Johnny Lin, Morgan Jang, Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54301 ) Change subject: [WIP]arch/x86: Tear down CAR in ramstage ...................................................................... Patch Set 2: (1 comment) File src/cpu/x86/mtrr/earlymtrr.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-119334): https://review.coreboot.org/c/coreboot/+/54301/comment/2ffff641_9fbad7a6 PS2, Line 121: __attribute__((used, __section__(".module_parameters"))) static const volatile uint64_t post_car_mtrrs; line over 96 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/54301 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I02e5017ab2b6a6fd30b37edad19165c2531c8fd1 Gerrit-Change-Number: 54301 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 14 May 2021 14:26:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP]arch/x86/postcar: Set up postcar in C code
by Arthur Heymans (Code Review) 14 May '21

14 May '21

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54299 ) Change subject: [WIP]arch/x86/postcar: Set up postcar in C code ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: This works on qemu. Breaks FSP likely -- To view, visit https://review.coreboot.org/c/coreboot/+/54299 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5ec10e84118197a04de0a5194336ef8bb049bba4 Gerrit-Change-Number: 54299 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 14 May 2021 14:19:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Kconfig: Fix BOOT_DEVICE_MEMORY_MAPPED
by Arthur Heymans (Code Review) 14 May '21

14 May '21

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/54302 ) Change subject: Kconfig: Fix BOOT_DEVICE_MEMORY_MAPPED ...................................................................... Kconfig: Fix BOOT_DEVICE_MEMORY_MAPPED Also non SPI x86 is memory mapped. Change-Id: I9c21e87d3375220a50645e0bf8b56fd23983f0c0 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/54302/1 diff --git a/src/Kconfig b/src/Kconfig index fe325e8..9a2aa22 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -583,7 +583,7 @@ config BOOT_DEVICE_MEMORY_MAPPED bool - default y if ARCH_X86 && BOOT_DEVICE_SPI_FLASH + default y if ARCH_X86 default n help Inform system if SPI is memory-mapped or not. -- To view, visit https://review.coreboot.org/c/coreboot/+/54302 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9c21e87d3375220a50645e0bf8b56fd23983f0c0 Gerrit-Change-Number: 54302 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: [WIP]arch/x86: Tear down CAR in ramstage
by Arthur Heymans (Code Review) 14 May '21

14 May '21

Attention is currently required from: Jason Glenesk, Anjaneya "Reddy" Chagam, Raul Rangel, Marshall Dawson, Jonathan Zhang, Johnny Lin, Arthur Heymans, Morgan Jang, Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Felix Held. Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Anjaneya "Reddy" Chagam, Marshall Dawson, Jonathan Zhang, Johnny Lin, Morgan Jang, Kyösti Mälkki, Aaron Durbin, Patrick Rudolph, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/54301 to look at the new patch set (#2). Change subject: [WIP]arch/x86: Tear down CAR in ramstage ...................................................................... [WIP]arch/x86: Tear down CAR in ramstage Postcar is a 'full' stage to just run a few instructions to disable CAR and setup MTRR for ramstage. This takes up a lot of place in ROM. This functionality can be moved into ramstage. Notes: Typically no caching is set up around cbmem so the LZMA compressed ramstage is being decompressed in uncached memory, which is likely to be slow. Change-Id: I02e5017ab2b6a6fd30b37edad19165c2531c8fd1 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/Kconfig M src/arch/x86/c_start.S M src/arch/x86/exit_car.S A src/arch/x86/exit_car.inc M src/arch/x86/postcar_loader.c M src/cpu/qemu-x86/Kconfig M src/cpu/x86/mtrr/Makefile.inc M src/cpu/x86/mtrr/earlymtrr.c M src/lib/program.ld M src/mainboard/emulation/qemu-i440fx/Kconfig M src/mainboard/emulation/qemu-i440fx/Makefile.inc M src/soc/amd/common/block/cpu/car/Makefile.inc M src/soc/intel/common/block/cpu/Makefile.inc M src/soc/intel/xeon_sp/Kconfig 14 files changed, 57 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/54301/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/54301 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I02e5017ab2b6a6fd30b37edad19165c2531c8fd1 Gerrit-Change-Number: 54301 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: console: Allow VPD to disable an otherwise enabled coreboot console
by Patrick Georgi (Code Review) 14 May '21

14 May '21

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45208 ) Change subject: console: Allow VPD to disable an otherwise enabled coreboot console ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: Since this is coming back to my agenda, I intend to revive this patch in some shape or form. Background: For some things we want to do in automated testing in CrOS we need access to a live feed of what happens during boot. One option would be to use the image.serial.bin that we create in the Chrome OS firmware build process and switch between the two images. Big downside: We'd test a very different binary (instead of a binary with a few different flags, as we customarily do), which brings test validity issues and also process difficulties (we'd have to keep image.serial.bin around in lots of places where we don't ship it today) Another option is to have the serial code present but normally disabled. So VPD is a bad place to put the toggle, got it. As for Marc's proposal to use the get_console_loglevel() mechanism: The idea isn't to reduce verbosity, it's about having all log data end up in cbmem at all times, but have serial used only occasionally, but with the above properties. GBB might be an easier sell wrt overhead brought into the bootblock (GBB support code is already compiled for the bootblock and liberally used across vboot), so that's something I'd be prepared to implement. That's still a relatively Chrome OS specific solution though, even if I can put most of the GBB wrangling into vboot and keep just a hook in the generic code. Thoughts? -- To view, visit https://review.coreboot.org/c/coreboot/+/45208 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4f1f5c45e5ea889176d04e0db438ca2aa7c536ee Gerrit-Change-Number: 45208 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Dossym Nurmukhanov <dossym(a)google.com> Gerrit-Reviewer: Greg Edelston <gredelston(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Fri, 14 May 2021 13:57:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Documentation/distributions: List System76
by Tim Crawford (Code Review) 14 May '21

14 May '21

Attention is currently required from: Jeremy Soller, Paul Menzel. Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54273 ) Change subject: Documentation/distributions: List System76 ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: From https://coreboot.org/users.html > System76 manufactures Linux laptops, desktops, and servers. Some models are sold with System76 Open Firmware, an open source distribution of firmware coreboot, EDK2, and System76 firmware applications. -- To view, visit https://review.coreboot.org/c/coreboot/+/54273 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I805f558514eb50580b5bd79bd4f964e66a15158d Gerrit-Change-Number: 54273 Gerrit-PatchSet: 2 Gerrit-Owner: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 14 May 2021 13:34:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP]arch/x86: Tear down CAR in ramstage
by Arthur Heymans (Code Review) 14 May '21

14 May '21

Attention is currently required from: Jason Glenesk, Anjaneya "Reddy" Chagam, Raul Rangel, Marshall Dawson, Jonathan Zhang, Johnny Lin, Morgan Jang, Patrick Rudolph, Felix Held. Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/54301 ) Change subject: [WIP]arch/x86: Tear down CAR in ramstage ...................................................................... [WIP]arch/x86: Tear down CAR in ramstage Postcar is a 'full' stage to just run a few instructions to disable CAR and setup MTRR for ramstage. This takes up a lot of place in ROM. This functionality can be moved into ramstage. Notes: Typically no caching is set up around cbmem so the LZMA compressed ramstage is being decompressed in uncached memory, which is likely to be slow. Change-Id: I02e5017ab2b6a6fd30b37edad19165c2531c8fd1 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/Kconfig M src/arch/x86/c_start.S M src/arch/x86/exit_car.S A src/arch/x86/exit_car.inc M src/arch/x86/postcar_loader.c M src/cpu/qemu-x86/Kconfig M src/cpu/x86/mtrr/Makefile.inc M src/lib/program.ld M src/mainboard/emulation/qemu-i440fx/Kconfig M src/mainboard/emulation/qemu-i440fx/Makefile.inc M src/soc/amd/common/block/cpu/car/Makefile.inc M src/soc/intel/common/block/cpu/Makefile.inc M src/soc/intel/xeon_sp/Kconfig 13 files changed, 56 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/54301/1 diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig index bb03db2..31170fa 100644 --- a/src/arch/x86/Kconfig +++ b/src/arch/x86/Kconfig @@ -207,12 +207,20 @@ hex default 0x2000000 +config RAMSTAGE_CAR_TEARDOWN + bool + depends on ARCH_X86 + depends on !RESET_VECTOR_IN_RAM + help + Tear down CAR in ramstage instead of in a separate postcar stage. + # Use the post CAR infrastructure for tearing down cache-as-ram # from a program loaded in RAM and subsequently loading ramstage. config POSTCAR_STAGE def_bool y depends on ARCH_X86 depends on !RESET_VECTOR_IN_RAM + depends on !RAMSTAGE_CAR_TEARDOWN config VERSTAGE_DEBUG_SPINLOOP bool diff --git a/src/arch/x86/c_start.S b/src/arch/x86/c_start.S index 19532d8..037bc21 100644 --- a/src/arch/x86/c_start.S +++ b/src/arch/x86/c_start.S @@ -59,6 +59,16 @@ movl %eax, _cbmem_top_ptr #endif +#if CONFIG(RAMSTAGE_CAR_TEARDOWN) +#include <cpu/x86/mtrr.h> +#include "exit_car.inc" + TEARDOWN_CAR + + movl $_estack, %esp + andl $0xfffffff0, %esp + call postcar_mtrr_setup +#endif + /** poison the stack. Code should not count on the * stack being full of zeros. This stack poisoning * recently uncovered a bug in the broadcast SIPI diff --git a/src/arch/x86/exit_car.S b/src/arch/x86/exit_car.S index 2bf1ed9..d02e28b 100644 --- a/src/arch/x86/exit_car.S +++ b/src/arch/x86/exit_car.S @@ -4,6 +4,8 @@ #include <cpu/x86/cr.h> #include <cpu/x86/cache.h> +#include "exit_car.inc" + /* Place the stack in the bss section. It's not necessary to define it in the * the linker script. */ .section .bss, "aw", @nobits @@ -44,21 +46,8 @@ clflush _cbmem_top_ptr skip_clflush: - /* chipset_teardown_car() is expected to disable cache-as-ram. */ - call chipset_teardown_car - /* Enable caching if not already enabled. */ -#ifdef __x86_64__ - mov %cr0, %rax - and $(~(CR0_CD | CR0_NW)), %eax - mov %rax, %cr0 -#else - mov %cr0, %eax - and $(~(CR0_CD | CR0_NW)), %eax - mov %eax, %cr0 -#endif - /* Ensure cache is clean. */ - invd + TEARDOWN_CAR movl $_estack, %esp /* Align stack to 16 bytes at call instruction. */ diff --git a/src/arch/x86/exit_car.inc b/src/arch/x86/exit_car.inc new file mode 100644 index 0000000..4c0ba65 --- /dev/null +++ b/src/arch/x86/exit_car.inc @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <cpu/x86/cr.h> +#include <cpu/x86/cache.h> + +.macro TEARDOWN_CAR + /* chipset_teardown_car() is expected to disable cache-as-ram. */ + call chipset_teardown_car + + /* Enable caching if not already enabled. */ +#ifdef __x86_64__ + mov %cr0, %rax + and $(~(CR0_CD | CR0_NW)), %eax + mov %rax, %cr0 +#else + mov %cr0, %eax + and $(~(CR0_CD | CR0_NW)), %eax + mov %eax, %cr0 +#endif + /* Ensure cache is clean. */ + invd +.endm diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index 9f5c47c..56c8776 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -131,8 +131,11 @@ void run_postcar_phase(struct postcar_frame *pcf) { - struct prog prog = - PROG_INIT(PROG_POSTCAR, CONFIG_CBFS_PREFIX "/postcar"); +#if CONFIG(RAMSTAGE_CAR_TEARDOWN) + struct prog prog = PROG_INIT(PROG_RAMSTAGE, CONFIG_CBFS_PREFIX "/ramstage"); +#else + struct prog prog = PROG_INIT(PROG_POSTCAR, CONFIG_CBFS_PREFIX "/postcar"); +#endif if (resume_from_stage_cache()) { stage_cache_load_stage(STAGE_POSTCAR, &prog); diff --git a/src/cpu/qemu-x86/Kconfig b/src/cpu/qemu-x86/Kconfig index 85f99e9..da66833 100644 --- a/src/cpu/qemu-x86/Kconfig +++ b/src/cpu/qemu-x86/Kconfig @@ -51,12 +51,10 @@ config CPU_QEMU_X86_64 bool "Experimental 64bit support" select ARCH_ALL_STAGES_X86_64 - select ARCH_POSTCAR_X86_64 config CPU_QEMU_X86_32 bool default n if CPU_QEMU_X86_64 default y select ARCH_ALL_STAGES_X86_32 - select ARCH_POSTCAR_X86_32 endif diff --git a/src/cpu/x86/mtrr/Makefile.inc b/src/cpu/x86/mtrr/Makefile.inc index a78517a..773c507 100644 --- a/src/cpu/x86/mtrr/Makefile.inc +++ b/src/cpu/x86/mtrr/Makefile.inc @@ -4,6 +4,7 @@ bootblock-y += earlymtrr.c verstage_x86-y += earlymtrr.c postcar-y += earlymtrr.c +ramstage-y += earlymtrr.c bootblock-y += debug.c romstage-y += debug.c diff --git a/src/lib/program.ld b/src/lib/program.ld index 8180d9f..32b4cb2 100644 --- a/src/lib/program.ld +++ b/src/lib/program.ld @@ -83,7 +83,7 @@ * traditional stages in the coreboot infrastructure. Therefore it's easier * to specialize this case. */ -#if ENV_RMODULE || ENV_POSTCAR +#if ENV_RMODULE || ENV_POSTCAR || ENV_RAMSTAGE _rmodule_params = .; KEEP(*(.module_parameters)); _ermodule_params = .; diff --git a/src/mainboard/emulation/qemu-i440fx/Kconfig b/src/mainboard/emulation/qemu-i440fx/Kconfig index 19f0fca..ebe17e4 100644 --- a/src/mainboard/emulation/qemu-i440fx/Kconfig +++ b/src/mainboard/emulation/qemu-i440fx/Kconfig @@ -16,6 +16,7 @@ select HAVE_ASAN_IN_ROMSTAGE select NO_SMM select BOOT_DEVICE_NOT_SPI_FLASH + select RAMSTAGE_CAR_TEARDOWN config VBOOT select VBOOT_MUST_REQUEST_DISPLAY diff --git a/src/mainboard/emulation/qemu-i440fx/Makefile.inc b/src/mainboard/emulation/qemu-i440fx/Makefile.inc index cac74ee..11f7c43 100644 --- a/src/mainboard/emulation/qemu-i440fx/Makefile.inc +++ b/src/mainboard/emulation/qemu-i440fx/Makefile.inc @@ -8,3 +8,4 @@ ramstage-y += fw_cfg.c ramstage-y += memmap.c ramstage-y += northbridge.c +ramstage-y += exit_car.S diff --git a/src/soc/amd/common/block/cpu/car/Makefile.inc b/src/soc/amd/common/block/cpu/car/Makefile.inc index 130b34b..b7726c1 100644 --- a/src/soc/amd/common/block/cpu/car/Makefile.inc +++ b/src/soc/amd/common/block/cpu/car/Makefile.inc @@ -5,6 +5,7 @@ bootblock-y += exit_car.S postcar-y += exit_car.S +ramstage-y += exit_car.S romstage-y += ap_exit_car.S romstage-y += exit_car.S diff --git a/src/soc/intel/common/block/cpu/Makefile.inc b/src/soc/intel/common/block/cpu/Makefile.inc index 42e392c..c9ea474 100644 --- a/src/soc/intel/common/block/cpu/Makefile.inc +++ b/src/soc/intel/common/block/cpu/Makefile.inc @@ -2,6 +2,7 @@ bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU)+= car/cache_as_ram_fsp.S ifeq ($(CONFIG_NO_FSP_TEMP_RAM_EXIT),y) postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += car/exit_car.S +ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += car/exit_car.S else postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += car/exit_car_fsp.S endif @@ -9,6 +10,7 @@ bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/cache_as_ram.S bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += ../../../../../cpu/x86/early_reset.S postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/exit_car.S +ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/exit_car.S endif bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c diff --git a/src/soc/intel/xeon_sp/Kconfig b/src/soc/intel/xeon_sp/Kconfig index 09f72a3..e93a279 100644 --- a/src/soc/intel/xeon_sp/Kconfig +++ b/src/soc/intel/xeon_sp/Kconfig @@ -33,7 +33,6 @@ select FSP_PLATFORM_MEMORY_SETTINGS_VERSIONS select FSP_T_XIP select FSP_M_XIP - select POSTCAR_STAGE select IOAPIC select PARALLEL_MP select PARALLEL_MP_AP_WORK @@ -60,6 +59,7 @@ select REG_SCRIPT select NO_FSP_TEMP_RAM_EXIT select INTEL_CAR_NEM # For postcar only now + select RAMSTAGE_CAR_TEARDOWN config MAINBOARD_USES_FSP2_0 bool @@ -70,7 +70,6 @@ depends on MAINBOARD_USES_FSP2_0 select PLATFORM_USES_FSP2_0 select UDK_202005_BINDING - select POSTCAR_STAGE config MAX_SOCKET int -- To view, visit https://review.coreboot.org/c/coreboot/+/54301 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I02e5017ab2b6a6fd30b37edad19165c2531c8fd1 Gerrit-Change-Number: 54301 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021