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 <[email protected]>
---
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 ------
4 files changed, 39 insertions(+), 24 deletions(-)
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index b3341e9..0fd2f0c 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -47,6 +47,7 @@ struct threshold_bank {
};
struct amd_northbridge {
+ u32 node;
struct pci_dev *misc;
struct pci_dev *link;
struct amd_l3_cache l3_cache;
@@ -76,15 +77,33 @@ static inline bool amd_nb_has_feature(unsigned feature)
return ((amd_northbridges.flags & feature) == feature);
}
-static inline struct amd_northbridge *node_to_amd_nb(int node)
+static inline int node_to_amd_index(u32 node)
{
- return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL;
+ int i;
+
+ for (i = 0; i < amd_northbridges.num; i++)
+ if (amd_northbridges.nb[i].node == node)
+ return i;
+
+ return 0;
+}
+
+static inline struct amd_northbridge *node_to_amd_nb(u32 node)
+{
+ return &amd_northbridges.nb[node_to_amd_index(node)];
+}
+
+/* AMD sets the first MC device at device ID 0x18 */
+static inline u32 get_node_id(struct pci_dev *pdev)
+{
+ return (pci_domain_nr(pdev->bus) << 8) | (PCI_SLOT(pdev->devfn) - 0x18);
}
#else
#define amd_nb_num(x) 0
#define amd_nb_has_feature(x) false
+#define node_to_amd_index(x) 0
#define node_to_amd_nb(x) NULL
#endif
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index aadf335..c29ce39 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -75,10 +75,10 @@ int amd_cache_northbridges(void)
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);
+ nb->misc = misc = next_northbridge(misc, amd_nb_misc_ids);
+ nb->node = get_node_id(misc);
+ nb->link = link = next_northbridge(link, amd_nb_link_ids);
+ nb++;
}
/* some CPU families (e.g. family 0x11) do not support GART */
@@ -212,6 +212,7 @@ int amd_set_subcaches(int cpu, int mask)
static int amd_cache_gart(void)
{
u16 i;
+ struct amd_northbridge *nb = amd_northbridges.nb;
if (!amd_nb_has_feature(AMD_NB_GART))
return 0;
@@ -222,9 +223,10 @@ static int amd_cache_gart(void)
return -ENOMEM;
}
- for (i = 0; i != amd_nb_num(); i++)
- pci_read_config_dword(node_to_amd_nb(i)->misc, 0x9c,
- &flush_words[i]);
+ for (i = 0; i != amd_nb_num(); i++) {
+ pci_read_config_dword(nb->misc, 0x9c, &flush_words[i]);
+ nb++;
+ }
return 0;
}
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 5a297a2..9c35565 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2549,7 +2549,7 @@ static int amd64_init_one_instance(struct pci_dev *F2)
struct mem_ctl_info *mci = NULL;
struct edac_mc_layer layers[2];
int err = 0, ret;
- u8 nid = get_node_id(F2);
+ u32 nid = get_node_id(F2);
ret = -ENOMEM;
pvt = kzalloc(sizeof(struct amd64_pvt), GFP_KERNEL);
@@ -2640,7 +2640,7 @@ err_ret:
static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
const struct pci_device_id *mc_type)
{
- u8 nid = get_node_id(pdev);
+ u32 nid = get_node_id(pdev);
struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
struct ecc_settings *s;
int ret = 0;
@@ -2656,7 +2656,7 @@ static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
if (!s)
goto err_out;
- ecc_stngs[nid] = s;
+ ecc_stngs[node_to_amd_index(nid)] = s;
if (!ecc_enabled(F3, nid)) {
ret = -ENODEV;
@@ -2680,7 +2680,7 @@ static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
err_enable:
kfree(s);
- ecc_stngs[nid] = NULL;
+ ecc_stngs[node_to_amd_index(nid)] = NULL;
err_out:
return ret;
@@ -2690,9 +2690,9 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
{
struct mem_ctl_info *mci;
struct amd64_pvt *pvt;
- u8 nid = get_node_id(pdev);
+ u32 nid = get_node_id(pdev);
struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
- struct ecc_settings *s = ecc_stngs[nid];
+ struct ecc_settings *s = ecc_stngs[node_to_amd_index(nid)];
mci = find_mci_by_dev(&pdev->dev);
del_mc_sysfs_attrs(mci);
@@ -2711,12 +2711,12 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
amd_report_gart_errors(false);
amd_unregister_ecc_decoder(amd64_decode_bus_error);
- kfree(ecc_stngs[nid]);
- ecc_stngs[nid] = NULL;
+ kfree(ecc_stngs[node_to_amd_index(nid)]);
+ ecc_stngs[node_to_amd_index(nid)] = NULL;
/* Free the EDAC CORE resources */
mci->pvt_info = NULL;
- mcis[nid] = NULL;
+ mcis[node_to_amd_index(nid)] = NULL;
kfree(pvt);
edac_mc_free(mci);
diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
index 8d48047..90cae61 100644
--- a/drivers/edac/amd64_edac.h
+++ b/drivers/edac/amd64_edac.h
@@ -290,12 +290,6 @@
/* MSRs */
#define MSR_MCGCTL_NBE BIT(4)
-/* AMD sets the first MC device at device ID 0x18. */
-static inline u8 get_node_id(struct pci_dev *pdev)
-{
- return PCI_SLOT(pdev->devfn) - 0x18;
-}
-
enum amd_families {
K8_CPUS = 0,
F10_CPUS,
--
1.7.9.5
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 <[email protected]>
> ---
> 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 ------
> 4 files changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
> index b3341e9..0fd2f0c 100644
> --- a/arch/x86/include/asm/amd_nb.h
> +++ b/arch/x86/include/asm/amd_nb.h
> @@ -47,6 +47,7 @@ struct threshold_bank {
> };
>
> struct amd_northbridge {
> + u32 node;
Right, as pointed out before, are 4 bytes really needed for node index
or smaller is still ok?
> struct pci_dev *misc;
> struct pci_dev *link;
> struct amd_l3_cache l3_cache;
> @@ -76,15 +77,33 @@ static inline bool amd_nb_has_feature(unsigned feature)
> return ((amd_northbridges.flags & feature) == feature);
> }
>
> -static inline struct amd_northbridge *node_to_amd_nb(int node)
> +static inline int node_to_amd_index(u32 node)
> {
> - return (node < amd_northbridges.num) ? &amd_northbridges.nb[node] : NULL;
> + int i;
> +
> + for (i = 0; i < amd_northbridges.num; i++)
> + if (amd_northbridges.nb[i].node == node)
> + return i;
> +
> + return 0;
Return -1 instead, in the failure case...
> +}
> +
> +static inline struct amd_northbridge *node_to_amd_nb(u32 node)
> +{
> + return &amd_northbridges.nb[node_to_amd_index(node)];
.. which needs to be handled here.
> +}
> +
> +/* AMD sets the first MC device at device ID 0x18 */
> +static inline u32 get_node_id(struct pci_dev *pdev)
> +{
> + return (pci_domain_nr(pdev->bus) << 8) | (PCI_SLOT(pdev->devfn) - 0x18);
> }
>
> #else
>
> #define amd_nb_num(x) 0
> #define amd_nb_has_feature(x) false
> +#define node_to_amd_index(x) 0
> #define node_to_amd_nb(x) NULL
Those are both used in other files and thus in the global namespace, can we
change them to
amd_nb_node_index(x)
and
and_nb_get_desc(node)
?
> #endif
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index aadf335..c29ce39 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -75,10 +75,10 @@ int amd_cache_northbridges(void)
>
> 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);
> + nb->misc = misc = next_northbridge(misc, amd_nb_misc_ids);
> + nb->node = get_node_id(misc);
> + nb->link = link = next_northbridge(link, amd_nb_link_ids);
> + nb++;
Pls reorder for better readability:
nb->misc = misc = next_northbridge(misc, amd_nb_misc_ids);
nb->link = link = next_northbridge(link, amd_nb_link_ids);
nb->node = get_node_id(misc);
nb++;
> }
>
> /* some CPU families (e.g. family 0x11) do not support GART */
> @@ -212,6 +212,7 @@ int amd_set_subcaches(int cpu, int mask)
> static int amd_cache_gart(void)
> {
> u16 i;
> + struct amd_northbridge *nb = amd_northbridges.nb;
>
> if (!amd_nb_has_feature(AMD_NB_GART))
> return 0;
> @@ -222,9 +223,10 @@ static int amd_cache_gart(void)
> return -ENOMEM;
> }
>
> - for (i = 0; i != amd_nb_num(); i++)
> - pci_read_config_dword(node_to_amd_nb(i)->misc, 0x9c,
> - &flush_words[i]);
> + for (i = 0; i != amd_nb_num(); i++) {
> + pci_read_config_dword(nb->misc, 0x9c, &flush_words[i]);
> + nb++;
> + }
>
> return 0;
> }
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 5a297a2..9c35565 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -2549,7 +2549,7 @@ static int amd64_init_one_instance(struct pci_dev *F2)
> struct mem_ctl_info *mci = NULL;
> struct edac_mc_layer layers[2];
> int err = 0, ret;
> - u8 nid = get_node_id(F2);
> + u32 nid = get_node_id(F2);
>
> ret = -ENOMEM;
> pvt = kzalloc(sizeof(struct amd64_pvt), GFP_KERNEL);
> @@ -2640,7 +2640,7 @@ err_ret:
> static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
> const struct pci_device_id *mc_type)
> {
> - u8 nid = get_node_id(pdev);
> + u32 nid = get_node_id(pdev);
Let's add a
node_idx = node_to_amd_index(nid);
and use it below instead, ok?
> struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
> struct ecc_settings *s;
> int ret = 0;
> @@ -2656,7 +2656,7 @@ static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
> if (!s)
> goto err_out;
>
> - ecc_stngs[nid] = s;
> + ecc_stngs[node_to_amd_index(nid)] = s;
>
> if (!ecc_enabled(F3, nid)) {
> ret = -ENODEV;
> @@ -2680,7 +2680,7 @@ static int __devinit amd64_probe_one_instance(struct pci_dev *pdev,
>
> err_enable:
> kfree(s);
> - ecc_stngs[nid] = NULL;
> + ecc_stngs[node_to_amd_index(nid)] = NULL;
>
> err_out:
> return ret;
> @@ -2690,9 +2690,9 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
> {
> struct mem_ctl_info *mci;
> struct amd64_pvt *pvt;
> - u8 nid = get_node_id(pdev);
> + u32 nid = get_node_id(pdev);
> struct pci_dev *F3 = node_to_amd_nb(nid)->misc;
> - struct ecc_settings *s = ecc_stngs[nid];
> + struct ecc_settings *s = ecc_stngs[node_to_amd_index(nid)];
>
> mci = find_mci_by_dev(&pdev->dev);
> del_mc_sysfs_attrs(mci);
> @@ -2711,12 +2711,12 @@ static void __devexit amd64_remove_one_instance(struct pci_dev *pdev)
> amd_report_gart_errors(false);
> amd_unregister_ecc_decoder(amd64_decode_bus_error);
>
> - kfree(ecc_stngs[nid]);
> - ecc_stngs[nid] = NULL;
> + kfree(ecc_stngs[node_to_amd_index(nid)]);
> + ecc_stngs[node_to_amd_index(nid)] = NULL;
>
> /* Free the EDAC CORE resources */
> mci->pvt_info = NULL;
> - mcis[nid] = NULL;
> + mcis[node_to_amd_index(nid)] = NULL;
>
> kfree(pvt);
> edac_mc_free(mci);
> diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h
> index 8d48047..90cae61 100644
> --- a/drivers/edac/amd64_edac.h
> +++ b/drivers/edac/amd64_edac.h
> @@ -290,12 +290,6 @@
> /* MSRs */
> #define MSR_MCGCTL_NBE BIT(4)
>
> -/* AMD sets the first MC device at device ID 0x18. */
> -static inline u8 get_node_id(struct pci_dev *pdev)
> -{
> - return PCI_SLOT(pdev->devfn) - 0x18;
> -}
> -
> enum amd_families {
> K8_CPUS = 0,
> F10_CPUS,
> --
> 1.7.9.5
Thanks.
--
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
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 <[email protected]>
> > ---
> > 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
On Fri, Oct 12, 2012 at 11:57:00AM +0200, Steffen Persvold wrote:
> This patch looks very clean and should serve our purpose as well (I'll
> double check with Daniel).
Thanks.
I'll run it today or next week to check whether it works as expected -
will let you know if there are issues.
> Regarding the size of the "node" variable, you asked before. The
> theoretical maximum number of AMD NBs we can have in a confederated
> NumaConnect system _today_ is 8*4096 (8 NBs per system, 4096
> systems) so technically this could fit into a u16 instead of a u32
> (you'll have to shift left by 3 instead of 8).
>
> However, to allow some flexibility I think a u32 is better and I
> think we can live with those two extra bytes per struct member, or ?
Yeah, we can leave it u32 (I did it so in my patch) because of two
reasons:
* node_to_amd_nb gets an int as an argument
* compiler padding
On the largest of your systems that's 2*8*4096 = 64K but I'll go ahead
and assume you have enough memory on those so that nobody cares about
64K there. :-)
On Fri, Oct 12, 2012 at 05:46:28PM +0800, Daniel J Blueman wrote:
> Nice. Sorry for the delays in getting back to you, since I'm just
> chasing down the last issue we see with kobjects, amongst other work.
>
> I'll check this out next week, and thanks for keeping an eye on it!
Sure. It is probably convenient that you're busy currently - I can test
it here first and fixup any issues before you do. I'll send an official
version with changelog etc after having tested it here.
Thank you both.
--
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
On 10/12/2012 11:33, Borislav Petkov wrote:
> 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 <[email protected]>
>>> ---
>>> 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.
>
Hi Boris,
This patch looks very clean and should serve our purpose as well (I'll
double check with Daniel).
Regarding the size of the "node" variable, you asked before. The
theoretical maximum number of AMD NBs we can have in a confederated
NumaConnect system _today_ is 8*4096 (8 NBs per system, 4096 systems) so
technically this could fit into a u16 instead of a u32 (you'll have to
shift left by 3 instead of 8).
However, to allow some flexibility I think a u32 is better and I think
we can live with those two extra bytes per struct member, or ?
Cheers,
--
Steffen Persvold, Chief Architect NumaChip
Numascale AS - http://www.numascale.com
Tel: +47 92 49 25 54 Skype: spersvold