Change in coreboot[master]: [NOTFORMERGE] soc/intel/xeon_sp: Use common block ACPI

[Kyösti Mälkki (Code Review)]

Hello Marc Jones,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/48831

to review the following change.

Change subject: [NOTFORMERGE] soc/intel/xeon_sp: Use common block ACPI
......................................................................

[NOTFORMERGE] soc/intel/xeon_sp: Use common block ACPI

Squashed CB:48248 + CB:48826

Change-Id: I87f15571f217a1527f6c8e68072cb83ee55de01e
Signed-off-by: Marc Jones 
Signed-off-by: Kyösti Mälkki 
---
M src/soc/intel/xeon_sp/Kconfig
M src/soc/intel/xeon_sp/Makefile.inc
M src/soc/intel/xeon_sp/acpi.c
M src/soc/intel/xeon_sp/cpx/soc_acpi.c
M src/soc/intel/xeon_sp/include/soc/acpi.h
M src/soc/intel/xeon_sp/include/soc/pm.h
M src/soc/intel/xeon_sp/skx/soc_acpi.c
7 files changed, 33 insertions(+), 253 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/48831/1

diff --git a/src/soc/intel/xeon_sp/Kconfig b/src/soc/intel/xeon_sp/Kconfig
index 664f960..3b95c70 100644
--- a/src/soc/intel/xeon_sp/Kconfig
+++ b/src/soc/intel/xeon_sp/Kconfig
@@ -54,6 +54,7 @@
    select SOC_INTEL_COMMON_BLOCK_PMC_DISCOVERABLE
    select SOC_INTEL_COMMON_BLOCK_SMM
    select SOC_INTEL_COMMON_BLOCK_TCO
+	select SOC_INTEL_COMMON_BLOCK_ACPI
    select TSC_MONOTONIC_TIMER
    select UDELAY_TSC
    select SUPPORT_CPU_UCODE_IN_CBFS
diff --git a/src/soc/intel/xeon_sp/Makefile.inc b/src/soc/intel/xeon_sp/Makefile.inc
index cfda7d5..b9ce294 100644
--- a/src/soc/intel/xeon_sp/Makefile.inc
+++ b/src/soc/intel/xeon_sp/Makefile.inc
@@ -8,10 +8,10 @@
 bootblock-y += bootblock.c spi.c lpc.c gpio.c pch.c
 romstage-y += romstage.c reset.c util.c spi.c gpio.c pmutil.c memmap.c
 romstage-y += ../../../cpu/intel/car/romstage.c
-ramstage-y += uncore.c reset.c util.c lpc.c spi.c gpio.c nb_acpi.c ramstage.c chip_common.c
+ramstage-y += uncore.c reset.c util.c lpc.c spi.c gpio.c ramstage.c chip_common.c
 ramstage-y += memmap.c pch.c
 ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_PMC) += pmc.c
-ramstage-$(CONFIG_HAVE_ACPI_TABLES) += acpi.c
+ramstage-$(CONFIG_HAVE_ACPI_TABLES) += nb_acpi.c acpi.c
 ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smmrelocate.c
 smm-$(CONFIG_HAVE_SMI_HANDLER) += smihandler.c pmc.c
 postcar-y += spi.c
diff --git a/src/soc/intel/xeon_sp/acpi.c b/src/soc/intel/xeon_sp/acpi.c
index 4c17eea..b271264 100644
--- a/src/soc/intel/xeon_sp/acpi.c
+++ b/src/soc/intel/xeon_sp/acpi.c
@@ -1,19 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */

#include <assert.h>
-#include <acpi/acpi_gnvs.h>
-#include <acpi/acpigen.h>
-#include <arch/smp/mpspec.h>
-#include <device/pci.h>
-#include <cbmem.h>
-#include <cpu/x86/smm.h>
-#include <intelblocks/lpc_lib.h>
-#include <soc/acpi.h>
-#include <soc/cpu.h>
-#include <soc/intel/common/acpi.h>
+#include <intelblocks/acpi.h>
 #include <soc/pci_devs.h>
-#include <soc/pm.h>
-#include <soc/soc_util.h>
 #include <soc/util.h>
 #include <string.h>

@@ -23,72 +12,18 @@
    return NULL;
 }

-static int acpi_sci_irq(void)
-{
-	int sci_irq = 9;
-	uint32_t scis;
-
-	scis = soc_read_sci_irq_select();
-	scis &= SCI_IRQ_SEL;
-	scis >>= SCI_IRQ_ADJUST;
-
-	/* Determine how SCI is routed. */
-	switch (scis) {
-	case SCIS_IRQ9:
-	case SCIS_IRQ10:
-	case SCIS_IRQ11:
-		sci_irq = scis - SCIS_IRQ9 + 9;
-		break;
-	case SCIS_IRQ20:
-	case SCIS_IRQ21:
-	case SCIS_IRQ22:
-	case SCIS_IRQ23:
-		sci_irq = scis - SCIS_IRQ20 + 20;
-		break;
-	default:
-		printk(BIOS_DEBUG, "Invalid SCI route! Defaulting to IRQ9.\n");
-		sci_irq = 9;
-		break;
-	}
-
-	printk(BIOS_DEBUG, "SCI is IRQ%d\n", sci_irq);
-	return sci_irq;
-}
-
-static unsigned long acpi_madt_irq_overrides(unsigned long current)
-{
-	int sci = acpi_sci_irq();
-	uint16_t flags = MP_IRQ_TRIGGER_LEVEL;
-
-	/* INT_SRC_OVR */
-	current += acpi_create_madt_irqoverride((void *)current, 0, 0, 2, 0);
-
-	flags |= soc_madt_sci_irq_polarity(sci);
-
-	/* SCI */
-	current += acpi_create_madt_irqoverride((void *)current, 0, sci, sci, flags);
-
-	current +=
-		acpi_create_madt_lapic_nmi((acpi_madt_lapic_nmi_t *) current, 0xff, 0x0d, 1);
-
-	return current;
-}
-
-static unsigned long add_madt_ioapic(unsigned long current, int socket, int stack,
+static void print_madt_ioapic(int socket, int stack,
    			     int ioapic_id, uint32_t ioapic_base, int gsi_base)
 {
    printk(BIOS_DEBUG, "Adding MADT IOAPIC for socket: %d, stack: %d, ioapic_id: 0x%x, "
           "ioapic_base: 0x%x, gsi_base: 0x%x\n",
           socket, stack,  ioapic_id, ioapic_base, gsi_base);
-	return acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current, ioapic_id,
-				       ioapic_base, gsi_base);
+	return;
 }

-unsigned long acpi_fill_madt(unsigned long current)
+const struct madt_ioapic_info *soc_get_ioapic_info(size_t *entries)
 {
    int cur_index;
-	int ioapic_id;
-	int gsi_base;
    const IIO_UDS *hob = get_iio_uds();

/* With XEON-SP FSP, PCH IOAPIC is allocated with first 120 GSIs. */
@@ -99,15 +34,16 @@
 #if (CONFIG(SOC_INTEL_SKYLAKE_SP))
    const int gsi_bases[] = { 0, 0x18, 0x20, 0x28, 0x30, 0x48, 0x50, 0x58, 0x60 };
 #endif
-	/* Local APICs */
-	current = xeonsp_acpi_create_madt_lapics(current);
+
+	static struct madt_ioapic_info madt_tbl[ARRAY_SIZE(gsi_bases)];

cur_index = 0;
-	gsi_base = gsi_bases[cur_index];
-	current += add_madt_ioapic(current, 0, 0, PCH_IOAPIC_ID,
-				   hob->PlatformData.IIO_resource[0].StackRes[0].IoApicBase,
-				   gsi_base);
-		++cur_index;
+	madt_tbl[cur_index].id  = PCH_IOAPIC_ID;
+	madt_tbl[cur_index].addr = hob->PlatformData.IIO_resource[0].StackRes[0].IoApicBase;
+	madt_tbl[cur_index].gsi_base = gsi_bases[cur_index];
+	print_madt_ioapic(0, 0,  madt_tbl[cur_index].id,
+			  madt_tbl[cur_index].addr, madt_tbl[cur_index].gsi_base);
+	++cur_index;

for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) {
    	for (int stack = 0; stack < MAX_IIO_STACK; ++stack) {
@@ -116,177 +52,24 @@
    		if (!is_iio_stack_res(ri))
    			continue;
    		assert(cur_index < ARRAY_SIZE(gsi_bases));
-			ioapic_id = soc_get_iio_ioapicid(socket, stack);
-			gsi_base = gsi_bases[cur_index];
-			uint32_t ioapic_base = ri->IoApicBase;
+			madt_tbl[cur_index].id = soc_get_iio_ioapicid(socket, stack);
+			madt_tbl[cur_index].gsi_base = gsi_bases[cur_index];
+			madt_tbl[cur_index].addr = ri->IoApicBase;

/*
    		 * Stack 0 has non-PCH IOAPIC and PCH IOAPIC.
    		 * The IIO IOAPIC is placed at 0x1000 from the reported base.
    		 */
    		if (stack == 0 && socket == 0)
-				ioapic_base += 0x1000;
+				madt_tbl[cur_index].addr += 0x1000;

-			current += add_madt_ioapic(current, socket, stack, ioapic_id,
-						   ioapic_base, gsi_base);
+			print_madt_ioapic(socket, stack, madt_tbl[cur_index].id,
+					  madt_tbl[cur_index].addr,
+					  madt_tbl[cur_index].gsi_base);
    		++cur_index;
    	}
    }

-	return acpi_madt_irq_overrides(current);
-}
-
-void acpi_fill_fadt(acpi_fadt_t *fadt)
-{
-	const uint16_t pmbase = ACPI_BASE_ADDRESS;
-
-	fadt->header.revision = get_acpi_table_revision(FADT);
-
-	fadt->sci_int = acpi_sci_irq();
-
-	if (permanent_smi_handler()) {
-		fadt->smi_cmd = APM_CNT;
-		fadt->acpi_enable = APM_CNT_ACPI_ENABLE;
-		fadt->acpi_disable = APM_CNT_ACPI_DISABLE;
-	}
-
-	fadt->pm1a_evt_blk = pmbase + PM1_STS;
-	fadt->pm1a_cnt_blk = pmbase + PM1_CNT;
-
-	fadt->gpe0_blk = pmbase + GPE0_STS(0);
-
-	fadt->pm1_evt_len = 4;
-	fadt->pm1_cnt_len = 2;
-
-	/* GPE0 STS/EN pairs each 32 bits wide. */
-	fadt->gpe0_blk_len = 2 * GPE0_REG_MAX * sizeof(uint32_t);
-
-	fadt->duty_offset = 1;
-	fadt->day_alrm = 0xd;
-
-	fadt->flags |= ACPI_FADT_WBINVD | ACPI_FADT_C1_SUPPORTED |
-			ACPI_FADT_C2_MP_SUPPORTED | ACPI_FADT_SLEEP_BUTTON |
-			ACPI_FADT_SEALED_CASE | ACPI_FADT_S4_RTC_WAKE |
-			ACPI_FADT_PLATFORM_CLOCK;
-
-	fadt->x_pm1a_evt_blk.space_id = ACPI_ADDRESS_SPACE_IO;
-	fadt->x_pm1a_evt_blk.bit_width = fadt->pm1_evt_len * 8;
-	fadt->x_pm1a_evt_blk.addrl = pmbase + PM1_STS;
-	fadt->x_pm1a_evt_blk.access_size = ACPI_ACCESS_SIZE_WORD_ACCESS;
-
-	fadt->x_pm1a_cnt_blk.space_id = ACPI_ADDRESS_SPACE_IO;
-	fadt->x_pm1a_cnt_blk.bit_width = fadt->pm1_cnt_len * 8;
-	fadt->x_pm1a_cnt_blk.addrl = pmbase + PM1_CNT;
-	fadt->x_pm1a_cnt_blk.access_size = ACPI_ACCESS_SIZE_WORD_ACCESS;
-
-	/*
-	 * Windows 10 requires x_gpe0_blk to be set starting with FADT revision 5.
-	 * The bit_width field intentionally overflows here.
-	 * The OSPM can instead use the values in `fadt->gpe0_blk{,_len}`, which
-	 * seems to work fine on Linux 5.0 and Windows 10.
-	 */
-	fadt->x_gpe0_blk.space_id = ACPI_ADDRESS_SPACE_IO;
-	fadt->x_gpe0_blk.bit_width = fadt->gpe0_blk_len * 8;
-	fadt->x_gpe0_blk.bit_offset = 0;
-	fadt->x_gpe0_blk.access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS;
-	fadt->x_gpe0_blk.addrl = fadt->gpe0_blk;
-	fadt->x_gpe0_blk.addrh = 0;
-}
-
-unsigned long southbridge_write_acpi_tables(const struct device *device,
-					     unsigned long current,
-					     struct acpi_rsdp *rsdp)
-{
-	current = acpi_write_hpet(device, current, rsdp);
-	current = (ALIGN(current, 16));
-	printk(BIOS_DEBUG, "current = %lx\n", current);
-	return current;
-}
-
-void southbridge_inject_dsdt(const struct device *device)
-{
-	struct global_nvs *gnvs;
-
-	gnvs = cbmem_find(CBMEM_ID_ACPI_GNVS);
-	if (!gnvs) {
-		gnvs = cbmem_add(CBMEM_ID_ACPI_GNVS, sizeof(*gnvs));
-		if (gnvs)
-			memset(gnvs, 0, sizeof(*gnvs));
-	}
-
-	if (gnvs) {
-		acpi_create_gnvs(gnvs);
-		/* TODO: tell SMI about it, if HAVE_SMI_HANDLER */
-		// apm_control(APM_CNT_GNVS_UPDATE);
-
-		/* Add it to DSDT.  */
-		printk(BIOS_SPEW, "%s injecting NVSA with 0x%x\n", __FILE__, (uint32_t)gnvs);
-		acpigen_write_scope("\");
-		acpigen_write_name_dword("NVSA", (uint32_t)gnvs);
-		acpigen_pop_len();
-	}
-}
-
-int calculate_power(int tdp, int p1_ratio, int ratio)
-{
-	u32 m;
-	u32 power;
-
-	/*
-	 * M = ((1.1 - ((p1_ratio - ratio) * 0.00625)) / 1.1) ^ 2
-	 *
-	 * Power = (ratio / p1_ratio) * m * tdp
-	 */
-
-	m = (110000 - ((p1_ratio - ratio) * 625)) / 11;
-	m = (m * m) / 1000;
-
-	power = ((ratio * 100000 / p1_ratio) / 100);
-	power *= (m / 100) * (tdp / 1000);
-	power /= 1000;
-
-	return (int)power;
-}
-
-acpi_tstate_t *soc_get_tss_table(int *entries)
-{
-	*entries = 0;
-	return NULL;
-}
-
-void generate_cpu_entries(const struct device *device)
-{
-	int core_id, cpu_id, pcontrol_blk = ACPI_BASE_ADDRESS;
-	int plen = 6;
-	int total_threads = dev_count_cpu();
-	int threads_per_package = get_threads_per_package();
-	int numcpus = total_threads / threads_per_package;
-
-	printk(BIOS_DEBUG, "Found %d CPU(s) with %d core(s) each, totalcores: %d.\n",
-	       numcpus, threads_per_package, total_threads);
-
-	for (cpu_id = 0; cpu_id < numcpus; cpu_id++) {
-		for (core_id = 0; core_id < threads_per_package; core_id++) {
-			if (core_id > 0) {
-				pcontrol_blk = 0;
-				plen = 0;
-			}
-
-			/* Generate processor _PR.CPUx */
-			acpigen_write_processor((cpu_id) * threads_per_package +
-						core_id, pcontrol_blk, plen);
-
-			/* NOTE: Intel idle driver doesn't use ACPI C-state tables */
-
-			/* Soc specific power states generation */
-			soc_power_states_generation(core_id, threads_per_package);
-
-			acpigen_pop_len();
-		}
-	}
-	/* PPKG is usually used for thermal management of the first and only package. */
-	acpigen_write_processor_package("PPKG", 0, threads_per_package);
-
-	/* Add a method to notify processor nodes */
-	acpigen_write_processor_cnot(threads_per_package);
+	*entries = cur_index;
+	return madt_tbl;
 }
diff --git a/src/soc/intel/xeon_sp/cpx/soc_acpi.c b/src/soc/intel/xeon_sp/cpx/soc_acpi.c
index 8e73008..d6eff5f 100644
--- a/src/soc/intel/xeon_sp/cpx/soc_acpi.c
+++ b/src/soc/intel/xeon_sp/cpx/soc_acpi.c
@@ -8,6 +8,7 @@
 #include <cpu/intel/turbo.h>
 #include <device/mmio.h>
 #include <device/pci.h>
+#include <intelblocks/acpi.h>
 #include <intelblocks/cpulib.h>
 #include <soc/acpi.h>
 #include <soc/cpu.h>
@@ -218,7 +219,7 @@
         ratio >= ratio_min; ratio -= ratio_step) {

/* Calculate power at this ratio */
-		power = calculate_power(power_max, ratio_max, ratio);
+		power = common_calculate_power_ratio(power_max, ratio_max, ratio);
    	clock = ratio * CONFIG_CPU_BCLK_MHZ;
    	//clock = 1;
    	acpigen_write_PSS_package(
diff --git a/src/soc/intel/xeon_sp/include/soc/acpi.h b/src/soc/intel/xeon_sp/include/soc/acpi.h
index 743251b..a236843 100644
--- a/src/soc/intel/xeon_sp/include/soc/acpi.h
+++ b/src/soc/intel/xeon_sp/include/soc/acpi.h
@@ -11,20 +11,8 @@
    uint8_t   buf[32];
 } MEM_BLK;

-/* P-state configuration */
-#define PSS_MAX_ENTRIES			16
-#define PSS_RATIO_STEP			1
-#define PSS_LATENCY_TRANSITION		10
-#define PSS_LATENCY_BUSMASTER		10
-
 unsigned long northbridge_write_acpi_tables(const struct device *device,
    unsigned long current, struct acpi_rsdp *rsdp);
-uint32_t soc_read_sci_irq_select(void);
-int soc_madt_sci_irq_polarity(int sci);
-void soc_power_states_generation(int core, int cores_per_package);
-acpi_tstate_t *soc_get_tss_table(int *entries);
-acpi_cstate_t *soc_get_cstate_map(size_t *entries);
-int calculate_power(int tdp, int p1_ratio, int ratio);
 void uncore_inject_dsdt(const struct device *device);
 unsigned long xeonsp_acpi_create_madt_lapics(unsigned long current);

diff --git a/src/soc/intel/xeon_sp/include/soc/pm.h b/src/soc/intel/xeon_sp/include/soc/pm.h
index f5a45c6..5087e4f 100644
--- a/src/soc/intel/xeon_sp/include/soc/pm.h
+++ b/src/soc/intel/xeon_sp/include/soc/pm.h
@@ -70,6 +70,12 @@
 #define ENABLE_SMI_PARAMS \
    (APMC_EN | GBL_SMI_EN | EOS)

+/* P-state configuration */
+#define PSS_MAX_ENTRIES			16
+#define PSS_RATIO_STEP			1
+#define PSS_LATENCY_TRANSITION		10
+#define PSS_LATENCY_BUSMASTER		10
+
 /* This is defined as ETR3 in EDS. We named it as ETR here for consistency */
 #define ETR		0xac
 #define  CF9_LOCK	(1 << 31)
diff --git a/src/soc/intel/xeon_sp/skx/soc_acpi.c b/src/soc/intel/xeon_sp/skx/soc_acpi.c
index 53b9d27..3ac46ff 100644
--- a/src/soc/intel/xeon_sp/skx/soc_acpi.c
+++ b/src/soc/intel/xeon_sp/skx/soc_acpi.c
@@ -8,6 +8,7 @@
 #include <cpu/intel/turbo.h>
 #include <device/mmio.h>
 #include <device/pci.h>
+#include <intelblocks/acpi.h>
 #include <soc/acpi.h>
 #include <soc/cpu.h>
 #include <soc/iomap.h>

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48831 )

Change subject: [NOTFORMERGE] soc/intel/xeon_sp: Use common block ACPI
......................................................................

Abandoned