On Thu, 16 Aug 2018 09:10:15 +0300 Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> wrote: > On Wed, Aug 15, 2018 at 04:43:08PM -0600, Alex Williamson wrote: > > Sorry for the self follow-up, but another user pointed me to these: > > > > https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/300-series-chipset-pch-datasheet-vol-1.pdf > > https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/300-series-c240-series-chipset-pch-spec-update.pdf > > > > And the device IDs match and the errata is #3 in the second link, yet: > > > > 00:1c.0 PCI bridge [0604]: Intel Corporation Device [8086:a338] (rev f0) (prog-if 00 [Normal decode]) > > 00:1c.2 PCI bridge [0604]: Intel Corporation Device [8086:a33a] (rev f0) (prog-if 00 [Normal decode]) > > Capabilities: [140 v1] Access Control Services > > ACSCap: SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ UpstreamFwd- EgressCtrl- DirectTrans- > > ACSCtl: SrcValid+ TransBlk- ReqRedir+ CmpltRedir+ UpstreamFwd- EgressCtrl- DirectTrans- > > 00:1c.5 PCI bridge [0604]: Intel Corporation Device [8086:a33d] (rev f0) (prog-if 00 [Normal decode]) > > Capabilities: [140 v1] Access Control Services > > ACSCap: SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ UpstreamFwd- EgressCtrl- DirectTrans- > > ACSCtl: SrcValid+ TransBlk- ReqRedir+ CmpltRedir+ UpstreamFwd- EgressCtrl- DirectTrans- > > 00:1c.6 PCI bridge [0604]: Intel Corporation Device [8086:a33e] (rev f0) (prog-if 00 [Normal decode]) > > Capabilities: [140 v1] Access Control Services > > ACSCap: SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ UpstreamFwd- EgressCtrl- DirectTrans- > > ACSCtl: SrcValid+ TransBlk- ReqRedir+ CmpltRedir+ UpstreamFwd- EgressCtrl- DirectTrans- > > 00:1c.7 PCI bridge [0604]: Intel Corporation Device [8086:a33f] (rev f0) (prog-if 00 [Normal decode]) > > Capabilities: [140 v1] Access Control Services > > ACSCap: SrcValid+ TransBlk+ ReqRedir+ CmpltRedir+ UpstreamFwd- EgressCtrl- DirectTrans- > > ACSCtl: SrcValid+ TransBlk- ReqRedir+ CmpltRedir+ UpstreamFwd- EgressCtrl- DirectTrans- > > > > And the dump of config space is (1c.2): > > > > 140: 0d 00 01 15 0f 00 0d 00 00 00 00 00 00 00 00 00 > > ^^^^^ ^^^^^ > > Cap Ctl > > It certainly looks like so. > > > So it sure seems like the quirk shouldn't apply here. Note that the > > user calls this a C246 chipset, though this isn't a directly addressed > > product SKU in either document. The LPC device ID is a309, which also > > is not directly addressed by the above datasheet. Has Intel fixed this > > in some SKUs, but not others, both using the same root port device > > IDs? Thanks, > > Let me ask around and see if someone here can explain this. Untested, but I wonder if we need something like below to validate that the standard control register is read-only, or in reality the upper half of the dword register where all the bits are reserved on afflicted devices. Adding a test of the LPC device ID seems even messier. Thanks, Alex diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index e249676fbf04..b67d4789a4bb 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -4352,6 +4352,7 @@ static bool pci_quirk_intel_spt_pch_acs_match(struct pci_dev *dev) static int pci_quirk_intel_spt_pch_acs(struct pci_dev *dev, u16 acs_flags) { int pos; + u16 std_ctrl; u32 cap, ctrl; if (!pci_quirk_intel_spt_pch_acs_match(dev)) @@ -4361,6 +4362,11 @@ static int pci_quirk_intel_spt_pch_acs(struct pci_dev *dev, u16 acs_flags) if (!pos) return -ENOTTY; + /* If the standard control bits are set, this is not our dev */ + pci_read_config_word(dev, pos + PCI_ACS_CTRL, &std_ctrl); + if (std_ctrl) + return -ENOTTY; + /* see pci_acs_flags_enabled() */ pci_read_config_dword(dev, pos + PCI_ACS_CAP, &cap); acs_flags &= (cap | PCI_ACS_EC); @@ -4612,6 +4618,7 @@ static int pci_quirk_enable_intel_pch_acs(struct pci_dev *dev) static int pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev) { int pos; + u16 std_ctrl; u32 cap, ctrl; if (!pci_quirk_intel_spt_pch_acs_match(dev)) @@ -4621,6 +4628,18 @@ static int pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev) if (!pos) return -ENOTTY; + /* If the std control word has bits set or is writable, do not quirk */ + pci_read_config_word(dev, pos + PCI_ACS_CTRL, &std_ctrl); + if (std_ctrl) + return -ENOTTY; + + pci_write_config_word(dev, pos + PCI_ACS_CTRL, 0xff); + pci_read_config_word(dev, pos + PCI_ACS_CTRL, &std_ctrl); + if (std_ctrl) { + pci_write_config_word(dev, pos + PCI_ACS_CTRL, 0); + return -ENOTTY; + } + pci_read_config_dword(dev, pos + PCI_ACS_CAP, &cap); pci_read_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, &ctrl);
