On Mon, Aug 15, 2016 at 03:23:16PM -0400, Keith Busch wrote: > On Mon, Aug 15, 2016 at 12:40:02PM -0500, Bjorn Helgaas wrote: > > On Mon, Aug 08, 2016 at 02:19:02PM -0600, Keith Busch wrote: > > > +/* > > > + * The PCIe slot capabilities for Intel compatible Hot-swap backplane advertise > > > + * attention and power indicators, but will do the wrong thing if used in a > > > + * standard way. Ignore these. > > > + */ > > > > Hmm. So I guess you're saying these devices are defective? Is there > > an erratum we can reference? > > > > What exactly does "do the wrong thing" mean? These are indicators, so > > the only thing we really do is turn them on and off. I think we do > > that with pcie_write_cmd_nowait(), and all the synchronization there > > is a little messy. Maybe we got that wrong somehow? > > > > It's hard to believe something as simple as controlling an LED is > > broken. If it *is* broken, I would think the breakage would be > > platform-dependent, not just device-dependent, i.e., I would suspect > > something wrong with motherboard wiring or firmware. > > This is actually a "feature". The devices listed in the patch re-purpose > the spec defined capability and control bits for Attention and Power > indicators. The control values match IBPI (International Blinking Pattern > Interpretation) rather than the spec definition. > > Since these operate in a non-standard way, we'd just as soon not let > the kernel know about them (an incorrect LED pattern will definitely > occur). The LEDs are to be set from user space by 'ledmon' instead. > > Had I my way, the hardware wouldn't advertise the capability in the > first place. I rarely get my way, so I instead get to publicly defend > the quirk. :) Usually when I think something is totally stupid, it's because I don't know the whole story. So it might make more sense and lead to a better solution if you could tell us more about your intent here. According to the Linux PCI database, the devices you want to quirk are: 2030 Sky Lake-E PCI Express Root Port 1A 2031 Sky Lake-E PCI Express Root Port 1B 2032 Sky Lake-E PCI Express Root Port 1C 2033 Sky Lake-E PCI Express Root Port 1D So are you saying that on every platform that uses Sky Lake-E, these indicators are non-standard in this way? IBPI looks like it's targeted at storage arrays, since it has states for "drive not present", "fail", "rebuild", "hotspare", etc. Maybe there's some sense for Sky Lake-E platforms with directly-attached storage. But if somebody built a Sky Lake-E platform with one of these Root Ports leading to a plain hotplug PCIe slot with regular indicators, your quirk would break them, wouldn't it? Or are you imposing constraints on how those Root Ports can be used? How does 'ledmon' manage the indicators? The kernel (pciehp) uses the Slot Control register, which is not completely trivial because of the Command Completed synchronization required. I'm hoping ledmon isn't going to mess up that synchronization. How does this work for other OSes? Are you proposing similar changes to Windows? What's your plan for backwards compatibility? Just accept that old OSes won't be able to operate the indicators correctly until they're patched with this quirk? You must have set that capability bit for some reason. You don't want the OS to consume it, so who *do* you expect to consume it, and how (direct PCI config access, lspci, etc.), and what are they supposed to do with it? Still scratching my head, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
