Clean ISA_IO/ISA_IO_16 and VGA_IO/VGA_IO_16 attribute usage in PCI bus driver/PCI host bridge drivers/LPC/VGA device drivers.

1. Fix the incorrect definition of EFI_PCI_IO_ATTRIBUTE_VGA_IO_16 (does not conform to the UEFI spec) in PciIo.h.
2. Add missing definitions of ISA and VGA IO attributes in PCI Root Bridge IO protocol.
3. Improve the algorithm in the PCI bus driver to get PCI platform policy from PCI Platform Protocol and PCI Override Protocol.
4. Update the PCI bus driver to use the PCI platform policy to determine the supported attributes that are returned by the EFI_PCI_IO_PROTOCOL.Attributes() function. This is required by the PI spec.
5. Add a backward compatibility workaround for PCI VGA drivers in Option ROM, which typically sets VGA_IO without checking supported attributes.
6. Update the PCI host bridge driver in PcAtChipsetPkg to report VGA_IO_16 and ISA_IO_16 instead of VGA_IO/ISA_IO attributes. Modern chipsets don’t have hardware capability to control 10-bit or 16-bit decoding for ISA/VGA aliases.
7. Update LPC/VGA device drivers to check supported attributes of VGA_IO/VGA_IO_16 or ISA_IO/ISA_IO_16.

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@11204 6f19259b-4bc3-4df7-8a09-765794883524

4 files changed, 79 insertions, 53 deletions

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index e268014d34..d77982c02a 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -309,8 +309,8 @@ extern UINT64                                       gAllOne;
 extern UINT64                                       gAllZero;

 extern EFI_PCI_PLATFORM_PROTOCOL                    *gPciPlatformProtocol;

 extern EFI_PCI_OVERRIDE_PROTOCOL                    *gPciOverrideProtocol;

-

-

+extern BOOLEAN                                      mReserveIsaAliases;

+extern BOOLEAN                                      mReserveVgaAliases;

 

 /**

   Macro that checks whether device is a GFX device.

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index c8689c0421..2dd5889afc 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -910,9 +910,10 @@ PciSetDeviceAttribute (
                   EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM         |

                   EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE;

 

-    if ((Attributes & EFI_PCI_IO_ATTRIBUTE_IO) != 0) {

-      Attributes |= EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO;

-      Attributes |= EFI_PCI_IO_ATTRIBUTE_ISA_IO;

+    if (IS_PCI_LPC (&PciIoDevice->Pci)) {

+        Attributes |= EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO;

+        Attributes |= (mReserveIsaAliases ? EFI_PCI_IO_ATTRIBUTE_ISA_IO : \

+                                            EFI_PCI_IO_ATTRIBUTE_ISA_IO_16);

     }

 

     if (IS_PCI_BRIDGE (&PciIoDevice->Pci) || IS_CARDBUS_BRIDGE (&PciIoDevice->Pci)) {

@@ -921,6 +922,14 @@ PciSetDeviceAttribute (
       //

       Attributes |= EFI_PCI_IO_ATTRIBUTE_IDE_SECONDARY_IO;

       Attributes |= EFI_PCI_IO_ATTRIBUTE_IDE_PRIMARY_IO;

+

+      if (mReserveVgaAliases) {

+        Attributes &= ~(UINT64)(EFI_PCI_IO_ATTRIBUTE_VGA_IO_16 | \

+                                EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16);

+      } else {

+        Attributes &= ~(UINT64)(EFI_PCI_IO_ATTRIBUTE_VGA_IO | \

+                                EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO);

+      }

     } else {

 

       if (IS_PCI_IDE (&PciIoDevice->Pci)) {

@@ -930,7 +939,8 @@ PciSetDeviceAttribute (
 

       if (IS_PCI_VGA (&PciIoDevice->Pci)) {

         Attributes |= EFI_PCI_IO_ATTRIBUTE_VGA_MEMORY;

-        Attributes |= EFI_PCI_IO_ATTRIBUTE_VGA_IO;

+        Attributes |= (mReserveVgaAliases ? EFI_PCI_IO_ATTRIBUTE_VGA_IO : \

+                                            EFI_PCI_IO_ATTRIBUTE_VGA_IO_16);

       }

     }

 

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index c39de9dd7e..5f6d3763e9 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1530,6 +1530,38 @@ PciIoAttributes (
   }

 

   //

+  // Check VGA and VGA16, they can not be set at the same time

+  //

+  if (((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_IO) != 0         &&

+       (Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_IO_16) != 0)         ||

+      ((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_IO) != 0         &&

+       (Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16) != 0) ||

+      ((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO) != 0 &&

+       (Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_IO_16) != 0)         ||

+      ((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO) != 0 &&

+       (Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16) != 0) ) {

+    return EFI_UNSUPPORTED;

+  }

+

+  //

+  // workaround for PCI drivers which always set ISA_IO or VGA_IO attribute without detecting support of

+  // ISA_IO/ISA_IO_16 or VGA_IO/VGA_IO_16 to maintain backward-compatibility.

+  //

+  if (((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_IO) != 0) && 

+      ((PciIoDevice->Supports & (EFI_PCI_IO_ATTRIBUTE_VGA_IO | EFI_PCI_IO_ATTRIBUTE_VGA_IO_16)) \

+        == EFI_PCI_IO_ATTRIBUTE_VGA_IO_16)) {

+    Attributes &= ~(UINT64)EFI_PCI_IO_ATTRIBUTE_VGA_IO;

+    Attributes |= EFI_PCI_IO_ATTRIBUTE_VGA_IO_16;

+  }

+

+  if (((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO) != 0) && 

+      ((PciIoDevice->Supports & (EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO | EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16)) \

+        == EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16)) {

+    Attributes &= ~(UINT64)EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO;

+    Attributes |= EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16;

+  }

+

+  //

   // If no attributes can be supported, then return.

   // Otherwise, set the attributes that it can support.

   //

@@ -1550,20 +1582,6 @@ PciIoAttributes (
   BridgeControl = 0;

 

   //

-  // Check VGA and VGA16, they can not be set at the same time

-  //

-  if (((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_IO) != 0         &&

-       (Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_IO_16) != 0)         ||

-      ((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_IO) != 0         &&

-       (Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16) != 0) ||

-      ((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO) != 0 &&

-       (Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_IO_16) != 0)         ||

-      ((Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO) != 0 &&

-       (Attributes & EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16) != 0) ) {

-    return EFI_UNSUPPORTED;

-  }

-

-  //

   // For PPB & P2C, set relevant attribute bits

   //

   if (IS_PCI_BRIDGE (&PciIoDevice->Pci) || IS_CARDBUS_BRIDGE (&PciIoDevice->Pci)) {

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index cca365370f..6fa4001a62 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -14,6 +14,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 

 #include "PciBus.h"

 

+//

+// The default policy for the PCI bus driver is NOT to reserve I/O ranges for both ISA aliases and VGA aliases.

+//

+BOOLEAN mReserveIsaAliases = FALSE;

+BOOLEAN mReserveVgaAliases = FALSE;

+BOOLEAN mPolicyDetermined  = FALSE;

+

 /**

   The function is used to skip VGA range.

 

@@ -188,46 +195,37 @@ CalculateApertureIo16 (
   LIST_ENTRY              *CurrentLink;

   PCI_RESOURCE_NODE       *Node;

   UINT64                  Offset;

-  BOOLEAN                 IsaEnable;

-  BOOLEAN                 VGAEnable;

   EFI_PCI_PLATFORM_POLICY PciPolicy;

 

-  //

-  // Always assume there is ISA device and VGA device on the platform

-  // will be customized later

-  //

-  IsaEnable = FALSE;

-  VGAEnable = FALSE;

+  if (!mPolicyDetermined) {

+    //

+    // Check PciPlatform policy

+    //

+    Status = EFI_NOT_FOUND;

+    PciPolicy = 0;

+    if (gPciPlatformProtocol != NULL) {

+      Status = gPciPlatformProtocol->GetPlatformPolicy (

+                                       gPciPlatformProtocol,

+                                       &PciPolicy

+                                       );

+    }

 

-  //

-  // Check PciPlatform policy

-  //

-  if (gPciPlatformProtocol != NULL) {

-    Status = gPciPlatformProtocol->GetPlatformPolicy (

-                                     gPciPlatformProtocol,

-                                     &PciPolicy

-                                     );

-    if (!EFI_ERROR (Status)) {

-      if ((PciPolicy & EFI_RESERVE_ISA_IO_ALIAS) != 0) {

-        IsaEnable = TRUE;

-      }

-      if ((PciPolicy & EFI_RESERVE_VGA_IO_ALIAS) != 0) {

-        VGAEnable = TRUE;

-      }

+    if (EFI_ERROR (Status) && gPciOverrideProtocol != NULL) {

+      Status = gPciOverrideProtocol->GetPlatformPolicy (

+                                       gPciOverrideProtocol,

+                                       &PciPolicy

+                                       );

     }

-  } else if (gPciOverrideProtocol != NULL) {

-    Status = gPciOverrideProtocol->GetPlatformPolicy (

-                                     gPciOverrideProtocol,

-                                     &PciPolicy

-                                     );

+

     if (!EFI_ERROR (Status)) {

       if ((PciPolicy & EFI_RESERVE_ISA_IO_ALIAS) != 0) {

-        IsaEnable = TRUE;

+        mReserveIsaAliases = TRUE;

       }

       if ((PciPolicy & EFI_RESERVE_VGA_IO_ALIAS) != 0) {

-        VGAEnable = TRUE;

+        mReserveVgaAliases = TRUE;

       }

     }

+    mPolicyDetermined = TRUE;

   }

 

   Aperture = 0;

@@ -261,13 +259,13 @@ CalculateApertureIo16 (
     // If both of them are enabled, then the IO resource would

     // become too limited to meet the requirement of most of devices.

     //

-    if (IsaEnable || VGAEnable) {

+    if (mReserveIsaAliases || mReserveVgaAliases) {

       if (!IS_PCI_BRIDGE (&(Node->PciDev->Pci)) && !IS_CARDBUS_BRIDGE (&(Node->PciDev->Pci))) {

         //

         // Check if there is need to support ISA/VGA decoding

         // If so, we need to avoid isa/vga aliasing range

         //

-        if (IsaEnable) {

+        if (mReserveIsaAliases) {

           SkipIsaAliasAperture (

             &Aperture,

             Node->Length

@@ -276,7 +274,7 @@ CalculateApertureIo16 (
           if (Offset != 0) {

             Aperture = Aperture + (Node->Alignment + 1) - Offset;

           }

-        } else if (VGAEnable) {

+        } else if (mReserveVgaAliases) {

           SkipVGAAperture (

             &Aperture,

             Node->Length