public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek <lersek@redhat.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Jordan Justen <jordan.l.justen@intel.com>
Subject: [PATCH 2/2] OvmfPkg: save on I/O port accesses when the debug port is not in use
Date: Thu, 16 Nov 2017 11:47:16 +0100	[thread overview]
Message-ID: <20171116104716.15144-3-pbonzini@redhat.com> (raw)
In-Reply-To: <20171116104716.15144-1-pbonzini@redhat.com>

When SEV is enabled, every debug message printed by OVMF to the
QEMU debug port traps from the guest to QEMU character by character
because "REP OUTSB" cannot be used by IoWriteFifo8.  Furthermore,
when OVMF is built with the DEBUG_VERBOSE bit (value 0x00400000)
enabled in "gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel", then the
OvmfPkg/IoMmuDxe driver, and the OvmfPkg/Library/BaseMemEncryptSevLib
library instance that is built into it, produce a huge amount of
log messages.  Therefore, in SEV guests, the boot time impact is huge
(about 45 seconds _additional_ time spent writing to the debug port).

While these messages are very useful for analyzing guest behavior,
most of the time the user won't be capturing the OVMF debug log.
In fact libvirt does not provide a method for configuring log capture;
users that wish to do this (or are instructed to do this) have to resort
to <qemu:arg>.

The debug console device provides a handy detection mechanism; when read,
it returns 0xE9 (which is very much unlike the 0xFF that is returned by
an unused port).  Use it to skip the possibly expensive OUT instructions
when the debug I/O port isn't plugged anywhere.

For SEC, the debug port has to be read before each full message.
However:

- if the debug port is available, then reading one byte before writing
a full message isn't tragic, especially because SEC doesn't print many
messages

- if the debug port is not available, then reading one byte instead of
writing a full message is still a win.

Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen (Intel address) <jordan.l.justen@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c  | 26 ++++++++--
 .../PlatformDebugLibIoPort/DebugLibDetect.c        | 29 +++++++++--
 .../PlatformDebugLibIoPort/DebugLibDetect.h        | 56 ++++++++++++++++++++++
 .../PlatformDebugLibIoPort/DebugLibDetectRom.c     | 20 +++++++-
 4 files changed, 122 insertions(+), 9 deletions(-)
 create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h

diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
index a5572a8eeb..79486ac8a6 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
@@ -23,6 +23,7 @@
 #include <Library/PcdLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugPrintErrorLevelLib.h>
+#include "DebugLibDetect.h"
 
 //
 // Define the maximum debug and assert message length that this library supports
@@ -62,9 +63,9 @@ DebugPrint (
   ASSERT (Format != NULL);
 
   //
-  // Check driver debug mask value and global mask
+  // Do nothing if the global mask disables this message or the device is inactive
   //
-  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
+  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0 || !PlatformDebugLibIoPortFound ()) {
     return;
   }
 
@@ -121,9 +122,11 @@ DebugAssert (
              FileName, (UINT64)LineNumber, Description);
 
   //
-  // Send the print string to the debug I/O port
+  // Send the print string to the debug I/O port, if present
   //
-  IoWriteFifo8 (PcdGet16 (PcdDebugIoPort), Length, Buffer);
+  if (PlatformDebugLibIoPortFound ()) {
+    IoWriteFifo8 (PcdGet16 (PcdDebugIoPort), Length, Buffer);
+  }
 
   //
   // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
@@ -266,3 +269,18 @@ DebugPrintLevelEnabled (
 {
   return (BOOLEAN) ((ErrorLevel & PcdGet32(PcdFixedDebugPrintErrorLevel)) != 0);
 }
+
+/**
+  Return the result of detecting the debug I/O port device.
+
+  @retval BOOLEAN   TRUE if the debug I/O port device was detected.
+
+**/
+BOOLEAN
+EFIAPI
+PlatformDebugLibIoPortDetect (
+  VOID
+  )
+{
+  return IoRead8 (PcdGet16 (PcdDebugIoPort)) == BOCHS_DEBUG_PORT_MAGIC;
+}
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
index fee908861b..610987aca9 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
@@ -1,6 +1,6 @@
 /** @file
-  Constructor code for QEMU debug port library.
-  Non-SEC instance.
+  Detection code for QEMU debug port.
+  Non-SEC instance, caches the result of detection.
 
   Copyright (c) 2017, Red Hat, Inc.<BR>
   This program and the accompanying materials
@@ -15,9 +15,16 @@
 
 #include <Base.h>
 #include <Uefi.h>
+#include "DebugLibDetect.h"
+
+//
+// Set to TRUE if the debug I/O port is enabled
+//
+STATIC BOOLEAN mDebugIoPortFound = FALSE;
 
 /**
-  This constructor function does not have anything to do.
+  This constructor function checks if the debug I/O port device is present,
+  caching the result for later use.
 
   @retval EFI_SUCCESS   The constructor always returns RETURN_SUCCESS.
 
@@ -28,5 +35,21 @@ PlatformDebugLibIoPortConstructor (
   VOID
   )
 {
+  mDebugIoPortFound = PlatformDebugLibIoPortDetect();
   return EFI_SUCCESS;
 }
+
+/**
+  Return the cached result of detecting the debug I/O port device.
+
+  @retval BOOLEAN   TRUE if the debug I/O port device was detected.
+
+**/
+BOOLEAN
+EFIAPI
+PlatformDebugLibIoPortFound (
+  VOID
+  )
+{
+  return mDebugIoPortFound;
+}
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
new file mode 100644
index 0000000000..c34ca9c72b
--- /dev/null
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
@@ -0,0 +1,56 @@
+/** @file
+  Base Debug library instance for QEMU debug port.
+  It uses PrintLib to send debug messages to a fixed I/O port.
+
+  Copyright (c) 2017, Red Hat, Inc.<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
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __DEBUG_IO_PORT_DETECT_H__
+#define __DEBUG_IO_PORT_DETECT_H__
+
+#include <Base.h>
+#include <Uefi.h>
+
+//
+// The constant value that is read from the debug I/O port
+//
+#define BOCHS_DEBUG_PORT_MAGIC    0xE9
+
+
+/**
+  Helper function to return whether the virtual machine has a debug I/O port.
+  PlatformDebugLibIoPortFound can call this function directly or cache the
+  result.
+
+  @retval BOOLEAN   TRUE if the debug I/O port device was detected.
+
+**/
+BOOLEAN
+EFIAPI
+PlatformDebugLibIoPortDetect (
+  VOID
+  );
+
+/**
+  Return whether the virtual machine has a debug I/O port.  DebugLib.c
+  calls this function instead of PlatformDebugLibIoPortDetect, to allow
+  caching if possible.
+
+  @retval BOOLEAN   TRUE if the debug I/O port device was detected.
+
+**/
+BOOLEAN
+EFIAPI
+PlatformDebugLibIoPortFound (
+  VOID
+  );
+
+#endif
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c
index 407fe613ff..f71b6567dc 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c
@@ -1,6 +1,6 @@
 /** @file
-  Constructor code for QEMU debug port library.
-  SEC instance.
+  Detection code for QEMU debug port.
+  SEC instance, cannot cache the result of detection.
 
   Copyright (c) 2017, Red Hat, Inc.<BR>
   This program and the accompanying materials
@@ -15,6 +15,7 @@
 
 #include <Base.h>
 #include <Uefi.h>
+#include "DebugLibDetect.h"
 
 /**
   This constructor function does not have to do anything.
@@ -30,3 +31,18 @@ PlatformRomDebugLibIoPortConstructor (
 {
   return EFI_SUCCESS;
 }
+
+/**
+  Return the result of detecting the debug I/O port device.
+
+  @retval BOOLEAN   TRUE if the debug I/O port device was detected.
+
+**/
+BOOLEAN
+EFIAPI
+PlatformDebugLibIoPortFound (
+  VOID
+  )
+{
+  return PlatformDebugLibIoPortDetect ();
+}
-- 
2.14.3



  parent reply	other threads:[~2017-11-16 10:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-16 10:47 [PATCH v2 0/2] OvmfPkg: save on I/O port accesses when the debug port is not in use Paolo Bonzini
2017-11-16 10:47 ` [PATCH 1/2] OvmfPkg: create a separate PlatformDebugLibIoPort instance for SEC Paolo Bonzini
2017-11-16 18:43   ` Laszlo Ersek
2017-11-16 10:47 ` Paolo Bonzini [this message]
2017-11-16 20:03   ` [PATCH 2/2] OvmfPkg: save on I/O port accesses when the debug port is not in use 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=20171116104716.15144-3-pbonzini@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