| Message ID | 1480088538-21834-4-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Ard, I found you removed the SDHCI special handling code. Only one minor comments in below. (you could search RShiftU64 in below). Reviewed-by: Ruiyu Ni <Ruiyu.ni@intel.com> Thanks/Ray > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Friday, November 25, 2016 11:42 PM > To: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Gao, Liming > <liming.gao@intel.com>; afish@apple.com; Ni, Ruiyu <ruiyu.ni@intel.com>; > Kinney, Michael D <michael.d.kinney@intel.com>; mw@semihalf.com; Tian, > Feng <feng.tian@intel.com> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Subject: [PATCH v4 3/5] MdeModulePkg: implement generic PCI I/O driver > for non-discoverable devices > > This implements support for non-discoverable PCI compatible devices, i.e, > devices that are not on a PCI bus but that can be controlled by generic PCI > drivers in 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. DMA is implemented as > coherent, support for non-coherent DMA is implemented by a subsequent > patch. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c > | 75 ++ > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > DeviceDxe.c | 235 ++++++ > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > DeviceDxe.inf | 52 ++ > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > DeviceIo.c | 797 ++++++++++++++++++++ > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > DeviceIo.h | 84 +++ > MdeModulePkg/MdeModulePkg.dsc | 1 + > 6 files changed, 1244 insertions(+) > > diff --git > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentNam > e.c > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentNam > e.c > new file mode 100644 > index 000000000000..6e51d00fe434 > --- /dev/null > +++ > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentNam > e.c > @@ -0,0 +1,75 @@ > +/** @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 "NonDiscoverablePciDeviceIo.h" > + > +// > +// 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. > +// > + > +STATIC > +EFI_UNICODE_STRING_TABLE mDriverNameTable[] = { > + { "eng;en", L"PCI I/O protocol emulation driver for non-discoverable > devices" }, > + { NULL, NULL } > +}; > + > +EFI_COMPONENT_NAME_PROTOCOL gComponentName; > + > +STATIC > +EFI_STATUS > +EFIAPI > +NonDiscoverablePciGetDriverName ( > + 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 > +NonDiscoverablePciGetDeviceName ( > + IN EFI_COMPONENT_NAME_PROTOCOL *This, > + IN EFI_HANDLE DeviceHandle, > + IN EFI_HANDLE ChildHandle, > + IN CHAR8 *Language, > + OUT CHAR16 **ControllerName > + ) > +{ > + return EFI_UNSUPPORTED; > +} > + > +EFI_COMPONENT_NAME_PROTOCOL gComponentName = { > + &NonDiscoverablePciGetDriverName, > + &NonDiscoverablePciGetDeviceName, > + "eng" // SupportedLanguages, ISO 639-2 language codes }; > + > +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2 = { > + (EFI_COMPONENT_NAME2_GET_DRIVER_NAME) > &NonDiscoverablePciGetDriverName, > + (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME) > +&NonDiscoverablePciGetDeviceName, > + "en" // SupportedLanguages, RFC 4646 language codes }; > diff --git > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > PciDeviceDxe.c > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > PciDeviceDxe.c > new file mode 100644 > index 000000000000..ee765d7a5d9c > --- /dev/null > +++ > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > Pc > +++ iDeviceDxe.c > @@ -0,0 +1,235 @@ > +/** @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 "NonDiscoverablePciDeviceIo.h" > + > +#include <Protocol/DriverBinding.h> > + > +// > +// We only support the following device types // STATIC CONST EFI_GUID > +* CONST SupportedNonDiscoverableDevices [] = { > + &gEdkiiNonDiscoverableAhciDeviceGuid, > + &gEdkiiNonDiscoverableEhciDeviceGuid, > + &gEdkiiNonDiscoverableNvmeDeviceGuid, > + &gEdkiiNonDiscoverableOhciDeviceGuid, > + &gEdkiiNonDiscoverableSdhciDeviceGuid, > + &gEdkiiNonDiscoverableUfsDeviceGuid, > + &gEdkiiNonDiscoverableUhciDeviceGuid, > + &gEdkiiNonDiscoverableXhciDeviceGuid, > +}; > + > +// > +// 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 > +NonDiscoverablePciDeviceSupported ( > + IN EFI_DRIVER_BINDING_PROTOCOL *This, > + IN EFI_HANDLE DeviceHandle, > + IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath > + ) > +{ > + NON_DISCOVERABLE_DEVICE *Device; > + EFI_STATUS Status; > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > + INTN Idx; > + > + Status = gBS->OpenProtocol (DeviceHandle, > + &gEdkiiNonDiscoverableDeviceProtocolGuid, (VOID **)&Device, > + This->DriverBindingHandle, DeviceHandle, > + EFI_OPEN_PROTOCOL_BY_DRIVER); if (EFI_ERROR > + (Status)) { > + return Status; > + } > + > + // > + // Restricted to DMA coherent for now // Status = EFI_UNSUPPORTED; > + if (Device->DmaType != NonDiscoverableDeviceDmaTypeCoherent) { > + goto CloseProtocol; > + } > + > + for (Idx = 0; Idx < ARRAY_SIZE (SupportedNonDiscoverableDevices); Idx++) > { > + if (CompareGuid (Device->Type, SupportedNonDiscoverableDevices > [Idx])) { > + Status = EFI_SUCCESS; > + break; > + } > + } > + > + if (EFI_ERROR (Status)) { > + goto CloseProtocol; > + } > + > + // > + // We only support MMIO devices, so iterate over the resources to > + ensure // that they only describe things that we can handle // for > + (Desc = Device->Resources; Desc->Desc != ACPI_END_TAG_DESCRIPTOR; > + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { > + if (Desc->Desc != ACPI_ADDRESS_SPACE_DESCRIPTOR || > + Desc->ResType != ACPI_ADDRESS_SPACE_TYPE_MEM) { > + Status = EFI_UNSUPPORTED; > + break; > + } > + } > + > +CloseProtocol: > + gBS->CloseProtocol (DeviceHandle, > &gEdkiiNonDiscoverableDeviceProtocolGuid, > + This->DriverBindingHandle, DeviceHandle); > + > + return Status; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +NonDiscoverablePciDeviceStart ( > + IN EFI_DRIVER_BINDING_PROTOCOL *This, > + IN EFI_HANDLE DeviceHandle, > + IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + EFI_STATUS Status; > + > + Dev = AllocateZeroPool (sizeof *Dev); if (Dev == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + Status = gBS->OpenProtocol (DeviceHandle, > + &gEdkiiNonDiscoverableDeviceProtocolGuid, > + (VOID **)&Dev->Device, 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 = NON_DISCOVERABLE_PCI_DEVICE_SIG; Status = > + gBS->InstallProtocolInterface (&DeviceHandle, &gEfiPciIoProtocolGuid, > + EFI_NATIVE_INTERFACE, &Dev->PciIo); if (EFI_ERROR > + (Status)) { > + goto CloseProtocol; > + } > + > + return EFI_SUCCESS; > + > +CloseProtocol: > + gBS->CloseProtocol (DeviceHandle, > &gEdkiiNonDiscoverableDeviceProtocolGuid, > + This->DriverBindingHandle, DeviceHandle); > + > +FreeDev: > + FreePool (Dev); > + > + return Status; > +} > + > + > +STATIC > +EFI_STATUS > +EFIAPI > +NonDiscoverablePciDeviceStop ( > + 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; > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + > + Status = gBS->OpenProtocol (DeviceHandle, &gEfiPciIoProtocolGuid, > + (VOID **)&PciIo, This->DriverBindingHandle, DeviceHandle, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL); if (EFI_ERROR > + (Status)) { > + return Status; > + } > + > + Dev = NON_DISCOVERABLE_PCI_DEVICE_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, > &gEdkiiNonDiscoverableDeviceProtocolGuid, > + 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 = { > + &NonDiscoverablePciDeviceSupported, > + &NonDiscoverablePciDeviceStart, > + &NonDiscoverablePciDeviceStop, > + 0x10, // Version, must be in [0x10 .. 0xFFFFFFEF] for IHV-developed > +drivers > + NULL, > + NULL > +}; > + > +// > +// Entry point of this driver. > +// > +EFI_STATUS > +EFIAPI > +NonDiscoverablePciDeviceDxeEntryPoint ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + return EfiLibInstallDriverBindingComponentName2 ( > + ImageHandle, > + SystemTable, > + &gDriverBinding, > + ImageHandle, > + &gComponentName, > + &gComponentName2 > + ); > +} > diff --git > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > PciDeviceDxe.inf > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > PciDeviceDxe.inf > new file mode 100644 > index 000000000000..996fe310e0e3 > --- /dev/null > +++ > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > Pc > +++ iDeviceDxe.inf > @@ -0,0 +1,52 @@ > +## @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 = 0x00010019 > + BASE_NAME = NonDiscoverablePciDeviceDxe > + FILE_GUID = 71fd84cd-353b-464d-b7a4-6ea7b96995cb > + MODULE_TYPE = UEFI_DRIVER > + VERSION_STRING = 1.0 > + ENTRY_POINT = NonDiscoverablePciDeviceDxeEntryPoint > + > +[Sources] > + ComponentName.c > + NonDiscoverablePciDeviceDxe.c > + NonDiscoverablePciDeviceIo.c > + NonDiscoverablePciDeviceIo.h > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + > +[LibraryClasses] > + BaseMemoryLib > + DebugLib > + MemoryAllocationLib > + UefiBootServicesTableLib > + UefiDriverEntryPoint > + UefiLib > + > +[Protocols] > + gEfiPciIoProtocolGuid ## BY_START > + gEdkiiNonDiscoverableDeviceProtocolGuid ## TO_START > + > +[Guids] > + gEdkiiNonDiscoverableAhciDeviceGuid > + gEdkiiNonDiscoverableEhciDeviceGuid > + gEdkiiNonDiscoverableNvmeDeviceGuid > + gEdkiiNonDiscoverableOhciDeviceGuid > + gEdkiiNonDiscoverableSdhciDeviceGuid > + gEdkiiNonDiscoverableUfsDeviceGuid > + gEdkiiNonDiscoverableUhciDeviceGuid > + gEdkiiNonDiscoverableXhciDeviceGuid > diff --git > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > PciDeviceIo.c > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > PciDeviceIo.c > new file mode 100644 > index 000000000000..fd59267a8d66 > --- /dev/null > +++ > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > Pc > +++ iDeviceIo.c > @@ -0,0 +1,797 @@ > +/** @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 "NonDiscoverablePciDeviceIo.h" > + > +#include <IndustryStandard/Acpi.h> > + > +#include <Protocol/PciRootBridgeIo.h> > + > +typedef struct { > + EFI_PHYSICAL_ADDRESS AllocAddress; > + VOID *HostAddress; > + EFI_PCI_IO_PROTOCOL_OPERATION Operation; > + UINTN NumberOfBytes; > +} NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO; > + > +// > +// Get the resource associated with BAR number 'BarIndex'. > +// > +STATIC > +EFI_STATUS > +GetBarResource ( > + IN NON_DISCOVERABLE_PCI_DEVICE *Dev, > + IN UINT8 BarIndex, > + OUT EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR **Descriptor > + ) > +{ > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > + > + if (BarIndex < Dev->BarOffset) { > + return EFI_NOT_FOUND; > + } > + > + BarIndex -= Dev->BarOffset; > + > + for (Desc = Dev->Device->Resources; > + Desc->Desc != ACPI_END_TAG_DESCRIPTOR; > + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { > + > + if (BarIndex == 0) { > + *Descriptor = Desc; > + return EFI_SUCCESS; > + } > + > + BarIndex -= 1; > + } > + return EFI_NOT_FOUND; > +} > + > +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 > + ) > +{ > + volatile UINT8 *Dst8; > + volatile UINT16 *Dst16; > + volatile UINT32 *Dst32; > + volatile CONST UINT8 *Src8; > + volatile CONST UINT16 *Src16; > + volatile CONST UINT32 *Src32; > + > + // > + // 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 > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + UINTN AlignMask; > + VOID *Address; > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > + EFI_STATUS Status; > + > + if (Buffer == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > + > + // > + // Only allow accesses to the BARs we emulate // Status = > + GetBarResource (Dev, BarIndex, &Desc); if (EFI_ERROR (Status)) { > + return Status; > + } > + > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { > + return EFI_UNSUPPORTED; > + } > + > + Address = (VOID *)(UINTN)(Desc->AddrRangeMin + 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 > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + UINTN AlignMask; > + VOID *Address; > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > + EFI_STATUS Status; > + > + if (Buffer == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > + > + // > + // Only allow accesses to the BARs we emulate // Status = > + GetBarResource (Dev, BarIndex, &Desc); if (EFI_ERROR (Status)) { > + return Status; > + } > + > + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { > + return EFI_UNSUPPORTED; > + } > + > + Address = (VOID *)(UINTN)(Desc->AddrRangeMin + 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 > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + VOID *Address; > + UINTN Length; > + > + if (Width < 0 || Width >= EfiPciIoWidthMaximum || Buffer == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > + Address = (UINT8 *)&Dev->ConfigSpace + Offset; Length = Count << > + ((UINTN)Width & 0x3); > + > + if (Offset + Length > sizeof (Dev->ConfigSpace)) { > + // > + // Read all zeroes for config space accesses beyond the first > + // 64 bytes > + // > + Length -= sizeof (Dev->ConfigSpace) - Offset; > + ZeroMem ((UINT8 *)Buffer + sizeof (Dev->ConfigSpace) - Offset, > + Length); > + > + Count -= Length >> ((UINTN)Width & 0x3); > + } > + 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 > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + VOID *Address; > + > + if (Width < 0 || Width >= EfiPciIoWidthMaximum || Buffer == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > + Address = (UINT8 *)&Dev->ConfigSpace + Offset; > + > + if (Offset + (Count << ((UINTN)Width & 0x3)) > sizeof (Dev->ConfigSpace)) > { > + return EFI_UNSUPPORTED; > + } > + > + 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 > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + EFI_STATUS Status; > + NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO *MapInfo; > + > + // > + // If HostAddress exceeds 4 GB, and this device does not support > + 64-bit DMA // addressing, we need to allocate a bounce buffer and copy > over the data. > + // > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > + if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == > 0 && > + (UINTN)HostAddress + *NumberOfBytes > SIZE_4GB) { > + > + // > + // Bounce buffering is not possible for consistent mappings > + // > + if (Operation == EfiPciIoOperationBusMasterCommonBuffer) { > + return EFI_UNSUPPORTED; > + } > + > + MapInfo = AllocatePool (sizeof *MapInfo); > + if (MapInfo == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + MapInfo->AllocAddress = MAX_UINT32; > + 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)) { > + // > + // If we fail here, it is likely because the system has no memory below > + // 4 GB to begin with. There is not much we can do about that other than > + // fail the map request. > + // > + FreePool (MapInfo); > + return EFI_DEVICE_ERROR; > + } > + if (Operation == EfiPciIoOperationBusMasterRead) { > + gBS->CopyMem ((VOID *)(UINTN)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 > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO *MapInfo; > + > + MapInfo = Mapping; > + if (MapInfo != NULL) { > + if (MapInfo->Operation == EfiPciIoOperationBusMasterWrite) { > + gBS->CopyMem (MapInfo->HostAddress, (VOID *)(UINTN)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 > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE *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)) != 0) { > + return EFI_UNSUPPORTED; > + } > + > + // > + // Allocate below 4 GB if the dual address cycle attribute has not > + // been set. If the system has no memory available below 4 GB, there > + // is little we can do except propagate the error. > + // > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > + if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == > 0) { > + AllocAddress = MAX_UINT32; > + 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; > + } > + > + *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 > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + BOOLEAN Enable; > + > + Dev = NON_DISCOVERABLE_PCI_DEVICE_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 device specific initialization now. > + // > + if (Enable && !Dev->Enabled && Dev->Device->Initialize != NULL) { > + Dev->Device->Initialize (Dev->Device); > + 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 > + ) > +{ > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor, *BarDesc; > + EFI_ACPI_END_TAG_DESCRIPTOR *End; > + EFI_STATUS Status; > + > + if (Supports == NULL && Resources == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > + > + Status = GetBarResource (Dev, BarIndex, &BarDesc); if (EFI_ERROR > + (Status)) { > + return Status; > + } > + > + // > + // Don't expose any configurable attributes for our emulated BAR // > + if (Supports != NULL) { > + *Supports = 0; > + } > + > + if (Resources != NULL) { > + Descriptor = AllocatePool (sizeof > (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + > + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR)); > + if (Descriptor == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + CopyMem (Descriptor, BarDesc, sizeof *Descriptor); > + > + End = (EFI_ACPI_END_TAG_DESCRIPTOR *) (Descriptor + 1); > + End->Desc = ACPI_END_TAG_DESCRIPTOR; > + End->Checksum = 0; > + > + *Resources = Descriptor; > + } > + return EFI_SUCCESS; > +} > + > +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 ( > + NON_DISCOVERABLE_PCI_DEVICE *Dev > + ) > +{ > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > + INTN Idx; > + > + InitializeListHead (&Dev->UncachedAllocationList); > + > + Dev->ConfigSpace.Hdr.VendorId = PCI_ID_VENDOR_UNKNOWN; > + Dev->ConfigSpace.Hdr.DeviceId = PCI_ID_DEVICE_DONTCARE; > + > + // Copy protocol structure > + CopyMem(&Dev->PciIo, &PciIoTemplate, sizeof PciIoTemplate); > + > + if (CompareGuid (Dev->Device->Type, > &gEdkiiNonDiscoverableAhciDeviceGuid)) { > + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_MASS_STORAGE_AHCI; > + Dev->ConfigSpace.Hdr.ClassCode[1] = > PCI_CLASS_MASS_STORAGE_SATADPA; > + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_MASS_STORAGE; > + Dev->BarOffset = 5; > + } else if (CompareGuid (Dev->Device->Type, > + &gEdkiiNonDiscoverableEhciDeviceGuid)) { > + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_EHCI; > + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB; > + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL; > + Dev->BarOffset = 0; > + } else if (CompareGuid (Dev->Device->Type, > + &gEdkiiNonDiscoverableNvmeDeviceGuid)) { > + Dev->ConfigSpace.Hdr.ClassCode[0] = 0x2; // PCI_IF_NVMHCI > + Dev->ConfigSpace.Hdr.ClassCode[1] = 0x8; // > PCI_CLASS_MASS_STORAGE_NVM > + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_MASS_STORAGE; > + Dev->BarOffset = 0; > + } else if (CompareGuid (Dev->Device->Type, > + &gEdkiiNonDiscoverableOhciDeviceGuid)) { > + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_OHCI; > + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB; > + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL; > + Dev->BarOffset = 0; > + } else if (CompareGuid (Dev->Device->Type, > + &gEdkiiNonDiscoverableSdhciDeviceGuid)) { > + Dev->ConfigSpace.Hdr.ClassCode[0] = 0x0; // don't care > + Dev->ConfigSpace.Hdr.ClassCode[1] = > PCI_SUBCLASS_SD_HOST_CONTROLLER; > + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SYSTEM_PERIPHERAL; > + Dev->BarOffset = 0; > + } else if (CompareGuid (Dev->Device->Type, > + &gEdkiiNonDiscoverableXhciDeviceGuid)) { > + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_XHCI; > + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB; > + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL; > + Dev->BarOffset = 0; > + } else if (CompareGuid (Dev->Device->Type, > + &gEdkiiNonDiscoverableUhciDeviceGuid)) { > + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_UHCI; > + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB; > + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL; > + Dev->BarOffset = 0; > + } else if (CompareGuid (Dev->Device->Type, > + &gEdkiiNonDiscoverableUfsDeviceGuid)) { > + Dev->ConfigSpace.Hdr.ClassCode[0] = 0x0; // don't care > + Dev->ConfigSpace.Hdr.ClassCode[1] = 0x9; // UFS controller subclass; > + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_MASS_STORAGE; > + Dev->BarOffset = 0; > + } else { > + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER); } > + > + // > + // Iterate over the resources to populate the virtual BARs // Idx = > + Dev->BarOffset; for (Desc = Dev->Device->Resources, Dev->BarCount = > + 0; > + Desc->Desc != ACPI_END_TAG_DESCRIPTOR; > + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { > + > + ASSERT (Desc->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR); > + ASSERT (Desc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM); > + > + if (Idx >= PCI_MAX_BARS || > + (Idx == PCI_MAX_BARS - 1 && Desc->AddrSpaceGranularity == 64)) { > + DEBUG ((DEBUG_ERROR, > + "%a: resource count exceeds number of emulated BARs\n", > + __FUNCTION__)); > + ASSERT (FALSE); > + break; > + } > + > + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; > + Dev->BarCount++; > + > + if (Desc->AddrSpaceGranularity == 64) { > + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; > + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc- > >AddrRangeMin >> 32); 1. You need to use RShiftU64 otherwise the above code will break the build process in 32bit. > + } > + } > +} > diff --git > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > PciDeviceIo.h > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > PciDeviceIo.h > new file mode 100644 > index 000000000000..bc0a3d3258f9 > --- /dev/null > +++ > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > Pc > +++ iDeviceIo.h > @@ -0,0 +1,84 @@ > +/** @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. > + > +**/ > + > +#ifndef __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > +#define __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > + > +#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/ComponentName.h> > +#include <Protocol/NonDiscoverableDevice.h> > +#include <Protocol/PciIo.h> > + > +#define NON_DISCOVERABLE_PCI_DEVICE_SIG SIGNATURE_32 ('P', 'P', 'I', > +'D') > + > +#define NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(PciIoPointer) \ > + CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \ > + NON_DISCOVERABLE_PCI_DEVICE_SIG) > + > +#define PCI_ID_VENDOR_UNKNOWN 0xffff > +#define PCI_ID_DEVICE_DONTCARE 0x0000 > + > +#define PCI_MAX_BARS 6 > + > +typedef struct { > + UINT32 Signature; > + // > + // The bound non-discoverable device protocol instance > + // > + NON_DISCOVERABLE_DEVICE *Device; > + // > + // 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 first virtual BAR to assign based on the resources described > + // by the non-discoverable device. > + // > + UINT32 BarOffset; > + // > + // The number of virtual BARs we expose based on the number of > + // resources > + // > + UINT32 BarCount; > + // > + // The PCI I/O attributes for this device > + // > + UINT64 Attributes; > + // > + // Whether this device has been enabled > + // > + BOOLEAN Enabled; > +} NON_DISCOVERABLE_PCI_DEVICE; > + > +VOID > +InitializePciIoProtocol ( > + NON_DISCOVERABLE_PCI_DEVICE *Device > + ); > + > +extern EFI_COMPONENT_NAME_PROTOCOL gComponentName; extern > +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2; > + > +#endif > diff --git a/MdeModulePkg/MdeModulePkg.dsc > b/MdeModulePkg/MdeModulePkg.dsc index 20e5e8c78be0..5dd30556cb3c > 100644 > --- a/MdeModulePkg/MdeModulePkg.dsc > +++ b/MdeModulePkg/MdeModulePkg.dsc > @@ -265,6 +265,7 @@ [Components] > MdeModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf > MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf > MdeModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf > + > + > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > Dev > + iceDxe.inf > > MdeModulePkg/Core/Dxe/DxeMain.inf { > <LibraryClasses> > -- > 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hi Ray, Ard is on holiday this week, and mostly managing to stay off the email... Anyway, I could really use this support in order to bring the Marvell Armada 70x0 support (worked on by the Semihalf guys) into OpenPlatformPkg, so I will pick this up while he is away - at least parts 1-3. 4/5 is required for 5/5, so I will not be looking to push the final one myself. Would you OK with pushing just 1-3 once we get this one resolved? On Mon, Nov 28, 2016 at 01:56:21AM +0000, Ni, Ruiyu wrote: > > diff --git > > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > PciDeviceIo.c > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > PciDeviceIo.c > > new file mode 100644 > > index 000000000000..fd59267a8d66 > > --- /dev/null > > +++ > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > Pc > > +++ iDeviceIo.c > > @@ -0,0 +1,797 @@ ... > > +VOID > > +InitializePciIoProtocol ( > > + NON_DISCOVERABLE_PCI_DEVICE *Dev > > + ) > > +{ > > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > > + INTN Idx; > > + > > + InitializeListHead (&Dev->UncachedAllocationList); The above line has crept in here, but belongs to 4/5 (causes compilation error). I will fix that and resend - is it OK if I resend this patch only? > > + // Iterate over the resources to populate the virtual BARs > > + // > > + Idx = Dev->BarOffset; > > + for (Desc = Dev->Device->Resources, Dev->BarCount = 0; > > + Desc->Desc != ACPI_END_TAG_DESCRIPTOR; > > + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { > > + > > + ASSERT (Desc->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR); > > + ASSERT (Desc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM); > > + > > + if (Idx >= PCI_MAX_BARS || > > + (Idx == PCI_MAX_BARS - 1 && Desc->AddrSpaceGranularity == 64)) { > > + DEBUG ((DEBUG_ERROR, > > + "%a: resource count exceeds number of emulated BARs\n", > > + __FUNCTION__)); > > + ASSERT (FALSE); > > + break; > > + } > > + > > + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; > > + Dev->BarCount++; > > + > > + if (Desc->AddrSpaceGranularity == 64) { > > + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; > > + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc->AddrRangeMin >> 32); > > 1. You need to use RShiftU64 otherwise the above code will break the > build process in 32bit. I don't understand how that could cause breakage (and experiments with IA32 and ARM fail to reproduce it with GCC). - Bar[x] is an array of UINT32 - Desc->AddrRangeMin is UINT64 - The result of (Desc->AddrRangeMin >> 32) is explicitly cast to UINT32 (the type of Bar[x]). Is the problem related to the shift value being signed? Or does some other compiler refuse to cast a UINT64 to a UINT32? Regards, Leif > > + } > > + } > > +} > > diff --git > > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > PciDeviceIo.h > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > PciDeviceIo.h > > new file mode 100644 > > index 000000000000..bc0a3d3258f9 > > --- /dev/null > > +++ > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > Pc > > +++ iDeviceIo.h > > @@ -0,0 +1,84 @@ > > +/** @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. > > + > > +**/ > > + > > +#ifndef __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > > +#define __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > > + > > +#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/ComponentName.h> > > +#include <Protocol/NonDiscoverableDevice.h> > > +#include <Protocol/PciIo.h> > > + > > +#define NON_DISCOVERABLE_PCI_DEVICE_SIG SIGNATURE_32 ('P', 'P', 'I', > > +'D') > > + > > +#define NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(PciIoPointer) \ > > + CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \ > > + NON_DISCOVERABLE_PCI_DEVICE_SIG) > > + > > +#define PCI_ID_VENDOR_UNKNOWN 0xffff > > +#define PCI_ID_DEVICE_DONTCARE 0x0000 > > + > > +#define PCI_MAX_BARS 6 > > + > > +typedef struct { > > + UINT32 Signature; > > + // > > + // The bound non-discoverable device protocol instance > > + // > > + NON_DISCOVERABLE_DEVICE *Device; > > + // > > + // 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 first virtual BAR to assign based on the resources described > > + // by the non-discoverable device. > > + // > > + UINT32 BarOffset; > > + // > > + // The number of virtual BARs we expose based on the number of > > + // resources > > + // > > + UINT32 BarCount; > > + // > > + // The PCI I/O attributes for this device > > + // > > + UINT64 Attributes; > > + // > > + // Whether this device has been enabled > > + // > > + BOOLEAN Enabled; > > +} NON_DISCOVERABLE_PCI_DEVICE; > > + > > +VOID > > +InitializePciIoProtocol ( > > + NON_DISCOVERABLE_PCI_DEVICE *Device > > + ); > > + > > +extern EFI_COMPONENT_NAME_PROTOCOL gComponentName; extern > > +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2; > > + > > +#endif > > diff --git a/MdeModulePkg/MdeModulePkg.dsc > > b/MdeModulePkg/MdeModulePkg.dsc index 20e5e8c78be0..5dd30556cb3c > > 100644 > > --- a/MdeModulePkg/MdeModulePkg.dsc > > +++ b/MdeModulePkg/MdeModulePkg.dsc > > @@ -265,6 +265,7 @@ [Components] > > MdeModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf > > MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf > > MdeModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf > > + > > + > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > > Dev > > + iceDxe.inf > > > > MdeModulePkg/Core/Dxe/DxeMain.inf { > > <LibraryClasses> > > -- > > 2.7.4 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Ray, any comments on the below? Regards, Leif On Wed, Nov 30, 2016 at 02:06:55PM +0000, Leif Lindholm wrote: > Hi Ray, > > Ard is on holiday this week, and mostly managing to stay off the > email... > > Anyway, I could really use this support in order to bring the Marvell > Armada 70x0 support (worked on by the Semihalf guys) into > OpenPlatformPkg, so I will pick this up while he is away - at least > parts 1-3. > > 4/5 is required for 5/5, so I will not be looking to push the final > one myself. > > Would you OK with pushing just 1-3 once we get this one resolved? > > On Mon, Nov 28, 2016 at 01:56:21AM +0000, Ni, Ruiyu wrote: > > > diff --git > > > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > > PciDeviceIo.c > > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > > PciDeviceIo.c > > > new file mode 100644 > > > index 000000000000..fd59267a8d66 > > > --- /dev/null > > > +++ > > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > > Pc > > > +++ iDeviceIo.c > > > @@ -0,0 +1,797 @@ > ... > > > +VOID > > > +InitializePciIoProtocol ( > > > + NON_DISCOVERABLE_PCI_DEVICE *Dev > > > + ) > > > +{ > > > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; > > > + INTN Idx; > > > + > > > + InitializeListHead (&Dev->UncachedAllocationList); > > The above line has crept in here, but belongs to 4/5 (causes > compilation error). I will fix that and resend - is it OK if I resend > this patch only? > > > > + // Iterate over the resources to populate the virtual BARs > > > + // > > > + Idx = Dev->BarOffset; > > > + for (Desc = Dev->Device->Resources, Dev->BarCount = 0; > > > + Desc->Desc != ACPI_END_TAG_DESCRIPTOR; > > > + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { > > > + > > > + ASSERT (Desc->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR); > > > + ASSERT (Desc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM); > > > + > > > + if (Idx >= PCI_MAX_BARS || > > > + (Idx == PCI_MAX_BARS - 1 && Desc->AddrSpaceGranularity == 64)) { > > > + DEBUG ((DEBUG_ERROR, > > > + "%a: resource count exceeds number of emulated BARs\n", > > > + __FUNCTION__)); > > > + ASSERT (FALSE); > > > + break; > > > + } > > > + > > > + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; > > > + Dev->BarCount++; > > > + > > > + if (Desc->AddrSpaceGranularity == 64) { > > > + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; > > > + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc->AddrRangeMin >> 32); > > > > 1. You need to use RShiftU64 otherwise the above code will break the > > build process in 32bit. > > I don't understand how that could cause breakage (and experiments with > IA32 and ARM fail to reproduce it with GCC). > > - Bar[x] is an array of UINT32 > - Desc->AddrRangeMin is UINT64 > - The result of (Desc->AddrRangeMin >> 32) is explicitly cast to > UINT32 (the type of Bar[x]). > > Is the problem related to the shift value being signed? > Or does some other compiler refuse to cast a UINT64 to a UINT32? > > Regards, > > Leif > > > > + } > > > + } > > > +} > > > diff --git > > > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > > PciDeviceIo.h > > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > > PciDeviceIo.h > > > new file mode 100644 > > > index 000000000000..bc0a3d3258f9 > > > --- /dev/null > > > +++ > > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > > > Pc > > > +++ iDeviceIo.h > > > @@ -0,0 +1,84 @@ > > > +/** @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. > > > + > > > +**/ > > > + > > > +#ifndef __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > > > +#define __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > > > + > > > +#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/ComponentName.h> > > > +#include <Protocol/NonDiscoverableDevice.h> > > > +#include <Protocol/PciIo.h> > > > + > > > +#define NON_DISCOVERABLE_PCI_DEVICE_SIG SIGNATURE_32 ('P', 'P', 'I', > > > +'D') > > > + > > > +#define NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(PciIoPointer) \ > > > + CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \ > > > + NON_DISCOVERABLE_PCI_DEVICE_SIG) > > > + > > > +#define PCI_ID_VENDOR_UNKNOWN 0xffff > > > +#define PCI_ID_DEVICE_DONTCARE 0x0000 > > > + > > > +#define PCI_MAX_BARS 6 > > > + > > > +typedef struct { > > > + UINT32 Signature; > > > + // > > > + // The bound non-discoverable device protocol instance > > > + // > > > + NON_DISCOVERABLE_DEVICE *Device; > > > + // > > > + // 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 first virtual BAR to assign based on the resources described > > > + // by the non-discoverable device. > > > + // > > > + UINT32 BarOffset; > > > + // > > > + // The number of virtual BARs we expose based on the number of > > > + // resources > > > + // > > > + UINT32 BarCount; > > > + // > > > + // The PCI I/O attributes for this device > > > + // > > > + UINT64 Attributes; > > > + // > > > + // Whether this device has been enabled > > > + // > > > + BOOLEAN Enabled; > > > +} NON_DISCOVERABLE_PCI_DEVICE; > > > + > > > +VOID > > > +InitializePciIoProtocol ( > > > + NON_DISCOVERABLE_PCI_DEVICE *Device > > > + ); > > > + > > > +extern EFI_COMPONENT_NAME_PROTOCOL gComponentName; extern > > > +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2; > > > + > > > +#endif > > > diff --git a/MdeModulePkg/MdeModulePkg.dsc > > > b/MdeModulePkg/MdeModulePkg.dsc index 20e5e8c78be0..5dd30556cb3c > > > 100644 > > > --- a/MdeModulePkg/MdeModulePkg.dsc > > > +++ b/MdeModulePkg/MdeModulePkg.dsc > > > @@ -265,6 +265,7 @@ [Components] > > > MdeModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf > > > MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf > > > MdeModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf > > > + > > > + > > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > > > Dev > > > + iceDxe.inf > > > > > > MdeModulePkg/Core/Dxe/DxeMain.inf { > > > <LibraryClasses> > > > -- > > > 2.7.4 > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Regards, Ray >-----Original Message----- >From: Leif Lindholm [mailto:leif.lindholm@linaro.org] >Sent: Wednesday, November 30, 2016 10:07 PM >To: Ni, Ruiyu <ruiyu.ni@intel.com> >Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; >afish@apple.com; Kinney, Michael D <michael.d.kinney@intel.com>; mw@semihalf.com; Tian, Feng <feng.tian@intel.com> >Subject: Re: [PATCH v4 3/5] MdeModulePkg: implement generic PCI I/O driver for non-discoverable devices > >Hi Ray, > >Ard is on holiday this week, and mostly managing to stay off the >email... > >Anyway, I could really use this support in order to bring the Marvell >Armada 70x0 support (worked on by the Semihalf guys) into >OpenPlatformPkg, so I will pick this up while he is away - at least >parts 1-3. > >4/5 is required for 5/5, so I will not be looking to push the final >one myself. > >Would you OK with pushing just 1-3 once we get this one resolved? > >On Mon, Nov 28, 2016 at 01:56:21AM +0000, Ni, Ruiyu wrote: >> > diff --git >> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable >> > PciDeviceIo.c >> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable >> > PciDeviceIo.c >> > new file mode 100644 >> > index 000000000000..fd59267a8d66 >> > --- /dev/null >> > +++ >> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable >> > Pc >> > +++ iDeviceIo.c >> > @@ -0,0 +1,797 @@ >... >> > +VOID >> > +InitializePciIoProtocol ( >> > + NON_DISCOVERABLE_PCI_DEVICE *Dev >> > + ) >> > +{ >> > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; >> > + INTN Idx; >> > + >> > + InitializeListHead (&Dev->UncachedAllocationList); > >The above line has crept in here, but belongs to 4/5 (causes >compilation error). I will fix that and resend - is it OK if I resend >this patch only? > >> > + // Iterate over the resources to populate the virtual BARs >> > + // >> > + Idx = Dev->BarOffset; >> > + for (Desc = Dev->Device->Resources, Dev->BarCount = 0; >> > + Desc->Desc != ACPI_END_TAG_DESCRIPTOR; >> > + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { >> > + >> > + ASSERT (Desc->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR); >> > + ASSERT (Desc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM); >> > + >> > + if (Idx >= PCI_MAX_BARS || >> > + (Idx == PCI_MAX_BARS - 1 && Desc->AddrSpaceGranularity == 64)) { >> > + DEBUG ((DEBUG_ERROR, >> > + "%a: resource count exceeds number of emulated BARs\n", >> > + __FUNCTION__)); >> > + ASSERT (FALSE); >> > + break; >> > + } >> > + >> > + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; >> > + Dev->BarCount++; >> > + >> > + if (Desc->AddrSpaceGranularity == 64) { >> > + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; >> > + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc->AddrRangeMin >> 32); >> >> 1. You need to use RShiftU64 otherwise the above code will break the >> build process in 32bit. > >I don't understand how that could cause breakage (and experiments with >IA32 and ARM fail to reproduce it with GCC). > >- Bar[x] is an array of UINT32 >- Desc->AddrRangeMin is UINT64 >- The result of (Desc->AddrRangeMin >> 32) is explicitly cast to > UINT32 (the type of Bar[x]). > >Is the problem related to the shift value being signed? >Or does some other compiler refuse to cast a UINT64 to a UINT32? Desc->AddrRangeMin is UINT64. My experience tells me that LShiftU64 or RShiftU64 should be used for shifting operation on UINT64. I guess the GCC might cleverly recognizes the ">>32" actually picks up the high-32 bits so in 32bit environment, it just uses value of the register which stores the high-32 bits. Can you check the assembly code GCC generates? or simply can you change ">>32" to ">>31" or ">>33" to see what happens? > >Regards, > >Leif > >> > + } >> > + } >> > +} >> > diff --git >> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable >> > PciDeviceIo.h >> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable >> > PciDeviceIo.h >> > new file mode 100644 >> > index 000000000000..bc0a3d3258f9 >> > --- /dev/null >> > +++ >> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable >> > Pc >> > +++ iDeviceIo.h >> > @@ -0,0 +1,84 @@ >> > +/** @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. >> > + >> > +**/ >> > + >> > +#ifndef __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ >> > +#define __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ >> > + >> > +#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/ComponentName.h> >> > +#include <Protocol/NonDiscoverableDevice.h> >> > +#include <Protocol/PciIo.h> >> > + >> > +#define NON_DISCOVERABLE_PCI_DEVICE_SIG SIGNATURE_32 ('P', 'P', 'I', >> > +'D') >> > + >> > +#define NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(PciIoPointer) \ >> > + CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \ >> > + NON_DISCOVERABLE_PCI_DEVICE_SIG) >> > + >> > +#define PCI_ID_VENDOR_UNKNOWN 0xffff >> > +#define PCI_ID_DEVICE_DONTCARE 0x0000 >> > + >> > +#define PCI_MAX_BARS 6 >> > + >> > +typedef struct { >> > + UINT32 Signature; >> > + // >> > + // The bound non-discoverable device protocol instance >> > + // >> > + NON_DISCOVERABLE_DEVICE *Device; >> > + // >> > + // 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 first virtual BAR to assign based on the resources described >> > + // by the non-discoverable device. >> > + // >> > + UINT32 BarOffset; >> > + // >> > + // The number of virtual BARs we expose based on the number of >> > + // resources >> > + // >> > + UINT32 BarCount; >> > + // >> > + // The PCI I/O attributes for this device >> > + // >> > + UINT64 Attributes; >> > + // >> > + // Whether this device has been enabled >> > + // >> > + BOOLEAN Enabled; >> > +} NON_DISCOVERABLE_PCI_DEVICE; >> > + >> > +VOID >> > +InitializePciIoProtocol ( >> > + NON_DISCOVERABLE_PCI_DEVICE *Device >> > + ); >> > + >> > +extern EFI_COMPONENT_NAME_PROTOCOL gComponentName; extern >> > +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2; >> > + >> > +#endif >> > diff --git a/MdeModulePkg/MdeModulePkg.dsc >> > b/MdeModulePkg/MdeModulePkg.dsc index 20e5e8c78be0..5dd30556cb3c >> > 100644 >> > --- a/MdeModulePkg/MdeModulePkg.dsc >> > +++ b/MdeModulePkg/MdeModulePkg.dsc >> > @@ -265,6 +265,7 @@ [Components] >> > MdeModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf >> > MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf >> > MdeModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf >> > + >> > + >> > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci >> > Dev >> > + iceDxe.inf >> > >> > MdeModulePkg/Core/Dxe/DxeMain.inf { >> > <LibraryClasses> >> > -- >> > 2.7.4 >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Fri, Dec 02, 2016 at 12:54:45PM +0000, Ni, Ruiyu wrote: > >> > + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; > >> > + Dev->BarCount++; > >> > + > >> > + if (Desc->AddrSpaceGranularity == 64) { > >> > + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; > >> > + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc->AddrRangeMin >> 32); > >> > >> 1. You need to use RShiftU64 otherwise the above code will break the > >> build process in 32bit. > > > >I don't understand how that could cause breakage (and experiments with > >IA32 and ARM fail to reproduce it with GCC). > > > >- Bar[x] is an array of UINT32 > >- Desc->AddrRangeMin is UINT64 > >- The result of (Desc->AddrRangeMin >> 32) is explicitly cast to > > UINT32 (the type of Bar[x]). > > > >Is the problem related to the shift value being signed? > >Or does some other compiler refuse to cast a UINT64 to a UINT32? > > Desc->AddrRangeMin is UINT64. My experience tells me that > LShiftU64 or RShiftU64 should be used for shifting operation > on UINT64. This is the bit that confuses me. There is no such automatic limitation in the C language (if long long is supported at all). So if we're working around some bug in a specific compiler - I am happy to do so. I just prefer knowing why rather than cargo-culting. > I guess the GCC might cleverly recognizes the ">>32" actually > picks up the high-32 bits so in 32bit environment, it just uses > value of the register which stores the high-32 bits. > Can you check the assembly code GCC generates? > or simply can you change ">>32" to ">>31" or ">>33" to see > what happens? There is no cleverness required, it's just a defined operation on a base type. But sure, for example: unsigned int shift(long long val) { return (val >> 33); } decodes as --- 0000000000000000 <shift>: 0: 48 89 f8 mov %rdi,%rax 3: 48 c1 f8 21 sar $0x21,%rax 7: c3 retq --- (in x86_64, using gcc -O2 to get rid of some redundant stack operations) The only thing that changes if the shift value is modified is that the 0x21 is changed accordingly. Change it to an inline constant, not much changes: unsigned int shiftconst(void) { unsigned long long var = 0xaaaabbbbccccddddULL; return (var >> 31); } decodes as --- 0000000000000000 <shiftconst>: 0: 55 push %rbp 1: 48 89 e5 mov %rsp,%rbp 4: 48 b8 dd dd cc cc bb movabs $0xaaaabbbbccccdddd,%rax b: bb aa aa e: 48 89 45 f8 mov %rax,-0x8(%rbp) 12: 48 8b 45 f8 mov -0x8(%rbp),%rax 16: 48 c1 e8 1f shr $0x1f,%rax 1a: 5d pop %rbp 1b: c3 retq --- (with -O0, to prevent the compiler from just doing the arithmetic compile time) Both also compile correctly for IA32 with -m32. The point is that there is nothing clever here for the compiler to do: casting a 64-bit int to a 32-bit int is an operation that discards the top 32 bits - but the behaviour is entirely defined. In fact, all architectures other than IA32/ARM use the Math64.c implementation of RShiftU64, which simply does the shift. And ARM does it only for GCC. I'm not sure why, I can't imagine a GCC version that would do anything more complicated than --- 00000000 <shift>: 0: e1a000c1 asr r0, r1, #1 4: e12fff1e bx lr --- (and that's with -march=armv4t onwards). So if we are saying that there are certain specific compilers that do not support 64-bit arithmetic for 32-bit architectures, and that those compilers are still supported by BaseTools - then I'm fine with that. But I don't want to replace functional C code with abstraction functions without a proper understanding of where it would be problematic. Regards, Leif > > > > >Regards, > > > >Leif > > > >> > + } > >> > + } > >> > +} > >> > diff --git > >> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > >> > PciDeviceIo.h > >> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > >> > PciDeviceIo.h > >> > new file mode 100644 > >> > index 000000000000..bc0a3d3258f9 > >> > --- /dev/null > >> > +++ > >> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > >> > Pc > >> > +++ iDeviceIo.h > >> > @@ -0,0 +1,84 @@ > >> > +/** @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. > >> > + > >> > +**/ > >> > + > >> > +#ifndef __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > >> > +#define __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > >> > + > >> > +#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/ComponentName.h> > >> > +#include <Protocol/NonDiscoverableDevice.h> > >> > +#include <Protocol/PciIo.h> > >> > + > >> > +#define NON_DISCOVERABLE_PCI_DEVICE_SIG SIGNATURE_32 ('P', 'P', 'I', > >> > +'D') > >> > + > >> > +#define NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(PciIoPointer) \ > >> > + CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \ > >> > + NON_DISCOVERABLE_PCI_DEVICE_SIG) > >> > + > >> > +#define PCI_ID_VENDOR_UNKNOWN 0xffff > >> > +#define PCI_ID_DEVICE_DONTCARE 0x0000 > >> > + > >> > +#define PCI_MAX_BARS 6 > >> > + > >> > +typedef struct { > >> > + UINT32 Signature; > >> > + // > >> > + // The bound non-discoverable device protocol instance > >> > + // > >> > + NON_DISCOVERABLE_DEVICE *Device; > >> > + // > >> > + // 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 first virtual BAR to assign based on the resources described > >> > + // by the non-discoverable device. > >> > + // > >> > + UINT32 BarOffset; > >> > + // > >> > + // The number of virtual BARs we expose based on the number of > >> > + // resources > >> > + // > >> > + UINT32 BarCount; > >> > + // > >> > + // The PCI I/O attributes for this device > >> > + // > >> > + UINT64 Attributes; > >> > + // > >> > + // Whether this device has been enabled > >> > + // > >> > + BOOLEAN Enabled; > >> > +} NON_DISCOVERABLE_PCI_DEVICE; > >> > + > >> > +VOID > >> > +InitializePciIoProtocol ( > >> > + NON_DISCOVERABLE_PCI_DEVICE *Device > >> > + ); > >> > + > >> > +extern EFI_COMPONENT_NAME_PROTOCOL gComponentName; extern > >> > +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2; > >> > + > >> > +#endif > >> > diff --git a/MdeModulePkg/MdeModulePkg.dsc > >> > b/MdeModulePkg/MdeModulePkg.dsc index 20e5e8c78be0..5dd30556cb3c > >> > 100644 > >> > --- a/MdeModulePkg/MdeModulePkg.dsc > >> > +++ b/MdeModulePkg/MdeModulePkg.dsc > >> > @@ -265,6 +265,7 @@ [Components] > >> > MdeModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf > >> > MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf > >> > MdeModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf > >> > + > >> > + > >> > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > >> > Dev > >> > + iceDxe.inf > >> > > >> > MdeModulePkg/Core/Dxe/DxeMain.inf { > >> > <LibraryClasses> > >> > -- > >> > 2.7.4 > >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
> On 2 Dec 2016, at 14:40, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Fri, Dec 02, 2016 at 12:54:45PM +0000, Ni, Ruiyu wrote: >>>>> + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; >>>>> + Dev->BarCount++; >>>>> + >>>>> + if (Desc->AddrSpaceGranularity == 64) { >>>>> + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; >>>>> + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc->AddrRangeMin >> 32); >>>> >>>> 1. You need to use RShiftU64 otherwise the above code will break the >>>> build process in 32bit. >>> >>> I don't understand how that could cause breakage (and experiments with >>> IA32 and ARM fail to reproduce it with GCC). >>> >>> - Bar[x] is an array of UINT32 >>> - Desc->AddrRangeMin is UINT64 >>> - The result of (Desc->AddrRangeMin >> 32) is explicitly cast to >>> UINT32 (the type of Bar[x]). >>> >>> Is the problem related to the shift value being signed? >>> Or does some other compiler refuse to cast a UINT64 to a UINT32? >> >> Desc->AddrRangeMin is UINT64. My experience tells me that >> LShiftU64 or RShiftU64 should be used for shifting operation >> on UINT64. > > This is the bit that confuses me. There is no such automatic > limitation in the C language (if long long is supported at all). So if > we're working around some bug in a specific compiler - I am happy to > do so. I just prefer knowing why rather than cargo-culting. > >> I guess the GCC might cleverly recognizes the ">>32" actually >> picks up the high-32 bits so in 32bit environment, it just uses >> value of the register which stores the high-32 bits. >> Can you check the assembly code GCC generates? >> or simply can you change ">>32" to ">>31" or ">>33" to see >> what happens? > > There is no cleverness required, it's just a defined operation on a > base type. But sure, for example: > > unsigned int shift(long long val) > { > return (val >> 33); > } > > decodes as > --- > 0000000000000000 <shift>: > 0: 48 89 f8 mov %rdi,%rax > 3: 48 c1 f8 21 sar $0x21,%rax > 7: c3 retq > --- > (in x86_64, using gcc -O2 to get rid of some redundant stack > operations) > > The only thing that changes if the shift value is modified is that the > 0x21 is changed accordingly. > > Change it to an inline constant, not much changes: > > unsigned int shiftconst(void) > { > unsigned long long var = 0xaaaabbbbccccddddULL; > return (var >> 31); > } > > decodes as > --- > 0000000000000000 <shiftconst>: > 0: 55 push %rbp > 1: 48 89 e5 mov %rsp,%rbp > 4: 48 b8 dd dd cc cc bb movabs $0xaaaabbbbccccdddd,%rax > b: bb aa aa > e: 48 89 45 f8 mov %rax,-0x8(%rbp) > 12: 48 8b 45 f8 mov -0x8(%rbp),%rax > 16: 48 c1 e8 1f shr $0x1f,%rax > 1a: 5d pop %rbp > 1b: c3 retq > --- > (with -O0, to prevent the compiler from just doing the arithmetic > compile time) > > Both also compile correctly for IA32 with -m32. > > The point is that there is nothing clever here for the compiler to do: > casting a 64-bit int to a 32-bit int is an operation that discards the > top 32 bits - but the behaviour is entirely defined. > > In fact, all architectures other than IA32/ARM use the Math64.c > implementation of RShiftU64, which simply does the shift. > And ARM does it only for GCC. I'm not sure why, I can't imagine a GCC > version that would do anything more complicated than > --- > 00000000 <shift>: > 0: e1a000c1 asr r0, r1, #1 > 4: e12fff1e bx lr > --- > (and that's with -march=armv4t onwards) That's a 32-bit operand. Shifting a 64-bit value will result in a call to a compiler intrinsic, which is kind of the point of Riuyu's remark > So if we are saying that there are certain specific compilers that do > not support 64-bit arithmetic for 32-bit architectures, and that those > compilers are still supported by BaseTools - then I'm fine with that. > > But I don't want to replace functional C code with abstraction > functions without a proper understanding of where it would be > problematic. > > Regards, > > Leif > >> >>> >>> Regards, >>> >>> Leif >>> >>>>> + } >>>>> + } >>>>> +} >>>>> diff --git >>>>> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable >>>>> PciDeviceIo.h >>>>> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable >>>>> PciDeviceIo.h >>>>> new file mode 100644 >>>>> index 000000000000..bc0a3d3258f9 >>>>> --- /dev/null >>>>> +++ >>>>> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable >>>>> Pc >>>>> +++ iDeviceIo.h >>>>> @@ -0,0 +1,84 @@ >>>>> +/** @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. >>>>> + >>>>> +**/ >>>>> + >>>>> +#ifndef __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ >>>>> +#define __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ >>>>> + >>>>> +#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/ComponentName.h> >>>>> +#include <Protocol/NonDiscoverableDevice.h> >>>>> +#include <Protocol/PciIo.h> >>>>> + >>>>> +#define NON_DISCOVERABLE_PCI_DEVICE_SIG SIGNATURE_32 ('P', 'P', 'I', >>>>> +'D') >>>>> + >>>>> +#define NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(PciIoPointer) \ >>>>> + CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \ >>>>> + NON_DISCOVERABLE_PCI_DEVICE_SIG) >>>>> + >>>>> +#define PCI_ID_VENDOR_UNKNOWN 0xffff >>>>> +#define PCI_ID_DEVICE_DONTCARE 0x0000 >>>>> + >>>>> +#define PCI_MAX_BARS 6 >>>>> + >>>>> +typedef struct { >>>>> + UINT32 Signature; >>>>> + // >>>>> + // The bound non-discoverable device protocol instance >>>>> + // >>>>> + NON_DISCOVERABLE_DEVICE *Device; >>>>> + // >>>>> + // 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 first virtual BAR to assign based on the resources described >>>>> + // by the non-discoverable device. >>>>> + // >>>>> + UINT32 BarOffset; >>>>> + // >>>>> + // The number of virtual BARs we expose based on the number of >>>>> + // resources >>>>> + // >>>>> + UINT32 BarCount; >>>>> + // >>>>> + // The PCI I/O attributes for this device >>>>> + // >>>>> + UINT64 Attributes; >>>>> + // >>>>> + // Whether this device has been enabled >>>>> + // >>>>> + BOOLEAN Enabled; >>>>> +} NON_DISCOVERABLE_PCI_DEVICE; >>>>> + >>>>> +VOID >>>>> +InitializePciIoProtocol ( >>>>> + NON_DISCOVERABLE_PCI_DEVICE *Device >>>>> + ); >>>>> + >>>>> +extern EFI_COMPONENT_NAME_PROTOCOL gComponentName; extern >>>>> +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2; >>>>> + >>>>> +#endif >>>>> diff --git a/MdeModulePkg/MdeModulePkg.dsc >>>>> b/MdeModulePkg/MdeModulePkg.dsc index 20e5e8c78be0..5dd30556cb3c >>>>> 100644 >>>>> --- a/MdeModulePkg/MdeModulePkg.dsc >>>>> +++ b/MdeModulePkg/MdeModulePkg.dsc >>>>> @@ -265,6 +265,7 @@ [Components] >>>>> MdeModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf >>>>> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf >>>>> MdeModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf >>>>> + >>>>> + >>>>> MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci >>>>> Dev >>>>> + iceDxe.inf >>>>> >>>>> MdeModulePkg/Core/Dxe/DxeMain.inf { >>>>> <LibraryClasses> >>>>> -- >>>>> 2.7.4 >>>> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Fri, Dec 02, 2016 at 03:07:39PM +0000, Ard Biesheuvel wrote: > > > On 2 Dec 2016, at 14:40, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > > > On Fri, Dec 02, 2016 at 12:54:45PM +0000, Ni, Ruiyu wrote: > >>>>> + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; > >>>>> + Dev->BarCount++; > >>>>> + > >>>>> + if (Desc->AddrSpaceGranularity == 64) { > >>>>> + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; > >>>>> + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc->AddrRangeMin >> 32); > >>>> > >>>> 1. You need to use RShiftU64 otherwise the above code will break the > >>>> build process in 32bit. > >>> > >>> I don't understand how that could cause breakage (and experiments with > >>> IA32 and ARM fail to reproduce it with GCC). > >>> > >>> - Bar[x] is an array of UINT32 > >>> - Desc->AddrRangeMin is UINT64 > >>> - The result of (Desc->AddrRangeMin >> 32) is explicitly cast to > >>> UINT32 (the type of Bar[x]). > >>> > >>> Is the problem related to the shift value being signed? > >>> Or does some other compiler refuse to cast a UINT64 to a UINT32? > >> > >> Desc->AddrRangeMin is UINT64. My experience tells me that > >> LShiftU64 or RShiftU64 should be used for shifting operation > >> on UINT64. > > > > This is the bit that confuses me. There is no such automatic > > limitation in the C language (if long long is supported at all). So if > > we're working around some bug in a specific compiler - I am happy to > > do so. I just prefer knowing why rather than cargo-culting. > > > >> I guess the GCC might cleverly recognizes the ">>32" actually > >> picks up the high-32 bits so in 32bit environment, it just uses > >> value of the register which stores the high-32 bits. > >> Can you check the assembly code GCC generates? > >> or simply can you change ">>32" to ">>31" or ">>33" to see > >> what happens? > > > > There is no cleverness required, it's just a defined operation on a > > base type. But sure, for example: > > > > unsigned int shift(long long val) > > { > > return (val >> 33); > > } > > > > decodes as > > --- > > 0000000000000000 <shift>: > > 0: 48 89 f8 mov %rdi,%rax > > 3: 48 c1 f8 21 sar $0x21,%rax > > 7: c3 retq > > --- > > (in x86_64, using gcc -O2 to get rid of some redundant stack > > operations) > > > > The only thing that changes if the shift value is modified is that the > > 0x21 is changed accordingly. > > > > Change it to an inline constant, not much changes: > > > > unsigned int shiftconst(void) > > { > > unsigned long long var = 0xaaaabbbbccccddddULL; > > return (var >> 31); > > } > > > > decodes as > > --- > > 0000000000000000 <shiftconst>: > > 0: 55 push %rbp > > 1: 48 89 e5 mov %rsp,%rbp > > 4: 48 b8 dd dd cc cc bb movabs $0xaaaabbbbccccdddd,%rax > > b: bb aa aa > > e: 48 89 45 f8 mov %rax,-0x8(%rbp) > > 12: 48 8b 45 f8 mov -0x8(%rbp),%rax > > 16: 48 c1 e8 1f shr $0x1f,%rax > > 1a: 5d pop %rbp > > 1b: c3 retq > > --- > > (with -O0, to prevent the compiler from just doing the arithmetic > > compile time) > > > > Both also compile correctly for IA32 with -m32. > > > > The point is that there is nothing clever here for the compiler to do: > > casting a 64-bit int to a 32-bit int is an operation that discards the > > top 32 bits - but the behaviour is entirely defined. > > > > In fact, all architectures other than IA32/ARM use the Math64.c > > implementation of RShiftU64, which simply does the shift. > > And ARM does it only for GCC. I'm not sure why, I can't imagine a GCC > > version that would do anything more complicated than > > --- > > 00000000 <shift>: > > 0: e1a000c1 asr r0, r1, #1 > > 4: e12fff1e bx lr > > --- > > (and that's with -march=armv4t onwards) > > That's a 32-bit operand. Shifting a 64-bit value will result in a > call to a compiler intrinsic, which is kind of the point of Riuyu's > remark No, it's not. As stated, that is the output from compiling: unsigned int shift(long long val) { return (val >> 33); } Unless you are claiming long long is 32-bit on ARM. Although I did mess up and make it signed :) When fixed, the code generation is identical but with an lsr instead. The compiler just operates directly on the top 32-bits (in r1), since the ones in r0 are irrelevant to the result. Change the shift to 31 and the compiler doesn't do anything cute, so the code is slightly more obvious: --- 00000000 <shift>: 0: e1a00fa0 lsr r0, r0, #31 4: e1800081 orr r0, r0, r1, lsl #1 8: e12fff1e bx lr --- But even if there were intrinsics generated, shouldn't we support the intrinsic instead of massaging the C code and hope we can prevent it from being generated. / Leif > > > > So if we are saying that there are certain specific compilers that do > > not support 64-bit arithmetic for 32-bit architectures, and that those > > compilers are still supported by BaseTools - then I'm fine with that. > > > > But I don't want to replace functional C code with abstraction > > functions without a proper understanding of where it would be > > problematic. > > > > Regards, > > > > Leif > > > >> > >>> > >>> Regards, > >>> > >>> Leif > >>> > >>>>> + } > >>>>> + } > >>>>> +} > >>>>> diff --git > >>>>> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > >>>>> PciDeviceIo.h > >>>>> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > >>>>> PciDeviceIo.h > >>>>> new file mode 100644 > >>>>> index 000000000000..bc0a3d3258f9 > >>>>> --- /dev/null > >>>>> +++ > >>>>> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable > >>>>> Pc > >>>>> +++ iDeviceIo.h > >>>>> @@ -0,0 +1,84 @@ > >>>>> +/** @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. > >>>>> + > >>>>> +**/ > >>>>> + > >>>>> +#ifndef __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > >>>>> +#define __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ > >>>>> + > >>>>> +#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/ComponentName.h> > >>>>> +#include <Protocol/NonDiscoverableDevice.h> > >>>>> +#include <Protocol/PciIo.h> > >>>>> + > >>>>> +#define NON_DISCOVERABLE_PCI_DEVICE_SIG SIGNATURE_32 ('P', 'P', 'I', > >>>>> +'D') > >>>>> + > >>>>> +#define NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(PciIoPointer) \ > >>>>> + CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \ > >>>>> + NON_DISCOVERABLE_PCI_DEVICE_SIG) > >>>>> + > >>>>> +#define PCI_ID_VENDOR_UNKNOWN 0xffff > >>>>> +#define PCI_ID_DEVICE_DONTCARE 0x0000 > >>>>> + > >>>>> +#define PCI_MAX_BARS 6 > >>>>> + > >>>>> +typedef struct { > >>>>> + UINT32 Signature; > >>>>> + // > >>>>> + // The bound non-discoverable device protocol instance > >>>>> + // > >>>>> + NON_DISCOVERABLE_DEVICE *Device; > >>>>> + // > >>>>> + // 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 first virtual BAR to assign based on the resources described > >>>>> + // by the non-discoverable device. > >>>>> + // > >>>>> + UINT32 BarOffset; > >>>>> + // > >>>>> + // The number of virtual BARs we expose based on the number of > >>>>> + // resources > >>>>> + // > >>>>> + UINT32 BarCount; > >>>>> + // > >>>>> + // The PCI I/O attributes for this device > >>>>> + // > >>>>> + UINT64 Attributes; > >>>>> + // > >>>>> + // Whether this device has been enabled > >>>>> + // > >>>>> + BOOLEAN Enabled; > >>>>> +} NON_DISCOVERABLE_PCI_DEVICE; > >>>>> + > >>>>> +VOID > >>>>> +InitializePciIoProtocol ( > >>>>> + NON_DISCOVERABLE_PCI_DEVICE *Device > >>>>> + ); > >>>>> + > >>>>> +extern EFI_COMPONENT_NAME_PROTOCOL gComponentName; extern > >>>>> +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2; > >>>>> + > >>>>> +#endif > >>>>> diff --git a/MdeModulePkg/MdeModulePkg.dsc > >>>>> b/MdeModulePkg/MdeModulePkg.dsc index 20e5e8c78be0..5dd30556cb3c > >>>>> 100644 > >>>>> --- a/MdeModulePkg/MdeModulePkg.dsc > >>>>> +++ b/MdeModulePkg/MdeModulePkg.dsc > >>>>> @@ -265,6 +265,7 @@ [Components] > >>>>> MdeModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf > >>>>> MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf > >>>>> MdeModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf > >>>>> + > >>>>> + > >>>>> MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci > >>>>> Dev > >>>>> + iceDxe.inf > >>>>> > >>>>> MdeModulePkg/Core/Dxe/DxeMain.inf { > >>>>> <LibraryClasses> > >>>>> -- > >>>>> 2.7.4 > >>>> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 2 December 2016 at 15:56, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Fri, Dec 02, 2016 at 03:07:39PM +0000, Ard Biesheuvel wrote: >> >> > On 2 Dec 2016, at 14:40, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> > >> > On Fri, Dec 02, 2016 at 12:54:45PM +0000, Ni, Ruiyu wrote: >> >>>>> + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; >> >>>>> + Dev->BarCount++; >> >>>>> + >> >>>>> + if (Desc->AddrSpaceGranularity == 64) { >> >>>>> + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; >> >>>>> + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc->AddrRangeMin >> 32); >> >>>> >> >>>> 1. You need to use RShiftU64 otherwise the above code will break the >> >>>> build process in 32bit. >> >>> >> >>> I don't understand how that could cause breakage (and experiments with >> >>> IA32 and ARM fail to reproduce it with GCC). >> >>> >> >>> - Bar[x] is an array of UINT32 >> >>> - Desc->AddrRangeMin is UINT64 >> >>> - The result of (Desc->AddrRangeMin >> 32) is explicitly cast to >> >>> UINT32 (the type of Bar[x]). >> >>> >> >>> Is the problem related to the shift value being signed? >> >>> Or does some other compiler refuse to cast a UINT64 to a UINT32? >> >> >> >> Desc->AddrRangeMin is UINT64. My experience tells me that >> >> LShiftU64 or RShiftU64 should be used for shifting operation >> >> on UINT64. >> > >> > This is the bit that confuses me. There is no such automatic >> > limitation in the C language (if long long is supported at all). So if >> > we're working around some bug in a specific compiler - I am happy to >> > do so. I just prefer knowing why rather than cargo-culting. >> > >> >> I guess the GCC might cleverly recognizes the ">>32" actually >> >> picks up the high-32 bits so in 32bit environment, it just uses >> >> value of the register which stores the high-32 bits. >> >> Can you check the assembly code GCC generates? >> >> or simply can you change ">>32" to ">>31" or ">>33" to see >> >> what happens? >> > >> > There is no cleverness required, it's just a defined operation on a >> > base type. But sure, for example: >> > >> > unsigned int shift(long long val) >> > { >> > return (val >> 33); >> > } >> > >> > decodes as >> > --- >> > 0000000000000000 <shift>: >> > 0: 48 89 f8 mov %rdi,%rax >> > 3: 48 c1 f8 21 sar $0x21,%rax >> > 7: c3 retq >> > --- >> > (in x86_64, using gcc -O2 to get rid of some redundant stack >> > operations) >> > >> > The only thing that changes if the shift value is modified is that the >> > 0x21 is changed accordingly. >> > >> > Change it to an inline constant, not much changes: >> > >> > unsigned int shiftconst(void) >> > { >> > unsigned long long var = 0xaaaabbbbccccddddULL; >> > return (var >> 31); >> > } >> > >> > decodes as >> > --- >> > 0000000000000000 <shiftconst>: >> > 0: 55 push %rbp >> > 1: 48 89 e5 mov %rsp,%rbp >> > 4: 48 b8 dd dd cc cc bb movabs $0xaaaabbbbccccdddd,%rax >> > b: bb aa aa >> > e: 48 89 45 f8 mov %rax,-0x8(%rbp) >> > 12: 48 8b 45 f8 mov -0x8(%rbp),%rax >> > 16: 48 c1 e8 1f shr $0x1f,%rax >> > 1a: 5d pop %rbp >> > 1b: c3 retq >> > --- >> > (with -O0, to prevent the compiler from just doing the arithmetic >> > compile time) >> > >> > Both also compile correctly for IA32 with -m32. >> > >> > The point is that there is nothing clever here for the compiler to do: >> > casting a 64-bit int to a 32-bit int is an operation that discards the >> > top 32 bits - but the behaviour is entirely defined. >> > >> > In fact, all architectures other than IA32/ARM use the Math64.c >> > implementation of RShiftU64, which simply does the shift. >> > And ARM does it only for GCC. I'm not sure why, I can't imagine a GCC >> > version that would do anything more complicated than >> > --- >> > 00000000 <shift>: >> > 0: e1a000c1 asr r0, r1, #1 >> > 4: e12fff1e bx lr >> > --- >> > (and that's with -march=armv4t onwards) >> >> That's a 32-bit operand. Shifting a 64-bit value will result in a >> call to a compiler intrinsic, which is kind of the point of Riuyu's >> remark > > No, it's not. > As stated, that is the output from compiling: > > unsigned int shift(long long val) > { > return (val >> 33); > } > > Unless you are claiming long long is 32-bit on ARM. > > Although I did mess up and make it signed :) > When fixed, the code generation is identical but with an lsr instead. > > The compiler just operates directly on the top 32-bits (in r1), since > the ones in r0 are irrelevant to the result. > OK, but that does mean it is performing optimization up to some point. So right shifts of 64-bit quantities by 32 bits or more will be 'optimized' by GCC into something that does not rely on the intrinsics. But this is not universally true for all toolchains (and variable shifts are probably treated differently as well) > Change the shift to 31 and the compiler doesn't do anything cute, so > the code is slightly more obvious: > --- > 00000000 <shift>: > 0: e1a00fa0 lsr r0, r0, #31 > 4: e1800081 orr r0, r0, r1, lsl #1 > 8: e12fff1e bx lr > --- > > But even if there were intrinsics generated, shouldn't we support the > intrinsic instead of massaging the C code and hope we can prevent it > from being generated. > This is exactly the issue at hand: EDK2 typically does *not* rely on such intrinsics, even if we were forced to add them for ARM because GCC cannot be instructed to inhibit such calls. Actually, I do not agree with the EDK2 position, i.e., I think there is no issue relying on routines that are typically provided by the C runtime. But this is MdeModulePkg, so it is not up to us. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Fri, Dec 02, 2016 at 06:42:58PM +0000, Ard Biesheuvel wrote: > >> > The point is that there is nothing clever here for the compiler to do: > >> > casting a 64-bit int to a 32-bit int is an operation that discards the > >> > top 32 bits - but the behaviour is entirely defined. > >> > > >> > In fact, all architectures other than IA32/ARM use the Math64.c > >> > implementation of RShiftU64, which simply does the shift. > >> > And ARM does it only for GCC. I'm not sure why, I can't imagine a GCC > >> > version that would do anything more complicated than > >> > --- > >> > 00000000 <shift>: > >> > 0: e1a000c1 asr r0, r1, #1 > >> > 4: e12fff1e bx lr > >> > --- > >> > (and that's with -march=armv4t onwards) > >> > >> That's a 32-bit operand. Shifting a 64-bit value will result in a > >> call to a compiler intrinsic, which is kind of the point of Riuyu's > >> remark > > > > No, it's not. > > As stated, that is the output from compiling: > > > > unsigned int shift(long long val) > > { > > return (val >> 33); > > } > > > > Unless you are claiming long long is 32-bit on ARM. > > > > Although I did mess up and make it signed :) > > When fixed, the code generation is identical but with an lsr instead. > > > > The compiler just operates directly on the top 32-bits (in r1), since > > the ones in r0 are irrelevant to the result. > > OK, but that does mean it is performing optimization up to some point. > So right shifts of 64-bit quantities by 32 bits or more will be > 'optimized' by GCC into something that does not rely on the > intrinsics. But this is not universally true for all toolchains And I'm OK with a statement of "we can't do this because version X of toolchain Y does not support it". But I am much less happy for people to introduce workarounds in functional code because of an issue that may no longer be relevant for any toolchains actually in use (or even supported by BaseTools). At which point can we know this problem goes away and we can program in regular C instead, if we decide to obsolete some extremely outdated toolchains? > (and variable shifts are probably treated differently as well) It has no effect on intrinsics being required or not, it simply requires some more stack shuffling. But yes, that removes all options for compiler shortcuts. > > Change the shift to 31 and the compiler doesn't do anything cute, so > > the code is slightly more obvious: > > --- > > 00000000 <shift>: > > 0: e1a00fa0 lsr r0, r0, #31 > > 4: e1800081 orr r0, r0, r1, lsl #1 > > 8: e12fff1e bx lr > > --- > > > > But even if there were intrinsics generated, shouldn't we support the > > intrinsic instead of massaging the C code and hope we can prevent it > > from being generated. > > This is exactly the issue at hand: EDK2 typically does *not* rely on > such intrinsics, even if we were forced to add them for ARM because > GCC cannot be instructed to inhibit such calls. > > Actually, I do not agree with the EDK2 position, i.e., I think there > is no issue relying on routines that are typically provided by the C > runtime. But this is MdeModulePkg, so it is not up to us. Indeed it is not. But this does not sound like a topic we want per-package rules on. Regards, Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c new file mode 100644 index 000000000000..6e51d00fe434 --- /dev/null +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c @@ -0,0 +1,75 @@ +/** @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 "NonDiscoverablePciDeviceIo.h" + +// +// 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. +// + +STATIC +EFI_UNICODE_STRING_TABLE mDriverNameTable[] = { + { "eng;en", L"PCI I/O protocol emulation driver for non-discoverable devices" }, + { NULL, NULL } +}; + +EFI_COMPONENT_NAME_PROTOCOL gComponentName; + +STATIC +EFI_STATUS +EFIAPI +NonDiscoverablePciGetDriverName ( + 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 +NonDiscoverablePciGetDeviceName ( + IN EFI_COMPONENT_NAME_PROTOCOL *This, + IN EFI_HANDLE DeviceHandle, + IN EFI_HANDLE ChildHandle, + IN CHAR8 *Language, + OUT CHAR16 **ControllerName + ) +{ + return EFI_UNSUPPORTED; +} + +EFI_COMPONENT_NAME_PROTOCOL gComponentName = { + &NonDiscoverablePciGetDriverName, + &NonDiscoverablePciGetDeviceName, + "eng" // SupportedLanguages, ISO 639-2 language codes +}; + +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2 = { + (EFI_COMPONENT_NAME2_GET_DRIVER_NAME) &NonDiscoverablePciGetDriverName, + (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME) &NonDiscoverablePciGetDeviceName, + "en" // SupportedLanguages, RFC 4646 language codes +}; diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c new file mode 100644 index 000000000000..ee765d7a5d9c --- /dev/null +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c @@ -0,0 +1,235 @@ +/** @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 "NonDiscoverablePciDeviceIo.h" + +#include <Protocol/DriverBinding.h> + +// +// We only support the following device types +// +STATIC +CONST EFI_GUID * CONST +SupportedNonDiscoverableDevices [] = { + &gEdkiiNonDiscoverableAhciDeviceGuid, + &gEdkiiNonDiscoverableEhciDeviceGuid, + &gEdkiiNonDiscoverableNvmeDeviceGuid, + &gEdkiiNonDiscoverableOhciDeviceGuid, + &gEdkiiNonDiscoverableSdhciDeviceGuid, + &gEdkiiNonDiscoverableUfsDeviceGuid, + &gEdkiiNonDiscoverableUhciDeviceGuid, + &gEdkiiNonDiscoverableXhciDeviceGuid, +}; + +// +// 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 +NonDiscoverablePciDeviceSupported ( + IN EFI_DRIVER_BINDING_PROTOCOL *This, + IN EFI_HANDLE DeviceHandle, + IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath + ) +{ + NON_DISCOVERABLE_DEVICE *Device; + EFI_STATUS Status; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + INTN Idx; + + Status = gBS->OpenProtocol (DeviceHandle, + &gEdkiiNonDiscoverableDeviceProtocolGuid, (VOID **)&Device, + This->DriverBindingHandle, DeviceHandle, + EFI_OPEN_PROTOCOL_BY_DRIVER); + if (EFI_ERROR (Status)) { + return Status; + } + + // + // Restricted to DMA coherent for now + // + Status = EFI_UNSUPPORTED; + if (Device->DmaType != NonDiscoverableDeviceDmaTypeCoherent) { + goto CloseProtocol; + } + + for (Idx = 0; Idx < ARRAY_SIZE (SupportedNonDiscoverableDevices); Idx++) { + if (CompareGuid (Device->Type, SupportedNonDiscoverableDevices [Idx])) { + Status = EFI_SUCCESS; + break; + } + } + + if (EFI_ERROR (Status)) { + goto CloseProtocol; + } + + // + // We only support MMIO devices, so iterate over the resources to ensure + // that they only describe things that we can handle + // + for (Desc = Device->Resources; Desc->Desc != ACPI_END_TAG_DESCRIPTOR; + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { + if (Desc->Desc != ACPI_ADDRESS_SPACE_DESCRIPTOR || + Desc->ResType != ACPI_ADDRESS_SPACE_TYPE_MEM) { + Status = EFI_UNSUPPORTED; + break; + } + } + +CloseProtocol: + gBS->CloseProtocol (DeviceHandle, &gEdkiiNonDiscoverableDeviceProtocolGuid, + This->DriverBindingHandle, DeviceHandle); + + return Status; +} + +STATIC +EFI_STATUS +EFIAPI +NonDiscoverablePciDeviceStart ( + IN EFI_DRIVER_BINDING_PROTOCOL *This, + IN EFI_HANDLE DeviceHandle, + IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath + ) +{ + NON_DISCOVERABLE_PCI_DEVICE *Dev; + EFI_STATUS Status; + + Dev = AllocateZeroPool (sizeof *Dev); + if (Dev == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + Status = gBS->OpenProtocol (DeviceHandle, + &gEdkiiNonDiscoverableDeviceProtocolGuid, + (VOID **)&Dev->Device, 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 = NON_DISCOVERABLE_PCI_DEVICE_SIG; + Status = gBS->InstallProtocolInterface (&DeviceHandle, &gEfiPciIoProtocolGuid, + EFI_NATIVE_INTERFACE, &Dev->PciIo); + if (EFI_ERROR (Status)) { + goto CloseProtocol; + } + + return EFI_SUCCESS; + +CloseProtocol: + gBS->CloseProtocol (DeviceHandle, &gEdkiiNonDiscoverableDeviceProtocolGuid, + This->DriverBindingHandle, DeviceHandle); + +FreeDev: + FreePool (Dev); + + return Status; +} + + +STATIC +EFI_STATUS +EFIAPI +NonDiscoverablePciDeviceStop ( + 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; + NON_DISCOVERABLE_PCI_DEVICE *Dev; + + Status = gBS->OpenProtocol (DeviceHandle, &gEfiPciIoProtocolGuid, + (VOID **)&PciIo, This->DriverBindingHandle, DeviceHandle, + EFI_OPEN_PROTOCOL_GET_PROTOCOL); + if (EFI_ERROR (Status)) { + return Status; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_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, &gEdkiiNonDiscoverableDeviceProtocolGuid, + 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 = { + &NonDiscoverablePciDeviceSupported, + &NonDiscoverablePciDeviceStart, + &NonDiscoverablePciDeviceStop, + 0x10, // Version, must be in [0x10 .. 0xFFFFFFEF] for IHV-developed drivers + NULL, + NULL +}; + +// +// Entry point of this driver. +// +EFI_STATUS +EFIAPI +NonDiscoverablePciDeviceDxeEntryPoint ( + IN EFI_HANDLE ImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +{ + return EfiLibInstallDriverBindingComponentName2 ( + ImageHandle, + SystemTable, + &gDriverBinding, + ImageHandle, + &gComponentName, + &gComponentName2 + ); +} diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf new file mode 100644 index 000000000000..996fe310e0e3 --- /dev/null +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf @@ -0,0 +1,52 @@ +## @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 = 0x00010019 + BASE_NAME = NonDiscoverablePciDeviceDxe + FILE_GUID = 71fd84cd-353b-464d-b7a4-6ea7b96995cb + MODULE_TYPE = UEFI_DRIVER + VERSION_STRING = 1.0 + ENTRY_POINT = NonDiscoverablePciDeviceDxeEntryPoint + +[Sources] + ComponentName.c + NonDiscoverablePciDeviceDxe.c + NonDiscoverablePciDeviceIo.c + NonDiscoverablePciDeviceIo.h + +[Packages] + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec + +[LibraryClasses] + BaseMemoryLib + DebugLib + MemoryAllocationLib + UefiBootServicesTableLib + UefiDriverEntryPoint + UefiLib + +[Protocols] + gEfiPciIoProtocolGuid ## BY_START + gEdkiiNonDiscoverableDeviceProtocolGuid ## TO_START + +[Guids] + gEdkiiNonDiscoverableAhciDeviceGuid + gEdkiiNonDiscoverableEhciDeviceGuid + gEdkiiNonDiscoverableNvmeDeviceGuid + gEdkiiNonDiscoverableOhciDeviceGuid + gEdkiiNonDiscoverableSdhciDeviceGuid + gEdkiiNonDiscoverableUfsDeviceGuid + gEdkiiNonDiscoverableUhciDeviceGuid + gEdkiiNonDiscoverableXhciDeviceGuid diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c new file mode 100644 index 000000000000..fd59267a8d66 --- /dev/null +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c @@ -0,0 +1,797 @@ +/** @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 "NonDiscoverablePciDeviceIo.h" + +#include <IndustryStandard/Acpi.h> + +#include <Protocol/PciRootBridgeIo.h> + +typedef struct { + EFI_PHYSICAL_ADDRESS AllocAddress; + VOID *HostAddress; + EFI_PCI_IO_PROTOCOL_OPERATION Operation; + UINTN NumberOfBytes; +} NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO; + +// +// Get the resource associated with BAR number 'BarIndex'. +// +STATIC +EFI_STATUS +GetBarResource ( + IN NON_DISCOVERABLE_PCI_DEVICE *Dev, + IN UINT8 BarIndex, + OUT EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR **Descriptor + ) +{ + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + + if (BarIndex < Dev->BarOffset) { + return EFI_NOT_FOUND; + } + + BarIndex -= Dev->BarOffset; + + for (Desc = Dev->Device->Resources; + Desc->Desc != ACPI_END_TAG_DESCRIPTOR; + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { + + if (BarIndex == 0) { + *Descriptor = Desc; + return EFI_SUCCESS; + } + + BarIndex -= 1; + } + return EFI_NOT_FOUND; +} + +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 + ) +{ + volatile UINT8 *Dst8; + volatile UINT16 *Dst16; + volatile UINT32 *Dst32; + volatile CONST UINT8 *Src8; + volatile CONST UINT16 *Src16; + volatile CONST UINT32 *Src32; + + // + // 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 + ) +{ + NON_DISCOVERABLE_PCI_DEVICE *Dev; + UINTN AlignMask; + VOID *Address; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + EFI_STATUS Status; + + if (Buffer == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + + // + // Only allow accesses to the BARs we emulate + // + Status = GetBarResource (Dev, BarIndex, &Desc); + if (EFI_ERROR (Status)) { + return Status; + } + + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { + return EFI_UNSUPPORTED; + } + + Address = (VOID *)(UINTN)(Desc->AddrRangeMin + 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 + ) +{ + NON_DISCOVERABLE_PCI_DEVICE *Dev; + UINTN AlignMask; + VOID *Address; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + EFI_STATUS Status; + + if (Buffer == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + + // + // Only allow accesses to the BARs we emulate + // + Status = GetBarResource (Dev, BarIndex, &Desc); + if (EFI_ERROR (Status)) { + return Status; + } + + if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) { + return EFI_UNSUPPORTED; + } + + Address = (VOID *)(UINTN)(Desc->AddrRangeMin + 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 + ) +{ + NON_DISCOVERABLE_PCI_DEVICE *Dev; + VOID *Address; + UINTN Length; + + if (Width < 0 || Width >= EfiPciIoWidthMaximum || Buffer == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + Address = (UINT8 *)&Dev->ConfigSpace + Offset; + Length = Count << ((UINTN)Width & 0x3); + + if (Offset + Length > sizeof (Dev->ConfigSpace)) { + // + // Read all zeroes for config space accesses beyond the first + // 64 bytes + // + Length -= sizeof (Dev->ConfigSpace) - Offset; + ZeroMem ((UINT8 *)Buffer + sizeof (Dev->ConfigSpace) - Offset, Length); + + Count -= Length >> ((UINTN)Width & 0x3); + } + 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 + ) +{ + NON_DISCOVERABLE_PCI_DEVICE *Dev; + VOID *Address; + + if (Width < 0 || Width >= EfiPciIoWidthMaximum || Buffer == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + Address = (UINT8 *)&Dev->ConfigSpace + Offset; + + if (Offset + (Count << ((UINTN)Width & 0x3)) > sizeof (Dev->ConfigSpace)) { + return EFI_UNSUPPORTED; + } + + 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 + ) +{ + NON_DISCOVERABLE_PCI_DEVICE *Dev; + EFI_STATUS Status; + NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO *MapInfo; + + // + // If HostAddress exceeds 4 GB, and this device does not support 64-bit DMA + // addressing, we need to allocate a bounce buffer and copy over the data. + // + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0 && + (UINTN)HostAddress + *NumberOfBytes > SIZE_4GB) { + + // + // Bounce buffering is not possible for consistent mappings + // + if (Operation == EfiPciIoOperationBusMasterCommonBuffer) { + return EFI_UNSUPPORTED; + } + + MapInfo = AllocatePool (sizeof *MapInfo); + if (MapInfo == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + MapInfo->AllocAddress = MAX_UINT32; + 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)) { + // + // If we fail here, it is likely because the system has no memory below + // 4 GB to begin with. There is not much we can do about that other than + // fail the map request. + // + FreePool (MapInfo); + return EFI_DEVICE_ERROR; + } + if (Operation == EfiPciIoOperationBusMasterRead) { + gBS->CopyMem ((VOID *)(UINTN)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 + ) +{ + NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO *MapInfo; + + MapInfo = Mapping; + if (MapInfo != NULL) { + if (MapInfo->Operation == EfiPciIoOperationBusMasterWrite) { + gBS->CopyMem (MapInfo->HostAddress, (VOID *)(UINTN)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 + ) +{ + NON_DISCOVERABLE_PCI_DEVICE *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)) != 0) { + return EFI_UNSUPPORTED; + } + + // + // Allocate below 4 GB if the dual address cycle attribute has not + // been set. If the system has no memory available below 4 GB, there + // is little we can do except propagate the error. + // + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) { + AllocAddress = MAX_UINT32; + 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; + } + + *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 + ) +{ + NON_DISCOVERABLE_PCI_DEVICE *Dev; + BOOLEAN Enable; + + Dev = NON_DISCOVERABLE_PCI_DEVICE_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 device specific initialization now. + // + if (Enable && !Dev->Enabled && Dev->Device->Initialize != NULL) { + Dev->Device->Initialize (Dev->Device); + 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 + ) +{ + NON_DISCOVERABLE_PCI_DEVICE *Dev; + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor, *BarDesc; + EFI_ACPI_END_TAG_DESCRIPTOR *End; + EFI_STATUS Status; + + if (Supports == NULL && Resources == NULL) { + return EFI_INVALID_PARAMETER; + } + + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); + + Status = GetBarResource (Dev, BarIndex, &BarDesc); + if (EFI_ERROR (Status)) { + return Status; + } + + // + // Don't expose any configurable attributes for our emulated BAR + // + if (Supports != NULL) { + *Supports = 0; + } + + if (Resources != NULL) { + Descriptor = AllocatePool (sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR)); + if (Descriptor == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + CopyMem (Descriptor, BarDesc, sizeof *Descriptor); + + End = (EFI_ACPI_END_TAG_DESCRIPTOR *) (Descriptor + 1); + End->Desc = ACPI_END_TAG_DESCRIPTOR; + End->Checksum = 0; + + *Resources = Descriptor; + } + return EFI_SUCCESS; +} + +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 ( + NON_DISCOVERABLE_PCI_DEVICE *Dev + ) +{ + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Desc; + INTN Idx; + + InitializeListHead (&Dev->UncachedAllocationList); + + Dev->ConfigSpace.Hdr.VendorId = PCI_ID_VENDOR_UNKNOWN; + Dev->ConfigSpace.Hdr.DeviceId = PCI_ID_DEVICE_DONTCARE; + + // Copy protocol structure + CopyMem(&Dev->PciIo, &PciIoTemplate, sizeof PciIoTemplate); + + if (CompareGuid (Dev->Device->Type, &gEdkiiNonDiscoverableAhciDeviceGuid)) { + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_MASS_STORAGE_AHCI; + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_MASS_STORAGE_SATADPA; + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_MASS_STORAGE; + Dev->BarOffset = 5; + } else if (CompareGuid (Dev->Device->Type, + &gEdkiiNonDiscoverableEhciDeviceGuid)) { + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_EHCI; + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB; + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL; + Dev->BarOffset = 0; + } else if (CompareGuid (Dev->Device->Type, + &gEdkiiNonDiscoverableNvmeDeviceGuid)) { + Dev->ConfigSpace.Hdr.ClassCode[0] = 0x2; // PCI_IF_NVMHCI + Dev->ConfigSpace.Hdr.ClassCode[1] = 0x8; // PCI_CLASS_MASS_STORAGE_NVM + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_MASS_STORAGE; + Dev->BarOffset = 0; + } else if (CompareGuid (Dev->Device->Type, + &gEdkiiNonDiscoverableOhciDeviceGuid)) { + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_OHCI; + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB; + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL; + Dev->BarOffset = 0; + } else if (CompareGuid (Dev->Device->Type, + &gEdkiiNonDiscoverableSdhciDeviceGuid)) { + Dev->ConfigSpace.Hdr.ClassCode[0] = 0x0; // don't care + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_SUBCLASS_SD_HOST_CONTROLLER; + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SYSTEM_PERIPHERAL; + Dev->BarOffset = 0; + } else if (CompareGuid (Dev->Device->Type, + &gEdkiiNonDiscoverableXhciDeviceGuid)) { + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_XHCI; + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB; + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL; + Dev->BarOffset = 0; + } else if (CompareGuid (Dev->Device->Type, + &gEdkiiNonDiscoverableUhciDeviceGuid)) { + Dev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_UHCI; + Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB; + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL; + Dev->BarOffset = 0; + } else if (CompareGuid (Dev->Device->Type, + &gEdkiiNonDiscoverableUfsDeviceGuid)) { + Dev->ConfigSpace.Hdr.ClassCode[0] = 0x0; // don't care + Dev->ConfigSpace.Hdr.ClassCode[1] = 0x9; // UFS controller subclass; + Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_MASS_STORAGE; + Dev->BarOffset = 0; + } else { + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER); + } + + // + // Iterate over the resources to populate the virtual BARs + // + Idx = Dev->BarOffset; + for (Desc = Dev->Device->Resources, Dev->BarCount = 0; + Desc->Desc != ACPI_END_TAG_DESCRIPTOR; + Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) { + + ASSERT (Desc->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR); + ASSERT (Desc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM); + + if (Idx >= PCI_MAX_BARS || + (Idx == PCI_MAX_BARS - 1 && Desc->AddrSpaceGranularity == 64)) { + DEBUG ((DEBUG_ERROR, + "%a: resource count exceeds number of emulated BARs\n", + __FUNCTION__)); + ASSERT (FALSE); + break; + } + + Dev->ConfigSpace.Device.Bar[Idx] = (UINT32)Desc->AddrRangeMin; + Dev->BarCount++; + + if (Desc->AddrSpaceGranularity == 64) { + Dev->ConfigSpace.Device.Bar[Idx] |= 0x4; + Dev->ConfigSpace.Device.Bar[++Idx] = (UINT32)(Desc->AddrRangeMin >> 32); + } + } +} diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h new file mode 100644 index 000000000000..bc0a3d3258f9 --- /dev/null +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h @@ -0,0 +1,84 @@ +/** @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. + +**/ + +#ifndef __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ +#define __NON_DISCOVERABLE_PCI_DEVICE_IO_H__ + +#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/ComponentName.h> +#include <Protocol/NonDiscoverableDevice.h> +#include <Protocol/PciIo.h> + +#define NON_DISCOVERABLE_PCI_DEVICE_SIG SIGNATURE_32 ('P', 'P', 'I', 'D') + +#define NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(PciIoPointer) \ + CR (PciIoPointer, NON_DISCOVERABLE_PCI_DEVICE, PciIo, \ + NON_DISCOVERABLE_PCI_DEVICE_SIG) + +#define PCI_ID_VENDOR_UNKNOWN 0xffff +#define PCI_ID_DEVICE_DONTCARE 0x0000 + +#define PCI_MAX_BARS 6 + +typedef struct { + UINT32 Signature; + // + // The bound non-discoverable device protocol instance + // + NON_DISCOVERABLE_DEVICE *Device; + // + // 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 first virtual BAR to assign based on the resources described + // by the non-discoverable device. + // + UINT32 BarOffset; + // + // The number of virtual BARs we expose based on the number of + // resources + // + UINT32 BarCount; + // + // The PCI I/O attributes for this device + // + UINT64 Attributes; + // + // Whether this device has been enabled + // + BOOLEAN Enabled; +} NON_DISCOVERABLE_PCI_DEVICE; + +VOID +InitializePciIoProtocol ( + NON_DISCOVERABLE_PCI_DEVICE *Device + ); + +extern EFI_COMPONENT_NAME_PROTOCOL gComponentName; +extern EFI_COMPONENT_NAME2_PROTOCOL gComponentName2; + +#endif diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc index 20e5e8c78be0..5dd30556cb3c 100644 --- a/MdeModulePkg/MdeModulePkg.dsc +++ b/MdeModulePkg/MdeModulePkg.dsc @@ -265,6 +265,7 @@ [Components] MdeModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf MdeModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf + MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf MdeModulePkg/Core/Dxe/DxeMain.inf { <LibraryClasses>
This implements support for non-discoverable PCI compatible devices, i.e, devices that are not on a PCI bus but that can be controlled by generic PCI drivers in 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. DMA is implemented as coherent, support for non-coherent DMA is implemented by a subsequent patch. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c | 75 ++ MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c | 235 ++++++ MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf | 52 ++ MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c | 797 ++++++++++++++++++++ MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h | 84 +++ MdeModulePkg/MdeModulePkg.dsc | 1 + 6 files changed, 1244 insertions(+) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel