Attention is currently required from: Kapil Porwal, Nick Vaccaro, V Sowmya.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80688?usp=email )
Change subject: soc/intel/alderlake: Add kconfig for Twin Lake
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80688?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: Ie4c5d137ee54512313344f853e7ca66d1fd25003
Gerrit-Change-Number: 80688
Gerrit-PatchSet: 4
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Fri, 23 Feb 2024 09:01:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Christian Walter, Felix Held, Jincheng Li, Johnny Lin, Nico Huber, Patrick Rudolph, Tim Chu.
Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80632?usp=email )
Change subject: soc/intel/xeon_sp: Add memory type check utils
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/xeon_sp/util.c:
https://review.coreboot.org/c/coreboot/+/80632/comment/8ccc8ce9_dd7b968a :
PS3, Line 126: __weak bool is_memtype_processor_attached(UINT16 mem_type)
: {
: return true;
: }
:
> The inline is conflict with weak, as below, […]
For this case, if we only inline but not add static, the optimization out will not happen. However, if we define static, we need to do this in header file which is not straightforward to support overrides unless a corresponding Kconfig is used.
This util checkers is rarely used and currently it only causes an 'continue' in uncore_acpi.c/ln110, hence even not being optimized out, the overheads can be quite minimal. So should we keep it as is? Or any other suggestions?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80632?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: I2f5f3c0f16dc50bc739146e46afce2e5fbf4f62c
Gerrit-Change-Number: 80632
Gerrit-PatchSet: 3
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: Jincheng Li <jincheng.li(a)intel.com>
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: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Fri, 23 Feb 2024 08:52:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Julius Werner, Paul Menzel.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69747?usp=email )
Change subject: arch/arm: Build test all arm targets with clang
......................................................................
Patch Set 20:
(1 comment)
File src/soc/nvidia/tegra124/Kconfig:
https://review.coreboot.org/c/coreboot/+/69747/comment/ac538fec_4d1131a6 :
PS20, Line 20: default n if SOC_NVIDIA_TEGRA124 && CHROMEOS
> Why not move these into the `if SOC_...` blocks below?
That does not work. Kconfig then thinks SOC_NVIDIA_TEGRA124 is a dependency.
--
To view, visit https://review.coreboot.org/c/coreboot/+/69747?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: I88cf8ce16fb6c61c19d615e396f5871179b06fc8
Gerrit-Change-Number: 69747
Gerrit-PatchSet: 20
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Fri, 23 Feb 2024 08:39:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal, Nick Vaccaro, Subrata Banik, V Sowmya.
Hello Nick Vaccaro, Subrata Banik, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80688?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+2 by Subrata Banik, Code-Review+2 by V Sowmya, Verified+1 by build bot (Jenkins)
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: soc/intel/alderlake: Add kconfig for Twin Lake
......................................................................
soc/intel/alderlake: Add kconfig for Twin Lake
Mainboards using Intel Twin Lake (TWL) SoC shall select
SOC_INTEL_TWINLAKE.
BUG=none
BRANCH=firmware-nissa-15217.B
TEST=Build and boot Google/Yaviks with Twin Lake kconfig enabled
Signed-off-by: Kapil Porwal <kapilporwal(a)google.com>
Change-Id: Ie4c5d137ee54512313344f853e7ca66d1fd25003
---
M src/soc/intel/alderlake/Kconfig
1 file changed, 9 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/80688/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/80688?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: Ie4c5d137ee54512313344f853e7ca66d1fd25003
Gerrit-Change-Number: 80688
Gerrit-PatchSet: 4
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-MessageType: newpatchset
Patrick Rudolph has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80551?usp=email )
Change subject: soc/intel/xeon_sp: Locate PCI devices by Ven/Dev ID
......................................................................
soc/intel/xeon_sp: Locate PCI devices by Ven/Dev ID
Since the ACPI code is looking for VtdBars, that only appear on
Vtd devices, search for the Vtd device in devicetree.
With the previous commit the VtdBar is now exposed as a resource
on the Vtd device and thus can easily be accessed and used.
Drop the FSP HOB parsing and just use coreboot native functions.
Allows the code to work with multiple PCI segment groups.
Change-Id: I2c752dc595ac4c901f2b3a96718e256e413c76a7
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80551
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz>
Reviewed-by: Shuo Liu <shuo.liu(a)intel.com>
---
M src/soc/intel/xeon_sp/uncore_acpi.c
1 file changed, 43 insertions(+), 34 deletions(-)
Approvals:
Shuo Liu: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
Arthur Heymans: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/uncore_acpi.c b/src/soc/intel/xeon_sp/uncore_acpi.c
index a9a5bac..5ee9087 100644
--- a/src/soc/intel/xeon_sp/uncore_acpi.c
+++ b/src/soc/intel/xeon_sp/uncore_acpi.c
@@ -10,7 +10,9 @@
#include <device/mmio.h>
#include <device/pci.h>
#include <device/pciexp.h>
+#include <device/pci_ids.h>
#include <soc/acpi.h>
+#include <soc/chip_common.h>
#include <soc/hest.h>
#include <soc/iomap.h>
#include <soc/numa.h>
@@ -18,7 +20,6 @@
#include <soc/soc_util.h>
#include <soc/util.h>
#include <intelblocks/p2sb.h>
-
#include "chip.h"
/* NUMA related ACPI table generation. SRAT, SLIT, etc */
@@ -382,26 +383,39 @@
static unsigned long acpi_create_atsr(unsigned long current, const IIO_UDS *hob)
{
+ struct device *child, *dev;
+ struct resource *resource;
+
+ /*
+ * The assumption made here is that the host bridges on a socket share the
+ * PCI segment group and thus only one ATSR header needs to be emitted for
+ * a single socket.
+ * This is easier than to sort the host bridges by PCI segment group first
+ * and then generate one ATSR header for every new segment.
+ */
for (int socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; ++socket) {
if (!soc_cpu_is_enabled(socket))
continue;
iio++;
-
- uint32_t pcie_seg = hob->PlatformData.CpuQpiInfo[socket].PcieSegment;
unsigned long tmp = current;
bool first = true;
- IIO_RESOURCE_INSTANCE iio_resource =
- hob->PlatformData.IIO_resource[socket];
- for (int stack = 0; stack < MAX_LOGIC_IIO_STACK; ++stack) {
- uint32_t bus = iio_resource.StackRes[stack].BusBase;
- uint32_t vtd_base = iio_resource.StackRes[stack].VtdBarAddress;
- if (!vtd_base)
+ dev = NULL;
+ while ((dev = dev_find_device(PCI_VID_INTEL, MMAP_VTD_CFG_REG_DEVID, dev))) {
+ /* Only add devices for the current socket */
+ if (iio_pci_domain_socket_from_dev(dev) != socket)
continue;
- uint64_t vtd_mmio_cap = read64p(vtd_base + VTD_EXT_CAP_LOW);
- printk(BIOS_SPEW, "%s socket: %d, stack: %d, bus: 0x%x, vtd_base: 0x%x, "
+ /* See if there is a resource with the appropriate index. */
+ resource = probe_resource(dev, VTD_BAR_CSR);
+ if (!resource)
+ continue;
+ int stack = iio_pci_domain_stack_from_dev(dev);
+
+ uint64_t vtd_mmio_cap = read64(res2mmio(resource, VTD_EXT_CAP_LOW, 0));
+ printk(BIOS_SPEW, "%s socket: %d, stack: %d, bus: 0x%x, vtd_base: %p, "
"vtd_mmio_cap: 0x%llx\n",
- __func__, socket, stack, bus, vtd_base, vtd_mmio_cap);
+ __func__, socket, stack, dev->upstream->secondary,
+ res2mmio(resource, 0, 0), vtd_mmio_cap);
// ATSR is applicable only for platform supporting device IOTLBs
// through the VT-d extended capability register
@@ -409,17 +423,15 @@
if ((vtd_mmio_cap & 0x4) == 0) // BIT 2
continue;
- if (bus == 0)
+ if (dev->upstream->secondary == 0 && dev->upstream->segment_group == 0)
continue;
- struct device *dev = pcidev_path_on_bus(bus, PCI_DEVFN(0, 0));
- while (dev) {
- if ((dev->hdr_type & 0x7f) == PCI_HEADER_TYPE_BRIDGE)
- current +=
+ for (child = dev->upstream->children; child; child = child->sibling) {
+ if ((child->hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE)
+ continue;
+ current +=
acpi_create_dmar_ds_pci_br_for_port(
- current, dev, pcie_seg, true, &first);
-
- dev = dev->sibling;
+ current, child, child->upstream->segment_group, true, &first);
}
}
if (tmp != current)
@@ -463,24 +475,21 @@
static unsigned long acpi_create_rhsa(unsigned long current)
{
- const IIO_UDS *hob = get_iio_uds();
+ struct device *dev = NULL;
+ struct resource *resource;
+ int socket;
- for (int socket = 0, iio = 0; iio < hob->PlatformData.numofIIO; ++socket) {
- if (!soc_cpu_is_enabled(socket))
+ while ((dev = dev_find_device(PCI_VID_INTEL, MMAP_VTD_CFG_REG_DEVID, dev))) {
+ /* See if there is a resource with the appropriate index. */
+ resource = probe_resource(dev, VTD_BAR_CSR);
+ if (!resource)
continue;
- iio++;
- IIO_RESOURCE_INSTANCE iio_resource =
- hob->PlatformData.IIO_resource[socket];
- for (int stack = 0; stack < MAX_LOGIC_IIO_STACK; ++stack) {
- uint32_t vtd_base = iio_resource.StackRes[stack].VtdBarAddress;
- if (!vtd_base)
- continue;
+ socket = iio_pci_domain_socket_from_dev(dev);
- printk(BIOS_DEBUG, "[Remapping Hardware Static Affinity] Base Address: 0x%x, "
- "Proximity Domain: 0x%x\n", vtd_base, socket);
- current += acpi_create_dmar_rhsa(current, vtd_base, socket);
- }
+ printk(BIOS_DEBUG, "[Remapping Hardware Static Affinity] Base Address: %p, "
+ "Proximity Domain: 0x%x\n", res2mmio(resource, 0, 0), socket);
+ current += acpi_create_dmar_rhsa(current, (uintptr_t)res2mmio(resource, 0, 0), socket);
}
return current;
--
To view, visit https://review.coreboot.org/c/coreboot/+/80551?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: I2c752dc595ac4c901f2b3a96718e256e413c76a7
Gerrit-Change-Number: 80551
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
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-MessageType: merged