All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Andreas Hartmann <andihartmann@01019freenet.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Joerg Roedel <joro@8bytes.org>
Subject: Re: [PATCH RFC] pci: ACS quirk for AMD southbridge
Date: Wed, 26 Jun 2013 09:51:04 -0600	[thread overview]
Message-ID: <1372261864.30572.553.camel@ul30vt.home> (raw)
In-Reply-To: <20130626171428.33940803@dualc.maya.org>

On Wed, 2013-06-26 at 17:14 +0200, Andreas Hartmann wrote:
> Bjorn Helgaas wrote:
> > [fix Joerg's email address]
> > 
> > On Tue, Jun 25, 2013 at 10:15 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >> On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
> >> <alex.williamson@redhat.com> wrote:
> >>> We've confirmed that peer-to-peer between these devices is
> >>> not possible.  We can therefore claim that they support a
> >>> subset of ACS.
> >>>
> >>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> >>> Cc: Joerg Roedel <Joerg.Roedel@amd.com>
> >>> ---
> >>>
> >>> Two things about this patch make me a little nervous.  The
> >>> first is that I'd really like to have a pci_is_pcie() test
> >>> in pci_mf_no_p2p_acs_enabled(), but these devices don't
> >>> have a PCIe capability.  That means that if there was a
> >>> topology where these devices sit on a legacy PCI bus,
> >>> we incorrectly return that we're ACS safe here.  That leads
> >>> to my second problem, pciids seems to suggest that some of
> >>> these functions have been around for a while.  Is it just
> >>> this package that's peer-to-peer safe, or is it safe to
> >>> assume that any previous assembly of these functions is
> >>> also p2p safe.  Maybe we need to factor in device revs if
> >>> that uniquely identifies this package?
> >>>
> >>> Looks like another useful device to potentially quirk
> >>> would be:
> >>>
> >>> 00:15.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI SB700/SB800/SB900 PCI to PCI bridge (PCIE port 0)
> >>> 00:15.1 PCI bridge: Advanced Micro Devices [AMD] nee ATI SB700/SB800/SB900 PCI to PCI bridge (PCIE port 1)
> >>> 00:15.2 PCI bridge: Advanced Micro Devices [AMD] nee ATI SB900 PCI to PCI bridge (PCIE port 2)
> >>> 00:15.3 PCI bridge: Advanced Micro Devices [AMD] nee ATI SB900 PCI to PCI bridge (PCIE port 3)
> >>>
> >>> 00:15.0 0604: 1002:43a0
> >>> 00:15.1 0604: 1002:43a1
> >>> 00:15.2 0604: 1002:43a2
> >>> 00:15.3 0604: 1002:43a3
> >>>
> >>>  drivers/pci/quirks.c |   29 +++++++++++++++++++++++++++++
> >>>  1 file changed, 29 insertions(+)
> >>>
> >>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> >>> index 4ebc865..2c84961 100644
> >>> --- a/drivers/pci/quirks.c
> >>> +++ b/drivers/pci/quirks.c
> >>> @@ -3271,11 +3271,40 @@ struct pci_dev *pci_get_dma_source(struct pci_dev *dev)
> >>>         return pci_dev_get(dev);
> >>>  }
> >>>
> >>> +/*
> >>> + * Multifunction devices that do not support peer-to-peer between
> >>> + * functions can claim to support a subset of ACS.  Such devices
> >>> + * effectively enable request redirect (RR) and completion redirect (CR)
> >>> + * since all transactions are redirected to the upstream root complex.
> >>> + */
> >>> +static int pci_mf_no_p2p_acs_enabled(struct pci_dev *dev, u16 acs_flags)
> >>> +{
> >>> +       if (!dev->multifunction)
> >>> +               return -ENODEV;
> >>> +
> >>> +       /* Filter out flags not applicable to multifunction */
> >>> +       acs_flags &= (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC | PCI_ACS_DT);
> >>> +
> >>> +       return acs_flags & ~(PCI_ACS_RR | PCI_ACS_CR) ? 0 : 1;
> >>> +}
> >>> +
> >>>  static const struct pci_dev_acs_enabled {
> >>>         u16 vendor;
> >>>         u16 device;
> >>>         int (*acs_enabled)(struct pci_dev *dev, u16 acs_flags);
> >>>  } pci_dev_acs_enabled[] = {
> >>> +       /*
> >>> +        * AMD/ATI multifunction southbridge devices.  AMD has confirmed
> >>> +        * that peer-to-peer between these devices is not possible, so
> >>> +        * they do support a subset of ACS even though the capability is
> >>> +        * not exposed in config space.
> >>> +        */
> >>> +       { PCI_VENDOR_ID_ATI, 0x4385, pci_mf_no_p2p_acs_enabled },
> >>> +       { PCI_VENDOR_ID_ATI, 0x439c, pci_mf_no_p2p_acs_enabled },
> >>> +       { PCI_VENDOR_ID_ATI, 0x4383, pci_mf_no_p2p_acs_enabled },
> >>> +       { PCI_VENDOR_ID_ATI, 0x439d, pci_mf_no_p2p_acs_enabled },
> >>> +       { PCI_VENDOR_ID_ATI, 0x4384, pci_mf_no_p2p_acs_enabled },
> >>> +       { PCI_VENDOR_ID_ATI, 0x4399, pci_mf_no_p2p_acs_enabled },
> >>>         { 0 }
> >>>  };
> >>>
> >>>
> >>
> >> I was looking for something else and found this old email.  This patch
> >> hasn't been applied and I haven't seen any discussion about it.  Is it
> >> still of interest?  It seems relevant to the current ACS discussion
> >> [1].
> 
> It is absolutely relevant. I always have to patch my kernel to get it
> working to put my pci device to VM. Meanwhile I'm doing it for
> kernel 3.9. I would be very glad to get these patches to the kernel as
> they don't do anything bad!

I'd still like to see this get in too.  IIRC, where we left off was that
Joerg had confirmed with the hardware folks that there is no
peer-to-peer between these devices, but we still had questions about
whether that was true for any instance of these vendor/device IDs.
These devices are re-used in several packages and I'm not sure if we
need to somehow figure out what package (ie. which chipset generation)
we're looking at to know if p2p is used.  Thanks,

Alex



  reply	other threads:[~2013-06-26 15:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-12  5:18 [PATCH RFC] pci: ACS quirk for AMD southbridge Alex Williamson
2012-07-12 16:29 ` Andreas Hartmann
2013-06-26  4:15 ` Bjorn Helgaas
2013-06-26  4:22   ` Bjorn Helgaas
2013-06-26 15:14     ` Andreas Hartmann
2013-06-26 15:51       ` Alex Williamson [this message]
2013-06-26 16:24         ` Andreas Hartmann
2013-06-26 16:44           ` Alex Williamson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1372261864.30572.553.camel@ul30vt.home \
    --to=alex.williamson@redhat.com \
    --cc=andihartmann@01019freenet.de \
    --cc=bhelgaas@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.