Attachment 217141 Details for Bug 248514 – Support Skylake-Xeon in imcsmb(4) (take 1)

[patch] Support Skylake-Xeon in imcsmb(4) (take 1)

imcsmb_slx.patch (text/plain), 8.17 KB, created by Ravi Pokala on 2020-08-11 04:07:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ravi Pokala

Created: 2020-08-11 04:07:03 UTC

Size: 8.17 KB

patch

obsolete

>Index: sys/dev/imcsmb/imcsmb_pci.c
>===================================================================
>--- sys/dev/imcsmb/imcsmb_pci.c	(revision 364087)
>+++ sys/dev/imcsmb/imcsmb_pci.c	(working copy)
>@@ -51,16 +51,16 @@
> #include "imcsmb_reg.h"
> #include "imcsmb_var.h"
> 
>-/* (Sandy,Ivy)bridge-Xeon and (Has,Broad)well-Xeon CPUs contain one or two
>- * "Integrated Memory Controllers" (iMCs), and each iMC contains two separate
>- * SMBus controllers. These are used for reading SPD data from the DIMMs, and
>- * for reading the "Thermal Sensor on DIMM" (TSODs). The iMC SMBus controllers
>- * are very simple devices, and have limited functionality compared to
>- * full-fledged SMBus controllers, like the one in Intel ICHs and PCHs.
>+/* (Sandy,Ivy)bridge-Xeon, (Has,Broad)well-Xeon, and Skylake-Xeon CPUs contain
>+ * one or two "Integrated Memory Controllers" (iMCs), and each iMC contains
>+ * two separate SMBus controllers. These are used for reading SPD data from the
>+ * DIMMs, and for reading the "Thermal Sensor on DIMM" (TSODs). The iMC SMBus
>+ * controllers are very simple devices, and have limited functionality compared
>+ * to full-fledged SMBus controllers, like the one in Intel ICHs and PCHs.
>  *
>  * The publicly available documentation for the iMC SMBus controllers can be
>- * found in the CPU datasheets for (Sandy,Ivy)bridge-Xeon and
>- * (Has,broad)well-Xeon, respectively:
>+ * found in the CPU datasheets for (Sandy,Ivy)bridge-Xeon, (Has,Broad)well-Xeon,
>+ * and Skylake-Xeon, respectively:
>  *
>  * https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/
>  *      Sandybridge     xeon-e5-1600-2600-vol-2-datasheet.pdf
>@@ -67,6 +67,7 @@
>  *      Ivybridge       xeon-e5-v2-datasheet-vol-2.pdf
>  *      Haswell         xeon-e5-v3-datasheet-vol-2.pdf
>  *      Broadwell       xeon-e5-v4-datasheet-vol-2.pdf
>+ *      Skylake         xeon-scalable-datasheet-vol-2.pdf
>  *
>  * Another useful resource is the Linux driver. It is not in the main tree.
>  *
>@@ -101,49 +102,59 @@
>  * driver such as this one.
>  */
> 
>-/* PCIe device IDs for (Sandy,Ivy)bridge)-Xeon and (Has,Broad)well-Xeon */
>+/* PCIe device IDs for (Sandy,Ivy)bridge)-Xeon, (Has,Broad)well-Xeon, and
>+ * Skylake-Xeon
>+ */
> #define PCI_VENDOR_INTEL		0x8086
> #define IMCSMB_PCI_DEV_ID_IMC0_SBX	0x3ca8
> #define IMCSMB_PCI_DEV_ID_IMC0_IBX	0x0ea8
> #define IMCSMB_PCI_DEV_ID_IMC0_HSX	0x2fa8
> #define IMCSMB_PCI_DEV_ID_IMC0_BDX	0x6fa8
>-/* (Sandy,Ivy)bridge-Xeon only have a single memory controller per socket */
>+#define IMCSMB_PCI_DEV_ID_IMC0_SLX	0x2085
>+/* Sandybridge-Xeon only has a single memory controller per socket */
>+#define IMCSMB_PCI_DEV_ID_IMC1_IBX	0x0e68
> #define IMCSMB_PCI_DEV_ID_IMC1_HSX	0x2f68
> #define IMCSMB_PCI_DEV_ID_IMC1_BDX	0x6f68
>+#define IMCSMB_PCI_DEV_ID_IMC1_SLX	0x2086
> 
> /* There are two SMBus controllers in each device. These define the registers
>  * for each of these devices.
>  */
>-static struct imcsmb_reg_set imcsmb_regs[] = {
>-	{
>-		.smb_stat = IMCSMB_REG_STATUS0,
>-		.smb_cmd = IMCSMB_REG_COMMAND0,
>-		.smb_cntl = IMCSMB_REG_CONTROL0
>-	},
>-	{
>-		.smb_stat = IMCSMB_REG_STATUS1,
>-		.smb_cmd = IMCSMB_REG_COMMAND1,
>-		.smb_cntl = IMCSMB_REG_CONTROL1
>-	},
>-};
>+static struct imcsmb_reg_set imcsmb_regs[2];
> 
> static struct imcsmb_pci_device {
> 	uint16_t	id;
> 	char		*name;
>+	uint16_t	base;
> } imcsmb_pci_devices[] = {
> 	{IMCSMB_PCI_DEV_ID_IMC0_SBX,
>-	    "Intel Sandybridge Xeon iMC 0 SMBus controllers"	},
>+	    "Intel Sandybridge Xeon iMC 0 SMBus controllers",
>+	    IMCSMB_REG_BASE_SBX},
> 	{IMCSMB_PCI_DEV_ID_IMC0_IBX,
>-	    "Intel Ivybridge Xeon iMC 0 SMBus controllers"	},
>+	    "Intel Ivybridge Xeon iMC 0 SMBus controllers",
>+	    IMCSMB_REG_BASE_IBX},
>+	{IMCSMB_PCI_DEV_ID_IMC1_IBX,
>+	    "Intel Ivybridge Xeon iMC 1 SMBus controllers",
>+	    IMCSMB_REG_BASE_IBX},
> 	{IMCSMB_PCI_DEV_ID_IMC0_HSX,
>-	    "Intel Haswell Xeon iMC 0 SMBus controllers"	},
>+	    "Intel Haswell Xeon iMC 0 SMBus controllers",
>+	    IMCSMB_REG_BASE_HSX},
> 	{IMCSMB_PCI_DEV_ID_IMC1_HSX,
>-	    "Intel Haswell Xeon iMC 1 SMBus controllers"	},
>+	    "Intel Haswell Xeon iMC 1 SMBus controllers",
>+	    IMCSMB_REG_BASE_HSX},
> 	{IMCSMB_PCI_DEV_ID_IMC0_BDX,
>-	    "Intel Broadwell Xeon iMC 0 SMBus controllers"	},
>+	    "Intel Broadwell Xeon iMC 0 SMBus controllers",
>+	    IMCSMB_REG_BASE_BDX},
> 	{IMCSMB_PCI_DEV_ID_IMC1_BDX,
>-	    "Intel Broadwell Xeon iMC 1 SMBus controllers"	},
>-	{0, NULL},
>+	    "Intel Broadwell Xeon iMC 1 SMBus controllers",
>+	    IMCSMB_REG_BASE_BDX},
>+	{IMCSMB_PCI_DEV_ID_IMC0_SLX,
>+	    "Intel Skylake Xeon iMC 0 SMBus controllers",
>+	    IMCSMB_REG_BASE_SLX},
>+	{IMCSMB_PCI_DEV_ID_IMC1_SLX,
>+	    "Intel Skylake Xeon iMC 1 SMBus controllers",
>+	    IMCSMB_REG_BASE_SLX},
>+	{0, NULL, 0},
> };
> 
> /* Device methods. */
>@@ -165,6 +176,9 @@
> imcsmb_pci_attach(device_t dev)
> {
> 	struct imcsmb_pci_softc *sc;
>+	struct imcsmb_pci_device *pci_device;
>+	uint16_t pci_dev_id;
>+	uint16_t base;
> 	device_t child;
> 	int rc;
> 	int unit;
>@@ -174,6 +188,26 @@
> 	sc->dev = dev;
> 	sc->semaphore = 0;
> 
>+	/* Walk the list of supported devices again to find the base address;
>+	 * we only get here if imcsmb_pci_probe() found a match, so it
>+	 * shouldn't be possible to end up with 'base' of 0; handle it anyway.
>+	 */
>+	base = 0;
>+	pci_dev_id = pci_get_device(dev);
>+	for (pci_device = imcsmb_pci_devices;
>+	    pci_device->base != 0;
>+	    pci_device++) {
>+		if (pci_dev_id == pci_device->id) {
>+			base = pci_device->base;
>+			break;
>+		}
>+	}
>+	if (base == 0) {
>+		device_printf(dev, "Could not find register base address\n");
>+		rc = ENXIO;
>+		goto out;
>+	}
>+
> 	/* Create the imcsmbX children */
> 	for (unit = 0; unit < 2; unit++) {
> 		child = device_add_child(dev, "imcsmb", -1);
>@@ -183,6 +217,12 @@
> 			rc = ENXIO;
> 			goto out;
> 		}
>+		/* Initialize the register array for the base address and
>+		 * unit.
>+		 */
>+		imcsmb_regs[unit].smb_stat = IMCSMB_REG_STATUS(base, unit);
>+		imcsmb_regs[unit].smb_cmd = IMCSMB_REG_COMMAND(base, unit);
>+		imcsmb_regs[unit].smb_cntl = IMCSMB_REG_CONTROL(base, unit);
> 		/* Set the child's ivars to point to the appropriate set of
> 		 * the PCI device's registers.
> 		 */
>Index: sys/dev/imcsmb/imcsmb_reg.h
>===================================================================
>--- sys/dev/imcsmb/imcsmb_reg.h	(revision 364087)
>+++ sys/dev/imcsmb/imcsmb_reg.h	(working copy)
>@@ -51,9 +51,9 @@
> 
> #include <dev/smbus/smbconf.h>
> 
>-/* Intel (Sandy,Ivy)bridge and (Has,Broad)well CPUs have integrated memory
>- * controllers (iMCs), each of which having up to two SMBus controllers. They
>- * are programmed via sets of registers in the same PCI device, which are
>+/* Intel (Sandy,Ivy)bridge, (Has,Broad)well, and Skylake CPUs have integrated
>+ * memory controllers (iMCs), each of which having up to two SMBus controllers.
>+ * They are programmed via sets of registers in the same PCI device, which are
>  * identical other than the register numbers.
>  *
>  * The full documentation for these registers can be found in volume two of the
>@@ -60,21 +60,28 @@
>  * datasheets for the CPUs. Refer to the links in imcsmb_pci.c
>  */
> 
>-#define	IMCSMB_REG_STATUS0			0x0180
>-#define	IMCSMB_REG_STATUS1			0x0190
>+/* The base address for the sets of iMC registers is CPU-specific. */
>+#define	IMCSMB_REG_BASE_SBX	0x0180
>+#define	IMCSMB_REG_BASE_IBX	0x0180
>+#define	IMCSMB_REG_BASE_HSX	0x0180
>+#define	IMCSMB_REG_BASE_BDX	0x0180
>+#define	IMCSMB_REG_BASE_SLX	0x0e80
>+
>+/* unit0: base; unit1: (base + 0x10) */
>+#define	IMCSMB_REG_STATUS(_b_, _u_)	((_b_) + ((_u_) * 0x10) + 0x00)
> #define		IMCSMB_STATUS_BUSY_BIT		0x10000000
> #define		IMCSMB_STATUS_BUS_ERROR_BIT	0x20000000
> #define		IMCSMB_STATUS_WRITE_DATA_DONE	0x40000000
> #define		IMCSMB_STATUS_READ_DATA_VALID	0x80000000
> 
>-#define	IMCSMB_REG_COMMAND0			0x0184
>-#define	IMCSMB_REG_COMMAND1			0x0194
>+/* unit0: (base + 0x04); unit1: (base + 0x14) */
>+#define	IMCSMB_REG_COMMAND(_b_, _u_)	((_b_) + ((_u_) * 0x10) + 0x04)
> #define		IMCSMB_CMD_WORD_ACCESS		0x20000000
> #define		IMCSMB_CMD_WRITE_BIT		0x08000000
> #define		IMCSMB_CMD_TRIGGER_BIT		0x80000000
> 
>-#define	IMCSMB_REG_CONTROL0			0x0188
>-#define	IMCSMB_REG_CONTROL1			0x0198
>+/* unit0: (base + 0x08); unit1: (base + 0x18) */
>+#define	IMCSMB_REG_CONTROL(_b_, _u_)	((_b_) + ((_u_) * 0x10) + 0x08)
> #define		IMCSMB_CNTL_POLL_EN		0x00000100
> #define		IMCSMB_CNTL_CLK_OVERRIDE	0x08000000
> #define		IMCSMB_CNTL_DTI_MASK		0xf0000000

Actions: View | Diff

Attachments on bug 248514: 217141