Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-26 Thread Alex Williamson
On Wed, 2013-06-26 at 18:24 +0200, Andreas Hartmann wrote:
> Alex Williamson wrote:
> > 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  
> >>> wrote:
>  On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
>   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 
> > Cc: Joerg Roedel 
> > ---
> >
> > 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 

Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-26 Thread Andreas Hartmann
Alex Williamson wrote:
> 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  wrote:
 On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
  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 
> Cc: Joerg Roedel 
> ---
>
> 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)
> 

Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-26 Thread Alex Williamson
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  wrote:
> >> On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
> >>  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 
> >>> Cc: Joerg Roedel 
> >>> ---
> >>>
> >>> 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.  

Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-26 Thread Andreas Hartmann
Bjorn Helgaas wrote:
> [fix Joerg's email address]
> 
> On Tue, Jun 25, 2013 at 10:15 PM, Bjorn Helgaas  wrote:
>> On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
>>  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 
>>> Cc: Joerg Roedel 
>>> ---
>>>
>>> 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!

My multifunction devices are the devices defined in the patch. My
current pci device passed through is a intel ethernet device:

-[:00]-+-00.0  Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge 
(external gfx0 port B)
   +-00.2  Advanced Micro Devices [AMD] nee ATI RD990 I/O Memory 
Management Unit (IOMMU)
   +-02.0-[01]--+-00.0  Advanced Micro Devices [AMD] nee ATI Turks 
[Radeon HD 6570]
   |\-00.1  Advanced Micro Devices [AMD] nee ATI Turks HDMI 
Audio [Radeon HD 6000 Series]
   +-04.0-[02]00.0  Etron Technology, Inc. EJ168 USB 3.0 Host 
Controller
   +-05.0-[03]00.0  Atheros Communications Inc. AR9300 Wireless LAN 
adaptor
   

Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-26 Thread Andreas Hartmann
Bjorn Helgaas wrote:
 [fix Joerg's email address]
 
 On Tue, Jun 25, 2013 at 10:15 PM, Bjorn Helgaas bhelg...@google.com wrote:
 On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
 alex.william...@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.william...@redhat.com
 Cc: Joerg Roedel joerg.roe...@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!

My multifunction devices are the devices defined in the patch. My
current pci device passed through is a intel ethernet device:

-[:00]-+-00.0  Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge 
(external gfx0 port B)
   +-00.2  Advanced Micro Devices [AMD] nee ATI RD990 I/O Memory 
Management Unit (IOMMU)
   +-02.0-[01]--+-00.0  Advanced Micro Devices [AMD] nee ATI Turks 
[Radeon HD 6570]
   |\-00.1  Advanced Micro Devices [AMD] nee ATI Turks HDMI 
Audio [Radeon HD 6000 Series]
   +-04.0-[02]00.0  Etron Technology, Inc. EJ168 USB 3.0 Host 
Controller
   +-05.0-[03]00.0  Atheros Communications Inc. AR9300 Wireless LAN 
adaptor
   +-09.0-[04]00.0  Realtek Semiconductor Co., Ltd. RTL8111/8168B 
PCI Express Gigabit Ethernet controller
   +-0a.0-[05]00.0  Etron Technology, Inc. EJ168 USB 3.0 Host 
Controller
 

Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-26 Thread Alex Williamson
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 bhelg...@google.com wrote:
  On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
  alex.william...@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.william...@redhat.com
  Cc: Joerg Roedel joerg.roe...@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


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-26 Thread Andreas Hartmann
Alex Williamson wrote:
 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 bhelg...@google.com wrote:
 On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
 alex.william...@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.william...@redhat.com
 Cc: Joerg Roedel joerg.roe...@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. 

Does this statement cover your question?
http://article.gmane.org/gmane.comp.emulators.kvm.devel/99402

Andreas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-26 Thread Alex Williamson
On Wed, 2013-06-26 at 18:24 +0200, Andreas Hartmann wrote:
 Alex Williamson wrote:
  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 bhelg...@google.com 
  wrote:
  On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
  alex.william...@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.william...@redhat.com
  Cc: Joerg Roedel joerg.roe...@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. 
 
 Does this statement cover your question?
 http://article.gmane.org/gmane.comp.emulators.kvm.devel/99402

Yeah, perhaps it does.  I initially disregarded it because it's easy to

Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-25 Thread Bjorn Helgaas
[fix Joerg's email address]

On Tue, Jun 25, 2013 at 10:15 PM, Bjorn Helgaas  wrote:
> On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
>  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 
>> Cc: Joerg Roedel 
>> ---
>>
>> 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].
>
> If it's relevant, what's the topology?  Apparently they don't have a
> PCIe capability.  Is the upstream device a PCIe device (a downstream
> port or a root port)?  I assume anything downstream from these AMD
> devices (0x4385, 0x439c, etc.) is plain PCI (not PCIe)?
>
> Bjorn
>
> [1] https://lkml.kernel.org/r/20130607163441.7733.23221.st...@bling.home
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-25 Thread Bjorn Helgaas
On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
 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 
> Cc: Joerg Roedel 
> ---
>
> 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].

If it's relevant, what's the topology?  Apparently they don't have a
PCIe capability.  Is the upstream device a PCIe device (a downstream
port or a root port)?  I assume anything downstream from these AMD
devices (0x4385, 0x439c, etc.) is plain PCI (not PCIe)?

Bjorn

[1] https://lkml.kernel.org/r/20130607163441.7733.23221.st...@bling.home
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-25 Thread Bjorn Helgaas
On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
alex.william...@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.william...@redhat.com
 Cc: Joerg Roedel joerg.roe...@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].

If it's relevant, what's the topology?  Apparently they don't have a
PCIe capability.  Is the upstream device a PCIe device (a downstream
port or a root port)?  I assume anything downstream from these AMD
devices (0x4385, 0x439c, etc.) is plain PCI (not PCIe)?

Bjorn

[1] https://lkml.kernel.org/r/20130607163441.7733.23221.st...@bling.home
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2013-06-25 Thread Bjorn Helgaas
[fix Joerg's email address]

On Tue, Jun 25, 2013 at 10:15 PM, Bjorn Helgaas bhelg...@google.com wrote:
 On Wed, Jul 11, 2012 at 11:18 PM, Alex Williamson
 alex.william...@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.william...@redhat.com
 Cc: Joerg Roedel joerg.roe...@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].

 If it's relevant, what's the topology?  Apparently they don't have a
 PCIe capability.  Is the upstream device a PCIe device (a downstream
 port or a root port)?  I assume anything downstream from these AMD
 devices (0x4385, 0x439c, etc.) is plain PCI (not PCIe)?

 Bjorn

 [1] https://lkml.kernel.org/r/20130607163441.7733.23221.st...@bling.home
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2012-07-12 Thread Andreas Hartmann
Hello Alex,

I tested the patch below against linux 3.4.4 and with this
PCI WLAN-device:

06:07.0 Network controller: Ralink corp. RT2800 802.11n PCI
06:07.0 0280: 1814:0601

The device resides behind a PCI to PCI bridge:

00:14.4 PCI bridge: Advanced Micro Devices [AMD] nee ATI SBx00 PCI to PCI 
Bridge (rev 40) (prog-if 01 [Subtractive decode])
00:14.4 0604: 1002:4384 (rev 40) (prog-if 01 [Subtractive decode])

The device works fine in kvm / 64bit. Surprisingly, it isn't necessary
at all to put the PCI to PCI bridge to the VM. It's enough to put the
WLAN-device to the VM and bind it to vfio-pci. That's all. The bridge
isn't bound to vfio-pci (it's bound to nothing).

I stripped off linux-pci because I'm no member of this list.


Thanks.
kind regards,
Andreas


Alex Williamson 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 
Tested-by: Andreas Hartmann 
> Cc: Joerg Roedel 
> ---
> 
> 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 }
>  };
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] pci: ACS quirk for AMD southbridge

2012-07-12 Thread Andreas Hartmann
Hello Alex,

I tested the patch below against linux 3.4.4 and with this
PCI WLAN-device:

06:07.0 Network controller: Ralink corp. RT2800 802.11n PCI
06:07.0 0280: 1814:0601

The device resides behind a PCI to PCI bridge:

00:14.4 PCI bridge: Advanced Micro Devices [AMD] nee ATI SBx00 PCI to PCI 
Bridge (rev 40) (prog-if 01 [Subtractive decode])
00:14.4 0604: 1002:4384 (rev 40) (prog-if 01 [Subtractive decode])

The device works fine in kvm / 64bit. Surprisingly, it isn't necessary
at all to put the PCI to PCI bridge to the VM. It's enough to put the
WLAN-device to the VM and bind it to vfio-pci. That's all. The bridge
isn't bound to vfio-pci (it's bound to nothing).

I stripped off linux-pci because I'm no member of this list.


Thanks.
kind regards,
Andreas


Alex Williamson 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.william...@redhat.com
Tested-by: Andreas Hartmann andihartm...@01019freenet.de
 Cc: Joerg Roedel joerg.roe...@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 }
  };
  
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH RFC] pci: ACS quirk for AMD southbridge

2012-07-11 Thread Alex Williamson
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 
Cc: Joerg Roedel 
---

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 }
 };
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH RFC] pci: ACS quirk for AMD southbridge

2012-07-11 Thread Alex Williamson
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.william...@redhat.com
Cc: Joerg Roedel joerg.roe...@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 }
 };
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/