coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3043 discussions

[S] Change in coreboot[main]: malloc/memalign: Return NULL if the request is too large
by Patrick Georgi (Code Review) 29 Jan '24

29 Jan '24

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80226?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: malloc/memalign: Return NULL if the request is too large ...................................................................... malloc/memalign: Return NULL if the request is too large It's what this function family is defined to do, we currently don't usually run into the case (see: not too many die() instances going around), it's more useful to try to recover, and the JPEG parser can run into it if the work buffer size exceeds the remaining heap, whereas its sole user (the bootsplash code) knows what to do when seeing a NULL. Use xmalloc() if you want an allocation that either works or dies. tl;dr: That code path isn't usually taken. Right now it crashes. With this patch it _might_ survive. There is a use-case for doing it like that now. Change-Id: I262fbad7daae0ca3aab583fda00665a2592deaa8 Signed-off-by: Patrick Georgi <patrick(a)coreboot.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80226 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Martin L Roth <gaumless(a)gmail.com> Reviewed-by: Eric Lai <ericllai(a)google.com> --- M src/lib/malloc.c M tests/lib/malloc-test.c 2 files changed, 8 insertions(+), 11 deletions(-) Approvals: build bot (Jenkins): Verified Martin L Roth: Looks good to me, approved Eric Lai: Looks good to me, approved diff --git a/src/lib/malloc.c b/src/lib/malloc.c index 052a53e..3029806 100644 --- a/src/lib/malloc.c +++ b/src/lib/malloc.c @@ -44,7 +44,10 @@ p, free_mem_ptr); printk(BIOS_ERR, "but free_mem_end_ptr is %p\n", free_mem_end_ptr); - die("Error! %s: Out of memory (free_mem_ptr >= free_mem_end_ptr)", __func__); + printk(BIOS_ERR, "Error! %s: Out of memory " + "(free_mem_ptr >= free_mem_end_ptr)", + __func__); + return NULL; } MALLOCDBG("%s %p\n", __func__, p); diff --git a/tests/lib/malloc-test.c b/tests/lib/malloc-test.c index 452d74f..f5d528e 100644 --- a/tests/lib/malloc-test.c +++ b/tests/lib/malloc-test.c @@ -34,11 +34,6 @@ TEST_SYMBOL(_heap, _test_heap); TEST_SYMBOL(_eheap, _etest_heap); -void die(const char *msg, ...) -{ - function_called(); -} - static int setup_test(void **state) { free_mem_ptr = &_heap; @@ -56,9 +51,8 @@ static void test_malloc_out_of_memory(void **state) { - /* Expect die() call if out of memory */ - expect_function_call(die); - cb_malloc(TEST_HEAP_SZ); + void *ptr = cb_malloc(TEST_HEAP_SZ); + assert_ptr_equal(ptr, NULL); } static void test_malloc_zero(void **state) @@ -102,8 +96,8 @@ static void test_memalign_out_of_memory(void **state) { - expect_function_call(die); - cb_memalign(16, TEST_HEAP_SZ); + void *ptr = cb_memalign(16, TEST_HEAP_SZ); + assert_ptr_equal(ptr, NULL); } static void test_memalign_zero(void **state) -- To view, visit https://review.coreboot.org/c/coreboot/+/80226?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: I262fbad7daae0ca3aab583fda00665a2592deaa8 Gerrit-Change-Number: 80226 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Brandon Weeks <bweeks(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: malloc/memalign: Return NULL if the request is too large
by Patrick Georgi (Code Review) 29 Jan '24

29 Jan '24

Attention is currently required from: Brandon Weeks, Jakub Czapiga, Julius Werner. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80226?usp=email ) Change subject: malloc/memalign: Return NULL if the request is too large ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/80226/comment/c24914fa_4d0312aa : PS1, Line 18: > nit: Maybe mention that callers that explicitly want the `die()` behavior should be using the `xmall […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/80226?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: I262fbad7daae0ca3aab583fda00665a2592deaa8 Gerrit-Change-Number: 80226 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Brandon Weeks <bweeks(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Brandon Weeks <bweeks(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Mon, 29 Jan 2024 19:12:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: malloc/memalign: Return NULL if the request is too large
by Patrick Georgi (Code Review) 29 Jan '24

29 Jan '24

Attention is currently required from: Brandon Weeks, Jakub Czapiga, Julius Werner, Patrick Georgi. Hello Brandon Weeks, Eric Lai, Jakub Czapiga, Julius Werner, Martin L Roth, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80226?usp=email to look at the new patch set (#3). Change subject: malloc/memalign: Return NULL if the request is too large ...................................................................... malloc/memalign: Return NULL if the request is too large It's what this function family is defined to do, we currently don't usually run into the case (see: not too many die() instances going around), it's more useful to try to recover, and the JPEG parser can run into it if the work buffer size exceeds the remaining heap, whereas its sole user (the bootsplash code) knows what to do when seeing a NULL. Use xmalloc() if you want an allocation that either works or dies. tl;dr: That code path isn't usually taken. Right now it crashes. With this patch it _might_ survive. There is a use-case for doing it like that now. Change-Id: I262fbad7daae0ca3aab583fda00665a2592deaa8 Signed-off-by: Patrick Georgi <patrick(a)coreboot.org> --- M src/lib/malloc.c M tests/lib/malloc-test.c 2 files changed, 8 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/80226/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/80226?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: I262fbad7daae0ca3aab583fda00665a2592deaa8 Gerrit-Change-Number: 80226 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Brandon Weeks <bweeks(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Brandon Weeks <bweeks(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: util/ifdtool: Add new cmdline to enable GPR0 protection
by Subrata Banik (Code Review) 29 Jan '24

29 Jan '24

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80235?usp=email ) Change subject: util/ifdtool: Add new cmdline to enable GPR0 protection ...................................................................... util/ifdtool: Add new cmdline to enable GPR0 protection This patch adds new cmdline `-E` support in the IFD Tool so that users (mostly factory) can enable GPR0 protection. Additionally, perform some code refactoring while adding support for enable GPR0 protection. BUG=b:270275115 TEST=Able to test GPR0 protection on google/rex and google/yahiko. > ifdtool -p mtl -E image.bin -O image.bin_lock --------- GPR0 Protected Range -------------- Start address = 0x00004000 End address = 0x00322fff Change-Id: I27c533ae4109c79299f4e7ff75e750d7cc64280f Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M util/ifdtool/ifdtool.c M util/ifdtool/ifdtool.h 2 files changed, 187 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/80235/1 diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c index 07cc268..0b9bef0 100644 --- a/util/ifdtool/ifdtool.c +++ b/util/ifdtool/ifdtool.c @@ -44,6 +44,8 @@ #define PLATFORM_HAS_10GBE_0_REGION (platform == PLATFORM_DNV) #define PLATFORM_HAS_10GBE_1_REGION (platform == PLATFORM_DNV) +#define CSE_REGION_DATA_PARTITION_OFFSET 0x18 + static int max_regions_from_fdbar(const struct fdbar *fdb); static int ifd_version; @@ -1551,14 +1553,26 @@ write_image(filename, image, size); } -static void disable_gpr0(const char *filename, char *image, int size) +static void print_gpr0_range(uint32_t gprd) { - struct fpsba *fpsba = find_fpsba(image, size); - if (!fpsba) - exit(EXIT_FAILURE); + printf("--------- GPR0 Protected Range --------------\n"); + /* + * Start Address: bit 0-15 of the GPRD represents the protected region start address, + * where bit 0-11 of the start address are assumed to be zero. + */ + printf("Start address = 0x%08x\n", (gprd & 0xffff) << 12); + /* + * End Address: bit 16-30 of the GPRD represents the protected region end address, + * where bit 0-11 of the end address are assumed to be 0xfff. + */ + printf("End address = 0x%08x\n", ((gprd >> 16) & 0x7fff) << 12 | 0xfff); +} +static uint32_t get_gpr0_offset(void) +{ /* Offset expressed as number of 32-bit fields from FPSBA */ - uint32_t gpr0_offset; + uint32_t gpr0_offset = 0xffffffff; + switch (platform) { case PLATFORM_CNL: gpr0_offset = 0x10; @@ -1574,9 +1588,24 @@ gpr0_offset = 0x40; break; default: + break; + } + + return gpr0_offset; +} + +static void disable_gpr0(const char *filename, char *image, int size) +{ + struct fpsba *fpsba = find_fpsba(image, size); + if (!fpsba) + exit(EXIT_FAILURE); + + uint32_t gpr0_offset = get_gpr0_offset(); + if (gpr0_offset == 0xffffffff) { fprintf(stderr, "Disabling GPR0 not supported on this platform\n"); exit(EXIT_FAILURE); } + /* If bit 31 is set then GPR0 protection is enable */ bool gpr0_status = fpsba->pchstrp[gpr0_offset] & 0x80000000; if (!gpr0_status) { @@ -1585,20 +1614,136 @@ } printf("Value at GPRD offset (%d) is 0x%08x\n", gpr0_offset, fpsba->pchstrp[gpr0_offset]); - printf("--------- GPR0 Protected Range --------------\n"); + print_gpr0_range(fpsba->pchstrp[gpr0_offset]); + /* 0 means GPR0 protection is disabled */ + fpsba->pchstrp[gpr0_offset] = 0; + write_image(filename, image, size); +} + +/* + * Helper function to parse the FITC to retrieve the FIT start offset and size. + * FITC is a sub-partition table inside CSE data partition known as FPT. + * + * CSE Region + * |-----> CSE Data Partition Offset + * | |-------> FPT Entry + * | | |-> Sub Partition 1 + * | | |-> Sub Partition 2 + * | | |-> FITC + * | | | | -> FITC Offset + * | | | | -> FITC Length + */ +static void parse_fitc_table(struct cse_fpt *fpt, uint32_t *offset, + size_t *size) +{ + for (size_t index = 1; index <= fpt->count; index++) { + struct cse_fpt_sub_part *fpt_sub_part = (struct cse_fpt_sub_part *)(fpt + index); + if (!strncmp(&fpt_sub_part->signature[0], "FITC", 4)) { + *offset = fpt_sub_part->offset; + *size = fpt_sub_part->length; + return; + } + continue; + } +} + +/* + * Formula to calculate the GPR0 protection range as below: + * Start: CSE Region Base Offset + * End: Till the end of FITC sub-partition + */ +static int calculate_gpr0_range(char *image, int size, + uint32_t *gpr0_start, uint32_t *gpr0_end) +{ + struct frba *frba = find_frba(image, size); + if (!frba) + return -1; + + struct region region = get_region(frba, REGION_ME); + if (region.size <= 0xfff) { + fprintf(stderr, "Region %s is disabled in target\n", + region_name(REGION_ME)); + return -1; + } + + /* CSE Region Start */ + uint32_t cse_region_start = region.base; + /* CSE Data Partition Offset (at 0x18 offset of CSE region) */ + uint32_t data_part_offset = *((uint32_t *)(image + cse_region_start + + CSE_REGION_DATA_PARTITION_OFFSET)); + /* Start reading the CSE Data Partition Table, also known as FPT */ + uint32_t data_part_start = data_part_offset + cse_region_start; + + uint32_t fitc_region_start; + size_t fitc_region_size; + /* + * FPT holds entry for own FPT data structure also bunch of sub-partitions. + * `FITC` is one of such sub-partition entry. + */ + parse_fitc_table(((struct cse_fpt *)(image + data_part_start)), + &fitc_region_start, &fitc_region_size); + + /* + * GPR0 protection is configured to the following range: + * start: CSE region base offset + * end: Till the end of FITC sub-partition (i.e. CSE region + data partition offset + + * FITC sub partition offset + FITC sub partition size) + */ + *gpr0_start = cse_region_start; + *gpr0_end = (cse_region_start + data_part_offset + + fitc_region_start + fitc_region_size) - 1; + + return 0; +} + +static void enable_gpr0(const char *filename, char *image, int size) +{ + struct fpsba *fpsba = find_fpsba(image, size); + if (!fpsba) + exit(EXIT_FAILURE); + + uint32_t gpr0_offset = get_gpr0_offset(); + if (gpr0_offset == 0xffffffff) { + fprintf(stderr, "Enabling GPR0 not supported on this platform\n"); + exit(EXIT_FAILURE); + } + + /* If bit 31 is set then GPR0 protection is enable */ + bool gpr0_status = fpsba->pchstrp[gpr0_offset] & 0x80000000; + if (gpr0_status) { + printf("GPR0 protection is already enabled\n"); + print_gpr0_range(fpsba->pchstrp[gpr0_offset]); + return; + } + + uint32_t gpr0_range_start, gpr0_range_end; + + if (calculate_gpr0_range(image, size, &gpr0_range_start, &gpr0_range_end)) + exit(EXIT_FAILURE); + + union gprd_info { + struct bit_field { + uint32_t start:16; + uint32_t end:15; + uint32_t enable:1; + } data; + uint32_t value; + } gprd; + /* * Start Address: bit 0-15 of the GPRD represents the protected region start address, * where bit 0-11 of the start address are assumed to be zero. */ - printf("Start address = 0x%08x\n", (fpsba->pchstrp[gpr0_offset] & 0xffff) << 12); + gprd.data.start = (gpr0_range_start >> 12) & 0xffff; /* * End Address: bit 16-30 of the GPRD represents the protected region end address, * where bit 0-11 of the end address are assumed to be 0xfff. */ - printf("End address = 0x%08x\n", - ((fpsba->pchstrp[gpr0_offset] >> 16) & 0x7fff) << 12 | 0xfff); - /* 0 means GPR0 protection is disabled */ - fpsba->pchstrp[gpr0_offset] = 0; + gprd.data.end = (gpr0_range_end >> 12) & 0x7fff; + gprd.data.enable = 1; + + fpsba->pchstrp[gpr0_offset] = gprd.value; + print_gpr0_range(gprd.value); write_image(filename, image, size); } @@ -1942,6 +2087,7 @@ " -r | --read Enable CPU/BIOS read access for ME region\n" " -u | --unlock Unlock firmware descriptor and ME region\n" " -g | --gpr0-disable Disable GPR0 (Global Protected Range) register\n" + " -E | --gpr0-enable Enable GPR0 (Global Protected Range) register\n" " -M | --altmedisable <0|1> Set the MeDisable and AltMeDisable (or HAP for skylake or newer platform)\n" " bits to disable ME\n" " -p | --platform Add platform-specific quirks\n" @@ -1975,7 +2121,7 @@ int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0, mode_validate = 0; int mode_layout = 0, mode_newlayout = 0, mode_density = 0, mode_setstrap = 0; int mode_read = 0, mode_altmedisable = 0, altmedisable = 0, mode_fmap_template = 0; - int mode_gpr0_disable = 0; + int mode_gpr0_disable = 0, mode_gpr0_enable = 0;; char *region_type_string = NULL, *region_fname = NULL; const char *layout_fname = NULL; char *new_filename = NULL; @@ -2002,6 +2148,7 @@ {"read", 0, NULL, 'r'}, {"unlock", 0, NULL, 'u'}, {"gpr0-disable", 0, NULL, 'g'}, + {"gpr0-enable", 0, NULL, 'E'}, {"version", 0, NULL, 'v'}, {"help", 0, NULL, 'h'}, {"platform", 0, NULL, 'p'}, @@ -2011,7 +2158,7 @@ {0, 0, 0, 0} }; - while ((opt = getopt_long(argc, argv, "S:V:df:F:D:C:M:xi:n:O:s:p:elrugvth?", + while ((opt = getopt_long(argc, argv, "S:V:df:F:D:C:M:xi:n:O:s:p:elrugEvth?", long_options, &option_index)) != EOF) { switch (opt) { case 'd': @@ -2217,6 +2364,9 @@ case 'g': mode_gpr0_disable = 1; break; + case 'E': + mode_gpr0_enable = 1; + break; case 'p': if (!strcmp(optarg, "aplk")) { platform = PLATFORM_APL; @@ -2270,7 +2420,7 @@ if ((mode_dump + mode_layout + mode_fmap_template + mode_extract + mode_inject + mode_setstrap + mode_newlayout + (mode_spifreq | mode_em100 | mode_unlocked | mode_locked) + mode_altmedisable + mode_validate + - mode_gpr0_disable) > 1) { + (mode_gpr0_disable | mode_gpr0_enable)) > 1) { fprintf(stderr, "You may not specify more than one mode.\n\n"); fprintf(stderr, "run '%s -h' for usage\n", argv[0]); exit(EXIT_FAILURE); @@ -2279,7 +2429,7 @@ if ((mode_dump + mode_layout + mode_fmap_template + mode_extract + mode_inject + mode_setstrap + mode_newlayout + mode_spifreq + mode_em100 + mode_locked + mode_unlocked + mode_density + mode_altmedisable + - mode_validate + mode_gpr0_disable) == 0) { + mode_validate + (mode_gpr0_disable | mode_gpr0_enable)) == 0) { fprintf(stderr, "You need to specify a mode.\n\n"); fprintf(stderr, "run '%s -h' for usage\n", argv[0]); exit(EXIT_FAILURE); @@ -2379,6 +2529,9 @@ if (mode_gpr0_disable) disable_gpr0(new_filename, image, size); + if (mode_gpr0_enable) + enable_gpr0(new_filename, image, size); + if (mode_setstrap) { struct fpsba *fpsba = find_fpsba(image, size); const struct fdbar *fdb = find_fd(image, size); diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h index 219634b..79fe0ab 100644 --- a/util/ifdtool/ifdtool.h +++ b/util/ifdtool/ifdtool.h @@ -208,3 +208,22 @@ const char *filename; const char *fmapname; }; + +struct cse_fpt { + const char signature[4]; + uint32_t count; + uint8_t header_version; + uint8_t entry_version; + uint8_t length; + uint8_t crc; + uint8_t reserved[20]; +}; + +struct cse_fpt_sub_part { + const char signature[4]; + uint32_t reserved_1; + uint32_t offset; + uint32_t length; + uint8_t reserved_2[12]; + uint32_t flags; +}; -- To view, visit https://review.coreboot.org/c/coreboot/+/80235?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: I27c533ae4109c79299f4e7ff75e750d7cc64280f Gerrit-Change-Number: 80235 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: mb/google/rex/variants/deku: Add H58G66BK7BX067 to RAM ID table
by Eran Mitrani (Code Review) 29 Jan '24

29 Jan '24

Attention is currently required from: Derek Huang, Dinesh Gehlot, Jakub Czapiga, Kevin Chiu, Tarun, Tony Huang. Eran Mitrani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80230?usp=email ) Change subject: mb/google/rex/variants/deku: Add H58G66BK7BX067 to RAM ID table ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80230?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: I31538988d1329d9e2f45d862eb0ae05c0d6a179e Gerrit-Change-Number: 80230 Gerrit-PatchSet: 2 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(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: Kevin Chiu <kevin.chiu.17802(a)gmail.com> Gerrit-Reviewer: Kevin Chiu <kevin.chiu(a)quanta.corp-partner.google.com> 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-Attention: Kevin Chiu <kevin.chiu(a)quanta.corp-partner.google.com> Gerrit-Attention: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Mon, 29 Jan 2024 19:00:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: device/device.h: Fix spelling mistake
by Felix Held (Code Review) 29 Jan '24

29 Jan '24

Attention is currently required from: Arthur Heymans. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80234?usp=email ) Change subject: device/device.h: Fix spelling mistake ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80234?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: I66ae5000f6f5c0e5bfe42bdfbbbcedec6df0c520 Gerrit-Change-Number: 80234 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 29 Jan 2024 18:25:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: device/device.h: Rename busses for clarity
by Felix Held (Code Review) 29 Jan '24

29 Jan '24

Attention is currently required from: Angel Pons, Arthur Heymans, Caveh Jalali, Chen, Gang C, Christian Walter, Cliff Huang, Dinesh Gehlot, Eran Mitrani, Eric Lai, Forest Mittelberg, Fred Reitberger, Jakub Czapiga, Jason Glenesk, Jason Nien, Jeff Daly, Jincheng Li, Johnny Lin, Jérémy Compostella, Kapil Porwal, Lance Zhao, Martin L Roth, Martin Roth, Matt DeVillier, Michał Żygowski, Nick Vaccaro, Piotr Król, Sean Rhodes, Shuo Liu, Subrata Banik, Tarun, Tim Chu, Tim Wawrzynczak, Vanessa Eusebio, Varshit Pandya, Werner Zeh. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78330?usp=email ) Change subject: device/device.h: Rename busses for clarity ...................................................................... Patch Set 23: (1 comment) Patchset: PS23: would be good if you can rebase this one to see if it still works, since i'm not sure if there have been any changes in the meantime that would result in breakage in combination with this patch. when that's done, i'll +2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78330?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: I80a81b6b8606e450ff180add9439481ec28c2420 Gerrit-Change-Number: 78330 Gerrit-PatchSet: 23 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com> Gerrit-Comment-Date: Mon, 29 Jan 2024 18:25:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: device/device.h: Drop multiple links
by Felix Held (Code Review) 29 Jan '24

29 Jan '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78328?usp=email ) Change subject: device/device.h: Drop multiple links ...................................................................... device/device.h: Drop multiple links Multiple links are unused throughout the tree and make the code more confusing as an iteration over all busses is needed to get downstream devices. This also not done consistently e.g. the allocator does not care about multiple links on busses. A better way of dealing multiple links below a device is to feature dummy devices with each their respective bus. This drops the sconfig capability to declare the same device multiple times which was previously used to declare multiple links. Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Change-Id: Iab6fe269faef46ae77ed1ea425440cf5c7dbd49b Reviewed-on: https://review.coreboot.org/c/coreboot/+/78328 Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Jincheng Li <jincheng.li(a)intel.com> Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> --- M src/arch/x86/mpspec.c M src/device/device.c M src/device/device_util.c M src/device/pci_device.c M src/device/root_device.c M src/include/device/device.h M src/northbridge/amd/pi/00730F01/northbridge.c M src/soc/amd/common/block/acpi/ivrs.c M src/soc/amd/common/block/lpc/lpc.c M src/soc/amd/stoneyridge/northbridge.c M src/soc/intel/common/block/lpc/lpc.c M src/southbridge/amd/pi/hudson/lpc.c M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.y 16 files changed, 187 insertions(+), 355 deletions(-) Approvals: Felix Held: Looks good to me, approved Lean Sheng Tan: Looks good to me, approved build bot (Jenkins): Verified Jincheng Li: Looks good to me, but someone else must approve diff --git a/src/arch/x86/mpspec.c b/src/arch/x86/mpspec.c index 7744f68..3ddee2f 100644 --- a/src/arch/x86/mpspec.c +++ b/src/arch/x86/mpspec.c @@ -282,48 +282,46 @@ int srcbus; int slot; - struct bus *link; unsigned char dstirq_x[4]; - for (link = dev->link_list; link; link = link->next) { + if (!dev->link_list) + return; - child = link->children; - srcbus = link->secondary; + child = dev->link_list->children; + srcbus = dev->link_list->secondary; - while (child) { - if (child->path.type != DEVICE_PATH_PCI) - goto next; + while (child) { + if (child->path.type != DEVICE_PATH_PCI) + goto next; - slot = (child->path.pci.devfn >> 3); - /* round pins */ + slot = (child->path.pci.devfn >> 3); + /* round pins */ + for (i = 0; i < 4; i++) + dstirq_x[i] = dstirq[(i + slot) % 4]; + + if ((child->class >> 16) != PCI_BASE_CLASS_BRIDGE) { + /* pci device */ + printk(BIOS_DEBUG, "route irq: %s\n", + dev_path(child)); for (i = 0; i < 4; i++) - dstirq_x[i] = dstirq[(i + slot) % 4]; - - if ((child->class >> 16) != PCI_BASE_CLASS_BRIDGE) { - /* pci device */ - printk(BIOS_DEBUG, "route irq: %s\n", - dev_path(child)); - for (i = 0; i < 4; i++) - smp_write_intsrc(mc, irqtype, irqflag, - srcbus, (slot<<2)|i, dstapic, - dstirq_x[i]); - goto next; - } - - switch (child->class>>8) { - case PCI_CLASS_BRIDGE_PCI: - case PCI_CLASS_BRIDGE_PCMCIA: - case PCI_CLASS_BRIDGE_CARDBUS: - printk(BIOS_DEBUG, "route irq bridge: %s\n", - dev_path(child)); - smp_write_intsrc_pci_bridge(mc, irqtype, - irqflag, child, dstapic, dstirq_x); - } - -next: - child = child->sibling; + smp_write_intsrc(mc, irqtype, irqflag, + srcbus, (slot<<2)|i, dstapic, + dstirq_x[i]); + goto next; } + switch (child->class>>8) { + case PCI_CLASS_BRIDGE_PCI: + case PCI_CLASS_BRIDGE_PCMCIA: + case PCI_CLASS_BRIDGE_CARDBUS: + printk(BIOS_DEBUG, "route irq bridge: %s\n", + dev_path(child)); + smp_write_intsrc_pci_bridge(mc, irqtype, + irqflag, child, dstapic, dstirq_x); + } + +next: + child = child->sibling; } } @@ -478,17 +476,16 @@ memset(buses, 0, sizeof(buses)); for (dev = all_devices; dev; dev = dev->next) { - struct bus *bus; - for (bus = dev->link_list; bus; bus = bus->next) { - if (bus->secondary > 255) { - printk(BIOS_ERR, - "A bus claims to have a bus ID > 255?!? Aborting"); - return; - } - buses[bus->secondary] = 1; - if (highest < bus->secondary) - highest = bus->secondary; + struct bus *bus = dev->link_list; + if (!bus) + continue; + if (bus->secondary > 255) { + printk(BIOS_ERR, "A bus claims to have a bus ID > 255?!? Aborting\n"); + return; } + buses[bus->secondary] = 1; + if (highest < bus->secondary) + highest = bus->secondary; } for (i = 0; i <= highest; i++) { if (buses[i]) { diff --git a/src/device/device.c b/src/device/device.c index d877e14..743a3af 100644 --- a/src/device/device.c +++ b/src/device/device.c @@ -187,13 +187,11 @@ { struct device *curdev; - printk(BIOS_SPEW, "%s %s segment group %d bus %d link: %d\n", dev_path(bus->dev), - __func__, bus->segment_group, bus->secondary, bus->link_num); + printk(BIOS_SPEW, "%s %s segment group %d bus %d\n", dev_path(bus->dev), + __func__, bus->segment_group, bus->secondary); /* Walk through all devices and find which resources they need. */ for (curdev = bus->children; curdev; curdev = curdev->sibling) { - struct bus *link; - if (!curdev->enabled) continue; @@ -207,12 +205,12 @@ curdev->ops->read_resources(curdev); /* Read in the resources behind the current device's links. */ - for (link = curdev->link_list; link; link = link->next) - read_resources(link); + if (curdev->link_list) + read_resources(curdev->link_list); } post_log_clear(); - printk(BIOS_SPEW, "%s %s segment group %d bus %d link: %d done\n", - dev_path(bus->dev), __func__, bus->segment_group, bus->secondary, bus->link_num); + printk(BIOS_SPEW, "%s %s segment group %d bus %d done\n", + dev_path(bus->dev), __func__, bus->segment_group, bus->secondary); } struct device *vga_pri = NULL; @@ -301,8 +299,8 @@ { struct device *curdev; - printk(BIOS_SPEW, "%s %s, segment group %d bus %d link: %d\n", - dev_path(bus->dev), __func__, bus->segment_group, bus->secondary, bus->link_num); + printk(BIOS_SPEW, "%s %s, segment group %d bus %d\n", + dev_path(bus->dev), __func__, bus->segment_group, bus->secondary); for (curdev = bus->children; curdev; curdev = curdev->sibling) { if (!curdev->enabled || !curdev->resource_list) @@ -317,8 +315,8 @@ curdev->ops->set_resources(curdev); } post_log_clear(); - printk(BIOS_SPEW, "%s %s, segment group %d bus %d link: %d done\n", - dev_path(bus->dev), __func__, bus->segment_group, bus->secondary, bus->link_num); + printk(BIOS_SPEW, "%s %s, segment group %d bus %d done\n", + dev_path(bus->dev), __func__, bus->segment_group, bus->secondary); } /** @@ -336,7 +334,6 @@ static void enable_resources(struct bus *link) { struct device *dev; - struct bus *c_link; for (dev = link->children; dev; dev = dev->sibling) { if (dev->enabled && dev->ops && dev->ops->enable_resources) { @@ -346,8 +343,8 @@ } for (dev = link->children; dev; dev = dev->sibling) { - for (c_link = dev->link_list; c_link; c_link = c_link->next) - enable_resources(c_link); + if (dev->link_list) + enable_resources(dev->link_list); } post_log_clear(); } @@ -394,17 +391,15 @@ do_scan_bus = 1; while (do_scan_bus) { - struct bus *link; + struct bus *link = busdev->link_list; busdev->ops->scan_bus(busdev); do_scan_bus = 0; - for (link = busdev->link_list; link; link = link->next) { - if (link->reset_needed) { - if (reset_bus(link)) - do_scan_bus = 1; - else - busdev->bus->reset_needed = 1; - } - } + if (!link || !link->reset_needed) + continue; + if (reset_bus(link)) + do_scan_bus = 1; + else + busdev->bus->reset_needed = 1; } scan_time = stopwatch_duration_msecs(&sw); @@ -523,13 +518,11 @@ */ void dev_enable(void) { - struct bus *link; - printk(BIOS_INFO, "Enabling resources...\n"); /* Now enable everything. */ - for (link = dev_root.link_list; link; link = link->next) - enable_resources(link); + if (dev_root.link_list) + enable_resources(dev_root.link_list); printk(BIOS_INFO, "done.\n"); } @@ -553,8 +546,7 @@ long init_time; if (dev->path.type == DEVICE_PATH_I2C) { - printk(BIOS_DEBUG, "smbus: %s[%d]->", - dev_path(dev->bus->dev), dev->bus->link_num); + printk(BIOS_DEBUG, "smbus: %s->", dev_path(dev->bus->dev)); } printk(BIOS_DEBUG, "%s init\n", dev_path(dev)); @@ -572,7 +564,6 @@ static void init_link(struct bus *link) { struct device *dev; - struct bus *c_link; for (dev = link->children; dev; dev = dev->sibling) { post_code(POSTCODE_BS_DEV_INIT); @@ -580,10 +571,9 @@ init_dev(dev); } - for (dev = link->children; dev; dev = dev->sibling) { - for (c_link = dev->link_list; c_link; c_link = c_link->next) - init_link(c_link); - } + for (dev = link->children; dev; dev = dev->sibling) + if (dev->link_list) + init_link(dev->link_list); } /** @@ -594,16 +584,14 @@ */ void dev_initialize(void) { - struct bus *link; - printk(BIOS_INFO, "Initializing devices...\n"); /* First call the mainboard init. */ init_dev(&dev_root); /* Now initialize everything. */ - for (link = dev_root.link_list; link; link = link->next) - init_link(link); + if (dev_root.link_list) + init_link(dev_root.link_list); post_log_clear(); printk(BIOS_INFO, "Devices initialized\n"); @@ -633,15 +621,13 @@ static void final_link(struct bus *link) { struct device *dev; - struct bus *c_link; for (dev = link->children; dev; dev = dev->sibling) final_dev(dev); - for (dev = link->children; dev; dev = dev->sibling) { - for (c_link = dev->link_list; c_link; c_link = c_link->next) - final_link(c_link); - } + for (dev = link->children; dev; dev = dev->sibling) + if (dev->link_list) + final_link(dev->link_list); } /** * Finalize all devices in the global device tree. @@ -651,16 +637,13 @@ */ void dev_finalize(void) { - struct bus *link; - printk(BIOS_INFO, "Finalize devices...\n"); /* First call the mainboard finalize. */ final_dev(&dev_root); /* Now finalize everything. */ - for (link = dev_root.link_list; link; link = link->next) - final_link(link); + final_link(dev_root.link_list); printk(BIOS_INFO, "Devices finalized\n"); } diff --git a/src/device/device_util.c b/src/device/device_util.c index 260c9b4..ea4ef27 100644 --- a/src/device/device_util.c +++ b/src/device/device_util.c @@ -560,16 +560,9 @@ /* If it is a subtractive resource recurse. */ if (res->flags & IORESOURCE_SUBTRACTIVE) { - struct bus *subbus; - for (subbus = curdev->link_list; subbus; - subbus = subbus->next) - if (subbus->link_num - == IOINDEX_SUBTRACTIVE_LINK(res->index)) - break; - if (!subbus) /* Why can subbus be NULL? */ - break; - search_bus_resources(subbus, type_mask, type, - search, gp); + if (curdev->link_list) + search_bus_resources(curdev->link_list, type_mask, type, + search, gp); continue; } search(gp, curdev, res); @@ -624,9 +617,8 @@ struct device *child; for (child = bus->children; child; child = child->sibling) { - struct bus *link; - for (link = child->link_list; link; link = link->next) - disable_children(link); + if (child->link_list) + disable_children(child->link_list); dev_set_enabled(child, 0); } } @@ -637,7 +629,6 @@ */ bool dev_is_active_bridge(struct device *dev) { - struct bus *link; struct device *child; if (!dev || !dev->enabled) @@ -646,71 +637,20 @@ if (!dev->link_list || !dev->link_list->children) return 0; - for (link = dev->link_list; link; link = link->next) { - for (child = link->children; child; child = child->sibling) { - if (child->path.type == DEVICE_PATH_NONE) - continue; - - if (child->enabled) - return 1; - } + for (child = dev->link_list->children; child; child = child->sibling) { + if (child->path.type == DEVICE_PATH_NONE) + continue; + if (child->enabled) + return 1; } return 0; } -/** - * Ensure the device has a minimum number of bus links. - * - * @param dev The device to add links to. - * @param total_links The minimum number of links to have. - */ -void add_more_links(struct device *dev, unsigned int total_links) -{ - struct bus *link, *last = NULL; - int link_num = -1; - - for (link = dev->link_list; link; link = link->next) { - if (link_num < link->link_num) - link_num = link->link_num; - last = link; - } - - if (last) { - int links = total_links - (link_num + 1); - if (links > 0) { - link = malloc(links * sizeof(*link)); - if (!link) - die("Couldn't allocate more links!\n"); - memset(link, 0, links * sizeof(*link)); - last->next = link; - } else { - /* No more links to add */ - return; - } - } else { - link = malloc(total_links * sizeof(*link)); - if (!link) - die("Couldn't allocate more links!\n"); - memset(link, 0, total_links * sizeof(*link)); - dev->link_list = link; - } - - for (link_num = link_num + 1; link_num < total_links; link_num++) { - link->link_num = link_num; - link->dev = dev; - link->next = link + 1; - last = link; - link = link->next; - } - last->next = NULL; -} - static void resource_tree(const struct device *root, int debug_level, int depth) { int i = 0; struct device *child; - struct bus *link; struct resource *res; char indent[30]; /* If your tree has more levels, it's wrong. */ @@ -732,10 +672,11 @@ res->index); } - for (link = root->link_list; link; link = link->next) { - for (child = link->children; child; child = child->sibling) - resource_tree(child, debug_level, depth + 1); - } + if (!root->link_list) + return; + + for (child = root->link_list->children; child; child = child->sibling) + resource_tree(child, debug_level, depth + 1); } void print_resource_tree(const struct device *root, int debug_level, @@ -760,7 +701,6 @@ char depth_str[20]; int i; struct device *sibling; - struct bus *link; for (i = 0; i < depth; i++) depth_str[i] = ' '; @@ -769,11 +709,11 @@ printk(debug_level, "%s%s: enabled %d\n", depth_str, dev_path(dev), dev->enabled); - for (link = dev->link_list; link; link = link->next) { - for (sibling = link->children; sibling; - sibling = sibling->sibling) - show_devs_tree(sibling, debug_level, depth + 1); - } + if (!dev->link_list) + return; + + for (sibling = dev->link_list->children; sibling; sibling = sibling->sibling) + show_devs_tree(sibling, debug_level, depth + 1); } void show_all_devs_tree(int debug_level, const char *msg) diff --git a/src/device/pci_device.c b/src/device/pci_device.c index acdad42..24b9523 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -713,16 +713,13 @@ void pci_dev_set_resources(struct device *dev) { struct resource *res; - struct bus *bus; u8 line; for (res = dev->resource_list; res; res = res->next) pci_set_resource(dev, res); - for (bus = dev->link_list; bus; bus = bus->next) { - if (bus->children) - assign_resources(bus); - } + if (dev->link_list && dev->link_list->children) + assign_resources(dev->link_list); /* Set a default latency timer. */ pci_write_config8(dev, PCI_LATENCY_TIMER, 0x40); diff --git a/src/device/root_device.c b/src/device/root_device.c index d8edbd5..54e82ea 100644 --- a/src/device/root_device.c +++ b/src/device/root_device.c @@ -38,31 +38,27 @@ void enable_static_devices(struct device *bus) { struct device *child; - struct bus *link; - for (link = bus->link_list; link; link = link->next) { - for (child = link->children; child; child = child->sibling) { - enable_static_device(child); - } - } + if (!bus->link_list) + return; + + for (child = bus->link_list->children; child; child = child->sibling) + enable_static_device(child); } void scan_generic_bus(struct device *bus) { struct device *child; - struct bus *link; static int bus_max = 0; printk(BIOS_SPEW, "%s for %s\n", __func__, dev_path(bus)); - for (link = bus->link_list; link; link = link->next) { + if (bus->link_list) { + bus->link_list->secondary = ++bus_max; - link->secondary = ++bus_max; - - for (child = link->children; child; child = child->sibling) { + for (child = bus->link_list->children; child; child = child->sibling) { enable_static_device(child); - printk(BIOS_DEBUG, "bus: %s[%d]->", dev_path(child->bus->dev), - child->bus->link_num); + printk(BIOS_DEBUG, "bus: %s->", dev_path(child->bus->dev)); } } @@ -86,14 +82,12 @@ */ void scan_static_bus(struct device *bus) { - struct bus *link; - printk(BIOS_SPEW, "%s for %s\n", __func__, dev_path(bus)); enable_static_devices(bus); - for (link = bus->link_list; link; link = link->next) - scan_bridges(link); + if (bus->link_list) + scan_bridges(bus->link_list); printk(BIOS_SPEW, "%s for %s done\n", __func__, dev_path(bus)); } diff --git a/src/include/device/device.h b/src/include/device/device.h index 13bc8dc..b7917e3 100644 --- a/src/include/device/device.h +++ b/src/include/device/device.h @@ -4,11 +4,13 @@ #define DEVICE_H -#include <device/resource.h> +#include <console/console.h> #include <device/path.h> #include <device/pci_type.h> +#include <device/resource.h> #include <smbios.h> #include <static.h> +#include <stdlib.h> #include <types.h> struct fw_config; @@ -78,10 +80,8 @@ struct bus { DEVTREE_CONST struct device *dev; /* This bridge device */ DEVTREE_CONST struct device *children; /* devices behind this bridge */ - DEVTREE_CONST struct bus *next; /* The next bridge on this device */ unsigned int bridge_ctrl; /* Bridge control register */ uint16_t bridge_cmd; /* Bridge command register */ - unsigned char link_num; /* The index of this link */ uint16_t secondary; /* secondary bus number */ uint16_t subordinate; /* subordinate bus number */ uint16_t max_subordinate; /* max subordinate bus number */ @@ -193,7 +193,6 @@ void dev_set_enabled(struct device *dev, int enable); void disable_children(struct bus *bus); bool dev_is_active_bridge(struct device *dev); -void add_more_links(struct device *dev, unsigned int total_links); bool is_dev_enabled(const struct device *const dev); bool is_devfn_enabled(unsigned int devfn); bool is_cpu(const struct device *cpu); diff --git a/src/northbridge/amd/pi/00730F01/northbridge.c b/src/northbridge/amd/pi/00730F01/northbridge.c index a4409fb..5e82ccd 100644 --- a/src/northbridge/amd/pi/00730F01/northbridge.c +++ b/src/northbridge/amd/pi/00730F01/northbridge.c @@ -235,7 +235,6 @@ unsigned long *current, uint16_t *ivhd_length) { struct device *sibling; - struct bus *link; if (!root_level) { root_level = malloc(sizeof(int8_t)); @@ -254,11 +253,11 @@ } } - for (link = dev->link_list; link; link = link->next) - for (sibling = link->children; sibling; sibling = - sibling->sibling) + if (dev->link_list) { + for (sibling = dev->link_list->children; sibling; sibling = sibling->sibling) add_ivhd_device_entries(dev, sibling, depth + 1, depth, root_level, current, ivhd_length); + } free(root_level); } diff --git a/src/soc/amd/common/block/acpi/ivrs.c b/src/soc/amd/common/block/acpi/ivrs.c index 9d0ece1..57b5974 100644 --- a/src/soc/amd/common/block/acpi/ivrs.c +++ b/src/soc/amd/common/block/acpi/ivrs.c @@ -140,7 +140,6 @@ unsigned long *current, uint16_t nb_bus) { struct device *sibling; - struct bus *link; if (!root_level) return; @@ -155,11 +154,11 @@ ivrs_add_device_or_bridge(parent, dev, current); } - for (link = dev->link_list; link; link = link->next) - for (sibling = link->children; sibling; sibling = - sibling->sibling) - add_ivhd_device_entries(dev, sibling, depth + 1, depth, root_level, - current, nb_bus); + if (!dev->link_list) + return; + for (sibling = dev->link_list->children; sibling; sibling = sibling->sibling) + add_ivhd_device_entries(dev, sibling, depth + 1, depth, root_level, current, + nb_bus); } static unsigned long acpi_ivhd_misc(unsigned long current, struct device *dev) diff --git a/src/soc/amd/common/block/lpc/lpc.c b/src/soc/amd/common/block/lpc/lpc.c index 4e81316..c076361 100644 --- a/src/soc/amd/common/block/lpc/lpc.c +++ b/src/soc/amd/common/block/lpc/lpc.c @@ -278,20 +278,20 @@ static void lpc_enable_children_resources(struct device *dev) { - struct bus *link; struct device *child; - for (link = dev->link_list; link; link = link->next) { - for (child = link->children; child; child = child->sibling) { - if (!child->enabled) - continue; - if (child->path.type != DEVICE_PATH_PNP) - continue; - if (CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) - configure_child_espi_windows(child); - else - configure_child_lpc_windows(dev, child); - } + if (!dev->link_list) + return; + + for (child = dev->link_list->children; child; child = child->sibling) { + if (!child->enabled) + continue; + if (child->path.type != DEVICE_PATH_PNP) + continue; + if (CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) + configure_child_espi_windows(child); + else + configure_child_lpc_windows(dev, child); } } diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index deebfb3..b95c4cd 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -52,16 +52,9 @@ static void create_vga_resource(struct device *dev) { - struct bus *link; - - /* find out which link the VGA card is connected, - * we only deal with the 'first' vga card */ - for (link = dev->link_list ; link ; link = link->next) - if (link->bridge_ctrl & PCI_BRIDGE_CTL_VGA) - break; - - /* no VGA card installed */ - if (link == NULL) + if (!dev->link_list) + return; + if (!(dev->link_list->bridge_ctrl & PCI_BRIDGE_CTL_VGA)) return; printk(BIOS_DEBUG, "VGA: %s has VGA device\n", dev_path(dev)); @@ -71,14 +64,11 @@ static void set_resources(struct device *dev) { - struct bus *bus; - /* do we need this? */ create_vga_resource(dev); - for (bus = dev->link_list ; bus ; bus = bus->next) - if (bus->children) - assign_resources(bus); + if (dev->link_list && dev->link_list->children) + assign_resources(dev->link_list); } static void northbridge_init(struct device *dev) diff --git a/src/soc/intel/common/block/lpc/lpc.c b/src/soc/intel/common/block/lpc/lpc.c index 6d3b90f..994ef9b 100644 --- a/src/soc/intel/common/block/lpc/lpc.c +++ b/src/soc/intel/common/block/lpc/lpc.c @@ -100,13 +100,13 @@ */ static void pch_lpc_set_child_resources(struct device *dev) { - struct bus *link; struct device *child; - for (link = dev->link_list; link; link = link->next) { - for (child = link->children; child; child = child->sibling) - pch_lpc_loop_resources(child); - } + if (!dev->link_list) + return; + + for (child = dev->link_list->children; child; child = child->sibling) + pch_lpc_loop_resources(child); } static void pch_lpc_set_resources(struct device *dev) diff --git a/src/southbridge/amd/pi/hudson/lpc.c b/src/southbridge/amd/pi/hudson/lpc.c index 0a4e038..266d75c 100644 --- a/src/southbridge/amd/pi/hudson/lpc.c +++ b/src/southbridge/amd/pi/hudson/lpc.c @@ -134,7 +134,6 @@ */ static void hudson_lpc_enable_childrens_resources(struct device *dev) { - struct bus *link; u32 reg, reg_x; int var_num = 0; u16 reg_var[3]; @@ -171,12 +170,10 @@ reg_var[1] = pci_read_config16(dev, 0x66); reg_var[0] = pci_read_config16(dev, 0x64); - for (link = dev->link_list; link; link = link->next) { - struct device *child; - for (child = link->children; child; - child = child->sibling) { - if (child->enabled - && (child->path.type == DEVICE_PATH_PNP)) { + struct device *child; + if (dev->link_list) { + for (child = dev->link_list->children; child; child = child->sibling) { + if (child->enabled && (child->path.type == DEVICE_PATH_PNP)) { struct resource *res; for (res = child->resource_list; res; res = res->next) { u32 base, end; /* don't need long long */ diff --git a/util/sconfig/main.c b/util/sconfig/main.c index dd26e07..533c9fb 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -81,7 +81,6 @@ static struct chip_instance mainboard_instance; static struct bus base_root_bus = { - .id = 0, .dev = &base_root_dev, }; @@ -95,7 +94,6 @@ }; static struct bus chipset_root_bus = { - .id = 0, .dev = &chipset_root_dev, }; @@ -109,7 +107,6 @@ }; static struct bus override_root_bus = { - .id = 0, .dev = &override_root_dev, }; @@ -723,30 +720,13 @@ bus->dev->ops_id = ops_id; } -/* - * Allocate a new bus for the provided device. - * - If this is the first bus being allocated under this device, then its id - * is set to 0 and bus and last_bus are pointed to the newly allocated bus. - * - If this is not the first bus under this device, then its id is set to 1 - * plus the id of last bus and newly allocated bus is added to the list of - * buses under the device. last_bus is updated to point to the newly - * allocated bus. - */ +/* Allocate a new bus for the provided device. */ static void alloc_bus(struct device *dev) { struct bus *bus = S_ALLOC(sizeof(*bus)); bus->dev = dev; - - if (dev->last_bus == NULL) { - bus->id = 0; - dev->bus = bus; - } else { - bus->id = dev->last_bus->id + 1; - dev->last_bus->next_bus = bus; - } - - dev->last_bus = bus; + dev->bus = bus; } /* @@ -814,14 +794,15 @@ if (parent->alias && !strcmp(parent->alias, alias)) return parent; - const struct bus *bus; - for (bus = parent->bus; bus; bus = bus->next_bus) { - const struct device *child; - for (child = bus->children; child; child = child->sibling) { - const struct device *const ret = find_alias(child, alias); - if (ret) - return ret; - } + const struct bus *bus = parent->bus; + if (!bus) + return NULL; + + const struct device *child; + for (child = bus->children; child; child = child->sibling) { + const struct device *const ret = find_alias(child, alias); + if (ret) + return ret; } return NULL; @@ -834,11 +815,11 @@ { struct device *new_d; - /* If device is found under parent, no need to allocate new device. */ + /* We don't allow duplicate devices in devicetree. */ new_d = get_dev(parent, path_a, path_b, bustype, chip_instance); if (new_d) { - alloc_bus(new_d); - return new_d; + printf("ERROR: Duplicate device! %s\n", new_d->name); + exit(1); } new_d = alloc_dev(parent); @@ -1084,11 +1065,8 @@ { struct bus *bus = dev->bus; - while (bus) { - if (bus->children) - return 1; - bus = bus->next_bus; - } + if (bus && bus->children) + return 1; return 0; } @@ -1098,7 +1076,7 @@ static int dev_id; if (ptr == &base_root_dev) { - fprintf(fil, "STORAGE struct bus %s_links[];\n", + fprintf(fil, "STORAGE struct bus %s_bus;\n", ptr->name); return; } @@ -1120,7 +1098,7 @@ fprintf(fil, "STORAGE struct resource %s_res[];\n", ptr->name); if (dev_has_children(ptr)) - fprintf(fil, "STORAGE struct bus %s_links[];\n", + fprintf(fil, "STORAGE struct bus %s_bus;\n", ptr->name); if (next) @@ -1192,35 +1170,18 @@ fprintf(fil, "\t };\n"); } -static void emit_bus(FILE *fil, struct bus *bus) +static void emit_dev_bus(FILE *fil, struct device *ptr) { - fprintf(fil, "\t\t[%d] = {\n", bus->id); - fprintf(fil, "\t\t\t.link_num = %d,\n", bus->id); - fprintf(fil, "\t\t\t.dev = &%s,\n", bus->dev->name); - if (bus->children) - fprintf(fil, "\t\t\t.children = &%s,\n", bus->children->name); - - if (bus->next_bus) - fprintf(fil, "\t\t\t.next=&%s_links[%d],\n", bus->dev->name, - bus->id + 1); - else - fprintf(fil, "\t\t\t.next = NULL,\n"); - fprintf(fil, "\t\t},\n"); -} - -static void emit_dev_links(FILE *fil, struct device *ptr) -{ - fprintf(fil, "STORAGE struct bus %s_links[] = {\n", + fprintf(fil, "STORAGE struct bus %s_bus = {\n", ptr->name); + assert(ptr->bus && ptr->bus->children); struct bus *bus = ptr->bus; - while (bus) { - emit_bus(fil, bus); - bus = bus->next_bus; - } + fprintf(fil, "\t.dev = &%s,\n", bus->dev->name); + fprintf(fil, "\t.children = &%s,\n", bus->children->name); - fprintf(fil, "\t};\n"); + fprintf(fil, "};\n"); } static struct chip_instance *get_chip_instance(const struct device *dev) @@ -1268,8 +1229,7 @@ else fprintf(fil, "\t.ops = NULL,\n"); fprintf(fil, "#endif\n"); - fprintf(fil, "\t.bus = &%s_links[%d],\n", ptr->parent->dev->name, - ptr->parent->id); + fprintf(fil, "\t.bus = &%s_bus,\n", ptr->parent->dev->name); fprintf(fil, "\t.path = {"); fprintf(fil, ptr->path, ptr->path_a, ptr->path_b); fprintf(fil, "},\n"); @@ -1290,7 +1250,7 @@ ptr->name); } if (has_children) - fprintf(fil, "\t.link_list = &%s_links[0],\n", + fprintf(fil, "\t.link_list = &%s_bus,\n", ptr->name); else fprintf(fil, "\t.link_list = NULL,\n"); @@ -1319,7 +1279,7 @@ emit_resources(fil, ptr); if (has_children) - emit_dev_links(fil, ptr); + emit_dev_bus(fil, ptr); } static void expose_device_names(FILE *fil, FILE *head, struct device *ptr, struct device *next) @@ -1370,11 +1330,8 @@ { struct bus *bus = d->bus; - while (bus) { - if (bus->children) - add_siblings_to_queue(bfs_q_head, bus->children); - bus = bus->next_bus; - } + if (dev_has_children(d)) + add_siblings_to_queue(bfs_q_head, bus->children); } static void walk_device_tree(FILE *fil, FILE *head, struct device *ptr, @@ -1724,12 +1681,9 @@ * +-----------------------------------------------------------------+ * | | | * | bus | Recursively call override_devicetree on | - * | last_bus | each bus of override device. It is assumed | + * | | each bus of override device. It is assumed | * | | that bus with id X under base device | - * | | to bus with id X under override device. If | - * | | override device has more buses than base | - * | | device, then new buses are allocated under | - * | | base device. | + * | | to bus with id X under override device. | * | | | * +-----------------------------------------------------------------+ */ @@ -1841,29 +1795,20 @@ /* * Now that the device properties are all copied over, look at each bus * of the override device and run override_devicetree in a recursive - * manner. The assumption here is that first bus of override device - * corresponds to first bus of base device and so on. If base device has - * lesser buses than override tree, then new buses are allocated for it. + * manner. If base device has no bus but the override tree has, then a new + * bus is allocated for it. */ struct bus *override_bus = override_dev->bus; struct bus *base_bus = base_dev->bus; - while (override_bus) { + /* + * If we have more buses in override tree device, then allocate + * a new bus for the base tree device as well. + */ + if (!base_bus) + alloc_bus(base_dev); - /* - * If we have more buses in override tree device, then allocate - * a new bus for the base tree device as well. - */ - if (!base_bus) { - alloc_bus(base_dev); - base_bus = base_dev->last_bus; - } - - override_devicetree(base_dev->bus, override_dev->bus); - - override_bus = override_bus->next_bus; - base_bus = base_bus->next_bus; - } + override_devicetree(base_dev->bus, override_dev->bus); } /* diff --git a/util/sconfig/sconfig.h b/util/sconfig/sconfig.h index cb8f7b3..bb9a98f 100644 --- a/util/sconfig/sconfig.h +++ b/util/sconfig/sconfig.h @@ -106,17 +106,11 @@ struct device; struct bus { - /* Instance/ID of the bus under the device. */ - int id; - /* Pointer to device to which this bus belongs. */ struct device *dev; /* Pointer to list of children. */ struct device *children; - - /* Pointer to next bus for the device. */ - struct bus *next_bus; }; struct device { @@ -157,10 +151,8 @@ /* Pointer to chip instance for this device. */ struct chip_instance *chip_instance; - /* Pointer to list of buses under this device. */ + /* Pointer to the bus under this device. */ struct bus *bus; - /* Pointer to last bus under this device. */ - struct bus *last_bus; /* Global identifier of the ops for this device. */ char *ops_id; diff --git a/util/sconfig/sconfig.tab.c_shipped b/util/sconfig/sconfig.tab.c_shipped index cb60fb5..5b10922 100644 --- a/util/sconfig/sconfig.tab.c_shipped +++ b/util/sconfig/sconfig.tab.c_shipped @@ -1238,7 +1238,7 @@ case 26: /* @2: %empty */ { (yyval.dev) = new_device_raw(cur_parent, cur_chip_instance, (yyvsp[-3].number), (yyvsp[-2].string), (yyvsp[-1].string), (yyvsp[0].number)); - cur_parent = (yyval.dev)->last_bus; + cur_parent = (yyval.dev)->bus; } break; @@ -1251,7 +1251,7 @@ case 28: /* @3: %empty */ { (yyval.dev) = new_device_reference(cur_parent, cur_chip_instance, (yyvsp[-1].string), (yyvsp[0].number)); - cur_parent = (yyval.dev)->last_bus; + cur_parent = (yyval.dev)->bus; } break; diff --git a/util/sconfig/sconfig.y b/util/sconfig/sconfig.y index 17ab9a5..8ec103c 100644 --- a/util/sconfig/sconfig.y +++ b/util/sconfig/sconfig.y @@ -44,7 +44,7 @@ device: DEVICE BUS NUMBER /* == devnum */ alias status { $<dev>$ = new_device_raw(cur_parent, cur_chip_instance, $<number>2, $<string>3, $<string>4, $<number>5); - cur_parent = $<dev>$->last_bus; + cur_parent = $<dev>$->bus; } devicechildren END { cur_parent = $<dev>6->parent; @@ -52,7 +52,7 @@ device: DEVICE REFERENCE STRING status { $<dev>$ = new_device_reference(cur_parent, cur_chip_instance, $<string>3, $<number>4); - cur_parent = $<dev>$->last_bus; + cur_parent = $<dev>$->bus; } devicechildren END { cur_parent = $<dev>5->parent; -- To view, visit https://review.coreboot.org/c/coreboot/+/78328?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: Iab6fe269faef46ae77ed1ea425440cf5c7dbd49b Gerrit-Change-Number: 78328 Gerrit-PatchSet: 18 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[L] Change in coreboot[main]: device/device.h: Drop multiple links
by Felix Held (Code Review) 29 Jan '24

29 Jan '24

Attention is currently required from: Arthur Heymans, Chen, Gang C, Fred Reitberger, Jason Glenesk, Jérémy Compostella, Martin L Roth, Matt DeVillier, Nico Huber. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78328?usp=email ) Change subject: device/device.h: Drop multiple links ...................................................................... Patch Set 17: Code-Review+2 (1 comment) Patchset: PS17: had another look and the current patchset looks good to me and it also worked as expected on mandolin when i tested it last week -- To view, visit https://review.coreboot.org/c/coreboot/+/78328?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: Iab6fe269faef46ae77ed1ea425440cf5c7dbd49b Gerrit-Change-Number: 78328 Gerrit-PatchSet: 17 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Mon, 29 Jan 2024 18:23:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp/spr: Create CXL ACPI resources only for CXL IIO stacks
by Lean Sheng Tan (Code Review) 29 Jan '24

29 Jan '24

Attention is currently required from: Shuo Liu. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80087?usp=email ) Change subject: soc/intel/xeon_sp/spr: Create CXL ACPI resources only for CXL IIO stacks ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/80087/comment/2fc61e59_afaa54ae : PS4, Line 7: soc/intel/xeon_sp/spr: Create CXL ACPI resources only for : CXL IIO stacks > Using line-wraps in the summary/title is uncommon in coreboot. […] sorry my bad, I forgot to catch that. -- To view, visit https://review.coreboot.org/c/coreboot/+/80087?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: I6c1b1343991bc73d90a433d959f6618bbf59532f Gerrit-Change-Number: 80087 Gerrit-PatchSet: 4 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.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: Patrick Rudolph <patrick.rudolph(a)9elements.com> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Comment-Date: Mon, 29 Jan 2024 18:19:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024