coreboot-gerrit February 2024

coreboot-gerrit@coreboot.org

1 participants
3041 discussions

[XS] Change in coreboot[main]: drivers/vpd: Return NULL for missing "feature_device_info"
by Kapil Porwal (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Gwendal Grignou, Nick Vaccaro, Shelley Chen, Subrata Banik. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80805?usp=email ) Change subject: drivers/vpd: Return NULL for missing "feature_device_info" ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80805?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: I76fc220ed792abdfefb0b1a37873b5b828bfdda8 Gerrit-Change-Number: 80805 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Comment-Date: Thu, 29 Feb 2024 18:12:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection
by Kapil Porwal (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Gwendal Grignou, Paul Menzel, Paz Zcharya, Subrata Banik. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80738?usp=email ) Change subject: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection ...................................................................... Patch Set 8: (1 comment) File src/vendorcode/google/chromeos/tpm_factory_config.c: https://review.coreboot.org/c/coreboot/+/80738/comment/4dcab166_d293fe50 : PS8, Line 81: vpd_get_feature_device_info Please use `device_info` here. -- To view, visit https://review.coreboot.org/c/coreboot/+/80738?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: I9bb1e868764738333977bd8c990bea4253c9d37b Gerrit-Change-Number: 80738 Gerrit-PatchSet: 8 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Paz Zcharya <pazz(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paz Zcharya <pazz(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Comment-Date: Thu, 29 Feb 2024 18:12:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/intel/gma: Allow SPARK function with side effects
by Felix Singer (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Nico Huber. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80616?usp=email ) Change subject: drivers/intel/gma: Allow SPARK function with side effects ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: Thank you both! -- To view, visit https://review.coreboot.org/c/coreboot/+/80616?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: I1eb879f57437587dc11d879fcc4042a70d384786 Gerrit-Change-Number: 80616 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Thu, 29 Feb 2024 18:04:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection
by Subrata Banik (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Gwendal Grignou, Kapil Porwal, Paul Menzel, Paz Zcharya. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80738?usp=email ) Change subject: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection ...................................................................... Patch Set 7: (4 comments) Patchset: PS7: > which one? marking resolved based on offline discussion. File src/vendorcode/google/chromeos/tpm_factory_config.c: https://review.coreboot.org/c/coreboot/+/80738/comment/5a276793_41d71ae9 : PS7, Line 49: * - Bits 3-0 (0x1): Must be 0x1 to signify compliance with Chromebook-Plus > > To be future-proof, the test on line 61 should be > > > > `return factory_config & CHROMEBOOK_PLUS_DEVICE;` > > If you read the bug ticket number attached to the commit message, you will see that the issue this CL aims to remedy is due to the fact that we were originally returning `return factory_config & CHROMEBOOK_PLUS_DEVICE;` > > When we `return factory_config & CHROMEBOOK_PLUS_DEVICE;`, that means we are considering a device qualified as CBX if `Bit 4 is set` or `Bit 0-3 is set` or `both are set`. > > However, in practice, we have seen devices that are CBX HW compliant (bit 0-3) but are not branded as CBX. As a result, we are displaying the CBX logo instead of the correct device segment logo (CB). This CL is intended to fix that issue, as described in the commit message. > > - If the GSC bit indicates that both Bit-4 and Bit 0-3 are set to 1, then we know that this is a HW branded CBX. > - If bit 0-3 is set, then it can be either a SB CBX or a regular CB. To detect which one it is properly, we are relying on VPD read. > - If none of the above logic is being met, then this device is definitely a CB. marking resolved based on offline discussion. https://review.coreboot.org/c/coreboot/+/80738/comment/91e18a20_06bb8248 : PS7, Line 74: * requirement. > Requested comment access to the doc. If there are corner cases, then line 97 is incorrect. marking resolved based on offline discussion. https://review.coreboot.org/c/coreboot/+/80738/comment/3f207026_9fd5f859 : PS7, Line 104: return strncmp(vpd_get_feature_device_info(), "CAI", 3) == 0; > As discussed in the meeting, in chromeos_device_branded_plus_soft() we can completely ignore GSC information and go by reading the VPD. yes, agreed. done. > Until protobuf decoding is added to coreboot, the device will be a regular chromebook when vpd_get_feature_device_info() is empty or indicates the feature_level is 0. marking resolved based on offline discussion. -- To view, visit https://review.coreboot.org/c/coreboot/+/80738?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: I9bb1e868764738333977bd8c990bea4253c9d37b Gerrit-Change-Number: 80738 Gerrit-PatchSet: 7 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Paz Zcharya <pazz(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paz Zcharya <pazz(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Comment-Date: Thu, 29 Feb 2024 17:53:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/vpd: Return NULL for missing "feature_device_info"
by Subrata Banik (Code Review) 29 Feb '24

29 Feb '24

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80805?usp=email ) Change subject: drivers/vpd: Return NULL for missing "feature_device_info" ...................................................................... drivers/vpd: Return NULL for missing "feature_device_info" Previously, vpd_get_feature_device_info() might have returned empty string if the "feature_device_info" VPD entry was not found. This change ensures a NULL return in such cases. BUG=b:324107408 TEST=Verify NULL is returned when calling vpd_get_feature_device_info() on a platform with a wiped VPD. Change-Id: I76fc220ed792abdfefb0b1a37873b5b828bfdda8 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/vpd/vpd_device_feature.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/80805/1 diff --git a/src/drivers/vpd/vpd_device_feature.c b/src/drivers/vpd/vpd_device_feature.c index 1c8682a..5413c01 100644 --- a/src/drivers/vpd/vpd_device_feature.c +++ b/src/drivers/vpd/vpd_device_feature.c @@ -11,5 +11,5 @@ if (vpd_gets(VPD_KEY_FEATURE_DEVICE_INFO, device_info, VPD_FEATURE_DEVICE_INFO_LEN, VPD_RW)) return device_info; - return ""; + return NULL; } -- To view, visit https://review.coreboot.org/c/coreboot/+/80805?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: I76fc220ed792abdfefb0b1a37873b5b828bfdda8 Gerrit-Change-Number: 80805 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection
by Subrata Banik (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Gwendal Grignou, Kapil Porwal, Paul Menzel, Paz Zcharya, Subrata Banik. Hello Gwendal Grignou, Kapil Porwal, Paul Menzel, Paz Zcharya, Shelley Chen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80738?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Code-Review+1 by Paul Menzel, Code-Review+2 by Kapil Porwal, Code-Review-1 by Gwendal Grignou, Verified+1 by build bot (Jenkins) Change subject: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection ...................................................................... vc/google/chromeos: Implement dynamic ChromeOS boot logo selection * Introduces logic to display context-specific boot splash logos. * Logo selection considers: * Chromebook-Plus hardware compliance (using factory_config). * VPD-based product segmentation (soft-branded vs. regular chromebook). * Default Chromebook logo as fallback for regular Chromebook. This patch fixes the problem where existing logic was unable to pick correct ChromeOS boot splash logo based on the product segmentation. Relation between product segment and boot splash screen: 1. Chromebook-Plus Hard-branded device: Renders "cb_plus_logo.bmp" logo 2. Chromebook-Plus Soft-branded device: Renders "cb_plus_logo.bmp" logo 3. Regular Chromebook device: Renders "cb_logo.bmp" BUG=b:324107408 TEST=Verified logo selection based on compliance and product requirements. Change-Id: I9bb1e868764738333977bd8c990bea4253c9d37b Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/vendorcode/google/chromeos/chromeos.h M src/vendorcode/google/chromeos/splash.c M src/vendorcode/google/chromeos/tpm_factory_config.c 3 files changed, 47 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/80738/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/80738?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: I9bb1e868764738333977bd8c990bea4253c9d37b Gerrit-Change-Number: 80738 Gerrit-PatchSet: 8 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Paz Zcharya <pazz(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paz Zcharya <pazz(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Gwendal Grignou <gwendal(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[main]: [WIP]soc/intel/xeon_sp: Use common _CRS code generation
by Patrick Rudolph (Code Review) 29 Feb '24

29 Feb '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Lean Sheng Tan, Shuo Liu, Tim Chu. Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80796?usp=email ) Change subject: [WIP]soc/intel/xeon_sp: Use common _CRS code generation ...................................................................... [WIP]soc/intel/xeon_sp: Use common _CRS code generation Drop SoC specific code and use generic implementation provided by pci_domain_fill_ssdt. Test: Booted on IBM/SBP1 to Ubuntu 22.04. Change-Id: I8b0bc2eb02569b5d74f8521d79e0af8fee880c80 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/cpx/chip.c M src/soc/intel/xeon_sp/cpx/soc_acpi.c M src/soc/intel/xeon_sp/skx/chip.c M src/soc/intel/xeon_sp/skx/soc_acpi.c M src/soc/intel/xeon_sp/spr/chip.c M src/soc/intel/xeon_sp/spr/ioat.c M src/soc/intel/xeon_sp/spr/soc_acpi.c 8 files changed, 11 insertions(+), 355 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/80796/1 diff --git a/src/soc/intel/xeon_sp/chip_common.c b/src/soc/intel/xeon_sp/chip_common.c index a0ecfd1..10b34c0 100644 --- a/src/soc/intel/xeon_sp/chip_common.c +++ b/src/soc/intel/xeon_sp/chip_common.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include <acpi/acpigen_pci.h> #include <assert.h> #include <console/console.h> #include <device/pci.h> @@ -173,6 +174,7 @@ .scan_bus = iio_pci_domain_scan_bus, #if CONFIG(HAVE_ACPI_TABLES) .acpi_name = soc_acpi_name, + .acpi_fill_ssdt = pci_domain_fill_ssdt, #endif }; @@ -186,6 +188,7 @@ .scan_bus = pci_host_bridge_scan_bus, #if CONFIG(HAVE_ACPI_TABLES) .acpi_name = soc_acpi_name, + .acpi_fill_ssdt = pci_domain_fill_ssdt, #endif }; diff --git a/src/soc/intel/xeon_sp/cpx/chip.c b/src/soc/intel/xeon_sp/cpx/chip.c index 5fc90e7..128a96e 100644 --- a/src/soc/intel/xeon_sp/cpx/chip.c +++ b/src/soc/intel/xeon_sp/cpx/chip.c @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ - +#include <acpi/acpigen_pci.h> #include <arch/ioapic.h> #include <console/console.h> #include <console/debug.h> @@ -34,6 +34,7 @@ #if CONFIG(HAVE_ACPI_TABLES) .write_acpi_tables = &northbridge_write_acpi_tables, .acpi_name = soc_acpi_name, + .acpi_fill_ssdt = pci_domain_fill_ssdt, #endif }; diff --git a/src/soc/intel/xeon_sp/cpx/soc_acpi.c b/src/soc/intel/xeon_sp/cpx/soc_acpi.c index 93b4c60..b2d6d79 100644 --- a/src/soc/intel/xeon_sp/cpx/soc_acpi.c +++ b/src/soc/intel/xeon_sp/cpx/soc_acpi.c @@ -43,69 +43,6 @@ void uncore_fill_ssdt(const struct device *device) { - struct iiostack_resource stack_info = {0}; - - /* Only add RTxx entries once. */ - if (device->upstream->secondary != 0 || device->upstream->segment_group != 0) - return; - - get_iiostack_info(&stack_info); - - for (uint8_t stack = 0; stack < stack_info.no_of_stacks; ++stack) { - const STACK_RES *ri = &stack_info.res[stack]; - char rtname[16]; - - snprintf(rtname, sizeof(rtname), "\\_SB.PC%02x", stack); - acpigen_write_scope(rtname); - - acpigen_write_name("_CRS"); - printk(BIOS_DEBUG, "\tCreating ResourceTemplate %s for stack: %d\n", - rtname, stack); - - acpigen_write_resourcetemplate_header(); - - /* bus resource */ - acpigen_resource_word(2, 0xc, 0, 0, ri->BusBase, ri->BusLimit, - 0x0, (ri->BusLimit - ri->BusBase + 1)); - - /* additional io resources on socket 0 bus 0 */ - if (stack == 0) { - /* ACPI 6.4.2.5 I/O Port Descriptor */ - acpigen_write_io16(0xCF8, 0xCFF, 0x1, 0x8, 1); - - /* IO decode CF8-CFF */ - acpigen_resource_word(1, 0xc, 0x3, 0, 0x0000, 0x03AF, 0, 0x03B0); - acpigen_resource_word(1, 0xc, 0x3, 0, 0x03E0, 0x0CF7, 0, 0x0918); - acpigen_resource_word(1, 0xc, 0x3, 0, 0x03B0, 0x03BB, 0, 0x000C); - acpigen_resource_word(1, 0xc, 0x3, 0, 0x03C0, 0x03DF, 0, 0x0020); - } - - /* IO resource */ - acpigen_resource_word(1, 0xc, 0x3, 0, ri->PciResourceIoBase, - ri->PciResourceIoLimit, 0x0, - (ri->PciResourceIoLimit - ri->PciResourceIoBase + 1)); - - /* additional mem32 resources on socket 0 bus 0 */ - if (stack == 0) { - acpigen_resource_dword(0, 0xc, 3, 0, VGA_MMIO_BASE, - VGA_MMIO_LIMIT, 0x0, VGA_MMIO_SIZE); - } - - /* Mem32 resource */ - acpigen_resource_dword(0, 0xc, 1, 0, ri->PciResourceMem32Base, - ri->PciResourceMem32Limit, 0x0, - (ri->PciResourceMem32Limit - ri->PciResourceMem32Base + 1)); - - /* Mem64 resource */ - acpigen_resource_qword(0, 0xc, 1, 0, ri->PciResourceMem64Base, - ri->PciResourceMem64Limit, 0x0, - (ri->PciResourceMem64Limit - ri->PciResourceMem64Base + 1)); - - acpigen_write_resourcetemplate_footer(); - - /* Scope */ - acpigen_pop_len(); - } } /* TODO: See if we can use the common generate_p_state_entries */ diff --git a/src/soc/intel/xeon_sp/skx/chip.c b/src/soc/intel/xeon_sp/skx/chip.c index 92663d9..752d54b 100644 --- a/src/soc/intel/xeon_sp/skx/chip.c +++ b/src/soc/intel/xeon_sp/skx/chip.c @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ - +#include <acpi/acpigen_pci.h> #include <cbfs.h> #include <console/console.h> #include <device/pci.h> @@ -20,6 +20,7 @@ #if CONFIG(HAVE_ACPI_TABLES) .write_acpi_tables = &northbridge_write_acpi_tables, .acpi_name = soc_acpi_name, + .acpi_fill_ssdt = pci_domain_fill_ssdt, #endif }; diff --git a/src/soc/intel/xeon_sp/skx/soc_acpi.c b/src/soc/intel/xeon_sp/skx/soc_acpi.c index 247d1dd..b98e998 100644 --- a/src/soc/intel/xeon_sp/skx/soc_acpi.c +++ b/src/soc/intel/xeon_sp/skx/soc_acpi.c @@ -56,79 +56,6 @@ void uncore_fill_ssdt(const struct device *device) { - const IIO_UDS *hob = get_iio_uds(); - - /* Only add RTxx entries once. */ - if (device->upstream->secondary != 0) - return; - - for (int socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; ++socket) { - if (!soc_cpu_is_enabled(socket)) - continue; - iio++; - IIO_RESOURCE_INSTANCE iio_resource = - hob->PlatformData.IIO_resource[socket]; - for (int stack = 0; stack <= PSTACK2; ++stack) { - const STACK_RES *ri = &iio_resource.StackRes[stack]; - char rtname[16]; - - snprintf(rtname, sizeof(rtname), "\\_SB.PC%02x", socket * MAX_IIO_STACK + stack); - acpigen_write_scope(rtname); - - acpigen_write_name("_CRS"); - - printk(BIOS_DEBUG, "\tCreating ResourceTemplate %s for socket: %d, stack: %d\n", - rtname, socket, stack); - - acpigen_write_resourcetemplate_header(); - - /* bus resource */ - acpigen_resource_word(2, 0xc, 0, 0, ri->BusBase, ri->BusLimit, - 0x0, (ri->BusLimit - ri->BusBase + 1)); - - // additional io resources on socket 0 bus 0 - if (socket == 0 && stack == 0) { - /* ACPI 6.4.2.5 I/O Port Descriptor */ - acpigen_write_io16(0xCF8, 0xCFF, 0x1, 0x8, 1); - - /* IO decode CF8-CFF */ - acpigen_resource_word(1, 0xc, 0x3, 0, 0x0000, 0x03AF, - 0, 0x03B0); - acpigen_resource_word(1, 0xc, 0x3, 0, 0x03E0, 0x0CF7, - 0, 0x0918); - acpigen_resource_word(1, 0xc, 0x3, 0, 0x03B0, 0x03BB, - 0, 0x000C); - acpigen_resource_word(1, 0xc, 0x3, 0, 0x03C0, 0x03DF, - 0, 0x0020); - } - - /* IO resource */ - acpigen_resource_word(1, 0xc, 0x3, 0, ri->PciResourceIoBase, - ri->PciResourceIoLimit, 0x0, - (ri->PciResourceIoLimit - ri->PciResourceIoBase + 1)); - - // additional mem32 resources on socket 0 bus 0 - if (socket == 0 && stack == 0) { - acpigen_resource_dword(0, 0xc, 3, 0, VGA_MMIO_BASE, - VGA_MMIO_LIMIT, 0x0, VGA_MMIO_SIZE); - } - - /* Mem32 resource */ - acpigen_resource_dword(0, 0xc, 1, 0, ri->PciResourceMem32Base, - ri->PciResourceMem32Limit, 0x0, - (ri->PciResourceMem32Limit - ri->PciResourceMem32Base + 1)); - - /* Mem64 resource */ - acpigen_resource_qword(0, 0xc, 1, 0, ri->PciResourceMem64Base, - ri->PciResourceMem64Limit, 0x0, - (ri->PciResourceMem64Limit - ri->PciResourceMem64Base + 1)); - - acpigen_write_resourcetemplate_footer(); - - /* Scope */ - acpigen_pop_len(); - } - } } void soc_power_states_generation(int core, int cores_per_package) diff --git a/src/soc/intel/xeon_sp/spr/chip.c b/src/soc/intel/xeon_sp/spr/chip.c index cc3719d..f677f69 100644 --- a/src/soc/intel/xeon_sp/spr/chip.c +++ b/src/soc/intel/xeon_sp/spr/chip.c @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ - +#include <acpi/acpigen_pci.h> #include <arch/ioapic.h> #include <console/console.h> #include <console/debug.h> @@ -46,6 +46,7 @@ #if CONFIG(HAVE_ACPI_TABLES) .write_acpi_tables = &northbridge_write_acpi_tables, .acpi_name = soc_acpi_name, + .acpi_fill_ssdt = pci_domain_fill_ssdt, #endif }; diff --git a/src/soc/intel/xeon_sp/spr/ioat.c b/src/soc/intel/xeon_sp/spr/ioat.c index b48a130..36b71e5 100644 --- a/src/soc/intel/xeon_sp/spr/ioat.c +++ b/src/soc/intel/xeon_sp/spr/ioat.c @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ - +#include <acpi/acpigen_pci.h> #include <stdbool.h> #include <console/console.h> @@ -24,6 +24,7 @@ .scan_bus = pci_host_bridge_scan_bus, #if CONFIG(HAVE_ACPI_TABLES) .acpi_name = soc_acpi_name, + .acpi_fill_ssdt = pci_domain_fill_ssdt, #endif }; diff --git a/src/soc/intel/xeon_sp/spr/soc_acpi.c b/src/soc/intel/xeon_sp/spr/soc_acpi.c index 42927c0..e5d8609 100644 --- a/src/soc/intel/xeon_sp/spr/soc_acpi.c +++ b/src/soc/intel/xeon_sp/spr/soc_acpi.c @@ -53,86 +53,6 @@ fill_fadt_extended_pm_io(fadt); } -static void create_dsdt_iou_pci_resource(uint8_t socket, uint8_t stack, const STACK_RES *ri, - bool stack_enabled) -{ - /* - Stacks 0 (TYPE_UBOX_IIO) - Scope: PC<socket><stack>, ResourceTemplate: P0RS - Stacks 1 .. 5 (TYPE_UBOX_IIO) - Scope: PC<socket><stack>, ResourceTemplate: RBRS - */ - /* write _CRS scope */ - char tres[16]; - snprintf(tres, sizeof(tres), "\\_SB.PC%d%X", socket, stack); - acpigen_write_scope(tres); - acpigen_write_name("_CRS"); - - printk(BIOS_DEBUG, "\tCreating ResourceTemplate %s for socket: %d, stack: %d\n", tres, - socket, stack); - - acpigen_write_resourcetemplate_header(); - - /* Bus Resource */ - if (stack_enabled) { - /* For stack with CXL device, the PCIe bus resource is BusBase only. */ - if (is_iio_cxl_stack_res(ri)) - acpigen_resource_word(2, 0xc, 0, 0, ri->BusBase, ri->BusBase, 0x0, 1); - else - acpigen_resource_word(2, 0xc, 0, 0, ri->BusBase, ri->BusLimit, 0x0, - (ri->BusLimit - ri->BusBase + 1)); - } else { - acpigen_resource_word(2, 0, 0, 0, 0, 0, 0, 0); - } - - /* Additional IO resources on socket 0 bus 0 */ - if (socket == 0 && stack == 0) { - /* ACPI 6.4.2.5 I/O Port Descriptor */ - acpigen_write_io16(0xCF8, 0xCFF, 0x1, 0x8, 1); - - /* IO decode CF8-CFF */ - acpigen_resource_word(1, 0xc, 0x3, 0, 0x0000, 0x03AF, 0, 0x03B0); - acpigen_resource_word(1, 0xc, 0x3, 0, 0x03E0, 0x0CF7, 0, 0x0918); - acpigen_resource_word(1, 0xc, 0x3, 0, 0x03B0, 0x03BB, 0, 0x000C); - acpigen_resource_word(1, 0xc, 0x3, 0, 0x03C0, 0x03DF, 0, 0x0020); - } - - /* IO resource */ - if (stack_enabled) { - acpigen_resource_word(1, 0xc, 0x3, 0, ri->PciResourceIoBase, - ri->PciResourceIoLimit, 0x0, - (ri->PciResourceIoLimit - ri->PciResourceIoBase + 1)); - - /* Additional Mem32 resources on socket 0 bus 0 */ - if (socket == 0 && stack == 0) { - acpigen_resource_dword(0, 0xc, 3, 0, VGA_MMIO_BASE, - VGA_MMIO_LIMIT, 0x0, VGA_MMIO_SIZE); - } - - /* Mem32 resource */ - acpigen_resource_dword( - 0, 0xc, 1, 0, ri->PciResourceMem32Base, ri->PciResourceMem32Limit, 0x0, - (ri->PciResourceMem32Limit - ri->PciResourceMem32Base + 1)); - - /* Mem64 resource */ - acpigen_resource_qword( - 0, 0xc, 1, 0, ri->PciResourceMem64Base, ri->PciResourceMem64Limit, 0x0, - (ri->PciResourceMem64Limit - ri->PciResourceMem64Base + 1)); - } else { - /* Zeroed IO resource */ - acpigen_resource_word(1, 0, 3, 0, 0, 0, 0, 0); - - /* Zeroed Mem32 resource */ - acpigen_resource_dword(0, 0, 1, 0, 0, 0, 0, 0); - - /* Zeroed Mem64 resource */ - acpigen_resource_dword(0, 0, 1, 0, 0, 0, 0, 0); - } - - acpigen_write_resourcetemplate_footer(); - acpigen_pop_len(); -} - static void create_dsdt_iou_cxl_resource(uint8_t socket, uint8_t stack, const STACK_RES *ri, bool stack_enabled) { /* @@ -186,138 +106,6 @@ acpigen_pop_len(); } -static void create_dsdt_ioat_resource(uint8_t socket, uint8_t stack, const STACK_RES *ri, bool stack_enabled) -{ - if (!stack_enabled) - return; - - /* - Stacks 8 .. B (TYPE_DINO) - Scope: DI<socket><stack> for DINO, ResourceTemplate: DT - Scope: CP<socket><stack> for CPM (i.e., QAT), ResourceTemplate: MT - Scope: HQ<socket><stack> for HQM (i.e., DLB), ResourceTemplate: HT - */ - - enum { - DSDT_DINO = 0, - DSDT_CPM, - DSDT_HQM, - DSDT_CPM1, - DSDT_HQM1 - }; - uint8_t rlist[] = {DSDT_DINO, DSDT_CPM, DSDT_HQM, DSDT_CPM1, DSDT_HQM1}; - - for (int i = 0; i < ARRAY_SIZE(rlist); ++i) { - uint8_t bus_base, bus_limit; - uint64_t mem64_base, mem64_limit; - char tres[16]; - - /* Note, This allocates the resources in a different order than - * coreboot (DINO base is last). This causes the kernel to - * reallocate the DINO BARs. - * TODO: Use the resource settings from coreboot */ - if (rlist[i] == DSDT_DINO) { - bus_base = ri->BusBase; - bus_limit = ri->BusBase; - mem64_base = ri->PciResourceMem64Base + CPM_MMIO_SIZE + HQM_MMIO_SIZE - + CPM_MMIO_SIZE + HQM_MMIO_SIZE; - mem64_limit = ri->PciResourceMem64Limit; - snprintf(tres, sizeof(tres), "\\_SB.DI%d%X", socket, stack); - } else if (rlist[i] == DSDT_CPM) { - bus_base = ri->BusBase + CPM_BUS_OFFSET; - bus_limit = bus_base + CPM_RESERVED_BUS; - mem64_base = ri->PciResourceMem64Base; - mem64_limit = mem64_base + CPM_MMIO_SIZE - 1; - snprintf(tres, sizeof(tres), "\\_SB.PM%d%X", socket, stack); - } else if (rlist[i] == DSDT_HQM) { - bus_base = ri->BusBase + HQM_BUS_OFFSET; - bus_limit = bus_base + HQM_RESERVED_BUS; - mem64_base = ri->PciResourceMem64Base + CPM_MMIO_SIZE; - mem64_limit = mem64_base + HQM_MMIO_SIZE - 1; - snprintf(tres, sizeof(tres), "\\_SB.HQ%d%X", socket, stack); - } else if (rlist[i] == DSDT_CPM1) { - bus_base = ri->BusBase + CPM1_BUS_OFFSET; - bus_limit = bus_base + CPM_RESERVED_BUS; - mem64_base = ri->PciResourceMem64Base + CPM_MMIO_SIZE + HQM_MMIO_SIZE; - mem64_limit = mem64_base + CPM_MMIO_SIZE - 1; - snprintf(tres, sizeof(tres), "\\_SB.PN%d%X", socket, stack); - } else { - bus_base = ri->BusBase + HQM1_BUS_OFFSET; - bus_limit = bus_base + HQM_RESERVED_BUS; - mem64_base = ri->PciResourceMem64Base + CPM_MMIO_SIZE + HQM_MMIO_SIZE - + CPM_MMIO_SIZE; - mem64_limit = mem64_base + HQM_MMIO_SIZE - 1; - snprintf(tres, sizeof(tres), "\\_SB.HR%d%X", socket, stack); - } - - /* Note, some SKU doesn't provide CPM1 and HQM1 and owns smaller bus ranges - accordingly*/ - if (bus_limit > ri->BusLimit) - continue; - - printk(BIOS_DEBUG, - "\tCreating Dino ResourceTemplate %s for socket: %d, " - "stack: %d\n bus_base:0x%x, bus_limit:0x%x\n", - tres, socket, stack, bus_base, bus_limit); - - acpigen_write_scope(tres); - acpigen_write_name("_CRS"); - acpigen_write_resourcetemplate_header(); - - acpigen_resource_word(2, 0xc, 0, 0, bus_base, bus_limit, 0x0, - (bus_limit - bus_base + 1)); - - /* Mem32 resource */ - if (rlist[i] == DSDT_DINO) - acpigen_resource_dword(0, 0xc, 1, 0, ri->PciResourceMem32Base, - ri->PciResourceMem32Limit, 0x0, - (ri->PciResourceMem32Limit - - ri->PciResourceMem32Base + 1)); - - /* Mem64 resource */ - acpigen_resource_qword(0, 0xc, 1, 0, mem64_base, mem64_limit, 0, - (mem64_limit - mem64_base + 1)); - - acpigen_write_resourcetemplate_footer(); - acpigen_pop_len(); - } -} - -static void create_dsdt_ubox_resource(uint8_t socket, uint8_t stack, const STACK_RES *ri, bool stack_enabled) -{ - /* - Stacks D .. E (TYPE_UBOX) - Scope: UC/UD<socket><0..1> for UBOX[1-2], ResourceTemplate: UT/UU - */ - for (int i = 0; i < 2; ++i) { - char tres[16]; - /* write _CRS scope */ - if (i == 0) - snprintf(tres, sizeof(tres), "\\_SB.UC%d%X", socket, stack); - else - snprintf(tres, sizeof(tres), "\\_SB.UD%d%X", socket, stack); - - printk(BIOS_DEBUG, "\tCreating ResourceTemplate %s for socket: %d, stack: %d\n", - tres, socket, stack); - - acpigen_write_scope(tres); - acpigen_write_name("_CRS"); - acpigen_write_resourcetemplate_header(); - - if (!stack_enabled) - acpigen_resource_word(2, 0, 0, 0, 0, 0, 0, 0); - else if (i == 0) - acpigen_resource_word(2, 0xc, 0, 0, ri->BusBase, ri->BusBase, 0x0, 1); - else - acpigen_resource_word(2, 0xc, 0, 0, ri->BusBase + 1, ri->BusBase + 1, - 0x0, 1); - - acpigen_write_resourcetemplate_footer(); - acpigen_pop_len(); - } -} - - /* * Add a DSDT ACPI Name field for STACK enable setting. * This is retrieved by the device _STA defined in iiostack.asl @@ -372,16 +160,13 @@ __func__, socket, stack, ri->Personality); if (stack <= IioStack5) { // TYPE_UBOX_IIO - create_dsdt_iou_pci_resource(socket, stack, ri, stack_enabled); if (is_iio_cxl_stack_res(ri)) create_dsdt_iou_cxl_resource(socket, stack, ri, stack_enabled); create_dsdt_stack_sta(socket, stack, ri, stack_enabled); } else if (stack >= IioStack8 && stack <= IioStack11) { // TYPE_DINO - create_dsdt_ioat_resource(socket, stack, ri, stack_enabled); create_dsdt_stack_sta(socket, stack, ri, stack_enabled); } else if (stack == IioStack13) { // TYPE_UBOX - create_dsdt_ubox_resource(socket, stack, ri, stack_enabled); create_dsdt_stack_sta(socket, stack, ri, stack_enabled); } } -- To view, visit https://review.coreboot.org/c/coreboot/+/80796?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: I8b0bc2eb02569b5d74f8521d79e0af8fee880c80 Gerrit-Change-Number: 80796 Gerrit-PatchSet: 1 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-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.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-MessageType: newchange

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp: Drop SPI_BASE from _CRS
by Patrick Rudolph (Code Review) 29 Feb '24

29 Feb '24

1 0

[M] Change in coreboot[main]: soc/intel/xeon_sp: Add ACPI names
by Patrick Rudolph (Code Review) 29 Feb '24

29 Feb '24

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp/spr: Fix IOAT resources
by Patrick Rudolph (Code Review) 29 Feb '24

29 Feb '24

1 0

← Newer
1
2
3
4
5
6
7
8
9
...
305
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2024