coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[M] Change in coreboot[main]: soc/intel/xeon_sp: Initial support for PCI multi segment groups
by Nico Huber (Code Review) 17 Mar '24

17 Mar '24

Attention is currently required from: Angel Pons, Annie Chen, Arthur Heymans, Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Felix Held, Jincheng Li, Johnny Lin, Naresh Solanki, Nill Ge, Patrick Rudolph, Shuo Liu, TangYiwei, Tim Chu. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79878?usp=email ) Change subject: soc/intel/xeon_sp: Initial support for PCI multi segment groups ...................................................................... Patch Set 18: Code-Review+1 (1 comment) File src/soc/intel/xeon_sp/chip_common.c: https://review.coreboot.org/c/coreboot/+/79878/comment/7b10484c_914a62be : PS15, Line 276: > Done Ah, just read above message after I wrote the below. Still going to post it for completeness... Maybe a misunderstanding of `struct device_path`. It doesn't describe a full path but just one element of the path, e.g. on this 'host bus' (expressed by the upstream pointer) this 'domain id' (expressed by struct device_path). We added some more information like the socket into the 'domain id' and later the secondary bus. While that's a convenient way to create unique ids, I don't think we should use that to retrieve information like socket or bus numbers from the domain path element. If such information is needed, we should get it from the device tree (e.g. the secondary bus is `dev->downstream->secondary` and not `dev->path.domain.domain & 0xff`; and the PCI segment group is `dev->downstream->segment_group` as correctly described in this patch). (If there is a need to retrieve the socket number, maybe it is time to introduce a socket node to the devicetree? or if that's too much overhead, make it official and add a `socket` field to `struct domain_path`?) -- To view, visit https://review.coreboot.org/c/coreboot/+/79878?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: I0ba5e426123234979d746d3bdfc1ddfbd71c3447 Gerrit-Change-Number: 79878 Gerrit-PatchSet: 18 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Annie Chen <chen.annieet(a)inventec.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Annie Chen <chen.annieet(a)inventec.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Naresh Solanki <naresh.solanki(a)9elements.com> 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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Sun, 17 Mar 2024 11:57:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/intel/mtl: Enable RAMTOP caching at SoC level for MTL devices
by Subrata Banik (Code Review) 17 Mar '24

17 Mar '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81271?usp=email ) Change subject: soc/intel/mtl: Enable RAMTOP caching at SoC level for MTL devices ...................................................................... soc/intel/mtl: Enable RAMTOP caching at SoC level for MTL devices This patch enables the `SOC_INTEL_COMMON_BASECODE_RAMTOP` configuration at the SoC level for all MTL devices. This change streamlines the configuration process, avoiding redundant selections on individual mainboards. BUG=b:306677879 BRANCH=firmware-rex-15709.B TEST=Verified boot functionality on google/ovis and google/rex. Change-Id: I3aa3a83c190d0a0e93c267222a9dca0ac7651f9c Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81271 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M src/mainboard/google/rex/Kconfig M src/mainboard/intel/mtlrvp/Kconfig M src/soc/intel/meteorlake/Kconfig 3 files changed, 1 insertion(+), 4 deletions(-) Approvals: Stefan Reinauer: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/mainboard/google/rex/Kconfig b/src/mainboard/google/rex/Kconfig index b162651..880680a 100644 --- a/src/mainboard/google/rex/Kconfig +++ b/src/mainboard/google/rex/Kconfig @@ -43,7 +43,6 @@ select RT8168_GEN_ACPI_POWER_RESOURCE select RT8168_GET_MAC_FROM_VPD select RT8168_SET_LED_MODE - select SOC_INTEL_COMMON_BASECODE_RAMTOP select SOC_INTEL_IOE_DIE_SUPPORT select SOC_INTEL_METEORLAKE_U_H select SOC_INTEL_ENABLE_USB4_PCIE_RESOURCES @@ -59,7 +58,6 @@ select HAVE_SLP_S0_GATE select MAINBOARD_HAS_CHROMEOS select MEMORY_SOLDERDOWN - select SOC_INTEL_COMMON_BASECODE_RAMTOP select SOC_INTEL_ENABLE_USB4_PCIE_RESOURCES select SOC_INTEL_IOE_DIE_SUPPORT select SOC_INTEL_METEORLAKE_U_H diff --git a/src/mainboard/intel/mtlrvp/Kconfig b/src/mainboard/intel/mtlrvp/Kconfig index 440466b..36842b5 100644 --- a/src/mainboard/intel/mtlrvp/Kconfig +++ b/src/mainboard/intel/mtlrvp/Kconfig @@ -19,7 +19,6 @@ select HAVE_ACPI_TABLES select HAVE_SPD_IN_CBFS select MAINBOARD_HAS_CHROMEOS - select SOC_INTEL_COMMON_BASECODE_RAMTOP select SOC_INTEL_COMMON_BLOCK_VARIANT_POWER_LIMIT select SOC_INTEL_CSE_LITE_SKU select SOC_INTEL_METEORLAKE_U_H diff --git a/src/soc/intel/meteorlake/Kconfig b/src/soc/intel/meteorlake/Kconfig index 43bb25e..a4ebad4 100644 --- a/src/soc/intel/meteorlake/Kconfig +++ b/src/soc/intel/meteorlake/Kconfig @@ -84,7 +84,7 @@ select SOC_INTEL_COMMON_BLOCK_XHCI select SOC_INTEL_COMMON_BLOCK_XHCI_ELOG select SOC_INTEL_COMMON_BASECODE - select SOC_INTEL_COMMON_BASECODE_RAMTOP if !MAINBOARD_HAS_CHROMEOS + select SOC_INTEL_COMMON_BASECODE_RAMTOP select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_CLIENT select SOC_INTEL_COMMON_RESET -- To view, visit https://review.coreboot.org/c/coreboot/+/81271?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: I3aa3a83c190d0a0e93c267222a9dca0ac7651f9c Gerrit-Change-Number: 81271 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: mb/google/rex: Reland RAMTOP caching for Ovis
by Subrata Banik (Code Review) 17 Mar '24

17 Mar '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81270?usp=email ) Change subject: mb/google/rex: Reland RAMTOP caching for Ovis ...................................................................... mb/google/rex: Reland RAMTOP caching for Ovis This patch ensures Ovis baseboard can select RAMTOP caching to improve the boot time w/o any runtime hang. BUG=b:306677879 BRANCH=firmware-rex-15709.B TEST=Verified boot on google/ovis with ~30ms savings in boot time. Change-Id: Ic0b73eb8fb9cd6ca70d3d7168b79dfd0fbc550e3 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81270 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M src/mainboard/google/rex/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved diff --git a/src/mainboard/google/rex/Kconfig b/src/mainboard/google/rex/Kconfig index 4fc1c32..b162651 100644 --- a/src/mainboard/google/rex/Kconfig +++ b/src/mainboard/google/rex/Kconfig @@ -43,6 +43,7 @@ select RT8168_GEN_ACPI_POWER_RESOURCE select RT8168_GET_MAC_FROM_VPD select RT8168_SET_LED_MODE + select SOC_INTEL_COMMON_BASECODE_RAMTOP select SOC_INTEL_IOE_DIE_SUPPORT select SOC_INTEL_METEORLAKE_U_H select SOC_INTEL_ENABLE_USB4_PCIE_RESOURCES -- To view, visit https://review.coreboot.org/c/coreboot/+/81270?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: Ic0b73eb8fb9cd6ca70d3d7168b79dfd0fbc550e3 Gerrit-Change-Number: 81270 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: soc/intel/cmn/ramtop: Refactor MTRR handling for RAMTOP range
by Subrata Banik (Code Review) 17 Mar '24

17 Mar '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81269?usp=email ) Change subject: soc/intel/cmn/ramtop: Refactor MTRR handling for RAMTOP range ...................................................................... soc/intel/cmn/ramtop: Refactor MTRR handling for RAMTOP range This patch refactors RAMTOP MTRR type selection to address a critical NEM logic bug on SoCs with non-power-of-two cache sets. This bug can cause runtime hangs when Write Back (WB) caching is enabled. Workaround: Force MTRR type to WC (Write Combining) on affected SoCs when the cache set count is not a power of two. BUG=b:306677879 BRANCH=firmware-rex-15709.B TEST=Verified boot on google/ovis and google/rex (including Ovis with non-power-of-two cache configuration). Change-Id: Ia9a8f0d37d581b05c19ea7f9b1a07933caa956d4 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81269 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/intel/common/basecode/ramtop/ramtop.c 1 file changed, 17 insertions(+), 1 deletion(-) Approvals: Stefan Reinauer: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/soc/intel/common/basecode/ramtop/ramtop.c b/src/soc/intel/common/basecode/ramtop/ramtop.c index ec326bb..9cef9b1 100644 --- a/src/soc/intel/common/basecode/ramtop/ramtop.c +++ b/src/soc/intel/common/basecode/ramtop/ramtop.c @@ -2,6 +2,7 @@ #include <commonlib/bsd/ipchksum.h> #include <console/console.h> +#include <cpu/cpu.h> #include <cpu/x86/mtrr.h> #include <intelbasecode/ramtop.h> #include <pc80/mc146818rtc.h> @@ -125,9 +126,24 @@ printk(BIOS_WARNING, "ramtop_table update failure due to no free MTRR available!\n"); return; } + + /* + * Background: Some SoCs have a critical bug inside the NEM logic which is responsible + * for mapping cached memory to physical memory during tear down and + * eventually malfunctions if the number of cache sets is not a power of two. + * This can lead to runtime hangs. + * + * Workaround: To mitigate this issue on affected SoCs, we force the MTRR type to + * WC (Write Combining) unless the cache set count is a power of two. + * This change alters caching behavior but prevents the runtime failures. + */ + unsigned int mtrr_type = MTRR_TYPE_WRCOMB; /* * We need to make sure late romstage (including FSP-M post mem) will be run * cached. Caching 16MB below ramtop is a safe to cover late romstage. */ - set_var_mtrr(mtrr, ramtop - 16 * MiB, 16 * MiB, MTRR_TYPE_WRBACK); + if (is_cache_sets_power_of_two()) + mtrr_type = MTRR_TYPE_WRBACK; + + set_var_mtrr(mtrr, ramtop - 16 * MiB, 16 * MiB, mtrr_type); } -- To view, visit https://review.coreboot.org/c/coreboot/+/81269?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: Ia9a8f0d37d581b05c19ea7f9b1a07933caa956d4 Gerrit-Change-Number: 81269 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: arch/x86: Add API to check if cache sets are power-of-two
by Subrata Banik (Code Review) 17 Mar '24

17 Mar '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81268?usp=email ) Change subject: arch/x86: Add API to check if cache sets are power-of-two ...................................................................... arch/x86: Add API to check if cache sets are power-of-two Introduce a function to determine whether the number of cache sets is a power of two. This aligns with common cache design practices that favor power-of-two counts for efficient indexing and addressing. BUG=b:306677879 BRANCH=firmware-rex-15709.B TEST=Verified functionality on google/ovis and google/rex (including a non-power-of-two Ovis configuration). Change-Id: I819e0d1aeb4c1dbe1cdf3115b2e172588a6e8da5 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81268 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/arch/x86/cpu_common.c M src/arch/x86/include/arch/cpu.h 2 files changed, 25 insertions(+), 0 deletions(-) Approvals: Stefan Reinauer: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/arch/x86/cpu_common.c b/src/arch/x86/cpu_common.c index c4d30a2..ee07214 100644 --- a/src/arch/x86/cpu_common.c +++ b/src/arch/x86/cpu_common.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <commonlib/helpers.h> #include <cpu/cpu.h> #include <types.h> @@ -234,3 +235,18 @@ return true; } + +bool is_cache_sets_power_of_two(void) +{ + struct cpu_cache_info info; + + if (!fill_cpu_cache_info(CACHE_L3, &info)) + return false; + + size_t cache_sets = cpu_get_cache_sets(&info); + + if (IS_POWER_OF_2(cache_sets)) + return true; + + return false; +} diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 0c4decf..fa0d5f4 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -329,6 +329,15 @@ */ bool fill_cpu_cache_info(uint8_t level, struct cpu_cache_info *info); +/* + * Determines whether the number of cache sets is a power of two. + * + * Cache designs often favor power-of-two set counts for efficient indexing + * and addressing. This function checks if the provided cache configuration + * adheres to this practice. + */ +bool is_cache_sets_power_of_two(void); + #if CONFIG(RESERVED_PHYSICAL_ADDRESS_BITS_SUPPORT) unsigned int get_reserved_phys_addr_bits(void); #else -- To view, visit https://review.coreboot.org/c/coreboot/+/81268?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: I819e0d1aeb4c1dbe1cdf3115b2e172588a6e8da5 Gerrit-Change-Number: 81268 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: soc/intel/xeon_sp: Initial support for PCI multi segment groups
by Patrick Rudolph (Code Review) 17 Mar '24

17 Mar '24

Attention is currently required from: Angel Pons, Annie Chen, Arthur Heymans, Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Felix Held, Jincheng Li, Johnny Lin, Naresh Solanki, Nico Huber, Nill Ge, Patrick Rudolph, Shuo Liu, TangYiwei, Tim Chu. Hello Angel Pons, Annie Chen, Arthur Heymans, Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Felix Held, Jincheng Li, Johnny Lin, Lean Sheng Tan, Naresh Solanki, Nico Huber, Nill Ge, Shuo Liu, TangYiwei, Tim Chu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79878?usp=email to look at the new patch set (#18). The following approvals got outdated and were removed: Code-Review+1 by Shuo Liu Change subject: soc/intel/xeon_sp: Initial support for PCI multi segment groups ...................................................................... soc/intel/xeon_sp: Initial support for PCI multi segment groups Add PCI enumeration support by reading the PCIeSegment reported in the FSP HOB and add it when creating the PCI domain for each stack. The PCI enumeration will be able to scan the additional PCI segment groups and properly handle those devices. TEST: Booted on ibm/sbp1 with multiple PCI segment groups enabled to ubuntu 22.04. Change-Id: I0ba5e426123234979d746d3bdfc1ddfbd71c3447 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M src/soc/intel/xeon_sp/chip_common.c M src/soc/intel/xeon_sp/include/soc/chip_common.h M src/soc/intel/xeon_sp/spr/ioat.c 3 files changed, 36 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/79878/18 -- To view, visit https://review.coreboot.org/c/coreboot/+/79878?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: I0ba5e426123234979d746d3bdfc1ddfbd71c3447 Gerrit-Change-Number: 79878 Gerrit-PatchSet: 18 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Annie Chen <chen.annieet(a)inventec.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Annie Chen <chen.annieet(a)inventec.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Naresh Solanki <naresh.solanki(a)9elements.com> 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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp: Remove function from global scope
by Shuo Liu (Code Review) 17 Mar '24

17 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81298?usp=email ) Change subject: soc/intel/xeon_sp: Remove function from global scope ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Are these the only sites to revise, do we have others? -- To view, visit https://review.coreboot.org/c/coreboot/+/81298?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: Idad0692cb15ee4fbc7e4af10b469790c5300d337 Gerrit-Change-Number: 81298 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.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: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Sun, 17 Mar 2024 11:43:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/intel/xeon_sp: Initial support for PCI multi segment groups
by Shuo Liu (Code Review) 17 Mar '24

17 Mar '24

Attention is currently required from: Angel Pons, Annie Chen, Arthur Heymans, Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Felix Held, Jincheng Li, Johnny Lin, Naresh Solanki, Nico Huber, Nill Ge, Patrick Rudolph, TangYiwei, Tim Chu. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79878?usp=email ) Change subject: soc/intel/xeon_sp: Initial support for PCI multi segment groups ...................................................................... Patch Set 17: (2 comments) File src/soc/intel/xeon_sp/chip_common.c: https://review.coreboot.org/c/coreboot/+/79878/comment/684539f6_764b2709 : PS15, Line 276: > I agree that once the domain objects are created, we should refer to segment id from these device st […] Done https://review.coreboot.org/c/coreboot/+/79878/comment/6cc4e7e5_fb80738f : PS15, Line 391: const size_t seg = hob->PlatformData.CpuQpiInfo[s].PcieSegment; > My question is more around here I suppose, since this is the only site to initially get segment id, […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/79878?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: I0ba5e426123234979d746d3bdfc1ddfbd71c3447 Gerrit-Change-Number: 79878 Gerrit-PatchSet: 17 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Annie Chen <chen.annieet(a)inventec.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Annie Chen <chen.annieet(a)inventec.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Naresh Solanki <naresh.solanki(a)9elements.com> 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: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Sun, 17 Mar 2024 11:42:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/intel/xeon_sp: Initial support for PCI multi segment groups
by Shuo Liu (Code Review) 17 Mar '24

17 Mar '24

Attention is currently required from: Angel Pons, Annie Chen, Arthur Heymans, Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Felix Held, Jincheng Li, Johnny Lin, Naresh Solanki, Nico Huber, Nill Ge, Patrick Rudolph, TangYiwei, Tim Chu. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79878?usp=email ) Change subject: soc/intel/xeon_sp: Initial support for PCI multi segment groups ...................................................................... Patch Set 17: Code-Review+1 (2 comments) File src/soc/intel/xeon_sp/chip_common.c: https://review.coreboot.org/c/coreboot/+/79878/comment/ae05dc76_b97df26b : PS15, Line 276: > Every PCI domain has the PCI segment id set and thus every struct bus will have the correct segment […] I agree that once the domain objects are created, we should refer to segment id from these device structs. https://review.coreboot.org/c/coreboot/+/79878/comment/f65fd096_bf0a1f29 : PS15, Line 391: const size_t seg = hob->PlatformData.CpuQpiInfo[s].PcieSegment; My question is more around here I suppose, since this is the only site to initially get segment id, it should be okay to keep this logic. -- To view, visit https://review.coreboot.org/c/coreboot/+/79878?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: I0ba5e426123234979d746d3bdfc1ddfbd71c3447 Gerrit-Change-Number: 79878 Gerrit-PatchSet: 17 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Annie Chen <chen.annieet(a)inventec.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Annie Chen <chen.annieet(a)inventec.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Naresh Solanki <naresh.solanki(a)9elements.com> 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: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Sun, 17 Mar 2024 11:42:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/intel/fsp2_0: Use DECLARE_REGION for FSP-M heap
by Shuo Liu (Code Review) 17 Mar '24

17 Mar '24

Attention is currently required from: Andrey Petrov, Arthur Heymans, Chen, Gang C, Felix Held, Julius Werner, Jérémy Compostella, Patrick Rudolph, Ronak Kanabar, Subrata Banik, Werner Zeh. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81097?usp=email ) Change subject: drivers/intel/fsp2_0: Use DECLARE_REGION for FSP-M heap ...................................................................... Patch Set 4: (2 comments) Patchset: PS3: Hi Julius, about the usage of DECLARE_REGION, as of now I'm inclined to still keep it inside if (CONFIG(FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND)) otherwise we need to write another #if (CONFIG(FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND)) block to include the definition. PS3: > Actually, I like that you reference the original patch but I don't think that "fixes" is appropriate […] Sure, updated. -- To view, visit https://review.coreboot.org/c/coreboot/+/81097?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: I5f7d7855592d99b074f7ef49c285a13f8105f089 Gerrit-Change-Number: 81097 Gerrit-PatchSet: 4 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sun, 17 Mar 2024 11:36:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024