OS-7907 PCI ID ambiguity leads to driver induced mayhem

Reviewed by: John Levon <john.levon@joyent.com>
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Approved by: Patrick Mooney <patrick.mooney@joyent.com>

3 files changed, 150 insertions, 109 deletions

diff --git a/usr/src/uts/intel/io/pci/pci_boot.c b/usr/src/uts/intel/io/pci/pci_boot.c
index 0bf28143cf..3ce28c88b7 100644
--- a/usr/src/uts/intel/io/pci/pci_boot.c
+++ b/usr/src/uts/intel/io/pci/pci_boot.c
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2018 Joyent, Inc.
+ * Copyright 2019 Joyent, Inc.
  */
 
 #include <sys/types.h>
@@ -2172,30 +2172,52 @@ subsys_compat_exclude(ushort_t venid, ushort_t devid, ushort_t subvenid,
 	if ((venid == 0x10de) && (is_display(classcode)))
 		return (B_TRUE);
 
+	/*
+	 * 8086,166 is the Ivy Bridge built-in graphics controller on some
+	 * models. Unfortunately 8086,2044 is the Skylake Server processor
+	 * memory channel device. The Ivy Bridge device uses the Skylake
+	 * ID as its sub-device ID. The GPU is not a memory controller DIMM
+	 * channel.
+	 */
+	if (venid == 0x8086 && devid == 0x166 && subvenid == 0x8086 &&
+	    subdevid == 0x2044) {
+		return (B_TRUE);
+	}
+
 	return (B_FALSE);
 }
 
 /*
- * Set the compatible property to a value compliant with
- * rev 2.1 of the IEEE1275 PCI binding.
- * (Also used for PCI-Express devices).
+ * Set the compatible property to a value compliant with rev 2.1 of the IEEE1275
+ * PCI binding. This is also used for PCI express devices and we have our own
+ * minor additions.
  *
  *   pciVVVV,DDDD.SSSS.ssss.RR	(0)
  *   pciVVVV,DDDD.SSSS.ssss	(1)
+ *   pciSSSS,ssss,s		(2+)
  *   pciSSSS,ssss		(2)
  *   pciVVVV,DDDD.RR		(3)
+ *   pciVVVV,DDDD,p		(4+)
  *   pciVVVV,DDDD		(4)
  *   pciclass,CCSSPP		(5)
  *   pciclass,CCSS		(6)
  *
- * The Subsystem (SSSS) forms are not inserted if
- * subsystem-vendor-id is 0.
+ * The Subsystem (SSSS) forms are not inserted if subsystem-vendor-id is 0 or if
+ * it is a case where we know that the IDs overlap.
+ *
+ * NOTE: For PCI-Express devices "pci" is replaced with "pciex" in 0-6 above and
+ * property 2 is not created as per "1275 bindings for PCI Express
+ * Interconnect".
  *
- * NOTE: For PCI-Express devices "pci" is replaced with "pciex" in 0-6 above
- * property 2 is not created as per "1275 bindings for PCI Express Interconnect"
+ * Unlike on SPARC, we generate both the "pciex" and "pci" versions of the
+ * above. The problem with property 2 is that it has an ambiguity with
+ * property 4. To make sure that drivers can specify either form of 2 or 4
+ * without ambiguity we add a suffix. The 'p' suffix represents the primary ID,
+ * meaning that it is guaranteed to be form 4. The 's' suffix means that it is
+ * sub-vendor and sub-device form, meaning it is guaranteed to be form 2.
  *
- * Set with setprop and \x00 between each
- * to generate the encoded string array form.
+ * Set with setprop and \x00 between each to generate the encoded string array
+ * form.
  */
 void
 add_compatible(dev_info_t *dip, ushort_t subvenid, ushort_t subdevid,
@@ -2204,7 +2226,7 @@ add_compatible(dev_info_t *dip, ushort_t subvenid, ushort_t subdevid,
 {
 	int i = 0;
 	int size = COMPAT_BUFSIZE;
-	char *compat[13];
+	char *compat[15];
 	char *buf, *curr;
 
 	curr = buf = kmem_alloc(size, KM_SLEEP);
@@ -2262,6 +2284,12 @@ add_compatible(dev_info_t *dip, ushort_t subvenid, ushort_t subdevid,
 
 		if (subsys_compat_exclude(vendorid, deviceid, subvenid,
 		    subdevid, revid, classcode) == B_FALSE) {
+			compat[i++] = curr;	/* form 2+ */
+			(void) snprintf(curr, size, "pci%x,%x,s", subvenid,
+			    subdevid);
+			size -= strlen(curr) + 1;
+			curr += strlen(curr) + 1;
+
 			compat[i++] = curr;	/* form 2 */
 			(void) snprintf(curr, size, "pci%x,%x", subvenid,
 			    subdevid);
@@ -2274,6 +2302,11 @@ add_compatible(dev_info_t *dip, ushort_t subvenid, ushort_t subdevid,
 	size -= strlen(curr) + 1;
 	curr += strlen(curr) + 1;
 
+	compat[i++] = curr;	/* form 4+ */
+	(void) snprintf(curr, size, "pci%x,%x,p", vendorid, deviceid);
+	size -= strlen(curr) + 1;
+	curr += strlen(curr) + 1;
+
 	compat[i++] = curr;	/* form 4 */
 	(void) snprintf(curr, size, "pci%x,%x", vendorid, deviceid);
 	size -= strlen(curr) + 1;
diff --git a/usr/src/uts/intel/os/driver_aliases b/usr/src/uts/intel/os/driver_aliases
index 050bc5205d..e914a9870f 100644
--- a/usr/src/uts/intel/os/driver_aliases
+++ b/usr/src/uts/intel/os/driver_aliases
@@ -582,85 +582,85 @@ igb "pciex8086,1f40"
 igb "pciex8086,1f41"
 igb "pciex8086,1f45"
 igb "pciex8086,438"
-imcstub "pci8086,e1e"
-imcstub "pci8086,e1f"
-imcstub "pci8086,e60"
-imcstub "pci8086,e68"
-imcstub "pci8086,e6a"
-imcstub "pci8086,e6b"
-imcstub "pci8086,e6c"
-imcstub "pci8086,e6d"
-imcstub "pci8086,e71"
-imcstub "pci8086,e79"
-imcstub "pci8086,ea0"
-imcstub "pci8086,ea8"
-imcstub "pci8086,eaa"
-imcstub "pci8086,eab"
-imcstub "pci8086,eac"
-imcstub "pci8086,ead"
-imcstub "pci8086,ec8"
-imcstub "pci8086,ec9"
-imcstub "pci8086,eca"
-imcstub "pci8086,2014"
-imcstub "pci8086,2016"
-imcstub "pci8086,2024"
-imcstub "pci8086,2040"
-imcstub "pci8086,2044"
-imcstub "pci8086,2048"
-imcstub "pci8086,2054"
-imcstub "pci8086,2055"
-imcstub "pci8086,2066"
-imcstub "pci8086,208e"
-imcstub "pci8086,2f1e"
-imcstub "pci8086,2f1f"
-imcstub "pci8086,2f28"
-imcstub "pci8086,2f60"
-imcstub "pci8086,2f68"
-imcstub "pci8086,2f6a"
-imcstub "pci8086,2f6b"
-imcstub "pci8086,2f6c"
-imcstub "pci8086,2f6d"
-imcstub "pci8086,2f71"
-imcstub "pci8086,2f79"
-imcstub "pci8086,2fa0"
-imcstub "pci8086,2fa8"
-imcstub "pci8086,2faa"
-imcstub "pci8086,2fab"
-imcstub "pci8086,2fac"
-imcstub "pci8086,2fad"
-imcstub "pci8086,2ffc"
-imcstub "pci8086,2ffd"
-imcstub "pci8086,3c71"
-imcstub "pci8086,3ca0"
-imcstub "pci8086,3ca8"
-imcstub "pci8086,3caa"
-imcstub "pci8086,3cab"
-imcstub "pci8086,3cac"
-imcstub "pci8086,3cad"
-imcstub "pci8086,3ce0"
-imcstub "pci8086,3ce3"
-imcstub "pci8086,3cf4"
-imcstub "pci8086,3cf5"
-imcstub "pci8086,3cf6"
-imcstub "pci8086,6f1e"
-imcstub "pci8086,6f1f"
-imcstub "pci8086,6f28"
-imcstub "pci8086,6f60"
-imcstub "pci8086,6f68"
-imcstub "pci8086,6f6a"
-imcstub "pci8086,6f6b"
-imcstub "pci8086,6f6c"
-imcstub "pci8086,6f6d"
-imcstub "pci8086,6f71"
-imcstub "pci8086,6f79"
-imcstub "pci8086,6fa0"
-imcstub "pci8086,6fa8"
-imcstub "pci8086,6faa"
-imcstub "pci8086,6fab"
-imcstub "pci8086,6fac"
-imcstub "pci8086,6fad"
-imcstub "pci8086,6ffc"
-imcstub "pci8086,6ffd"
+imcstub "pci8086,e1e,p"
+imcstub "pci8086,e1f,p"
+imcstub "pci8086,e60,p"
+imcstub "pci8086,e68,p"
+imcstub "pci8086,e6a,p"
+imcstub "pci8086,e6b,p"
+imcstub "pci8086,e6c,p"
+imcstub "pci8086,e6d,p"
+imcstub "pci8086,e71,p"
+imcstub "pci8086,e79,p"
+imcstub "pci8086,ea0,p"
+imcstub "pci8086,ea8,p"
+imcstub "pci8086,eaa,p"
+imcstub "pci8086,eab,p"
+imcstub "pci8086,eac,p"
+imcstub "pci8086,ead,p"
+imcstub "pci8086,ec8,p"
+imcstub "pci8086,ec9,p"
+imcstub "pci8086,eca,p"
+imcstub "pci8086,2014,p"
+imcstub "pci8086,2016,p"
+imcstub "pci8086,2024,p"
+imcstub "pci8086,2040,p"
+imcstub "pci8086,2044,p"
+imcstub "pci8086,2048,p"
+imcstub "pci8086,2054,p"
+imcstub "pci8086,2055,p"
+imcstub "pci8086,2066,p"
+imcstub "pci8086,208e,p"
+imcstub "pci8086,2f1e,p"
+imcstub "pci8086,2f1f,p"
+imcstub "pci8086,2f28,p"
+imcstub "pci8086,2f60,p"
+imcstub "pci8086,2f68,p"
+imcstub "pci8086,2f6a,p"
+imcstub "pci8086,2f6b,p"
+imcstub "pci8086,2f6c,p"
+imcstub "pci8086,2f6d,p"
+imcstub "pci8086,2f71,p"
+imcstub "pci8086,2f79,p"
+imcstub "pci8086,2fa0,p"
+imcstub "pci8086,2fa8,p"
+imcstub "pci8086,2faa,p"
+imcstub "pci8086,2fab,p"
+imcstub "pci8086,2fac,p"
+imcstub "pci8086,2fad,p"
+imcstub "pci8086,2ffc,p"
+imcstub "pci8086,2ffd,p"
+imcstub "pci8086,3c71,p"
+imcstub "pci8086,3ca0,p"
+imcstub "pci8086,3ca8,p"
+imcstub "pci8086,3caa,p"
+imcstub "pci8086,3cab,p"
+imcstub "pci8086,3cac,p"
+imcstub "pci8086,3cad,p"
+imcstub "pci8086,3ce0,p"
+imcstub "pci8086,3ce3,p"
+imcstub "pci8086,3cf4,p"
+imcstub "pci8086,3cf5,p"
+imcstub "pci8086,3cf6,p"
+imcstub "pci8086,6f1e,p"
+imcstub "pci8086,6f1f,p"
+imcstub "pci8086,6f28,p"
+imcstub "pci8086,6f60,p"
+imcstub "pci8086,6f68,p"
+imcstub "pci8086,6f6a,p"
+imcstub "pci8086,6f6b,p"
+imcstub "pci8086,6f6c,p"
+imcstub "pci8086,6f6d,p"
+imcstub "pci8086,6f71,p"
+imcstub "pci8086,6f79,p"
+imcstub "pci8086,6fa0,p"
+imcstub "pci8086,6fa8,p"
+imcstub "pci8086,6faa,p"
+imcstub "pci8086,6fab,p"
+imcstub "pci8086,6fac,p"
+imcstub "pci8086,6fad,p"
+imcstub "pci8086,6ffc,p"
+imcstub "pci8086,6ffd,p"
 imcstub "pciex8086,e1e"
 imcstub "pciex8086,e1f"
 imcstub "pciex8086,e60"
diff --git a/usr/src/uts/sun4/io/pcicfg.c b/usr/src/uts/sun4/io/pcicfg.c
index da6f740eeb..b1bb75ab1c 100644
--- a/usr/src/uts/sun4/io/pcicfg.c
+++ b/usr/src/uts/sun4/io/pcicfg.c
@@ -20,6 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2019 Joyent, Inc.
  */
 
 /*
@@ -560,8 +561,7 @@ _fini(void)
 }
 
 int
-_info(modinfop)
-struct modinfo *modinfop;
+_info(struct modinfo *modinfop)
 {
 	return (mod_info(&modlinkage, modinfop));
 }
@@ -1324,7 +1324,7 @@ pcicfg_ntbridge_unconfigure_child(dev_info_t *new_device, uint_t devno)
 {
 
 	dev_info_t	*new_ntbridgechild;
-	int 		len, bus;
+	int		len, bus;
 	uint16_t	vid;
 	ddi_acc_handle_t	config_handle;
 	pcicfg_bus_range_t pci_bus_range;
@@ -1491,7 +1491,7 @@ pcicfg_indirect_map(dev_info_t *dip)
 
 static uint_t
 pcicfg_get_ntbridge_child_range(dev_info_t *dip, uint64_t *boundbase,
-				uint64_t *boundlen, uint_t space_type)
+    uint64_t *boundlen, uint_t space_type)
 {
 	int		length, found = DDI_FAILURE, acount, i, ibridge;
 	pci_regspec_t	*assigned;
@@ -2548,8 +2548,7 @@ pcicfg_alloc_hole(hole_t *addr_hole, uint64_t *alast, uint32_t length)
 }
 
 static void
-pcicfg_get_mem(pcicfg_phdl_t *entry,
-	uint32_t length, uint64_t *ans)
+pcicfg_get_mem(pcicfg_phdl_t *entry, uint32_t length, uint64_t *ans)
 {
 	uint64_t new_mem;
 
@@ -2565,8 +2564,7 @@ pcicfg_get_mem(pcicfg_phdl_t *entry,
 }
 
 static void
-pcicfg_get_io(pcicfg_phdl_t *entry,
-	uint32_t length, uint32_t *ans)
+pcicfg_get_io(pcicfg_phdl_t *entry, uint32_t length, uint32_t *ans)
 {
 	uint32_t new_io;
 	uint64_t io_last;
@@ -3404,7 +3402,7 @@ pcicfg_device_off(ddi_acc_handle_t config_handle)
  */
 static int
 pcicfg_set_standard_props(dev_info_t *dip, ddi_acc_handle_t config_handle,
-	uint8_t pcie_dev)
+    uint8_t pcie_dev)
 {
 	int ret;
 	uint16_t val, cap_ptr;
@@ -3598,7 +3596,7 @@ pcicfg_set_busnode_props(dev_info_t *dip, uint8_t pcie_device_type,
 
 static int
 pcicfg_set_childnode_props(dev_info_t *dip, ddi_acc_handle_t config_handle,
-	uint8_t pcie_dev)
+    uint8_t pcie_dev)
 {
 
 	int		ret;
@@ -3720,6 +3718,10 @@ pcicfg_set_childnode_props(dev_info_t *dip, ddi_acc_handle_t config_handle,
 
 	/* pciSSSS.ssss  -> not created for PCIe as per PCIe bindings */
 	if (!pcie_dev) {
+		(void) sprintf(buffer, "pci%x,%x,s", sub_vid, sub_sid);
+		compat[n] = kmem_alloc(strlen(buffer) + 1, KM_SLEEP);
+		(void) strcpy(compat[n++], buffer);
+
 		(void) sprintf(buffer, "pci%x,%x", sub_vid, sub_sid);
 		compat[n] = kmem_alloc(strlen(buffer) + 1, KM_SLEEP);
 		(void) strcpy(compat[n++], buffer);
@@ -3735,6 +3737,12 @@ pcicfg_set_childnode_props(dev_info_t *dip, ddi_acc_handle_t config_handle,
 	compat[n] = kmem_alloc(strlen(buffer) + 1, KM_SLEEP);
 	(void) strcpy(compat[n++], buffer);
 
+	if (!pcie_dev) {
+		(void) sprintf(buffer, "%s%x,%x,p", pprefix, vid, did);
+		compat[n] = kmem_alloc(strlen(buffer) + 1, KM_SLEEP);
+		(void) strcpy(compat[n++], buffer);
+	}
+
 	/* pci[ex]class,CCSSPP */
 	(void) sprintf(buffer, "%sclass,%02x%02x%02x", pprefix,
 	    pclass, psubclass, pif);
@@ -3774,7 +3782,7 @@ pcicfg_set_childnode_props(dev_info_t *dip, ddi_acc_handle_t config_handle,
 
 static void
 pcicfg_set_bus_numbers(ddi_acc_handle_t config_handle,
-uint_t primary, uint_t secondary, uint_t subordinate)
+    uint_t primary, uint_t secondary, uint_t subordinate)
 {
 	DEBUG3("Setting bridge bus-range %d,%d,%d\n", primary, secondary,
 	    subordinate);
@@ -3886,7 +3894,7 @@ pcicfg_setup_bridge(pcicfg_phdl_t *entry,
 
 static void
 pcicfg_update_bridge(pcicfg_phdl_t *entry,
-	ddi_acc_handle_t handle)
+    ddi_acc_handle_t handle)
 {
 	uint_t length;
 
@@ -3943,7 +3951,7 @@ pcicfg_update_bridge(pcicfg_phdl_t *entry,
 /*ARGSUSED*/
 static void
 pcicfg_disable_bridge_probe_err(dev_info_t *dip, ddi_acc_handle_t h,
-	pcicfg_err_regs_t *regs)
+    pcicfg_err_regs_t *regs)
 {
 	uint16_t val;
 
@@ -3982,7 +3990,7 @@ pcicfg_disable_bridge_probe_err(dev_info_t *dip, ddi_acc_handle_t h,
 /*ARGSUSED*/
 static void
 pcicfg_enable_bridge_probe_err(dev_info_t *dip, ddi_acc_handle_t h,
-	pcicfg_err_regs_t *regs)
+    pcicfg_err_regs_t *regs)
 {
 	/* clear any pending errors */
 	pci_config_put16(h, PCI_CONF_STAT, PCI_STAT_S_TARG_AB|
@@ -4890,7 +4898,7 @@ failedchild:
 
 static int
 pcicfg_probe_bridge(dev_info_t *new_child, ddi_acc_handle_t h, uint_t bus,
-	uint_t *highest_bus, boolean_t is_pcie)
+    uint_t *highest_bus, boolean_t is_pcie)
 {
 	uint64_t next_bus;
 	uint_t new_bus, num_slots;
@@ -5741,7 +5749,7 @@ pcicfg_config_teardown(ddi_acc_handle_t *handle)
 
 static int
 pcicfg_add_config_reg(dev_info_t *dip,
-	uint_t bus, uint_t device, uint_t func)
+    uint_t bus, uint_t device, uint_t func)
 {
 	int reg[10] = { PCI_ADDR_CONFIG, 0, 0, 0, 0};
 
@@ -6729,8 +6737,8 @@ pcicfg_remove_assigned_prop(dev_info_t *dip, pci_regspec_t *oldone)
 
 static int
 pcicfg_map_phys(dev_info_t *dip, pci_regspec_t *phys_spec,
-	caddr_t *addrp, ddi_device_acc_attr_t *accattrp,
-	ddi_acc_handle_t *handlep)
+    caddr_t *addrp, ddi_device_acc_attr_t *accattrp,
+    ddi_acc_handle_t *handlep)
 {
 	ddi_map_req_t mr;
 	ddi_acc_hdl_t *hp;
@@ -6805,7 +6813,7 @@ pcicfg_ari_configure(dev_info_t *dip)
 #ifdef DEBUG
 static void
 debug(char *fmt, uintptr_t a1, uintptr_t a2, uintptr_t a3,
-	uintptr_t a4, uintptr_t a5)
+    uintptr_t a4, uintptr_t a5)
 {
 	if (pcicfg_debug == 1) {
 		prom_printf("pcicfg: ");