public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Aleksei Kovura <alex3kov@zoho.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Ruiyu Ni <ruiyu.ni@intel.com>
Subject: [PATCH 3/3] OvmfPkg/QemuVideoDxe/VbeShim: handle PAM1 register on Q35 correctly
Date: Tue, 19 Sep 2017 21:18:15 +0200	[thread overview]
Message-ID: <20170919191815.3004-4-lersek@redhat.com> (raw)
In-Reply-To: <20170919191815.3004-1-lersek@redhat.com>

In commit db27e9f3d8f0 ("OvmfPkg/LegacyRegion: Support legacy region
manipulation of Q35", 2016-03-15), Ray extended the
OvmfPkg/Csm/CsmSupportLib PAM register manipulation to Q35. However, we
missed that the same should be done to the QemuVideoDxe VBE Shim as well.

The omission has caused no problems in practice on Q35, because QEMU has
let us write to the ROM area, regardless of the PAM1 setting, all this
time. This has now changed with recent QEMU commit 208fa0e43645 ("pc: make
'pc.rom' readonly when machine has PCI enabled", 2017-07-28). The QEMU
commit exposes the OVMF bug when Windows 7 is started on Q35, using QEMU
2.10 -- the VBE Shim is no longer put in place and Windows 7 cannot find
it.

To remedy this, assign the "Pam1Address" local variable a PciLib address
that matches the board type (i440fx vs. q35).

Regarding the PcdLib dependency: QemuVideoDxe already uses PcdLib, both
directly (see "PcdDriverSupportedEfiVersion") and indirectly (e.g. via the
DxePciLibI440FxQ35 PciLib instance). Add PcdLib to [LibraryClasses] for
completeness.

Cc: Aleksei Kovura <alex3kov@zoho.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Ref: https://bugs.launchpad.net/qemu/+bug/1715700
Reported-by: Aleksei Kovura <alex3kov@zoho.com>
Special-thanks-to: Gerd Hoffmann <kraxel@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |  3 ++-
 OvmfPkg/QemuVideoDxe/VbeShim.c        | 27 +++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index 7c7d429bca27..577e07b0a8bf 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -60,6 +60,7 @@ [LibraryClasses]
   DebugLib
   DevicePathLib
   MemoryAllocationLib
+  PcdLib
   PciLib
   PrintLib
   TimerLib
@@ -75,4 +76,4 @@ [Protocols]
 
 [Pcd]
   gOptionRomPkgTokenSpaceGuid.PcdDriverSupportedEfiVersion
-
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.c b/OvmfPkg/QemuVideoDxe/VbeShim.c
index bc90e067266d..e45a08e8873f 100644
--- a/OvmfPkg/QemuVideoDxe/VbeShim.c
+++ b/OvmfPkg/QemuVideoDxe/VbeShim.c
@@ -25,6 +25,7 @@
 #include <Library/DebugLib.h>
 #include <Library/PciLib.h>
 #include <Library/PrintLib.h>
+#include <OvmfPlatforms.h>
 
 #include "Qemu.h"
 #include "VbeShim.h"
@@ -64,6 +65,7 @@ InstallVbeShim (
   UINTN                Segment0Pages;
   IVT_ENTRY            *Int0x10;
   EFI_STATUS           Segment0AllocationStatus;
+  UINT16               HostBridgeDevId;
   UINTN                Pam1Address;
   UINT8                Pam1;
   UINTN                SegmentCPages;
@@ -131,7 +133,30 @@ InstallVbeShim (
   //
   // Put the shim in place first.
   //
-  Pam1Address = PCI_LIB_ADDRESS (0, 0, 0, 0x5A);
+  // Start by determining the address of the PAM1 register.
+  //
+  HostBridgeDevId = PcdGet16 (PcdOvmfHostBridgePciDevId);
+  switch (HostBridgeDevId) {
+  case INTEL_82441_DEVICE_ID:
+    Pam1Address = PMC_REGISTER_PIIX4 (PIIX4_PAM1);
+    break;
+  case INTEL_Q35_MCH_DEVICE_ID:
+    Pam1Address = DRAMC_REGISTER_Q35 (MCH_PAM1);
+    break;
+  default:
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: unknown host bridge device ID: 0x%04x\n",
+      __FUNCTION__,
+      HostBridgeDevId
+      ));
+    ASSERT (FALSE);
+
+    if (!EFI_ERROR (Segment0AllocationStatus)) {
+      gBS->FreePages (Segment0, Segment0Pages);
+    }
+    return;
+  }
   //
   // low nibble covers 0xC0000 to 0xC3FFF
   // high nibble covers 0xC4000 to 0xC7FFF
-- 
2.14.1.3.gb7cf6e02401b



  parent reply	other threads:[~2017-09-19 19:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-19 19:18 [PATCH 0/3] OvmfPkg/QemuVideoDxe/VbeShim: handle PAM1 register on Q35 correctly Laszlo Ersek
2017-09-19 19:18 ` [PATCH 1/3] OvmfPkg/CsmSupportLib: move PAM register addresses to IndustryStandard Laszlo Ersek
2017-09-19 19:18 ` [PATCH 2/3] OvmfPkg/QemuVideoDxe/VbeShim: rename Status to Segment0AllocationStatus Laszlo Ersek
2017-09-19 19:18 ` Laszlo Ersek [this message]
2017-09-19 19:36 ` [PATCH 0/3] OvmfPkg/QemuVideoDxe/VbeShim: handle PAM1 register on Q35 correctly Laszlo Ersek
2017-09-20  7:05 ` Aleksei
2017-09-20 11:43   ` Laszlo Ersek
2017-09-20 14:36 ` Aleksei
2017-09-20 18:11 ` Jordan Justen
2017-09-20 18:28   ` Laszlo Ersek

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=20170919191815.3004-4-lersek@redhat.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