coreboot-gerrit February 2021

coreboot-gerrit@coreboot.org

1 participants
7192 discussions

Change in coreboot[master]: Documentation: Add chain-include list of headers
by Julius Werner (Code Review) 04 Feb '21

04 Feb '21

Attention is currently required from: HAOUAS Elyes. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50286 ) Change subject: Documentation: Add chain-include list of headers ...................................................................... Patch Set 1: (1 comment) File Documentation/chain-include.md: https://review.coreboot.org/c/coreboot/+/50286/comment/55f9572d_2b8fbdb4 PS1, Line 33: Also: fmap.h -> commonlib/bsd/fmap_serialized.h rmodule.h -> commonlib/rmodule-defs.h console/console.h -> commonlib/loglevel.h cbmem.h -> commonlib/cbmem_id.h metadata_hash.h -> commonlib/bsd/metadata_hash.h cbfs_private.h -> commonlib/bsd/cbfs_private.h timestamp.h -> commonlib/timestamp_serialized.h boot/coreboot_tables.h -> commonlib/coreboot_tables.h stddef.h -> commonlib/helpers.h -> commonlib/bsd/helpers.h memlayout.h -> arch/memlayout.h cpu/cpu.h -> arch/cpu.h smp/atomic.h -> arch/smp/atomic.h smp/spinlock.h -> arch/smp/spinlock.h -- To view, visit https://review.coreboot.org/c/coreboot/+/50286 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I474d0d4bd660b62c24508bc3eba67154e820d8a4 Gerrit-Change-Number: 50286 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Comment-Date: Fri, 05 Feb 2021 00:21:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/stoneyridge: remove STONEYRIDGE_ACPI_IO_BASE Kconfig option
by Felix Held (Code Review) 04 Feb '21

04 Feb '21

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/50292 ) Change subject: soc/amd/stoneyridge: remove STONEYRIDGE_ACPI_IO_BASE Kconfig option ...................................................................... soc/amd/stoneyridge: remove STONEYRIDGE_ACPI_IO_BASE Kconfig option No board in tree selects a different base address, so this can be removed from Kconfig and be treated like the other base addresses in the I/O space that are defines in iomap.h. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Iec3d4476e3a6a5d2b226edef4c41f503a0c81f33 --- M src/soc/amd/stoneyridge/Kconfig M src/soc/amd/stoneyridge/include/soc/iomap.h 2 files changed, 1 insertion(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/50292/1 diff --git a/src/soc/amd/stoneyridge/Kconfig b/src/soc/amd/stoneyridge/Kconfig index baf3f4e..b400e1b 100644 --- a/src/soc/amd/stoneyridge/Kconfig +++ b/src/soc/amd/stoneyridge/Kconfig @@ -250,13 +250,6 @@ Set this option to y for serial IRQ in continuous mode. Otherwise it is in quiet mode. -config STONEYRIDGE_ACPI_IO_BASE - hex - default 0x400 - help - Base address for the ACPI registers. - This value must match the hardcoded value of AGESA. - config CONSOLE_UART_BASE_ADDRESS depends on CONSOLE_SERIAL hex diff --git a/src/soc/amd/stoneyridge/include/soc/iomap.h b/src/soc/amd/stoneyridge/include/soc/iomap.h index e532f18..32618e2 100644 --- a/src/soc/amd/stoneyridge/include/soc/iomap.h +++ b/src/soc/amd/stoneyridge/include/soc/iomap.h @@ -27,7 +27,7 @@ /* I/O Ranges */ #define ACPI_SMI_CTL_PORT 0xb2 -#define ACPI_IO_BASE CONFIG_STONEYRIDGE_ACPI_IO_BASE +#define ACPI_IO_BASE 0x400 #define ACPI_PM_EVT_BLK (ACPI_IO_BASE + 0x00) /* 4 bytes */ #define ACPI_PM1_STS (ACPI_PM_EVT_BLK + 0x00) /* 2 bytes */ #define ACPI_PM1_EN (ACPI_PM_EVT_BLK + 0x02) /* 2 bytes */ -- To view, visit https://review.coreboot.org/c/coreboot/+/50292 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec3d4476e3a6a5d2b226edef4c41f503a0c81f33 Gerrit-Change-Number: 50292 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/stoneyridge: remove STONEYRIDGE_ prefix of ACPI_IO_BASE define
by Felix Held (Code Review) 04 Feb '21

04 Feb '21

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/50291 ) Change subject: soc/amd/stoneyridge: remove STONEYRIDGE_ prefix of ACPI_IO_BASE define ...................................................................... soc/amd/stoneyridge: remove STONEYRIDGE_ prefix of ACPI_IO_BASE define Since I'm not sure if there are non-upstream boards that change the default of the Kconfig value and the comment says that it needs to match the binaryPI build, I'll do that change in a follow-up patch to allow easy local reverts of that. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ic0f08c6cb951994be6db19e10f73f0c621521c70 --- M src/soc/amd/stoneyridge/acpi.c M src/soc/amd/stoneyridge/include/soc/iomap.h 2 files changed, 11 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/50291/1 diff --git a/src/soc/amd/stoneyridge/acpi.c b/src/soc/amd/stoneyridge/acpi.c index ae2de67..af3bf12 100644 --- a/src/soc/amd/stoneyridge/acpi.c +++ b/src/soc/amd/stoneyridge/acpi.c @@ -59,7 +59,7 @@ */ void acpi_fill_fadt(acpi_fadt_t *fadt) { - printk(BIOS_DEBUG, "pm_base: 0x%04x\n", STONEYRIDGE_ACPI_IO_BASE); + printk(BIOS_DEBUG, "pm_base: 0x%04x\n", ACPI_IO_BASE); fadt->sci_int = 9; /* IRQ 09 - ACPI SCI */ diff --git a/src/soc/amd/stoneyridge/include/soc/iomap.h b/src/soc/amd/stoneyridge/include/soc/iomap.h index 350618f..e532f18 100644 --- a/src/soc/amd/stoneyridge/include/soc/iomap.h +++ b/src/soc/amd/stoneyridge/include/soc/iomap.h @@ -27,16 +27,16 @@ /* I/O Ranges */ #define ACPI_SMI_CTL_PORT 0xb2 -#define STONEYRIDGE_ACPI_IO_BASE CONFIG_STONEYRIDGE_ACPI_IO_BASE -#define ACPI_PM_EVT_BLK (STONEYRIDGE_ACPI_IO_BASE + 0x00) /* 4 bytes */ -#define ACPI_PM1_STS (ACPI_PM_EVT_BLK + 0x00) /* 2 bytes */ -#define ACPI_PM1_EN (ACPI_PM_EVT_BLK + 0x02) /* 2 bytes */ -#define ACPI_PM1_CNT_BLK (STONEYRIDGE_ACPI_IO_BASE + 0x04) /* 2 bytes */ -#define ACPI_CPU_CONTROL (STONEYRIDGE_ACPI_IO_BASE + 0x08) /* 6 bytes */ -#define ACPI_GPE0_BLK (STONEYRIDGE_ACPI_IO_BASE + 0x10) /* 8 bytes */ -#define ACPI_GPE0_STS (ACPI_GPE0_BLK + 0x00) /* 4 bytes */ -#define ACPI_GPE0_EN (ACPI_GPE0_BLK + 0x04) /* 4 bytes */ -#define ACPI_PM_TMR_BLK (STONEYRIDGE_ACPI_IO_BASE + 0x18) /* 4 bytes */ +#define ACPI_IO_BASE CONFIG_STONEYRIDGE_ACPI_IO_BASE +#define ACPI_PM_EVT_BLK (ACPI_IO_BASE + 0x00) /* 4 bytes */ +#define ACPI_PM1_STS (ACPI_PM_EVT_BLK + 0x00) /* 2 bytes */ +#define ACPI_PM1_EN (ACPI_PM_EVT_BLK + 0x02) /* 2 bytes */ +#define ACPI_PM1_CNT_BLK (ACPI_IO_BASE + 0x04) /* 2 bytes */ +#define ACPI_CPU_CONTROL (ACPI_IO_BASE + 0x08) /* 6 bytes */ +#define ACPI_GPE0_BLK (ACPI_IO_BASE + 0x10) /* 8 bytes */ +#define ACPI_GPE0_STS (ACPI_GPE0_BLK + 0x00) /* 4 bytes */ +#define ACPI_GPE0_EN (ACPI_GPE0_BLK + 0x04) /* 4 bytes */ +#define ACPI_PM_TMR_BLK (ACPI_IO_BASE + 0x18) /* 4 bytes */ #define SMB_BASE_ADDR 0xb00 #define PM2_INDEX 0xcd0 #define PM2_DATA 0xcd1 -- To view, visit https://review.coreboot.org/c/coreboot/+/50291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic0f08c6cb951994be6db19e10f73f0c621521c70 Gerrit-Change-Number: 50291 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates
by build bot (Jenkins) (Code Review) 04 Feb '21

04 Feb '21

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49766 ) Change subject: soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates ...................................................................... Patch Set 12: (8 comments) File src/soc/intel/tigerlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/49766/comment/9ed34929_4f2e2671 PS12, Line 60: if((cpu_id == CPUID_TIGERLAKE_A0) || (cpu_id ==CPUID_TIGERLAKE_B0)) that open brace { should be on the previous line https://review.coreboot.org/c/coreboot/+/49766/comment/fe0585c0_3f039559 PS12, Line 60: if((cpu_id == CPUID_TIGERLAKE_A0) || (cpu_id ==CPUID_TIGERLAKE_B0)) spaces required around that '==' (ctx:WxV) https://review.coreboot.org/c/coreboot/+/49766/comment/7cc273d9_65b0e220 PS12, Line 60: if((cpu_id == CPUID_TIGERLAKE_A0) || (cpu_id ==CPUID_TIGERLAKE_B0)) space required before the open parenthesis '(' https://review.coreboot.org/c/coreboot/+/49766/comment/ac44255d_f99032fe PS12, Line 62: if((mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_2_2) || (mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_4_2)) line over 96 characters https://review.coreboot.org/c/coreboot/+/49766/comment/39902f84_6cd5777a PS12, Line 62: if((mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_2_2) || (mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_4_2)) that open brace { should be on the previous line https://review.coreboot.org/c/coreboot/+/49766/comment/69281857_7453ca18 PS12, Line 62: if((mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_2_2) || (mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_4_2)) space required before the open parenthesis '(' https://review.coreboot.org/c/coreboot/+/49766/comment/3dc47cdb_8ccd0719 PS12, Line 89: /* If external phy gating is not implemented, S0i3.3/S0i3.4/S0i2.2 are not recommended. */ line over 96 characters https://review.coreboot.org/c/coreboot/+/49766/comment/f18a94c7_30823ab3 PS12, Line 94: if (is_dev_enabled(pcidev_path_on_root(PCH_DEVFN_CNVI_BT)) || is_dev_enabled(pcidev_path_on_root(PCH_DEVFN_CNVI_WIFI)) || is_dev_enabled(pcidev_path_on_root(PCH_DEVFN_ISH))) line over 96 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/49766 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9 Gerrit-Change-Number: 49766 Gerrit-PatchSet: 12 Gerrit-Owner: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com> Gerrit-Reviewer: Alex Levin <levinale(a)google.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 05 Feb 2021 00:00:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates
by Shreesh Chhabbi (Code Review) 04 Feb '21

04 Feb '21

Attention is currently required from: Shreesh Chhabbi. Hello build bot (Jenkins), Furquan Shaikh, Ravishankar Sarawadi, Duncan Laurie, Alex Levin, Duncan Laurie, Sukumar Ghorai, Raj Astekar, Patrick Rudolph, Shreesh Chhabbi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/49766 to look at the new patch set (#12). Change subject: soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates ...................................................................... soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates S0i3.2 and S0i3.3 are applicable only if wake on voice is disabled. As per Platform Design Guide, S0i3.2 and S0i3.3 substates need to be disabled for Tigerlake. Signed-off-by: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9 --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 72 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/49766/12 -- To view, visit https://review.coreboot.org/c/coreboot/+/49766 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9 Gerrit-Change-Number: 49766 Gerrit-PatchSet: 12 Gerrit-Owner: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com> Gerrit-Reviewer: Alex Levin <levinale(a)google.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by Felix Held (Code Review) 04 Feb '21

04 Feb '21

Attention is currently required from: Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Andrey Petrov, Patrick Rudolph, Felix Held. Hello Justin Frodsham, build bot (Jenkins), Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Subrata Banik, Andrey Petrov, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/50241 to look at the new patch set (#4). Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size If the UPD size in coreboot sizes mismatches the one from the FSP-M binary, we're running into trouble. If the expected size is smaller than the UPD size the FSP provides, call die(), since the target buffer isn't large enough so only the beginning of the UPD defaults from the FSP will get copied into the buffer. We ran into the issue in soc/amd/cezanne, where the UPD struct in coreboot was smaller than the one in the FSP, so the defaults didn't get completely copied. TEST=Mandolin still boots. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d --- M src/drivers/intel/fsp2_0/memory_init.c 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/50241/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by Felix Held (Code Review) 04 Feb '21

04 Feb '21

Attention is currently required from: Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Andrey Petrov, Patrick Rudolph, Felix Held. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50241 ) Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... Patch Set 3: (1 comment) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/50241/comment/71f4349f_a52a88d1 PS1, Line 242: != > After looking at the code again, yes you're right. […] i split that into the two cases and added a comment explaining the two cases and only call die() in the case where we're sure that things will be broken -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 04 Feb 2021 23:39:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by build bot (Jenkins) (Code Review) 04 Feb '21

04 Feb '21

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by Felix Held (Code Review) 04 Feb '21

04 Feb '21

Attention is currently required from: Furquan Shaikh, Meera Ravindranath, Andrey Petrov, Patrick Rudolph, Felix Held. Hello Justin Frodsham, build bot (Jenkins), Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Subrata Banik, Andrey Petrov, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/50241 to look at the new patch set (#3). Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size If the UPD size in coreboot sizes mismatches the one from the FSP-M binary, call die(). We ran into the issue in soc/amd/cezanne, where the UPD struct in coreboot was smaller than the one in the FSP, so the defaults didn't get completely copied. TEST=Mandolin still boots. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d --- M src/drivers/intel/fsp2_0/memory_init.c 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/50241/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by build bot (Jenkins) (Code Review) 04 Feb '21

04 Feb '21

Attention is currently required from: Furquan Shaikh, Meera Ravindranath, Andrey Petrov, Patrick Rudolph. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50241 ) Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... Patch Set 2: (10 comments) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/50241/comment/93a85176_06498264 PS2, Line 242: /* trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/dc131924_1b8e05da PS2, Line 244: * use a static buffer for the FSP-M UPDs which is sizeof(FSPM_UPD) trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/fa3cf0d2_61c9a60d PS2, Line 245: * byes long, since that is the value known at compile time. If trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/366f8d64_0eac1687 PS2, Line 246: * hdr->cfg_region_size is bigger than that, not all UPD defaults will trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/f69c8675_ff3f3156 PS2, Line 247: * be copied, so it'll contain random data at the end, so we just call trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/7d8f1bf9_c2f23903 PS2, Line 248: * die() in that case. If hdr->cfg_region_size is smaller than that, trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/c90927b6_675785a0 PS2, Line 249: * there's a mismatch between the FSP and the header, but since it trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/525420f3_b7677027 PS2, Line 250: * will copy the full UPD defaults to the buffer, we try to continue trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/abc55497_386b351a PS2, Line 251: * and hope that there wasn't any incompatible change in the UPDs. 'wasn' may be misspelled - perhaps 'was'? https://review.coreboot.org/c/coreboot/+/50241/comment/c4de3840_55b8e7a4 PS2, Line 252: */ trailing whitespace -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 04 Feb 2021 23:36:50 +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 February 2021