Project

General

Profile

Feature #11610

PCI ID ambiguity leads to driver induced mayhem

Added by Robert Mustacchi about 1 year ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Category:
kernel
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

Recently, we encountered a boot hang on a system that was narrowed down to being related to OS-6500. The system in question was an Intel NUC - BOXDC3217IYE. This has an Ivy Bridge era Core i3 in it. This was odd, as we wouldn't have expected a client Ivy Bridge i3 to have any of the RAS features that would relate to the updated Intel memory controller driver.

To figure out what was going on, we tried setting a few breakpoints to see what we would hit. This included:

::bp imc`imc_attach
::bp imc`imc_detach
::bp imc`imc_stub_scan
::bp imcstub`imcstub_attach
::bp imcstub`imcstub_detach

It turned out we hit imcsub`imcstub_attach. This was unexpected. Here's the devinfo bits from this device that we were able to correlate it to:

        display, instance #0
            Driver properties:
                name='ddi-no-autodetach' type=int items=1 dev=none
                    value=00000001
                name='fm-errcb-capable' type=boolean dev=none
                name='fm-ereport-capable' type=boolean dev=none
                name='primary-controller' type=int items=1 dev=none
                    value=00000001
            Hardware properties:
                name='acpi-namespace' type=string items=1
                    value='\_SB_.PCI0.GFX0'
                name='assigned-addresses' type=int items=30
                    value=83001010.00000000.f7800000.00000000.00400000.c3001018.00000000.e0000000.00000000.10000000.81001020.00000000.0000f000.00000000.00000040.a1001000.00000000.000003b0.00000000.0000000c.a1001000.00000000.000003c0.00000000.00000020.a2001000.00000000.000a0000.00000000.00020000
                name='reg' type=int items=35
                    value=00001000.00000000.00000000.00000000.00000000.03001010.00000000.00000000.00000000.00400000.43001018.00000000.00000000.00000000.10000000.01001020.00000000.00000000.00000000.00000040.a1001000.00000000.000003b0.00000000.0000000c.a1001000.00000000.000003c0.00000000.00000020.a2001000.00000000.000a0000.00000000.00020000
                name='compatible' type=string items=7
                    value='pci8086,166.8086.2044.9' + 'pci8086,166.8086.2044' + 'pci8086,2044' + 'pci8086,166.9' + 'pci8086,166' + 'pciclass,030000' + 'pciclass,0300'
                name='model' type=string items=1
                    value='VGA compatible controller'
                name='power-consumption' type=int items=2
                    value=00000001.00000001
                name='fast-back-to-back' type=boolean
                name='devsel-speed' type=int items=1
                    value=00000000
                name='interrupts' type=int items=1
                    value=00000001
                name='max-latency' type=int items=1
                    value=00000000
                name='min-grant' type=int items=1
                    value=00000000
                name='subsystem-vendor-id' type=int items=1
                    value=00008086
                name='subsystem-id' type=int items=1
                    value=00002044
                name='device_type' type=string items=1
                    value='display'
                name='unit-address' type=string items=1
                    value='2'
                name='class-code' type=int items=1
                    value=00030000
                name='revision-id' type=int items=1
                    value=00000009
                name='vendor-id' type=int items=1
                    value=00008086
                name='device-id' type=int items=1
                    value=00000166
                name='vendor-name' type=string items=1
                    value='Intel Corporation'
                name='device-name' type=string items=1
                    value='3rd Gen Core processor Graphics Controller'
                name='subsystem-name' type=string items=1
                    value='unknown subsystem'

Note, this is not meant to be deceiving. It is in fact the GPU. The problem here is that the firmware has programmed it with what is almost certainly the wrong subsystem ID – 8086, 2044. This ID is used on Skylake Scalable Processors (aka server processors) to refer to the integrated memory controller. This means that we have an alias of pci8086,2044. However, the above also has an alias of 8086,2044 because of the sub-vendor and sub-device IDs.

The fundamental problem is that IEEE 1275's pci supplement assumed that the subvendor/subdevice space and the vendor/device space wouldn't overlap in incompatible ways as we have here. This probably lead to us taking over the VGA adapter with a more specific driver than the normal one which would attach to the class.

Unfortunately, a deficiency of the IEEE 1275 PCI compatible register set is that it doesn't have a way to distinguish when a value was the primary vendor and device ID and when it was the secondary.

To rectify this, I propose that we add two new IDs to the compatible array which make it clear whether they are the primary or the secondary/sub-system ID. These would be placed ahead of their corresponding ones. So, if we had a device with vendor, device IDs 8086, 166 and subsystem IDs 1234, 5678, then while we'd normally have the IDs pci1234,5678 and pci8086,166, I suggest adding a final bit that indicates whether this is the primary or the secondary with a ,p or a ,s as appropriate. This would cause these two IDs to look like pci1234,5678,s and pci8086,166,p. This means that this run of IDs (ignoring all the other ones that exist) would be pci1234,5678,s pci1234,5678 pci8086,166,p pci8086,166.

Now you might ask why we shouldn't just remove the subsystem IDs and use the more qualified forms or why we shouldn't just use these new ones in lieu of the old ones. The problem is that many device drivers have aliases with these and it can be rather difficulty to know which are meant to be subsystem IDs and which aren't. In either case it can end up leading to folks loosing access to devices – which is unacceptable. Adding additional IDs that won't match anything that already exists will help solve this problem.

More rationale is written up in illumos IPD 9

#1

Updated by Robert Mustacchi about 1 year ago

I've tested this in the following manners:

  • Used prtconf -v on a booted system to verify that we had aliases and that everything came up
  • Brian Bennett tested this on the originally impacted NUC
  • I spot-checked a few different systems to make sure they booted
  • I tested this on a 2 socket Skylake system and verified that we still had working memory controller drivers
#2

Updated by Electric Monk about 1 year ago

  • Status changed from New to Closed

git commit bfa93d3911fd4856c353c9b190c18cdb98fc36b4

commit  bfa93d3911fd4856c353c9b190c18cdb98fc36b4
Author: Robert Mustacchi <rm@joyent.com>
Date:   2019-09-06T17:26:55.000Z

    11610 PCI ID ambiguity leads to driver induced mayhem
    11611 pchtemp driver should use new IPD 9 aliases
    Reviewed by: John Levon <john.levon@joyent.com>
    Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Also available in: Atom PDF