[edk2-devel] [PATCH edk2-platforms 12/16] Silicon/NXP: PciSegmentLib: LsGen4Ctrl: Add Workaround for A-011264

[Wasim Khan (OSS)]

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel at arm.com>
> Sent: Friday, May 22, 2020 3:10 PM
> To: Wasim Khan (OSS) <wasim.khan at oss.nxp.com>; devel at edk2.groups.io;
> Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Vabhav Sharma
> <vabhav.sharma at nxp.com>; Varun Sethi <V.Sethi at nxp.com>;
> leif at nuviainc.com; jon at solid-run.com
> Cc: Wasim Khan <wasim.khan at nxp.com>
> Subject: Re: [PATCH edk2-platforms 12/16] Silicon/NXP: PciSegmentLib:
> LsGen4Ctrl: Add Workaround for A-011264
> 
> On 5/22/20 1:02 AM, Wasim Khan wrote:
> > From: Wasim Khan <wasim.khan at nxp.com>
> >
> > With PCIe LsGen4 controller, clearing the Bus Master Enable bit in
> > Command register blocks all outbound transactions to be sent out in RC
> > mode.
> >
> > According to PCI Express base specification, the Command register’s
> > Bus Master Enable bit of a PCI Express RC controller can only control
> > the forwarding of memory requests received at its root port in the
> > upstream direction. In other words, clearing the Bus Master Enable bit
> > must not block all outbound transactions to be sent out toward RC’s
> > downstream devices. Due to this erratum, when the Command register’s
> > Bus Master Enable bit is cleared, all the outbound transactions from
> > the device’s internal bus masters, including but not limited to
> > configuration read and write transactions, are terminated with the
> > slave error (SLVERR) response status on the PCI Express RC
> > controller’s internal AXI bus interface.
> >
> > Signed-off-by: Wasim Khan <wasim.khan at nxp.com>
> > ---
> >   Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c | 17
> +++++++++++++++++
> >   1 file changed, 17 insertions(+)
> >
> > diff --git a/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> > b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> > index 02a1525ef308..c3bc14820ea5 100755
> > --- a/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> > +++ b/Silicon/NXP/Library/PciSegmentLib/PciSegmentLib.c
> > @@ -39,6 +39,21 @@ static BOOLEAN PciLsGen4Ctrl;
> >
> >   STATIC
> >   VOID
> > +PciLsGen4SetBusMaster (
> > +  IN EFI_PHYSICAL_ADDRESS Dbi
> > +  )
> > +{
> > +  UINT32 Val;
> > +
> > +  /* Make sure the Master Enable bit not cleared */
> 
> Please use // style comments

OK

> 
> > +  Val = PciLsGen4Read32 ((UINTN)Dbi, PCI_COMMAND_OFFSET);
> > +  if (!(Val & EFI_PCI_COMMAND_BUS_MASTER)) {
> > +    PciLsGen4Write32 ((UINTN)Dbi, PCI_COMMAND_OFFSET, Val |
> > +EFI_PCI_COMMAND_BUS_MASTER);
> > +  }
> > +}
> > +
> > +STATIC
> > +VOID
> >   PcieCfgSetTarget (
> >     IN EFI_PHYSICAL_ADDRESS Dbi,
> >     IN UINT32 Target)
> > @@ -71,6 +86,8 @@ PciLsGen4GetConfigBase (
> >     UINT32 Target;
> >
> >     if (Bus) {
> > +    PciLsGen4SetBusMaster (PCI_SEG0_DBI_BASE + PCI_DBI_SIZE_DIFF*
> > + Segment);
> > +
> 
> Can't we just set this once and never touch it? Or is that tricky?

Yes, it's a bit tricky . I checked that we are getting multiple calls when BusMaster is zero.
As per the Errata, we must ensure that Bus Master is never zero so I am always checking the Bus Master bit , and set to 1 , when zero.

> 
> 
> >       Target = ((((Address >> 20) & 0xFF) << 24) |
> >                (((Address >> 15) & 0x1F) << 19) |
> >                (((Address >> 12) & 0x7) << 16));
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60191): https://edk2.groups.io/g/devel/message/60191
Mute This Topic: https://groups.io/mt/74395750/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-