public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver.
@ 2017-05-23 16:15 Scott Telford
  2017-05-23 16:42 ` Ard Biesheuvel
  0 siblings, 1 reply; 9+ messages in thread
From: Scott Telford @ 2017-05-23 16:15 UTC (permalink / raw)
  To: edk2-devel, feng.tian, star.zeng

Copy workaround previously in
ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciRootBridge.c:PciRbPciRead()
to RootBridgeIoPciAccess(), to avoid spurious multiple detections when
scanning buses.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Telford <stelford@cadence.com>
---
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index a0e7e5b..3cca3c1 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -945,6 +945,19 @@ RootBridgeIoPciAccess (
     PciAddress.ExtendedRegister = PciAddress.Register;
   }
 
+  // The UEFI PCI enumerator scans for devices at all possible addresses,
+  // and ignores some PCI rules - this results in some hardware being
+  // detected multiple times. We work around this by faking absent
+  // devices
+  if ((PciAddress.Bus == 0) && ((PciAddress.Device != 0) || (PciAddress.Function != 0))) {
+    *((UINT32 *)Buffer) = 0xffffffff;
+    return EFI_SUCCESS;
+  }
+  if ((PciAddress.Bus == 1) && ((PciAddress.Device != 0) || (PciAddress.Function != 0))) {
+    *((UINT32 *)Buffer) = 0xffffffff;
+    return EFI_SUCCESS;
+  }
+
   Address = PCI_SEGMENT_LIB_ADDRESS (
               RootBridge->RootBridgeIo.SegmentNumber,
               PciAddress.Bus,
-- 
2.2.2



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver.
  2017-05-23 16:15 [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver Scott Telford
@ 2017-05-23 16:42 ` Ard Biesheuvel
  2017-05-24  8:34   ` Scott Telford
  0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2017-05-23 16:42 UTC (permalink / raw)
  To: Scott Telford; +Cc: edk2-devel@lists.01.org, Tian, Feng, Zeng, Star

On 23 May 2017 at 09:15, Scott Telford <stelford@cadence.com> wrote:
> Copy workaround previously in
> ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciRootBridge.c:PciRbPciRead()
> to RootBridgeIoPciAccess(), to avoid spurious multiple detections when
> scanning buses.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Scott Telford <stelford@cadence.com>
> ---
>  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 13 +++++++++++++

This does not belong in the generic driver.

Could you please explain in more detail what the issue is? In any
case, we will need to put this workaround in a Juno specific
implementation of PciExpressLib

>  1 file changed, 13 insertions(+)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> index a0e7e5b..3cca3c1 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> @@ -945,6 +945,19 @@ RootBridgeIoPciAccess (
>      PciAddress.ExtendedRegister = PciAddress.Register;
>    }
>
> +  // The UEFI PCI enumerator scans for devices at all possible addresses,
> +  // and ignores some PCI rules - this results in some hardware being
> +  // detected multiple times. We work around this by faking absent
> +  // devices
> +  if ((PciAddress.Bus == 0) && ((PciAddress.Device != 0) || (PciAddress.Function != 0))) {
> +    *((UINT32 *)Buffer) = 0xffffffff;
> +    return EFI_SUCCESS;
> +  }
> +  if ((PciAddress.Bus == 1) && ((PciAddress.Device != 0) || (PciAddress.Function != 0))) {
> +    *((UINT32 *)Buffer) = 0xffffffff;
> +    return EFI_SUCCESS;
> +  }
> +
>    Address = PCI_SEGMENT_LIB_ADDRESS (
>                RootBridge->RootBridgeIo.SegmentNumber,
>                PciAddress.Bus,
> --
> 2.2.2
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver.
  2017-05-23 16:42 ` Ard Biesheuvel
@ 2017-05-24  8:34   ` Scott Telford
  2017-05-24 14:03     ` Ard Biesheuvel
  0 siblings, 1 reply; 9+ messages in thread
From: Scott Telford @ 2017-05-24  8:34 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Tian, Feng, Zeng, Star

Hi Ard,

Firstly, this patch was meant for my edk2-staging branch, not mainline edk2 - sorry, forgot to edit the subject line!

The issue is that, without this workaround, PCI(e) bridges and devices will be detected multiple times during bus scanning, e.g. a bridge at bus 1 device 0 will also be seen at bus 1 device 1, bus 1 device 2 etc and hence all the devices on the other side of the bridge will be duplicated too. I copied this workaround from the old Juno PCIe driver as I was seeing the same problem when I was testing the Cadence PCIe host bridge library I have been working on. I agree there should probably be a more elegant solution, but I don't know the generic PCI driver code well enough to suggest one at the moment.


Regards,
Scott.

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 23 May 2017 17:42
> To: Scott Telford <stelford@cadence.com>
> Cc: edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; Tian, Feng
> <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH] Copy bus scanning workaround from ARM Juno
> PCIe driver.
> 
> On 23 May 2017 at 09:15, Scott Telford <stelford@cadence.com> wrote:
> > Copy workaround previously in
> >
> ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciRootBridge.c:Pci
> RbPciRead()
> > to RootBridgeIoPciAccess(), to avoid spurious multiple detections when
> > scanning buses.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Scott Telford <stelford@cadence.com>
> > ---
> >  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 13
> +++++++++++++
> 
> This does not belong in the generic driver.
> 
> Could you please explain in more detail what the issue is? In any
> case, we will need to put this workaround in a Juno specific
> implementation of PciExpressLib
> 
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > index a0e7e5b..3cca3c1 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > @@ -945,6 +945,19 @@ RootBridgeIoPciAccess (
> >      PciAddress.ExtendedRegister = PciAddress.Register;
> >    }
> >
> > +  // The UEFI PCI enumerator scans for devices at all possible addresses,
> > +  // and ignores some PCI rules - this results in some hardware being
> > +  // detected multiple times. We work around this by faking absent
> > +  // devices
> > +  if ((PciAddress.Bus == 0) && ((PciAddress.Device != 0) ||
> (PciAddress.Function != 0))) {
> > +    *((UINT32 *)Buffer) = 0xffffffff;
> > +    return EFI_SUCCESS;
> > +  }
> > +  if ((PciAddress.Bus == 1) && ((PciAddress.Device != 0) ||
> (PciAddress.Function != 0))) {
> > +    *((UINT32 *)Buffer) = 0xffffffff;
> > +    return EFI_SUCCESS;
> > +  }
> > +
> >    Address = PCI_SEGMENT_LIB_ADDRESS (
> >                RootBridge->RootBridgeIo.SegmentNumber,
> >                PciAddress.Bus,
> > --
> > 2.2.2
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lists.01.org_mailman_listinfo_edk2-
> 2Ddevel&d=DwIBaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-
> _haXqY&r=0b2qZ7fqn6FWL0d7Bhx7saDL-
> B7sx3Cxz3HPARO7ozc&m=7SGL_JTC4ZjVpm7zTv_uO5MHMY48vYsBzhKmKB
> q66zw&s=W6S9XFt8B-FdfcvWjCtvHTGo3uddEyMfM6BIEMe8dtY&e=

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver.
  2017-05-24  8:34   ` Scott Telford
@ 2017-05-24 14:03     ` Ard Biesheuvel
  2017-05-26 14:50       ` Leif Lindholm
  0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2017-05-24 14:03 UTC (permalink / raw)
  To: Scott Telford, Leif Lindholm
  Cc: edk2-devel@lists.01.org, Tian, Feng, Zeng, Star

(+ Leif)

On 24 May 2017 at 01:34, Scott Telford <stelford@cadence.com> wrote:
> Hi Ard,
>
> Firstly, this patch was meant for my edk2-staging branch, not mainline edk2 - sorry, forgot to edit the subject line!
>

Ah ok. In that case, do whatever you like :-)

> The issue is that, without this workaround, PCI(e) bridges and devices will be detected multiple times during bus scanning, e.g. a bridge at bus 1 device 0 will also be seen at bus 1 device 1, bus 1 device 2 etc and hence all the devices on the other side of the bridge will be duplicated too. I copied this workaround from the old Juno PCIe driver as I was seeing the same problem when I was testing the Cadence PCIe host bridge library I have been working on. I agree there should probably be a more elegant solution, but I don't know the generic PCI driver code well enough to suggest one at the moment.
>

As I said, the workaround belongs in PciExpressLib. You can just clone
that and put the workaround in there.

Interestingly, though, this PCIe IP works fine with the generic ECAM
driver in Linux, so I wonder what the difference is.

Leif, were you aware of this issue?


>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 23 May 2017 17:42
>> To: Scott Telford <stelford@cadence.com>
>> Cc: edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; Tian, Feng
>> <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: Re: [edk2] [PATCH] Copy bus scanning workaround from ARM Juno
>> PCIe driver.
>>
>> On 23 May 2017 at 09:15, Scott Telford <stelford@cadence.com> wrote:
>> > Copy workaround previously in
>> >
>> ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciRootBridge.c:Pci
>> RbPciRead()
>> > to RootBridgeIoPciAccess(), to avoid spurious multiple detections when
>> > scanning buses.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Scott Telford <stelford@cadence.com>
>> > ---
>> >  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 13
>> +++++++++++++
>>
>> This does not belong in the generic driver.
>>
>> Could you please explain in more detail what the issue is? In any
>> case, we will need to put this workaround in a Juno specific
>> implementation of PciExpressLib
>>
>> >  1 file changed, 13 insertions(+)
>> >
>> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
>> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
>> > index a0e7e5b..3cca3c1 100644
>> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
>> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
>> > @@ -945,6 +945,19 @@ RootBridgeIoPciAccess (
>> >      PciAddress.ExtendedRegister = PciAddress.Register;
>> >    }
>> >
>> > +  // The UEFI PCI enumerator scans for devices at all possible addresses,
>> > +  // and ignores some PCI rules - this results in some hardware being
>> > +  // detected multiple times. We work around this by faking absent
>> > +  // devices
>> > +  if ((PciAddress.Bus == 0) && ((PciAddress.Device != 0) ||
>> (PciAddress.Function != 0))) {
>> > +    *((UINT32 *)Buffer) = 0xffffffff;
>> > +    return EFI_SUCCESS;
>> > +  }
>> > +  if ((PciAddress.Bus == 1) && ((PciAddress.Device != 0) ||
>> (PciAddress.Function != 0))) {
>> > +    *((UINT32 *)Buffer) = 0xffffffff;
>> > +    return EFI_SUCCESS;
>> > +  }
>> > +
>> >    Address = PCI_SEGMENT_LIB_ADDRESS (
>> >                RootBridge->RootBridgeIo.SegmentNumber,
>> >                PciAddress.Bus,
>> > --
>> > 2.2.2
>> >
>> > _______________________________________________
>> > edk2-devel mailing list
>> > edk2-devel@lists.01.org
>> > https://urldefense.proofpoint.com/v2/url?u=https-
>> 3A__lists.01.org_mailman_listinfo_edk2-
>> 2Ddevel&d=DwIBaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-
>> _haXqY&r=0b2qZ7fqn6FWL0d7Bhx7saDL-
>> B7sx3Cxz3HPARO7ozc&m=7SGL_JTC4ZjVpm7zTv_uO5MHMY48vYsBzhKmKB
>> q66zw&s=W6S9XFt8B-FdfcvWjCtvHTGo3uddEyMfM6BIEMe8dtY&e=


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver.
  2017-05-24 14:03     ` Ard Biesheuvel
@ 2017-05-26 14:50       ` Leif Lindholm
  2017-05-26 17:37         ` Ard Biesheuvel
  0 siblings, 1 reply; 9+ messages in thread
From: Leif Lindholm @ 2017-05-26 14:50 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Scott Telford, edk2-devel@lists.01.org, Tian, Feng, Zeng, Star

On Wed, May 24, 2017 at 07:03:10AM -0700, Ard Biesheuvel wrote:
> (+ Leif)
> 
> On 24 May 2017 at 01:34, Scott Telford <stelford@cadence.com> wrote:
> > Hi Ard,
> >
> > Firstly, this patch was meant for my edk2-staging branch, not
> > mainline edk2 - sorry, forgot to edit the subject line!
> 
> Ah ok. In that case, do whatever you like :-)

Well, let's try to get keep the staging branch in a state that doesn't
require too heavy reworking before final upstreaming.

> > The issue is that, without this workaround, PCI(e) bridges and
> > devices will be detected multiple times during bus scanning,
> > e.g. a bridge at bus 1 device 0 will also be seen at bus 1 device
> > 1, bus 1 device 2 etc and hence all the devices on the other side
> > of the bridge will be duplicated too. I copied this workaround
> > from the old Juno PCIe driver as I was seeing the same problem
> > when I was testing the Cadence PCIe host bridge library I have
> > been working on. I agree there should probably be a more elegant
> > solution, but I don't know the generic PCI driver code well enough
> > to suggest one at the moment.
> 
> As I said, the workaround belongs in PciExpressLib. You can just clone
> that and put the workaround in there.
> 
> Interestingly, though, this PCIe IP works fine with the generic ECAM
> driver in Linux, so I wonder what the difference is.
> 
> Leif, were you aware of this issue?

I was not aware of this issue.
So a clone of PciExpressLib, whilst suboptimal, sounds like the way to
go for now.

Regards,

Leif

> >> -----Original Message-----
> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> >> Sent: 23 May 2017 17:42
> >> To: Scott Telford <stelford@cadence.com>
> >> Cc: edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; Tian, Feng
> >> <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
> >> Subject: Re: [edk2] [PATCH] Copy bus scanning workaround from ARM Juno
> >> PCIe driver.
> >>
> >> On 23 May 2017 at 09:15, Scott Telford <stelford@cadence.com> wrote:
> >> > Copy workaround previously in
> >> >
> >> ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciRootBridge.c:Pci
> >> RbPciRead()
> >> > to RootBridgeIoPciAccess(), to avoid spurious multiple detections when
> >> > scanning buses.
> >> >
> >> > Contributed-under: TianoCore Contribution Agreement 1.0
> >> > Signed-off-by: Scott Telford <stelford@cadence.com>
> >> > ---
> >> >  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 13
> >> +++++++++++++
> >>
> >> This does not belong in the generic driver.
> >>
> >> Could you please explain in more detail what the issue is? In any
> >> case, we will need to put this workaround in a Juno specific
> >> implementation of PciExpressLib
> >>
> >> >  1 file changed, 13 insertions(+)
> >> >
> >> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> >> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> >> > index a0e7e5b..3cca3c1 100644
> >> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> >> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> >> > @@ -945,6 +945,19 @@ RootBridgeIoPciAccess (
> >> >      PciAddress.ExtendedRegister = PciAddress.Register;
> >> >    }
> >> >
> >> > +  // The UEFI PCI enumerator scans for devices at all possible addresses,
> >> > +  // and ignores some PCI rules - this results in some hardware being
> >> > +  // detected multiple times. We work around this by faking absent
> >> > +  // devices
> >> > +  if ((PciAddress.Bus == 0) && ((PciAddress.Device != 0) ||
> >> (PciAddress.Function != 0))) {
> >> > +    *((UINT32 *)Buffer) = 0xffffffff;
> >> > +    return EFI_SUCCESS;
> >> > +  }
> >> > +  if ((PciAddress.Bus == 1) && ((PciAddress.Device != 0) ||
> >> (PciAddress.Function != 0))) {
> >> > +    *((UINT32 *)Buffer) = 0xffffffff;
> >> > +    return EFI_SUCCESS;
> >> > +  }
> >> > +
> >> >    Address = PCI_SEGMENT_LIB_ADDRESS (
> >> >                RootBridge->RootBridgeIo.SegmentNumber,
> >> >                PciAddress.Bus,
> >> > --
> >> > 2.2.2
> >> >
> >> > _______________________________________________
> >> > edk2-devel mailing list
> >> > edk2-devel@lists.01.org
> >> > https://urldefense.proofpoint.com/v2/url?u=https-
> >> 3A__lists.01.org_mailman_listinfo_edk2-
> >> 2Ddevel&d=DwIBaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-
> >> _haXqY&r=0b2qZ7fqn6FWL0d7Bhx7saDL-
> >> B7sx3Cxz3HPARO7ozc&m=7SGL_JTC4ZjVpm7zTv_uO5MHMY48vYsBzhKmKB
> >> q66zw&s=W6S9XFt8B-FdfcvWjCtvHTGo3uddEyMfM6BIEMe8dtY&e=


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver.
  2017-05-26 14:50       ` Leif Lindholm
@ 2017-05-26 17:37         ` Ard Biesheuvel
  2017-05-29 16:14           ` Scott Telford
  0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2017-05-26 17:37 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: Scott Telford, edk2-devel@lists.01.org, Tian, Feng, Zeng, Star

On 26 May 2017 at 16:50, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, May 24, 2017 at 07:03:10AM -0700, Ard Biesheuvel wrote:
>> (+ Leif)
>>
>> On 24 May 2017 at 01:34, Scott Telford <stelford@cadence.com> wrote:
>> > Hi Ard,
>> >
>> > Firstly, this patch was meant for my edk2-staging branch, not
>> > mainline edk2 - sorry, forgot to edit the subject line!
>>
>> Ah ok. In that case, do whatever you like :-)
>
> Well, let's try to get keep the staging branch in a state that doesn't
> require too heavy reworking before final upstreaming.
>
>> > The issue is that, without this workaround, PCI(e) bridges and
>> > devices will be detected multiple times during bus scanning,
>> > e.g. a bridge at bus 1 device 0 will also be seen at bus 1 device
>> > 1, bus 1 device 2 etc and hence all the devices on the other side
>> > of the bridge will be duplicated too. I copied this workaround
>> > from the old Juno PCIe driver as I was seeing the same problem
>> > when I was testing the Cadence PCIe host bridge library I have
>> > been working on. I agree there should probably be a more elegant
>> > solution, but I don't know the generic PCI driver code well enough
>> > to suggest one at the moment.
>>
>> As I said, the workaround belongs in PciExpressLib. You can just clone
>> that and put the workaround in there.
>>
>> Interestingly, though, this PCIe IP works fine with the generic ECAM
>> driver in Linux, so I wonder what the difference is.
>>
>> Leif, were you aware of this issue?
>
> I was not aware of this issue.
> So a clone of PciExpressLib, whilst suboptimal, sounds like the way to
> go for now.
>

I'd still like to understand why this issue does not occur under
Linux, or if it does occur, if we need an ECAM quirk for Juno to work
around it.

Could you give an example of which hardware combination triggers this issue?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver.
  2017-05-26 17:37         ` Ard Biesheuvel
@ 2017-05-29 16:14           ` Scott Telford
  2017-05-30 10:13             ` Ard Biesheuvel
  0 siblings, 1 reply; 9+ messages in thread
From: Scott Telford @ 2017-05-29 16:14 UTC (permalink / raw)
  To: Ard Biesheuvel, Leif Lindholm
  Cc: edk2-devel@lists.01.org, Tian, Feng, Zeng, Star

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 26 May 2017 18:38
> To: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Scott Telford <stelford@cadence.com>; edk2-devel@lists.01.org <edk2-
> devel@ml01.01.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH] Copy bus scanning workaround from ARM Juno
> PCIe driver.

> I'd still like to understand why this issue does not occur under
> Linux, or if it does occur, if we need an ECAM quirk for Juno to work
> around it.
> 
> Could you give an example of which hardware combination triggers this
> issue?

I don't have access to a Juno board here, so can't reproduce the original problem. It would certainly be interesting to know if the current Juno platform code using the generic PCIe driver suffers from the same problem or not.

The platform I've been working with only exists within a Cadence emulation environment. I've seen the problem when testing an ASMedia ASM1182e bridge and an Intel I210 Ethernet card so it's probably not specific to certain PCIe bridges/devices.

The original Juno PCIe driver (including the workaround) was committed by Olivier Martin at ARM, but I'm not sure if he's still there?

I'm working on moving the workaround into the PciExpressLib layer.

Regards,
Scott.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver.
  2017-05-29 16:14           ` Scott Telford
@ 2017-05-30 10:13             ` Ard Biesheuvel
  2017-05-31 10:51               ` Scott Telford
  0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2017-05-30 10:13 UTC (permalink / raw)
  To: Scott Telford
  Cc: Leif Lindholm, edk2-devel@lists.01.org, Tian, Feng, Zeng, Star

On 29 May 2017 at 16:14, Scott Telford <stelford@cadence.com> wrote:
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: 26 May 2017 18:38
>> To: Leif Lindholm <leif.lindholm@linaro.org>
>> Cc: Scott Telford <stelford@cadence.com>; edk2-devel@lists.01.org <edk2-
>> devel@ml01.01.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star
>> <star.zeng@intel.com>
>> Subject: Re: [edk2] [PATCH] Copy bus scanning workaround from ARM Juno
>> PCIe driver.
>
>> I'd still like to understand why this issue does not occur under
>> Linux, or if it does occur, if we need an ECAM quirk for Juno to work
>> around it.
>>
>> Could you give an example of which hardware combination triggers this
>> issue?
>
> I don't have access to a Juno board here, so can't reproduce the original problem. It would certainly be interesting to know if the current Juno platform code using the generic PCIe driver suffers from the same problem or not.
>
> The platform I've been working with only exists within a Cadence emulation environment. I've seen the problem when testing an ASMedia ASM1182e bridge and an Intel I210 Ethernet card so it's probably not specific to certain PCIe bridges/devices.
>
> The original Juno PCIe driver (including the workaround) was committed by Olivier Martin at ARM, but I'm not sure if he's still there?
>

He used to maintain the ARM bits in Tianocore/EDK2 but he left ARM two
years ago.

> I'm working on moving the workaround into the PciExpressLib layer.
>

Yes, that is best for now.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver.
  2017-05-30 10:13             ` Ard Biesheuvel
@ 2017-05-31 10:51               ` Scott Telford
  0 siblings, 0 replies; 9+ messages in thread
From: Scott Telford @ 2017-05-31 10:51 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Leif Lindholm, edk2-devel@lists.01.org, Tian, Feng, Zeng, Star

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 30 May 2017 11:14
> To: Scott Telford <stelford@cadence.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>; edk2-devel@lists.01.org <edk2-
> devel@ml01.01.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH] Copy bus scanning workaround from ARM Juno
> PCIe driver.

> >> I'd still like to understand why this issue does not occur under
> >> Linux, or if it does occur, if we need an ECAM quirk for Juno to work
> >> around it.

Reading up on the subject, in the PCI Express Base Specification 3.0, section 7.3.1 (Configuration Transaction Rules: Device Number) states "Non-ARI [Alternative Routing-ID Interpretation] Devices must respond to all Type 0 Configuration Read Requests, regardless of the Device Number specified in the Request", which I guess would explain what I'm seeing with the same device appearing at all possible device numbers when reading the VIDs, without the workaround present.

In Linux's drivers/pci/probe.c, the functions pci_scan_slot() and only_one_child() will scan only device 0 function 0 if the parent bus is a PCIe root port or downstream port. See the comment for commit f07852d6442c46c50b59c7e2acc8a1b291f9ab6d: "We can then speed up the pci_scan_slot by skipping the scan of subsequent devfns for PCIe devices which are the direct children of Root Ports or Downstream Ports.  These devices are only permitted to implement device 0, unless they are ARI devices, in which case they'll be scanned by the ARI code above." So it doesn't matter if they respond to all device numbers, because we know they're only allowed to have a device 0.

However, PciLib.c:PciScanBus() in TianoCore will always scan for all possible devices. Looks like it should be behaving more like the Linux driver in the case of PCIe?

Regards,
Scott.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-05-31 10:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-23 16:15 [PATCH] Copy bus scanning workaround from ARM Juno PCIe driver Scott Telford
2017-05-23 16:42 ` Ard Biesheuvel
2017-05-24  8:34   ` Scott Telford
2017-05-24 14:03     ` Ard Biesheuvel
2017-05-26 14:50       ` Leif Lindholm
2017-05-26 17:37         ` Ard Biesheuvel
2017-05-29 16:14           ` Scott Telford
2017-05-30 10:13             ` Ard Biesheuvel
2017-05-31 10:51               ` Scott Telford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox