Re: [PATCH] PCI: Add ACS quirk for Intel 300 series chipset

[Mika Westerberg <mika.westerberg@linux.intel.com>]

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@linux.intel.com> 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).