coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[S] Change in coreboot[master]: [UNTESTED] vc/google/chromeec/acpi: write OIPG in DECLARE_NO_CROS_GPI...
by Felix Held (Code Review) 13 Apr '23

13 Apr '23

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/74375 ) Change subject: [UNTESTED] vc/google/chromeec/acpi: write OIPG in DECLARE_NO_CROS_GPIOS case ...................................................................... [UNTESTED] vc/google/chromeec/acpi: write OIPG in DECLARE_NO_CROS_GPIOS case When a mainboard selects ACPI_SOC_NVS and CHROMEOS, CHROMEOS_NVS will be selected. This causes vc/google/chromeec/acpi/chromeos.asl to be included in the DSDT and chromeos_acpi_gpio_generate to be called when generating the coreboot SSDT. When a mainboard also uses DECLARE_NO_CROS_GPIOS(), this will cause variant_cros_gpio.count to be 0 and variant_cros_gpio.gpios to be NULL. chromeos_acpi_gpio_generate only checked if the GPIO table was non-NULL, which caused the function to exit early and not generate the OIPG package which causes the kernel to complain about referencing the non-existing OIPG package. To avoid this, only exit in the GPIO table pointer being NULL case if the number of GPIOs is non-0. TEST=None Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ie340003afb718b1454c2da4a479882b71714c3c7 --- M src/vendorcode/google/chromeos/acpi.c 1 file changed, 25 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/74375/1 diff --git a/src/vendorcode/google/chromeos/acpi.c b/src/vendorcode/google/chromeos/acpi.c index 801f33d..fb6d0a5 100644 --- a/src/vendorcode/google/chromeos/acpi.c +++ b/src/vendorcode/google/chromeos/acpi.c @@ -14,7 +14,7 @@ num = variant_cros_gpio.count; gpios = variant_cros_gpio.gpios; - if (!gpios) + if (num && !gpios) return; acpigen_write_scope("\\"); -- To view, visit https://review.coreboot.org/c/coreboot/+/74375 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie340003afb718b1454c2da4a479882b71714c3c7 Gerrit-Change-Number: 74375 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: soc/amd/common: Identify FSP HOB type GUIDs
by Felix Held (Code Review) 13 Apr '23

13 Apr '23

Attention is currently required from: Jason Glenesk, Raul Rangel, Martin L Roth, Matt DeVillier, Fred Reitberger. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74344 ) Change subject: soc/amd/common: Identify FSP HOB type GUIDs ...................................................................... Patch Set 1: (3 comments) Patchset: PS1: haven't looked at the actual guids yet; only at the general code File src/soc/amd/common/fsp/hob_display.c: PS1: i personally would prefer to use the coreboot type guid_t and the macro GUID_INIT instead of the EFI type EFI_GUID. that's what's used in the soc/amd subtree; soc/intel and the FSP driver however use the non-coreboot-native EFI types. either way works and the binary representation ends up being the same, so i don't have a very strong opinion on this https://review.coreboot.org/c/coreboot/+/74344/comment/01e3fa9a_c4505bba PS1, Line 5: #include <lib.h> i don't see what this is needed for. you'll need to include types.h or the more specific header that provides the size_t definition -- To view, visit https://review.coreboot.org/c/coreboot/+/74344 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e0b4c3e3c7b6271ac64f9296c8a3f46d28e244d Gerrit-Change-Number: 74344 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Thu, 13 Apr 2023 11:47:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: cpu/x86/mp_init.c: Set topology on BSP
by Subrata Banik (Code Review) 13 Apr '23

13 Apr '23

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74374 ) Change subject: cpu/x86/mp_init.c: Set topology on BSP ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > Done Thanks -- To view, visit https://review.coreboot.org/c/coreboot/+/74374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3fae5efa86d8efc474c129b48bdfa1d1e2306acf Gerrit-Change-Number: 74374 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 13 Apr 2023 11:32:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: cpu/x86/mp_init.c: Set topology on BSP
by Subrata Banik (Code Review) 13 Apr '23

13 Apr '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74374 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: cpu/x86/mp_init.c: Set topology on BSP ...................................................................... cpu/x86/mp_init.c: Set topology on BSP The BSP might have non-zero lapicid so set the topology accordingly, without assuming it is 0. This fixes a cpu exception on at least Intel Meteorlake. This was caused by FSP CPU PPI being giving incorrect information about the BSP topology. This problem was introduced by 8b8400a "drivers/fsp2_0/mp_service_ppi: Use struct device to fill in buffer" which sets the PPI struct based on struct device. TESTED on google/rex Change-Id: I3fae5efa86d8efc474c129b48bdfa1d1e2306acf Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/74374 Reviewed-by: Subrata Banik <subratabanik(a)google.com> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Reviewed-by: Tarun Tuli <taruntuli(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/cpu/x86/mp_init.c 1 file changed, 27 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Tarun Tuli: Looks good to me, approved Kapil Porwal: Looks good to me, approved diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 4e92b75..28d6092 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -554,6 +554,7 @@ return CB_ERR; } bsp->path.apic.initial_lapicid = initial_lapicid(); + set_cpu_topology_from_leaf_b(bsp); /* Find the device structure for the boot CPU. */ set_cpu_info(0, bsp); -- To view, visit https://review.coreboot.org/c/coreboot/+/74374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3fae5efa86d8efc474c129b48bdfa1d1e2306acf Gerrit-Change-Number: 74374 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: cpu/intel/speedstep: Refactor P-state coordination
by Arthur Heymans (Code Review) 13 Apr '23

13 Apr '23

Attention is currently required from: Kyösti Mälkki. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74398 ) Change subject: cpu/intel/speedstep: Refactor P-state coordination ...................................................................... Patch Set 1: (1 comment) File src/cpu/intel/speedstep/acpi.c: https://review.coreboot.org/c/coreboot/+/74398/comment/ffca4101_3585a0f7 PS1, Line 66: ((cpuid_eax(1) >> 4) & 0xffff) == 0x1067 sidenote: get_fms() might be nice. -- To view, visit https://review.coreboot.org/c/coreboot/+/74398 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I12462f271821d3d8fe3324d84a65c2341729591e Gerrit-Change-Number: 74398 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Thu, 13 Apr 2023 11:26:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: cpu/x86/mp_init.c: Set topology on BSP
by Arthur Heymans (Code Review) 13 Apr '23

13 Apr '23

Attention is currently required from: Lean Sheng Tan, Yu-Ping Wu. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74374 ) Change subject: cpu/x86/mp_init.c: Set topology on BSP ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > > can you please improve the commit msg and mention that i have tested on google/rex. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/74374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3fae5efa86d8efc474c129b48bdfa1d1e2306acf Gerrit-Change-Number: 74374 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 13 Apr 2023 11:14:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: cpu/x86/mp_init.c: Set topology on BSP
by Arthur Heymans (Code Review) 13 Apr '23

13 Apr '23

Attention is currently required from: Arthur Heymans, Lean Sheng Tan, Yu-Ping Wu. Hello build bot (Jenkins), Tarun Tuli, Subrata Banik, Kapil Porwal, Lean Sheng Tan, Yu-Ping Wu, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74374 to look at the new patch set (#2). Change subject: cpu/x86/mp_init.c: Set topology on BSP ...................................................................... cpu/x86/mp_init.c: Set topology on BSP The BSP might have non-zero lapicid so set the topology accordingly, without assuming it is 0. This fixes a cpu exception on at least Intel Meteorlake. This was caused by FSP CPU PPI being giving incorrect information about the BSP topology. This problem was introduced by 8b8400a "drivers/fsp2_0/mp_service_ppi: Use struct device to fill in buffer" which sets the PPI struct based on struct device. TESTED on google/rex Change-Id: I3fae5efa86d8efc474c129b48bdfa1d1e2306acf Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/x86/mp_init.c 1 file changed, 22 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/74374/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3fae5efa86d8efc474c129b48bdfa1d1e2306acf Gerrit-Change-Number: 74374 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: cpu/intel/speedstep: Separate single SSDT CPU entry
by build bot (Jenkins) (Code Review) 13 Apr '23

13 Apr '23

Attention is currently required from: Kyösti Mälkki. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74400 ) Change subject: cpu/intel/speedstep: Separate single SSDT CPU entry ...................................................................... Patch Set 2: (1 comment) File src/cpu/intel/speedstep/acpi.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-174112): https://review.coreboot.org/c/coreboot/+/74400/comment/55bcaa0d_6d584a3b PS2, Line 73: static void generate_cpu_entry(const struct device *device, int cpu, int core, int cores_per_package) line length of 101 exceeds 100 columns -- To view, visit https://review.coreboot.org/c/coreboot/+/74400 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibe5d84c8fbff79cc73b01eee0980cbed71ceb506 Gerrit-Change-Number: 74400 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Thu, 13 Apr 2023 11:08:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: cpu/intel/speedstep: Separate single SSDT CPU entry
by Kyösti Mälkki (Code Review) 13 Apr '23

13 Apr '23

Attention is currently required from: Kyösti Mälkki. Hello Paul Menzel, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74400 to look at the new patch set (#2). Change subject: cpu/intel/speedstep: Separate single SSDT CPU entry ...................................................................... cpu/intel/speedstep: Separate single SSDT CPU entry Change-Id: Ibe5d84c8fbff79cc73b01eee0980cbed71ceb506 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/cpu/intel/speedstep/acpi.c 1 file changed, 53 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/74400/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74400 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibe5d84c8fbff79cc73b01eee0980cbed71ceb506 Gerrit-Change-Number: 74400 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: cpu/x86/mp_init.c: Set topology on BSP
by Tarun Tuli (Code Review) 13 Apr '23

13 Apr '23

Attention is currently required from: Arthur Heymans, Lean Sheng Tan, Yu-Ping Wu. Tarun Tuli has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74374 ) Change subject: cpu/x86/mp_init.c: Set topology on BSP ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3fae5efa86d8efc474c129b48bdfa1d1e2306acf Gerrit-Change-Number: 74374 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 13 Apr 2023 11:07:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2023