Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52780 )
Change subject: northbridge/amd/agesa/family14: enable RESOURCE_ALLOCATOR_V4 ......................................................................
northbridge/amd/agesa/family14: 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 apu1 2GB
Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com Change-Id: Ia01c5e238d53f719bb0d15e106517b99432a0c31 --- M src/northbridge/amd/agesa/family14/Kconfig M src/northbridge/amd/agesa/family14/northbridge.c 2 files changed, 127 insertions(+), 285 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/52780/1
diff --git a/src/northbridge/amd/agesa/family14/Kconfig b/src/northbridge/amd/agesa/family14/Kconfig index 9c7eb92..ab84475 100644 --- a/src/northbridge/amd/agesa/family14/Kconfig +++ b/src/northbridge/amd/agesa/family14/Kconfig @@ -2,7 +2,6 @@
config NORTHBRIDGE_AMD_AGESA_FAMILY14 bool - select RESOURCE_ALLOCATOR_V3
if NORTHBRIDGE_AMD_AGESA_FAMILY14
diff --git a/src/northbridge/amd/agesa/family14/northbridge.c b/src/northbridge/amd/agesa/family14/northbridge.c index 020c9c6..6222140 100644 --- a/src/northbridge/amd/agesa/family14/northbridge.c +++ b/src/northbridge/amd/agesa/family14/northbridge.c @@ -26,65 +26,6 @@ static struct device *__f4_dev[FX_DEVS]; static unsigned int fx_devs = 0;
-struct dram_base_mask_t { - u32 base; //[47:27] at [28:8] - u32 mask; //[47:27] at [28:8] and enable at bit 0 -}; - -static struct dram_base_mask_t get_dram_base_mask(u32 nodeid) -{ - struct device *dev; - struct dram_base_mask_t d; - dev = __f1_dev[0]; - - u32 temp; - temp = pci_read_config32(dev, 0x44); //[39:24] at [31:16] - d.mask = (temp & 0xffff0000); // mask out DramMask [26:24] too - - temp = pci_read_config32(dev, 0x40); //[35:24] at [27:16] - d.mask |= (temp & 1); // read enable bit - - d.base = (temp & 0x0fff0000); // mask out DramBase [26:24) too - - return d; -} - -static u32 get_io_addr_index(u32 nodeid, u32 linkn) -{ - return 0; -} - -static u32 get_mmio_addr_index(u32 nodeid, u32 linkn) -{ - return 0; -} - -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 */ - tempreg = (nodeid & 0xf) | ((nodeid & 0x30) << (8 - 4)) | (linkn << 4) | - ((io_max & 0xf0) << (12 - 4)); //limit - pci_write_config32(__f1_dev[0], reg+4, tempreg); - - tempreg = 3 | ((io_min & 0xf0) << (12 - 4)); //base :ISA and VGA ? - pci_write_config32(__f1_dev[0], 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 tempreg; - /* io range allocation */ - tempreg = (nodeid & 0xf) | (linkn << 4) | (mmio_max & 0xffffff00); - pci_write_config32(__f1_dev[0], reg + 4, tempreg); - tempreg = 3 | (nodeid & 0x30) | (mmio_min & 0xffffff00); - pci_write_config32(__f1_dev[0], reg, tempreg); -} - static struct device *get_node_pci(u32 nodeid, u32 fn) { return pcidev_on_root(DEV_CDB + nodeid, fn); @@ -106,13 +47,6 @@ } }
-static u32 f1_read_config32(unsigned int reg) -{ - if (fx_devs == 0) - get_fx_devs(); - return pci_read_config32(__f1_dev[0], reg); -} - static void f1_write_config32(unsigned int reg, u32 value) { int i; @@ -127,6 +61,33 @@ } }
+static int get_dram_base_limit(u32 nodeid, resource_t *basek, resource_t *limitk) +{ + u32 temp; + + if (fx_devs == 0) + get_fx_devs(); + + + temp = pci_read_config32(__f1_dev[nodeid], 0x40 + (nodeid << 3)); //[35:24] at [27:16] + if (!(temp & 1)) + return 0; // this memory range is not enabled + /* + * BKDG: {DramBase[35:24], 00_0000h} <= address[35:0] so shift left by 8 bits + * for physical address and the convert to KiB by shifting 10 bits left + */ + *basek = ((temp & 0x0fff0000)) >> (10 - 8); + /* + * BKDG: address[35:0] <= {DramLimit[35: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)); //[35:24] at [27:16] + *limitk = ((temp & 0x0fff0000) | 0xffff) >> (10 - 8); + *limitk += 1; // round up last byte + + return 1; +} + static u32 amdfam14_nodeid(struct device *dev) { return (dev->path.pci.devfn >> 3) - DEV_CDB; @@ -148,6 +109,12 @@
}
+/** + * @return + * @retval 2 resource does not exist, usable + * @retval 0 resource exists, not usable + * @retval 1 resource exist, resource has been allocated before + */ static int reg_useable(unsigned int reg, struct device *goal_dev, unsigned int goal_nodeid, unsigned int goal_link) { @@ -193,8 +160,8 @@ * but we need one index to differ them. So,same node and same * link can have multi range */ - u32 index = get_io_addr_index(nodeid, link); - reg = 0x110 + (index << 24) + (4 << 20); // index could be 0, 255 + u32 index = 0; + reg = 0x110 + (index << 24) + (4 << 20); // index could be 0, 255 }
resource = new_resource(dev, IOINDEX(0x1000 + reg, link)); @@ -229,7 +196,7 @@ * but we need one index to differ them. So,same node and same * link can have multi range */ - u32 index = get_mmio_addr_index(nodeid, link); + u32 index = 0; reg = 0x110 + (index << 24) + (6 << 20); // index could be 0, 63
} @@ -278,21 +245,6 @@ } }
-static u32 my_find_pci_tolm(struct bus *bus, u32 tolm) -{ - struct resource *min; - unsigned long mask_match = IORESOURCE_MEM | IORESOURCE_ASSIGNED; - min = 0; - search_bus_resources(bus, mask_match, mask_match, tolm_test, - &min); - if (min && tolm > min->base) { - tolm = min->base; - } - return tolm; -} - -#if CONFIG_HW_MEM_HOLE_SIZEK != 0 - struct hw_mem_hole_info { unsigned int hole_startk; int node_id; @@ -301,14 +253,16 @@ static struct hw_mem_hole_info get_hw_mem_hole_info(void) { struct hw_mem_hole_info mem_hole; + resource_t base_k, limit_k;
mem_hole.hole_startk = CONFIG_HW_MEM_HOLE_SIZEK; mem_hole.node_id = -1;
- struct dram_base_mask_t d; + if (fx_devs == 0) + get_fx_devs(); + u32 hole; - d = get_dram_base_mask(0); - if (d.mask & 1) { + if (get_dram_base_limit(0, &base_k, &limit_k)) { hole = pci_read_config32(__f1_dev[0], 0xf0); if (hole & 1) { // we find the hole mem_hole.hole_startk = (hole & (0xff << 24)) >> 10; @@ -317,7 +271,36 @@ } return mem_hole; } -#endif + +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 is enabled (bits [11:0] C6Base). Ff CC6 is not enabled, the base + * must be zero according to BKDG. + */ + if (pci_read_config32(__f4_dev[0], 0x12C) & 0xfff) { + resource_t c6basek; + c6basek = pci_read_config32(__f4_dev[0], 0x12C) & 0xfff; // [35:24] at [11:0] + /* + * Shift left by 24 bits for physical address and the convert to KiB by + * shifting 10 bits left. The C6Base is shifted 14 bits left thus no overflow. + */ + c6basek = c6basek << (24 - 10); + mmio_resource(dev, index++, c6basek, 16*1024); + } +}
static void nb_read_resources(struct device *dev) { @@ -339,60 +322,11 @@ * the CPU_CLUSTER. */ mmconf_resource(dev, MMIO_CONF_BASE); + + add_fixed_resources(dev, 0); }
-static void set_resource(struct device *dev, struct resource *resource, - u32 nodeid) -{ - resource_t rbase, rend; - unsigned int reg, link_num; - char buf[50]; - - printk(BIOS_DEBUG, "\nFam14h - %s\n", __func__); - - /* 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, 1); // [39:8] - } - resource->flags |= IORESOURCE_STORED; - snprintf(buf, sizeof(buf), " <node %x link %x>", nodeid, link_num); - report_resource_stored(dev, resource, buf); -} - -#if CONFIG(CONSOLE_VGA_MULTI) extern struct device *vga_pri; // the primary vga device, defined in device.c -#endif
static void create_vga_resource(struct device *dev, unsigned int nodeid) { @@ -404,7 +338,8 @@ * 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(CONSOLE_VGA_MULTI) + if (!CONFIG(CONSOLE_VGA_MULTI)) + break; printk(BIOS_DEBUG, "VGA: vga_pri bus num = %d bus range [%d,%d]\n", vga_pri->bus->secondary, link->secondary, @@ -412,7 +347,6 @@ /* 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; } } @@ -429,8 +363,6 @@ static void nb_set_resources(struct device *dev) { unsigned int nodeid; - struct bus *bus; - struct resource *res;
printk(BIOS_DEBUG, "\nFam14h - %s\n", __func__);
@@ -439,181 +371,92 @@
create_vga_resource(dev, nodeid);
- /* 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); - } - } + pci_dev_set_resources(dev); }
/* Domain/Root Complex related code */
static void domain_read_resources(struct device *dev) { - unsigned int reg; - printk(BIOS_DEBUG, "\nFam14h - %s\n", __func__);
- /* Find the already assigned resource pairs */ - get_fx_devs(); - for (reg = 0x80; reg <= 0xc0; 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_set_resources(struct device *dev) -{ - printk(BIOS_DEBUG, "\nFam14h - %s\n", __func__); - printk(BIOS_DEBUG, " amsr - incoming dev = %p\n", dev); - unsigned long mmio_basek; - u32 pci_tolm; int idx; - struct bus *link; -#if CONFIG_HW_MEM_HOLE_SIZEK != 0 struct hw_mem_hole_info mem_hole; + resource_t basek, limitk, sizek; u32 reset_memhole = 1; -#endif
- pci_tolm = 0xffffffffUL; - for (link = dev->link_list; link; link = link->next) { - pci_tolm = my_find_pci_tolm(link, pci_tolm); + 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; + reset_memhole = 0; + } }
- // 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; - reset_memhole = 0; - } -#endif - idx = 0x10;
- struct dram_base_mask_t d; - resource_t basek, limitk, sizek; // 4 1T + if (get_dram_base_limit(0, &basek, &limitk)) { + sizek = limitk - basek;
- d = get_dram_base_mask(0); + printk(BIOS_DEBUG, "(before): basek=%08llx, limitk=%08llx, sizek=%08llx,\n", + basek, limitk, sizek);
- if (d.mask & 1) { - basek = ((resource_t) ((u64) d.base)) << 8; - limitk = (resource_t) (((u64) d.mask << 8) | 0xFFFFFF); - printk(BIOS_DEBUG, - "adsr: (before) basek = %llx, limitk = %llx.\n", basek, - limitk); - - /* Convert these values to multiples of 1K for ease of math. */ - basek >>= 10; - limitk >>= 10; - sizek = limitk - basek + 1; - - printk(BIOS_DEBUG, - "adsr: (after) basek = %llx, limitk = %llx, sizek = %llx.\n", - basek, limitk, sizek); - - /* see if we need a hole from 0xa0000 to 0xbffff */ - if ((basek < 640) && (sizek > 768)) { - printk(BIOS_DEBUG,"adsr - 0xa0000 to 0xbffff resource.\n"); - ram_resource(dev, (idx | 0), basek, 640 - basek); + /* see if we need a hole from 0xa0000 to 0xfffff */ + if ((basek < (0xa0000 >> 10) && (sizek > (0x100000 >> 10)))) { + ram_resource(dev, idx, basek, (0xa0000 >> 10) - basek); idx += 0x10; - basek = 768; - sizek = limitk - 768; + basek = (1*MiB) >> 10; + sizek = limitk - basek; }
- printk(BIOS_DEBUG, - "adsr: mmio_basek=%08lx, basek=%08llx, limitk=%08llx\n", - mmio_basek, basek, limitk); - /* split the region to accommodate pci memory space */ - if ((basek < 4 * 1024 * 1024) && (limitk > mmio_basek)) { + if ((basek < 4*1024*1024) && (limitk > mmio_basek)) { if (basek <= mmio_basek) { unsigned int pre_sizek; pre_sizek = mmio_basek - basek; if (pre_sizek > 0) { - ram_resource(dev, idx, basek, - pre_sizek); + ram_resource(dev, idx, basek, pre_sizek); idx += 0x10; sizek -= pre_sizek; } basek = mmio_basek; } - if ((basek + sizek) <= 4 * 1024 * 1024) { + if ((basek + sizek) <= 4*1024*1024) { sizek = 0; } else { - basek = 4 * 1024 * 1024; - sizek -= (4 * 1024 * 1024 - mmio_basek); + uint64_t topmem2 = bsp_topmem2(); + basek = 4*1024*1024; + sizek = topmem2/1024 - basek; } }
- ram_resource(dev, (idx | 0), basek, sizek); + ram_resource(dev, idx, basek, sizek); idx += 0x10; - printk(BIOS_DEBUG, - "%d: mmio_basek=%08lx, basek=%08llx, limitk=%08llx\n", 0, - mmio_basek, basek, limitk); + printk(BIOS_DEBUG, "(after): basek=%08llx, limitk=%08llx, sizek=%08llx,\n", + basek, limitk, sizek); + } else { + die("No memory in the system!"); } - printk(BIOS_DEBUG, " adsr - mmio_basek = %lx.\n", mmio_basek); + printk(BIOS_DEBUG, " mmio_basek = %lx.\n", mmio_basek);
add_uma_resource_below_tolm(dev, 7); - - for (link = dev->link_list; link; link = link->next) { - if (link->children) { - assign_resources(link); - } - } - printk(BIOS_DEBUG, " adsr - leaving this lovely routine.\n"); }
static const char *domain_acpi_name(const struct device *dev) @@ -790,13 +633,13 @@ }
static struct device_operations northbridge_operations = { - .read_resources = nb_read_resources, - .set_resources = nb_set_resources, - .enable_resources = pci_dev_enable_resources, - .acpi_fill_ssdt = northbridge_fill_ssdt_generator, + .read_resources = nb_read_resources, + .set_resources = nb_set_resources, + .enable_resources = pci_dev_enable_resources, + .ops_pci = &pci_dev_ops_pci, + .init = northbridge_init, + .acpi_fill_ssdt = northbridge_fill_ssdt_generator, .write_acpi_tables = agesa_write_acpi_tables, - .init = northbridge_init, - .enable = 0,.ops_pci = 0, };
static const struct pci_driver northbridge_driver __pci_driver = { @@ -814,7 +657,7 @@
static struct device_operations pci_domain_ops = { .read_resources = domain_read_resources, - .set_resources = domain_set_resources, + .set_resources = pci_domain_set_resources, .scan_bus = pci_domain_scan_bus, .acpi_name = domain_acpi_name, };