coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3043 discussions

[XS] Change in coreboot[main]: src/arch/x86/exit_car: Add proper x86_64 code
by Martin L Roth (Code Review) 08 Jan '24

08 Jan '24

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79162?usp=email ) Change subject: src/arch/x86/exit_car: Add proper x86_64 code ...................................................................... src/arch/x86/exit_car: Add proper x86_64 code Don't truncate upper bits in assembly code and thus allow loading of postcar stage above 4GiB. Tested on qemu with cbmem_top set to TOUUD. Change-Id: I42d1086f1220e44076ccf613244fc3c6d804805b Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Signed-off-by: Benjamin Doron <benjamin.doron(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/79162 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/exit_car.S 1 file changed, 6 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/src/arch/x86/exit_car.S b/src/arch/x86/exit_car.S index c9ff866..d435dbe 100644 --- a/src/arch/x86/exit_car.S +++ b/src/arch/x86/exit_car.S @@ -67,8 +67,14 @@ invd movl $_estack, %esp +#if ENV_X86_64 + /* Align stack to 16 bytes at call instruction. */ + movq $0xfffffffffffffff0, %rax + and %rax, %rsp +#else /* Align stack to 16 bytes at call instruction. */ andl $0xfffffff0, %esp +#endif /* Call this in assembly as some platforms like to mess with the bootflow and call into main directly from chipset_teardown_car. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/79162?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: I42d1086f1220e44076ccf613244fc3c6d804805b Gerrit-Change-Number: 79162 Gerrit-PatchSet: 4 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: Documentation: Improve x86_64
by Martin L Roth (Code Review) 08 Jan '24

08 Jan '24

Attention is currently required from: Arthur Heymans, Benjamin Doron, Felix Held, Patrick Rudolph. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79160?usp=email ) Change subject: Documentation: Improve x86_64 ...................................................................... Patch Set 2: (4 comments) File Documentation/arch/x86/x86_64.md: https://review.coreboot.org/c/coreboot/+/79160/comment/6b7713bc_1984a5f0 : PS1, Line 3: experimental > I'd prefer to make x86_64 support default in the next releases. That sounds great to me. https://review.coreboot.org/c/coreboot/+/79160/comment/529f5050_c941ad79 : PS1, Line 24: : The large memory model causes the compiler to emit 64bit addressing : instructions, which are not only slower, but also increase code size. > The x86_64 code runs a bit faster, but takes longer to be fetched from SPI as the size is bigger. […] So can change this to: ``` Page tables can be used to provide security benefits, such as by marking memory as non-executable or removing it entirely. This could be useful for SMM: mark regular DRAM as NX. The large memory model causes the compiler to emit 64bit addressing instructions, which increases code size. In theory, this is roughly made up for by the faster execution of the x86_64 code. ``` https://review.coreboot.org/c/coreboot/+/79160/comment/fcf76fdd_6ad36b6c : PS1, Line 28: must generate page tables at build time > It's describing the current implementation, not a requirement. Sure, Can we update the text to reflect that? Maybe: ``` For the current implementation, coreboot generates page tables at build time that configure 0-4GiB as WB, using 2MiB-pages. ``` https://review.coreboot.org/c/coreboot/+/79160/comment/0e07fb3c_916b9407 : PS1, Line 140: Here's a list of known issues: > It's under "KVM enabled qemu", so yes it's referring to qemu only. Real hardware is fine. Change to `Known issues for QEMU:`? -- To view, visit https://review.coreboot.org/c/coreboot/+/79160?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: Ia5ba51be629a8c878aad64d3297176457cf8e855 Gerrit-Change-Number: 79160 Gerrit-PatchSet: 2 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 08 Jan 2024 19:39:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com> Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: Choose Correct FW splash screen at runtime
by Julius Werner (Code Review) 08 Jan '24

08 Jan '24

Attention is currently required from: Andrey Petrov, Ronak Kanabar, Shelley Chen, Subrata Banik. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79775?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: Choose Correct FW splash screen at runtime ...................................................................... Patch Set 8: Code-Review+2 (4 comments) File src/include/bootsplash.h: https://review.coreboot.org/c/coreboot/+/79775/comment/2879d3a4_e6a59a70 : PS8, Line 24: get_bmp_file nit: Since this is now an exported function I think it should start with `bmp_` for cleaner namespacing (e.g. `bmp_logo_filename()` or something like that). File src/lib/Kconfig: https://review.coreboot.org/c/coreboot/+/79775/comment/6ce1e835_d64f11e2 : PS6, Line 153: HAVE_CUSTOM_BMP_LOGO > Acknowledged It probably makes sense to inject these options from the ebuild based on a USE flag. File src/vendorcode/google/chromeos/Kconfig: https://review.coreboot.org/c/coreboot/+/79775/comment/2546d0f8_dd55aa59 : PS8, Line 88: to nit: typo? https://review.coreboot.org/c/coreboot/+/79775/comment/278fc528_b85dfe9d : PS8, Line 89: an nit: typo? -- To view, visit https://review.coreboot.org/c/coreboot/+/79775?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: I56613d1e7e81e25b31ad034edae0f716c94c4960 Gerrit-Change-Number: 79775 Gerrit-PatchSet: 8 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Mon, 08 Jan 2024 18:28:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Shelley Chen <shchen(a)google.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: libpayload: Move back the ttb_buffer section
by Julius Werner (Code Review) 08 Jan '24

08 Jan '24

Attention is currently required from: Yi Chou. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79800?usp=email ) Change subject: libpayload: Move back the ttb_buffer section ...................................................................... Patch Set 3: Code-Review+2 (2 comments) File payloads/libpayload/arch/arm64/libpayload.ldscript: https://review.coreboot.org/c/coreboot/+/79800/comment/1610f893_312fec5b : PS3, Line 65: *(.bss.ttb_buffer) I think you might as well remove this completely. We only need this for depthcharge since the whole BSS-erasing thing only happens in depthcharge. With this new approach, it's enough to do this only in the depthcharge linker script and let the default libpayload script subsume it under `.bss.*`. File payloads/libpayload/arch/arm64/mmu.c: https://review.coreboot.org/c/coreboot/+/79800/comment/62a77f56_db402cb4 : PS3, Line 45: /* Don't chage the name of this variable without also changing the name in the (Would need to change this text a bit then to explain that we refer to this in the linker script for ChormeOS's depthcharge payload and to please not change the name without discussing with us. Maybe put your email address as a contact.) -- To view, visit https://review.coreboot.org/c/coreboot/+/79800?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: I9bb08878dd4be01d9ed3f96933f774dd6296f76e Gerrit-Change-Number: 79800 Gerrit-PatchSet: 3 Gerrit-Owner: Yi Chou <yich(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yi Chou <yich(a)google.com> Gerrit-Comment-Date: Mon, 08 Jan 2024 18:08:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: cpu/x86/smi_trigger: use enum cb_err as apm_control return type
by Jérémy Compostella (Code Review) 08 Jan '24

08 Jan '24

Attention is currently required from: Felix Held. Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79835?usp=email ) Change subject: cpu/x86/smi_trigger: use enum cb_err as apm_control return type ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/79835?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: I07ced74cae915df52a9d439835b84237d51fdd11 Gerrit-Change-Number: 79835 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 08 Jan 2024 17:31:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: arch/x86/include/smm: improve documentation of call_smm
by Jérémy Compostella (Code Review) 08 Jan '24

08 Jan '24

Attention is currently required from: Felix Held, Matt DeVillier. Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79834?usp=email ) Change subject: arch/x86/include/smm: improve documentation of call_smm ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/79834/comment/595fb9fb_d8858e93 : PS2, Line 9: Since the inline assembly code doesn't make it exactly obvious how this This sentence is a bit confusing in IMO. -- To view, visit https://review.coreboot.org/c/coreboot/+/79834?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: I3566af191492ce00a3033335ff80e01c33e98e63 Gerrit-Change-Number: 79834 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 08 Jan 2024 17:30:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[NoOp] Change in coreboot[main]: arch/x86/include: rename smm.h to smm_call.h
by Jérémy Compostella (Code Review) 08 Jan '24

08 Jan '24

Attention is currently required from: Felix Held. Jérémy Compostella has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79833?usp=email ) Change subject: arch/x86/include: rename smm.h to smm_call.h ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/79833?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: Ia907ad92459e835feeddf7eb4743a38f99549179 Gerrit-Change-Number: 79833 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 08 Jan 2024 17:23:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: Choose Correct FW splash screen at runtime
by Shelley Chen (Code Review) 08 Jan '24

08 Jan '24

Attention is currently required from: Andrey Petrov, Julius Werner, Ronak Kanabar, Subrata Banik. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79775?usp=email ) Change subject: Choose Correct FW splash screen at runtime ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79775?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: I56613d1e7e81e25b31ad034edae0f716c94c4960 Gerrit-Change-Number: 79775 Gerrit-PatchSet: 8 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Mon, 08 Jan 2024 16:45:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: sb/amd/pi/hudson/smhandler: use apm_get_apmc() in APMC SMI handler
by Felix Held (Code Review) 08 Jan '24

08 Jan '24

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79850?usp=email ) Change subject: sb/amd/pi/hudson/smhandler: use apm_get_apmc() in APMC SMI handler ...................................................................... sb/amd/pi/hudson/smhandler: use apm_get_apmc() in APMC SMI handler Instead of open-coding this functionality and using non-common defines, call the apm_get_apmc() helper function. This also brings this more in line with the newer AMD SoCs. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ic16596404f46bf431e1c5db56859ddfea5fccbf8 --- M src/southbridge/amd/pi/hudson/smihandler.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/79850/1 diff --git a/src/southbridge/amd/pi/hudson/smihandler.c b/src/southbridge/amd/pi/hudson/smihandler.c index 966550f..8b88c06 100644 --- a/src/southbridge/amd/pi/hudson/smihandler.c +++ b/src/southbridge/amd/pi/hudson/smihandler.c @@ -25,7 +25,7 @@ static void hudson_apmc_smi_handler(void) { u32 reg32; - const uint8_t cmd = inb(ACPI_SMI_CTL_PORT); + const uint8_t cmd = apm_get_apmc(); switch (cmd) { case APM_CNT_ACPI_ENABLE: -- To view, visit https://review.coreboot.org/c/coreboot/+/79850?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: Ic16596404f46bf431e1c5db56859ddfea5fccbf8 Gerrit-Change-Number: 79850 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: sb/amd/pi/hudson: select HAVE_CONFIGURABLE_APMC_SMI_PORT
by Felix Held (Code Review) 08 Jan '24

08 Jan '24

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79849?usp=email ) Change subject: sb/amd/pi/hudson: select HAVE_CONFIGURABLE_APMC_SMI_PORT ...................................................................... sb/amd/pi/hudson: select HAVE_CONFIGURABLE_APMC_SMI_PORT Select HAVE_CONFIGURABLE_APMC_SMI_PORT and implement the pm_acpi_smi_cmd_port helper function. TEST=APU2 still compiles with HAVE_SMI_HANDLER selected and NO_SMM select removed. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I8f79d8c1d59aa1b6c1145dd0b1cbc9010a1c57e7 --- M src/southbridge/amd/pi/hudson/Kconfig M src/southbridge/amd/pi/hudson/smi_util.c 2 files changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/79849/1 diff --git a/src/southbridge/amd/pi/hudson/Kconfig b/src/southbridge/amd/pi/hudson/Kconfig index 0b90b9a..04d77fd 100644 --- a/src/southbridge/amd/pi/hudson/Kconfig +++ b/src/southbridge/amd/pi/hudson/Kconfig @@ -13,6 +13,7 @@ select ACPI_COMMON_MADT_IOAPIC select ACPI_COMMON_MADT_LAPIC select ACPI_CUSTOM_MADT + select HAVE_CONFIGURABLE_APMC_SMI_PORT select HAVE_USBDEBUG_OPTIONS select HAVE_CF9_RESET select HAVE_CF9_RESET_PREPARE diff --git a/src/southbridge/amd/pi/hudson/smi_util.c b/src/southbridge/amd/pi/hudson/smi_util.c index 3dc2d8a..70b3585 100644 --- a/src/southbridge/amd/pi/hudson/smi_util.c +++ b/src/southbridge/amd/pi/hudson/smi_util.c @@ -6,7 +6,9 @@ #include <amdblocks/acpimmio.h> #include <console/console.h> +#include <cpu/x86/smm.h> +#include "hudson.h" #include "smi.h" #define HUDSON_SMI_ACPI_COMMAND 75 @@ -77,3 +79,8 @@ { configure_smi(HUDSON_SMI_ACPI_COMMAND, SMI_MODE_SMI); } + +uint16_t pm_acpi_smi_cmd_port(void) +{ + return pm_read16(PM_ACPI_SMI_CMD); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/79849?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: I8f79d8c1d59aa1b6c1145dd0b1cbc9010a1c57e7 Gerrit-Change-Number: 79849 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024