Re: [PATCH 3/5] EmbeddedPkg: implement generic platform PCI I/O driver

[Leif Lindholm <leif.lindholm@linaro.org>]

On Mon, Oct 31, 2016 at 06:13:08PM +0000, Ard Biesheuvel wrote:
> This implements support for platform PCI I/O devices, i.e, devices that
> are not on a PCI bus but that can be drived by generic PCI drivers in

drived->driven? (or substitute "controlled")

> EDK2.
> 
> This is implemented as a UEFI driver, which means we take full advantage
> of the UEFI driver model, and only instantiate those devices that are
> necessary for booting.
> 
> Care is taken to deal with DMA addressing limitations: DMA mappings and
> allocations are moved below 4 GB if the PCI driver has not informed us
> that the device being driven is 64-bit DMA capable.

How do we deal with these devices if there is no RAM below 4GB?
(Signalling an error and aborting is fine, but worth calling out even
here in the commit message.)

> 
> For now, this driver supports coherent DMA only, but support for
> non-coherent DMA is planned as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.c      | 649 ++++++++++++++++++++
>  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.h      |  67 ++
>  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.c   | 268 ++++++++
>  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf |  41 ++
>  EmbeddedPkg/EmbeddedPkg.dsc                               |   1 +
>  5 files changed, 1026 insertions(+)
> 
> diff --git a/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.c b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.c
> new file mode 100644
> index 000000000000..97ed19353347
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.c
> @@ -0,0 +1,649 @@
> +/** @file
> +
> +  Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> +  Copyright (c) 2016, Linaro, Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include "PlatformPciIo.h"
> +
> +#include <Protocol/PciRootBridgeIo.h>
> +
> +typedef struct {
> +  EFI_PHYSICAL_ADDRESS            AllocAddress;
> +  VOID                            *HostAddress;
> +  EFI_PCI_IO_PROTOCOL_OPERATION   Operation;
> +  UINTN                           NumberOfBytes;
> +} PLATFORM_PCI_IO_MAP_INFO;
> +
> +STATIC
> +EFI_STATUS
> +PciIoPollMem (
> +  IN  EFI_PCI_IO_PROTOCOL          *This,
> +  IN  EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> +  IN  UINT8                        BarIndex,
> +  IN  UINT64                       Offset,
> +  IN  UINT64                       Mask,
> +  IN  UINT64                       Value,
> +  IN  UINT64                       Delay,
> +  OUT UINT64                       *Result
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoPollIo (
> +  IN  EFI_PCI_IO_PROTOCOL          *This,
> +  IN  EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> +  IN  UINT8                        BarIndex,
> +  IN  UINT64                       Offset,
> +  IN  UINT64                       Mask,
> +  IN  UINT64                       Value,
> +  IN  UINT64                       Delay,
> +  OUT UINT64                       *Result
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoMemRW (
> +  IN    EFI_PCI_IO_PROTOCOL_WIDTH Width,
> +  IN    UINTN                     Count,
> +  IN    UINTN                     DstStride,
> +  IN    VOID                      *Dst,
> +  IN    UINTN                     SrcStride,
> +  OUT   CONST VOID                *Src
> +  )
> +{
> +  UINT8         *Dst8;
> +  UINT16        *Dst16;
> +  UINT32        *Dst32;
> +  CONST UINT8   *Src8;
> +  CONST UINT16  *Src16;
> +  CONST UINT32  *Src32;

Do any or all of these need to be volatile to ensure retaining access
order and size (and if not, could we call that out explicitly)?

> +
> +  //
> +  // Loop for each iteration and move the data
> +  //
> +  switch (Width & 0x3) {
> +  case EfiPciWidthUint8:
> +    Dst8 = (UINT8 *)Dst;
> +    Src8 = (UINT8 *)Src;
> +    for (;Count > 0; Count--, Dst8 += DstStride, Src8 += SrcStride) {
> +      *Dst8 = *Src8;
> +    }
> +    break;
> +  case EfiPciWidthUint16:
> +    Dst16 = (UINT16 *)Dst;
> +    Src16 = (UINT16 *)Src;
> +    for (;Count > 0; Count--, Dst16 += DstStride, Src16 += SrcStride) {
> +      *Dst16 = *Src16;
> +    }
> +    break;
> +  case EfiPciWidthUint32:
> +    Dst32 = (UINT32 *)Dst;
> +    Src32 = (UINT32 *)Src;
> +    for (;Count > 0; Count--, Dst32 += DstStride, Src32 += SrcStride) {
> +      *Dst32 = *Src32;
> +    }
> +    break;
> +  default:
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoMemRead (
> +  IN     EFI_PCI_IO_PROTOCOL          *This,
> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> +  IN     UINT8                        BarIndex,
> +  IN     UINT64                       Offset,
> +  IN     UINTN                        Count,
> +  IN OUT VOID                         *Buffer
> +  )
> +{
> +  PLATFORM_PCI_IO_DEV   *Dev;
> +  UINTN                 AlignMask;
> +  VOID                  *Address;
> +
> +  if (Buffer == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> +
> +  //
> +  // Only allow accesses to the single BAR we emulate
> +  //
> +  if (BarIndex != Dev->BarIndex || Offset >= Dev->BarSize) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  Address = (VOID*)(UINTN)(Dev->ConfigSpace.Device.Bar[BarIndex] + Offset);
> +  AlignMask = (1 << (Width & 0x03)) - 1;
> +  if ((UINTN)Address & AlignMask) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  switch (Width) {
> +  case EfiPciWidthUint8:
> +  case EfiPciWidthUint16:
> +  case EfiPciWidthUint32:
> +  case EfiPciWidthUint64:
> +    return PciIoMemRW (Width, Count, 1, Buffer, 1, Address);
> +
> +  case EfiPciWidthFifoUint8:
> +  case EfiPciWidthFifoUint16:
> +  case EfiPciWidthFifoUint32:
> +  case EfiPciWidthFifoUint64:
> +    return PciIoMemRW (Width, Count, 1, Buffer, 0, Address);
> +
> +  case EfiPciWidthFillUint8:
> +  case EfiPciWidthFillUint16:
> +  case EfiPciWidthFillUint32:
> +  case EfiPciWidthFillUint64:
> +    return PciIoMemRW (Width, Count, 0, Buffer, 1, Address);
> +
> +  default:
> +    break;
> +  }
> +  return EFI_INVALID_PARAMETER;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoMemWrite (
> +  IN     EFI_PCI_IO_PROTOCOL          *This,
> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> +  IN     UINT8                        BarIndex,
> +  IN     UINT64                       Offset,
> +  IN     UINTN                        Count,
> +  IN OUT VOID                         *Buffer
> +  )
> +{
> +  PLATFORM_PCI_IO_DEV   *Dev;
> +  UINTN                 AlignMask;
> +  VOID                  *Address;
> +
> +  if (Buffer == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> +
> +  //
> +  // Only allow accesses to the single BAR we emulate
> +  //
> +  if (BarIndex != Dev->BarIndex || Offset >= Dev->BarSize) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  Address = (VOID*)(UINTN)(Dev->ConfigSpace.Device.Bar[BarIndex] + Offset);
> +  AlignMask = (1 << (Width & 0x03)) - 1;
> +  if ((UINTN)Address & AlignMask) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  switch (Width) {
> +  case EfiPciWidthUint8:
> +  case EfiPciWidthUint16:
> +  case EfiPciWidthUint32:
> +  case EfiPciWidthUint64:
> +    return PciIoMemRW (Width, Count, 1, Address, 1, Buffer);
> +
> +  case EfiPciWidthFifoUint8:
> +  case EfiPciWidthFifoUint16:
> +  case EfiPciWidthFifoUint32:
> +  case EfiPciWidthFifoUint64:
> +    return PciIoMemRW (Width, Count, 0, Address, 1, Buffer);
> +
> +  case EfiPciWidthFillUint8:
> +  case EfiPciWidthFillUint16:
> +  case EfiPciWidthFillUint32:
> +  case EfiPciWidthFillUint64:
> +    return PciIoMemRW (Width, Count, 1, Address, 0, Buffer);
> +
> +  default:
> +    break;
> +  }
> +  return EFI_INVALID_PARAMETER;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoIoRead (
> +  IN EFI_PCI_IO_PROTOCOL              *This,
> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> +  IN     UINT8                        BarIndex,
> +  IN     UINT64                       Offset,
> +  IN     UINTN                        Count,
> +  IN OUT VOID                         *Buffer
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoIoWrite (
> +  IN EFI_PCI_IO_PROTOCOL              *This,
> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> +  IN     UINT8                        BarIndex,
> +  IN     UINT64                       Offset,
> +  IN     UINTN                        Count,
> +  IN OUT VOID                         *Buffer
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoPciRead (
> +  IN     EFI_PCI_IO_PROTOCOL       *This,
> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH  Width,
> +  IN     UINT32                     Offset,
> +  IN     UINTN                      Count,
> +  IN OUT VOID                      *Buffer
> +  )
> +{
> +  PLATFORM_PCI_IO_DEV *Dev;
> +  VOID                *Address;
> +
> +  if ((Width < 0) || (Width >= EfiPciIoWidthMaximum) || (Buffer == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> +
> +  Address = (UINT8 *)&Dev->ConfigSpace + Offset;
> +
> +  return PciIoMemRW (Width, Count, 1, Buffer, 1, Address);
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoPciWrite (
> +  IN EFI_PCI_IO_PROTOCOL              *This,
> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> +  IN     UINT32                       Offset,
> +  IN     UINTN                        Count,
> +  IN OUT VOID                         *Buffer
> +  )
> +{
> +  PLATFORM_PCI_IO_DEV *Dev;
> +  VOID                *Address;
> +
> +  if ((Width < 0) || (Width >= EfiPciIoWidthMaximum) || (Buffer == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> +
> +  Address = (UINT8 *)&Dev->ConfigSpace + Offset;
> +
> +  return PciIoMemRW (Width, Count, 1, Address, 1, Buffer);
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoCopyMem (
> +  IN EFI_PCI_IO_PROTOCOL              *This,
> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> +  IN     UINT8                        DestBarIndex,
> +  IN     UINT64                       DestOffset,
> +  IN     UINT8                        SrcBarIndex,
> +  IN     UINT64                       SrcOffset,
> +  IN     UINTN                        Count
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_STATUS
> +CoherentPciIoMap (
> +  IN     EFI_PCI_IO_PROTOCOL            *This,
> +  IN     EFI_PCI_IO_PROTOCOL_OPERATION  Operation,
> +  IN     VOID                           *HostAddress,
> +  IN OUT UINTN                          *NumberOfBytes,
> +  OUT    EFI_PHYSICAL_ADDRESS           *DeviceAddress,
> +  OUT    VOID                           **Mapping
> +  )
> +{
> +  PLATFORM_PCI_IO_DEV       *Dev;
> +  EFI_STATUS                Status;
> +  PLATFORM_PCI_IO_MAP_INFO  *MapInfo;
> +
> +  //
> +  // If this device does not support 64-bit DMA addressing, we need to allocate
> +  // a bounce buffer and copy over the data if HostAddress is above 4 GB.
> +  //
> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> +  if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0 &&
> +      (UINTN) HostAddress >= SIZE_4GB) {
> +
> +    //
> +    // Bounce buffering is not possible for consistent mappings
> +    //
> +    if (Operation == EfiPciIoOperationBusMasterCommonBuffer) {
> +      return EFI_UNSUPPORTED;
> +    }
> +
> +    MapInfo = (PLATFORM_PCI_IO_MAP_INFO *)AllocatePool (sizeof *MapInfo);
> +    if (MapInfo == NULL) {
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    MapInfo->AllocAddress = SIZE_4GB - 1;
> +    MapInfo->HostAddress = HostAddress;
> +    MapInfo->Operation = Operation;
> +    MapInfo->NumberOfBytes = *NumberOfBytes;
> +
> +    Status = gBS->AllocatePages (AllocateMaxAddress, EfiBootServicesData,
> +                    EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes),
> +                    &MapInfo->AllocAddress);
> +    if (EFI_ERROR (Status)) {

Comment on how this can mean the system does not have RAM < 4GB and
cannot support this device?

> +      FreePool (MapInfo);
> +      return Status;
> +    }
> +    if (Operation == EfiPciIoOperationBusMasterRead) {
> +      gBS->CopyMem ((VOID *)MapInfo->AllocAddress, HostAddress, *NumberOfBytes);
> +    }
> +    *DeviceAddress = MapInfo->AllocAddress;
> +    *Mapping = MapInfo;
> +  } else {
> +    *DeviceAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress;
> +    *Mapping = NULL;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +CoherentPciIoUnmap (
> +  IN  EFI_PCI_IO_PROTOCOL          *This,
> +  IN  VOID                         *Mapping
> +  )
> +{
> +  PLATFORM_PCI_IO_MAP_INFO  *MapInfo;
> +
> +  MapInfo = Mapping;
> +  if (MapInfo != NULL) {
> +    if (MapInfo->Operation == EfiPciIoOperationBusMasterWrite) {
> +      gBS->CopyMem (MapInfo->HostAddress, (VOID *)MapInfo->AllocAddress,
> +             MapInfo->NumberOfBytes);
> +    }
> +    gBS->FreePages (MapInfo->AllocAddress,
> +           EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes));
> +    FreePool (MapInfo);
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +CoherentPciIoAllocateBuffer (
> +  IN  EFI_PCI_IO_PROTOCOL         *This,
> +  IN  EFI_ALLOCATE_TYPE           Type,
> +  IN  EFI_MEMORY_TYPE             MemoryType,
> +  IN  UINTN                       Pages,
> +  OUT VOID                        **HostAddress,
> +  IN  UINT64                      Attributes
> +  )
> +{
> +  PLATFORM_PCI_IO_DEV       *Dev;
> +  EFI_PHYSICAL_ADDRESS      AllocAddress;
> +  EFI_ALLOCATE_TYPE         AllocType;
> +  EFI_STATUS                Status;
> +
> +  if (Attributes &
> +      (~(EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE |
> +         EFI_PCI_ATTRIBUTE_MEMORY_CACHED         ))) {

Is that indentation before ))) intentional?

> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  //
> +  // Allocate below 4 GB if the dual address cycle attribute has not
> +  // been set.

Comment on incompatibility if no RAM below 4 GB?

> +  //
> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> +  if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
> +    AllocAddress = SIZE_4GB - 1;
> +    AllocType = AllocateMaxAddress;
> +  } else {
> +    AllocType = AllocateAnyPages;
> +  }
> +
> +  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &AllocAddress);
> +  if (!EFI_ERROR (Status)) {
> +    *HostAddress = (VOID *)(UINTN)AllocAddress;
> +  }
> +  return Status;
> +}
> +
> +STATIC
> +EFI_STATUS
> +CoherentPciIoFreeBuffer (
> +  IN  EFI_PCI_IO_PROTOCOL         *This,
> +  IN  UINTN                       Pages,
> +  IN  VOID                        *HostAddress
> +  )
> +{
> +  FreePages (HostAddress, Pages);
> +  return EFI_SUCCESS;
> +}
> +
> +
> +STATIC
> +EFI_STATUS
> +PciIoFlush (
> +  IN EFI_PCI_IO_PROTOCOL          *This
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoGetLocation (
> +  IN   EFI_PCI_IO_PROTOCOL  *This,
> +  OUT  UINTN                *SegmentNumber,
> +  OUT  UINTN                *BusNumber,
> +  OUT  UINTN                *DeviceNumber,
> +  OUT  UINTN                *FunctionNumber
> +  )
> +{
> +  if ((SegmentNumber == NULL) || (BusNumber      == NULL) ||
> +      (DeviceNumber  == NULL) || (FunctionNumber == NULL)    ) {
> +    return EFI_INVALID_PARAMETER;

Indentation before ) intentional?

> +  }
> +
> +  *SegmentNumber  = 0;
> +  *BusNumber      = 0xff;
> +  *DeviceNumber   = 0;
> +  *FunctionNumber = 0;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoAttributes (
> +  IN  EFI_PCI_IO_PROTOCOL                      *This,
> +  IN  EFI_PCI_IO_PROTOCOL_ATTRIBUTE_OPERATION  Operation,
> +  IN  UINT64                                   Attributes,
> +  OUT UINT64                                   *Result OPTIONAL
> +  )
> +{
> +  PLATFORM_PCI_IO_DEV *Dev;
> +  BOOLEAN             Enable;
> +
> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> +
> +  Enable = FALSE;
> +  switch (Operation) {
> +  case EfiPciIoAttributeOperationGet:
> +    if (Result == NULL) {
> +      return EFI_INVALID_PARAMETER;
> +    }
> +    *Result = Dev->Attributes;
> +    break;
> +
> +  case EfiPciIoAttributeOperationSupported:
> +    if (Result == NULL) {
> +      return EFI_INVALID_PARAMETER;
> +    }
> +    *Result = EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
> +    break;
> +
> +  case EfiPciIoAttributeOperationEnable:
> +    Attributes |= Dev->Attributes;
> +  case EfiPciIoAttributeOperationSet:
> +    Enable = ((~Dev->Attributes & Attributes) & EFI_PCI_DEVICE_ENABLE) != 0;
> +    Dev->Attributes = Attributes;
> +    break;
> +
> +  case EfiPciIoAttributeOperationDisable:
> +    Dev->Attributes &= ~Attributes;
> +    break;
> +
> +  default:
> +    return EFI_INVALID_PARAMETER;
> +  };
> +
> +  //
> +  // If we're setting any of the EFI_PCI_DEVICE_ENABLE bits, perform
> +  // the platform device specific initialization now.
> +  //
> +  if (Enable && !Dev->Enabled && Dev->PlatformPciIo->Initialize != NULL) {
> +    Dev->PlatformPciIo->Initialize (Dev->PlatformPciIo);
> +    Dev->Enabled = TRUE;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoGetBarAttributes (
> +  IN EFI_PCI_IO_PROTOCOL             *This,
> +  IN  UINT8                          BarIndex,
> +  OUT UINT64                         *Supports, OPTIONAL
> +  OUT VOID                           **Resources OPTIONAL
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_STATUS
> +PciIoSetBarAttributes (
> +  IN EFI_PCI_IO_PROTOCOL              *This,
> +  IN     UINT64                       Attributes,
> +  IN     UINT8                        BarIndex,
> +  IN OUT UINT64                       *Offset,
> +  IN OUT UINT64                       *Length
> +  )
> +{
> +  ASSERT (FALSE);
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC CONST EFI_PCI_IO_PROTOCOL PciIoTemplate =
> +{
> +  PciIoPollMem,
> +  PciIoPollIo,
> +  { PciIoMemRead, PciIoMemWrite },
> +  { PciIoIoRead,  PciIoIoWrite },
> +  { PciIoPciRead, PciIoPciWrite },
> +  PciIoCopyMem,
> +  CoherentPciIoMap,
> +  CoherentPciIoUnmap,
> +  CoherentPciIoAllocateBuffer,
> +  CoherentPciIoFreeBuffer,
> +  PciIoFlush,
> +  PciIoGetLocation,
> +  PciIoAttributes,
> +  PciIoGetBarAttributes,
> +  PciIoSetBarAttributes,
> +  0,
> +  0
> +};
> +
> +VOID
> +InitializePciIoProtocol (
> +  PLATFORM_PCI_IO_DEV     *PlatformPciIoDev
> +  )
> +{
> +  PlatformPciIoDev->ConfigSpace.Hdr.VendorId = 0xFFFF;    // no vendor
> +  PlatformPciIoDev->ConfigSpace.Hdr.DeviceId = 0x0000;    // device id ignored

Are these IDs noncontroversial?
Would it be preferable to allocate some real ones?

> +
> +  switch (PlatformPciIoDev->PlatformPciIo->DeviceType) {
> +  case PlatformPciIoDeviceOhci:
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_OHCI;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL;
> +    PlatformPciIoDev->BarSize = SIZE_64KB;

Is the 64KB mainly for SBSA compliance, or is it implicit for OHCI?

> +    break;
> +
> +  case PlatformPciIoDeviceUhci:
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_UHCI;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL;
> +    PlatformPciIoDev->BarSize = SIZE_64KB;
> +    break;
> +
> +  case PlatformPciIoDeviceEhci:
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_EHCI;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL;
> +    PlatformPciIoDev->BarSize = SIZE_64KB;
> +    break;
> +
> +  case PlatformPciIoDeviceXhci:
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_XHCI;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL;
> +    PlatformPciIoDev->BarIndex = 0;
> +    PlatformPciIoDev->BarSize = SIZE_64KB;
> +    break;
> +
> +  case PlatformPciIoDeviceAhci:
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_MASS_STORAGE_AHCI;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_MASS_STORAGE_SATADPA;
> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_MASS_STORAGE;
> +    PlatformPciIoDev->BarIndex = 5;
> +    PlatformPciIoDev->BarSize = SIZE_1KB;
> +    break;
> +

And as a follow-on to comment on previous patch: SdMmc and Ufs?

> +  default:
> +    ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> +  }
> +
> +  PlatformPciIoDev->ConfigSpace.Device.Bar[PlatformPciIoDev->BarIndex] =
> +                                        PlatformPciIoDev->PlatformPciIo->BaseAddress;
> +
> +  // Copy protocol structure
> +  CopyMem(&PlatformPciIoDev->PciIo, &PciIoTemplate, sizeof PciIoTemplate);
> +}
> diff --git a/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.h b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.h
> new file mode 100644
> index 000000000000..8fd8dc5e4a11
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.h
> @@ -0,0 +1,67 @@
> +/** @file
> +
> +  Copyright (C) 2016, Linaro Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include <IndustryStandard/Pci.h>
> +
> +#include <Protocol/PciIo.h>
> +#include <Protocol/PlatformPciIo.h>
> +
> +#define PLATFORM_PCI_IO_SIG SIGNATURE_32 ('P', 'P', 'I', 'D')
> +
> +#define PLATFORM_PCI_IO_DEV_FROM_PCI_IO(PciIoPointer) \
> +          CR (PciIoPointer, PLATFORM_PCI_IO_DEV, PciIo, PLATFORM_PCI_IO_SIG)
> +
> +typedef struct {
> +  UINT32                  Signature;
> +  //
> +  // The bound platform PCI I/O protocol instance
> +  //
> +  PLATFORM_PCI_IO         *PlatformPciIo;
> +  //
> +  // The exposed PCI I/O protocol instance.
> +  //
> +  EFI_PCI_IO_PROTOCOL     PciIo;
> +  //
> +  // The emulated PCI config space of the device. Only the minimally required
> +  // items are assigned.
> +  //
> +  PCI_TYPE00              ConfigSpace;
> +  //
> +  // The BAR index which exposes the MMIO control region of the device
> +  //
> +  UINTN                   BarIndex;
> +  //
> +  // The size of the MMIO control region of the device
> +  //
> +  UINTN                   BarSize;
> +  //
> +  // The PCI I/O attributes for this device
> +  //
> +  UINT64                  Attributes;
> +  //
> +  // Whether this device has been enabled
> +  //
> +  BOOLEAN                 Enabled;
> +} PLATFORM_PCI_IO_DEV;
> +
> +VOID
> +InitializePciIoProtocol (
> +  PLATFORM_PCI_IO_DEV     *PlatformPciIoDev
> +  );
> diff --git a/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.c b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.c
> new file mode 100644
> index 000000000000..7f3306e7e891
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.c
> @@ -0,0 +1,268 @@
> +/** @file
> +
> +  Copyright (C) 2016, Linaro Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include "PlatformPciIo.h"
> +
> +#include <Protocol/ComponentName.h>
> +#include <Protocol/DriverBinding.h>
> +
> +//
> +// Probe, start and stop functions of this driver, called by the DXE core for
> +// specific devices.
> +//
> +// The following specifications document these interfaces:
> +// - Driver Writer's Guide for UEFI 2.3.1 v1.01, 9 Driver Binding Protocol
> +// - UEFI Spec 2.3.1 + Errata C, 10.1 EFI Driver Binding Protocol
> +//
> +// The implementation follows:
> +// - Driver Writer's Guide for UEFI 2.3.1 v1.01
> +//   - 5.1.3.4 OpenProtocol() and CloseProtocol()
> +// - UEFI Spec 2.3.1 + Errata C
> +//   -  6.3 Protocol Handler Services
> +//
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +PlatformPciIoDriverBindingSupported (
> +  IN EFI_DRIVER_BINDING_PROTOCOL *This,
> +  IN EFI_HANDLE                  DeviceHandle,
> +  IN EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath
> +  )
> +{
> +  PLATFORM_PCI_IO       *PlatformPciIo;
> +  EFI_STATUS            Status;
> +
> +  Status = gBS->OpenProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> +                  (VOID **)&PlatformPciIo, This->DriverBindingHandle,
> +                  DeviceHandle, EFI_OPEN_PROTOCOL_BY_DRIVER);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // We only support the following device types
> +  //
> +  switch (PlatformPciIo->DeviceType) {
> +  case PlatformPciIoDeviceOhci:
> +  case PlatformPciIoDeviceUhci:
> +  case PlatformPciIoDeviceEhci:
> +  case PlatformPciIoDeviceXhci:
> +  case PlatformPciIoDeviceAhci:

SdMmc, Ufs?

> +    //
> +    // Restricted to DMA coherent for now
> +    //
> +    if (PlatformPciIo->DmaType == PlatformPciIoDmaCoherent) {
> +      Status = EFI_SUCCESS;
> +      break;
> +    }
> +  default:
> +    Status = EFI_UNSUPPORTED;
> +  }
> +
> +  gBS->CloseProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> +         This->DriverBindingHandle, DeviceHandle);
> +
> +  return Status;
> +}
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +PlatformPciIoDriverBindingStart (
> +  IN EFI_DRIVER_BINDING_PROTOCOL *This,
> +  IN EFI_HANDLE                  DeviceHandle,
> +  IN EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath
> +  )
> +{
> +  PLATFORM_PCI_IO_DEV   *Dev;
> +  EFI_STATUS            Status;
> +
> +  Dev = (PLATFORM_PCI_IO_DEV *) AllocateZeroPool (sizeof *Dev);

You usually don't put a space after the cast.

> +  if (Dev == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  Status = gBS->OpenProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> +                  (VOID **)&Dev->PlatformPciIo, This->DriverBindingHandle,
> +                  DeviceHandle, EFI_OPEN_PROTOCOL_BY_DRIVER);
> +  if (EFI_ERROR (Status)) {
> +    goto FreeDev;
> +  }
> +
> +  InitializePciIoProtocol (Dev);
> +
> +  //
> +  // Setup complete, attempt to export the driver instance's EFI_PCI_IO_PROTOCOL
> +  // interface.
> +  //
> +  Dev->Signature = PLATFORM_PCI_IO_SIG;
> +  Status = gBS->InstallProtocolInterface (&DeviceHandle, &gEfiPciIoProtocolGuid,
> +                  EFI_NATIVE_INTERFACE, &Dev->PciIo);
> +  if (EFI_ERROR (Status)) {
> +    goto CloseProtocol;
> +  }
> +
> +  return EFI_SUCCESS;
> +
> +CloseProtocol:
> +  gBS->CloseProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> +         This->DriverBindingHandle, DeviceHandle);
> +
> +FreeDev:
> +  FreePool (Dev);
> +
> +  return Status;
> +}
> +
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +PlatformPciIoDriverBindingStop (
> +  IN EFI_DRIVER_BINDING_PROTOCOL *This,
> +  IN EFI_HANDLE                  DeviceHandle,
> +  IN UINTN                       NumberOfChildren,
> +  IN EFI_HANDLE                  *ChildHandleBuffer
> +  )
> +{
> +  EFI_STATUS                      Status;
> +  EFI_PCI_IO_PROTOCOL             *PciIo;
> +  PLATFORM_PCI_IO_DEV             *Dev;
> +
> +  Status = gBS->OpenProtocol (DeviceHandle, &gEfiPciIoProtocolGuid,
> +                  (VOID **)&PciIo, This->DriverBindingHandle, DeviceHandle,
> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO (PciIo);
> +
> +  //
> +  // Handle Stop() requests for in-use driver instances gracefully.
> +  //
> +  Status = gBS->UninstallProtocolInterface (DeviceHandle,
> +                  &gEfiPciIoProtocolGuid, &Dev->PciIo);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  gBS->CloseProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> +         This->DriverBindingHandle, DeviceHandle);
> +
> +  FreePool (Dev);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +
> +//
> +// The static object that groups the Supported() (ie. probe), Start() and
> +// Stop() functions of the driver together. Refer to UEFI Spec 2.3.1 + Errata
> +// C, 10.1 EFI Driver Binding Protocol.
> +//
> +STATIC EFI_DRIVER_BINDING_PROTOCOL gDriverBinding = {
> +  &PlatformPciIoDriverBindingSupported,
> +  &PlatformPciIoDriverBindingStart,
> +  &PlatformPciIoDriverBindingStop,
> +  0x10, // Version, must be in [0x10 .. 0xFFFFFFEF] for IHV-developed drivers
> +  NULL,
> +  NULL
> +};
> +
> +
> +//
> +// The purpose of the following scaffolding (EFI_COMPONENT_NAME_PROTOCOL and
> +// EFI_COMPONENT_NAME2_PROTOCOL implementation) is to format the driver's name
> +// in English, for display on standard console devices. This is recommended for
> +// UEFI drivers that follow the UEFI Driver Model. Refer to the Driver Writer's
> +// Guide for UEFI 2.3.1 v1.01, 11 UEFI Driver and Controller Names.
> +//

Should the strings be split out in order to permit proper unicode
strings without polluting the C file? As this will be a core library,
I would quite like to see translations to all kinds of languages.

> +
> +STATIC
> +EFI_UNICODE_STRING_TABLE mDriverNameTable[] = {
> +  { "eng;en", L"PCI I/O protocol emulation driver for platform devices" },
> +  { NULL,     NULL                   }
> +};
> +
> +STATIC
> +EFI_COMPONENT_NAME_PROTOCOL gComponentName;
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +PlatformPciIoGetDriverName (
> +  IN  EFI_COMPONENT_NAME_PROTOCOL *This,
> +  IN  CHAR8                       *Language,
> +  OUT CHAR16                      **DriverName
> +  )
> +{
> +  return LookupUnicodeString2 (
> +           Language,
> +           This->SupportedLanguages,
> +           mDriverNameTable,
> +           DriverName,
> +           (BOOLEAN)(This == &gComponentName) // Iso639Language
> +           );
> +}
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +PlatformPciIoGetDeviceName (
> +  IN  EFI_COMPONENT_NAME_PROTOCOL *This,
> +  IN  EFI_HANDLE                  DeviceHandle,
> +  IN  EFI_HANDLE                  ChildHandle,
> +  IN  CHAR8                       *Language,
> +  OUT CHAR16                      **ControllerName
> +  )
> +{
> +  return EFI_UNSUPPORTED;
> +}
> +
> +STATIC
> +EFI_COMPONENT_NAME_PROTOCOL gComponentName = {
> +  &PlatformPciIoGetDriverName,
> +  &PlatformPciIoGetDeviceName,
> +  "eng" // SupportedLanguages, ISO 639-2 language codes
> +};
> +
> +STATIC
> +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2 = {
> +  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME)     &PlatformPciIoGetDriverName,
> +  (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME) &PlatformPciIoGetDeviceName,
> +  "en" // SupportedLanguages, RFC 4646 language codes
> +};
> +
> +
> +//
> +// Entry point of this driver.
> +//
> +EFI_STATUS
> +EFIAPI
> +PlatformPciIoDxeEntryPoint (
> +  IN EFI_HANDLE       ImageHandle,
> +  IN EFI_SYSTEM_TABLE *SystemTable
> +  )
> +{
> +  return EfiLibInstallDriverBindingComponentName2 (
> +           ImageHandle,
> +           SystemTable,
> +           &gDriverBinding,
> +           ImageHandle,
> +           &gComponentName,
> +           &gComponentName2
> +           );
> +}
> diff --git a/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf
> new file mode 100644
> index 000000000000..2b0baf06732c
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf
> @@ -0,0 +1,41 @@
> +## @file
> +# Copyright (C) 2016, Linaro Ltd.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010017

0019?

> +  BASE_NAME                      = PlatformPciIoDxe
> +  FILE_GUID                      = 71fd84cd-353b-464d-b7a4-6ea7b96995cb
> +  MODULE_TYPE                    = UEFI_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = PlatformPciIoDxeEntryPoint
> +
> +[Sources]
> +  PlatformPciIoDxe.c
> +  PlatformPciIo.c
> +  PlatformPciIo.h
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  DebugLib
> +  MemoryAllocationLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UefiLib
> +
> +[Protocols]
> +  gEfiPciIoProtocolGuid           ## BY_START
> +  gPlatformPciIoProtocolGuid      ## TO_START
> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> index d47c836379c9..47f9d47eb378 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> @@ -291,6 +291,7 @@ [Components.common]
>    EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
>  
>    EmbeddedPkg/Library/PlatformPciIoDeviceRegistrationLib/PlatformPciIoDeviceRegistrationLib.inf
> +  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf
>  
>  [Components.IA32, Components.X64, Components.IPF, Components.ARM]
>    EmbeddedPkg/GdbStub/GdbStub.inf
> -- 
> 2.7.4
>