On Wed, Nov 02, 2016 at 01:39:26PM +0000, Ard Biesheuvel wrote: > On 1 November 2016 at 22:22, Leif Lindholm <leif.lindh...@linaro.org> wrote: > > 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") > > > > That's a typo, not a speako. But yes, let me clean that up > > >> 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.) > > > > The same thing the normal PCI I/O implementations do: return an error. > If you plug in a EHCI controller on Seattle that does not support > 64-bit DMA, calls to AllocateBuffer() and Map() will fail in exactly > the same way.
OK - if it's that straightforward, please ignore all of my other comments on that topic. > >> > >> 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.biesheu...@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)? > > > > Good question. But perhaps it is better to put a MemoryFence() at the > end of each loop iteration? That wouldn't prevent repeated reads from the same location, or reads done as a larger element. It would make either very unlikely, but it wouldn't prevent. > >> + >> + // > >> + // 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? > > > > Sure > > >> + 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? > > > > That was simply copied from the Omap35xxPkg PciEmulation code. I will > clean that up > > > >> + 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? > > > > Ack > > >> + // > >> + 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? > > > > Same here > > >> + } > >> + > >> + *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? > > > > They are unallocated, and unpopulated PCI config space reads back as > all zeroes, so 0xFFFF is the least likely to ever become allocated. > Device ID is scoped by vendor ID, so it's a don't care in any case. > > On top of that, the whole point of this driver is to match on class > codes, so we are by definition not interested in attaching drivers > that look for a particular Vendor/Device ID I don't disagree with any of what you're saying - I'd just like some input from someone who feels competent to comment. If 0xFFFF is acceptable to everyone, it coud make sense to have a PCI_VENDOR_NONE define. > >> + > >> + 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? > > > > Copy/paste from Juno. No clue tbh Ah :) > >> + 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? > > > > Indeed. Added in v2 Thanks! > > >> + 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? > > > > Yeah yeah > > >> + // > >> + // 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. > > > > Ack > > >> + 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. > > > > Do you mean a separate ComponentName.c like other drivers? Yeah. Regards, Leif > >> + > >> +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 > >> > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
