Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52761 )
Change subject: northbridge/amd/pi/00730F01: enable RESOURCE_ALLOCATOR_V4 ......................................................................
northbridge/amd/pi/00730F01: enable RESOURCE_ALLOCATOR_V4
Do not use get_dram_base_mask to calculate system DRAM limits. Shift operation around values operating on base and mask were causing overflows and thus incorrect system DRAM limit. Another function returning base and limit in KiB has been developed to avoid data loss. Additionally remove obsolete resource-setting functions and correctly report all DRAM and reserved resources.
TEST=boot Debian with Linux 4.14 on apu2 4GB ECC and apu3 2GB no ECC
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: I0387071748262fdeaa5f4d9a71bb87d4d83241b6 --- M src/northbridge/amd/pi/00730F01/Kconfig M src/northbridge/amd/pi/00730F01/northbridge.c 2 files changed, 94 insertions(+), 293 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/52761/1
diff --git a/src/northbridge/amd/pi/00730F01/Kconfig b/src/northbridge/amd/pi/00730F01/Kconfig index 0aaaf49..6d321e9 100644 --- a/src/northbridge/amd/pi/00730F01/Kconfig +++ b/src/northbridge/amd/pi/00730F01/Kconfig @@ -2,7 +2,6 @@
config NORTHBRIDGE_AMD_PI_00730F01 bool - select RESOURCE_ALLOCATOR_V3
if NORTHBRIDGE_AMD_PI_00730F01
diff --git a/src/northbridge/amd/pi/00730F01/northbridge.c b/src/northbridge/amd/pi/00730F01/northbridge.c index 2019fae..e08d86d 100644 --- a/src/northbridge/amd/pi/00730F01/northbridge.c +++ b/src/northbridge/amd/pi/00730F01/northbridge.c @@ -29,11 +29,6 @@ #define PCIE_CAP_AER BIT(5) #define PCIE_CAP_ACS BIT(6)
-typedef struct dram_base_mask { - u32 base; //[47:27] at [28:8] - u32 mask; //[47:27] at [28:8] and enable at bit 0 -} dram_base_mask_t; - static unsigned int node_nums; static unsigned int sblink; static struct device *__f0_dev[MAX_NODE_NUMS]; @@ -42,51 +37,6 @@ static struct device *__f4_dev[MAX_NODE_NUMS]; static unsigned int fx_devs = 0;
-static dram_base_mask_t get_dram_base_mask(u32 nodeid) -{ - struct device *dev; - dram_base_mask_t d; - dev = __f1_dev[0]; - u32 temp; - temp = pci_read_config32(dev, 0x44 + (nodeid << 3)); //[39:24] at [31:16] - d.mask = ((temp & 0xfff80000)>>(8+3)); // mask out DramMask [26:24] too - temp = pci_read_config32(dev, 0x144 + (nodeid <<3)) & 0xff; //[47:40] at [7:0] - d.mask |= temp<<21; - temp = pci_read_config32(dev, 0x40 + (nodeid << 3)); //[39:24] at [31:16] - d.mask |= (temp & 1); // enable bit - d.base = ((temp & 0xfff80000)>>(8+3)); // mask out DramBase [26:24) too - temp = pci_read_config32(dev, 0x140 + (nodeid <<3)) & 0xff; //[47:40] at [7:0] - d.base |= temp<<21; - return d; -} - -static void set_io_addr_reg(struct device *dev, u32 nodeid, u32 linkn, u32 reg, - u32 io_min, u32 io_max) -{ - u32 i; - u32 tempreg; - /* io range allocation */ - tempreg = (nodeid&0xf) | ((nodeid & 0x30)<<(8-4)) | (linkn<<4) | ((io_max&0xf0)<<(12-4)); //limit - for (i = 0; i < node_nums; i++) - pci_write_config32(__f1_dev[i], reg+4, tempreg); - tempreg = 3 /*| (3<<4)*/ | ((io_min&0xf0)<<(12-4)); //base :ISA and VGA ? - for (i = 0; i < node_nums; i++) - pci_write_config32(__f1_dev[i], reg, tempreg); -} - -static void set_mmio_addr_reg(u32 nodeid, u32 linkn, u32 reg, u32 index, u32 mmio_min, u32 mmio_max, u32 nodes) -{ - u32 i; - u32 tempreg; - /* io range allocation */ - tempreg = (nodeid&0xf) | (linkn<<4) | (mmio_max&0xffffff00); //limit - for (i = 0; i < nodes; i++) - pci_write_config32(__f1_dev[i], reg+4, tempreg); - tempreg = 3 | (nodeid & 0x30) | (mmio_min&0xffffff00); - for (i = 0; i < node_nums; i++) - pci_write_config32(__f1_dev[i], reg, tempreg); -} - static struct device *get_node_pci(u32 nodeid, u32 fn) { return pcidev_on_root(DEV_CDB + nodeid, fn); @@ -109,25 +59,31 @@ printk(BIOS_DEBUG, "fx_devs = 0x%x\n", fx_devs); }
-static u32 f1_read_config32(unsigned int reg) +static int get_dram_base_limit(u32 nodeid, resource_t *basek, resource_t *limitk) { - if (fx_devs == 0) - get_fx_devs(); - return pci_read_config32(__f1_dev[0], reg); -} + u32 temp;
-static void f1_write_config32(unsigned int reg, u32 value) -{ - int i; if (fx_devs == 0) get_fx_devs(); - for (i = 0; i < fx_devs; i++) { - struct device *dev; - dev = __f1_dev[i]; - if (dev && dev->enabled) { - pci_write_config32(dev, reg, value); - } - } + + + temp = pci_read_config32(__f1_dev[nodeid], 0x40 + (nodeid << 3)); //[39:24] at [31:16] + if (!(temp & 1)) + return 0; // this memory range is not enabled + /* + * BKDG: {DramBase[39:24], 00_0000h} <= address[39:0] so shift left by 8 bits + * for physical address and the convert to KiB by shifting 10 bits left + */ + *basek = ((temp & 0xffff0000)) >> (10 - 8); + /* + * BKDG address[39:0] <= {DramLimit[39:24], FF_FFFFh} converted as above but + * ORed with 0xffff to get real limit before shifting. + */ + temp = pci_read_config32(__f1_dev[nodeid], 0x44 + (nodeid << 3)); //[39:24] at [31:16] + *limitk = ((temp & 0xffff0000) | 0xffff) >> (10 - 8); + *limitk += 1; // round up last byte + + return 1; }
static u32 amdfam16_nodeid(struct device *dev) @@ -135,18 +91,6 @@ return (dev->path.pci.devfn >> 3) - DEV_CDB; }
-static void set_vga_enable_reg(u32 nodeid, u32 linkn) -{ - u32 val; - - val = 1 | (nodeid<<4) | (linkn<<12); - /* it will routing - * (1)mmio 0xa0000:0xbffff - * (2)io 0x3b0:0x3bb, 0x3c0:0x3df - */ - f1_write_config32(0xf4, val); - -}
/** * @return @@ -278,11 +222,36 @@
}
-static void read_resources(struct device *dev) +static void add_fixed_resources(struct device *dev, int index) +{ + /* Reserve everything between A segment and 1MB: + * + * 0xa0000 - 0xbffff: legacy VGA + * 0xc0000 - 0xcffff: VGA OPROM + * 0xe0000 - 0xfffff: SeaBIOS, if used + */ + mmio_resource(dev, index++, 0xa0000 >> 10, (0xc0000 - 0xa0000) >> 10); + reserved_ram_resource(dev, index++, 0xc0000 >> 10, (0x100000 - 0xc0000) >> 10); + + if (fx_devs == 0) + get_fx_devs(); + + /* Check if CC6 save area is enabled (bit 18 CC6SaveEn) */ + if (pci_read_config32(__f2_dev[0], 0x118) & (1 << 18)) { + /* Add CC6 DRAM UC resource residing at DRAM Limit of size 16MB as per BKDG */ + resource_t basek, limitk; + if (!get_dram_base_limit(0, &basek, &limitk)) + return; + mmio_resource(dev, index++, limitk, 16*1024); + } +} + +static void nb_read_resources(struct device *dev) { u32 nodeid; struct bus *link; struct resource *res; + int idx = 0;
nodeid = amdfam16_nodeid(dev); for (link = dev->link_list; link; link = link->next) { @@ -303,108 +272,8 @@ res->base = IO_APIC2_ADDR; res->size = 0x00001000; res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; -}
-static void set_resource(struct device *dev, struct resource *resource, u32 nodeid) -{ - resource_t rbase, rend; - unsigned int reg, link_num; - char buf[50]; - - /* Make certain the resource has actually been set */ - if (!(resource->flags & IORESOURCE_ASSIGNED)) { - return; - } - - /* If I have already stored this resource don't worry about it */ - if (resource->flags & IORESOURCE_STORED) { - return; - } - - /* Only handle PCI memory and IO resources */ - if (!(resource->flags & (IORESOURCE_MEM | IORESOURCE_IO))) - return; - - /* Ensure I am actually looking at a resource of function 1 */ - if ((resource->index & 0xffff) < 0x1000) { - return; - } - /* Get the base address */ - rbase = resource->base; - - /* Get the limit (rounded up) */ - rend = resource_end(resource); - - /* Get the register and link */ - reg = resource->index & 0xfff; // 4k - link_num = IOINDEX_LINK(resource->index); - - if (resource->flags & IORESOURCE_IO) { - set_io_addr_reg(dev, nodeid, link_num, reg, rbase>>8, rend>>8); - } - else if (resource->flags & IORESOURCE_MEM) { - set_mmio_addr_reg(nodeid, link_num, reg, (resource->index >>24), rbase>>8, rend>>8, node_nums); // [39:8] - } - resource->flags |= IORESOURCE_STORED; - snprintf(buf, sizeof(buf), " <node %x link %x>", - nodeid, link_num); - report_resource_stored(dev, resource, 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, unsigned int nodeid) -{ - 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) { -#if CONFIG(MULTIPLE_VGA_ADAPTERS) - extern struct device *vga_pri; // the primary vga device, defined in device.c - printk(BIOS_DEBUG, "VGA: vga_pri bus num = %d bus range [%d,%d]\n", vga_pri->bus->secondary, - link->secondary,link->subordinate); - /* We need to make sure the vga_pri is under the link */ - if ((vga_pri->bus->secondary >= link->secondary) && - (vga_pri->bus->secondary <= link->subordinate)) -#endif - break; - } - } - - /* no VGA card installed */ - if (link == NULL) - return; - - printk(BIOS_DEBUG, "VGA: %s (aka node %d) link %d has VGA device\n", dev_path(dev), nodeid, sblink); - set_vga_enable_reg(nodeid, sblink); -} - -static void set_resources(struct device *dev) -{ - unsigned int nodeid; - struct bus *bus; - struct resource *res; - - /* Find the nodeid */ - nodeid = amdfam16_nodeid(dev); - - create_vga_resource(dev, nodeid); //TODO: do we need this? - - /* Set each resource we have found */ - for (res = dev->resource_list; res; res = res->next) { - set_resource(dev, res, nodeid); - } - - for (bus = dev->link_list; bus; bus = bus->next) { - if (bus->children) { - assign_resources(bus); - } - } + add_fixed_resources(dev, idx++); }
static void northbridge_init(struct device *dev) @@ -846,11 +715,12 @@ }
static struct device_operations northbridge_operations = { - .read_resources = read_resources, - .set_resources = set_resources, - .enable_resources = pci_dev_enable_resources, - .init = northbridge_init, - .acpi_fill_ssdt = northbridge_fill_ssdt_generator, + .read_resources = nb_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_dev_enable_resources, + .init = northbridge_init, + .ops_pci = &pci_dev_ops_pci, + .acpi_fill_ssdt = northbridge_fill_ssdt_generator, .write_acpi_tables = agesa_write_acpi_tables, };
@@ -918,62 +788,23 @@ .final = fam16_finalize, };
-static void domain_read_resources(struct device *dev) -{ - unsigned int reg; - - /* Find the already assigned resource pairs */ - get_fx_devs(); - for (reg = 0x80; reg <= 0xd8; reg+= 0x08) { - u32 base, limit; - base = f1_read_config32(reg); - limit = f1_read_config32(reg + 0x04); - /* Is this register allocated? */ - if ((base & 3) != 0) { - unsigned int nodeid, reg_link; - struct device *reg_dev; - if (reg < 0xc0) { // mmio - nodeid = (limit & 0xf) + (base&0x30); - } else { // io - nodeid = (limit & 0xf) + ((base>>4)&0x30); - } - reg_link = (limit >> 4) & 7; - reg_dev = __f0_dev[nodeid]; - if (reg_dev) { - /* Reserve the resource */ - struct resource *res; - res = new_resource(reg_dev, IOINDEX(0x1000 + reg, reg_link)); - if (res) { - res->flags = 1; - } - } - } - } - /* FIXME: do we need to check extend conf space? - I don't believe that much preset value */ - pci_domain_read_resources(dev); -} - -static void domain_enable_resources(struct device *dev) -{ -} - -#if CONFIG_HW_MEM_HOLE_SIZEK != 0 struct hw_mem_hole_info { unsigned int hole_startk; int node_id; }; + static struct hw_mem_hole_info get_hw_mem_hole_info(void) { struct hw_mem_hole_info mem_hole; int i; + resource_t base_k, limit_k; + mem_hole.hole_startk = CONFIG_HW_MEM_HOLE_SIZEK; mem_hole.node_id = -1; for (i = 0; i < node_nums; i++) { - dram_base_mask_t d; u32 hole; - d = get_dram_base_mask(i); - if (!(d.mask & 1)) continue; // no memory on this node + if (!get_dram_base_limit(i, &base_k, &limit_k)) + continue; // no memory on this node hole = pci_read_config32(__f1_dev[i], 0xf0); if (hole & 2) { // we find the hole mem_hole.hole_startk = (hole & (0xff<<24)) >> 10; @@ -988,90 +819,68 @@ if (mem_hole.node_id == -1) { resource_t limitk_pri = 0; for (i = 0; i < node_nums; i++) { - dram_base_mask_t d; - resource_t base_k, limit_k; - d = get_dram_base_mask(i); - if (!(d.base & 1)) continue; - base_k = ((resource_t)(d.base & 0x1fffff00)) <<9; - if (base_k > 4 *1024 * 1024) break; // don't need to go to check + if (!get_dram_base_limit(i, &base_k, &limit_k)) + continue; // no memory on this node + if (base_k > 4*1024*1024) + break; // don't need to go to check if (limitk_pri != base_k) { // we find the hole mem_hole.hole_startk = (unsigned int)limitk_pri; // must be below 4G mem_hole.node_id = i; break; //only one hole } - limit_k = ((resource_t)(((d.mask & ~1) + 0x000FF) & 0x1fffff00)) << 9; limitk_pri = limit_k; } } return mem_hole; } -#endif
-static void domain_set_resources(struct device *dev) +static void domain_read_resources(struct device *dev) { unsigned long mmio_basek; - u32 pci_tolm; int i, idx; - struct bus *link; -#if CONFIG_HW_MEM_HOLE_SIZEK != 0 struct hw_mem_hole_info mem_hole; -#endif
- pci_tolm = 0xffffffffUL; - for (link = dev->link_list; link; link = link->next) { - pci_tolm = find_pci_tolm(link); + pci_domain_read_resources(dev); + + /* TOP_MEM MSR is our boundary between DRAM and MMIO under 4G */ + mmio_basek = bsp_topmem() >> 10; + + if (fx_devs == 0) + get_fx_devs(); + + if (CONFIG_HW_MEM_HOLE_SIZEK != 0) { + /* if the hw mem hole is already set in raminit stage, here we will compare + * mmio_basek and hole_basek. if mmio_basek is bigger that hole_basek and will + * use hole_basek as mmio_basek and we don't need to reset hole. + * otherwise We reset the hole to the mmio_basek + */ + mem_hole = get_hw_mem_hole_info(); + + // Use hole_basek as mmio_basek, and we don't need to reset hole anymore + if ((mem_hole.node_id != -1) && (mmio_basek > mem_hole.hole_startk)) { + mmio_basek = mem_hole.hole_startk; + } }
- // FIXME handle interleaved nodes. If you fix this here, please fix - // amdk8, too. - mmio_basek = pci_tolm >> 10; - /* Round mmio_basek to something the processor can support */ - mmio_basek &= ~((1 << 6) -1); - - // FIXME improve mtrr.c so we don't use up all of the mtrrs with a 64M - // MMIO hole. If you fix this here, please fix amdk8, too. - /* Round the mmio hole to 64M */ - mmio_basek &= ~((64*1024) - 1); - -#if CONFIG_HW_MEM_HOLE_SIZEK != 0 - /* if the hw mem hole is already set in raminit stage, here we will compare - * mmio_basek and hole_basek. if mmio_basek is bigger that hole_basek and will - * use hole_basek as mmio_basek and we don't need to reset hole. - * otherwise We reset the hole to the mmio_basek - */ - - mem_hole = get_hw_mem_hole_info(); - - // Use hole_basek as mmio_basek, and we don't need to reset hole anymore - if ((mem_hole.node_id != -1) && (mmio_basek > mem_hole.hole_startk)) { - mmio_basek = mem_hole.hole_startk; - } -#endif - idx = 0x10; for (i = 0; i < node_nums; i++) { - dram_base_mask_t d; resource_t basek, limitk, sizek; // 4 1T
- d = get_dram_base_mask(i); - - if (!(d.mask & 1)) continue; - basek = ((resource_t)(d.base & 0x1fffff00)) << 9; // could overflow, we may lost 6 bit here - limitk = ((resource_t)(((d.mask & ~1) + 0x000FF) & 0x1fffff00)) << 9; + if (!get_dram_base_limit(i, &basek, &limitk)) + continue;
sizek = limitk - basek; + printk(BIOS_DEBUG, "node %d: basek=%08llx, limitk=%08llx, sizek=%08llx,\n", + i, basek, limitk, sizek);
- /* see if we need a hole from 0xa0000 to 0xbffff */ - if ((basek < ((8*64)+(8*16))) && (sizek > ((8*64)+(16*16)))) { - ram_resource(dev, (idx | i), basek, ((8*64)+(8*16)) - basek); + /* see if we need a hole from 0xa0000 to 0xfffff */ + if ((basek < (0xa0000 >> 10) && (sizek > (0x100000 >> 10)))) { + ram_resource(dev, (idx | i), basek, (0xa0000 >> 10) - basek); idx += 0x10; - basek = (8*64)+(16*16); - sizek = limitk - ((8*64)+(16*16)); - + basek = 0x100000 >> 10; + sizek = limitk - basek; }
- //printk(BIOS_DEBUG, "node %d : mmio_basek=%08lx, basek=%08llx, limitk=%08llx\n", i, mmio_basek, basek, limitk); - /* split the region to accommodate pci memory space */ if ((basek < 4*1024*1024) && (limitk > mmio_basek)) { if (basek <= mmio_basek) { @@ -1101,12 +910,6 @@ }
add_uma_resource_below_tolm(dev, 7); - - for (link = dev->link_list; link; link = link->next) { - if (link->children) { - assign_resources(link); - } - } }
static const char *domain_acpi_name(const struct device *dev) @@ -1119,8 +922,7 @@
static struct device_operations pci_domain_ops = { .read_resources = domain_read_resources, - .set_resources = domain_set_resources, - .enable_resources = domain_enable_resources, + .set_resources = pci_domain_set_resources, .scan_bus = pci_domain_scan_bus, .acpi_name = domain_acpi_name, };