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: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Jordan Justen <jordan.l.justen@intel.com>
Subject: [PATCH] OvmfPkg/PlatformDebugLibIoPort: fix port detection for use in the DXE Core
Date: Fri,  3 Aug 2018 02:30:45 +0200	[thread overview]
Message-ID: <20180803003045.31740-1-lersek@redhat.com> (raw)

The DXE Core is one of those modules that call
ProcessLibraryConstructorList() manually.

Before DxeMain() [MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c] calls
ProcessLibraryConstructorList(), and through it, our
PlatformDebugLibIoPortConstructor() function, DxeMain() invokes the
DEBUG() macro multiple times. That macro lands in our
PlatformDebugLibIoPortFound() function -- which currently relies on the
"mDebugIoPortFound" global variable that has (not yet) been set by the
constructor. As a result, early debug messages from the DXE Core are lost.

Move the device detection into PlatformDebugLibIoPortFound(), also caching
the fact (not just the result) of the device detection.

(We could introduce a separate DebugLib instance just for the DXE Core,
but the above approach works for all modules that currently consume the
PlatformDebugLibIoPort instance (which means "everything but SEC").)

This restores messages such as:

> CoreInitializeMemoryServices:
>   BaseAddress - 0x7AF21000 Length - 0x3CDE000 MinimalMemorySizeNeeded - 0x10F4000

Keep the empty constructor function -- OVMF's DebugLib instances have
always had constructors; we had better not upset constructor dependency
ordering by making our instance(s) constructor-less.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Fixes: c09d9571300a089c35f5df2773b70edc25050d0d
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    Brijesh, can you please test this patch on SEV, with and without
    capturing the debug port? (In the first case, the debug log should just
    work; in the second case, the boot should remain fast.) Thanks!
    
    Repo:   https://github.com/lersek/edk2.git
    Branch: debuglib_dxecore

 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c | 24 ++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
index 81c44eece95f..74aef2e37b42 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
@@ -16,14 +16,26 @@
 #include <Base.h>
 #include "DebugLibDetect.h"
 
+
+//
+// Set to TRUE if the debug I/O port has been checked
+//
+STATIC BOOLEAN mDebugIoPortChecked = FALSE;
 //
 // Set to TRUE if the debug I/O port is enabled
 //
 STATIC BOOLEAN mDebugIoPortFound = FALSE;
 
 /**
-  This constructor function checks if the debug I/O port device is present,
-  caching the result for later use.
+  This constructor function must not do anything.
+
+  Some modules consuming this library instance, such as the DXE Core, invoke
+  the DEBUG() macro before they explicitly call
+  ProcessLibraryConstructorList(). Therefore the auto-generated call from
+  ProcessLibraryConstructorList() to this constructor function may be preceded
+  by some calls to PlatformDebugLibIoPortFound() below. Hence
+  PlatformDebugLibIoPortFound() must not rely on anything this constructor
+  could set up.
 
   @retval RETURN_SUCCESS   The constructor always returns RETURN_SUCCESS.
 
@@ -34,12 +46,12 @@ PlatformDebugLibIoPortConstructor (
   VOID
   )
 {
-  mDebugIoPortFound = PlatformDebugLibIoPortDetect();
   return RETURN_SUCCESS;
 }
 
 /**
-  Return the cached result of detecting the debug I/O port device.
+  At the first call, check if the debug I/O port device is present, and cache
+  the result for later use. At subsequent calls, return the cached result.
 
   @retval TRUE   if the debug I/O port device was detected.
   @retval FALSE  otherwise
@@ -51,5 +63,9 @@ PlatformDebugLibIoPortFound (
   VOID
   )
 {
+  if (!mDebugIoPortChecked) {
+    mDebugIoPortFound = PlatformDebugLibIoPortDetect ();
+    mDebugIoPortChecked = TRUE;
+  }
   return mDebugIoPortFound;
 }
-- 
2.14.1.3.gb7cf6e02401b



             reply	other threads:[~2018-08-03  0:30 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-03  0:30 Laszlo Ersek [this message]
2018-08-03  6:42 ` [PATCH] OvmfPkg/PlatformDebugLibIoPort: fix port detection for use in the DXE Core Jordan Justen
2018-08-03 15:08   ` Laszlo Ersek
2018-08-06 18:26     ` Jordan Justen
2018-08-06 19:11       ` Laszlo Ersek
2018-08-03 14:21 ` Brijesh Singh

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=20180803003045.31740-1-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