[Linaro-uefi,Linaro-uefi,v1,11/21] Hisilicon: Fix SCT PCIBusSupportTest error

Message ID 1490015485-53685-12-git-send-email-chenhui.sun@linaro.org
State New
Headers show
Series
  • D02/D03 platforms bug fix
Related show

Commit Message

Chenhui Sun March 20, 2017, 1:11 p.m.
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL/EFI_PCI_IO_PROTOCOL must returns
EFI_INVALID_PARAMETER, change return value.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: wanglijun <wanglijun@huawei.com>
Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
---
 .../Drivers/PciHostBridgeDxe/PciRootBridgeIo.c          | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Leif Lindholm March 21, 2017, 3:28 p.m. | #1
On Mon, Mar 20, 2017 at 09:11:15PM +0800, Chenhui Sun wrote:
> EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL/EFI_PCI_IO_PROTOCOL must returns
> EFI_INVALID_PARAMETER, change return value.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: wanglijun <wanglijun@huawei.com>
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
> ---
>  .../Drivers/PciHostBridgeDxe/PciRootBridgeIo.c          | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c
> index 03edcf1..30619f5 100644
> --- a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c
> +++ b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c
> @@ -824,8 +824,8 @@ RootBridgeConstructor (
>    PrivateData->Supports    = EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO | EFI_PCI_ATTRIBUTE_IDE_SECONDARY_IO | \
>                               EFI_PCI_ATTRIBUTE_ISA_IO_16 | EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO | \
>                               EFI_PCI_ATTRIBUTE_VGA_MEMORY | \
> -                             EFI_PCI_ATTRIBUTE_VGA_IO_16  | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16 | \
> -                             EFI_PCI_ATTRIBUTE_VALID_FOR_ALLOCATE_BUFFER;
> +                             EFI_PCI_ATTRIBUTE_VGA_IO_16  | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16;
> +

This deletion of EFI_PCI_ATTRIBUTE_VALID_FOR_ALLOCATE_BUFFER does not
obviously relate to what the commit message describes. Can you expand
the explanation, please?

>    PrivateData->Attributes  = PrivateData->Supports;
>  
>    Protocol->ParentHandle   = HostBridgeHandle;
> @@ -1275,6 +1275,9 @@ RootBridgeIoPollMem (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> +  if (Width > EfiPciWidthUint64) {
> +    return EFI_INVALID_PARAMETER;
> +  }

Add blank line after?

>    //
>    // No matter what, always do a single poll.
>    //
> @@ -2010,7 +2013,10 @@ RootBridgeIoMap (
>    )
>  {
>    DMA_MAP_OPERATION   DmaOperation;
> -

Why delete this blank line?

> +  if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||
> +      Mapping == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }

I would prefer a blank line here.

>    if (Operation == EfiPciOperationBusMasterRead) {
>      DmaOperation = MapOperationBusMasterRead;
>    } else if (Operation == EfiPciOperationBusMasterWrite) {
> @@ -2245,6 +2251,11 @@ RootBridgeIoSetAttributes (
>  
>    PrivateData = DRIVER_INSTANCE_FROM_PCI_ROOT_BRIDGE_IO_THIS(This);
>  
> +  if((Attributes & (EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE | EFI_PCI_ATTRIBUTE_MEMORY_CACHED)) &&
> +     (ResourceBase == NULL || ResourceLength == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    if (Attributes != 0) {
>      if ((Attributes & (~(PrivateData->Supports))) != 0) {
>        return EFI_UNSUPPORTED;
> -- 
> 1.9.1
>

Patch hide | download patch | download mbox

diff --git a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c
index 03edcf1..30619f5 100644
--- a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -824,8 +824,8 @@  RootBridgeConstructor (
   PrivateData->Supports    = EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO | EFI_PCI_ATTRIBUTE_IDE_SECONDARY_IO | \
                              EFI_PCI_ATTRIBUTE_ISA_IO_16 | EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO | \
                              EFI_PCI_ATTRIBUTE_VGA_MEMORY | \
-                             EFI_PCI_ATTRIBUTE_VGA_IO_16  | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16 | \
-                             EFI_PCI_ATTRIBUTE_VALID_FOR_ALLOCATE_BUFFER;
+                             EFI_PCI_ATTRIBUTE_VGA_IO_16  | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16;
+
   PrivateData->Attributes  = PrivateData->Supports;
 
   Protocol->ParentHandle   = HostBridgeHandle;
@@ -1275,6 +1275,9 @@  RootBridgeIoPollMem (
     return EFI_INVALID_PARAMETER;
   }
 
+  if (Width > EfiPciWidthUint64) {
+    return EFI_INVALID_PARAMETER;
+  }
   //
   // No matter what, always do a single poll.
   //
@@ -2010,7 +2013,10 @@  RootBridgeIoMap (
   )
 {
   DMA_MAP_OPERATION   DmaOperation;
-
+  if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||
+      Mapping == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
   if (Operation == EfiPciOperationBusMasterRead) {
     DmaOperation = MapOperationBusMasterRead;
   } else if (Operation == EfiPciOperationBusMasterWrite) {
@@ -2245,6 +2251,11 @@  RootBridgeIoSetAttributes (
 
   PrivateData = DRIVER_INSTANCE_FROM_PCI_ROOT_BRIDGE_IO_THIS(This);
 
+  if((Attributes & (EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE | EFI_PCI_ATTRIBUTE_MEMORY_CACHED)) &&
+     (ResourceBase == NULL || ResourceLength == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   if (Attributes != 0) {
     if ((Attributes & (~(PrivateData->Supports))) != 0) {
       return EFI_UNSUPPORTED;