public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ray Ni <ray.ni@intel.com>
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni <ruiyu.ni@intel.com>, Hao Wu <hao.a.wu@intel.com>,
	Dandan Bi <dandan.bi@intel.com>
Subject: [PATCH] MdeModulePkg/PciBus: Fix a bug PPB MEM32 BAR isn't restored sometimes
Date: Fri,  1 Feb 2019 16:59:10 +0800	[thread overview]
Message-ID: <20190201085910.241544-1-ray.ni@intel.com> (raw)

From: Ruiyu Ni <ruiyu.ni@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1505

When a device under PPB contains option ROM but doesn't require 32bit
MMIO, ProgrameUpstreamBridgeForRom() cannot correctly restore the
PPB MEM32 RANGE BAR. It causes the 32bit MMIO conflict which may
cause system hangs in boot.

The root cause is when ProgrameUpstreamBridgeForRom() calls
ProgramPpbApperture() to restore the PPB MEM32 RANGE BAR, the
ProgramPpbApperture() skips to program the BAR when the resource
length is 0.

This patch fixes this issue by not calling ProgramPpbApperture().
Instead, it directly programs the PPB MEM32 RANGE BAR.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
---
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c    | 53 +++++++++----------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index d3cbefbadf..77cdc3e844 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -1,7 +1,7 @@
 /** @file
   PCI resouces support functions implemntation for PCI Bus module.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 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
@@ -1661,57 +1661,52 @@ ProgrameUpstreamBridgeForRom (
   IN BOOLEAN         Enable
   )
 {
-  PCI_IO_DEVICE     *Parent;
-  PCI_RESOURCE_NODE Node;
-  UINT64            Base;
-  UINT64            Length;
+  PCI_IO_DEVICE       *Parent;
+  EFI_PCI_IO_PROTOCOL *PciIo;
+  UINT16              Base;
+  UINT16              Limit;
   //
   // For root bridge, just return.
   //
   Parent = PciDevice->Parent;
-  ZeroMem (&Node, sizeof (Node));
   while (Parent != NULL) {
     if (!IS_PCI_BRIDGE (&Parent->Pci)) {
       break;
     }
 
-    Node.PciDev     = Parent;
-    Node.Alignment  = 0;
-    Node.Bar        = PPB_MEM32_RANGE;
-    Node.ResType    = PciBarTypeMem32;
-    Node.Offset     = 0;
+    PciIo = &Parent->PciIo;
 
     //
     // Program PPB to only open a single <= 16MB apperture
     //
     if (Enable) {
-      //
-      // Save the original PPB_MEM32_RANGE BAR.
-      // The values will be changed by ProgramPpbApperture().
-      //
-      Base   = Parent->PciBar[Node.Bar].BaseAddress;
-      Length = Parent->PciBar[Node.Bar].Length;
-
       //
       // Only cover MMIO for Option ROM.
       //
-      Node.Length     = PciDevice->RomSize;
-      ProgramPpbApperture (OptionRomBase, &Node);
-
-      //
-      // Restore the original PPB_MEM32_RANGE BAR.
-      // So the MEM32 RANGE BAR register can be restored when disable the decoding.
-      //
-      Parent->PciBar[Node.Bar].BaseAddress = Base;
-      Parent->PciBar[Node.Bar].Length      = Length;
+      Base  = (UINT16) (OptionRomBase >> 16);
+      Limit = (UINT16) ((OptionRomBase + PciDevice->RomSize - 1) >> 16);
+      PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, Bridge.MemoryBase),  1, &Base);
+      PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, Bridge.MemoryLimit), 1, &Limit);
 
       PCI_ENABLE_COMMAND_REGISTER (Parent, EFI_PCI_COMMAND_MEMORY_SPACE);
     } else {
       //
       // Cover 32bit MMIO for devices below the bridge.
       //
-      Node.Length     = Parent->PciBar[Node.Bar].Length;
-      ProgramPpbApperture (Parent->PciBar[Node.Bar].BaseAddress, &Node);
+      if (Parent->PciBar[PPB_MEM32_RANGE].Length == 0) {
+        //
+        // When devices under the bridge contains Option ROM and doesn't require 32bit MMIO.
+        //
+        Base  = (UINT16) gAllOne;
+        Limit = (UINT16) gAllZero;
+      } else {
+        Base  = (UINT16) (Parent->PciBar[PPB_MEM32_RANGE].BaseAddress >> 16);
+        Limit = (UINT16)
+                    ((Parent->PciBar[PPB_MEM32_RANGE].BaseAddress + Parent->PciBar[PPB_MEM32_RANGE].Length - 1) >> 16);
+      }
+      PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, Bridge.MemoryBase),  1, &Base);
+      PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, Bridge.MemoryLimit), 1, &Limit);
+
       PCI_DISABLE_COMMAND_REGISTER (Parent, EFI_PCI_COMMAND_MEMORY_SPACE);
     }
 
-- 
2.20.1.windows.1



             reply	other threads:[~2019-02-01  8:56 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-01  8:59 Ray Ni [this message]
2019-02-07  2:17 ` [PATCH] MdeModulePkg/PciBus: Fix a bug PPB MEM32 BAR isn't restored sometimes Wu, Hao A
2019-02-11  1:25   ` Wu, Hao A

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190201085910.241544-1-ray.ni@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox