EDAC, sb_edac: Use cpu family/model in driver detection

Instead of picking a random PCI ID from the dozen or so we need to
access, just use x86_match_cpu() to pick based on CPU model number. The
choosing of PCI devices has been problematic in the past, see

  11249e739929 ("sb_edac: Fix detection on SNB machines")

which fixed problems introduced by

  d0585cd815fa ("sb_edac: Claim a different PCI device").

This is especially ugly if future hardware might not even have
EDAC-relevant registers in PCI config space and we would still be
required to choose some "random" PCI devices to scan for just so our
driver loads.

Is this cleaner/clearer? It deletes much more code than it adds. Only
tested on Broadwell. The driver loads/unloads and loads again. Still
decodes errors too.

Signed-off-by: Tony Luck <tony.luck@intel.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Borislav Petkov <bp@suse.de>

1 files changed, 36 insertions, 98 deletions

diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 342167496626..be398e0cf08a 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -21,6 +21,8 @@
 #include <linux/smp.h>
 #include <linux/bitmap.h>
 #include <linux/math64.h>
+#include <linux/mod_devicetable.h>
+#include <asm/cpu_device_id.h>
 #include <asm/processor.h>
 #include <asm/mce.h>
 
@@ -28,8 +30,6 @@
 
 /* Static vars */
 static LIST_HEAD(sbridge_edac_list);
-static DEFINE_MUTEX(sbridge_edac_lock);
-static int probed;
 
 /*
  * Alter this version for the module when modifications are made
@@ -651,18 +651,6 @@ static const struct pci_id_table pci_dev_descr_broadwell_table[] = {
         {0,}                    /* 0 terminated list. */
 };
 
-/*
- *      pci_device_id   table for which devices we are looking for
- */
-static const struct pci_device_id sbridge_pci_tbl[] = {
-        {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0)},
-        {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA0_TA)},
-        {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_HASWELL_IMC_HA0)},
-        {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BROADWELL_IMC_HA0)},
-        {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_KNL_IMC_SAD0)},
-        {0,}                    /* 0 terminated list. */
-};
-
 
 /****************************************************************************
                         Ancillary status routines
@@ -3344,62 +3332,40 @@ fail0:
         return rc;
 }
 
+#define ICPU(model, table) \
+        { X86_VENDOR_INTEL, 6, model, 0, (unsigned long)&table }
+
+/* Order here must match "enum type" */
+static const struct x86_cpu_id sbridge_cpuids[] = {
+        ICPU(0x2d, pci_dev_descr_sbridge_table),        /* SANDY_BRIDGE */
+        ICPU(0x3e, pci_dev_descr_ibridge_table),        /* IVY_BRIDGE */
+        ICPU(0x3f, pci_dev_descr_haswell_table),        /* HASWELL */
+        ICPU(0x4f, pci_dev_descr_broadwell_table),      /* BROADWELL */
+        ICPU(0x57, pci_dev_descr_knl_table),            /* KNIGHTS_LANDING */
+        { }
+};
+MODULE_DEVICE_TABLE(x86cpu, sbridge_cpuids);
+
 /*
- *      sbridge_probe   Probe for ONE instance of device to see if it is
+ *      sbridge_probe   Get all devices and register memory controllers
  *                      present.
  *      return:
  *              0 for FOUND a device
  *              < 0 for error code
  */
 
-static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+static int sbridge_probe(const struct x86_cpu_id *id)
 {
         int rc = -ENODEV;
         u8 mc, num_mc = 0;
         struct sbridge_dev *sbridge_dev;
-        enum type type = SANDY_BRIDGE;
+        struct pci_id_table *ptable = (struct pci_id_table *)id->driver_data;
 
         /* get the pci devices we want to reserve for our use */
-        mutex_lock(&sbridge_edac_lock);
+        rc = sbridge_get_all_devices(&num_mc, ptable);
 
-        /*
-         * All memory controllers are allocated at the first pass.
-         */
-        if (unlikely(probed >= 1)) {
-                mutex_unlock(&sbridge_edac_lock);
-                return -ENODEV;
-        }
-        probed++;
-
-        switch (pdev->device) {
-        case PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA0_TA:
-                rc = sbridge_get_all_devices(&num_mc,
-                                        pci_dev_descr_ibridge_table);
-                type = IVY_BRIDGE;
-                break;
-        case PCI_DEVICE_ID_INTEL_SBRIDGE_IMC_HA0:
-                rc = sbridge_get_all_devices(&num_mc,
-                                        pci_dev_descr_sbridge_table);
-                type = SANDY_BRIDGE;
-                break;
-        case PCI_DEVICE_ID_INTEL_HASWELL_IMC_HA0:
-                rc = sbridge_get_all_devices(&num_mc,
-                                        pci_dev_descr_haswell_table);
-                type = HASWELL;
-                break;
-        case PCI_DEVICE_ID_INTEL_BROADWELL_IMC_HA0:
-                rc = sbridge_get_all_devices(&num_mc,
-                                        pci_dev_descr_broadwell_table);
-                type = BROADWELL;
-            break;
-        case PCI_DEVICE_ID_INTEL_KNL_IMC_SAD0:
-                rc = sbridge_get_all_devices_knl(&num_mc,
-                                        pci_dev_descr_knl_table);
-                type = KNIGHTS_LANDING;
-                break;
-        }
         if (unlikely(rc < 0)) {
-                edac_dbg(0, "couldn't get all devices for 0x%x\n", pdev->device);
+                edac_dbg(0, "couldn't get all devices\n");
                 goto fail0;
         }
 
@@ -3410,14 +3376,13 @@ static int sbridge_probe(struct pci_dev *pdev, const struct pci_device_id *id)
                          mc, mc + 1, num_mc);
 
                 sbridge_dev->mc = mc++;
-                rc = sbridge_register_mci(sbridge_dev, type);
+                rc = sbridge_register_mci(sbridge_dev, id - sbridge_cpuids);
                 if (unlikely(rc < 0))
                         goto fail1;
         }
 
         sbridge_printk(KERN_INFO, "%s\n", SBRIDGE_REVISION);
 
-        mutex_unlock(&sbridge_edac_lock);
         return 0;
 
 fail1:
@@ -3426,74 +3391,47 @@ fail1:
 
         sbridge_put_all_devices();
 fail0:
-        mutex_unlock(&sbridge_edac_lock);
         return rc;
 }
 
 /*
- *      sbridge_remove  destructor for one instance of device
+ *      sbridge_remove  cleanup
  *
  */
-static void sbridge_remove(struct pci_dev *pdev)
+static void sbridge_remove(void)
 {
         struct sbridge_dev *sbridge_dev;
 
         edac_dbg(0, "\n");
 
-        /*
-         * we have a trouble here: pdev value for removal will be wrong, since
-         * it will point to the X58 register used to detect that the machine
-         * is a Nehalem or upper design. However, due to the way several PCI
-         * devices are grouped together to provide MC functionality, we need
-         * to use a different method for releasing the devices
-         */
-
-        mutex_lock(&sbridge_edac_lock);
-
-        if (unlikely(!probed)) {
-                mutex_unlock(&sbridge_edac_lock);
-                return;
-        }
-
         list_for_each_entry(sbridge_dev, &sbridge_edac_list, list)
                 sbridge_unregister_mci(sbridge_dev);
 
         /* Release PCI resources */
         sbridge_put_all_devices();
-
-        probed--;
-
-        mutex_unlock(&sbridge_edac_lock);
 }
 
-MODULE_DEVICE_TABLE(pci, sbridge_pci_tbl);
-
-/*
- *      sbridge_driver  pci_driver structure for this module
- *
- */
-static struct pci_driver sbridge_driver = {
-        .name     = "sbridge_edac",
-        .probe    = sbridge_probe,
-        .remove   = sbridge_remove,
-        .id_table = sbridge_pci_tbl,
-};
-
 /*
  *      sbridge_init            Module entry function
  *                      Try to initialize this module for its devices
  */
 static int __init sbridge_init(void)
 {
-        int pci_rc;
+        const struct x86_cpu_id *id;
+        int rc;
 
         edac_dbg(2, "\n");
 
+        id = x86_match_cpu(sbridge_cpuids);
+        if (!id)
+                return -ENODEV;
+
         /* Ensure that the OPSTATE is set correctly for POLL or NMI */
         opstate_init();
 
-        pci_rc = pci_register_driver(&sbridge_driver);
-        if (pci_rc >= 0) {
+        rc = sbridge_probe(id);
+
+        if (rc >= 0) {
                 mce_register_decode_chain(&sbridge_mce_dec);
                 if (get_edac_report_status() == EDAC_REPORTING_DISABLED)
                         sbridge_printk(KERN_WARNING, "Loading driver, error reporting disabled.\n");
@@ -3501,9 +3439,9 @@ static int __init sbridge_init(void)
         }
 
         sbridge_printk(KERN_ERR, "Failed to register device with error %d.\n",
-                      pci_rc);
+                      rc);
 
-        return pci_rc;
+        return rc;
 }
 
 /*
@@ -3513,7 +3451,7 @@ static int __init sbridge_init(void)
 static void __exit sbridge_exit(void)
 {
         edac_dbg(2, "\n");
-        pci_unregister_driver(&sbridge_driver);
+        sbridge_remove();
         mce_unregister_decode_chain(&sbridge_mce_dec);
 }