coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: src/drivers/intel/fsp2_0: allow larger FSP 2.0 header
by Furquan Shaikh (Code Review) 10 Jul '21

10 Jul '21

Attention is currently required from: Martin Roth, Marshall Dawson, Nikolai Vyssotski, Andrey Petrov, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56190 ) Change subject: src/drivers/intel/fsp2_0: allow larger FSP 2.0 header ...................................................................... Patch Set 2: Code-Review-2 (1 comment) File src/drivers/intel/fsp2_0/include/fsp/info_header.h: https://review.coreboot.org/c/coreboot/+/56190/comment/b1cfa114_4da88906 PS2, Line 13: however, some FSP 2.0 variants have it as well. Then it is not conforming to FSP specification: http://www.intel.com/content/dam/www/public/us/en/documents/technical-speci… https://cdrdv2.intel.com/v1/dl/getContent/627153 FspMultiPhaseSiInitEntryOffset was added in FSP v2.2 and v2.0 must not have this field. If you are moving to a newer EDK2 version, then I believe you will have to ensure that your FSP implementation conforms to FSP v2.2. Making this change in coreboot is not correct. It violates the spec. -- To view, visit https://review.coreboot.org/c/coreboot/+/56190 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie8422447b2cff0a6c536e13014905ffa15c70586 Gerrit-Change-Number: 56190 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sat, 10 Jul 2021 00:15:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/drivers/intel/fsp2_0: allow larger FSP 2.0 header
by Nikolai Vyssotski (Code Review) 10 Jul '21

10 Jul '21

Nikolai Vyssotski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56190 ) Change subject: src/drivers/intel/fsp2_0: allow larger FSP 2.0 header ...................................................................... src/drivers/intel/fsp2_0: allow larger FSP 2.0 header This is in preparation for migrating EDK2 to more recent version(s). In EDK2 repo commit f2cdb268ef appended an additional field to FSP 2.0 header (FspMultiPhaseSiInitEntryOffset). This increases the length of the header from 0x48 to 0x4c. This field is not used in coreboot yet. Allow headers of both lengths to be accepted. BUG=b:181766974 TEST=build/boot with both header flavors Signed-off-by: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Change-Id: Ie8422447b2cff0a6c536e13014905ffa15c70586 --- M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/util.c 2 files changed, 12 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/56190/1 diff --git a/src/drivers/intel/fsp2_0/include/fsp/info_header.h b/src/drivers/intel/fsp2_0/include/fsp/info_header.h index e08828a..e23e1d0 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/info_header.h +++ b/src/drivers/intel/fsp2_0/include/fsp/info_header.h @@ -6,11 +6,15 @@ #include <types.h> #define FSP_HDR_OFFSET 0x94 -#if CONFIG(PLATFORM_USES_FSP2_2) -#define FSP_HDR_LEN 0x4c -#else + +/* + * 0x48 is the length of header for FSP 2.0 (per specification). + * 0x4c is the length of header for FSP 2.2, + * however, some FSP 2.0 variants have it as well. + */ +#define FSP_HDR_LEN_EX 0x4c #define FSP_HDR_LEN 0x48 -#endif + #define FSP_HDR_SIGNATURE "FSPH" #define FSP_HDR_ATTRIB_FSPT 1 #define FSP_HDR_ATTRIB_FSPM 2 diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c index e9809e1..24af62b 100644 --- a/src/drivers/intel/fsp2_0/util.c +++ b/src/drivers/intel/fsp2_0/util.c @@ -13,13 +13,15 @@ static bool looks_like_fsp_header(const uint8_t *raw_hdr) { + uint32_t fsp_header_length = read32(raw_hdr + 4); + if (memcmp(raw_hdr, FSP_HDR_SIGNATURE, 4)) { printk(BIOS_ALERT, "Did not find a valid FSP signature\n"); return false; } - if (read32(raw_hdr + 4) != FSP_HDR_LEN) { - printk(BIOS_ALERT, "FSP header has invalid length\n"); + if ((fsp_header_length != FSP_HDR_LEN) && (fsp_header_length != FSP_HDR_LEN_EX)) { + printk(BIOS_ALERT, "FSP header has invalid length: %d\n", fsp_header_length); return false; } -- To view, visit https://review.coreboot.org/c/coreboot/+/56190 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie8422447b2cff0a6c536e13014905ffa15c70586 Gerrit-Change-Number: 56190 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: util/cbfstool: Allow setting alignment for payload
by Julius Werner (Code Review) 10 Jul '21

10 Jul '21

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55973 ) Change subject: util/cbfstool: Allow setting alignment for payload ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Is that new? I was getting something that wasn't 64 byte aligned, hence this CL. Oh wait, sorry, I confused myself about how stuff works again. CBFS file *entries* have a minimum alignment of 64. What you care about is CBFS file *data*, and for that the minimum alignment is 4 (I think?). So you do need -a 64. Sorry. Carry on. -- To view, visit https://review.coreboot.org/c/coreboot/+/55973 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f4aea5f0cbeaa8e761212041099b37f4718ac39 Gerrit-Change-Number: 55973 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 09 Jul 2021 23:37:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87
by Tim Wawrzynczak (Code Review) 10 Jul '21

10 Jul '21

Attention is currently required from: Furquan Shaikh, Subrata Banik, Nick Vaccaro, EricR Lai. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51374 ) Change subject: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87 ...................................................................... Patch Set 9: (2 comments) File src/soc/intel/common/block/cpu/car/cache_as_ram.S: https://review.coreboot.org/c/coreboot/+/51374/comment/1c0b08df_96927244 PS9, Line 528: esi > doesn't `edi` holds the number of data ways here ? Sorry, `esi` holds the non-eviction mask (say, 0b00000111), `edi` holds the total number of ways (say, 0xc). I don't see data_ways (let's say 0x4) stored in any register here... does data_ways needs to be recalculated, e,g.? ``` /* eax (data_ways) = (data size) CONFIG_DCACHE_RAM_SIZE / way size (ecx) */ mov $CONFIG_DCACHE_RAM_SIZE, %eax div %ecx /* backup data_ways to ebx */ mov eax, ebx /* Get SFWayCnt */ mov $IA32_SF_QOS_INFO, %ecx rdmsr and $0x3f, %eax /* eax = eax - ebx (SFWayCnt - data_ways) */ sub %ebx, %eax ... ``` https://review.coreboot.org/c/coreboot/+/51374/comment/7534b227_b3087852 PS9, Line 533: esi > doesn't `edi` holds the number of data ways here ? with the above, `ebx` has data_ways backed up in it -- To view, visit https://review.coreboot.org/c/coreboot/+/51374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd0b7e1a90cad4a4837adf6067fe8301dcd0a941 Gerrit-Change-Number: 51374 Gerrit-PatchSet: 9 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Fri, 09 Jul 2021 23:05:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/cbfstool: Allow setting alignment for payload
by Raul Rangel (Code Review) 10 Jul '21

10 Jul '21

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55973 ) Change subject: util/cbfstool: Allow setting alignment for payload ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > BTW, in case you weren't aware, 64 is the minimum alignment for all CBFS files. […] Is that new? I was getting something that wasn't 64 byte aligned, hence this CL. -- To view, visit https://review.coreboot.org/c/coreboot/+/55973 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f4aea5f0cbeaa8e761212041099b37f4718ac39 Gerrit-Change-Number: 55973 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 09 Jul 2021 22:55:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87
by Tim Wawrzynczak (Code Review) 10 Jul '21

10 Jul '21

Attention is currently required from: Furquan Shaikh, Subrata Banik, Nick Vaccaro, EricR Lai. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51374 ) Change subject: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87 ...................................................................... Patch Set 9: (2 comments) File src/soc/intel/common/block/cpu/car/cache_as_ram.S: https://review.coreboot.org/c/coreboot/+/51374/comment/1783d31e_934f7853 PS9, Line 528: esi doesn't `edi` holds the number of data ways here ? https://review.coreboot.org/c/coreboot/+/51374/comment/b224ae4c_ef3a7ced PS9, Line 533: esi doesn't `edi` holds the number of data ways here ? -- To view, visit https://review.coreboot.org/c/coreboot/+/51374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd0b7e1a90cad4a4837adf6067fe8301dcd0a941 Gerrit-Change-Number: 51374 Gerrit-PatchSet: 9 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Fri, 09 Jul 2021 22:43:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Use SPR for backing up data way and eviction mask
by Tim Wawrzynczak (Code Review) 10 Jul '21

10 Jul '21

Attention is currently required from: Furquan Shaikh, Subrata Banik, Angel Pons, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56161 ) Change subject: soc/intel/common: Use SPR for backing up data way and eviction mask ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: let others look too -- To view, visit https://review.coreboot.org/c/coreboot/+/56161 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I12e0cb7bb050e4f7b17ecf30108db335d1d82ab7 Gerrit-Change-Number: 56161 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 09 Jul 2021 22:43:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: include/cpu/x86/msr: add mca_get_bank_count function
by Tim Wawrzynczak (Code Review) 10 Jul '21

10 Jul '21

Attention is currently required from: Raul Rangel, Marshall Dawson, Felix Held. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56183 ) Change subject: include/cpu/x86/msr: add mca_get_bank_count function ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56183 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id118a900edbe1f67aabcd109d2654c167b6345ea Gerrit-Change-Number: 56183 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 09 Jul 2021 22:41:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/intel/usb4/retimer: Update code to assign correct port number
by Tim Wawrzynczak (Code Review) 10 Jul '21

10 Jul '21

Attention is currently required from: Furquan Shaikh, Maulik V Vaghela. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55945 ) Change subject: drivers/intel/usb4/retimer: Update code to assign correct port number ...................................................................... Patch Set 13: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55945 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib11637ae39046e0afdacd33bc34e8a59e6f2bfb1 Gerrit-Change-Number: 55945 Gerrit-PatchSet: 13 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Abhijeet Rao <abhijeet.rao(a)intel.corp-partner.google.com> Gerrit-CC: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Comment-Date: Fri, 09 Jul 2021 22:40:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/cbfstool: Allow setting alignment for payload
by Julius Werner (Code Review) 10 Jul '21

10 Jul '21

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55973 ) Change subject: util/cbfstool: Allow setting alignment for payload ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: BTW, in case you weren't aware, 64 is the minimum alignment for all CBFS files. If that's all you need, you don't need to pass -a explicitly. -- To view, visit https://review.coreboot.org/c/coreboot/+/55973 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9f4aea5f0cbeaa8e761212041099b37f4718ac39 Gerrit-Change-Number: 55973 Gerrit-PatchSet: 2 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 09 Jul 2021 22:39:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021