coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[S] Change in coreboot[main]: amdfwtool: Set the table size for L1 separately
by Felix Held (Code Review) 06 Mar '24

06 Mar '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78274?usp=email ) Change subject: amdfwtool: Set the table size for L1 separately ...................................................................... amdfwtool: Set the table size for L1 separately The space defined by size of the L1 table can not overlap with ISH header. For other cases, the size defines the directory and its content. The PSP spec does not say it quite clearly. This change is partly based … [View More]on guess and can make extraction tool work so far. Change-Id: Id4fbc6d57d7ea070a9478649a96af92be9441289 Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78274 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M util/amdfwtool/amdfwtool.c 1 file changed, 23 insertions(+), 13 deletions(-) Approvals: Felix Held: Looks good to me, approved build bot (Jenkins): Verified diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index e37c635..61ccefb 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -566,16 +566,17 @@ set_current_pointer(ctx, ALIGN_UP(ctx->current + add, align)); } -static void *new_psp_dir(context *ctx, int multi) +static void *new_psp_dir(context *ctx, amd_cb_config *cb_config) { void *ptr; + uint32_t align_end = cb_config->need_ish ? TABLE_ALIGNMENT : 1; /* * Force both onto boundary when multi. Primary table is after * updatable table, so alignment ensures primary can stay intact * if secondary is reprogrammed. */ - if (multi) + if (cb_config->multi_level) adjust_current_pointer(ctx, 0, TABLE_ERASE_ALIGNMENT); else adjust_current_pointer(ctx, 0, TABLE_ALIGNMENT); @@ -586,7 +587,7 @@ ((psp_directory_header *)ptr)->additional_info_fields.address_mode = ctx->address_mode; adjust_current_pointer(ctx, sizeof(psp_directory_header) + MAX_PSP_ENTRIES * sizeof(psp_directory_entry), - 1); + align_end); return ptr; } @@ -612,7 +613,8 @@ return ptr; } -static void fill_dir_header(void *directory, uint32_t count, uint32_t cookie, context *ctx) +static void fill_dir_header(void *directory, uint32_t count, uint32_t cookie, + context *ctx, amd_cb_config *cb_config) { psp_combo_directory *cdir = directory; psp_directory_table *dir = directory; @@ -647,7 +649,14 @@ break; case PSP_COOKIE: case PSPL2_COOKIE: - table_size = ctx->current - ctx->current_table; + if (cookie == PSP_COOKIE && cb_config->need_ish) + /* The ISH header can not be in the space defined by L1 table size. + * The space is allocated when the L1 header is created. */ + table_size = TABLE_ALIGNMENT; + else + /* Generally table size not just constains the header, + * but all the FWs. */ + table_size = ctx->current - ctx->current_table; if ((table_size % TABLE_ALIGNMENT) != 0) { fprintf(stderr, "The PSP table size should be 4K aligned\n"); amdfwtool_cleanup(ctx); @@ -1116,7 +1125,7 @@ count++; } - fill_dir_header(pspdir, count, cookie, ctx); + fill_dir_header(pspdir, count, cookie, ctx, cb_config); ctx->current_table = current_table_save; } @@ -1479,7 +1488,7 @@ count++; } - fill_dir_header(biosdir, count, cookie, ctx); + fill_dir_header(biosdir, count, cookie, ctx, cb_config); } enum { @@ -2347,13 +2356,13 @@ if (cb_config.multi_level) { /* Do 2nd PSP directory followed by 1st */ - pspdir2 = new_psp_dir(&ctx, cb_config.multi_level); + pspdir2 = new_psp_dir(&ctx, &cb_config); integrate_psp_firmwares(&ctx, pspdir2, NULL, NULL, amd_psp_fw_table, PSPL2_COOKIE, &cb_config); if (cb_config.recovery_ab && !cb_config.recovery_ab_single_copy) { /* Create a copy of PSP Directory 2 in the backup slot B. Related biosdir2_b copy will be created later. */ - pspdir2_b = new_psp_dir(&ctx, cb_config.multi_level); + pspdir2_b = new_psp_dir(&ctx, &cb_config); integrate_psp_firmwares(&ctx, pspdir2_b, NULL, NULL, amd_psp_fw_table, PSPL2_COOKIE, &cb_config); } else { @@ -2367,12 +2376,12 @@ */ pspdir2_b = NULL; /* More explicitly */ } - pspdir = new_psp_dir(&ctx, cb_config.multi_level); + pspdir = new_psp_dir(&ctx, &cb_config); integrate_psp_firmwares(&ctx, pspdir, pspdir2, pspdir2_b, amd_psp_fw_table, PSP_COOKIE, &cb_config); } else { /* flat: PSP 1 cookie and no pointer to 2nd table */ - pspdir = new_psp_dir(&ctx, cb_config.multi_level); + pspdir = new_psp_dir(&ctx, &cb_config); integrate_psp_firmwares(&ctx, pspdir, NULL, NULL, amd_psp_fw_table, PSP_COOKIE, &cb_config); } @@ -2388,7 +2397,8 @@ psp_combo_dir->entries[combo_index].lvl2_addr = BUFF_TO_RUN_MODE(ctx, pspdir, AMD_ADDR_REL_BIOS); - fill_dir_header(psp_combo_dir, combo_index + 1, PSP2_COOKIE, &ctx); + fill_dir_header(psp_combo_dir, combo_index + 1, + PSP2_COOKIE, &ctx, &cb_config); } if (have_bios_tables(amd_bios_table)) { @@ -2441,7 +2451,7 @@ BUFF_TO_RUN_MODE(ctx, biosdir, AMD_ADDR_REL_BIOS); fill_dir_header(bhd_combo_dir, combo_index + 1, - BHD2_COOKIE, &ctx); + BHD2_COOKIE, &ctx, &cb_config); } } } while (cb_config.use_combo && ++combo_index < MAX_COMBO_ENTRIES && -- To view, visit https://review.coreboot.org/c/coreboot/+/78274?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: Id4fbc6d57d7ea070a9478649a96af92be9441289 Gerrit-Change-Number: 78274 Gerrit-PatchSet: 15 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged [View Less]

1 0

[S] Change in coreboot[main]: src/soc/intel/common: Add Intel RMT+ support in DEV mode
by Arthur Heymans (Code Review) 06 Mar '24

06 Mar '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Gaggery Tsai, Jamie Ryu, Jérémy Compostella, Kane Chen, Karthik Ramasubramanian, Krishna P Bhat D, Pratikkumar V Prajapati, Ravishankar Sarawadi, Ronak Kanabar, Subrata Banik, V Sowmya, Wonkyu Kim. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77904?usp=email ) Change subject: src/soc/intel/common: Add Intel RMT+ support in DEV mode ...................................… [View More]................................... Patch Set 7: (2 comments) File src/soc/intel/common/block/memory/Kconfig: https://review.coreboot.org/c/coreboot/+/77904/comment/03cd7f2a_6aa86837 : PS7, Line 33: depends on VBOOT why? File src/soc/intel/common/block/memory/meminit.c: https://review.coreboot.org/c/coreboot/+/77904/comment/de57fac3_79f5acad : PS7, Line 228: vboot_developer_mode_enabled() why? -- To view, visit https://review.coreboot.org/c/coreboot/+/77904?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: I03df4d0a927caa101da3db078b268dcdb4e78550 Gerrit-Change-Number: 77904 Gerrit-PatchSet: 7 Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 06 Mar 2024 10:29:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment [View Less]

1 0

[S] Change in coreboot[main]: src/soc/intel/common: Add Intel RMT+ support in DEV mode
by Arthur Heymans (Code Review) 06 Mar '24

06 Mar '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Gaggery Tsai, Jamie Ryu, Jérémy Compostella, Kane Chen, Karthik Ramasubramanian, Krishna P Bhat D, Pratikkumar V Prajapati, Ravishankar Sarawadi, Ronak Kanabar, Subrata Banik, V Sowmya, Wonkyu Kim. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77904?usp=email ) Change subject: src/soc/intel/common: Add Intel RMT+ support in DEV mode ...................................… [View More]................................... Patch Set 7: (1 comment) File src/soc/intel/common/block/memory/meminit.c: https://review.coreboot.org/c/coreboot/+/77904/comment/1daf6efa_27018979 : PS7, Line 221: __weak void enable_rmt_plus_platform(FSP_M_CONFIG *mem_cfg) : { : printk(BIOS_DEBUG, "RMT+ is not supported.\n"); : } Why do you need this? The Kconfig should guard it? -- To view, visit https://review.coreboot.org/c/coreboot/+/77904?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: I03df4d0a927caa101da3db078b268dcdb4e78550 Gerrit-Change-Number: 77904 Gerrit-PatchSet: 7 Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 06 Mar 2024 10:28:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment [View Less]

1 0

[L] Change in coreboot[main]: [Squash]: Add ACPI BDAT support
by Arthur Heymans (Code Review) 06 Mar '24

06 Mar '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Chen, Gang C, Cliff Huang, Gaggery Tsai, Jamie Ryu, Jérémy Compostella, Kane Chen, Karthik Ramasubramanian, Krishna P Bhat D, Lance Zhao, Pratikkumar V Prajapati, Ravishankar Sarawadi, Ronak Kanabar, Subrata Banik, Tim Wawrzynczak, V Sowmya, Wonkyu Kim. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77255?usp=email ) Change subject: [Squash]: Add ACPI BDAT support .................… [View More]..................................................... Patch Set 15: Code-Review+1 (4 comments) File src/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/77255/comment/474b03fb_be7fb726 : PS15, Line 1108: /* Calculate checksum */ : header->checksum = acpi_checksum((void *)bdat, header->length); remove File src/soc/intel/common/block/acpi/acpi_bdat.c: https://review.coreboot.org/c/coreboot/+/77255/comment/792c5e56_7741ff16 : PS15, Line 23: static void print_guid(EFI_GUID *guid) : { : int i; : if (guid != NULL) { : printk(BIOS_DEBUG, "GUID = "); : printk(BIOS_DEBUG, "0x%08x-", guid->Data1); : printk(BIOS_DEBUG, "0x%04x-", guid->Data2); : printk(BIOS_DEBUG, "0x%04x-", guid->Data3); : for (i = 0; i < GUID_TAIL_BYTES; i++) { : printk(BIOS_DEBUG, "0x%02x", guid->Data4[i]); : if (i != (GUID_TAIL_BYTES-1)) : printk(BIOS_DEBUG, "-"); : } : printk(BIOS_DEBUG, "\n"); : } : } this is reimplemented a lot. Can it be added to common code somewhere or reuse existing code. https://review.coreboot.org/c/coreboot/+/77255/comment/0f628fad_f843c6e8 : PS15, Line 40: get_crc16 Use CRC marcro? https://review.coreboot.org/c/coreboot/+/77255/comment/2db46581_fa7eaf54 : PS15, Line 86: if (guid != NULL) reduce indentation: if (guid == NULL) continue; ... -- To view, visit https://review.coreboot.org/c/coreboot/+/77255?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: I5eb57f65ef5f24458f09587b7c7694156f2ed1ce Gerrit-Change-Number: 77255 Gerrit-PatchSet: 15 Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 06 Mar 2024 10:27:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment [View Less]

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp: Unshare UDK binding among Xeon-SP platforms
by Lean Sheng Tan (Code Review) 06 Mar '24

06 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Patrick Rudolph, Shuo Liu, Tim Chu. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81041?usp=email ) Change subject: soc/intel/xeon_sp: Unshare UDK binding among Xeon-SP platforms ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81041?usp=email To … [View More]

1 0

[M] Change in coreboot[main]: soc/intel/xeon_sp: Add ACPI names
by Lean Sheng Tan (Code Review) 06 Mar '24

06 Mar '24

Lean Sheng Tan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80792?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/intel/xeon_sp: Add ACPI names ...................................................................... soc/intel/xeon_sp: Add ACPI names Set the unused 'name' property of the domain device and store the ACPI name. Every IIO stack can have multiple … [View More]domain devices, each owning a subset of the available bus range within the stack. The name will be used in future changes to generate ACPI names in SSDT code generation. It can also be used to identify the domain type by looking at the first two characters of the name. Change-Id: Ic4cc81d198fb88300394055682a3954bf22db570 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80792 Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Shuo Liu <shuo.liu(a)intel.com> --- M src/soc/intel/xeon_sp/acpi.c M src/soc/intel/xeon_sp/chip_common.c M src/soc/intel/xeon_sp/cpx/chip.c M src/soc/intel/xeon_sp/include/soc/acpi.h M src/soc/intel/xeon_sp/include/soc/chip_common.h M src/soc/intel/xeon_sp/skx/chip.c M src/soc/intel/xeon_sp/spr/chip.c M src/soc/intel/xeon_sp/spr/ioat.c 8 files changed, 85 insertions(+), 39 deletions(-) Approvals: Shuo Liu: Looks good to me, but someone else must approve build bot (Jenkins): Verified Lean Sheng Tan: Looks good to me, approved diff --git a/src/soc/intel/xeon_sp/acpi.c b/src/soc/intel/xeon_sp/acpi.c index 5eaa762..0cc0931 100644 --- a/src/soc/intel/xeon_sp/acpi.c +++ b/src/soc/intel/xeon_sp/acpi.c @@ -1,7 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ #include <assert.h> +#include <commonlib/stdlib.h> #include <intelblocks/acpi.h> +#include <soc/chip_common.h> #include <soc/pci_devs.h> #include <soc/util.h> #include <stdint.h> @@ -125,3 +127,32 @@ return index; } + +void iio_domain_set_acpi_name(struct device *dev, const char *prefix) +{ + const union xeon_domain_path dn = { + .domain_path = dev->path.domain.domain + }; + + assert(dn.socket < CONFIG_MAX_SOCKET); + assert(dn.stack < 16); + assert(prefix != NULL && strlen(prefix) == 2); + + if (dn.socket >= CONFIG_MAX_SOCKET || dn.stack >= 16 || + !prefix || strlen(prefix) != 2) + return; + + char *name = xmalloc(ACPI_NAME_BUFFER_SIZE); + snprintf(name, ACPI_NAME_BUFFER_SIZE, "%s%1X%1X", prefix, dn.socket, dn.stack); + dev->name = name; +} + +const char *soc_acpi_name(const struct device *dev) +{ + if (dev->path.type == DEVICE_PATH_DOMAIN) + return dev->name; + + /* FIXME: Add SoC specific device names here */ + + return NULL; +} diff --git a/src/soc/intel/xeon_sp/chip_common.c b/src/soc/intel/xeon_sp/chip_common.c index 0c1d52b..03ae70a 100644 --- a/src/soc/intel/xeon_sp/chip_common.c +++ b/src/soc/intel/xeon_sp/chip_common.c @@ -2,8 +2,10 @@ #include <assert.h> #include <console/console.h> -#include <post.h> #include <device/pci.h> +#include <intelblocks/acpi.h> +#include <post.h> +#include <soc/acpi.h> #include <soc/chip_common.h> #include <soc/soc_util.h> #include <soc/util.h> @@ -170,6 +172,9 @@ .read_resources = iio_pci_domain_read_resources, .set_resources = pci_domain_set_resources, .scan_bus = iio_pci_domain_scan_bus, +#if CONFIG(HAVE_ACPI_TABLES) + .acpi_name = soc_acpi_name, +#endif }; /* @@ -180,6 +185,9 @@ .read_resources = noop_read_resources, .set_resources = noop_set_resources, .scan_bus = pci_host_bridge_scan_bus, +#if CONFIG(HAVE_ACPI_TABLES) + .acpi_name = soc_acpi_name, +#endif }; /* @@ -194,6 +202,8 @@ .domain_path = dp.domain_path }; + /* Only expect 2 UBOX buses here */ + assert(bus_base + 1 == bus_limit); for (int i = bus_base; i <= bus_limit; i++) { new_path.bus = i; @@ -208,6 +218,8 @@ die("%s: out of memory.\n", __func__); domain->ops = &ubox_pcie_domain_ops; + const char *prefix = (i == bus_base) ? DOMAIN_TYPE_UBX0 : DOMAIN_TYPE_UBX1; + iio_domain_set_acpi_name(domain, prefix); struct bus *const bus = alloc_bus(domain); bus->secondary = i; @@ -226,9 +238,10 @@ for (int s = 0; s < hob->PlatformData.numofIIO; ++s) { for (int x = 0; x < MAX_LOGIC_IIO_STACK; ++x) { - if (s == 0 && x == 0) + if (s == 0 && x == 0) { + iio_domain_set_acpi_name(dev, DOMAIN_TYPE_PCIE); continue; - + } const STACK_RES *ri = &hob->PlatformData.IIO_resource[s].StackRes[x]; if (ri->BusBase > ri->BusLimit) continue; @@ -249,6 +262,7 @@ die("%s: out of memory.\n", __func__); iio_domain->ops = &iio_pcie_domain_ops; + iio_domain_set_acpi_name(iio_domain, DOMAIN_TYPE_PCIE); } else if (CONFIG(HAVE_IOAT_DOMAINS)) soc_create_ioat_domains(dn, dev->upstream, ri); } diff --git a/src/soc/intel/xeon_sp/cpx/chip.c b/src/soc/intel/xeon_sp/cpx/chip.c index 6274f02..5fc90e7 100644 --- a/src/soc/intel/xeon_sp/cpx/chip.c +++ b/src/soc/intel/xeon_sp/cpx/chip.c @@ -27,22 +27,13 @@ mainboard_silicon_init_params(silupd); } -#if CONFIG(HAVE_ACPI_TABLES) -const char *soc_acpi_name(const struct device *dev) -{ - if (dev->path.type == DEVICE_PATH_DOMAIN) - return "PC00"; - return NULL; -} -#endif - static struct device_operations pci_domain_ops = { .read_resources = iio_pci_domain_read_resources, .set_resources = pci_domain_set_resources, .scan_bus = iio_pci_domain_scan_bus, #if CONFIG(HAVE_ACPI_TABLES) .write_acpi_tables = &northbridge_write_acpi_tables, - .acpi_name = soc_acpi_name + .acpi_name = soc_acpi_name, #endif }; diff --git a/src/soc/intel/xeon_sp/include/soc/acpi.h b/src/soc/intel/xeon_sp/include/soc/acpi.h index 47c4090..afccfe4 100644 --- a/src/soc/intel/xeon_sp/include/soc/acpi.h +++ b/src/soc/intel/xeon_sp/include/soc/acpi.h @@ -29,4 +29,6 @@ unsigned long acpi_fill_hmat(unsigned long current); unsigned long cxl_fill_srat(unsigned long current); +void iio_domain_set_acpi_name(struct device *dev, const char *prefix); + #endif /* _SOC_ACPI_H_ */ diff --git a/src/soc/intel/xeon_sp/include/soc/chip_common.h b/src/soc/intel/xeon_sp/include/soc/chip_common.h index 3727777..a010bd1 100644 --- a/src/soc/intel/xeon_sp/include/soc/chip_common.h +++ b/src/soc/intel/xeon_sp/include/soc/chip_common.h @@ -15,6 +15,20 @@ }; }; +/* + * Every STACK can have multiple PCI domains with an unique domain type. + * This is only of cosmetic nature and generates more readable ACPI code, + * but isn't technical necessary. + */ +#define DOMAIN_TYPE_CPM0 "PM" +#define DOMAIN_TYPE_CPM1 "PN" +#define DOMAIN_TYPE_DINO "DI" +#define DOMAIN_TYPE_HQM0 "HQ" +#define DOMAIN_TYPE_HQM1 "HR" +#define DOMAIN_TYPE_PCIE "PC" +#define DOMAIN_TYPE_UBX0 "UC" +#define DOMAIN_TYPE_UBX1 "UD" + void iio_pci_domain_read_resources(struct device *dev); void iio_pci_domain_scan_bus(struct device *dev); void attach_iio_stacks(struct device *dev); diff --git a/src/soc/intel/xeon_sp/skx/chip.c b/src/soc/intel/xeon_sp/skx/chip.c index 68737ed..92663d9 100644 --- a/src/soc/intel/xeon_sp/skx/chip.c +++ b/src/soc/intel/xeon_sp/skx/chip.c @@ -13,22 +13,13 @@ #include <soc/soc_util.h> #include <soc/util.h> -#if CONFIG(HAVE_ACPI_TABLES) -const char *soc_acpi_name(const struct device *dev) -{ - if (dev->path.type == DEVICE_PATH_DOMAIN) - return "PC00"; - return NULL; -} -#endif - static struct device_operations pci_domain_ops = { .read_resources = iio_pci_domain_read_resources, .set_resources = pci_domain_set_resources, .scan_bus = iio_pci_domain_scan_bus, #if CONFIG(HAVE_ACPI_TABLES) .write_acpi_tables = &northbridge_write_acpi_tables, - .acpi_name = soc_acpi_name + .acpi_name = soc_acpi_name, #endif }; diff --git a/src/soc/intel/xeon_sp/spr/chip.c b/src/soc/intel/xeon_sp/spr/chip.c index d57b240..cc3719d 100644 --- a/src/soc/intel/xeon_sp/spr/chip.c +++ b/src/soc/intel/xeon_sp/spr/chip.c @@ -8,6 +8,7 @@ #include <device/pci_ids.h> #include <device/pci_def.h> #include <device/pciexp.h> +#include <intelblocks/acpi.h> #include <intelblocks/gpio.h> #include <intelblocks/lpc_lib.h> #include <intelblocks/p2sb.h> @@ -37,15 +38,6 @@ mainboard_silicon_init_params(silupd); } -#if CONFIG(HAVE_ACPI_TABLES) -const char *soc_acpi_name(const struct device *dev); -const char *soc_acpi_name(const struct device *dev) -{ - if (dev->path.type == DEVICE_PATH_DOMAIN) - return "PC00"; - return NULL; -} -#endif static struct device_operations pci_domain_ops = { .read_resources = iio_pci_domain_read_resources, @@ -53,7 +45,7 @@ .scan_bus = iio_pci_domain_scan_bus, #if CONFIG(HAVE_ACPI_TABLES) .write_acpi_tables = &northbridge_write_acpi_tables, - .acpi_name = soc_acpi_name + .acpi_name = soc_acpi_name, #endif }; diff --git a/src/soc/intel/xeon_sp/spr/ioat.c b/src/soc/intel/xeon_sp/spr/ioat.c index 0d81d0d..5528efa 100644 --- a/src/soc/intel/xeon_sp/spr/ioat.c +++ b/src/soc/intel/xeon_sp/spr/ioat.c @@ -8,6 +8,8 @@ #include <defs_iio.h> #include <hob_iiouds.h> +#include <intelblocks/acpi.h> +#include <soc/acpi.h> #include <IioPcieConfigUpd.h> #include <soc/chip_common.h> @@ -21,12 +23,16 @@ .read_resources = noop_read_resources, .set_resources = pci_domain_set_resources, .scan_bus = pci_host_bridge_scan_bus, +#if CONFIG(HAVE_ACPI_TABLES) + .acpi_name = soc_acpi_name, +#endif }; static void create_ioat_domain(const union xeon_domain_path dp, struct bus *const upstream, const unsigned int bus_base, const unsigned int bus_limit, const resource_t mem32_base, const resource_t mem32_limit, - const resource_t mem64_base, const resource_t mem64_limit) + const resource_t mem64_base, const resource_t mem64_limit, + const char *prefix) { union xeon_domain_path new_path = { .domain_path = dp.domain_path @@ -44,6 +50,7 @@ die("%s: out of memory.\n", __func__); domain->ops = &ioat_domain_ops; + iio_domain_set_acpi_name(domain, prefix); struct bus *const bus = alloc_bus(domain); bus->secondary = bus_base; @@ -94,14 +101,16 @@ mem64_limit = mem64_base + CPM_MMIO_SIZE - 1; bus_base = sr->BusBase + CPM_BUS_OFFSET; bus_limit = bus_base + CPM_RESERVED_BUS; - create_ioat_domain(path, bus, bus_base, bus_limit, 0, -1, mem64_base, mem64_limit); + create_ioat_domain(path, bus, bus_base, bus_limit, 0, -1, mem64_base, mem64_limit, + DOMAIN_TYPE_CPM0); /* HQM0 */ mem64_base = mem64_limit + 1; mem64_limit = mem64_base + HQM_MMIO_SIZE - 1; bus_base = sr->BusBase + HQM_BUS_OFFSET; bus_limit = bus_base + HQM_RESERVED_BUS; - create_ioat_domain(path, bus, bus_base, bus_limit, 0, -1, mem64_base, mem64_limit); + create_ioat_domain(path, bus, bus_base, bus_limit, 0, -1, mem64_base, mem64_limit, + DOMAIN_TYPE_HQM0); /* CPM1 (optional) */ mem64_base = mem64_limit + 1; @@ -109,7 +118,8 @@ bus_base = sr->BusBase + CPM1_BUS_OFFSET; bus_limit = bus_base + CPM_RESERVED_BUS; if (bus_limit <= sr->BusLimit) - create_ioat_domain(path, bus, bus_base, bus_limit, 0, -1, mem64_base, mem64_limit); + create_ioat_domain(path, bus, bus_base, bus_limit, 0, -1, mem64_base, mem64_limit, + DOMAIN_TYPE_CPM1); /* HQM1 (optional) */ mem64_base = mem64_limit + 1; @@ -117,7 +127,8 @@ bus_base = sr->BusBase + HQM1_BUS_OFFSET; bus_limit = bus_base + HQM_RESERVED_BUS; if (bus_limit <= sr->BusLimit) - create_ioat_domain(path, bus, bus_base, bus_limit, 0, -1, mem64_base, mem64_limit); + create_ioat_domain(path, bus, bus_base, bus_limit, 0, -1, mem64_base, mem64_limit, + DOMAIN_TYPE_HQM1); /* DINO */ mem64_base = mem64_limit + 1; @@ -125,5 +136,5 @@ bus_base = sr->BusBase; bus_limit = bus_base; create_ioat_domain(path, bus, bus_base, bus_limit, sr->PciResourceMem32Base, sr->PciResourceMem32Limit, - mem64_base, mem64_limit); + mem64_base, mem64_limit, DOMAIN_TYPE_DINO); } -- To view, visit https://review.coreboot.org/c/coreboot/+/80792?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: Ic4cc81d198fb88300394055682a3954bf22db570 Gerrit-Change-Number: 80792 Gerrit-PatchSet: 5 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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged [View Less]

1 0

[S] Change in coreboot[main]: soc/intel/cmn/cse: Simplify logic to get CSE RW version
by Paul Menzel (Code Review) 06 Mar '24

06 Mar '24

Attention is currently required from: Arthur Heymans, Kapil Porwal, Nick Vaccaro. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81051?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: soc/intel/cmn/cse: Simplify logic to get CSE RW version ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/… [View More]

1 0

[L] Change in coreboot[master]: mb/supermicro/x11ssh: Add Supermicro X11SSH-TF
by ghost (Code Review) 06 Mar '24

06 Mar '24

ghost has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32734?usp=email ) Change subject: mb/supermicro/x11ssh: Add Supermicro X11SSH-TF ...................................................................... Patch Set 82: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/32734?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-… [View More]

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp: Unshare UDK binding among Xeon-SP platforms
by Shuo Liu (Code Review) 06 Mar '24

06 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Jincheng Li, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Hello Arthur Heymans, Christian Walter, Jincheng Li, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Tim Chu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81041?usp=email to look at the new patch set (#6). Change subject: soc/intel/xeon_sp: Unshare UDK binding among Xeon-SP platforms ..… [View More]

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp: Share numa.c among Xeon-SP platforms
by Shuo Liu (Code Review) 06 Mar '24

06 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Hello Arthur Heymans, Christian Walter, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Tim Chu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81042?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: … [View More]

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024