ArmPlatformPkg: Make PCI emulation more compliant with the UEFI spec

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ronald Cron <Ronald.Cron@arm.com>
Reviewed-by: Olivier Martin <olivier.martin@arm.com>



git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16583 6f19259b-4bc3-4df7-8a09-765794883524

ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/PciEmulation.c

@@ -159,28 +159,70 @@ PciIoIoWrite (
   return EFI_UNSUPPORTED;
 }
 
+/**
+  Enable a PCI driver to read PCI controller registers in PCI configuration space.
+
+  @param[in]      This    A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[in]      Width   Signifies the width of the memory operations.
+  @param[in]      Offset  The offset within the PCI configuration space for
+                          the PCI controller.
+  @param[in]      Count   The number of PCI configuration operations to
+                          perform. Bytes moved is Width size * Count,
+                          starting at Offset.
+
+  @param[in out]  Buffer  The destination buffer to store the results.
+
+  @retval  EFI_SUCCESS            The data was read from the PCI controller.
+  @retval  EFI_INVALID_PARAMETER  "Width" is invalid.
+  @retval  EFI_INVALID_PARAMETER  "Buffer" is NULL.
+
+**/
 EFI_STATUS
 PciIoPciRead (
-  IN EFI_PCI_IO_PROTOCOL              *This,
+  IN     EFI_PCI_IO_PROTOCOL       *This,
-  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
+  IN     EFI_PCI_IO_PROTOCOL_WIDTH  Width,
-  IN     UINT32                       Offset,
+  IN     UINT32                     Offset,
-  IN     UINTN                        Count,
+  IN     UINTN                      Count,
-  IN OUT VOID                         *Buffer
+  IN OUT VOID                      *Buffer
   )
 {
   EFI_PCI_IO_PRIVATE_DATA *Private = EFI_PCI_IO_PRIVATE_DATA_FROM_THIS (This);
   EFI_STATUS Status;
 
-  Status = PciRootBridgeIoMemRW ((EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH)Width,
+  if ((Width < 0) || (Width >= EfiPciIoWidthMaximum) || (Buffer == NULL)) {
-                               Count,
+    return EFI_INVALID_PARAMETER;
-                               TRUE,
+  }
-                               (PTR)(UINTN)Buffer,
+
-                               TRUE,
+  Status = PciRootBridgeIoMemRW (
-                               (PTR)(UINTN)(((UINT8 *)Private->ConfigSpace) + Offset)  //Fix me ConfigSpace
+             (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH)Width,
-                              );
+             Count,
+             TRUE,
+             (PTR)(UINTN)Buffer,
+             TRUE,
+             (PTR)(UINTN)(((UINT8 *)Private->ConfigSpace) + Offset)  //Fix me ConfigSpace
+             );
+
   return Status;
 }
 
+/**
+  Enable a PCI driver to write PCI controller registers in PCI configuration space.
+
+  @param[in]      This    A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[in]      Width   Signifies the width of the memory operations.
+  @param[in]      Offset  The offset within the PCI configuration space for
+                          the PCI controller.
+  @param[in]      Count   The number of PCI configuration operations to
+                          perform. Bytes moved is Width size * Count,
+                          starting at Offset.
+
+  @param[in out]  Buffer  The source buffer to write data from.
+
+  @retval  EFI_SUCCESS            The data was read from the PCI controller.
+  @retval  EFI_INVALID_PARAMETER  "Width" is invalid.
+  @retval  EFI_INVALID_PARAMETER  "Buffer" is NULL.
+
+**/
 EFI_STATUS
 PciIoPciWrite (
   IN EFI_PCI_IO_PROTOCOL              *This,
@@ -192,6 +234,10 @@ PciIoPciWrite (
 {
   EFI_PCI_IO_PRIVATE_DATA *Private = EFI_PCI_IO_PRIVATE_DATA_FROM_THIS (This);
 
+  if ((Width < 0) || (Width >= EfiPciIoWidthMaximum) || (Buffer == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   return PciRootBridgeIoMemRW ((EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH) Width,
                                Count,
                                TRUE,
@@ -249,18 +295,44 @@ PciIoUnmap (
   return DmaUnmap (Mapping);
 }
 
+/**
+  Allocate pages that are suitable for an EfiPciIoOperationBusMasterCommonBuffer
+  mapping.
+
+  @param[in]   This         A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[in]   Type         This parameter is not used and must be ignored.
+  @param[in]   MemoryType   The type of memory to allocate, EfiBootServicesData or
+                            EfiRuntimeServicesData.
+  @param[in]   Pages        The number of pages to allocate.
+  @param[out]  HostAddress  A pointer to store the base system memory address of
+                            the allocated range.
+  @param[in]   Attributes   The requested bit mask of attributes for the allocated
+                            range. Only the attributes,
+                            EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE and
+                            EFI_PCI_ATTRIBUTE_MEMORY_CACHED may be used with this
+                            function. If any other bits are set, then EFI_UNSUPPORTED
+                            is returned. This function ignores this bit mask.
+
+  @retval  EFI_SUCCESS            The requested memory pages were allocated.
+  @retval  EFI_INVALID_PARAMETER  HostAddress is NULL.
+  @retval  EFI_INVALID_PARAMETER  MemoryType is invalid.
+  @retval  EFI_UNSUPPORTED        Attributes is unsupported.
+  @retval  EFI_OUT_OF_RESOURCES   The memory pages could not be allocated.
+
+**/
 EFI_STATUS
 PciIoAllocateBuffer (
-  IN EFI_PCI_IO_PROTOCOL           *This,
+  IN EFI_PCI_IO_PROTOCOL  *This,
-  IN  EFI_ALLOCATE_TYPE            Type,
+  IN  EFI_ALLOCATE_TYPE   Type,
-  IN  EFI_MEMORY_TYPE              MemoryType,
+  IN  EFI_MEMORY_TYPE     MemoryType,
-  IN  UINTN                        Pages,
+  IN  UINTN               Pages,
-  OUT VOID                         **HostAddress,
+  OUT VOID                **HostAddress,
-  IN  UINT64                       Attributes
+  IN  UINT64              Attributes
   )
 {
-  if (Attributes & EFI_PCI_ATTRIBUTE_INVALID_FOR_ALLOCATE_BUFFER) {
+  if (Attributes &
-    // Check this
+      (~(EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE |
+         EFI_PCI_ATTRIBUTE_MEMORY_CACHED         ))) {
     return EFI_UNSUPPORTED;
   }
 
@@ -287,36 +359,70 @@ PciIoFlush (
   return EFI_SUCCESS;
 }
 
+/**
+  Retrieves this PCI controller's current PCI bus number, device number, and function number.
+
+  @param[in]   This            A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[out]  SegmentNumber   The PCI controller's current PCI segment number.
+  @param[out]  BusNumber       The PCI controller's current PCI bus number.
+  @param[out]  DeviceNumber    The PCI controller's current PCI device number.
+  @param[out]  FunctionNumber  The PCI controller’s current PCI function number.
+
+  @retval  EFI_SUCCESS            The PCI controller location was returned.
+  @retval  EFI_INVALID_PARAMETER  At least one out of the four output parameters is
+                                  a NULL pointer.
+**/
 EFI_STATUS
 PciIoGetLocation (
-  IN EFI_PCI_IO_PROTOCOL          *This,
+  IN   EFI_PCI_IO_PROTOCOL  *This,
-  OUT UINTN                       *SegmentNumber,
+  OUT  UINTN                *SegmentNumber,
-  OUT UINTN                       *BusNumber,
+  OUT  UINTN                *BusNumber,
-  OUT UINTN                       *DeviceNumber,
+  OUT  UINTN                *DeviceNumber,
-  OUT UINTN                       *FunctionNumber
+  OUT  UINTN                *FunctionNumber
   )
 {
   EFI_PCI_IO_PRIVATE_DATA *Private = EFI_PCI_IO_PRIVATE_DATA_FROM_THIS (This);
 
-  if (SegmentNumber != NULL) {
+  if ((SegmentNumber == NULL) || (BusNumber      == NULL) ||
-    *SegmentNumber = Private->Segment;
+      (DeviceNumber  == NULL) || (FunctionNumber == NULL)    ) {
+    return EFI_INVALID_PARAMETER;
   }
 
-  if (BusNumber != NULL) {
+  *SegmentNumber  = Private->Segment;
-    *BusNumber = 0xff;
+  *BusNumber      = 0xff;
-  }
+  *DeviceNumber   = 0;
-
+  *FunctionNumber = 0;
-  if (DeviceNumber != NULL) {
-    *DeviceNumber = 0;
-  }
-
-  if (FunctionNumber != NULL) {
-    *FunctionNumber = 0;
-  }
 
   return EFI_SUCCESS;
 }
 
+/**
+  Performs an operation on the attributes that this PCI controller supports.
+
+  The operations include getting the set of supported attributes, retrieving
+  the current attributes, setting the current attributes, enabling attributes,
+  and disabling attributes.
+
+  @param[in]   This        A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[in]   Operation   The operation to perform on the attributes for this
+                           PCI controller.
+  @param[in]   Attributes  The mask of attributes that are used for Set,
+                           Enable and Disable operations.
+  @param[out]  Result      A pointer to the result mask of attributes that are
+                           returned for the Get and Supported operations. This
+                           is an optional parameter that may be NULL for the
+                           Set, Enable, and Disable operations.
+
+  @retval  EFI_SUCCESS            The operation on the PCI controller's
+                                  attributes was completed. If the operation
+                                  was Get or Supported, then the attribute mask
+                                  is returned in Result.
+  @retval  EFI_INVALID_PARAMETER  Operation is greater than or equal to
+                                  EfiPciIoAttributeOperationMaximum.
+  @retval  EFI_INVALID_PARAMETER  Operation is Get and Result is NULL.
+  @retval  EFI_INVALID_PARAMETER  Operation is Supported and Result is NULL.
+
+**/
 EFI_STATUS
 PciIoAttributes (
   IN EFI_PCI_IO_PROTOCOL                       *This,
@@ -331,18 +437,22 @@ PciIoAttributes (
     if (Result == NULL) {
       return EFI_INVALID_PARAMETER;
     }
+    //
     // We are not a real PCI device so just say things we kind of do
-    *Result = EFI_PCI_IO_ATTRIBUTE_MEMORY | EFI_PCI_IO_ATTRIBUTE_BUS_MASTER | EFI_PCI_DEVICE_ENABLE;
+    //
+    *Result = EFI_PCI_DEVICE_ENABLE;
     break;
 
   case EfiPciIoAttributeOperationSet:
   case EfiPciIoAttributeOperationEnable:
   case EfiPciIoAttributeOperationDisable:
+    if (Attributes & (~EFI_PCI_DEVICE_ENABLE)) {
+      return EFI_UNSUPPORTED;
+    }
     // Since we are not a real PCI device no enable/set or disable operations exist.
     return EFI_SUCCESS;
 
   default:
-  ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   };
   return EFI_SUCCESS;

Omap35xxPkg/PciEmulation/PciEmulation.c

@@ -206,26 +206,70 @@ PciIoIoWrite (
   return EFI_UNSUPPORTED;
 }
 
+/**
+  Enable a PCI driver to read PCI controller registers in PCI configuration space.
+
+  @param[in]      This    A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[in]      Width   Signifies the width of the memory operations.
+  @param[in]      Offset  The offset within the PCI configuration space for
+                          the PCI controller.
+  @param[in]      Count   The number of PCI configuration operations to
+                          perform. Bytes moved is Width size * Count,
+                          starting at Offset.
+
+  @param[in out]  Buffer  The destination buffer to store the results.
+
+  @retval  EFI_SUCCESS            The data was read from the PCI controller.
+  @retval  EFI_INVALID_PARAMETER  "Width" is invalid.
+  @retval  EFI_INVALID_PARAMETER  "Buffer" is NULL.
+
+**/
 EFI_STATUS
 PciIoPciRead (
-  IN EFI_PCI_IO_PROTOCOL              *This,
+  IN     EFI_PCI_IO_PROTOCOL       *This,
-  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
+  IN     EFI_PCI_IO_PROTOCOL_WIDTH  Width,
-  IN     UINT32                       Offset,
+  IN     UINT32                     Offset,
-  IN     UINTN                        Count,
+  IN     UINTN                      Count,
-  IN OUT VOID                         *Buffer
+  IN OUT VOID                      *Buffer
   )
 {
-  EFI_PCI_IO_PRIVATE_DATA *Private = EFI_PCI_IO_PRIVATE_DATA_FROM_THIS(This);
+  EFI_PCI_IO_PRIVATE_DATA *Private = EFI_PCI_IO_PRIVATE_DATA_FROM_THIS (This);
+  EFI_STATUS Status;
 
-  return PciRootBridgeIoMemRW ((EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH)Width,
+  if ((Width < 0) || (Width >= EfiPciIoWidthMaximum) || (Buffer == NULL)) {
-                               Count,
+    return EFI_INVALID_PARAMETER;
-                               TRUE,
+  }
-                               (PTR)(UINTN)Buffer,
+
-                               TRUE,
+  Status = PciRootBridgeIoMemRW (
-                               (PTR)(UINTN)(((UINT8 *)Private->ConfigSpace) + Offset)
+             (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH)Width,
-                              );
+             Count,
+             TRUE,
+             (PTR)(UINTN)Buffer,
+             TRUE,
+             (PTR)(UINTN)(((UINT8 *)Private->ConfigSpace) + Offset)  //Fix me ConfigSpace
+             );
+
+  return Status;
 }
 
+/**
+  Enable a PCI driver to write PCI controller registers in PCI configuration space.
+
+  @param[in]      This    A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[in]      Width   Signifies the width of the memory operations.
+  @param[in]      Offset  The offset within the PCI configuration space for
+                          the PCI controller.
+  @param[in]      Count   The number of PCI configuration operations to
+                          perform. Bytes moved is Width size * Count,
+                          starting at Offset.
+
+  @param[in out]  Buffer  The source buffer to write data from.
+
+  @retval  EFI_SUCCESS            The data was read from the PCI controller.
+  @retval  EFI_INVALID_PARAMETER  "Width" is invalid.
+  @retval  EFI_INVALID_PARAMETER  "Buffer" is NULL.
+
+**/
 EFI_STATUS
 PciIoPciWrite (
   IN EFI_PCI_IO_PROTOCOL              *This,
@@ -235,7 +279,11 @@ PciIoPciWrite (
   IN OUT VOID                         *Buffer
   )
 {
-  EFI_PCI_IO_PRIVATE_DATA *Private = EFI_PCI_IO_PRIVATE_DATA_FROM_THIS(This);
+  EFI_PCI_IO_PRIVATE_DATA *Private = EFI_PCI_IO_PRIVATE_DATA_FROM_THIS (This);
+
+  if ((Width < 0) || (Width >= EfiPciIoWidthMaximum) || (Buffer == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
 
   return PciRootBridgeIoMemRW ((EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH) Width,
                                Count,
@@ -294,18 +342,44 @@ PciIoUnmap (
   return DmaUnmap (Mapping);
 }
 
+/**
+  Allocate pages that are suitable for an EfiPciIoOperationBusMasterCommonBuffer
+  mapping.
+
+  @param[in]   This         A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[in]   Type         This parameter is not used and must be ignored.
+  @param[in]   MemoryType   The type of memory to allocate, EfiBootServicesData or
+                            EfiRuntimeServicesData.
+  @param[in]   Pages        The number of pages to allocate.
+  @param[out]  HostAddress  A pointer to store the base system memory address of
+                            the allocated range.
+  @param[in]   Attributes   The requested bit mask of attributes for the allocated
+                            range. Only the attributes,
+                            EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE and
+                            EFI_PCI_ATTRIBUTE_MEMORY_CACHED may be used with this
+                            function. If any other bits are set, then EFI_UNSUPPORTED
+                            is returned. This function ignores this bit mask.
+
+  @retval  EFI_SUCCESS            The requested memory pages were allocated.
+  @retval  EFI_INVALID_PARAMETER  HostAddress is NULL.
+  @retval  EFI_INVALID_PARAMETER  MemoryType is invalid.
+  @retval  EFI_UNSUPPORTED        Attributes is unsupported.
+  @retval  EFI_OUT_OF_RESOURCES   The memory pages could not be allocated.
+
+**/
 EFI_STATUS
 PciIoAllocateBuffer (
-  IN EFI_PCI_IO_PROTOCOL           *This,
+  IN EFI_PCI_IO_PROTOCOL  *This,
-  IN  EFI_ALLOCATE_TYPE            Type,
+  IN  EFI_ALLOCATE_TYPE   Type,
-  IN  EFI_MEMORY_TYPE              MemoryType,
+  IN  EFI_MEMORY_TYPE     MemoryType,
-  IN  UINTN                        Pages,
+  IN  UINTN               Pages,
-  OUT VOID                         **HostAddress,
+  OUT VOID                **HostAddress,
-  IN  UINT64                       Attributes
+  IN  UINT64              Attributes
   )
 {
-  if (Attributes & EFI_PCI_ATTRIBUTE_INVALID_FOR_ALLOCATE_BUFFER) {
+  if (Attributes &
-    // Check this
+      (~(EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE |
+         EFI_PCI_ATTRIBUTE_MEMORY_CACHED         ))) {
     return EFI_UNSUPPORTED;
   }
 
@@ -332,36 +406,70 @@ PciIoFlush (
   return EFI_SUCCESS;
 }
 
+/**
+  Retrieves this PCI controller's current PCI bus number, device number, and function number.
+
+  @param[in]   This            A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[out]  SegmentNumber   The PCI controller's current PCI segment number.
+  @param[out]  BusNumber       The PCI controller's current PCI bus number.
+  @param[out]  DeviceNumber    The PCI controller's current PCI device number.
+  @param[out]  FunctionNumber  The PCI controller’s current PCI function number.
+
+  @retval  EFI_SUCCESS            The PCI controller location was returned.
+  @retval  EFI_INVALID_PARAMETER  At least one out of the four output parameters is
+                                  a NULL pointer.
+**/
 EFI_STATUS
 PciIoGetLocation (
-  IN EFI_PCI_IO_PROTOCOL          *This,
+  IN   EFI_PCI_IO_PROTOCOL  *This,
-  OUT UINTN                       *SegmentNumber,
+  OUT  UINTN                *SegmentNumber,
-  OUT UINTN                       *BusNumber,
+  OUT  UINTN                *BusNumber,
-  OUT UINTN                       *DeviceNumber,
+  OUT  UINTN                *DeviceNumber,
-  OUT UINTN                       *FunctionNumber
+  OUT  UINTN                *FunctionNumber
   )
 {
-  EFI_PCI_IO_PRIVATE_DATA *Private = EFI_PCI_IO_PRIVATE_DATA_FROM_THIS(This);
+  EFI_PCI_IO_PRIVATE_DATA *Private = EFI_PCI_IO_PRIVATE_DATA_FROM_THIS (This);
 
-  if (SegmentNumber != NULL) {
+  if ((SegmentNumber == NULL) || (BusNumber      == NULL) ||
-    *SegmentNumber = Private->Segment;
+      (DeviceNumber  == NULL) || (FunctionNumber == NULL)    ) {
+    return EFI_INVALID_PARAMETER;
   }
 
-  if (BusNumber != NULL) {
+  *SegmentNumber  = Private->Segment;
-    *BusNumber = 0xff;
+  *BusNumber      = 0xff;
-  }
+  *DeviceNumber   = 0;
-
+  *FunctionNumber = 0;
-  if (DeviceNumber != NULL) {
-    *DeviceNumber = 0;
-  }
-
-  if (FunctionNumber != NULL) {
-    *FunctionNumber = 0;
-  }
 
   return EFI_SUCCESS;
 }
 
+/**
+  Performs an operation on the attributes that this PCI controller supports.
+
+  The operations include getting the set of supported attributes, retrieving
+  the current attributes, setting the current attributes, enabling attributes,
+  and disabling attributes.
+
+  @param[in]   This        A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param[in]   Operation   The operation to perform on the attributes for this
+                           PCI controller.
+  @param[in]   Attributes  The mask of attributes that are used for Set,
+                           Enable and Disable operations.
+  @param[out]  Result      A pointer to the result mask of attributes that are
+                           returned for the Get and Supported operations. This
+                           is an optional parameter that may be NULL for the
+                           Set, Enable, and Disable operations.
+
+  @retval  EFI_SUCCESS            The operation on the PCI controller's
+                                  attributes was completed. If the operation
+                                  was Get or Supported, then the attribute mask
+                                  is returned in Result.
+  @retval  EFI_INVALID_PARAMETER  Operation is greater than or equal to
+                                  EfiPciIoAttributeOperationMaximum.
+  @retval  EFI_INVALID_PARAMETER  Operation is Get and Result is NULL.
+  @retval  EFI_INVALID_PARAMETER  Operation is Supported and Result is NULL.
+
+**/
 EFI_STATUS
 PciIoAttributes (
   IN EFI_PCI_IO_PROTOCOL                       *This,
@@ -376,18 +484,22 @@ PciIoAttributes (
     if (Result == NULL) {
       return EFI_INVALID_PARAMETER;
     }
+    //
     // We are not a real PCI device so just say things we kind of do
-    *Result = EFI_PCI_IO_ATTRIBUTE_MEMORY | EFI_PCI_IO_ATTRIBUTE_BUS_MASTER | EFI_PCI_DEVICE_ENABLE;
+    //
+    *Result = EFI_PCI_DEVICE_ENABLE;
     break;
 
   case EfiPciIoAttributeOperationSet:
   case EfiPciIoAttributeOperationEnable:
   case EfiPciIoAttributeOperationDisable:
+    if (Attributes & (~EFI_PCI_DEVICE_ENABLE)) {
+      return EFI_UNSUPPORTED;
+    }
     // Since we are not a real PCI device no enable/set or disable operations exist.
     return EFI_SUCCESS;
 
   default:
-  ASSERT (FALSE);
     return EFI_INVALID_PARAMETER;
   };
   return EFI_SUCCESS;