On Thu, Aug 16, 2018 at 02:25:04PM -0600, Alex Williamson wrote:
> On Thu, 16 Aug 2018 22:28:05 +0300
> Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> wrote:
>
> > On Thu, Aug 16, 2018 at 09:13:03AM -0600, Alex Williamson wrote:
> > > 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);
> >
> > I don't know ACS well but could the above have some unwanted
> > side-effects, even if we write back zeroes below?
>
> It can certainly influence in-flight packet routing, but at the point
> we're performing this test, we're about to do that anyway. This should
> only be happening during discovery and we're limited to a set of root
> ports for this test, so we shouldn't have any drivers attached
> downstream from here. For the majority of devices that would pass
> through this quirk, we expect the register to behave as if it were
> read-only so we can only potentially break the already broken C246 port
> through here.
OK.
> We could possibly refine this to fully replace pci_std_enable_acs(),
> even for the matched Intel root ports that seem to be fixed by
> attempting to set the requested flags at the standard location, test if
> they stick, if so consider it done (exit success rather than -ENOTTY),
> if not try the dword version. Thanks,
Before going there, I would like to get some definitive answer from our
PCIe people regarding this (currently waiting for their reply).
