coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5823 discussions

Change in coreboot[master]: util/intelp2m: use import once for all included modules
by Patrick Georgi (Code Review) 02 Jul '21

02 Jul '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55985 ) Change subject: util/intelp2m: use import once for all included modules ...................................................................... util/intelp2m: use import once for all included modules There is no need to repeat "import" for each module in GoLang. Use this keyword only once in each file for code cleanliness. Change-Id: Ibb24fafd409b31b174946a39ca1f810d59b87e76 Signed-off-by: Maxim Polyakov <max.senia.poliak(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55985 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/intelp2m/fields/cb/cb.go M util/intelp2m/fields/fields.go M util/intelp2m/fields/raw/raw.go M util/intelp2m/main.go M util/intelp2m/parser/parser.go M util/intelp2m/platforms/apl/macro.go M util/intelp2m/platforms/cnl/macro.go M util/intelp2m/platforms/common/macro.go M util/intelp2m/platforms/lbg/macro.go M util/intelp2m/platforms/snr/macro.go 10 files changed, 62 insertions(+), 56 deletions(-) Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved diff --git a/util/intelp2m/fields/cb/cb.go b/util/intelp2m/fields/cb/cb.go index 3bda62e..3603aac 100644 --- a/util/intelp2m/fields/cb/cb.go +++ b/util/intelp2m/fields/cb/cb.go @@ -1,7 +1,9 @@ package cb -import "../../config" -import "../../platforms/common" +import ( + "../../config" + "../../platforms/common" +) type FieldMacros struct {} diff --git a/util/intelp2m/fields/fields.go b/util/intelp2m/fields/fields.go index d2ca0e8..e8d8625 100644 --- a/util/intelp2m/fields/fields.go +++ b/util/intelp2m/fields/fields.go @@ -1,11 +1,12 @@ package fields -import "../config" -import "../platforms/common" - -import "./fsp" -import "./cb" -import "./raw" +import ( + "../config" + "../platforms/common" + "./fsp" + "./cb" + "./raw" +) // InterfaceSet - set the interface for decoding configuration // registers DW0 and DW1. diff --git a/util/intelp2m/fields/raw/raw.go b/util/intelp2m/fields/raw/raw.go index a54e51d..e6fa25c 100644 --- a/util/intelp2m/fields/raw/raw.go +++ b/util/intelp2m/fields/raw/raw.go @@ -1,7 +1,9 @@ package raw -import "fmt" -import "../../platforms/common" +import ( + "fmt" + "../../platforms/common" +) type FieldMacros struct {} diff --git a/util/intelp2m/main.go b/util/intelp2m/main.go index 511f54d..fdfc34f 100644 --- a/util/intelp2m/main.go +++ b/util/intelp2m/main.go @@ -1,11 +1,12 @@ package main -import "flag" -import "fmt" -import "os" - -import "./parser" -import "./config" +import ( + "flag" + "fmt" + "os" + "./parser" + "./config" +) // generateOutputFile - generates include file // parser : parser data structure diff --git a/util/intelp2m/parser/parser.go b/util/intelp2m/parser/parser.go index c0dc65f..bfcdff8 100644 --- a/util/intelp2m/parser/parser.go +++ b/util/intelp2m/parser/parser.go @@ -5,15 +5,14 @@ "fmt" "strings" "strconv" + "../platforms/common" + "../platforms/snr" + "../platforms/lbg" + "../platforms/apl" + "../platforms/cnl" + "../config" ) -import "../platforms/common" -import "../platforms/snr" -import "../platforms/lbg" -import "../platforms/apl" -import "../platforms/cnl" -import "../config" - // PlatformSpecific - platform-specific interface type PlatformSpecific interface { GenMacro(id string, dw0 uint32, dw1 uint32, ownership uint8) string diff --git a/util/intelp2m/platforms/apl/macro.go b/util/intelp2m/platforms/apl/macro.go index 0364067..5f27074 100644 --- a/util/intelp2m/platforms/apl/macro.go +++ b/util/intelp2m/platforms/apl/macro.go @@ -1,12 +1,12 @@ package apl -import "fmt" -import "strconv" - -// Local packages -import "../common" -import "../../config" -import "../../fields" +import ( + "fmt" + "strconv" + "../common" + "../../config" + "../../fields" +) const ( PAD_CFG_DW0_RO_FIELDS = (0x1 << 27) | (0x1 << 24) | (0x3 << 21) | (0xf << 16) | 0xfc diff --git a/util/intelp2m/platforms/cnl/macro.go b/util/intelp2m/platforms/cnl/macro.go index c3bdfc8..714bfa1 100644 --- a/util/intelp2m/platforms/cnl/macro.go +++ b/util/intelp2m/platforms/cnl/macro.go @@ -1,13 +1,13 @@ package cnl -import "strings" -import "fmt" - -// Local packages -import "../common" -import "../../config" -import "../../fields" -import "../snr" +import ( + "strings" + "fmt" + "../common" + "../../config" + "../../fields" + "../snr" +) const ( PAD_CFG_DW0_RO_FIELDS = (0x1 << 27) | (0x1 << 24) | (0x3 << 21) | (0xf << 16) | 0xfc diff --git a/util/intelp2m/platforms/common/macro.go b/util/intelp2m/platforms/common/macro.go index 38937f6..ed42729 100644 --- a/util/intelp2m/platforms/common/macro.go +++ b/util/intelp2m/platforms/common/macro.go @@ -1,9 +1,10 @@ package common -import "strconv" -import "sync" - -import "../../config" +import ( + "strconv" + "sync" + "../../config" +) type Fields interface { DecodeDW0() diff --git a/util/intelp2m/platforms/lbg/macro.go b/util/intelp2m/platforms/lbg/macro.go index 6b44a25..003ea3f 100644 --- a/util/intelp2m/platforms/lbg/macro.go +++ b/util/intelp2m/platforms/lbg/macro.go @@ -1,12 +1,12 @@ package lbg -import "fmt" - -// Local packages -import "../../config" -import "../../fields" -import "../common" -import "../snr" +import ( + "fmt" + "../../config" + "../../fields" + "../common" + "../snr" +) const ( PAD_CFG_DW0_RO_FIELDS = (0x1 << 27) | (0x1 << 24) | (0x3 << 21) | (0xf << 16) | 0xfc diff --git a/util/intelp2m/platforms/snr/macro.go b/util/intelp2m/platforms/snr/macro.go index fb530c5..cebb0cd 100644 --- a/util/intelp2m/platforms/snr/macro.go +++ b/util/intelp2m/platforms/snr/macro.go @@ -1,12 +1,12 @@ package snr -import "strings" -import "fmt" - -// Local packages -import "../common" -import "../../config" -import "../../fields" +import ( + "strings" + "fmt" + "../common" + "../../config" + "../../fields" +) const ( PAD_CFG_DW0_RO_FIELDS = (0x1 << 27) | (0x1 << 24) | (0x3 << 21) | (0xf << 16) | 0xfc -- To view, visit https://review.coreboot.org/c/coreboot/+/55985 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibb24fafd409b31b174946a39ca1f810d59b87e76 Gerrit-Change-Number: 55985 Gerrit-PatchSet: 3 Gerrit-Owner: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/intel/alderlake: Add USB TCSS enablement
by Patrick Georgi (Code Review) 02 Jul '21

02 Jul '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55361 ) Change subject: soc/intel/alderlake: Add USB TCSS enablement ...................................................................... soc/intel/alderlake: Add USB TCSS enablement In order to detect USB Type C device port as Super Speed, we need to set corresponding bit in UPD UsbTcPortEn. This patch will use device path to determine which port should be enabled. BUG=b:184324979 Test=Boot board, USB Type C must be functional and operate at Super Speed. Signed-off-by: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Change-Id: I7da63f21d51889a888699540f780cb26b480c26d Reviewed-on: https://review.coreboot.org/c/coreboot/+/55361 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/soc/intel/alderlake/fsp_params.c 1 file changed, 14 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved Sridhar Siricilla: Looks good to me, but someone else must approve diff --git a/src/soc/intel/alderlake/fsp_params.c b/src/soc/intel/alderlake/fsp_params.c index 783d2a9..e29133e 100644 --- a/src/soc/intel/alderlake/fsp_params.c +++ b/src/soc/intel/alderlake/fsp_params.c @@ -304,6 +304,20 @@ /* D3Hot and D3Cold for TCSS */ s_cfg->D3HotEnable = !config->TcssD3HotDisable; s_cfg->D3ColdEnable = !config->TcssD3ColdDisable; + + s_cfg->UsbTcPortEn = 0; + for (int i = 0; i < MAX_TYPE_C_PORTS; i++) { + /* TCSS xHCI --> Root Hub --> Type-C Port */ + const struct device_path port_path[] = { + {.type = DEVICE_PATH_PCI, .pci.devfn = SA_DEVFN_TCSS_XHCI}, + {.type = DEVICE_PATH_USB, .usb.port_type = 0, .usb.port_id = 0}, + {.type = DEVICE_PATH_USB, .usb.port_type = 3, .usb.port_id = i} }; + const struct device *port = find_dev_nested_path(pci_root_bus(), port_path, + ARRAY_SIZE(port_path)); + + if (is_dev_enabled(port)) + s_cfg->UsbTcPortEn |= BIT(i); + } } static void fill_fsps_chipset_lockdown_params(FSP_S_CONFIG *s_cfg, -- To view, visit https://review.coreboot.org/c/coreboot/+/55361 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7da63f21d51889a888699540f780cb26b480c26d Gerrit-Change-Number: 55361 Gerrit-PatchSet: 11 Gerrit-Owner: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Sanrio Alvares <sanrio.alvares(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: arch/x86: Save resume vector to stack in x86_64 mode
by Arthur Heymans (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Patrick Rudolph, Iru Cai, Iru Cai (vimacs). Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55947 ) Change subject: arch/x86: Save resume vector to stack in x86_64 mode ...................................................................... Patch Set 2: (1 comment) File src/arch/x86/wakeup.S: https://review.coreboot.org/c/coreboot/+/55947/comment/672963f3_533b663a PS2, Line 23: mov > In Long Mode the default data size is still 32bits and edi is a 32bit operant, so it likely does the right thing already. Done. -- To view, visit https://review.coreboot.org/c/coreboot/+/55947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I45f2678071b2511c0af5dce9d9b73ac70dfd7252 Gerrit-Change-Number: 55947 Gerrit-PatchSet: 2 Gerrit-Owner: Iru Cai (vimacs) <mytbk920423(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Iru Cai <mytbk920423(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Iru Cai <mytbk920423(a)gmail.com> Gerrit-Attention: Iru Cai (vimacs) <mytbk920423(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Jul 2021 07:06:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede/var/sasukette: Configure I2C times for touchpad
by Paul Menzel (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Henry Sun, Tao Xia, Karthik Ramasubramanian. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56016 ) Change subject: mb/google/dedede/var/sasukette: Configure I2C times for touchpad ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/56016 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibe1603a48a3e841b6a50aa0c703697ec615b2854 Gerrit-Change-Number: 56016 Gerrit-PatchSet: 1 Gerrit-Owner: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Rui Huang <huangrui(a)huaqin.corp-partner.google.com> Gerrit-CC: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com> Gerrit-CC: zanxi chen <chenzanxi(a)huaqin.corp-partner.google.com> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 02 Jul 2021 06:25:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: arch/x86: Save resume vector to stack in x86_64 mode
by Paul Menzel (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Arthur Heymans, Iru Cai, Iru Cai (vimacs). Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55947 ) Change subject: arch/x86: Save resume vector to stack in x86_64 mode ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/55947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I45f2678071b2511c0af5dce9d9b73ac70dfd7252 Gerrit-Change-Number: 55947 Gerrit-PatchSet: 2 Gerrit-Owner: Iru Cai (vimacs) <mytbk920423(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Iru Cai <mytbk920423(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Iru Cai <mytbk920423(a)gmail.com> Gerrit-Attention: Iru Cai (vimacs) <mytbk920423(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Jul 2021 06:25:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP]Documentation: Improve x86_64
by Paul Menzel (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Patrick Rudolph. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56017 ) Change subject: [WIP]Documentation: Improve x86_64 ...................................................................... Patch Set 1: (11 comments) Patchset: PS1: Thank you for documenting this. File Documentation/arch/x86/x86_64.md: https://review.coreboot.org/c/coreboot/+/56017/comment/9420abcb_632522b9 PS1, Line 4: build built https://review.coreboot.org/c/coreboot/+/56017/comment/9aa3e343_eafe62a6 PS1, Line 5: UEFIs UEFI’s https://review.coreboot.org/c/coreboot/+/56017/comment/63e51bb4_72204805 PS1, Line 5: only some only runs(?) some https://review.coreboot.org/c/coreboot/+/56017/comment/34f26b49_53b19037 PS1, Line 5: UEFIs implementation Is that the specification? If not: > some UEFI implementations https://review.coreboot.org/c/coreboot/+/56017/comment/12d4c3d9_884c6fbd PS1, Line 9: following below https://review.coreboot.org/c/coreboot/+/56017/comment/f386b2d8_3a178f9c PS1, Line 13: * Intel Sandy Bridge boards Maybe add the `git grep` command to find all boards, or mention the Kconfig symbol? https://review.coreboot.org/c/coreboot/+/56017/comment/e347423a_afdb846a PS1, Line 22: * The BIOS region is large enough to contain the page tables “BIOS region” refers to Intel hardware, right? https://review.coreboot.org/c/coreboot/+/56017/comment/0b969b5f_d61bd216 PS1, Line 48: contains contain https://review.coreboot.org/c/coreboot/+/56017/comment/66a4d238_8a277ee8 PS1, Line 58: It cannot use 32bit addressing as the memory model Please add a dot/period at the end of sentences. https://review.coreboot.org/c/coreboot/+/56017/comment/b312d193_c53910fb PS1, Line 62: - This requires a complete memory map to be present in postcar stage Ditto. -- To view, visit https://review.coreboot.org/c/coreboot/+/56017 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia5ba51be629a8c878aad64d3297176457cf8e855 Gerrit-Change-Number: 56017 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 02 Jul 2021 06:24:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/brya0: Enable Crashlog
by EricR Lai (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Shobhit Srivastava, Tim Wawrzynczak, Azhar Shaikh. EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55326 ) Change subject: mb/google/brya/brya0: Enable Crashlog ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55326 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I936e73e808e0a05e8b7822cddbb5ee3fa7dee13e Gerrit-Change-Number: 55326 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Azhar Shaikh <azhar.shaikh(a)intel.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Shobhit Srivastava <shobhit.srivastava(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shobhit Srivastava <shobhit.srivastava(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Azhar Shaikh <azhar.shaikh(a)intel.com> Gerrit-Comment-Date: Fri, 02 Jul 2021 05:54:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/adlrvp: Fix DDR5 Boot issue
by Meera Ravindranath (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Bora Guvendik, Furquan Shaikh, Selma Bensaid, Maulik V Vaghela, Paul Menzel, Angel Pons, Subrata Banik. Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50996 ) Change subject: mb/adlrvp: Fix DDR5 Boot issue ...................................................................... Patch Set 13: (13 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/50996/comment/d7719f84_582a4508 PS4, Line 7: Boot > boot Ack https://review.coreboot.org/c/coreboot/+/50996/comment/3b103f9a_dfe432e2 PS4, Line 12: implemention > implementation Ack Commit Message: https://review.coreboot.org/c/coreboot/+/50996/comment/d4caf477_05e02d0a PS6, Line 7: Boot > boot Ack https://review.coreboot.org/c/coreboot/+/50996/comment/b79e2f6f_6349bb51 PS6, Line 7: mb/adlrvp > mb/intel/adlrvp Ack https://review.coreboot.org/c/coreboot/+/50996/comment/d1c2554b_3087b357 PS6, Line 7: Fix DDR5 Boot issue > Pass DIMM slave address to FSP to fix DDR5 boot Ack https://review.coreboot.org/c/coreboot/+/50996/comment/d79286c3_ad55eed0 PS6, Line 9: DDR5 boot is broken due to reading of spd data : via SMBus through coreboot > Since the beginning, or did some commit introduce the regression. Ack https://review.coreboot.org/c/coreboot/+/50996/comment/8f8df221_148ed1d5 PS6, Line 9: Currently, DDR5 boot is broken due to reading of spd data : via SMBus through coreboot. This implementation requires study : of spd5 architecture and is currently Work In Progress. > Please note, what problem is seen, like: […] Ack https://review.coreboot.org/c/coreboot/+/50996/comment/d7456ac5_0f69b3b6 PS6, Line 12: implemention > implementation Ack File src/mainboard/intel/adlrvp/romstage_fsp_params.c: https://review.coreboot.org/c/coreboot/+/50996/comment/cf5fa065_b5705e74 PS6, Line 34: int > unsigned int Ack https://review.coreboot.org/c/coreboot/+/50996/comment/821f3fc3_a50cbaa8 PS6, Line 61: CONFIG_MRC_CHANNEL_WIDTH ; > Please remove the space before the ;. Ack File src/mainboard/intel/adlrvp/romstage_fsp_params.c: https://review.coreboot.org/c/coreboot/+/50996/comment/b3781482_cad29fb3 PS8, Line 63: mupd->FspmConfig.SpdAddressTable[i] = spd_array[i]; > I just noticed that the `phys_to_mrc_map` array for DDR5 looks odd. […] Ack File src/mainboard/intel/adlrvp/romstage_fsp_params.c: https://review.coreboot.org/c/coreboot/+/50996/comment/dccd771b_652fa9b6 PS9, Line 58: &mupd->FspmConfig > check this CL https://review.coreboot. […] Ack File src/mainboard/intel/adlrvp/romstage_fsp_params.c: https://review.coreboot.org/c/coreboot/+/50996/comment/00b27dcd_e1940ad0 PS10, Line 58: half_populated > No Subrata, since we are passing the address at the exact array element where the DIMM is to be foun […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/50996 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9998bfcd12b81c11fcc9f791da2a27d3c788e48a Gerrit-Change-Number: 50996 Gerrit-PatchSet: 13 Gerrit-Owner: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Comment-Date: Fri, 02 Jul 2021 05:43:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP]Documentation: Improve x86_64
by Patrick Rudolph (Code Review) 02 Jul '21

02 Jul '21

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56017 ) Change subject: [WIP]Documentation: Improve x86_64 ...................................................................... [WIP]Documentation: Improve x86_64 Change-Id: Ia5ba51be629a8c878aad64d3297176457cf8e855 Signed-off-by: Patrick Rudolph <siro(a)das-labor.org> --- A Documentation/arch/x86/x86_64.md 1 file changed, 72 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/56017/1 diff --git a/Documentation/arch/x86/x86_64.md b/Documentation/arch/x86/x86_64.md new file mode 100644 index 0000000..021ad7c --- /dev/null +++ b/Documentation/arch/x86/x86_64.md @@ -0,0 +1,72 @@ +# x86_64 architecture documentation + +This section contains documentation about coreboot's experimental +x86_64 support. In coreboot every stage (when enabled) is build for +x86_64, contrary to UEFIs implementation that only some stages in x86_64. +On UEFI the PEI phase, which is x86_32, brings up DRAM and installs +page tables for the x86_64 DXE and BDS phases. + +The differences can be explained as following: + +## Supported boards +* QEMU x86 boards +* Intel Sandy Bridge boards + +## Reference implementation +The reference implementation is +* [QEMU i440fx](../../mainboard/emulation/qemu-i440fx.md) +* [QEMU Q35](../../mainboard/emulation/qemu-q35.md) + +## Hardware requirements for x86_64 support +* The CPU supports long mode (x86_64) +* The BIOS region is large enough to contain the page tables + (24 KiB minimum) besides the coreboot code itself. +* The BIOS region of the SPI flash is memory mapped at reset + +## Current software constrains for x86_64 support +* 0-4GiB are identity mapped using 2MiB-pages as WB +* Memory above 4GiB isn't accessible +* page tables reside in memory mapped ROM +* x86 payloads are loaded below 4GiB in physical memory and are jumped + to in *protected mode* +* The high dword of pointers is always zero +* All memory returned by malloc must be below 4GiB in physical memory +* All code that is to be run must be below 4GiB in physical memory +* The compiler supports generating code for the *large memory model* + (-mcmodel=large). + +## Page tables +Page tables are generated by a tool in `util/pgtblgen/pgtblgen`. It writes +the page tables to a file which is then included into the CBFS as file called +`pagetables`. + +To generate the static page tables it must know the physical address where to +place the file. The pagetable must be placed in flash that is memory mapped at +reset and thus can be read in bootblock's assembly code without additional +setup. + +The page tables contains the following structure: +* PML4E pointing to PDPE +* PDPE with *$n* entries each pointing to PDE +* *$n* PDEs with 512 entries each + +At the moment *$n* is 4, which results in identity mapping the lower 4 GiB. + +## Pros / Cons + +* x86_64 code is bigger than the x86_32 code as it uses 64bit absolute addressing. + It cannot use 32bit addressing as the memory model + +## TODO +1. Identity map memory above 4GiB in ramstage + - This requires a complete memory map to be present in postcar stage +2. Fine grained page tables for SMM: + * Must not have execute and write permissions for the same page. + * Must allow only that TSEG pages can be marked executable + * Must reside in SMRAM +3. Support 64bit PCI BARs above 4GiB +4. Place and run code above 4GiB + +## Porting other boards +* Fix compilation errors +* Test libgfxinit / VGA Option ROMs / FSP -- To view, visit https://review.coreboot.org/c/coreboot/+/56017 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia5ba51be629a8c878aad64d3297176457cf8e855 Gerrit-Change-Number: 56017 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: arch/x86: Save resume vector to stack in x86_64 mode
by Patrick Rudolph (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Arthur Heymans, Iru Cai, Iru Cai (vimacs). Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55947 ) Change subject: arch/x86: Save resume vector to stack in x86_64 mode ...................................................................... Patch Set 2: (1 comment) File src/arch/x86/wakeup.S: https://review.coreboot.org/c/coreboot/+/55947/comment/96150844_550b423a PS2, Line 23: mov > Explicitly use movl to make sure only 4 bytes get moved? In Long Mode the default data size is still 32bits and edi is a 32bit operant, so it likely does the right thing already. -- To view, visit https://review.coreboot.org/c/coreboot/+/55947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I45f2678071b2511c0af5dce9d9b73ac70dfd7252 Gerrit-Change-Number: 55947 Gerrit-PatchSet: 2 Gerrit-Owner: Iru Cai (vimacs) <mytbk920423(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Iru Cai <mytbk920423(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Iru Cai <mytbk920423(a)gmail.com> Gerrit-Attention: Iru Cai (vimacs) <mytbk920423(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Jul 2021 05:04:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021