coreboot-gerrit August 2023

coreboot-gerrit@coreboot.org

1 participants
6717 discussions

[S] Change in coreboot[master]: acpi/acpi.c: Move setting FADT SCI INT to arch specific code
by Kyösti Mälkki (Code Review) 02 Aug '23

02 Aug '23

Attention is currently required from: Arthur Heymans, Lance Zhao, Tim Wawrzynczak. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76295?usp=email ) Change subject: acpi/acpi.c: Move setting FADT SCI INT to arch specific code ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/76295?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic1533cb520a057b29fc8f926db38338cd3401b18 Gerrit-Change-Number: 76295 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 02 Aug 2023 07:34:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/google/nissa/var/craaskov: Configure GPIOs according to schematics
by Ian Feng (Code Review) 02 Aug '23

02 Aug '23

Attention is currently required from: Chen-Tsung Hsieh, Derek Huang, Dtrain Hsu, Jamie Chen, Kenny Pan, Paul Menzel, Rex Chou, Shou-Chieh Hsu, Sumeet R Pawnikar, Tarun Tuli. Ian Feng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76636?usp=email ) Change subject: mb/google/nissa/var/craaskov: Configure GPIOs according to schematics ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/76636?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I17fc9333a0ef592ea36b196b3fd417be47fb82bb Gerrit-Change-Number: 76636 Gerrit-PatchSet: 12 Gerrit-Owner: Rex Chou <rex_chou(a)compal.corp-partner.google.com> Gerrit-Reviewer: Chen-Tsung Hsieh <chentsung(a)google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Kenny Pan <kennypan(a)google.com> Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Chen-Tsung Hsieh <chentsung(a)google.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Rex Chou <rex_chou(a)compal.corp-partner.google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Attention: Kenny Pan <kennypan(a)google.com> Gerrit-Attention: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Comment-Date: Wed, 02 Aug 2023 07:32:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: acpi/acpi.c: Move MADT lapic addr to APIC code
by Kyösti Mälkki (Code Review) 02 Aug '23

02 Aug '23

Attention is currently required from: Arthur Heymans, Lance Zhao, Tim Wawrzynczak. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76070?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: acpi/acpi.c: Move MADT lapic addr to APIC code ...................................................................... Patch Set 4: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/76070/comment/1767814c_8bbdee95 : PS4, Line 7: acpi/acpi.c: Move MADT lapic addr to APIC code acpi/acpi.c: Add and use acpi_arch_fill_madt() -- To view, visit https://review.coreboot.org/c/coreboot/+/76070?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4e5032fd02af7e8e9ffd2e20aa214a8392ab6335 Gerrit-Change-Number: 76070 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 02 Aug 2023 07:31:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: arch/arm64: Hook up ACPI table generation
by Arthur Heymans (Code Review) 02 Aug '23

02 Aug '23

Attention is currently required from: Arthur Heymans, Kyösti Mälkki, Lean Sheng Tan, Nico Huber. Hello Julius Werner, Kyösti Mälkki, Lean Sheng Tan, Nico Huber, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/76071?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Code-Review+1 by Lean Sheng Tan, Verified+1 by build bot (Jenkins) Change subject: arch/arm64: Hook up ACPI table generation ...................................................................... arch/arm64: Hook up ACPI table generation Linux is able to detect and use ACPI tables on an out of tree target using hacked version of u-boot to pass the via via UEFI. Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Change-Id: I4f60c546ec262ffb4d447fe6476844cf5a1b756d --- M src/arch/arm64/Kconfig M src/arch/arm64/tables.c 2 files changed, 20 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/76071/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/76071?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4f60c546ec262ffb4d447fe6476844cf5a1b756d Gerrit-Change-Number: 76071 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: acpi.c: Fill in >4G FADT entries correctly
by Arthur Heymans (Code Review) 02 Aug '23

02 Aug '23

Attention is currently required from: Felix Held, Kyösti Mälkki, Lance Zhao, Lean Sheng Tan, Nico Huber, Tim Wawrzynczak. Hello Felix Held, Lance Zhao, Lean Sheng Tan, Nico Huber, Tim Wawrzynczak, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/76179?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Code-Review+2 by Lean Sheng Tan, Verified+1 by build bot (Jenkins) Change subject: acpi.c: Fill in >4G FADT entries correctly ...................................................................... acpi.c: Fill in >4G FADT entries correctly Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Change-Id: I84ab0068e8409a5e525ddc781347087680d80640 --- M src/acpi/acpi.c 1 file changed, 10 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/76179/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/76179?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84ab0068e8409a5e525ddc781347087680d80640 Gerrit-Change-Number: 76179 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: acpi.c: Fill in >4G FADT entries correctly
by Arthur Heymans (Code Review) 02 Aug '23

02 Aug '23

Attention is currently required from: Felix Held, Kyösti Mälkki, Lance Zhao, Nico Huber, Tim Wawrzynczak. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76179?usp=email ) Change subject: acpi.c: Fill in >4G FADT entries correctly ...................................................................... Patch Set 5: (2 comments) File src/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/76179/comment/cf3b6d02_290af735 : PS5, Line 1092: fadt->x_firmware_ctl_h = (uint32_t)((uint64_t)(uintptr_t)facs >> 32); > Wouldn't write shift work here without warnings with just: > (uintptr_t)facs >> 32 That results in shift-count-overflow warning/error on 32bit targets. All the casts are to make the compiler happy in both 32 and 64bit cases. https://review.coreboot.org/c/coreboot/+/76179/comment/a5cd44ff_88738af3 : PS5, Line 1096: fadt->dsdt = (unsigned long)dsdt; > (uintptr_t) for consistency here? Done -- To view, visit https://review.coreboot.org/c/coreboot/+/76179?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I84ab0068e8409a5e525ddc781347087680d80640 Gerrit-Change-Number: 76179 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 02 Aug 2023 07:19:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: acpi.c: Guard FACS generation
by 9elements QA (Code Review) 02 Aug '23

02 Aug '23

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76178?usp=email ) Change subject: acpi.c: Guard FACS generation ...................................................................... Patch Set 4: Automatic boot test returned (PASS/FAIL/TOTAL): 8 / 4 / 12 FAIL: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/175959 FAIL: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/175958 FAIL: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload TianoCore : https://lava.9esec.io/r/175957 FAIL: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/175956 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload TianoCore : https://lava.9esec.io/r/175955 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload SeaBIOS : https://lava.9esec.io/r/175954 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload TianoCore : https://lava.9esec.io/r/175953 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload SeaBIOS : https://lava.9esec.io/r/175952 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/175951 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/175950 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/175949 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/175948 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/76178?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7a1f36616e7f6adb021625e62e0fdf81864c7ac3 Gerrit-Change-Number: 76178 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-Comment-Date: Wed, 02 Aug 2023 07:17:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: acpi.c: Find FACS using 64bit address fields
by Arthur Heymans (Code Review) 02 Aug '23

02 Aug '23

Attention is currently required from: Lance Zhao. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76293?usp=email ) Change subject: acpi.c: Find FACS using 64bit address fields ...................................................................... Patch Set 2: (1 comment) File src/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/76293/comment/ad0d1637_79074969 : PS2, Line 1633: | (uint64_t)fadt->x_firmware_ctl_h << 32); > For some reason, src/southbridge/amd/pi/hudson/fadt.c has this: > > fadt->x_firmware_ctl_l = 0; /* set to 0 if firmware_ctrl is used */ > fadt->x_firmware_ctl_h = 0; > > The only hardware using this now, pcengines/apu[2..6] do not support S3 but I think this change would break S3 resume if FACS was ever located above 4G. I guess that can safely be removed. -- To view, visit https://review.coreboot.org/c/coreboot/+/76293?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I406b9b470d6e76867e47cfda427b199e20cc9b32 Gerrit-Change-Number: 76293 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Comment-Date: Wed, 02 Aug 2023 07:14:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/meteorlake: Hook up UPD for C1 C-state auto-demotion
by Sukumar Ghorai (Code Review) 02 Aug '23

02 Aug '23

Attention is currently required from: Jakub Czapiga, Kapil Porwal, Paul Menzel, Subrata Banik, Sukumar Ghorai, Tarun Tuli. Hello Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun Tuli, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/76826?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: soc/intel/meteorlake: Hook up UPD for C1 C-state auto-demotion ...................................................................... soc/intel/meteorlake: Hook up UPD for C1 C-state auto-demotion FSP has a parameter to enable/disable c1-state autodemotion feature. Boards/Baseboard can choose to use this feature as per requirement. This patch hooks up this parameter to devicetree. BUG=b:286328295 TEST=Check code compiles & boot google/rex, and correct value has been passed to FSP. Change-Id: I2cc60bd297271fcb3000c0298af71208e3be60fc Signed-off-by: Sukumar Ghorai <sukumar.ghorai(a)intel.com> --- M src/soc/intel/meteorlake/chip.h M src/soc/intel/meteorlake/fsp_params.c 2 files changed, 14 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/76826/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/76826?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2cc60bd297271fcb3000c0298af71208e3be60fc Gerrit-Change-Number: 76826 Gerrit-PatchSet: 6 Gerrit-Owner: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[master]: arch/x86: Adjust stack pointer to point to dummy argument
by Arthur Heymans (Code Review) 02 Aug '23

02 Aug '23

Attention is currently required from: Elyes HAOUAS, Maximilian Brune, Ronak Kanabar, Sridhar Siricilla, Subrata Banik. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76878?usp=email ) Change subject: arch/x86: Adjust stack pointer to point to dummy argument ...................................................................... Patch Set 1: Code-Review-2 (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/76878/comment/f7196e60_d97dc949 : PS1, Line 9: The patch pushes dummy argument so that stack pointer points to this : dummy element instead of coreboot table address as depthcharge picks the : second element from top of the stack as coreboot table address. The coreboot Documentation specifies that payloads need to be entered in 32bit mode and while not documented the argument containing the coreboot tables just follows the ABI which here means pushing a 32bit variable on the stack. I think you should fix this in depthcharge rather than break all other payloads that consume the calling argument to pass the coreboot tables. Patchset: PS1: This looks like a depthcharge problem and not a coreboot problem. File src/arch/x86/c_exit.S: https://review.coreboot.org/c/coreboot/+/76878/comment/6aa843ed_5f514c7d : PS1, Line 32: subl $12, %esp Stack is not aligned to 16 any longer when calling a function. https://review.coreboot.org/c/coreboot/+/76878/comment/ae7c9802_a04d364a : PS1, Line 34: pushl $0 /* Dummy argument */ The 32bit ABI has argument 0 pushed last to the stack. This will break all payloads that expect it. -- To view, visit https://review.coreboot.org/c/coreboot/+/76878?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6e4034f65f5db23560f774f56e0edfb08e4283ad Gerrit-Change-Number: 76878 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Elyes HAOUAS Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Elyes HAOUAS Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Comment-Date: Wed, 02 Aug 2023 07:04:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2023