Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83480?usp=email )
Change subject: soc/intel/cmn/cpu: Introduce common CAR APIs
......................................................................
soc/intel/cmn/cpu: Introduce common CAR APIs
This patch adds `car_lib.c` to the IA common code to consolidate
SoC-agnostic CAR APIs. Initially, it includes `car_report_cache_info()`
to provide a unified way to read cache information, reducing the need
for SoC-specific implementations.
TEST=Builds successfully for google/rex.
Change-Id: I2ff84b27736057d19d4ec68c9afcb9b22e778f55
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/common/block/cpu/Makefile.mk
A src/soc/intel/common/block/cpu/car/car_lib.c
A src/soc/intel/common/block/include/intelblocks/car_lib.h
3 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/83480/1
diff --git a/src/soc/intel/common/block/cpu/Makefile.mk b/src/soc/intel/common/block/cpu/Makefile.mk
index 8dd6796..d4c2a6c 100644
--- a/src/soc/intel/common/block/cpu/Makefile.mk
+++ b/src/soc/intel/common/block/cpu/Makefile.mk
@@ -12,6 +12,8 @@
postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/exit_car.S
endif
+bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/car_lib.c
+
bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c
romstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c
ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c
diff --git a/src/soc/intel/common/block/cpu/car/car_lib.c b/src/soc/intel/common/block/cpu/car/car_lib.c
new file mode 100644
index 0000000..a3c0d80
--- /dev/null
+++ b/src/soc/intel/common/block/cpu/car/car_lib.c
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <arch/cpu.h>
+#include <console/console.h>
+#include <intelblocks/car_lib.h>
+
+/*
+ * Gathers and prints information about the CPU's L3 cache.
+ *
+ * This function does the following:
+ * 1. Sets the cache level of interest to L3.
+ * 2. Prints the following cache details to the console:
+ * - Cache level
+ * - Associativity (number of ways)
+ * - Number of physical partitions
+ * - Line size (in bytes)
+ * - Number of sets
+ * - Total cache size (in MiB), calculated using the 'get_cache_size' function.
+ */
+void car_report_cache_info(void)
+{
+ int cache_level = CACHE_L3;
+ struct cpu_cache_info info;
+
+ if (!fill_cpu_cache_info(cache_level, &info))
+ return;
+
+ printk(BIOS_INFO, "Cache: Level %d: ", cache_level);
+ printk(BIOS_INFO, "Associativity = %zd Partitions = %zd Line Size = %zd Sets = %zd\n",
+ info.num_ways, info.physical_partitions, info.line_size, info.num_sets);
+
+ printk(BIOS_INFO, "Cache size = %zu MiB\n", get_cache_size(&info)/MiB);
+}
diff --git a/src/soc/intel/common/block/include/intelblocks/car_lib.h b/src/soc/intel/common/block/include/intelblocks/car_lib.h
new file mode 100644
index 0000000..f106874
--- /dev/null
+++ b/src/soc/intel/common/block/include/intelblocks/car_lib.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef SOC_INTEL_COMMON_BLOCK_CAR_LIB_H
+#define SOC_INTEL_COMMON_BLOCK_CAR_LIB_H
+
+#include <types.h>
+
+/* Gathers and prints information about the CPU's L3 cache */
+void car_report_cache_info(void);
+
+#endif /* SOC_INTEL_COMMON_BLOCK_CAR_LIB_H */
--
To view, visit https://review.coreboot.org/c/coreboot/+/83480?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I2ff84b27736057d19d4ec68c9afcb9b22e778f55
Gerrit-Change-Number: 83480
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin Roth, Matt DeVillier.
Felix Held has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83446?usp=email )
Change subject: soc/amd/common/block/psp_gen2: add get_psp_mmio_base
......................................................................
Patch Set 2:
(1 comment)
File src/soc/amd/common/block/psp/psp_gen2.c:
https://review.coreboot.org/c/coreboot/+/83446/comment/fb84876e_b2af1a42?us… :
PS2, Line 62: /* Don't cache the PSP MMIO base if the register isn't locked */
> I don't think this is right. We want to cache the value before the lock happens. […]
with caching i mean put it into a static variable so that we don't have to do the lookup of the psp mmio base address too many times.
when the base address lock bit isn't set bit base address and enable bit are set, it's still valid to use, but we can't cache and reuse the base address the next time, since it might have changed when we have called into vendorcode in between.
alternatively we can treat the psp mmio base address register not being locked as error case; i think when the register is written, the lock bit is also set.
saving the value and then still comparing with the hardware register doesn't make any sense to me; that combines the added complexity with the higher performance impact
--
To view, visit https://review.coreboot.org/c/coreboot/+/83446?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I1d51e30f186508b0fe1ab5eb79c73e6d4b9d1a4a
Gerrit-Change-Number: 83446
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Tue, 16 Jul 2024 13:32:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Martin Roth, Matt DeVillier.
Felix Held has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83443?usp=email )
Change subject: soc/amd/*/root_complex: introduce and use domain_iohc_info struct
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/block/root_complex/root_complex.c:
https://review.coreboot.org/c/coreboot/+/83443/comment/78bd80a8_877e20d0?us… :
PS1, Line 15: domain->path.domain.domain
> Nit: This domaindomaindomain is kind of ugly. […]
yeah, there are bit too many 'domain' in there. changing the last one to domain_id sounds like a plan. i think we do have some new helper function to get the domain of a device in coreboot; would need to have a look. i'd do that as a separate patch, since it's a separate logical change. will keep this one as unresolved until i've added that change at the end of the patch train so that i don't forget
--
To view, visit https://review.coreboot.org/c/coreboot/+/83443?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ifce3d2b540d14ba3cba36f7cbf248fb7c63483fe
Gerrit-Change-Number: 83443
Gerrit-PatchSet: 1
Gerrit-Owner: 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: 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: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Tue, 16 Jul 2024 13:24:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Attention is currently required from: Felix Held, Maxim, Paul Menzel.
Angel Pons has posted comments on this change by Maxim. ( https://review.coreboot.org/c/coreboot/+/83196?usp=email )
Change subject: util/superiotool: Add extra selectors support
......................................................................
Patch Set 6: Code-Review+1
(1 comment)
File util/superiotool/superiotool.c:
https://review.coreboot.org/c/coreboot/+/83196/comment/ac06af3f_9b8173cf?us… :
PS5, Line 87: static void set_extra_selector(uint16_t port, const struct extra_selector *esel)
: {
: if (esel->idx == 0) /* entry without extra selector */
: return;
:
: uint8_t reg_val = regval(port, esel->idx);
: reg_val &= ~esel->mask;
: reg_val |= esel->val;
: regwrite(port, esel->idx, reg_val);
:
: reg_val = regval(port, esel->idx) & esel->mask;
:
: printf(" -- ESEL[%02xh] 0x%02x", esel->idx, reg_val);
: if (esel->name != NULL)
: printf(" (%s)", esel->name);
: printf(" --");
:
: if (verbose)
: printf(" config: idx=%02xh, mask=%02xh, val=%02xh state --", esel->idx, esel->mask,
: esel->val);
: }
:
> If you mean the absence of a line break \n, then it is already taken into account in dump_regs(). […]
Verbose is indeed verbose, but it doesn't seem too bad (user requested to be spammed anyway).
--
To view, visit https://review.coreboot.org/c/coreboot/+/83196?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If56af9f977381e637245bdd26563f5ba7e6cbead
Gerrit-Change-Number: 83196
Gerrit-PatchSet: 6
Gerrit-Owner: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 16 Jul 2024 13:17:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Maxim <max.senia.poliak(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Angel Pons, Felix Held, Paul Menzel.
Hello Angel Pons, Felix Held, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83196?usp=email
to look at the new patch set (#6).
Change subject: util/superiotool: Add extra selectors support
......................................................................
util/superiotool: Add extra selectors support
Some chips have specific selectors (in addition to LDN-register) that
affect the register space. At the same time, the utility doesn't provide
a simple and convenient method for configuring such selectors (as in the
case of LDN-selector) to create a dump, because this may be configured
by several fields of the register, and the values of its other fields
should not change after setting in the BIOS (fintek [1,2]).
Just add a structure with an index, mask, and value for the selector
inside the superio_registers chip for the corresponding LDN to switch
the register bank:
{FINTEK_F81966_DID, "F81962/F81964/F81966/F81967", {
* * *
{NOLDN, "Global",
{0x28,0x2a,0x2b,0x2c,EOT},
{0x00,0x00,0x00,0x00,EOT},
{.idx = 0x27, .mask = 0xd, .val = 0x1} /* update extra selector */
},
{0x03, "LPT",
{0x30,0x60,0x61,0x70,0x74,0xf0,EOT},
{NANA,0x03,0x78,0x07,0x03,0xc2,EOT} /* without extra selector */
},
* * *
Tested with Fintek F81966 on Asrock IMB-1222:
- run superiotool on Ubuntu and dump the registers for the board with
the vendor's firmware;
- add the superio chip initialization code to the board configuration
in coreboot and build the project;
- boot Ubuntu on the board with coreboot and re-dump the registers;
- the register values from the board configuration code are the same
in both dumps.
Found Fintek F81962/F81964/F81966/F81967 (vid=0x3419, id=0x0215) at 0x2e
(Global) -- ESEL[27h] 0x00 (Port Select Register) --
idx 02 07 20 21 23 24 25 26 27 28 29 2a 2b 2c 2d
val 00 0b 15 02 19 34 5a 23 80 a0 f0 45 02 e3 2e
def NA 00 15 02 19 34 00 23 02 a0 00 00 02 0c 28
* * *
The changes do not affect the configuration of existing chips, which
was tested on the Asrock H110-STX motherboard with Nuvoton NCT5539D
(the dump before and after the changes are the same).
[1] CB:83004
[2] CB:83019
Change-Id: If56af9f977381e637245bdd26563f5ba7e6cbead
Signed-off-by: Maxim Polyakov <max.senia.poliak(a)gmail.com>
---
M util/superiotool/superiotool.c
M util/superiotool/superiotool.h
2 files changed, 33 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/83196/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/83196?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If56af9f977381e637245bdd26563f5ba7e6cbead
Gerrit-Change-Number: 83196
Gerrit-PatchSet: 6
Gerrit-Owner: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Felix Singer, Jeremy Soller, Paul Menzel.
Tim Crawford has posted comments on this change by Tim Crawford. ( https://review.coreboot.org/c/coreboot/+/82609?usp=email )
Change subject: mb/system76/mtl: Add Darter Pro 10
......................................................................
Patch Set 6:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/82609/comment/01f82415_a949d582?us… :
PS4, Line 11: There are 2 variants
> Mention _B?
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/82609?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iaef03a47cf108591ef823bfa779777c7c05c6337
Gerrit-Change-Number: 82609
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 16 Jul 2024 12:25:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>