coreboot-gerrit December 2023

coreboot-gerrit@coreboot.org

1 participants
2354 discussions

[XS] Change in coreboot[main]: ec/lenovo/h8/acpi/thermal: Make NameSeg FPWR all upper case
by Paul Menzel (Code Review) 03 Dec '23

03 Dec '23

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79367?usp=email ) Change subject: ec/lenovo/h8/acpi/thermal: Make NameSeg FPWR all upper case ...................................................................... ec/lenovo/h8/acpi/thermal: Make NameSeg FPWR all upper case Building the Lenovo T60/T60p, iasl 20230628 shows the remark below: dsdt.asl 2099: PowerResource (FPwR, 0, 0) Remark 2182 - ^ At least one lower case letter found in NameSeg, ASL is case insensitive - converting to upper case (FPWR) dsdt.asl 2118: Name (_PR0, Package () { FPwR }) Remark 2182 - ^ At least one lower case letter found in NameSeg, ASL is case insensitive - converting to upper case (FPWR) Address it by making it all upper case. Change-Id: Ia7924b015e76c43818d2d82da35ce0013d721c26 Fixes: 3ab13a8691cb ("ec/lenovo/h8/acpi/thermal: Add support for passive cooling") Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de> --- M src/ec/lenovo/h8/acpi/thermal.asl 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/79367/1 diff --git a/src/ec/lenovo/h8/acpi/thermal.asl b/src/ec/lenovo/h8/acpi/thermal.asl index 4f3b76e..fa5a282 100644 --- a/src/ec/lenovo/h8/acpi/thermal.asl +++ b/src/ec/lenovo/h8/acpi/thermal.asl @@ -97,7 +97,7 @@ Name (_AL0, Package () { FAN }) - PowerResource (FPwR, 0, 0) + PowerResource (FPWR, 0, 0) { /* * WINDOWS BUG: Don't read from EmbeddedControl @@ -127,7 +127,7 @@ Device (FAN) { Name (_HID, EISAID ("PNP0C0B")) - Name (_PR0, Package () { FPwR }) + Name (_PR0, Package () { FPWR }) } } -- To view, visit https://review.coreboot.org/c/coreboot/+/79367?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: Ia7924b015e76c43818d2d82da35ce0013d721c26 Gerrit-Change-Number: 79367 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: mb/google/rex/var/screebo: Override power limits
by Subrata Banik (Code Review) 03 Dec '23

03 Dec '23

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Paul Menzel, Tarun. Hello Dinesh Gehlot, Eran Mitrani, Kapil Porwal, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79330?usp=email to look at the new patch set (#4). Change subject: mb/google/rex/var/screebo: Override power limits ...................................................................... mb/google/rex/var/screebo: Override power limits This patch allows variants to override the default baseboard PLx limits. Additionally, rearrange the include header files alphabetically. BUG=b:313667378 TEST=Able to boot google/screebo with modified power limits. Before: [DEBUG] WEAK: src/mainboard/google/rex/variants/baseboard/rex/ ramstage.c/variant_devtree_update called [INFO ] Overriding power limits PL1 (mW) (10000, 15000) PL2 (mW) (40000, 40000) PL4 (W) (84) After: [INFO ] Overriding power limits PL1 (mW) (10000, 15000) PL2 (mW) (40000, 40000) PL4 (W) (84) Change-Id: Ic66872c530963238a0bf5eebbd5b5a76a7985e5c Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/mainboard/google/rex/variants/screebo/variant.c 1 file changed, 65 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/79330/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/79330?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: Ic66872c530963238a0bf5eebbd5b5a76a7985e5c Gerrit-Change-Number: 79330 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: mb/google/rex/var/screebo: Override power limits
by Subrata Banik (Code Review) 03 Dec '23

03 Dec '23

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Paul Menzel, Tarun. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79330?usp=email ) Change subject: mb/google/rex/var/screebo: Override power limits ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/79330/comment/7778412b_673e9add : PS3, Line 21: [INFO ] Overriding power limits PL1 (mW) (10000, 15000) : PL2 (mW) (40000, 40000) PL4 (W) (84) : : After: : : [INFO ] Overriding power limits PL1 (mW) (10000, 15000) : PL2 (mW) (40000, 40000) PL4 (W) (84) > Thank you for the explanation. So, the paste demonstrate that the debug line is gone? exactly the `DEBUG` line suggests that, we were using baseline default value (aka no override). And modified CL shows that we are using an overridden value from variant directory. > > I was confused by “This patch modifies the power limits value to enhance …” in the beginning. But no power limits are changed? updated the commit msg. -- To view, visit https://review.coreboot.org/c/coreboot/+/79330?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: Ic66872c530963238a0bf5eebbd5b5a76a7985e5c Gerrit-Change-Number: 79330 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Sun, 03 Dec 2023 01:49:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: arch/riscv/payload: Replace old RISC-V CSR names with their values
by Maximilian Brune (Code Review) 03 Dec '23

03 Dec '23

Attention is currently required from: Philipp Hug, Zebreus, ron minnich. Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79364?usp=email ) Change subject: arch/riscv/payload: Replace old RISC-V CSR names with their values ...................................................................... Patch Set 3: (1 comment) File src/arch/riscv/payload.c: https://review.coreboot.org/c/coreboot/+/79364/comment/1082f13f_f4c88c44 : PS3, Line 49: write_csr(0x0004, 0); I would just remove the code altogether. Like you said its not part of the RISC-V ISA Manual Privileged Specification anymore and as far as I know it is also not part of any other ratified extension. It is not used by any code in coreboot and will therefore not break anything. Also I would like to avoid using CSRs that are not part of the base specification inside architecture code. If there is a use case for a platform in coreboot that needs this (maybe M-U mode only platforms?) we can worry about it then. -- To view, visit https://review.coreboot.org/c/coreboot/+/79364?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: I7c8f2a06a109333f95230bf0a3056c8d5c8a9132 Gerrit-Change-Number: 79364 Gerrit-PatchSet: 3 Gerrit-Owner: Zebreus <lennarteichhorn(a)googlemail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Zebreus <lennarteichhorn(a)googlemail.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Sun, 03 Dec 2023 00:52:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/rex/var/ovis: Add power limit support for MCH ID 0x7d14
by Paul Menzel (Code Review) 02 Dec '23

02 Dec '23

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Subrata Banik, Tarun. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79332?usp=email ) Change subject: mb/google/rex/var/ovis: Add power limit support for MCH ID 0x7d14 ...................................................................... Patch Set 5: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/79332?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: Iff71adb4e26d18970b5947927c258419f751de32 Gerrit-Change-Number: 79332 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Sat, 02 Dec 2023 20:10:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: mb/google/rex/var/screebo: Override power limits
by Paul Menzel (Code Review) 02 Dec '23

02 Dec '23

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Subrata Banik, Tarun. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79330?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: mb/google/rex/var/screebo: Override power limits ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/79330/comment/d33b2a24_41dd0c55 : PS3, Line 21: [INFO ] Overriding power limits PL1 (mW) (10000, 15000) : PL2 (mW) (40000, 40000) PL4 (W) (84) : : After: : : [INFO ] Overriding power limits PL1 (mW) (10000, 15000) : PL2 (mW) (40000, 40000) PL4 (W) (84) > > The values are unchanged? […] Thank you for the explanation. So, the paste demonstrate that the debug line is gone? I was confused by “This patch modifies the power limits value to enhance …” in the beginning. But no power limits are changed? -- To view, visit https://review.coreboot.org/c/coreboot/+/79330?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: Ic66872c530963238a0bf5eebbd5b5a76a7985e5c Gerrit-Change-Number: 79330 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Sat, 02 Dec 2023 20:10:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: acpi: Add PPTT support
by Lean Sheng Tan (Code Review) 02 Dec '23

02 Dec '23

Lean Sheng Tan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78071?usp=email ) Change subject: acpi: Add PPTT support ...................................................................... acpi: Add PPTT support This patch adds code to generate Processor Properties Topology Tables (PPTT) compliant to the ACPI 6.4 specification. - The 'acpi_get_pptt_topology' hook is mandatory once ACPI_PPTT is selected. Its purpose is to return a pointer to a topology tree, which describes the relationship between CPUs and caches. The hook can be provided by, for example, mainboard code. Background: We are currently working on mainboard code for qemu-sbsa and Neoverse N2. Both require a valid PPTT table. Patch was tested against the qemu-sbsa board. Change-Id: Ia119e1ba15756704668116bdbc655190ec94ff10 Signed-off-by: David Milosevic <David.Milosevic(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78071 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> --- M src/acpi/Kconfig M src/acpi/Makefile.inc M src/acpi/acpi.c A src/acpi/acpi_pptt.c M src/include/acpi/acpi.h 5 files changed, 324 insertions(+), 1 deletion(-) Approvals: Lean Sheng Tan: Looks good to me, approved build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/src/acpi/Kconfig b/src/acpi/Kconfig index cb2befb..cf51969 100644 --- a/src/acpi/Kconfig +++ b/src/acpi/Kconfig @@ -91,3 +91,17 @@ default 144 help Set the maximum size of all ACPI tables in KiB. + +config ACPI_PPTT + bool + depends on HAVE_ACPI_TABLES + help + Selected to build an ACPI Processor Properties Topology Table. + +config ACPI_PPTT_MAX_CACHES + int + depends on ACPI_PPTT + default 4 + help + This variable sets the maximum number of distinct caches per + topology level. Increasing this option also increases stack usage. diff --git a/src/acpi/Makefile.inc b/src/acpi/Makefile.inc index cb0bbb4..6bb34cb 100644 --- a/src/acpi/Makefile.inc +++ b/src/acpi/Makefile.inc @@ -8,6 +8,7 @@ ramstage-y += acpi_dmar.c ramstage-y += acpi_hpet.c endif +ramstage-$(CONFIG_ACPI_PPTT) += acpi_pptt.c ramstage-y += acpigen.c ramstage-y += acpigen_dptf.c ramstage-y += acpigen_dsm.c diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c index 942d464..e40d4ac 100644 --- a/src/acpi/acpi.c +++ b/src/acpi/acpi.c @@ -1204,6 +1204,18 @@ return lpi_desc->header.length; } +static void acpi_create_pptt(acpi_header_t *header, void *unused) +{ + if (!CONFIG(ACPI_PPTT)) + return; + + if (acpi_fill_header(header, "PPTT", PPTT, sizeof(acpi_pptt_t)) != CB_SUCCESS) + return; + + acpi_pptt_t *pptt = (acpi_pptt_t *)header; + acpi_create_pptt_body(pptt); +} + static uint8_t acpi_spcr_type(void) { /* 16550-compatible with parameters defined in Generic Address Structure */ @@ -1394,6 +1406,7 @@ { acpi_create_bert, NULL, sizeof(acpi_bert_t) }, { acpi_create_spcr, NULL, sizeof(acpi_spcr_t) }, { acpi_create_gtdt, NULL, sizeof(acpi_gtdt_t) }, + { acpi_create_pptt, NULL, sizeof(acpi_pptt_t) }, }; current = start; @@ -1743,6 +1756,8 @@ return 4; case GTDT: return 3; + case PPTT: /* ACPI 6.4 */ + return 3; default: return -1; } diff --git a/src/acpi/acpi_pptt.c b/src/acpi/acpi_pptt.c new file mode 100644 index 0000000..3822bf7 --- /dev/null +++ b/src/acpi/acpi_pptt.c @@ -0,0 +1,186 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <acpi/acpi.h> +#include <include/assert.h> +#include <console/console.h> + +/* + * A structure to hold a cache pointer + * and its corresponding reference within + * the PPTT table. + */ +struct cache_reference { + struct pptt_cache *cache; // cache pointer + u32 ref; // and its reference within PPTT +}; + +/* + * A list of 'struct cache_reference', used + * to keep track of generated caches per topology level. + */ +struct cache_list { + u32 n_caches; // number of caches in list + struct cache_reference cache_refs[CONFIG_ACPI_PPTT_MAX_CACHES]; // cache reference list +}; + +/* + * Start of the PPTT table. Constant + * value as soon as we enter acpi_create_pptt_body. + */ +static uintptr_t pptt_start; + +/* --- Helper Functions (non exposed) --- */ + +static inline u32 node_to_reference(const void *node) +{ + /* + * References are the offset from the start + * of the PPTT table. + * + * PPTT + * +---------+ <- pptt_start (acpi_pptt_t) <---+ + * | | | node - pptt_start + * | | | + * +---------+ <- node (cpu or cache) <--------+ + * | | + * | | + * | | + * +---------+ + */ + return ((uintptr_t)node - pptt_start); +} + +static u32 count_resources(struct pptt_cpu_resources *res) +{ + u32 n_resources = 0; + + while (res != NULL) { + n_resources += 1; + res = res->next; + } + + return n_resources; +} + +static u32 cache_list_ref_of(struct cache_list *cache_list, const struct pptt_cache *cache) +{ + /* + * Lookup the PPTT reference of 'cache'. + * Return 0, if no PPTT structure exists for 'cache'. + */ + + for (int i = 0; i < cache_list->n_caches; i++) { + if (cache_list->cache_refs[i].cache == cache) + return cache_list->cache_refs[i].ref; + } + + /* no cache reference found */ + return 0; +} + +static inline void cache_list_append(struct cache_list *cache_list, struct pptt_cache *cache, const u32 ref) +{ + printk(BIOS_DEBUG, "acpi: pptt: cache=%p ref=%u\n", cache, ref); + + cache_list->cache_refs[cache_list->n_caches].cache = cache; + cache_list->cache_refs[cache_list->n_caches].ref = ref; + + cache_list->n_caches += 1; +} + +static u32 new_pptt_cache(unsigned long *current, struct pptt_cache *cache, struct cache_list *cache_list) +{ + static u32 unique_cache_id = 1; + u32 current_reference = 0; + + if ((current_reference = cache_list_ref_of(cache_list, cache)) != 0) + return current_reference; + + if (cache_list->n_caches >= CONFIG_ACPI_PPTT_MAX_CACHES) { + printk(BIOS_WARNING, "acpi: pptt: Too many distinct caches! PPTT incomplete.\n"); + return 0; + } + + acpi_pptt_cache_node_t *cache_node = (acpi_pptt_cache_node_t *)*current; + memset(cache_node, 0x0, sizeof(acpi_pptt_cache_node_t)); + + cache_node->type = PPTT_NODE_TYPE_CACHE; + cache_node->length = sizeof(acpi_pptt_cache_node_t); + + cache_node->flags = cache->flags.raw; + cache_node->size = cache->size; + cache_node->n_sets = cache->numsets; + cache_node->associativity = cache->associativity; + + cache_node->attributes = cache->attributes; + cache_node->line_size = cache->line_size; + cache_node->cache_id = unique_cache_id++; + + *current += cache_node->length; + + current_reference = node_to_reference(cache_node); + cache_list_append(cache_list, cache, current_reference); + + if (cache->next_level != NULL) + cache_node->next_level = new_pptt_cache(current, cache->next_level, cache_list); + + return current_reference; +} + +static u32 new_pptt_cpu(unsigned long *current, const struct pptt_topology *cpu, const u32 parent_ref, struct cache_list *cache_list) +{ + acpi_pptt_cpu_node_t *cpu_node = (acpi_pptt_cpu_node_t *)*current; + + const u32 n_resources = count_resources(cpu->resources); + const u32 structure_length = sizeof(acpi_pptt_cpu_node_t) + (n_resources * sizeof(u32)); + + memset(cpu_node, 0x0, structure_length); + + cpu_node->type = PPTT_NODE_TYPE_CPU; + cpu_node->length = structure_length; + cpu_node->flags = cpu->flags.raw; + cpu_node->processor_id = cpu->processor_id; + cpu_node->parent = parent_ref; + + *current += cpu_node->length; + + for (struct pptt_cpu_resources *it = cpu->resources; it != NULL; it = it->next) + cpu_node->resources[cpu_node->n_resources++] = new_pptt_cache(current, it->cache, cache_list); + + return node_to_reference(cpu_node); +} + +static void setup_topology(const struct pptt_topology *node, const u32 parent_ref, unsigned long *current) +{ + struct cache_list cache_list = { + .cache_refs = { }, + .n_caches = 0 + }; + + while (node != NULL) { + const u32 cpu_ref = new_pptt_cpu(current, node, parent_ref, &cache_list); + setup_topology(node->child, cpu_ref, current); + + node = node->sibling; + } +} + +/* --- PPTT generation helper functions (exposed) --- */ + +void acpi_create_pptt_body(acpi_pptt_t *pptt) +{ + /* set start of pptt table */ + pptt_start = (uintptr_t)pptt; + + /* locate start of pptt body */ + unsigned long current = (unsigned long)(pptt->body); + + /* retrieve processor topology */ + const struct pptt_topology *topology_tree = acpi_get_pptt_topology(); + + /* write processor properties topology table to memory */ + setup_topology(topology_tree, 0, &current); + + /* update length field in pptt header */ + pptt->header.length = current - (unsigned long)pptt; +} diff --git a/src/include/acpi/acpi.h b/src/include/acpi/acpi.h index 0941dee..a298e39 100644 --- a/src/include/acpi/acpi.h +++ b/src/include/acpi/acpi.h @@ -93,6 +93,7 @@ LPIT, /* Low Power Idle Table */ MADT, /* Multiple APIC Description Table */ MCFG, /* PCI Express Memory Mapped Configuration */ + PPTT, /* Processor Properties Topology Table */ RSDP, /* Root System Description Pointer */ RSDT, /* Root System Description Table */ SLIT, /* System Locality Distance Information Table */ @@ -102,7 +103,6 @@ TCPA, /* Trusted Computing Platform Alliance Table */ TPM2, /* Trusted Platform Module 2.0 Table */ XSDT, /* Extended System Description Table */ - /* Additional proprietary tables used by coreboot */ CRAT, /* Component Resource Attribute Table */ NHLT, /* Non HD audio Link Table */ @@ -1419,6 +1419,110 @@ acpi_einj_action_table_t action_table[ACTION_COUNT]; } __packed acpi_einj_t; +/* PPTT definitions */ + +#define PPTT_NODE_TYPE_CPU 0 +#define PPTT_NODE_TYPE_CACHE 1 + +/* PPTT structures for ACPI generation */ + +typedef struct acpi_pptt_cpu_node { + u8 type; // type = 0 (processor structure specification) + u8 length; // in bytes + u8 reserved[2]; // reserved, must be zero + u32 flags; // processor hierarchy node structure flags + u32 parent; // reference (delta of pptt-start and node) to parent node, must be zero if no parent + u32 processor_id; // must match id in MADT, if actual processor + u32 n_resources; // number of resource structure references + u32 resources[]; // resource structure references +} acpi_pptt_cpu_node_t; + +typedef struct acpi_pptt_cache_node { + u8 type; // type = 1 (cache type structure) + u8 length; // length = 28 + u8 reserved[2]; // reserved, must be zero + u32 flags; // cache structure flags + u32 next_level; // reference to next level cache, null if last cache level + u32 size; // cache size in bytes + u32 n_sets; // number of sets in the cache + u8 associativity; // integer number of ways + u8 attributes; // bits[7:5] reserved, must be zero + u16 line_size; // in bytes + u32 cache_id; // unique, non-zero +} acpi_pptt_cache_node_t; + +union acpi_pptt_body { + acpi_pptt_cpu_node_t cpu; + acpi_pptt_cache_node_t cache; +}; + +typedef struct acpi_pptt { + acpi_header_t header; + + /* + * followed by a variable length body + * consisting of processor topology structures. + * + * see acpi_pptt_cpu_node and + * acpi_pptt_cache_node. + */ + union acpi_pptt_body body[]; +} __packed acpi_pptt_t; + +/* PPTT structures for topology description */ + +union pptt_cache_flags { + struct { + u32 size_valid : 1; + u32 n_sets_valid : 1; + u32 associativity_valid : 1; + u32 alloc_type_valid : 1; + u32 cache_type_valid : 1; + u32 write_policy_valid : 1; + u32 line_size_valid : 1; + u32 cache_id_valid : 1; + u32 reserved : 24; + }; + + u32 raw; +}; + +union pptt_cpu_flags { + struct { + u32 is_physical_package : 1; + u32 processor_id_valid : 1; + u32 is_thread : 1; + u32 is_leaf : 1; + u32 is_identical_impl : 1; + u32 reserved : 27; + }; + + u32 raw; +}; + +struct pptt_cache { + u32 size; + u32 numsets; + u8 associativity; + u8 attributes; + u16 line_size; + union pptt_cache_flags flags; + struct pptt_cache *next_level; +}; + +struct pptt_cpu_resources { + struct pptt_cache *cache; + struct pptt_cpu_resources *next; +}; + +struct pptt_topology { + u32 processor_id; + union pptt_cpu_flags flags; + struct pptt_cpu_resources *resources; + struct pptt_topology *sibling; + struct pptt_topology *child; +}; + /* SPCR (Serial Port Console Redirection Table) */ typedef struct acpi_spcr { acpi_header_t header; @@ -1589,6 +1693,9 @@ int acpi_create_cedt_cfmws(acpi_cedt_cfmws_t *cfmws, u64 base_hpa, u64 window_size, u8 eniw, u32 hbig, u16 restriction, u16 qtg_id, const u32 *interleave_target); +/* PPTT related functions */ +void acpi_create_pptt_body(acpi_pptt_t *pptt); +struct pptt_topology *acpi_get_pptt_topology(void); int acpi_create_madt_ioapic_from_hw(acpi_madt_ioapic_t *ioapic, u32 addr); -- To view, visit https://review.coreboot.org/c/coreboot/+/78071?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: Ia119e1ba15756704668116bdbc655190ec94ff10 Gerrit-Change-Number: 78071 Gerrit-PatchSet: 24 Gerrit-Owner: David Milosevic <David.Milosevic(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: mb/google/rex/var/screebo: Override power limits
by Subrata Banik (Code Review) 02 Dec '23

02 Dec '23

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Paul Menzel, Tarun. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79330?usp=email ) Change subject: mb/google/rex/var/screebo: Override power limits ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/79330/comment/b1ec49ef_ddadd24c : PS3, Line 21: [INFO ] Overriding power limits PL1 (mW) (10000, 15000) : PL2 (mW) (40000, 40000) PL4 (W) (84) : : After: : : [INFO ] Overriding power limits PL1 (mW) (10000, 15000) : PL2 (mW) (40000, 40000) PL4 (W) (84) > The values are unchanged? yes, that's because we are now overriding the same value (previously used to override by baseboard) from a variant directory. This will help in future variants design to override the baseboard aka default (which might be the case with Screebo). The CB:79331 ensures that baseboard can only program the default PL limits which is performance oriented and variants can override the defaults with something that is more appropriate from the design standpoint. With CB:79330 and CB:79331, we will see that Rex is baseboard supporting performance design limits but screebo would override with power efficient design PL value. I hope it clarifies your doubt -- To view, visit https://review.coreboot.org/c/coreboot/+/79330?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: Ic66872c530963238a0bf5eebbd5b5a76a7985e5c Gerrit-Change-Number: 79330 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Sat, 02 Dec 2023 18:09:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: mb/google/rex: Enhance power limit override mechanism
by Subrata Banik (Code Review) 02 Dec '23

02 Dec '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/79329?usp=email ) Change subject: mb/google/rex: Enhance power limit override mechanism ...................................................................... mb/google/rex: Enhance power limit override mechanism This patch expands the power limit override capability to include variants directories, enabling them to modify power limit settings configured by the baseboard. Previously, only the baseboard could override power limit settings. For instance, while the google/rex baseboard sets the PL1 max power limit to 15W, the google/screebo variant couldn't override this value. This enhancement empowers variants directories to override baseboard- configured power limit settings, allowing for greater flexibility and control over power limits. BUG=b:313667378 TEST=Able to call into _weak implementation of `variant_devtree_update` unless there is one override. [DEBUG] WEAK: src/mainboard/google/rex/variants/baseboard/rex/ ramstage.c/variant_devtree_update called [INFO ] Overriding power limits PL1 (mW) (10000, 15000) PL2 (mW) (40000, 40000) PL4 (W) (84) Change-Id: Ib07691625e075b0fbab42271512322ffc60ba13b Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/79329 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> --- M src/mainboard/google/rex/mainboard.c M src/mainboard/google/rex/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/rex/variants/baseboard/ovis/ramstage.c M src/mainboard/google/rex/variants/baseboard/rex/ramstage.c 4 files changed, 22 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Kapil Porwal: Looks good to me, approved diff --git a/src/mainboard/google/rex/mainboard.c b/src/mainboard/google/rex/mainboard.c index 619ca71..3ca85ff 100644 --- a/src/mainboard/google/rex/mainboard.c +++ b/src/mainboard/google/rex/mainboard.c @@ -41,12 +41,12 @@ fw_config_gpio_padbased_override(padbased_table); gpio_configure_pads_with_padbased(padbased_table); free(padbased_table); - variant_devtree_update(); + baseboard_devtree_update(); } -void __weak variant_devtree_update(void) +void __weak baseboard_devtree_update(void) { - /* Override dev tree settings per board */ + /* Override dev tree settings per baseboard */ } void __weak variant_generate_s0ix_hook(enum s0ix_entry entry) diff --git a/src/mainboard/google/rex/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/rex/variants/baseboard/include/baseboard/variants.h index afed754..bd3d535 100644 --- a/src/mainboard/google/rex/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/rex/variants/baseboard/include/baseboard/variants.h @@ -31,7 +31,9 @@ void variant_generate_s0ix_hook(enum s0ix_entry entry); -/* Modify devictree settings during ramstage */ +/* Modify devictree settings during ramstage by baseboard */ +void baseboard_devtree_update(void); +/* Modify devictree settings during ramstage by dedicated variant */ void variant_devtree_update(void); #endif /*__BASEBOARD_VARIANTS_H__ */ diff --git a/src/mainboard/google/rex/variants/baseboard/ovis/ramstage.c b/src/mainboard/google/rex/variants/baseboard/ovis/ramstage.c index 5955b2a..17d39ae 100644 --- a/src/mainboard/google/rex/variants/baseboard/ovis/ramstage.c +++ b/src/mainboard/google/rex/variants/baseboard/ovis/ramstage.c @@ -21,8 +21,15 @@ }, }; -void variant_devtree_update(void) +void __weak variant_devtree_update(void) { + printk(BIOS_DEBUG, "WEAK: %s/%s called\n", __FILE__, __func__); + size_t total_entries = ARRAY_SIZE(limits); variant_update_cpu_power_limits(limits, total_entries); } + +void baseboard_devtree_update(void) +{ + variant_devtree_update(); +} diff --git a/src/mainboard/google/rex/variants/baseboard/rex/ramstage.c b/src/mainboard/google/rex/variants/baseboard/rex/ramstage.c index 02bdca5d..7c096510 100644 --- a/src/mainboard/google/rex/variants/baseboard/rex/ramstage.c +++ b/src/mainboard/google/rex/variants/baseboard/rex/ramstage.c @@ -96,8 +96,10 @@ }; #endif -void variant_devtree_update(void) +void __weak variant_devtree_update(void) { + printk(BIOS_DEBUG, "WEAK: %s/%s called\n", __FILE__, __func__); + const struct cpu_tdp_power_limits *limits = performance_efficient_limits; size_t limits_size = ARRAY_SIZE(performance_efficient_limits); @@ -115,3 +117,8 @@ variant_update_cpu_power_limits(limits, limits_size); } + +void baseboard_devtree_update(void) +{ + variant_devtree_update(); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/79329?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: Ib07691625e075b0fbab42271512322ffc60ba13b Gerrit-Change-Number: 79329 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)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-MessageType: merged

1 0

[XS] Change in coreboot[main]: mb/google/rex/var/ovis: Add power limit support for MCH ID 0x7d14
by Subrata Banik (Code Review) 02 Dec '23

02 Dec '23

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Paul Menzel, Tarun. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79332?usp=email ) Change subject: mb/google/rex/var/ovis: Add power limit support for MCH ID 0x7d14 ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/79332/comment/995e13a1_b8a216a2 : PS4, Line 10: 0x7d14 DID. > . done -- To view, visit https://review.coreboot.org/c/coreboot/+/79332?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: Iff71adb4e26d18970b5947927c258419f751de32 Gerrit-Change-Number: 79332 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Sat, 02 Dec 2023 18:03:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Stefan Reinauer <stefan.reinauer(a)coreboot.org> 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 December 2023