On 23 May 2018 at 22:21, Laszlo Ersek <ler...@redhat.com> wrote: > Add a library class, and a UEFI_DRIVER lib instance, that are layered on > top of PciCapLib, and allow clients to plug an EFI_PCI_IO_PROTOCOL backend > into PciCapLib, for config space access. > > (Side note: >
Again, please retain the below. > Although the UEFI spec says that EFI_PCI_IO_PROTOCOL_CONFIG() returns > EFI_UNSUPPORTED if "[t]he address range specified by Offset, Width, and > Count is not valid for the PCI configuration header of the PCI > controller", this patch doesn't directly document the EFI_UNSUPPORTED > error code, for ProtoDevTransferConfig() and its callers > ProtoDevReadConfig() and ProtoDevWriteConfig(). Instead, the patch refers > to "unspecified error codes". The reason is that in edk2, the > PciIoConfigRead() and PciIoConfigWrite() functions [1] can also return > EFI_INVALID_PARAMETER for the above situation. > > Namely, PciIoConfigRead() and PciIoConfigWrite() first call > PciIoVerifyConfigAccess(), which indeed produces the standard > EFI_UNSUPPORTED error code, if the device's config space is exceeded. > However, if PciIoVerifyConfigAccess() passes, and we reach > RootBridgeIoPciRead() and RootBridgeIoPciWrite() [2], then > RootBridgeIoCheckParameter() can still fail, e.g. if the root bridge > doesn't support extended config space (see commit 014b472053ae3). > > For all kinds of Limit violations in IO, MMIO, and config space, > RootBridgeIoCheckParameter() returns EFI_INVALID_PARAMETER, not > EFI_UNSUPPORTED. That error code is then propagated up to, and out of, > PciIoConfigRead() and PciIoConfigWrite(). > > [1] MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c > [2] MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > ) > > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > > Notes: > v2: > - move library from MdePkg to OvmfPkg, for initial introduction > > OvmfPkg/OvmfPkg.dec | 5 + > OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf | 36 +++ > OvmfPkg/Include/Library/PciCapPciIoLib.h | 58 +++++ > OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h | 44 ++++ > OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c | 243 > ++++++++++++++++++++ > 5 files changed, 386 insertions(+) > > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec > index fbec1cfe4a8e..dc5597db4136 100644 > --- a/OvmfPkg/OvmfPkg.dec > +++ b/OvmfPkg/OvmfPkg.dec > @@ -35,6 +35,11 @@ [LibraryClasses] > # config space. > PciCapLib|Include/Library/PciCapLib.h > > + ## @libraryclass Layered on top of PciCapLib, allows clients to plug an > + # EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config > + # space access. > + PciCapPciIoLib|Include/Library/PciCapPciIoLib.h > + > ## @libraryclass Layered on top of PciCapLib, allows clients to plug a > # PciSegmentLib backend into PciCapLib, for config space > # access. > diff --git a/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf > b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf > new file mode 100644 > index 000000000000..2e14acb0ab75 > --- /dev/null > +++ b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf > @@ -0,0 +1,36 @@ > +## @file > +# Plug an EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space > access. > +# > +# Copyright (C) 2018, Red Hat, Inc. > +# > +# 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 = 1.27 > + BASE_NAME = UefiPciCapPciIoLib > + FILE_GUID = 4102F4FE-DA10-4F0F-AC18-4982ED506154 > + MODULE_TYPE = UEFI_DRIVER > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = PciCapPciIoLib > + > +[Sources] > + UefiPciCapPciIoLib.h > + UefiPciCapPciIoLib.c > + > +[Packages] > + MdePkg/MdePkg.dec > + OvmfPkg/OvmfPkg.dec > + > +[LibraryClasses] > + DebugLib > + MemoryAllocationLib > + > +[Protocols] > + gEfiPciIoProtocolGuid ## CONSUMES > diff --git a/OvmfPkg/Include/Library/PciCapPciIoLib.h > b/OvmfPkg/Include/Library/PciCapPciIoLib.h > new file mode 100644 > index 000000000000..553715fd5080 > --- /dev/null > +++ b/OvmfPkg/Include/Library/PciCapPciIoLib.h > @@ -0,0 +1,58 @@ > +/** @file > + Library class layered on top of PciCapLib that allows clients to plug an > + EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space access. > + > + Copyright (C) 2018, Red Hat, Inc. > + > + 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 __PCI_CAP_PCI_IO_LIB_H__ > +#define __PCI_CAP_PCI_IO_LIB_H__ > + > +#include <Protocol/PciIo.h> > + > +#include <Library/PciCapLib.h> > + > + > +/** > + Create a PCI_CAP_DEV object from an EFI_PCI_IO_PROTOCOL instance. The > config > + space accessors are based upon EFI_PCI_IO_PROTOCOL.Pci.Read() and > + EFI_PCI_IO_PROTOCOL.Pci.Write(). > + > + @param[in] PciIo EFI_PCI_IO_PROTOCOL representation of the PCI > device. > + > + @param[out] PciDevice The PCI_CAP_DEV object constructed as described > above. > + PciDevice can be passed to the PciCapLib APIs. > + > + @retval EFI_SUCCESS PciDevice has been constructed and output. > + > + @retval EFI_OUT_OF_RESOURCES Memory allocation failed. > +**/ > +EFI_STATUS > +EFIAPI > +PciCapPciIoDeviceInit ( > + IN EFI_PCI_IO_PROTOCOL *PciIo, > + OUT PCI_CAP_DEV **PciDevice > + ); > + > + > +/** > + Free the resources used by PciDevice. > + > + @param[in] PciDevice The PCI_CAP_DEV object to free, originally produced > by > + PciCapPciIoDeviceInit(). > +**/ > +VOID > +EFIAPI > +PciCapPciIoDeviceUninit ( > + IN PCI_CAP_DEV *PciDevice > + ); > + > +#endif // __PCI_CAP_PCI_IO_LIB_H__ > diff --git a/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h > b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h > new file mode 100644 > index 000000000000..a94f65e5a4e3 > --- /dev/null > +++ b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.h > @@ -0,0 +1,44 @@ > +/** @file > + Plug an EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space access > + -- internal macro and type definitions. > + > + Copyright (C) 2018, Red Hat, Inc. > + > + 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 __UEFI_PCI_CAP_PCI_IO_LIB_H__ > +#define __UEFI_PCI_CAP_PCI_IO_LIB_H__ > + > +#include <Library/DebugLib.h> > + > +#include <Library/PciCapPciIoLib.h> > + > +#define PROTO_DEV_SIG SIGNATURE_64 ('P', 'C', 'P', 'I', 'O', 'P', 'R', 'T') > + > +typedef struct { > + // > + // Signature identifying the derived class. > + // > + UINT64 Signature; > + // > + // Members added by the derived class, specific to the use of > + // EFI_PCI_IO_PROTOCOL. > + // > + EFI_PCI_IO_PROTOCOL *PciIo; > + // > + // Base class. > + // > + PCI_CAP_DEV BaseDevice; > +} PROTO_DEV; > + > +#define PROTO_DEV_FROM_PCI_CAP_DEV(PciDevice) \ > + CR (PciDevice, PROTO_DEV, BaseDevice, PROTO_DEV_SIG) > + > +#endif // __UEFI_PCI_CAP_PCI_IO_LIB_H__ > diff --git a/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c > b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c > new file mode 100644 > index 000000000000..84369e4dc3a8 > --- /dev/null > +++ b/OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.c > @@ -0,0 +1,243 @@ > +/** @file > + Plug an EFI_PCI_IO_PROTOCOL backend into PciCapLib, for config space > access. > + > + Copyright (C) 2018, Red Hat, Inc. > + > + This program and the accompanying materials are licensed and made available > + under the terms and conditions of the BSD License which accompanies this > + distribution. The full text of the license may be found at > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > WITHOUT > + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +**/ > + > +#include <Library/MemoryAllocationLib.h> > + > +#include "UefiPciCapPciIoLib.h" > + > + > +/** > + Transfer bytes between the config space of a given PCI device and a memory > + buffer. > + > + ProtoDevTransferConfig() performs as few config space accesses as possible > + (without attempting 64-bit wide accesses). > + > + @param[in] PciIo The EFI_PCI_IO_PROTOCOL representation of the > + PCI device. > + > + @param[in] TransferFunction The EFI_PCI_IO_PROTOCOL_CONFIG function that > + implements the transfer. The direction of the > + transfer is inherent to TransferFunction. > + TransferFunction() is required to return an > + unspecified error if any sub-transfer within > + Size bytes from ConfigOffset exceeds the > config > + space limit of the PCI device. > + > + @param[in] ConfigOffset The offset in the config space of the PCI > device > + at which the transfer should commence. > + > + @param[in,out] Buffer The memory buffer where the transfer should > + occur. > + > + @param[in] Size The number of bytes to transfer. > + > + @retval EFI_SUCCESS Size bytes have been transferred between config space > + and Buffer. > + > + @return Error codes propagated from TransferFunction(). Fewer > + than Size bytes may have been transferred. > +**/ > +STATIC > +EFI_STATUS > +ProtoDevTransferConfig ( > + IN EFI_PCI_IO_PROTOCOL *PciIo, > + IN EFI_PCI_IO_PROTOCOL_CONFIG TransferFunction, > + IN UINT16 ConfigOffset, > + IN OUT UINT8 *Buffer, > + IN UINT16 Size > + ) > +{ > + while (Size > 0) { > + EFI_PCI_IO_PROTOCOL_WIDTH Width; > + UINT16 Count; > + EFI_STATUS Status; > + UINT16 Progress; > + > + // > + // Pick the largest access size that is allowed by the remaining transfer > + // Size and by the alignment of ConfigOffset. > + // > + // When the largest access size is available, transfer as many bytes as > + // possible in one iteration of the loop. Otherwise, transfer only one > + // unit, to improve the alignment. > + // > + if (Size >= BIT2 && (ConfigOffset & (BIT2 - 1)) == 0) { Ugh. Just use '4' or sizeof(UINT32). > + Width = EfiPciIoWidthUint32; > + Count = Size >> Width; > + } else if (Size >= BIT1 && (ConfigOffset & (BIT1 - 1)) == 0) { > + Width = EfiPciIoWidthUint16; > + Count = 1; > + } else { > + Width = EfiPciIoWidthUint8; > + Count = 1; > + } > + Status = TransferFunction (PciIo, Width, ConfigOffset, Count, Buffer); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + Progress = Count << Width; > + ConfigOffset += Progress; > + Buffer += Progress; > + Size -= Progress; > + } > + return EFI_SUCCESS; > +} > + > + > +/** > + Read the config space of a given PCI device (both normal and extended). > + > + ProtoDevReadConfig() performs as few config space accesses as possible > + (without attempting 64-bit wide accesses). > + > + ProtoDevReadConfig() returns an unspecified error if accessing Size bytes > + from SourceOffset exceeds the config space limit of the PCI device. Fewer > + than Size bytes may have been read in this case. > + > + @param[in] PciDevice Implementation-specific unique > representation > + of the PCI device in the PCI hierarchy. > + > + @param[in] SourceOffset Source offset in the config space of the PCI > + device to start reading from. > + > + @param[out] DestinationBuffer Buffer to store the read data to. > + > + @param[in] Size The number of bytes to transfer. > + > + @retval RETURN_SUCCESS Size bytes have been transferred from config space > to > + DestinationBuffer. > + > + @return Error codes propagated from > + EFI_PCI_IO_PROTOCOL.Pci.Read(). Fewer than Size > bytes > + may have been read. > +**/ > +STATIC > +RETURN_STATUS > +EFIAPI > +ProtoDevReadConfig ( > + IN PCI_CAP_DEV *PciDevice, > + IN UINT16 SourceOffset, > + OUT VOID *DestinationBuffer, > + IN UINT16 Size > + ) > +{ > + PROTO_DEV *ProtoDev; > + > + ProtoDev = PROTO_DEV_FROM_PCI_CAP_DEV (PciDevice); > + return ProtoDevTransferConfig (ProtoDev->PciIo, ProtoDev->PciIo->Pci.Read, > + SourceOffset, DestinationBuffer, Size); > +} > + > + > +/** > + Write the config space of a given PCI device (both normal and extended). > + > + ProtoDevWriteConfig() performs as few config space accesses as possible > + (without attempting 64-bit wide accesses). > + > + ProtoDevWriteConfig() returns an unspecified error if accessing Size bytes > at > + DestinationOffset exceeds the config space limit of the PCI device. Fewer > + than Size bytes may have been written in this case. > + > + @param[in] PciDevice Implementation-specific unique representation > + of the PCI device in the PCI hierarchy. > + > + @param[in] DestinationOffset Destination offset in the config space of the > + PCI device to start writing at. > + > + @param[in] SourceBuffer Buffer to read the data to be stored from. > + > + @param[in] Size The number of bytes to transfer. > + > + @retval RETURN_SUCCESS Size bytes have been transferred from SourceBuffer > to > + config space. > + > + @return Error codes propagated from > + EFI_PCI_IO_PROTOCOL.Pci.Write(). Fewer than Size > + bytes may have been written. > +**/ > +STATIC > +RETURN_STATUS > +EFIAPI > +ProtoDevWriteConfig ( > + IN PCI_CAP_DEV *PciDevice, > + IN UINT16 DestinationOffset, > + IN VOID *SourceBuffer, > + IN UINT16 Size > + ) > +{ > + PROTO_DEV *ProtoDev; > + > + ProtoDev = PROTO_DEV_FROM_PCI_CAP_DEV (PciDevice); > + return ProtoDevTransferConfig (ProtoDev->PciIo, ProtoDev->PciIo->Pci.Write, > + DestinationOffset, SourceBuffer, Size); > +} > + > + > +/** > + Create a PCI_CAP_DEV object from an EFI_PCI_IO_PROTOCOL instance. The > config > + space accessors are based upon EFI_PCI_IO_PROTOCOL.Pci.Read() and > + EFI_PCI_IO_PROTOCOL.Pci.Write(). > + > + @param[in] PciIo EFI_PCI_IO_PROTOCOL representation of the PCI > device. > + > + @param[out] PciDevice The PCI_CAP_DEV object constructed as described > above. > + PciDevice can be passed to the PciCapLib APIs. > + > + @retval EFI_SUCCESS PciDevice has been constructed and output. > + > + @retval EFI_OUT_OF_RESOURCES Memory allocation failed. > +**/ > +EFI_STATUS > +EFIAPI > +PciCapPciIoDeviceInit ( > + IN EFI_PCI_IO_PROTOCOL *PciIo, > + OUT PCI_CAP_DEV **PciDevice > + ) > +{ > + PROTO_DEV *ProtoDev; > + > + ProtoDev = AllocatePool (sizeof *ProtoDev); > + if (ProtoDev == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + ProtoDev->Signature = PROTO_DEV_SIG; > + ProtoDev->PciIo = PciIo; > + ProtoDev->BaseDevice.ReadConfig = ProtoDevReadConfig; > + ProtoDev->BaseDevice.WriteConfig = ProtoDevWriteConfig; > + > + *PciDevice = &ProtoDev->BaseDevice; > + return EFI_SUCCESS; > +} > + > + > +/** > + Free the resources used by PciDevice. > + > + @param[in] PciDevice The PCI_CAP_DEV object to free, originally produced > by > + PciCapPciIoDeviceInit(). > +**/ > +VOID > +EFIAPI > +PciCapPciIoDeviceUninit ( > + IN PCI_CAP_DEV *PciDevice > + ) > +{ > + PROTO_DEV *ProtoDev; > + > + ProtoDev = PROTO_DEV_FROM_PCI_CAP_DEV (PciDevice); > + FreePool (ProtoDev); > +} > -- > 2.14.1.3.gb7cf6e02401b > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel