coreboot-gerrit February 2024

coreboot-gerrit@coreboot.org

1 participants
3066 discussions

[S] Change in coreboot[main]: configs: Add test yaviks config
by David Ruth (Code Review) 02 Feb '24

02 Feb '24

David Ruth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/80306?usp=email ) Change subject: configs: Add test yaviks config ...................................................................... Abandoned No longer necessary with https://review.coreboot.org/c/coreboot/+/80305 -- To view, visit https://review.coreboot.org/c/coreboot/+/80306?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iacb476284b479f364710e154b203b32c5397e106 Gerrit-Change-Number: 80306 Gerrit-PatchSet: 1 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[S] Change in coreboot[main]: configs: Add test yaviks config
by David Ruth (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Nico Huber. David Ruth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80306?usp=email ) Change subject: configs: Add test yaviks config ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > This shouldn't be necessary anymore. The default configs are always build tested […] Thanks! I'll drop this then. -- To view, visit https://review.coreboot.org/c/coreboot/+/80306?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iacb476284b479f364710e154b203b32c5397e106 Gerrit-Change-Number: 80306 Gerrit-PatchSet: 1 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Fri, 02 Feb 2024 18:02:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: configs: Add test yaviks config
by Nico Huber (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: David Ruth. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80306?usp=email ) Change subject: configs: Add test yaviks config ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: This shouldn't be necessary anymore. The default configs are always build tested and with the preceding change, USE_MTCL is the default for this board already. -- To view, visit https://review.coreboot.org/c/coreboot/+/80306?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iacb476284b479f364710e154b203b32c5397e106 Gerrit-Change-Number: 80306 Gerrit-PatchSet: 1 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Feb 2024 18:00:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS
by Nico Huber (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: David Ruth, Eric Lai, Kapil Porwal, Nick Vaccaro. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80305?usp=email ) Change subject: mb/google/nissa/var/yaviks: Enable USE_MTCL for YAVIKS ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/80305?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iec54fc582d68b443665fceda47187c28f1a9216c Gerrit-Change-Number: 80305 Gerrit-PatchSet: 3 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Feb 2024 17:58:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/wifi: Use depends instead of if in Kconfig
by Nico Huber (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: David Ruth, David Ruth, Eric Lai. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80307?usp=email ) Change subject: drivers/wifi: Use depends instead of if in Kconfig ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: I fail to see how this is better? It loses the grouping and explicitness. -- To view, visit https://review.coreboot.org/c/coreboot/+/80307?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3604b394f999b28de4723337b3b6b4e21139c83b Gerrit-Change-Number: 80307 Gerrit-PatchSet: 2 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: David Ruth <druth(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Attention: David Ruth <druth(a)google.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 17:58:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/wifi: Add MTCL function to ACPI SSDT
by Nico Huber (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Arthur Heymans, Cliff Huang, David Ruth, Eric Lai, Kapil Porwal, Lance Zhao, Nick Vaccaro, Paul Menzel, Subrata Banik, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email ) Change subject: drivers/wifi: Add MTCL function to ACPI SSDT ...................................................................... Patch Set 19: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/80170?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9b5e7312a44e114270e664b983626faa6cfee350 Gerrit-Change-Number: 80170 Gerrit-PatchSet: 19 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 02 Feb 2024 17:56:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: treewide: Change I2C/SMBUS master/slave to controller/target
by Martin L Roth (Code Review) 02 Feb '24

02 Feb '24

Hello Arthur Heymans, Caveh Jalali, Christian Walter, Felix Held, Forest Mittelberg, Fred Reitberger, Hung-Te Lin, Jakub Czapiga, Jason Glenesk, Jeff Daly, Johnny Lin, Jonathan Zhang, Julius Werner, Lance Zhao, Matt DeVillier, Raul Rangel, Sean Rhodes, Tim Chu, Tim Wawrzynczak, Vanessa Eusebio, Xi Chen, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/77098?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: treewide: Change I2C/SMBUS master/slave to controller/target ...................................................................... treewide: Change I2C/SMBUS master/slave to controller/target The I2C and SMBUS specifications have updated their terminology from master/slave to controller/target. As stated in the coreboot language style page, using the word 'slave' should be avoided when possible [1] except when discussing the term as applicable by the current standards. As the SMBUS and I2C standards are no longer using this terminology, the updated terms controller/target should be used in the coreboot I2C/SMBUS code. We'll keep register names for devices are they are in the datasheets. [1] https://doc.coreboot.org/community/language_style.html#discussing-words-to-… Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I6c141fa4322de9e9ed208589f591560fed609825 --- M Documentation/technotes/console.md M Documentation/tutorial/part3.md M src/acpi/device.c M src/commonlib/bsd/include/commonlib/bsd/cb_err.h M src/console/Kconfig M src/device/dram/rcd.c M src/device/i2c_bus.c M src/device/software_i2c.c M src/drivers/analogix/anx7625/anx7625.c M src/drivers/i2c/designware/dw_i2c.c M src/drivers/i2c/tpm/tpm.c M src/drivers/i2c/ww_ring/ww_ring.c M src/drivers/intel/gma/edid.c M src/drivers/intel/gma/edid.h M src/drivers/smbus/i2c_smbus_console.c M src/drivers/smbus/sc16is7xx_init.c M src/ec/google/chromeec/ec_i2c.c M src/ec/kontron/kempld/kempld_i2c.c M src/include/device/i2c.h M src/include/device/i2c_bus.h M src/include/device/i2c_simple.h M src/include/device/smbus_host.h M src/include/smbios.h M src/lib/smbios.c M src/mainboard/asus/p8x7x-series/variants/p8c_ws/overridetree.cb M src/mainboard/google/auron/variants/auron_paine/include/variant/acpi/mainboard.asl M src/mainboard/google/auron/variants/auron_yuna/include/variant/acpi/mainboard.asl M src/mainboard/google/auron/variants/buddy/include/variant/acpi/mainboard.asl M src/mainboard/google/auron/variants/gandof/include/variant/acpi/mainboard.asl M src/mainboard/google/auron/variants/lulu/include/variant/acpi/mainboard.asl M src/mainboard/google/auron/variants/samus/include/variant/acpi/mainboard.asl M src/mainboard/google/cyan/acpi/touchscreen_elan.asl M src/mainboard/google/cyan/acpi/touchscreen_melfas.asl M src/mainboard/google/cyan/acpi/touchscreen_synaptics.asl M src/mainboard/google/cyan/acpi/trackpad_atmel.asl M src/mainboard/google/cyan/acpi/trackpad_elan.asl M src/mainboard/google/daisy/exynos5250.h M src/mainboard/google/daisy/mainboard.c M src/mainboard/google/kukui/boardid.c M src/mainboard/google/octopus/variants/baseboard/devicetree.cb M src/mainboard/google/rambi/acpi/mainboard.asl M src/mainboard/google/rambi/acpi/touchscreen_atmel.asl M src/mainboard/google/rambi/acpi/touchscreen_elan.asl M src/mainboard/google/rambi/acpi/touchscreen_wdt.asl M src/mainboard/google/rambi/acpi/trackpad_atmel.asl M src/mainboard/google/rambi/acpi/trackpad_elan.asl M src/mainboard/google/slippy/variants/falco/include/variant/acpi/mainboard.asl M src/mainboard/google/slippy/variants/leon/include/variant/acpi/mainboard.asl M src/mainboard/google/slippy/variants/peppy/include/variant/acpi/mainboard.asl M src/mainboard/google/slippy/variants/wolf/include/variant/acpi/mainboard.asl M src/mainboard/intel/glkrvp/touchpad.asl M src/mainboard/intel/strago/acpi/mainboard.asl M src/northbridge/intel/i945/gma.c M src/soc/amd/stoneyridge/smbus_spd.c M src/soc/cavium/cn81xx/twsi.c M src/soc/cavium/common/bdk-coreboot.c M src/soc/intel/apollolake/chip.h M src/soc/intel/baytrail/scc.c M src/soc/intel/common/block/include/intelblocks/imc.h M src/soc/intel/common/block/smbus/smbus.c M src/soc/mediatek/common/i2c.c M src/soc/nvidia/tegra/i2c.c M src/soc/qualcomm/common/qupv3_i2c.c M src/soc/qualcomm/ipq40xx/i2c.c M src/soc/qualcomm/ipq806x/i2c.c M src/soc/qualcomm/qcs405/i2c.c M src/soc/rockchip/common/i2c.c M src/soc/rockchip/rk3288/hdmi.c M src/soc/samsung/exynos5250/i2c.c M src/soc/samsung/exynos5420/i2c.c M src/southbridge/amd/pi/hudson/smbus_spd.c M src/southbridge/intel/bd82x6x/smbus.c M src/southbridge/intel/common/smbus.c M src/southbridge/intel/ibexpeak/smbus.c M src/southbridge/intel/lynxpoint/smbus.c M tests/device/i2c-test.c M util/intelvbttool/intelvbttool.c 77 files changed, 270 insertions(+), 270 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/77098/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/77098?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6c141fa4322de9e9ed208589f591560fed609825 Gerrit-Change-Number: 77098 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: Xi Chen <xixi.chen(a)mediatek.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[main]: treewide: Change I2C/SMBUS master/slave to controller/target
by Martin L Roth (Code Review) 02 Feb '24

02 Feb '24

Martin L Roth has restored this change. ( https://review.coreboot.org/c/coreboot/+/77098?usp=email ) Change subject: treewide: Change I2C/SMBUS master/slave to controller/target ...................................................................... Restored -- To view, visit https://review.coreboot.org/c/coreboot/+/77098?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6c141fa4322de9e9ed208589f591560fed609825 Gerrit-Change-Number: 77098 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: Xi Chen <xixi.chen(a)mediatek.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: restore

1 0

[M] Change in coreboot[main]: soc/amd: commonize PCI root IOAPIC initialization
by Matt DeVillier (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80286?usp=email ) Change subject: soc/amd: commonize PCI root IOAPIC initialization ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/80286/comment/e0de71cb_188d1eb6 : PS1, Line 17: Both the FCH IOAPIC and the PCIe root IOAPIC are still initialized : on Mandolin > haven't checked the timing before this patch, but the ioapic ids stay the same, since we have a spec […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/80286?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I7c0ec6ac2f11cb11e46248cceec96c1fd2a49c16 Gerrit-Change-Number: 80286 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 02 Feb 2024 17:37:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: cpu/x86/smm: Set up page tables in safe SMRAM
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Jérémy Compostella. Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80336?usp=email ) Change subject: cpu/x86/smm: Set up page tables in safe SMRAM ...................................................................... cpu/x86/smm: Set up page tables in safe SMRAM Relying on page tables being in RO flash is not safe in every setup, therefore set up some page tables in SMRAM that the permanent smihandler can use. Tested on QEMU. Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Change-Id: Icb3086abd577b9abb9966dd910a264a873ace4ed --- M src/cpu/x86/mp_init.c M src/cpu/x86/smm/smm_module_loader.c M src/include/cpu/x86/smm.h 3 files changed, 54 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/80336/1 diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 5a0d8a7..8f18de1 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -791,7 +791,6 @@ .num_cpus = num_cpus, .cpu_save_state_size = save_state_size, .num_concurrent_save_states = num_cpus, - .cr3 = read_cr3(), }; printk(BIOS_DEBUG, "Installing permanent SMM handler to 0x%08lx\n", smbase); diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index d965593..ca8c75d 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -8,6 +8,7 @@ #include <cpu/cpu.h> #include <cpu/x86/smm.h> #include <device/device.h> +#include <device/mmio.h> #include <rmodule.h> #include <stdio.h> #include <string.h> @@ -259,6 +260,7 @@ stub_params->stack_top = stack_top; stub_params->stack_size = g_stack_size; stub_params->c_handler = (uintptr_t)params->handler; + stub_params->cr3 = params->cr3; /* This runs on the BSP. All the APs are its siblings */ struct cpu_info *info = cpu_info(); @@ -353,8 +355,8 @@ region_end(&region)); } -/* STM + Handler + (Stub + Save state) * CONFIG_MAX_CPUS + stacks */ -#define SMM_REGIONS_ARRAY_SIZE (1 + 1 + CONFIG_MAX_CPUS * 2 + 1) +/* STM + Handler + (Stub + Save state) * CONFIG_MAX_CPUS + stacks + page tables*/ +#define SMM_REGIONS_ARRAY_SIZE (1 + 1 + CONFIG_MAX_CPUS * 2 + 1 + 1) static int append_and_check_region(const struct region smram, const struct region region, @@ -389,6 +391,39 @@ return 0; } +#define _PRES (1ULL << 0) +#define _RW (1ULL << 1) +#define _US (1ULL << 2) +#define _A (1ULL << 5) +#define _D (1ULL << 6) +#define _PS (1ULL << 7) +#define _GEN_DIR(a) (_PRES + _RW + _US + _A + (a)) +#define _GEN_PAGE(a) (_PRES + _RW + _US + _PS + _A + _D + (a)) +#define PAGE_SIZE 8 + +/* Return the PM4LE */ +static uintptr_t install_page_table(const uintptr_t handler_base) +{ + const bool one_g_pages = !!(cpuid_edx(0x80000001) & (1 << 26)); + /* 4 1G pages or 4 PDPE entries with 512 * 2M pages */ + const size_t pages_needed = one_g_pages ? 4 : 2048 + 4; + const uintptr_t pages_base = ALIGN_DOWN(handler_base - pages_needed * PAGE_SIZE, 4096); + const uintptr_t pm4le = ALIGN_DOWN(pages_base - 8, 4096); + + if (one_g_pages) { + for (int i = 0; i < 4; i++) + write64p(pages_base + i * PAGE_SIZE, _GEN_PAGE(1 * GiB * i)); + write64p(pm4le, _GEN_DIR(pages_base)); + } else { + for (int i = 0; i < 2048; i++) + write64p(pages_base + i * PAGE_SIZE, _GEN_PAGE(2 * MiB * i)); + write64p(pm4le, _GEN_DIR(pages_base + 2048 * PAGE_SIZE)); + for (int i = 0; i < 4; i++) + write64p(pages_base + (2048 + i) * PAGE_SIZE, _GEN_DIR(pages_base + 4096 * i)); + } + return pm4le; +} + /* *The SMM module is placed within the provided region in the following * manner: @@ -398,6 +433,8 @@ * +-----------------+ * | smi handler | * | ... | + * +-----------------+ + * | page tables | * +-----------------+ <- cpu0 * | stub code | <- cpu1 * | stub code | <- cpu2 @@ -453,7 +490,20 @@ if (append_and_check_region(smram, handler, region_list, "HANDLER")) return -1; - uintptr_t stub_segment_base = handler_base - SMM_CODE_SEGMENT_SIZE; + uintptr_t stub_segment_base; + if (ENV_X86_64) { + uintptr_t pt_base = install_page_table(handler_base); + struct region page_tables = { + .offset = pt_base, + .size = handler_base - pt_base, + }; + if (append_and_check_region(smram, page_tables, region_list, "PAGE TABLES")) + return -1; + params->cr3 = pt_base; + stub_segment_base = pt_base - SMM_CODE_SEGMENT_SIZE; + } else { + stub_segment_base = handler_base - SMM_CODE_SEGMENT_SIZE; + } if (!smm_create_map(stub_segment_base, params->num_concurrent_save_states, params)) { printk(BIOS_ERR, "%s: Error creating CPU map\n", __func__); diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 6a0ec3f..a12065b 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -109,6 +109,7 @@ u32 stack_size; u32 stack_top; u32 c_handler; + u32 cr3; /* The apic_id_to_cpu provides a mapping from APIC id to CPU number. * The CPU number is indicated by the index into the array by matching * the default APIC id and value at the index. The stub loader -- To view, visit https://review.coreboot.org/c/coreboot/+/80336?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Icb3086abd577b9abb9966dd910a264a873ace4ed Gerrit-Change-Number: 80336 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2024