From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22e.google.com (mail-it0-x22e.google.com [IPv6:2607:f8b0:4001:c0b::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id F24D681C4D for ; Wed, 2 Nov 2016 06:40:15 -0700 (PDT) Received: by mail-it0-x22e.google.com with SMTP id m138so82946552itm.1 for ; Wed, 02 Nov 2016 06:40:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=GZR3lmOTInzesNlgOw6OGuyrXHzYcgiN8J7cgDpbtWs=; b=g027/Jqrw7Ngro0dg3SREYndgawymOpHFzzRigoYwH1Ol9oBd+wgANf2tXMr2183bY 41k2Eu8i9wcFK55oG6qjs1yP6i23dcNIlYwwhq4A3bjRdARZEHiJrcmFyQmWNmkpCHSW Zuo3YwrPQ+KSt2pJIWthtupDstuk7THTvi20E= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=GZR3lmOTInzesNlgOw6OGuyrXHzYcgiN8J7cgDpbtWs=; b=nNfDwyKDAljMuKYbXnD7h0iC3YyLH3Dtv4JWMByAGUfO49//RAw495WO5u79j8ek+X K20M/X4Si2hjxPJVJPO1ahEEnbSotBv5nq5eogmWUeLnTwoYhGrgtFhr7FklqBr6iUzG QorXAWFxWK6De5wAmAbb7p4eGLxgP7nCO7D7EfuGgXgipGsQurjsMQ2h9dMIaoiSymYL Lx/g5r3XLpO7GUr6cloqlSqCACgxxEn6giDSr8vT+aBL83ZOtdC/4RbKmkEF7HhJEwt9 AKcXKAHNTBjgtLnqp0HLUFX11CT9dmH6mf933BMpbr35pfFc0C7DSSW4B3LYbNEnZ/9O qelg== X-Gm-Message-State: ABUngvfpqiBjVONrM+atdC0+QZyN3FHkfPBe6uiYCnSWOyQavo/pymTbNc/A9jUvHE4zrvJR098JdneP7+y2gT6F X-Received: by 10.107.2.8 with SMTP id 8mr948267ioc.83.1478093967704; Wed, 02 Nov 2016 06:39:27 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.59.147 with HTTP; Wed, 2 Nov 2016 06:39:26 -0700 (PDT) In-Reply-To: <20161101222201.GO1161@bivouac.eciton.net> References: <1477937590-10361-1-git-send-email-ard.biesheuvel@linaro.org> <1477937590-10361-4-git-send-email-ard.biesheuvel@linaro.org> <20161101222201.GO1161@bivouac.eciton.net> From: Ard Biesheuvel Date: Wed, 2 Nov 2016 13:39:26 +0000 Message-ID: To: Leif Lindholm Cc: edk2-devel-01 Subject: Re: [PATCH 3/5] EmbeddedPkg: implement generic platform PCI I/O driver X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Nov 2016 13:40:16 -0000 Content-Type: text/plain; charset=UTF-8 On 1 November 2016 at 22:22, Leif Lindholm 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. >> >> 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 >> --- >> 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.
>> + Copyright (c) 2016, Linaro, Ltd. All rights reserved.
>> + >> + 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 >> + >> +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? >> + >> + // >> + // 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 >> + >> + 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 >> + 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 >> + 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.
>> + >> + 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 >> +#include >> +#include >> +#include >> +#include >> + >> +#include >> + >> +#include >> +#include >> + >> +#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.
>> + >> + 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 >> +#include >> + >> +// >> +// 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? >> + >> +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 >>