Re: [edk2-devel] [edk2-platforms][PATCH v4 2/5] Silicon/ARM/N1SoC: Implement Neoverse N1 Soc specific PciExpressLib

["Thomas Abraham" <thomas.abraham@arm.com>]

Hi Pranav,

On Sun, Jul 19, 2020 at 2:19 PM Pranav Madhu <pranav.madhu@arm.com> wrote:
>
> From: Deepak Pandey <deepak.pandey@arm.com>
>
> A slave error is generated when host accesses the config space of
> non-available device or unimplemented function on a given bus. So
> implement a Neoverse N1 SoC specific PciExpressLib library with a
> workaround to return 0xffffffff for all such access.
>
> This library is inherited from MdePkg/Library/BasePciExpressLib and
> based on commit 9344f0921518 of that library in the tianocore/edk2
> project.
>
> In addition to this, the Neoverse N1 SoC has two other limitations which
> affect the access to the PCIe root port:
>   1. ECAM space is not contiguous, root port ECAM (BDF = 0:0:0) is
>      isolated from rest of the downstream hierarchy ECAM space.
>   2. Root port ECAM space is not capable of 8bit/16bit writes.
> This library includes workaround for these limitations as well.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Signed-off-by: Pranav Madhu <pranav.madhu@arm.com>
> ---
>  Silicon/ARM/NeoverseN1Soc/NeoverseN1Soc.dec                                    |    4 +
>  Silicon/ARM/NeoverseN1Soc/Library/NeoverseN1SocPciExpressLib/PciExpressLib.inf |   56 +
>  Silicon/ARM/NeoverseN1Soc/Library/NeoverseN1SocPciExpressLib/PciExpressLib.c   | 1589 ++++++++++++++++++++
>  3 files changed, 1649 insertions(+)
>

<...>

> +UINT8
> +EFIAPI
> +PciExpressWrite8 (
> +  IN      UINTN                     Address,
> +  IN      UINT8                     Value
> +  )
> +{
> +  UINT8 Bus, Device, Function;
> +  UINT8 Offset;
> +  UINT32 Data;
> +
> +  ASSERT_INVALID_PCI_ADDRESS (Address);
> +
> +  Bus = GET_BUS_NUM (Address);
> +  Device = GET_DEV_NUM (Address);
> +  Function = GET_FUNC_NUM (Address);
> +
> +  //
> +  // 8-bit and 16-bit writes to root port config space is not supported due to
> +  // a hardware limitation. As a workaround, perform a read-update-write
> +  // sequence on the whole 32-bit word of the root port config register such
> +  // that only the specified 8-bits of that word are updated.
> +  //
> +  if ((Bus == 0) && (Device == 0) && (Function == 0)) {
> +    Offset = Address & 0x3;
> +    Address &= 0xFFFFFFFC;
> +    Data = MmioRead32 ((UINTN)GetPciExpressAddress (Address));

nit: There should have been a space before the GetPciExpressAddress.
This is inconsistent with the rest of the file.

Thanks,
Thomas.

<...>