coreboot-gerrit April 2024

coreboot-gerrit@coreboot.org

1 participants
3292 discussions

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Enhance portability with uintptr_t/size_t
by Subrata Banik (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, Paul Menzel, Ronak Kanabar. Hello Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, Paul Menzel, Ronak Kanabar, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81615?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: drivers/intel/fsp2_0: Enhance portability with uintptr_t/size_t ...................................................................... drivers/intel/fsp2_0: Enhance portability with uintptr_t/size_t Replace fixed-width integers for pointers and sizes with uintptr_t and size_t, promoting portability across 32-bit and 64-bit architectures. For FSP-API specific UPD assignments, rely on `efi_uintn_t` rather fixed size datatype uint32_t/uint64_t. BUG=b:242829490 TEST=Firmware splash screen visible on google/rex0 w/ both 32-bit and 64-bit compilation. Change-Id: Iab5c612e0640441a2a10e77949416de2afdb8985 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/intel/fsp2_0/fsp_gop_blt.c M src/drivers/intel/fsp2_0/include/fsp/fsp_gop_blt.h 2 files changed, 9 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/81615/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/81615?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: Iab5c612e0640441a2a10e77949416de2afdb8985 Gerrit-Change-Number: 81615 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: lib: Refactor bmp_load_logo() implementation
by Subrata Banik (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, Ronak Kanabar, Sean Rhodes, Subrata Banik. Hello Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, Ronak Kanabar, Sean Rhodes, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81618?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: lib: Refactor bmp_load_logo() implementation ...................................................................... lib: Refactor bmp_load_logo() implementation This refactoring ensures bmp_load_logo() takes logo_size as an argument, returning a valid logo_ptr only if logo_size is non-zero. This prevents potential errors from mismatched size assumption. BUG=b:242829490 TEST=google/rex0 builds successfully. Change-Id: I14bc54670a67980ec93bc366b274832d1f959e50 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/intel/fsp1_1/ramstage.c M src/drivers/intel/fsp2_0/fsp_gop_blt.c M src/include/bootsplash.h M src/lib/bmp_logo.c M src/soc/amd/mendocino/fsp_s_params.c M src/soc/intel/apollolake/chip.c M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/skylake/chip.c 8 files changed, 29 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/81618/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81618?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: I14bc54670a67980ec93bc366b274832d1f959e50 Gerrit-Change-Number: 81618 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: 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: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/craaskov: Disable external fivr
by Paul Menzel (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Dinesh Gehlot, Dtrain Hsu, Ian Feng, Kapil Porwal, Nick Vaccaro, Subrata Banik. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81607?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: mb/google/nissa/var/craaskov: Disable external fivr ...................................................................... Patch Set 6: (2 comments) Patchset: PS6: Would be nice, if this scenario could be supported by devicetree. File src/mainboard/google/brya/variants/craaskov/variant.c: https://review.coreboot.org/c/coreboot/+/81607/comment/2814e996_64defc36 : PS6, Line 57: External external Though you could fix the other occurrences in a patch before. -- To view, visit https://review.coreboot.org/c/coreboot/+/81607?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: I9280a86bf78caa10b527a6569ac580dfe1d66f60 Gerrit-Change-Number: 81607 Gerrit-PatchSet: 6 Gerrit-Owner: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:33:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: mb/google/brox: Configure ISH device based on FW_CONFIG
by 9elements QA (Code Review) 02 Apr '24

02 Apr '24

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81418?usp=email ) Change subject: mb/google/brox: Configure ISH device based on FW_CONFIG ...................................................................... Patch Set 4: Automatic boot test returned (PASS/FAIL/TOTAL): 11 / 1 / 12 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES_ and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/204614 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/204612 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/204611 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/204610 PASS: 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/204609 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload SeaBIOS : https://lava.9esec.io/r/204608 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/204607 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload SeaBIOS : https://lava.9esec.io/r/204606 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/204605 FAIL: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/204604 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/204603 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/204602 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/81418?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: I678416acd48e03ab77ae299beae6e295a688b8df Gerrit-Change-Number: 81418 Gerrit-PatchSet: 4 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Tanu Malhotra <tanu.malhotra(a)intel.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: Yuval Peress <peress(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:31:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/rex/var/deku: Swap LAN device indices for correct MAC address
by Subrata Banik (Code Review) 02 Apr '24

02 Apr '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81564?usp=email ) ( 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: mb/google/rex/var/deku: Swap LAN device indices for correct MAC address ...................................................................... mb/google/rex/var/deku: Swap LAN device indices for correct MAC address Deku has two Ethernet ports. Currently both get assigned the wrong MAC address due to the LAN devices indices being swapped and vpd ethernet_mac0() affects device eth1 and vpd ethernet_mac1() affects device eth0. Correct the device indices for LAN devices so ethernet_mac[0-1] in vpd can apply to the correct ethernet ports. BUG=b:320203629 BRANCH=firmware-rex-15709.B TEST=vpd -s ethernet_mac0=<mac address0> vpd -s ethernet_mac1=<mac address1> reboot the system and check ifconfig eth0 and eth1 MAC addresses are fetched correctly Change-Id: Id1508104cbb5cf0a234f34f9db19cc535fdb634b Signed-off-by: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81564 Reviewed-by: Eric Lai <ericllai(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> Reviewed-by: Derek Huang <derekhuang(a)google.com> --- M src/mainboard/google/rex/variants/deku/overridetree.cb 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: Subrata Banik: Looks good to me, approved Derek Huang: Looks good to me, approved Eric Lai: Looks good to me, approved build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve diff --git a/src/mainboard/google/rex/variants/deku/overridetree.cb b/src/mainboard/google/rex/variants/deku/overridetree.cb index 0a383eb..ab0c6dc 100644 --- a/src/mainboard/google/rex/variants/deku/overridetree.cb +++ b/src/mainboard/google/rex/variants/deku/overridetree.cb @@ -83,7 +83,7 @@ chip drivers/net register "customized_leds" = "0x05af" register "wake" = "GPE0_DW0_01" # GPP_D01 - register "device_index" = "0" + register "device_index" = "1" register "add_acpi_dma_property" = "true" device pci 00.0 on end end @@ -98,7 +98,7 @@ chip drivers/net register "customized_leds" = "0x05af" register "wake" = "GPE0_DW1_04" # GPP_E04 - register "device_index" = "1" + register "device_index" = "0" register "add_acpi_dma_property" = "true" device pci 00.0 on end end -- To view, visit https://review.coreboot.org/c/coreboot/+/81564?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: Id1508104cbb5cf0a234f34f9db19cc535fdb634b Gerrit-Change-Number: 81564 Gerrit-PatchSet: 6 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kevin Chiu <kevin.chiu.17802(a)gmail.com> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: mb/google/rex/var/deku: Swap LAN device indices for correct MAC address
by Paul Menzel (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Tarun, Tony Huang, Zhuohao Lee. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81564?usp=email ) Change subject: mb/google/rex/var/deku: Swap LAN device indices for correct MAC address ...................................................................... Patch Set 5: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/81564?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: Id1508104cbb5cf0a234f34f9db19cc535fdb634b Gerrit-Change-Number: 81564 Gerrit-PatchSet: 5 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kevin Chiu <kevin.chiu.17802(a)gmail.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:22:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: soc/intel/xeon_sp/spr: Support dynamic domain SSDT generation
by Arthur Heymans (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81377?usp=email ) Change subject: soc/intel/xeon_sp/spr: Support dynamic domain SSDT generation ...................................................................... Patch Set 19: (1 comment) File src/soc/intel/xeon_sp/spr/soc_acpi.c: https://review.coreboot.org/c/coreboot/+/81377/comment/abc0aa48_0a1e3ce4 : PS19, Line 226: Domain0 device defined in uncore.asl maybe state that it's needed to access IO on PCH in DSDT? -- To view, visit https://review.coreboot.org/c/coreboot/+/81377?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: Icc5843feadc840d87c49b2aa4259716264520dba Gerrit-Change-Number: 81377 Gerrit-PatchSet: 19 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:16:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: mb/intel/beechnutcity_crb: Add GNR/SRF-SP 2S server board Beechnut City
by Arthur Heymans (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Chen, Gang C, David Hendricks, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Paul Menzel, Shuo Liu, TangYiwei. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81322?usp=email ) Change subject: mb/intel/beechnutcity_crb: Add GNR/SRF-SP 2S server board Beechnut City ...................................................................... Patch Set 11: Code-Review+2 (4 comments) Patchset: PS11: Looks good. A few nits File configs/builder/config.intel.crb.bnc: https://review.coreboot.org/c/coreboot/+/81322/comment/9dab4f8e_54d5b5a4 : PS11, Line 27: # : CONFIG_IFD_BIN_PATH="site-local/beechnutcity/descriptor.bin" : CONFIG_CPU_UCODE_BINARIES="site-local/beechnutcity/ucode.mcb" : CONFIG_FSP_T_FILE="site-local/beechnutcity/Server_T.fd" : CONFIG_FSP_M_FILE="site-local/beechnutcity/Server_M.fd" : CONFIG_FSP_S_FILE="site-local/beechnutcity/Server_S.fd" : CONFIG_FSP_HEADER_PATH="src/vendorcode/intel/fsp/fsp2_0/graniterapids/sp/" I'm not sure if upstream code should have references to site-local stuff? File src/mainboard/intel/beechnutcity_crb/Kconfig: https://review.coreboot.org/c/coreboot/+/81322/comment/a3e14dc4_41ff0265 : PS11, Line 8: select MAINBOARD_USES_FSP2_0 This should be removed (also from all other xeon_sp boards) File src/mainboard/intel/beechnutcity_crb/romstage.c: https://review.coreboot.org/c/coreboot/+/81322/comment/760c7179_78b94bd8 : PS11, Line 21: /* Set BIOS regeion UPD, otherwise MTRR might set incorrectly during TempRamExit API */ : mupd->FspmConfig.BiosRegionBase = FMAP_SECTION_SI_ALL_START + FMAP_SECTION_SI_ALL_SIZE; : mupd->FspmConfig.BiosRegionSize = FMAP_SECTION_FLASH_SIZE - FMAP_SECTION_SI_ALL_SIZE; This is not mainboard specific. I suggest adding a SI_BIOS region that has all the memory mapped stuff below it and use that instead of doing some computations. -- To view, visit https://review.coreboot.org/c/coreboot/+/81322?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: I3f6a0fb97b62baadb438fb9f11fdd78fccb3f89a Gerrit-Change-Number: 81322 Gerrit-PatchSet: 11 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Comment-Date: Tue, 02 Apr 2024 09:07:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: haswell NRI: Initialise MPLL
by Máté Kukri (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Bill XIE, Martin L Roth. Máté Kukri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77050?usp=email ) Change subject: haswell NRI: Initialise MPLL ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: This applies to the whole series, not just this commit, but this commit seems to have the most discussion. First of, this is absolutely amazing work, and in an ideal world it should have been merged years ago *but* - I've been looking at what it would take to reverse engineer Intel raminit algorithms using publically available datasheets and I/O tracing for the last few years and what kind of code that would result in - I've *also done* some static analysis of "MemoryInit.efi" from a Haswell mainboard to debug why it is failing when running via SerialICE - Without speculating *how* this was written, I think it would be extremely hard to justify how this code *is not* a derivative work of Intel's MRC If this was written with access to NDA documentation with Intel's blessing, then please ignore this as my brainfart, but *if not* I don't really see how this can be DCO signed-off as GPLv2-or-later. In the latter case, the best use of this for this code is a source-available, inspectable replacement for the MRC, but probably shouldn't be merged into coreboot in my opinion. -- To view, visit https://review.coreboot.org/c/coreboot/+/77050?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: I1f5fd87eee77b2a042b6065178718e8bf806887f Gerrit-Change-Number: 77050 Gerrit-PatchSet: 4 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Máté Kukri <kukri.mate(a)gmail.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 09:04:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[main]: soc/intel/xeon_sp: Add Granite Rapids initial codes
by Felix Singer (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Chen, Gang C, Christian Walter, David Hendricks, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Nico Huber, Patrick Rudolph, Paul Menzel, Shuo Liu, TangYiwei, Tim Chu. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81316?usp=email ) Change subject: soc/intel/xeon_sp: Add Granite Rapids initial codes ...................................................................... Patch Set 33: (6 comments) File src/soc/intel/xeon_sp/gnr/Makefile.inc: https://review.coreboot.org/c/coreboot/+/81316/comment/d3053b2a_0c4a3ae3 : PS33, Line 12: romstage-y += romstage.c soc_util.c : romstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c : : ramstage-y += chip.c cpu.c soc_util.c ramstage.c soc_acpi.c : ramstage-y += ../chip_gen6.c : : CPPFLAGS_common += -I$(src)/soc/intel/xeon_sp/gnr/include -I$(src)/soc/intel/xeon_sp/gnr : Use one value per line File src/soc/intel/xeon_sp/gnr/chip.h: https://review.coreboot.org/c/coreboot/+/81316/comment/4166b458_5aa042b2 : PS33, Line 16: u Use bool where possible. File src/soc/intel/xeon_sp/gnr/include/soc/pci_devs.h: https://review.coreboot.org/c/coreboot/+/81316/comment/7f18486f_b25e6528 : PS33, Line 184: // IIO DFX Global D7F7 registers : #define IIO_DFX_TSWCTL0 0x30c : #define IIO_DFX_LCK_CTL 0x504 : : // XHCI register : #define SYS_BUS_CFG2 0x44 : : /* Please be consistent with comment style. File src/soc/intel/xeon_sp/gnr/include/soc/soc_msr.h: https://review.coreboot.org/c/coreboot/+/81316/comment/5f2285c4_4506ed1e : PS33, Line 3: // TEMPORARY PLACE HOLDER! DO NOT USE! Eh? File src/soc/intel/xeon_sp/gnr/soc_util.c: https://review.coreboot.org/c/coreboot/+/81316/comment/4780b99a_9f9d5379 : PS33, Line 143: bool is_memtype_reserved(uint16_t mem_type) : { : return FALSE; : } : : bool is_memtype_non_volatile(uint16_t mem_type) : { : return FALSE; : } : : bool is_memtype_processor_attached(uint16_t mem_type) : { : return TRUE; : } : Please use true/false, the definitions from coreboot. File src/soc/intel/xeon_sp/uncore.c: PS33: The code movements should be separated out into their own commit. -- To view, visit https://review.coreboot.org/c/coreboot/+/81316?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: I3084e1b5abf25d8d9504bebeaed2a15b916ed56b Gerrit-Change-Number: 81316 Gerrit-PatchSet: 33 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: TangYiwei Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 08:48:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2024