| Message ID | 20180111050208.18349-1-alistair@popple.id.au |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] Witherspoon: Remove old Witherspoon platform definition | expand |
On 11/01/18 16:02, Alistair Popple wrote: > An old Witherspoon platform definition was added to aid the transition from > versions of Hostboot which didn't have the correct NVLink HDAT information > available and/or planar VPD. These system should now be updated so remove > the possibly incorrect default assumption. > > This may disable NVLink on old out-dated systems but it can easily be > restored with the appropriate FW and/or VPD updates. In any case there is a > a 50% chance the existing default behaviour was incorrect as it only > supports 6 GPU systems. Using an incorrect platform definition leads to > undefined behaviour which is more difficult to detect/debug than not > creating the NVLink devices so remove the possibly incorrect default > behaviour. > > Signed-off-by: Alistair Popple <alistair@popple.id.au> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > --- > > Changes from v1: > > - Remove checks for NPU node and PCIe slot information in > witherspoon_probe() > > platforms/astbmc/witherspoon.c | 356 ----------------------------------------- > 1 file changed, 356 deletions(-) > > diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c > index b9ed2778..9460b6d1 100644 > --- a/platforms/astbmc/witherspoon.c > +++ b/platforms/astbmc/witherspoon.c > @@ -30,317 +30,10 @@ > > #include "astbmc.h" > > -static const struct slot_table_entry witherspoon_slot1[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0,0), > - .name = "SLOT0" > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_slot2_shared[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0,0), > - .name = "SLOT1" > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_slot3[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0,0), > - .name = "SLOT2" > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_slot4[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0,0), > - .name = "SLOT3" > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu0[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0x80,0), > - .name = "GPU0", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu1[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0xa0,0), > - .name = "GPU1", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu2[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0xc0,0), > - .name = "GPU2", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu3[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0x60,0), > - .name = "GPU3", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu4[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0x80,0), > - .name = "GPU4", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu5[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0xa0,0), > - .name = "GPU5", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx0_down[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x4a,0), > - .children = witherspoon_gpu0, > - .name = "GPU0 down", > - }, > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x4b,0), > - .children = witherspoon_gpu1, > - .name = "GPU1 down", > - }, > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x4c,0), > - .children = witherspoon_gpu2, > - .name = "GPU2 down", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx1_down[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x44,0), > - .children = witherspoon_gpu3, > - .name = "GPU3 down", > - }, > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x45,0), > - .children = witherspoon_gpu4, > - .name = "GPU4 down", > - }, > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x4d,0), > - .children = witherspoon_gpu5, > - .name = "GPU5 down", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx0_up[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x20,0), > - .children = witherspoon_plx0_down, > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx1_up[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x20,0), > - .children = witherspoon_plx1_down, > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx0_phb[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0,0), > - .children = witherspoon_plx0_up, > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx1_phb[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0,0), > - .children = witherspoon_plx1_up, > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_npu0_slots[] = { > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(0), > - .name = "GPU0", > - }, > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(1), > - .name = "GPU1", > - }, > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(2), > - .name = "GPU2", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_npu8_slots[] = { > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(0), > - .name = "GPU3", > - }, > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(1), > - .name = "GPU4", > - }, > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(2), > - .name = "GPU5", > - }, > - { .etype = st_end }, > -}; > - > -/* > - * Slot numbering: > - * > - * slot 1 - x4 slot > - * slot 2 - shared slot, 8x to each chip's PHB3 > - * slot 3 - 16x \w CAPI, second chip > - * slot 4 - 16x \w CAPI, first chip > - */ > - > -static const struct slot_table_entry witherspoon_phb_table[] = { > - ST_PHB_ENTRY(0, 0, witherspoon_slot4), > - ST_PHB_ENTRY(0, 3, witherspoon_slot2_shared), > - ST_PHB_ENTRY(0, 4, witherspoon_plx0_phb), > - ST_PHB_ENTRY(0, 7, witherspoon_npu0_slots), > - > - ST_PHB_ENTRY(8, 0, witherspoon_slot3), > - ST_PHB_ENTRY(8, 3, witherspoon_slot2_shared), > - ST_PHB_ENTRY(8, 4, witherspoon_slot1), > - ST_PHB_ENTRY(8, 5, witherspoon_plx1_phb), > - ST_PHB_ENTRY(8, 8, witherspoon_npu8_slots), > - > - { .etype = st_end }, > -}; > - > -#define NPU_BASE 0x5011000 > -#define NPU_SIZE 0x2c > -#define NPU_INDIRECT0 0x8000000009010c3f > -#define NPU_INDIRECT1 0x800000000c010c3f > - > -static void create_link(struct dt_node *npu, int group, int index) > -{ > - struct dt_node *link; > - uint32_t lane_mask; > - uint64_t phy; > - char namebuf[32]; > - > - snprintf(namebuf, sizeof(namebuf), "link@%x", index); > - link = dt_new(npu, namebuf); > - > - dt_add_property_string(link, "compatible", "ibm,npu-link"); > - dt_add_property_cells(link, "ibm,npu-link-index", index); > - > - if (!(index / 3)) > - phy = NPU_INDIRECT0; > - else > - phy = NPU_INDIRECT1; > - > - switch (index % 3) { > - case 0: > - lane_mask = 0xf1e000; > - break; > - > - case 1: > - lane_mask = 0x0e1870; > - break; > - > - case 2: > - lane_mask = 0x00078f; > - break; > - > - default: > - assert(0); > - } > - > - dt_add_property_u64s(link, "ibm,npu-phy", phy); > - dt_add_property_cells(link, "ibm,npu-lane-mask", lane_mask); > - dt_add_property_cells(link, "ibm,npu-group-id", group); > -} > - > -static void dt_create_npu2(void) > -{ > - struct dt_node *xscom, *npu; > - char namebuf[32]; > - int phb_index = 7; > - int npu_index = 0; > - > - dt_for_each_compatible(dt_root, xscom, "ibm,xscom") { > - snprintf(namebuf, sizeof(namebuf), "npu@%x", NPU_BASE); > - npu = dt_new(xscom, namebuf); > - dt_add_property_cells(npu, "reg", NPU_BASE, NPU_SIZE); > - dt_add_property_strings(npu, "compatible", "ibm,power9-npu"); > - > - dt_add_property_cells(npu, "ibm,phb-index", phb_index++); > - dt_add_property_cells(npu, "ibm,npu-index", npu_index++); > - dt_add_property_cells(npu, "ibm,npu-links", 6); > - > - create_link(npu, 0, 0); > - create_link(npu, 0, 1); > - create_link(npu, 1, 2); > - create_link(npu, 1, 3); > - create_link(npu, 2, 4); > - create_link(npu, 2, 5); > - } > -} > - > static bool witherspoon_probe(void) > { > if (!dt_node_is_compatible(dt_root, "ibm,witherspoon")) > return false; > - if (!dt_find_by_name(dt_root, "ibm,pcie-slots")) > - return false; > - if (!dt_find_compatible_node(dt_root, NULL, "ibm,power9-npu")) > - return false; > > /* Lot of common early inits here */ > astbmc_early_init(); > @@ -351,37 +44,6 @@ static bool witherspoon_probe(void) > return true; > } > > -static bool old_witherspoon_probe(void) > -{ > - struct dt_node *slots, *npu; > - > - if (!dt_node_is_compatible(dt_root, "ibm,witherspoon")) > - return false; > - > - slots = dt_find_by_name(dt_root, "ibm,pcie-slots"); > - npu = dt_find_compatible_node(dt_root, NULL, "ibm,power9-npu"); > - > - if (slots && npu) > - return false; > - > - /* Lot of common early inits here */ > - astbmc_early_init(); > - > - /* Setup UART for use by OPAL (Linux hvc) */ > - uart_set_console_policy(UART_CONSOLE_OPAL); > - > - > - /* Add NPU2 bindings */ > - if (!npu) > - dt_create_npu2(); > - > - slot_table_init(witherspoon_phb_table); > - > - prerror("!!! Old witherspoon firmware detected. Update hostboot and fix the Planar VPD !!!\n"); > - > - return true; > -} > - > static void phb4_activate_shared_slot_witherspoon(struct proc_chip *chip) > { > uint64_t val; > @@ -488,21 +150,3 @@ DECLARE_PLATFORM(witherspoon) = { > > .pci_get_slot_info = dt_slot_get_slot_info, > }; > - > -DECLARE_PLATFORM(old_witherspoon) = { > - .name = "Witherspoon (old)", > - .probe = old_witherspoon_probe, > - .init = astbmc_init, > - .pre_pci_fixup = witherspoon_pre_pci_fixup, > - .start_preload_resource = flash_start_preload_resource, > - .resource_loaded = flash_resource_loaded, > - .bmc = &astbmc_openbmc, > - .cec_power_down = astbmc_ipmi_power_down, > - .cec_reboot = astbmc_ipmi_reboot, > - .elog_commit = ipmi_elog_commit, > - .exit = ipmi_wdt_final_reset, > - .terminate = ipmi_terminate, > - > - .pci_get_slot_info = slot_table_get_slot_info, > - .pci_probe_complete = check_all_slot_table, > -}; >
There's a chance this will break systems with out of date firmware/VPD information, but we don't support those anyway. Anyone affected should know how to fix it themselves. Reviewed-by: Oliver O'Halloran <oohall@gmail.com> On Thu, Jan 11, 2018 at 4:02 PM, Alistair Popple <alistair@popple.id.au> wrote: > An old Witherspoon platform definition was added to aid the transition from > versions of Hostboot which didn't have the correct NVLink HDAT information > available and/or planar VPD. These system should now be updated so remove > the possibly incorrect default assumption. > > This may disable NVLink on old out-dated systems but it can easily be > restored with the appropriate FW and/or VPD updates. In any case there is a > a 50% chance the existing default behaviour was incorrect as it only > supports 6 GPU systems. Using an incorrect platform definition leads to > undefined behaviour which is more difficult to detect/debug than not > creating the NVLink devices so remove the possibly incorrect default > behaviour. > > Signed-off-by: Alistair Popple <alistair@popple.id.au> > --- > > Changes from v1: > > - Remove checks for NPU node and PCIe slot information in > witherspoon_probe() > > platforms/astbmc/witherspoon.c | 356 ----------------------------------------- > 1 file changed, 356 deletions(-) > > diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c > index b9ed2778..9460b6d1 100644 > --- a/platforms/astbmc/witherspoon.c > +++ b/platforms/astbmc/witherspoon.c > @@ -30,317 +30,10 @@ > > #include "astbmc.h" > > -static const struct slot_table_entry witherspoon_slot1[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0,0), > - .name = "SLOT0" > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_slot2_shared[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0,0), > - .name = "SLOT1" > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_slot3[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0,0), > - .name = "SLOT2" > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_slot4[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0,0), > - .name = "SLOT3" > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu0[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0x80,0), > - .name = "GPU0", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu1[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0xa0,0), > - .name = "GPU1", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu2[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0xc0,0), > - .name = "GPU2", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu3[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0x60,0), > - .name = "GPU3", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu4[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0x80,0), > - .name = "GPU4", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_gpu5[] = { > - { > - .etype = st_pluggable_slot, > - .location = ST_LOC_DEVFN(0xa0,0), > - .name = "GPU5", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx0_down[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x4a,0), > - .children = witherspoon_gpu0, > - .name = "GPU0 down", > - }, > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x4b,0), > - .children = witherspoon_gpu1, > - .name = "GPU1 down", > - }, > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x4c,0), > - .children = witherspoon_gpu2, > - .name = "GPU2 down", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx1_down[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x44,0), > - .children = witherspoon_gpu3, > - .name = "GPU3 down", > - }, > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x45,0), > - .children = witherspoon_gpu4, > - .name = "GPU4 down", > - }, > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x4d,0), > - .children = witherspoon_gpu5, > - .name = "GPU5 down", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx0_up[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x20,0), > - .children = witherspoon_plx0_down, > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx1_up[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0x20,0), > - .children = witherspoon_plx1_down, > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx0_phb[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0,0), > - .children = witherspoon_plx0_up, > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_plx1_phb[] = { > - { > - .etype = st_builtin_dev, > - .location = ST_LOC_DEVFN(0,0), > - .children = witherspoon_plx1_up, > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_npu0_slots[] = { > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(0), > - .name = "GPU0", > - }, > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(1), > - .name = "GPU1", > - }, > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(2), > - .name = "GPU2", > - }, > - { .etype = st_end }, > -}; > - > -static const struct slot_table_entry witherspoon_npu8_slots[] = { > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(0), > - .name = "GPU3", > - }, > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(1), > - .name = "GPU4", > - }, > - { > - .etype = st_npu_slot, > - .location = ST_LOC_NPU_GROUP(2), > - .name = "GPU5", > - }, > - { .etype = st_end }, > -}; > - > -/* > - * Slot numbering: > - * > - * slot 1 - x4 slot > - * slot 2 - shared slot, 8x to each chip's PHB3 > - * slot 3 - 16x \w CAPI, second chip > - * slot 4 - 16x \w CAPI, first chip > - */ > - > -static const struct slot_table_entry witherspoon_phb_table[] = { > - ST_PHB_ENTRY(0, 0, witherspoon_slot4), > - ST_PHB_ENTRY(0, 3, witherspoon_slot2_shared), > - ST_PHB_ENTRY(0, 4, witherspoon_plx0_phb), > - ST_PHB_ENTRY(0, 7, witherspoon_npu0_slots), > - > - ST_PHB_ENTRY(8, 0, witherspoon_slot3), > - ST_PHB_ENTRY(8, 3, witherspoon_slot2_shared), > - ST_PHB_ENTRY(8, 4, witherspoon_slot1), > - ST_PHB_ENTRY(8, 5, witherspoon_plx1_phb), > - ST_PHB_ENTRY(8, 8, witherspoon_npu8_slots), > - > - { .etype = st_end }, > -}; > - > -#define NPU_BASE 0x5011000 > -#define NPU_SIZE 0x2c > -#define NPU_INDIRECT0 0x8000000009010c3f > -#define NPU_INDIRECT1 0x800000000c010c3f > - > -static void create_link(struct dt_node *npu, int group, int index) > -{ > - struct dt_node *link; > - uint32_t lane_mask; > - uint64_t phy; > - char namebuf[32]; > - > - snprintf(namebuf, sizeof(namebuf), "link@%x", index); > - link = dt_new(npu, namebuf); > - > - dt_add_property_string(link, "compatible", "ibm,npu-link"); > - dt_add_property_cells(link, "ibm,npu-link-index", index); > - > - if (!(index / 3)) > - phy = NPU_INDIRECT0; > - else > - phy = NPU_INDIRECT1; > - > - switch (index % 3) { > - case 0: > - lane_mask = 0xf1e000; > - break; > - > - case 1: > - lane_mask = 0x0e1870; > - break; > - > - case 2: > - lane_mask = 0x00078f; > - break; > - > - default: > - assert(0); > - } > - > - dt_add_property_u64s(link, "ibm,npu-phy", phy); > - dt_add_property_cells(link, "ibm,npu-lane-mask", lane_mask); > - dt_add_property_cells(link, "ibm,npu-group-id", group); > -} > - > -static void dt_create_npu2(void) > -{ > - struct dt_node *xscom, *npu; > - char namebuf[32]; > - int phb_index = 7; > - int npu_index = 0; > - > - dt_for_each_compatible(dt_root, xscom, "ibm,xscom") { > - snprintf(namebuf, sizeof(namebuf), "npu@%x", NPU_BASE); > - npu = dt_new(xscom, namebuf); > - dt_add_property_cells(npu, "reg", NPU_BASE, NPU_SIZE); > - dt_add_property_strings(npu, "compatible", "ibm,power9-npu"); > - > - dt_add_property_cells(npu, "ibm,phb-index", phb_index++); > - dt_add_property_cells(npu, "ibm,npu-index", npu_index++); > - dt_add_property_cells(npu, "ibm,npu-links", 6); > - > - create_link(npu, 0, 0); > - create_link(npu, 0, 1); > - create_link(npu, 1, 2); > - create_link(npu, 1, 3); > - create_link(npu, 2, 4); > - create_link(npu, 2, 5); > - } > -} > - > static bool witherspoon_probe(void) > { > if (!dt_node_is_compatible(dt_root, "ibm,witherspoon")) > return false; > - if (!dt_find_by_name(dt_root, "ibm,pcie-slots")) > - return false; > - if (!dt_find_compatible_node(dt_root, NULL, "ibm,power9-npu")) > - return false; > > /* Lot of common early inits here */ > astbmc_early_init(); > @@ -351,37 +44,6 @@ static bool witherspoon_probe(void) > return true; > } > > -static bool old_witherspoon_probe(void) > -{ > - struct dt_node *slots, *npu; > - > - if (!dt_node_is_compatible(dt_root, "ibm,witherspoon")) > - return false; > - > - slots = dt_find_by_name(dt_root, "ibm,pcie-slots"); > - npu = dt_find_compatible_node(dt_root, NULL, "ibm,power9-npu"); > - > - if (slots && npu) > - return false; > - > - /* Lot of common early inits here */ > - astbmc_early_init(); > - > - /* Setup UART for use by OPAL (Linux hvc) */ > - uart_set_console_policy(UART_CONSOLE_OPAL); > - > - > - /* Add NPU2 bindings */ > - if (!npu) > - dt_create_npu2(); > - > - slot_table_init(witherspoon_phb_table); > - > - prerror("!!! Old witherspoon firmware detected. Update hostboot and fix the Planar VPD !!!\n"); > - > - return true; > -} > - > static void phb4_activate_shared_slot_witherspoon(struct proc_chip *chip) > { > uint64_t val; > @@ -488,21 +150,3 @@ DECLARE_PLATFORM(witherspoon) = { > > .pci_get_slot_info = dt_slot_get_slot_info, > }; > - > -DECLARE_PLATFORM(old_witherspoon) = { > - .name = "Witherspoon (old)", > - .probe = old_witherspoon_probe, > - .init = astbmc_init, > - .pre_pci_fixup = witherspoon_pre_pci_fixup, > - .start_preload_resource = flash_start_preload_resource, > - .resource_loaded = flash_resource_loaded, > - .bmc = &astbmc_openbmc, > - .cec_power_down = astbmc_ipmi_power_down, > - .cec_reboot = astbmc_ipmi_reboot, > - .elog_commit = ipmi_elog_commit, > - .exit = ipmi_wdt_final_reset, > - .terminate = ipmi_terminate, > - > - .pci_get_slot_info = slot_table_get_slot_info, > - .pci_probe_complete = check_all_slot_table, > -}; > -- > 2.11.0 >
Alistair Popple <alistair@popple.id.au> writes: > An old Witherspoon platform definition was added to aid the transition from > versions of Hostboot which didn't have the correct NVLink HDAT information > available and/or planar VPD. These system should now be updated so remove > the possibly incorrect default assumption. > > This may disable NVLink on old out-dated systems but it can easily be > restored with the appropriate FW and/or VPD updates. In any case there is a > a 50% chance the existing default behaviour was incorrect as it only > supports 6 GPU systems. Using an incorrect platform definition leads to > undefined behaviour which is more difficult to detect/debug than not > creating the NVLink devices so remove the possibly incorrect default > behaviour. > > Signed-off-by: Alistair Popple <alistair@popple.id.au> > --- > > Changes from v1: > > - Remove checks for NPU node and PCIe slot information in > witherspoon_probe() Merged to master as of 6f8c49b0bac4975aceef96e2f11c70bdf75c284e
On Thu, Jan 11, 2018 at 04:02:08PM +1100, Alistair Popple wrote: >An old Witherspoon platform definition was added to aid the transition from >versions of Hostboot which didn't have the correct NVLink HDAT information >available and/or planar VPD. These system should now be updated so remove >the possibly incorrect default assumption. Reviewed-by: Reza Arbab <arbab@linux.vnet.ibm.com>
diff --git a/platforms/astbmc/witherspoon.c b/platforms/astbmc/witherspoon.c index b9ed2778..9460b6d1 100644 --- a/platforms/astbmc/witherspoon.c +++ b/platforms/astbmc/witherspoon.c @@ -30,317 +30,10 @@ #include "astbmc.h" -static const struct slot_table_entry witherspoon_slot1[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0,0), - .name = "SLOT0" - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_slot2_shared[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0,0), - .name = "SLOT1" - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_slot3[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0,0), - .name = "SLOT2" - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_slot4[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0,0), - .name = "SLOT3" - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_gpu0[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0x80,0), - .name = "GPU0", - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_gpu1[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0xa0,0), - .name = "GPU1", - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_gpu2[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0xc0,0), - .name = "GPU2", - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_gpu3[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0x60,0), - .name = "GPU3", - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_gpu4[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0x80,0), - .name = "GPU4", - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_gpu5[] = { - { - .etype = st_pluggable_slot, - .location = ST_LOC_DEVFN(0xa0,0), - .name = "GPU5", - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_plx0_down[] = { - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0x4a,0), - .children = witherspoon_gpu0, - .name = "GPU0 down", - }, - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0x4b,0), - .children = witherspoon_gpu1, - .name = "GPU1 down", - }, - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0x4c,0), - .children = witherspoon_gpu2, - .name = "GPU2 down", - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_plx1_down[] = { - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0x44,0), - .children = witherspoon_gpu3, - .name = "GPU3 down", - }, - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0x45,0), - .children = witherspoon_gpu4, - .name = "GPU4 down", - }, - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0x4d,0), - .children = witherspoon_gpu5, - .name = "GPU5 down", - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_plx0_up[] = { - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0x20,0), - .children = witherspoon_plx0_down, - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_plx1_up[] = { - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0x20,0), - .children = witherspoon_plx1_down, - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_plx0_phb[] = { - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0,0), - .children = witherspoon_plx0_up, - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_plx1_phb[] = { - { - .etype = st_builtin_dev, - .location = ST_LOC_DEVFN(0,0), - .children = witherspoon_plx1_up, - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_npu0_slots[] = { - { - .etype = st_npu_slot, - .location = ST_LOC_NPU_GROUP(0), - .name = "GPU0", - }, - { - .etype = st_npu_slot, - .location = ST_LOC_NPU_GROUP(1), - .name = "GPU1", - }, - { - .etype = st_npu_slot, - .location = ST_LOC_NPU_GROUP(2), - .name = "GPU2", - }, - { .etype = st_end }, -}; - -static const struct slot_table_entry witherspoon_npu8_slots[] = { - { - .etype = st_npu_slot, - .location = ST_LOC_NPU_GROUP(0), - .name = "GPU3", - }, - { - .etype = st_npu_slot, - .location = ST_LOC_NPU_GROUP(1), - .name = "GPU4", - }, - { - .etype = st_npu_slot, - .location = ST_LOC_NPU_GROUP(2), - .name = "GPU5", - }, - { .etype = st_end }, -}; - -/* - * Slot numbering: - * - * slot 1 - x4 slot - * slot 2 - shared slot, 8x to each chip's PHB3 - * slot 3 - 16x \w CAPI, second chip - * slot 4 - 16x \w CAPI, first chip - */ - -static const struct slot_table_entry witherspoon_phb_table[] = { - ST_PHB_ENTRY(0, 0, witherspoon_slot4), - ST_PHB_ENTRY(0, 3, witherspoon_slot2_shared), - ST_PHB_ENTRY(0, 4, witherspoon_plx0_phb), - ST_PHB_ENTRY(0, 7, witherspoon_npu0_slots), - - ST_PHB_ENTRY(8, 0, witherspoon_slot3), - ST_PHB_ENTRY(8, 3, witherspoon_slot2_shared), - ST_PHB_ENTRY(8, 4, witherspoon_slot1), - ST_PHB_ENTRY(8, 5, witherspoon_plx1_phb), - ST_PHB_ENTRY(8, 8, witherspoon_npu8_slots), - - { .etype = st_end }, -}; - -#define NPU_BASE 0x5011000 -#define NPU_SIZE 0x2c -#define NPU_INDIRECT0 0x8000000009010c3f -#define NPU_INDIRECT1 0x800000000c010c3f - -static void create_link(struct dt_node *npu, int group, int index) -{ - struct dt_node *link; - uint32_t lane_mask; - uint64_t phy; - char namebuf[32]; - - snprintf(namebuf, sizeof(namebuf), "link@%x", index); - link = dt_new(npu, namebuf); - - dt_add_property_string(link, "compatible", "ibm,npu-link"); - dt_add_property_cells(link, "ibm,npu-link-index", index); - - if (!(index / 3)) - phy = NPU_INDIRECT0; - else - phy = NPU_INDIRECT1; - - switch (index % 3) { - case 0: - lane_mask = 0xf1e000; - break; - - case 1: - lane_mask = 0x0e1870; - break; - - case 2: - lane_mask = 0x00078f; - break; - - default: - assert(0); - } - - dt_add_property_u64s(link, "ibm,npu-phy", phy); - dt_add_property_cells(link, "ibm,npu-lane-mask", lane_mask); - dt_add_property_cells(link, "ibm,npu-group-id", group); -} - -static void dt_create_npu2(void) -{ - struct dt_node *xscom, *npu; - char namebuf[32]; - int phb_index = 7; - int npu_index = 0; - - dt_for_each_compatible(dt_root, xscom, "ibm,xscom") { - snprintf(namebuf, sizeof(namebuf), "npu@%x", NPU_BASE); - npu = dt_new(xscom, namebuf); - dt_add_property_cells(npu, "reg", NPU_BASE, NPU_SIZE); - dt_add_property_strings(npu, "compatible", "ibm,power9-npu"); - - dt_add_property_cells(npu, "ibm,phb-index", phb_index++); - dt_add_property_cells(npu, "ibm,npu-index", npu_index++); - dt_add_property_cells(npu, "ibm,npu-links", 6); - - create_link(npu, 0, 0); - create_link(npu, 0, 1); - create_link(npu, 1, 2); - create_link(npu, 1, 3); - create_link(npu, 2, 4); - create_link(npu, 2, 5); - } -} - static bool witherspoon_probe(void) { if (!dt_node_is_compatible(dt_root, "ibm,witherspoon")) return false; - if (!dt_find_by_name(dt_root, "ibm,pcie-slots")) - return false; - if (!dt_find_compatible_node(dt_root, NULL, "ibm,power9-npu")) - return false; /* Lot of common early inits here */ astbmc_early_init(); @@ -351,37 +44,6 @@ static bool witherspoon_probe(void) return true; } -static bool old_witherspoon_probe(void) -{ - struct dt_node *slots, *npu; - - if (!dt_node_is_compatible(dt_root, "ibm,witherspoon")) - return false; - - slots = dt_find_by_name(dt_root, "ibm,pcie-slots"); - npu = dt_find_compatible_node(dt_root, NULL, "ibm,power9-npu"); - - if (slots && npu) - return false; - - /* Lot of common early inits here */ - astbmc_early_init(); - - /* Setup UART for use by OPAL (Linux hvc) */ - uart_set_console_policy(UART_CONSOLE_OPAL); - - - /* Add NPU2 bindings */ - if (!npu) - dt_create_npu2(); - - slot_table_init(witherspoon_phb_table); - - prerror("!!! Old witherspoon firmware detected. Update hostboot and fix the Planar VPD !!!\n"); - - return true; -} - static void phb4_activate_shared_slot_witherspoon(struct proc_chip *chip) { uint64_t val; @@ -488,21 +150,3 @@ DECLARE_PLATFORM(witherspoon) = { .pci_get_slot_info = dt_slot_get_slot_info, }; - -DECLARE_PLATFORM(old_witherspoon) = { - .name = "Witherspoon (old)", - .probe = old_witherspoon_probe, - .init = astbmc_init, - .pre_pci_fixup = witherspoon_pre_pci_fixup, - .start_preload_resource = flash_start_preload_resource, - .resource_loaded = flash_resource_loaded, - .bmc = &astbmc_openbmc, - .cec_power_down = astbmc_ipmi_power_down, - .cec_reboot = astbmc_ipmi_reboot, - .elog_commit = ipmi_elog_commit, - .exit = ipmi_wdt_final_reset, - .terminate = ipmi_terminate, - - .pci_get_slot_info = slot_table_get_slot_info, - .pci_probe_complete = check_all_slot_table, -};
An old Witherspoon platform definition was added to aid the transition from versions of Hostboot which didn't have the correct NVLink HDAT information available and/or planar VPD. These system should now be updated so remove the possibly incorrect default assumption. This may disable NVLink on old out-dated systems but it can easily be restored with the appropriate FW and/or VPD updates. In any case there is a a 50% chance the existing default behaviour was incorrect as it only supports 6 GPU systems. Using an incorrect platform definition leads to undefined behaviour which is more difficult to detect/debug than not creating the NVLink devices so remove the possibly incorrect default behaviour. Signed-off-by: Alistair Popple <alistair@popple.id.au> --- Changes from v1: - Remove checks for NPU node and PCIe slot information in witherspoon_probe() platforms/astbmc/witherspoon.c | 356 ----------------------------------------- 1 file changed, 356 deletions(-)