New subject: Change in coreboot[master]: soc/amd/picasso: Update northbridge

18 Jul 2019

Hello Martin Roth, Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/34424
to review the following change.
Change subject: _WIP_ soc/amd/picasso: Update northbridge
......................................................................
_WIP_ soc/amd/picasso: Update northbridge
todo: need to update northbridge_init() to set NP regions.  Waiting
on agreement for publishing NDA info in b:137781581
Family 17h devices is designed with a new internal architecture,
frequently referred to as the data fabric.  Although designed to
behave somewhat like the older integrated northbridge designs,
the D18Fx definitions are completely new.
AGESA sets up most of the necessary configuration.  coreboot still
enables the VGA decode if used.
Change-Id: I44a4a97765151fbcfe4c5d8de200e3e015aaaf2e
Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com
---
M src/soc/amd/picasso/chip.c
M src/soc/amd/picasso/include/soc/northbridge.h
M src/soc/amd/picasso/include/soc/pci_devs.h
M src/soc/amd/picasso/northbridge.c
4 files changed, 89 insertions(+), 287 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/34424/1

diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c
index d94091d..f34e696 100644
--- a/src/soc/amd/picasso/chip.c
+++ b/src/soc/amd/picasso/chip.c
@@ -120,6 +120,7 @@
 {
    fsp_silicon_init(acpi_is_wakeup_s3());
+	northbridge_init();
    southbridge_init(chip_info);
    setup_bsp_ramtop();
 }
diff --git a/src/soc/amd/picasso/include/soc/northbridge.h b/src/soc/amd/picasso/include/soc/northbridge.h
index cb683c2..1a440aa 100644
--- a/src/soc/amd/picasso/include/soc/northbridge.h
+++ b/src/soc/amd/picasso/include/soc/northbridge.h
@@ -20,58 +20,12 @@
 #include <device/device.h>
 #include <types.h>
-/* D1F1 - HDA Configuration Registers */
-#define HDA_DEV_CTRL_STATUS	0x60
-#define   HDA_NO_SNOOP_EN	BIT(11)
+/* D18F0 - Fabric Configuration registers */
+#define FABRIC_ID_IOMS0		9
-/* D18F0 - HT Configuration Registers */
-#define D18F0_NODE_ID		0x60
-#define D18F0_CPU_CNT		0x62 /* BKDG defines as a field in DWORD 0x60 */
-# define CPU_CNT_MASK		0x1f /*  CpuCnt + 1 = no. CPUs */
-#define HT_INIT_CONTROL		0x6c
-# define HTIC_BIOSR_DETECT	((1 << 5) | (1 << 9) | (1 << 10))
-# define HTIC_COLD_RST_DET	BIT(4)
-
-/* D18F1 - Address Map Registers */
-
-/* MMIO base and limit */
-#define D18F1_MMIO_BASE0_LO	0x80
-# define MMIO_WE		(1 << 1)
-# define MMIO_RE		(1 << 0)
-#define D18F1_MMIO_LIMIT0_LO	0x84
-# define MMIO_NP		(1 << 7)
-#define D18F1_IO_BASE0_LO	0xc0
-#define D18F1_IO_BASE1_LO	0xc8
-#define D18F1_IO_BASE2_LO	0xd0
-#define D18F1_IO_BASE3_LO	0xd8
-#define D18F1_MMIO_BASE7_LO	0xb8
-#define D18F1_MMIO_BASELIM0_HI	0x180
-#define D18F1_MMIO_BASE8_LO	0x1a0
-#define D18F1_MMIO_LIMIT8_LO	0x1a4
-#define D18F1_MMIO_BASE11_LO	0x1b8
-#define D18F1_MMIO_BASELIM8_HI	0x1c0
-#define NB_MMIO_BASE_LO(reg)	((reg) * 2 * sizeof(uint32_t) + (((reg) < 8) \
-					? D18F1_MMIO_BASE0_LO \
-					: D18F1_MMIO_BASE8_LO \
-						- 8 * sizeof(uint64_t)))
-#define NB_MMIO_LIMIT_LO(reg)	(NB_MMIO_BASE_LO(reg) + sizeof(uint32_t))
-#define NB_MMIO_BASELIM_HI(reg)	((reg) * sizeof(uint32_t) + (((reg) < 8) \
-					? D18F1_MMIO_BASELIM0_HI \
-					: D18F1_MMIO_BASELIM8_HI \
-						- 8 * sizeof(uint32_t)))
-/* I/O base and limit */
-#define D18F1_IO_BASE0		0xc0
-# define IO_WE			(1 << 1)
-# define IO_RE			(1 << 0)
-#define D18F1_IO_LIMIT0		0xc4
-#define NB_IO_BASE(reg)		((reg) * 2 * sizeof(uint32_t) + D18F1_IO_BASE0)
-#define NB_IO_LIMIT(reg)	(NB_IO_BASE(reg) + sizeof(uint32_t))
-
-#define D18F1_DRAM_HOLE		0xf0
-# define DRAM_HOIST_VALID	(1 << 1)
-# define DRAM_HOLE_VALID	(1 << 0)
-#define D18F1_VGAEN		0xf4
-# define VGA_ADDR_ENABLE	(1 << 0)
+#define D18F0_VGAEN		0x80
+#define   VGA_ADDR_ENABLE	BIT(0)
+#define   VGA_DST_ID_SH	4
enum {
    /* SMM handler area. */
@@ -98,7 +52,7 @@
 int smm_subregion(int sub, void **start, size_t *size);
 void domain_enable_resources(struct device *dev);
 void domain_set_resources(struct device *dev);
-void fam15_finalize(void *chip_info);
+void northbridge_init(void);
 void set_warm_reset_flag(void);
 int is_warm_reset(void);
diff --git a/src/soc/amd/picasso/include/soc/pci_devs.h b/src/soc/amd/picasso/include/soc/pci_devs.h
index acde455..9a9c184 100644
--- a/src/soc/amd/picasso/include/soc/pci_devs.h
+++ b/src/soc/amd/picasso/include/soc/pci_devs.h
@@ -102,47 +102,36 @@
 #define HDA1_DEVFN		PCI_DEVFN(HDA1_DEV, HDA1_FUNC)
 #define SOC_HDA1_DEV		_SOC_DEV(HDA1_DEV, HDA1_FUNC)
-/* HT Configuration */
-#define HT_DEV			0x18
-#define HT_FUNC			0
-#define HT_DEVID		0x15b0
-#define HT_DEVFN		PCI_DEVFN(HT_DEV, HT_FUNC)
-#define SOC_HT_DEV		_SOC_DEV(HT_DEV, HT_FUNC)
+/* Data Fabric functions */
+#define DF_DEV			0x18
-/* Address Maps */
-#define ADDR_DEV		0x18
-#define ADDR_FUNC		1
-#define ADDR_DEVID		0x15b1
-#define ADDR_DEVFN		PCI_DEVFN(ADDR_DEV, ADDR_FUNC)
-#define SOC_ADDR_DEV		_SOC_DEV(ADDR_DEV, ADDR_FUNC)
+#define DF_F0_DEVID		0x15e8
+#define DF_F0_DEVFN		PCI_DEVFN(DF_DEV, 0)
+#define SOC_DF_F0_DEVFN		_SOC_DEV(DF_DEV, 0)
-/* DRAM Configuration */
-#define DCT_DEV			0x18
-#define DCT_FUNC		2
-#define DCT_DEVID		0x15b2
-#define DCT_DEVFN		PCI_DEVFN(DCT_DEV, DCT_FUNC)
-#define SOC_DCT_DEV		_SOC_DEV(DCT_DEV, DCT_FUNC)
+#define DF_F1_DEVID		0x15e9
+#define DF_F1_DEVFN		PCI_DEVFN(DF_DEV, 1)
+#define SOC_DF_F1_DEVFN		_SOC_DEV(DF_DEV, 1)
-/* Misc. Configuration */
-#define MISC_DEV		0x18
-#define MISC_FUNC		3
-#define MISC_DEVID		0x15b3
-#define MISC_DEVFN		PCI_DEVFN(MISC_DEV, MISC_FUNC)
-#define SOC_MISC_DEV		_SOC_DEV(MISC_DEV, MISC_FUNC)
+#define DF_F2_DEVID		0x15ea
+#define DF_F2_DEVFN		PCI_DEVFN(DF_DEV, 2)
+#define SOC_DF_F2_DEVFN		_SOC_DEV(DF_DEV, 2)
-/* PM Configuration */
-#define PM_DEV			0x18
-#define PM_FUNC			4
-#define PM_DEVID		0x15b4
-#define PM_DEVFN		PCI_DEVFN(PM_DEV, PM_FUNC)
-#define SOC_PM_DEV		_SOC_DEV(PM_DEV, PM_FUNC)
+#define DF_F3_DEVID		0x15eb
+#define DF_F3_DEVFN		PCI_DEVFN(DF_DEV, 3)
+#define SOC_DF_F3_DEVFN		_SOC_DEV(DF_DEV, 3)
-/* Northbridge Configuration */
-#define NB_DEV			0x18
-#define NB_FUNC			5
-#define NB_DEVID		0x15b5
-#define NB_DEVFN		PCI_DEVFN(NB_DEV, NB_FUNC)
-#define SOC_NB_DEV		_SOC_DEV(NB_DEV, NB_FUNC)
+#define DF_F4_DEVID		0x15ec
+#define DF_F4_DEVFN		PCI_DEVFN(DF_DEV, 4)
+#define SOC_DF_F4_DEVFN		_SOC_DEV(DF_DEV, 4)
+
+#define DF_F5_DEVID		0x15ed
+#define DF_F5_DEVFN		PCI_DEVFN(DF_DEV, 5)
+#define SOC_DF_F5_DEVFN		_SOC_DEV(DF_DEV, 5)
+
+#define DF_F6_DEVID		0x15ee
+#define DF_F6_DEVFN		PCI_DEVFN(DF_DEV, 6)
+#define SOC_DF_F6_DEVFN		_SOC_DEV(DF_DEV, 6)
/* USB 3.1 */
 #define XHCI0_DEV		0x0
diff --git a/src/soc/amd/picasso/northbridge.c b/src/soc/amd/picasso/northbridge.c
index 2335d0d..0b30efa 100644
--- a/src/soc/amd/picasso/northbridge.c
+++ b/src/soc/amd/picasso/northbridge.c
@@ -36,128 +36,63 @@
 #include <stdlib.h>
 #include <string.h>
 #include <arch/bert_storage.h>
+#include <fsp/util.h>
#include "chip.h"
-static void set_io_addr_reg(struct device *dev, u32 nodeid, u32 linkn, u32 reg,
-			u32 io_min, u32 io_max)
-{
-	u32 tempreg;
-
-	/* io range allocation.  Limit */
-	tempreg = (nodeid & 0xf) | ((nodeid & 0x30) << (8 - 4)) | (linkn << 4)
-						| ((io_max & 0xf0) << (12 - 4));
-	pci_write_config32(SOC_ADDR_DEV, reg + 4, tempreg);
-	tempreg = 3 | ((io_min & 0xf0) << (12 - 4)); /* base: ISA and VGA ? */
-	pci_write_config32(SOC_ADDR_DEV, reg, tempreg);
-}
-
-static void set_mmio_addr_reg(u32 nodeid, u32 linkn, u32 reg, u32 index,
-						u32 mmio_min, u32 mmio_max)
-{
-	u32 tempreg;
-
-	/* io range allocation.  Limit */
-	tempreg = (nodeid & 0xf) | (linkn << 4) | (mmio_max & 0xffffff00);
-		pci_write_config32(SOC_ADDR_DEV, reg + 4, tempreg);
-	tempreg = 3 | (nodeid & 0x30) | (mmio_min & 0xffffff00);
-		pci_write_config32(SOC_ADDR_DEV, reg, tempreg);
-}
-
 static void read_resources(struct device *dev)
 {
-	/*
-	 * This MMCONF resource must be reserved in the PCI domain.
-	 * It is not honored by the coreboot resource allocator if it is in
-	 * the CPU_CLUSTER.
-	 */
+	msr_t tom = rdmsr(TOP_MEM);
+	uint32_t mem_useable = (uintptr_t)cbmem_top();
+	int idx = 0x10;
+	const struct hob_header *hob = fsp_get_hob_list();
+	struct hob_resource *res;
+
+	/* 0x0 - 0x9ffff */
+	ram_resource(dev, idx++, 0, 0xa0000 / KiB);
+
+	/* 0xa0000 - 0xbffff: legacy VGA */
+	mmio_resource(dev, idx++, 0xa0000 / KiB, 0x20000 / KiB);
+
+	/* 0xc0000 - 0xfffff: Option ROM */
+	reserved_ram_resource(dev, idx++, 0xc0000 / KiB, 0x40000 / KiB);
+
+	/* 0x100000 (1MiB) - low top useable RAM
+	 * cbmem_top() accounts for low UMA and TSEG if they are used. */
+	ram_resource(dev, idx++, (1 * MiB) / KiB,
+			(mem_useable - (1 * MiB)) / KiB);
+
+	/* Low top useable RAM -> Low top RAM (bottom pci mmio hole) */
+	reserved_ram_resource(dev, idx++, mem_useable / KiB,
+					(tom.lo - mem_useable) / KiB);
+
    mmconf_resource(dev, MMIO_CONF_BASE);
-}
-static void set_resource(struct device *dev, struct resource *res, u32 nodeid)
-{
-	resource_t rbase, rend;
-	unsigned int reg, link_num;
-	char buf[50];
-
-	/* Make certain the resource has actually been set */
-	if (!(res->flags & IORESOURCE_ASSIGNED))
+	if (!hob) {
+		printk(BIOS_ERR, "Error: %s incomplete becuase no HOB list was found\n",
+				__func__);
    	return;
+	}
-	/* If I have already stored this resource don't worry about it */
-	if (res->flags & IORESOURCE_STORED)
-		return;
+	for (; hob->type != HOB_TYPE_END_OF_HOB_LIST; hob = fsp_next_hob(hob)) {
-	/* Only handle PCI memory and IO resources */
-	if (!(res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
-		return;
+		if (hob->type != HOB_TYPE_RESOURCE_DESCRIPTOR)
+			continue;
-	/* Ensure I am actually looking at a resource of function 1 */
-	if ((res->index & 0xffff) < 0x1000)
-		return;
+		res = (struct hob_resource *)fsp_hob_header_to_resource(hob);
-	/* Get the base address */
-	rbase = res->base;
-
-	/* Get the limit (rounded up) */
-	rend  = resource_end(res);
-
-	/* Get the register and link */
-	reg  = res->index & 0xfff; /* 4k */
-	link_num = IOINDEX_LINK(res->index);
-
-	if (res->flags & IORESOURCE_IO)
-		set_io_addr_reg(dev, nodeid, link_num, reg, rbase>>8, rend>>8);
-	else if (res->flags & IORESOURCE_MEM)
-		set_mmio_addr_reg(nodeid, link_num, reg,
-				(res->index >> 24), rbase >> 8, rend >> 8);
-
-	res->flags |= IORESOURCE_STORED;
-	snprintf(buf, sizeof(buf), " <node %x link %x>",
-			nodeid, link_num);
-	report_resource_stored(dev, res, buf);
-}
-
-/**
- * I tried to reuse the resource allocation code in set_resource()
- * but it is too difficult to deal with the resource allocation magic.
- */
-
-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)
-		return;
-
-	printk(BIOS_DEBUG, "VGA: %s has VGA device\n",	dev_path(dev));
-	/* Route A0000-BFFFF, IO 3B0-3BB 3C0-3DF */
-	pci_write_config32(SOC_ADDR_DEV, D18F1_VGAEN, VGA_ADDR_ENABLE);
-}
-
-static void set_resources(struct device *dev)
-{
-	struct bus *bus;
-	struct resource *res;
-
-
-	/* do we need this? */
-	create_vga_resource(dev);
-
-	/* Set each resource we have found */
-	for (res = dev->resource_list ; res ; res = res->next)
-		set_resource(dev, res, 0);
-
-	for (bus = dev->link_list ; bus ; bus = bus->next)
-		if (bus->children)
-			assign_resources(bus);
+		if (res->type == EFI_RESOURCE_SYSTEM_MEMORY && !res->addr)
+			continue; /* 0 through TOM was set above */
+		else if (res->type == EFI_RESOURCE_SYSTEM_MEMORY)
+			ram_resource(dev, idx++, res->addr / KiB, res->length / KiB);
+		else if (res->type == EFI_RESOURCE_MEMORY_MAPPED_IO)
+			continue;
+		else if (res->type == EFI_RESOURCE_MEMORY_RESERVED)
+			reserved_ram_resource(dev, idx++, res->addr / KiB, res->length / KiB);
+		else
+			printk(BIOS_ERR, "Error: failed to set resources for type %d\n",
+					res->type);
+	}
 }
unsigned long acpi_fill_mcfg(unsigned long current)
@@ -203,7 +138,6 @@
static struct device_operations northbridge_operations = {
    .read_resources	  = read_resources,
-	.set_resources	  = set_resources,
    .enable_resources = pci_dev_enable_resources,
    .acpi_fill_ssdt_generator = northbridge_fill_ssdt_generator,
    .write_acpi_tables = agesa_write_acpi_tables,
@@ -211,103 +145,27 @@
    .ops_pci	  = 0,
 };
-static const struct pci_driver family15_northbridge __pci_driver = {
+static const struct pci_driver family17_northbridge __pci_driver = {
    .ops	= &northbridge_operations,
    .vendor = PCI_VENDOR_ID_AMD,
-	.device = PCI_DEVICE_ID_AMD_15H_MODEL_707F_NB_HT,
+	.device = PCI_DEVICE_ID_AMD_17H_MODEL_101F_NB,
 };
-/*
- * Enable VGA cycles.  Set memory ranges of the FCH legacy devices (TPM, HPET,
- * BIOS RAM, Watchdog Timer, IOAPIC and ACPI) as non-posted.  Set remaining
- * MMIO to posted.  Route all I/O to the southbridge.
- */
-void amd_initcpuio(void)
+void northbridge_init(void)
 {
-	uintptr_t topmem = bsp_topmem();
-	uintptr_t base, limit;
-
-	/* Enable legacy video routing: D18F1xF4 VGA Enable */
-	pci_write_config32(SOC_ADDR_DEV, D18F1_VGAEN, VGA_ADDR_ENABLE);
-
-	/* Non-posted: range(HPET-LAPIC) or 0xfed00000 through 0xfee00000-1 */
-	base = (HPET_BASE_ADDRESS >> 8) | MMIO_WE | MMIO_RE;
-	limit = (ALIGN_DOWN(LOCAL_APIC_ADDR - 1, 64 * KiB) >> 8) | MMIO_NP;
-	pci_write_config32(SOC_ADDR_DEV, NB_MMIO_LIMIT_LO(0), limit);
-	pci_write_config32(SOC_ADDR_DEV, NB_MMIO_BASE_LO(0), base);
-
-	/* Remaining PCI hole posted MMIO: TOM-HPET (TOM through 0xfed00000-1 */
-	base = (topmem >> 8) | MMIO_WE | MMIO_RE;
-	limit = ALIGN_DOWN(HPET_BASE_ADDRESS - 1, 64 * KiB) >> 8;
-	pci_write_config32(SOC_ADDR_DEV, NB_MMIO_LIMIT_LO(1), limit);
-	pci_write_config32(SOC_ADDR_DEV, NB_MMIO_BASE_LO(1), base);
-
-	/* Route all I/O downstream */
-	base = 0 | IO_WE | IO_RE;
-	limit = ALIGN_DOWN(0xffff, 4 * KiB);
-	pci_write_config32(SOC_ADDR_DEV, NB_IO_LIMIT(0), limit);
-	pci_write_config32(SOC_ADDR_DEV, NB_IO_BASE(0), base);
-}
-
-void fam15_finalize(void *chip_info)
-{
-	u32 value;
-
-	/* disable No Snoop */
-	value = pci_read_config32(SOC_HDA0_DEV, HDA_DEV_CTRL_STATUS);
-	value &= ~HDA_NO_SNOOP_EN;
-	pci_write_config32(SOC_HDA0_DEV, HDA_DEV_CTRL_STATUS, value);
+	/*
+	 * AGESA has already programmed the NB MMIO routing, however nothing
+	 * is yet marked as non-posted.
+	 *
+	 * TODO: Remove the settings from AGESA and allow coreboot to own
+	 * everything.  If not practical, consider erasing all settings and
+	 * reprogram for coreboot.  At that time, make the following more
+	 * flexible.
+	 */
 }
void domain_set_resources(struct device *dev)
 {
-	uint64_t uma_base = get_uma_base();
-	uint32_t uma_size = get_uma_size();
-	uint32_t mem_useable = (uintptr_t)cbmem_top();
-	msr_t tom = rdmsr(TOP_MEM);
-	msr_t high_tom = rdmsr(TOP_MEM2);
-	uint64_t high_mem_useable;
-	int idx = 0x10;
-
-	/* 0x0 -> 0x9ffff */
-	ram_resource(dev, idx++, 0, 0xa0000 / KiB);
-
-	/* 0xa0000 -> 0xbffff: legacy VGA */
-	mmio_resource(dev, idx++, 0xa0000 / KiB, 0x20000 / KiB);
-
-	/* 0xc0000 -> 0xfffff: Option ROM */
-	reserved_ram_resource(dev, idx++, 0xc0000 / KiB, 0x40000 / KiB);
-
-	/*
-	 * 0x100000 (1MiB) -> low top useable RAM
-	 * cbmem_top() accounts for low UMA and TSEG if they are used.
-	 */
-	ram_resource(dev, idx++, (1 * MiB) / KiB,
-			(mem_useable - (1 * MiB)) / KiB);
-
-	/* Low top useable RAM -> Low top RAM (bottom pci mmio hole) */
-	reserved_ram_resource(dev, idx++, mem_useable / KiB,
-					(tom.lo - mem_useable) / KiB);
-
-	/* If there is memory above 4GiB */
-	if (high_tom.hi) {
-		/* 4GiB -> high top useable */
-		if (uma_base >= (4ull * GiB))
-			high_mem_useable = uma_base;
-		else
-			high_mem_useable = ((uint64_t)high_tom.lo |
-						((uint64_t)high_tom.hi << 32));
-
-		ram_resource(dev, idx++, (4ull * GiB) / KiB,
-				((high_mem_useable - (4ull * GiB)) / KiB));
-
-		/* High top useable RAM -> high top RAM */
-		if (uma_base >= (4ull * GiB)) {
-			reserved_ram_resource(dev, idx++, uma_base / KiB,
-						uma_size / KiB);
-		}
-	}
-
    assign_resources(dev->link_list);
 }
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/34424
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I44a4a97765151fbcfe4c5d8de200e3e015aaaf2e
Gerrit-Change-Number: 34424
Gerrit-PatchSet: 1
Gerrit-Owner: Marshall Dawson marshalldawson3rd@gmail.com
Gerrit-Reviewer: Furquan Shaikh furquan@google.com
Gerrit-Reviewer: Martin Roth martinroth@google.com
Gerrit-MessageType: newchange



    

Change in coreboot[master]: _WIP_ soc/amd/picasso: Update northbridge