Re: [PATCH v2] Fix AMD Northbridge-ID contiguity assumptions

From: Borislav Petkov
Date: Fri Oct 12 2012 - 05:33:04 EST


On Thu, Oct 04, 2012 at 03:18:02PM +0200, Borislav Petkov wrote:
> On Wed, Oct 03, 2012 at 09:21:14PM +0800, Daniel J Blueman wrote:
> > The AMD Northbridge initialisation code and EDAC assume the Northbridge IDs
> > are contiguous, which no longer holds on federated systems with multiple
> > HyperTransport fabrics and multiple PCI domains.
> >
> > Address this assumption by searching the Northbridge ID array, rather than
> > directly indexing it, using the upper bits for the PCI domain.
> >
> > v2: Fix Northbridge entry initialisation
> >
> > Tested on a single-socket system and 3-server federated system.
> >
> > Signed-off-by: Daniel J Blueman <daniel@xxxxxxxxxxxxxxxxxx>
> > ---
> > arch/x86/include/asm/amd_nb.h | 23 +++++++++++++++++++++--
> > arch/x86/kernel/amd_nb.c | 16 +++++++++-------
> > drivers/edac/amd64_edac.c | 18 +++++++++---------
> > drivers/edac/amd64_edac.h | 6 ------

Ok,

I've been meaning to clean up that amd_nb.c code which iterates over all
PCI devices on the system just so it can count the NBs and then do it
again in order to do the ->misc and ->link assignment.

So below is what I've come up with and it builds but it is completely
untested and I might be completely off, for all I know.

The basic idea, though, is to have the first 8 NB descriptors in an
array of 8 and use that for a fast lookup on all those single-board
machines where the number of the northbridges is the number of physical
processors on the board (or x2, if a MCM).

Then, there's a linked list of all further NB descriptors which should
work in your case of confederated systems.

Btw, I've also reused your get_node_id function and the edac changes are
still pending but they should be trivial once this new approach pans
out.

Thanks.

--
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index b3341e9cd8fd..14579f836c28 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -4,6 +4,8 @@
#include <linux/ioport.h>
#include <linux/pci.h>

+#define NUM_POSSIBLE_NBS 8
+
struct amd_nb_bus_dev_range {
u8 bus;
u8 dev_base;
@@ -51,12 +53,22 @@ struct amd_northbridge {
struct pci_dev *link;
struct amd_l3_cache l3_cache;
struct threshold_bank *bank4;
+ u32 node;
+ struct list_head nbl;
};

struct amd_northbridge_info {
u16 num;
u64 flags;
- struct amd_northbridge *nb;
+
+ /*
+ * The first 8 elems are for fast lookup of NB descriptors on single-
+ * system image setups, i.e. "normal" boxes. The nb_list, OTOH, is a
+ * list of additional NB descriptors which exist on confederate systems
+ * like NumaScale.
+ */
+ struct amd_northbridge *nbs[NUM_POSSIBLE_NBS];
+ struct list_head nb_list;
};
extern struct amd_northbridge_info amd_northbridges;

@@ -78,7 +90,22 @@ static inline bool amd_nb_has_feature(unsigned feature)

static inline struct amd_northbridge *node_to_amd_nb(int node)
{
- return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL;
+ struct amd_northbridge_info *nbi = &amd_northbridges;
+ struct amd_northbridge *nb, *nb_tmp;
+
+ if (node < NUM_POSSIBLE_NBS && node < nbi->num)
+ return nbi->nbs[node];
+
+ list_for_each_entry_safe(nb, nb_tmp, &nbi->nb_list, nbl)
+ if (node == nb->node)
+ return nb;
+
+ return NULL;
+}
+
+static inline u32 amd_get_node_id(struct pci_dev *pdev)
+{
+ return (pci_domain_nr(pdev->bus) << 8) | (PCI_SLOT(pdev->devfn) - 0x18);
}

#else
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index aadf3359e2a7..85b47a907c91 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -50,58 +50,71 @@ static struct pci_dev *next_northbridge(struct pci_dev *dev,
return dev;
}

-int amd_cache_northbridges(void)
+static int _alloc_nb_desc(struct pci_dev *misc)
{
- u16 i = 0;
+ struct amd_northbridge_info *nbi = &amd_northbridges;
struct amd_northbridge *nb;
- struct pci_dev *misc, *link;

- if (amd_nb_num())
- return 0;
+ nb = kzalloc(sizeof(*nb), GFP_KERNEL);
+ if (!nb)
+ return -ENOMEM;
+
+ nb->misc = misc;
+ nb->link = next_northbridge(nb->link, amd_nb_link_ids);
+
+ if (nbi->num < NUM_POSSIBLE_NBS) {
+ nbi->nbs[nbi->num] = nb;
+ } else {
+ INIT_LIST_HEAD(&nb->nbl);
+ list_add_tail(&nb->nbl, &nbi->nb_list);
+ }
+
+ nb->node = amd_get_node_id(misc);
+ nbi->num++;
+
+ return 0;
+}

- misc = NULL;
- while ((misc = next_northbridge(misc, amd_nb_misc_ids)) != NULL)
- i++;
+int amd_cache_northbridges(void)
+{
+ struct amd_northbridge_info *nbi = &amd_northbridges;
+ struct cpuinfo_x86 *c = &boot_cpu_data;
+ struct pci_dev *misc = NULL;

- if (i == 0)
+ if (amd_nb_num())
return 0;

- nb = kzalloc(i * sizeof(struct amd_northbridge), GFP_KERNEL);
- if (!nb)
- return -ENOMEM;
+ INIT_LIST_HEAD(&nbi->nb_list);

- amd_northbridges.nb = nb;
- amd_northbridges.num = i;
+ do {
+ misc = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, misc);
+ if (!misc)
+ break;

- link = misc = NULL;
- for (i = 0; i != amd_nb_num(); i++) {
- node_to_amd_nb(i)->misc = misc =
- next_northbridge(misc, amd_nb_misc_ids);
- node_to_amd_nb(i)->link = link =
- next_northbridge(link, amd_nb_link_ids);
- }
+ if (pci_match_id(amd_nb_misc_ids, misc)) {
+ if (_alloc_nb_desc(misc) < 0)
+ return -ENOMEM;
+ }
+ } while (misc);

/* some CPU families (e.g. family 0x11) do not support GART */
- if (boot_cpu_data.x86 == 0xf || boot_cpu_data.x86 == 0x10 ||
- boot_cpu_data.x86 == 0x15)
- amd_northbridges.flags |= AMD_NB_GART;
+ if (c->x86 == 0xf || c->x86 == 0x10 || c->x86 == 0x15)
+ nbi->flags |= AMD_NB_GART;

/*
* Some CPU families support L3 Cache Index Disable. There are some
* limitations because of E382 and E388 on family 0x10.
*/
- if (boot_cpu_data.x86 == 0x10 &&
- boot_cpu_data.x86_model >= 0x8 &&
- (boot_cpu_data.x86_model > 0x9 ||
- boot_cpu_data.x86_mask >= 0x1))
- amd_northbridges.flags |= AMD_NB_L3_INDEX_DISABLE;
+ if (c->x86 == 0x10 && c->x86_model >= 0x8 &&
+ (c->x86_model > 0x9 || c->x86_mask >= 0x1))
+ nbi->flags |= AMD_NB_L3_INDEX_DISABLE;

- if (boot_cpu_data.x86 == 0x15)
- amd_northbridges.flags |= AMD_NB_L3_INDEX_DISABLE;
+ if (c->x86 == 0x15)
+ nbi->flags |= AMD_NB_L3_INDEX_DISABLE;

/* L3 cache partitioning is supported on family 0x15 */
- if (boot_cpu_data.x86 == 0x15)
- amd_northbridges.flags |= AMD_NB_L3_PARTITIONING;
+ if (c->x86 == 0x15)
+ nbi->flags |= AMD_NB_L3_PARTITIONING;

return 0;
}

--
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/