Question: PCIe DPC not allowing for link retraining and bus re-scan

Question: PCIe DPC not allowing for link retraining and bus re-scan

[Gabriele Paoloni]

Hi Keith

I am looking at the current DPC implementation in:
http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-dpc.c


If my understanding is correct upon an incoming DPC event
the DPC IRQ handler will:
1) stop and remove all the devices in the hierarchy under the
downstream port that raised the event
2) wait for the data link to be inactive
3) keep the downstream port status in DPC (in fact we set the DPC
   trigger status again here:
   http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-dpc.c#L55)

Now looking at the specs we have (section 6.2.10):
"After software releases the Downstream Port from DPC, the associated Link
will normally attempt to retrain. Software can use Data Link Layer State
Changed interrupts, DL_Active ERR_COR signaling, or both, to signal when 
the Link reaches the DL_Active state again"

So if my understanding is correct in the current Linux implementation
It is not possible to recover a PCIe hierarchy from a DPC event, is it correct?
If this is correct, shouldn't we change the current implementation to release
the port from DPC and re-scan the secondary bus after the conditions described
in "PCIe 3.1 section 6.2.10" are met (I.e. <<Data Link Layer Link Active bit in
the 15 Link Status register reads 0b>> and if it is a root port also <<until
the DPC RP Busy bit reads 0b>>) ?  


Many Thanks

Gab

 

Re: Question: PCIe DPC not allowing for link retraining and bus re-scan

[Keith Busch]

On Mon, Jan 30, 2017 at 11:56:23AM +0000, Gabriele Paoloni wrote:
> Hi Keith
> 
> I am looking at the current DPC implementation in:
> http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-dpc.c
> 
> 
> If my understanding is correct upon an incoming DPC event
> the DPC IRQ handler will:
> 1) stop and remove all the devices in the hierarchy under the
> downstream port that raised the event
> 2) wait for the data link to be inactive
> 3) keep the downstream port status in DPC (in fact we set the DPC
>    trigger status again here:
>    http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-dpc.c#L55)
> 
> Now looking at the specs we have (section 6.2.10):
> "After software releases the Downstream Port from DPC, the associated Link
> will normally attempt to retrain. Software can use Data Link Layer State
> Changed interrupts, DL_Active ERR_COR signaling, or both, to signal when 
> the Link reaches the DL_Active state again"
> 
> So if my understanding is correct in the current Linux implementation
> It is not possible to recover a PCIe hierarchy from a DPC event, is it correct?
> If this is correct, shouldn't we change the current implementation to release
> the port from DPC and re-scan the secondary bus after the conditions described
> in "PCIe 3.1 section 6.2.10" are met (I.e. <<Data Link Layer Link Active bit in
> the 15 Link Status register reads 0b>> and if it is a root port also <<until
> the DPC RP Busy bit reads 0b>>) ?

That sounds reasonable. In all testing I've done, DPC events were
triggered as part of a hot-plug event, so the secondary bus rescan would
automatically happen when a device is hot inserted. The DPC driver didn't
need to initiate the rescan in that case.

But surprise removal is certainly not the only condition DPC can contain a
port, so I agree with you that we may require pcie-dpc initiate a rescan
after releasing the port from containment. I also agree on needing to
poll RP busy for root ports.

Do you want to send patches for these? I could also write them up,
but I don't have a DPC capable root port to test that part.

Thanks,
Keith

RE: Question: PCIe DPC not allowing for link retraining and bus re-scan

[Gabriele Paoloni]

Hi Keith many thanks for coming back on this

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Keith Busch
> Sent: 30 January 2017 16:15
> To: Gabriele Paoloni
> Cc: linux-pci@vger.kernel.org; Linuxarm; liudongdong (C); zhangjukuo;
> Wangzhou (B)
> Subject: Re: Question: PCIe DPC not allowing for link retraining and
> bus re-scan
> 
> On Mon, Jan 30, 2017 at 11:56:23AM +0000, Gabriele Paoloni wrote:
> > Hi Keith
> >
> > I am looking at the current DPC implementation in:
> > http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-dpc.c
> >
> >
> > If my understanding is correct upon an incoming DPC event
> > the DPC IRQ handler will:
> > 1) stop and remove all the devices in the hierarchy under the
> > downstream port that raised the event
> > 2) wait for the data link to be inactive
> > 3) keep the downstream port status in DPC (in fact we set the DPC
> >    trigger status again here:
> >    http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-
> dpc.c#L55)
> >
> > Now looking at the specs we have (section 6.2.10):
> > "After software releases the Downstream Port from DPC, the associated
> Link
> > will normally attempt to retrain. Software can use Data Link Layer
> State
> > Changed interrupts, DL_Active ERR_COR signaling, or both, to signal
> when
> > the Link reaches the DL_Active state again"
> >
> > So if my understanding is correct in the current Linux implementation
> > It is not possible to recover a PCIe hierarchy from a DPC event, is
> it correct?
> > If this is correct, shouldn't we change the current implementation to
> release
> > the port from DPC and re-scan the secondary bus after the conditions
> described
> > in "PCIe 3.1 section 6.2.10" are met (I.e. <<Data Link Layer Link
> Active bit in
> > the 15 Link Status register reads 0b>> and if it is a root port also
> <<until
> > the DPC RP Busy bit reads 0b>>) ?
> 
> That sounds reasonable. In all testing I've done, DPC events were
> triggered as part of a hot-plug event, so the secondary bus rescan
> would
> automatically happen when a device is hot inserted. The DPC driver
> didn't
> need to initiate the rescan in that case.
> 
> But surprise removal is certainly not the only condition DPC can
> contain a
> port, so I agree with you that we may require pcie-dpc initiate a
> rescan
> after releasing the port from containment. I also agree on needing to
> poll RP busy for root ports.
> 
> Do you want to send patches for these? I could also write them up,
> but I don't have a DPC capable root port to test that part.

I don't mind writing the patches but currently I also do not have a
platform to test on.

Do you know if there is any Intel Server/Desktop with full support of
DPC on the RP including "Software Triggering of DPC" (maybe some
machines that are out on the market but you do not have there with you)?

As an alternative I am thinking that maybe I can find a switch with
full DPC support including "Software Triggering of DPC". In this case
we test anything except the RP Busy bit...

The idea is to have a sort of DPC SW Injection module to test DPC.

Cheers
Gab

> 
> Thanks,
> Keith
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question: PCIe DPC not allowing for link retraining and bus re-scan

[Keith Busch]

On Tue, Jan 31, 2017 at 09:35:58AM +0000, Gabriele Paoloni wrote:
> I don't mind writing the patches but currently I also do not have a
> platform to test on.
> 
> Do you know if there is any Intel Server/Desktop with full support of
> DPC on the RP including "Software Triggering of DPC" (maybe some
> machines that are out on the market but you do not have there with you)?

Currently available Intel offerings do not have eDPC capable root ports.
I use switches from Microsemi and PLX with these capabilities on their
downstream ports so I'm limited to non-root port testing at the moment.

> As an alternative I am thinking that maybe I can find a switch with
> full DPC support including "Software Triggering of DPC". In this case
> we test anything except the RP Busy bit...
> 
> The idea is to have a sort of DPC SW Injection module to test DPC.

I think you can s/w inject from userspace with setpci:

  # setpci -s <B:D.f> ECAP_DPC+6.w=40:40

The port has to have a software trigger capability. I can get such a
switch, but don't have one immediately available.

Anyway, I'll write a couple patches and post them when I can get
validation. Thank you for pointing out the current gaps.

RE: Question: PCIe DPC not allowing for link retraining and bus re-scan

[Gabriele Paoloni]

Hi Keith

> -----Original Message-----
> From: Keith Busch [mailto:keith.busch@intel.com]
> Sent: 31 January 2017 15:54
> To: Gabriele Paoloni
> Cc: linux-pci@vger.kernel.org; Linuxarm; liudongdong (C); zhangjukuo;
> Wangzhou (B)
> Subject: Re: Question: PCIe DPC not allowing for link retraining and
> bus re-scan
> 
> On Tue, Jan 31, 2017 at 09:35:58AM +0000, Gabriele Paoloni wrote:
> > I don't mind writing the patches but currently I also do not have a
> > platform to test on.
> >
> > Do you know if there is any Intel Server/Desktop with full support of
> > DPC on the RP including "Software Triggering of DPC" (maybe some
> > machines that are out on the market but you do not have there with
> you)?
> 
> Currently available Intel offerings do not have eDPC capable root
> ports.
> I use switches from Microsemi and PLX with these capabilities on their
> downstream ports so I'm limited to non-root port testing at the moment.
> 
> > As an alternative I am thinking that maybe I can find a switch with
> > full DPC support including "Software Triggering of DPC". In this case
> > we test anything except the RP Busy bit...
> >
> > The idea is to have a sort of DPC SW Injection module to test DPC.
> 
> I think you can s/w inject from userspace with setpci:
> 
>   # setpci -s <B:D.f> ECAP_DPC+6.w=40:40
> 
> The port has to have a software trigger capability. I can get such a
> switch, but don't have one immediately available.
> 
> Anyway, I'll write a couple patches and post them when I can get
> validation. Thank you for pointing out the current gaps.

Many thanks for this, I'll try to get such switch too so maybe
I can help with development and/or validation. 
If you know of specific switches that support for sure such SW
trigger feature please let me know and I'll try to buy them.

I'll ping you as soon as I am able to get something to test this.

Cheers
Gab

Re: Question: PCIe DPC not allowing for link retraining and bus re-scan

[Keith Busch]

On Tue, Jan 31, 2017 at 04:59:31PM +0000, Gabriele Paoloni wrote:
> Many thanks for this, I'll try to get such switch too so maybe
> I can help with development and/or validation. 
> If you know of specific switches that support for sure such SW
> trigger feature please let me know and I'll try to buy them.

I'm not trying to advertise for any particular product, but this is what
I'm currently using for testing my NVMe drives:

  http://www.serialcables.com/products.asp?cat=350&tier=264

It uses these PCIe switches:

  https://www.broadcom.com/products/pcie-switches-bridges/expressfabric/pex9781

I just use this for PCIe SSDs, but there are other vendors that have
capable switches too.

These do support DPC sofware triggering, and the method I mentioned
before is correct:

  # setpci -s <B:D.f> ECAP_DPC+6.w=40:40

On all my testing, the DPC event necesssarily triggers a PCIe HP event
that automatically does the rescan when the containment is released.
Here is a sequence from a single software trigger and nothing else,
using 4.10-rc6:

  root@localhost: ~# setpci -s 89:03.0 ECAP_DPC+6.w=40:40

  dpc 0000:89:03.0:pcie210: DPC containment event, status:0x002f source:0x0000
  dpc 0000:89:03.0:pcie210: DPC extended error triggered, remove downstream devices
  pciehp 0000:89:03.0:pcie204: Slot(3): Link Down
  dpc 0000:89:03.0:pcie210: DPC containment event, status:0x0006 source:0x0000
  pciehp 0000:89:03.0:pcie204: Slot(3): Card not present
  dpc 0000:89:03.0:pcie210: DPC containment event, status:0x0006 source:0x0000
  pciehp 0000:89:03.0:pcie204: Slot(3): Card present
  dpc 0000:89:03.0:pcie210: DPC containment event, status:0x0006 source:0x0000
  pciehp 0000:89:03.0:pcie204: Slot(3): Link Up
  pciehp 0000:89:03.0:pcie204: Slot(3): Link Up event ignored; already powering on
  pci 0000:8d:00.0: [8086:0953] type 00 class 0x010802
  pci 0000:8d:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
  pci 0000:8d:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
  pci 0000:8d:00.0: Max Payload Size set to 256 (was 128, max 256)
  pci 0000:8d:00.0: BAR 6: assigned [mem 0xec000000-0xec00ffff pref]
  pci 0000:8d:00.0: BAR 0: assigned [mem 0xec010000-0xec013fff 64bit]
  pcieport 0000:89:03.0: PCI bridge to [bus 8d]
  pcieport 0000:89:03.0:   bridge window [mem 0xec000000-0xec0fffff]
  pcieport 0000:89:03.0:   bridge window [mem 0x840c00000-0x840dfffff 64bit pre

So I think we're okay to not have DPC initiate the rescan since it would
just compete with pciehp. The only thing we need to do differently is
wait for RP busy if it implements that capability, and no such device
for general availabilty currently exists, as far as I know.

RE: Question: PCIe DPC not allowing for link retraining and bus re-scan

[Gabriele Paoloni]

Hi Keith

Many thanks for coming back on this

> -----Original Message-----
> From: Keith Busch [mailto:keith.busch@intel.com]
> Sent: 02 February 2017 23:23
> To: Gabriele Paoloni
> Cc: linux-pci@vger.kernel.org; Linuxarm; liudongdong (C); zhangjukuo;
> Wangzhou (B)
> Subject: Re: Question: PCIe DPC not allowing for link retraining and
> bus re-scan
> 
> On Tue, Jan 31, 2017 at 04:59:31PM +0000, Gabriele Paoloni wrote:
> > Many thanks for this, I'll try to get such switch too so maybe
> > I can help with development and/or validation.
> > If you know of specific switches that support for sure such SW
> > trigger feature please let me know and I'll try to buy them.
> 
> I'm not trying to advertise for any particular product, but this is
> what
> I'm currently using for testing my NVMe drives:
> 
>   http://www.serialcables.com/products.asp?cat=350&tier=264
> 
> It uses these PCIe switches:
> 
>   https://www.broadcom.com/products/pcie-switches-
> bridges/expressfabric/pex9781

Many thanks for the details, eventually I'll try to buys these...

> 
> I just use this for PCIe SSDs, but there are other vendors that have
> capable switches too.
> 
> These do support DPC sofware triggering, and the method I mentioned
> before is correct:
> 
>   # setpci -s <B:D.f> ECAP_DPC+6.w=40:40
> 
> On all my testing, the DPC event necesssarily triggers a PCIe HP event
> that automatically does the rescan when the containment is released.
> Here is a sequence from a single software trigger and nothing else,
> using 4.10-rc6:

Yes, I have actually read the specs twice...

in 6.2.10 we have:
"Software can use Data Link Layer State Changed interrupts,
DL_Active ERR_COR signaling, or both, to signal when the Link reaches
the DL_Active state again"

and again in 6.2.10.5 Implementation note:
"It is recommended that operating systems use Data Link Layer State
Changed interrupts for signaling when DL_ACTIVE changes state."

So you are right "Data Link Layer State Changed" is a hot-plug
event that will trigger the re-enumeration


> 
>   root@localhost: ~# setpci -s 89:03.0 ECAP_DPC+6.w=40:40
> 
>   dpc 0000:89:03.0:pcie210: DPC containment event, status:0x002f
> source:0x0000
>   dpc 0000:89:03.0:pcie210: DPC extended error triggered, remove
> downstream devices
>   pciehp 0000:89:03.0:pcie204: Slot(3): Link Down
>   dpc 0000:89:03.0:pcie210: DPC containment event, status:0x0006
> source:0x0000
>   pciehp 0000:89:03.0:pcie204: Slot(3): Card not present
>   dpc 0000:89:03.0:pcie210: DPC containment event, status:0x0006
> source:0x0000
>   pciehp 0000:89:03.0:pcie204: Slot(3): Card present
>   dpc 0000:89:03.0:pcie210: DPC containment event, status:0x0006
> source:0x0000
>   pciehp 0000:89:03.0:pcie204: Slot(3): Link Up
>   pciehp 0000:89:03.0:pcie204: Slot(3): Link Up event ignored; already
> powering on
>   pci 0000:8d:00.0: [8086:0953] type 00 class 0x010802
>   pci 0000:8d:00.0: reg 0x10: [mem 0x00000000-0x00003fff 64bit]
>   pci 0000:8d:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
>   pci 0000:8d:00.0: Max Payload Size set to 256 (was 128, max 256)
>   pci 0000:8d:00.0: BAR 6: assigned [mem 0xec000000-0xec00ffff pref]
>   pci 0000:8d:00.0: BAR 0: assigned [mem 0xec010000-0xec013fff 64bit]
>   pcieport 0000:89:03.0: PCI bridge to [bus 8d]
>   pcieport 0000:89:03.0:   bridge window [mem 0xec000000-0xec0fffff]
>   pcieport 0000:89:03.0:   bridge window [mem 0x840c00000-0x840dfffff
> 64bit pre
> 
> So I think we're okay to not have DPC initiate the rescan since it
> would
> just compete with pciehp. The only thing we need to do differently is
> wait for RP busy if it implements that capability, and no such device
> for general availabilty currently exists, as far as I know.

I will send out the patch once I have a root port that supports DPC

Many Thanks again
Gab

RE: Question: PCIe DPC not allowing for link retraining and bus re-scan

[Gabriele Paoloni]

Hi Keith

One thing that I do not understand

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Gabriele Paoloni
> Sent: 30 January 2017 11:56
> To: keith.busch@intel.com
> Cc: linux-pci@vger.kernel.org; Linuxarm; liudongdong (C); zhangjukuo;
> Wangzhou (B)
> Subject: Question: PCIe DPC not allowing for link retraining and bus
> re-scan
> 
> Hi Keith
> 
> I am looking at the current DPC implementation in:
> http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-dpc.c
> 
> 
> If my understanding is correct upon an incoming DPC event
> the DPC IRQ handler will:
> 1) stop and remove all the devices in the hierarchy under the
> downstream port that raised the event
> 2) wait for the data link to be inactive
> 3) keep the downstream port status in DPC (in fact we set the DPC
>    trigger status again here:
>    http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-
> dpc.c#L55)

As you can see above, after waiting for the link to be inactive
we raise again a DPC event (according to PCIe3.0 6.2.10.1 we have
DPC Interrupt Enable bit set to 1b and we set DPC Interrupt Status bit
to 1b), also we keep the port in DPC status as we set also "DPC Trigger
Status" to 1b. However this seems wrong to me as I would expect SW to
clear "DPC Trigger Status" to allow the link to retrain and then to
raise the Hot-plug event...

What do you think?

Thanks
Gab

[...]

Re: Question: PCIe DPC not allowing for link retraining and bus re-scan

[Keith Busch]

On Fri, Feb 03, 2017 at 10:35:47AM +0000, Gabriele Paoloni wrote:
> > If my understanding is correct upon an incoming DPC event
> > the DPC IRQ handler will:
> > 1) stop and remove all the devices in the hierarchy under the
> > downstream port that raised the event
> > 2) wait for the data link to be inactive
> > 3) keep the downstream port status in DPC (in fact we set the DPC
> >    trigger status again here:
> >    http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-
> > dpc.c#L55)
> 
> As you can see above, after waiting for the link to be inactive
> we raise again a DPC event (according to PCIe3.0 6.2.10.1 we have
> DPC Interrupt Enable bit set to 1b and we set DPC Interrupt Status bit
> to 1b), also we keep the port in DPC status as we set also "DPC Trigger
> Status" to 1b. However this seems wrong to me as I would expect SW to
> clear "DPC Trigger Status" to allow the link to retrain and then to
> raise the Hot-plug event...
> 
> What do you think?

I think it's just fine as-is. These are defined as RW1C fields, meaning
write 1 to clear.

RE: Question: PCIe DPC not allowing for link retraining and bus re-scan

[Gabriele Paoloni]

Hi Keith

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Keith Busch
> Sent: 03 February 2017 20:52
> To: Gabriele Paoloni
> Cc: linux-pci@vger.kernel.org; Linuxarm; liudongdong (C); zhangjukuo;
> Wangzhou (B)
> Subject: Re: Question: PCIe DPC not allowing for link retraining and
> bus re-scan
> 
> On Fri, Feb 03, 2017 at 10:35:47AM +0000, Gabriele Paoloni wrote:
> > > If my understanding is correct upon an incoming DPC event
> > > the DPC IRQ handler will:
> > > 1) stop and remove all the devices in the hierarchy under the
> > > downstream port that raised the event
> > > 2) wait for the data link to be inactive
> > > 3) keep the downstream port status in DPC (in fact we set the DPC
> > >    trigger status again here:
> > >    http://lxr.free-electrons.com/source/drivers/pci/pcie/pcie-
> > > dpc.c#L55)
> >
> > As you can see above, after waiting for the link to be inactive
> > we raise again a DPC event (according to PCIe3.0 6.2.10.1 we have
> > DPC Interrupt Enable bit set to 1b and we set DPC Interrupt Status
> bit
> > to 1b), also we keep the port in DPC status as we set also "DPC
> Trigger
> > Status" to 1b. However this seems wrong to me as I would expect SW to
> > clear "DPC Trigger Status" to allow the link to retrain and then to
> > raise the Hot-plug event...
> >
> > What do you think?
> 
> I think it's just fine as-is. These are defined as RW1C fields, meaning
> write 1 to clear.

Uh yes you right

Many thanks

Gab