Re: [PATCH v2 2/6] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment

["Tian, Feng" <feng.tian@intel.com>]

2/3/4 looks good to me

Reviewed-by: Feng Tian <feng.tian@intel.com>

Thanks
Feng

-----Original Message-----
From: Ni, Ruiyu 
Sent: Monday, February 6, 2017 2:01 PM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff <jeff.fan@intel.com>; Tian, Feng <feng.tian@intel.com>
Subject: [PATCH v2 2/6] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment

PI spec IncompatiblePciSupport part defines (UINT64) -1 as all BARs and 0 to use existing alignment. PciBus driver didn't accept these values. It treated 0xFF as all BARs and 0xFFFFFFFFFFFFFFFFULL to use existing alignment.
The patch changes the code to still accept old values while also accept values defined in PI spec. So that the driver can provide backward compatibility and follow spec.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
---
 .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c       | 37 ++++++++++++++--------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index ac4d323..978116a 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -1,7 +1,7 @@
 /** @file
   PCI emumeration support functions implementation for PCI Bus module.
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>  This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License @@ -17,6 +17,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 extern CHAR16  *mBarTypeStr[];
 
+
+#define OLD_ALIGN   0xFFFFFFFFFFFFFFFFULL
+#define EVEN_ALIGN  0xFFFFFFFFFFFFFFFEULL #define SQUAD_ALIGN 
+0xFFFFFFFFFFFFFFFDULL #define DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL
+
 /**
   This routine is used to check whether the pci device is present.
 
@@ -1335,8 +1341,8 @@ UpdatePciInfo (
   )
 {
   EFI_STATUS                        Status;
-  UINTN                             BarIndex;
-  UINTN                             BarEndIndex;
+  UINT64                            BarIndex;
+  UINT64                            BarEndIndex;
   BOOLEAN                           SetFlag;
   VOID                              *Configuration;
   EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Ptr; @@ -1390,18 +1396,19 @@ UpdatePciInfo (
       break;
     }
 
-    BarIndex    = (UINTN) Ptr->AddrTranslationOffset;
+    BarIndex    = Ptr->AddrTranslationOffset;
     BarEndIndex = BarIndex;
 
     //
     // Update all the bars in the device
+    // Compare against 0xFF is to keep backward compatibility.
     //
-    if (BarIndex == PCI_BAR_ALL) {
+    if ((BarIndex == (UINT64)-1) || (BarIndex == 0xFF)) {
       BarIndex    = 0;
       BarEndIndex = PCI_MAX_BAR - 1;
     }
 
-    if (BarIndex > PCI_MAX_BAR) {
+    if (BarIndex >= PCI_MAX_BAR) {
       Ptr++;
       continue;
     }
@@ -1472,7 +1479,7 @@ UpdatePciInfo (
         //
         // Update the new length for the device
         //
-        if (Ptr->AddrLen != PCI_BAR_NOCHANGE) {
+        if (Ptr->AddrLen != 0) {
           PciIoDevice->PciBar[BarIndex].Length = Ptr->AddrLen;
         }
       }
@@ -1488,6 +1495,8 @@ UpdatePciInfo (
 
 /**
   This routine will update the alignment with the new alignment.
+  Compare with OLD_ALIGN/EVEN_ALIGN/SQUAD_ALIGN/DQUAD_ALIGN is to keep  
+ backward compatibility.
 
   @param Alignment    Input Old alignment. Output updated alignment.
   @param NewAlignment New alignment.
@@ -1506,15 +1515,15 @@ SetNewAlign (
   // The new alignment is the same as the original,
   // so skip it
   //
-  if (NewAlignment == PCI_BAR_OLD_ALIGN) {
+  if ((NewAlignment == 0) || (NewAlignment == OLD_ALIGN)) {
     return ;
   }
   //
   // Check the validity of the parameter
   //
-   if (NewAlignment != PCI_BAR_EVEN_ALIGN  &&
-       NewAlignment != PCI_BAR_SQUAD_ALIGN &&
-       NewAlignment != PCI_BAR_DQUAD_ALIGN ) {
+   if (NewAlignment != EVEN_ALIGN  &&
+       NewAlignment != SQUAD_ALIGN &&
+       NewAlignment != DQUAD_ALIGN ) {
     *Alignment = NewAlignment;
     return ;
   }
@@ -1533,15 +1542,15 @@ SetNewAlign (
   //
   // Adjust the alignment to even, quad or double quad boundary
   //
-  if (NewAlignment == PCI_BAR_EVEN_ALIGN) {
+  if (NewAlignment == EVEN_ALIGN) {
     if ((OldAlignment & 0x01) != 0) {
       OldAlignment = OldAlignment + 2 - (OldAlignment & 0x01);
     }
-  } else if (NewAlignment == PCI_BAR_SQUAD_ALIGN) {
+  } else if (NewAlignment == SQUAD_ALIGN) {
     if ((OldAlignment & 0x03) != 0) {
       OldAlignment = OldAlignment + 4 - (OldAlignment & 0x03);
     }
-  } else if (NewAlignment == PCI_BAR_DQUAD_ALIGN) {
+  } else if (NewAlignment == DQUAD_ALIGN) {
     if ((OldAlignment & 0x07) != 0) {
       OldAlignment = OldAlignment + 8 - (OldAlignment & 0x07);
     }
--
2.9.0.windows.1