public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/5] OvmfXen: Cleanup debug options
@ 2020-04-23  9:53 Anthony PERARD
  2020-04-23  9:53 ` [PATCH v2 1/5] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT Anthony PERARD
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Anthony PERARD @ 2020-04-23  9:53 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, Julien Grall,
	Anthony Perard

Patch series available in this git branch:
git://xenbits.xen.org/people/aperard/ovmf.git br.ovmfxen-debug-io-v2

v2:
- reuse PlatformDebugLibIoPort rather than duplicate it.

Remove non working DEBUG_ON_SERIAL_PORT, then add a new LibIoPort which always
writes to the IO port.

Issues was discovered and discuss in this mail threads:
    <e1f7e62c-adb4-eace-3828-7d73ae59ecd4@bsdio.com>
    "OvmfPkg XenPkg: X64 DEBUG GCC5 -DDEBUG_ON_SERIAL_PORT=TRUE build is broken"

Cheers,

Anthony PERARD (5):
  OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT
  OvmfPkg/PlatformDebugLibIoPort: Reword QEMU to hypervisor
  OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection
  OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant
  OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag

 OvmfPkg/OvmfXen.dsc                           | 49 +++++--------------
 .../PlatformDebugLibIoPort.inf                |  1 +
 .../PlatformRomDebugLibIoPort.inf             |  1 +
 ...f => PlatformRomDebugLibIoPortNocheck.inf} | 14 +++---
 .../PlatformDebugLibIoPort/DebugLibDetect.h   |  8 +--
 .../DebugIoPortNocheck.c                      | 25 ++++++++++
 .../PlatformDebugLibIoPort/DebugIoPortQemu.c  | 34 +++++++++++++
 .../Library/PlatformDebugLibIoPort/DebugLib.c | 18 +------
 .../PlatformDebugLibIoPort/DebugLibDetect.c   |  2 +-
 .../DebugLibDetectRom.c                       |  2 +-
 10 files changed, 84 insertions(+), 70 deletions(-)
 copy OvmfPkg/Library/PlatformDebugLibIoPort/{PlatformRomDebugLibIoPort.inf => PlatformRomDebugLibIoPortNocheck.inf} (65%)
 create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c
 create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT
  2020-04-23  9:53 [PATCH v2 0/5] OvmfXen: Cleanup debug options Anthony PERARD
@ 2020-04-23  9:53 ` Anthony PERARD
  2020-04-23 10:52   ` [edk2-devel] " Philippe Mathieu-Daudé
  2020-04-23  9:53 ` [PATCH v2 2/5] OvmfPkg/PlatformDebugLibIoPort: Reword QEMU to hypervisor Anthony PERARD
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Anthony PERARD @ 2020-04-23  9:53 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, Julien Grall,
	Anthony Perard

Remove support for DEBUG_ON_SERIAL_PORT because OvmfXen can't be build
with it due to a circular dependency:
  DebugLib        : BaseDebugLibSerialPort ->
  SerialPortLib   : XenConsoleSerialPortLib ->
  XenHypercallLib : XenHypercallLib ->
  DebugLib

Also, if that dependency is fixed, I think it would be harder to find
which console the debug is sent to when running an HVM guest. The xen
console isn't the serial console used by default. Furthermore,
XenHypercallLib isn't initialised early enough, so we would loose
debug output from the SEC phase and early PEI phase.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/OvmfXen.dsc | 40 +---------------------------------------
 1 file changed, 1 insertion(+), 39 deletions(-)

diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 47ee8db8b884..4859faf1bff7 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -205,17 +205,14 @@ [LibraryClasses]
   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
   TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
   RealTimeClockLib|OvmfPkg/Library/XenRealTimeClockLib/XenRealTimeClockLib.inf
+  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
 
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
 
 [LibraryClasses.common.SEC]
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
-!ifdef $(DEBUG_ON_SERIAL_PORT)
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!else
   DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
-!endif
   ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
   ExtractGuidedSectionLib|MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
 !if $(SOURCE_DEBUG_ENABLE) == TRUE
@@ -236,11 +233,6 @@ [LibraryClasses.common.PEI_CORE]
   ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
-!ifdef $(DEBUG_ON_SERIAL_PORT)
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!else
-  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
-!endif
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
 
 [LibraryClasses.common.PEIM]
@@ -252,11 +244,6 @@ [LibraryClasses.common.PEIM]
   ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
-!ifdef $(DEBUG_ON_SERIAL_PORT)
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!else
-  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
-!endif
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
   ResourcePublicationLib|MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.inf
   ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
@@ -274,11 +261,6 @@ [LibraryClasses.common.DXE_CORE]
   DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
   MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
-!ifdef $(DEBUG_ON_SERIAL_PORT)
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!else
-  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
-!endif
   ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
 !if $(SOURCE_DEBUG_ENABLE) == TRUE
   DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
@@ -292,11 +274,6 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
-!ifdef $(DEBUG_ON_SERIAL_PORT)
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!else
-  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
-!endif
   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
@@ -308,11 +285,6 @@ [LibraryClasses.common.UEFI_DRIVER]
   DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
-!ifdef $(DEBUG_ON_SERIAL_PORT)
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!else
-  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
-!endif
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
 
@@ -322,11 +294,6 @@ [LibraryClasses.common.DXE_DRIVER]
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
-!ifdef $(DEBUG_ON_SERIAL_PORT)
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!else
-  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
-!endif
   PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
   QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
@@ -344,11 +311,6 @@ [LibraryClasses.common.UEFI_APPLICATION]
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
-!ifdef $(DEBUG_ON_SERIAL_PORT)
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!else
-  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
-!endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
 
 ################################################################################
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] OvmfPkg/PlatformDebugLibIoPort: Reword QEMU to hypervisor
  2020-04-23  9:53 [PATCH v2 0/5] OvmfXen: Cleanup debug options Anthony PERARD
  2020-04-23  9:53 ` [PATCH v2 1/5] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT Anthony PERARD
@ 2020-04-23  9:53 ` Anthony PERARD
  2020-04-24 17:48   ` [edk2-devel] " Laszlo Ersek
  2020-04-23  9:53 ` [PATCH v2 3/5] OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection Anthony PERARD
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Anthony PERARD @ 2020-04-23  9:53 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, Julien Grall,
	Anthony Perard

We are going to reuse PlatformDebugLibIoPort to use debug IO port from
hypervisors that aren't QEMU, so reword "QEMU" to "hypervisor" in the
descriptions.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h    | 2 +-
 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c          | 2 +-
 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c    | 2 +-
 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
index 71a7f33aaf17..4677c85ac3c4 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
@@ -1,5 +1,5 @@
 /** @file
-  Base Debug library instance for QEMU debug port.
+  Base Debug library instance for hypervisor debug port.
   It uses PrintLib to send debug messages to a fixed I/O port.
 
   Copyright (c) 2017, Red Hat, Inc.<BR>
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
index 3dfa3126c3d0..ec2e677afd8d 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
@@ -1,5 +1,5 @@
 /** @file
-  Base Debug library instance for QEMU debug port.
+  Base Debug library instance for hypervisor debug port.
   It uses PrintLib to send debug messages to a fixed I/O port.
 
   Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
index 2c659e7aea0a..8c466e64e0a6 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
@@ -1,5 +1,5 @@
 /** @file
-  Detection code for QEMU debug port.
+  Detection code for hypervisor debug port.
   Non-SEC instance, caches the result of detection.
 
   Copyright (c) 2017, Red Hat, Inc.<BR>
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c
index 03ba4a0a7b08..8cbdbd94fc0e 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c
@@ -1,5 +1,5 @@
 /** @file
-  Detection code for QEMU debug port.
+  Detection code for hypervisor debug port.
   SEC instance, cannot cache the result of detection.
 
   Copyright (c) 2017, Red Hat, Inc.<BR>
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/5] OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection
  2020-04-23  9:53 [PATCH v2 0/5] OvmfXen: Cleanup debug options Anthony PERARD
  2020-04-23  9:53 ` [PATCH v2 1/5] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT Anthony PERARD
  2020-04-23  9:53 ` [PATCH v2 2/5] OvmfPkg/PlatformDebugLibIoPort: Reword QEMU to hypervisor Anthony PERARD
@ 2020-04-23  9:53 ` Anthony PERARD
  2020-04-23 10:50   ` [edk2-devel] " Philippe Mathieu-Daudé
  2020-04-24 17:52   ` Laszlo Ersek
  2020-04-23  9:53 ` [PATCH v2 4/5] OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant Anthony PERARD
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 15+ messages in thread
From: Anthony PERARD @ 2020-04-23  9:53 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, Julien Grall,
	Anthony Perard

Factor out debug port detection in PlatformDebugLibIoPort.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 .../PlatformDebugLibIoPort.inf                |  1 +
 .../PlatformRomDebugLibIoPort.inf             |  1 +
 .../PlatformDebugLibIoPort/DebugLibDetect.h   |  6 ----
 .../PlatformDebugLibIoPort/DebugIoPortQemu.c  | 34 +++++++++++++++++++
 .../Library/PlatformDebugLibIoPort/DebugLib.c | 16 ---------
 5 files changed, 36 insertions(+), 22 deletions(-)
 create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c

diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
index c09f312ffb1d..94ab9105077a 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
@@ -24,6 +24,7 @@ [Defines]
 #
 
 [Sources]
+  DebugIoPortQemu.c
   DebugLib.c
   DebugLibDetect.c
   DebugLibDetect.h
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
index ab27f6327a38..8f721d249dd5 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
@@ -24,6 +24,7 @@ [Defines]
 #
 
 [Sources]
+  DebugIoPortQemu.c
   DebugLib.c
   DebugLibDetect.h
   DebugLibDetectRom.c
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
index 4677c85ac3c4..6d08909dbc58 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
@@ -12,12 +12,6 @@
 
 #include <Base.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
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c
new file mode 100644
index 000000000000..bf9119807a6c
--- /dev/null
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c
@@ -0,0 +1,34 @@
+/** @file
+  Detection code for QEMU debug port.
+
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2012, Red Hat, Inc.<BR>
+  Copyright (c) 2020, Citrix Systems, Inc.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include "DebugLibDetect.h"
+
+//
+// The constant value that is read from the debug I/O port
+//
+#define BOCHS_DEBUG_PORT_MAGIC    0xE9
+
+/**
+  Return the result of detecting the debug I/O port device.
+
+  @retval TRUE   if the debug I/O port device was detected.
+  @retval FALSE  otherwise
+
+**/
+BOOLEAN
+EFIAPI
+PlatformDebugLibIoPortDetect (
+  VOID
+  )
+{
+  return IoRead8 (PcdGet16 (PcdDebugIoPort)) == BOCHS_DEBUG_PORT_MAGIC;
+}
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
index ec2e677afd8d..dffb20822d18 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
@@ -359,19 +359,3 @@ DebugPrintLevelEnabled (
 {
   return (BOOLEAN) ((ErrorLevel & PcdGet32(PcdFixedDebugPrintErrorLevel)) != 0);
 }
-
-/**
-  Return the result of detecting the debug I/O port device.
-
-  @retval TRUE   if the debug I/O port device was detected.
-  @retval FALSE  otherwise
-
-**/
-BOOLEAN
-EFIAPI
-PlatformDebugLibIoPortDetect (
-  VOID
-  )
-{
-  return IoRead8 (PcdGet16 (PcdDebugIoPort)) == BOCHS_DEBUG_PORT_MAGIC;
-}
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/5] OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant
  2020-04-23  9:53 [PATCH v2 0/5] OvmfXen: Cleanup debug options Anthony PERARD
                   ` (2 preceding siblings ...)
  2020-04-23  9:53 ` [PATCH v2 3/5] OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection Anthony PERARD
@ 2020-04-23  9:53 ` Anthony PERARD
  2020-04-24 17:58   ` [edk2-devel] " Laszlo Ersek
  2020-04-23  9:53 ` [PATCH v2 5/5] OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag Anthony PERARD
  2020-04-28 21:23 ` [edk2-devel] [PATCH v2 0/5] OvmfXen: Cleanup debug options Laszlo Ersek
  5 siblings, 1 reply; 15+ messages in thread
From: Anthony PERARD @ 2020-04-23  9:53 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, Julien Grall,
	Anthony Perard

Introduce PlatformRomDebugLibIoPortNocheck which doesn't try to detect
the debug IO port. Instead, debug logs are always written to the IO port.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 ...f => PlatformRomDebugLibIoPortNocheck.inf} | 15 ++++++-----
 .../DebugIoPortNocheck.c                      | 25 +++++++++++++++++++
 2 files changed, 32 insertions(+), 8 deletions(-)
 copy OvmfPkg/Library/PlatformDebugLibIoPort/{PlatformRomDebugLibIoPort.inf => PlatformRomDebugLibIoPortNocheck.inf} (65%)
 create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c

diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
similarity index 65%
copy from OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
copy to OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
index 8f721d249dd5..34034e1eb887 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
@@ -1,9 +1,8 @@
 ## @file
-#  Instance of Debug Library for the QEMU debug console port.
+#  Instance of Debug Library for an hypervisor debug console port.
 #  It uses Print Library to produce formatted output strings.
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
-#  Copyright (c) 2017, Red Hat, Inc.<BR>
+#  Copyright (c) 2020, Citrix Systems, Inc.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -12,11 +11,11 @@
 
 [Defines]
   INF_VERSION                    = 0x00010005
-  BASE_NAME                      = PlatformRomDebugLibIoPort
-  FILE_GUID                      = CEB0D9D3-328F-4C24-8C02-28FA1986AE1B
+  BASE_NAME                      = PlatformRomDebugLibIoPortNocheck
+  FILE_GUID                      = 92AEB68E-C2CF-466E-9AB2-3F5E713F7DE6
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = DebugLib|SEC
+  LIBRARY_CLASS                  = DebugLib|SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER SMM_CORE DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION
   CONSTRUCTOR                    = PlatformRomDebugLibIoPortConstructor
 
 #
@@ -24,10 +23,10 @@ [Defines]
 #
 
 [Sources]
-  DebugIoPortQemu.c
+  DebugIoPortNocheck.c
   DebugLib.c
-  DebugLibDetect.h
   DebugLibDetectRom.c
+  DebugLibDetect.h
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c
new file mode 100644
index 000000000000..0ef7920a8fb8
--- /dev/null
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c
@@ -0,0 +1,25 @@
+/** @file
+  Dectection code for hypervisor debug port.
+
+  Copyright (c) 2020, Citrix Systems, Inc.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "DebugLibDetect.h"
+
+/**
+  Always return TRUE without detection as the debug I/O port is always
+  present.
+
+  @retval TRUE   The debug I/O port is always present.
+
+**/
+BOOLEAN
+EFIAPI
+PlatformDebugLibIoPortDetect (
+  VOID
+  )
+{
+  return TRUE;
+}
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 5/5] OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag
  2020-04-23  9:53 [PATCH v2 0/5] OvmfXen: Cleanup debug options Anthony PERARD
                   ` (3 preceding siblings ...)
  2020-04-23  9:53 ` [PATCH v2 4/5] OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant Anthony PERARD
@ 2020-04-23  9:53 ` Anthony PERARD
  2020-04-24 18:05   ` [edk2-devel] " Laszlo Ersek
  2020-04-28 21:23 ` [edk2-devel] [PATCH v2 0/5] OvmfXen: Cleanup debug options Laszlo Ersek
  5 siblings, 1 reply; 15+ messages in thread
From: Anthony PERARD @ 2020-04-23  9:53 UTC (permalink / raw)
  To: devel
  Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, Julien Grall,
	Anthony Perard

Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag to enable logging
debug output to the Xen console.

This will work with both Xen HVM guest and Xen PVH guest whereas the
default PlatformDebugLibIoPort works only in HVM when QEMU is present.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 OvmfPkg/OvmfXen.dsc | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 4859faf1bff7..0a8fd26990a3 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -205,14 +205,22 @@ [LibraryClasses]
   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
   TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
   RealTimeClockLib|OvmfPkg/Library/XenRealTimeClockLib/XenRealTimeClockLib.inf
+!ifdef $(DEBUG_ON_HYPERVISOR_CONSOLE)
+  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
+!else
   DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
+!endif
 
 [LibraryClasses.common]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
 
 [LibraryClasses.common.SEC]
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
+!ifdef $(DEBUG_ON_HYPERVISOR_CONSOLE)
+  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
+!else
   DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
+!endif
   ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
   ExtractGuidedSectionLib|MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
 !if $(SOURCE_DEBUG_ENABLE) == TRUE
@@ -405,6 +413,11 @@ [PcdsFixedAtBuild]
   #
 !include NetworkPkg/NetworkPcds.dsc.inc
 
+!ifdef $(DEBUG_ON_HYPERVISOR_CONSOLE)
+  ## Set Xen's debug IO port for PlatformDebugLibIoPort
+  gUefiOvmfPkgTokenSpaceGuid.PcdDebugIoPort|0xe9
+!endif
+
   # IRQs 5, 9, 10, 11 are level-triggered
   gUefiOvmfPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20
 
-- 
Anthony PERARD


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 3/5] OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection
  2020-04-23  9:53 ` [PATCH v2 3/5] OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection Anthony PERARD
@ 2020-04-23 10:50   ` Philippe Mathieu-Daudé
  2020-04-24 17:52   ` Laszlo Ersek
  1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 10:50 UTC (permalink / raw)
  To: devel, anthony.perard
  Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, Julien Grall

On 4/23/20 11:53 AM, Anthony PERARD wrote:
> Factor out debug port detection in PlatformDebugLibIoPort.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>   .../PlatformDebugLibIoPort.inf                |  1 +
>   .../PlatformRomDebugLibIoPort.inf             |  1 +
>   .../PlatformDebugLibIoPort/DebugLibDetect.h   |  6 ----
>   .../PlatformDebugLibIoPort/DebugIoPortQemu.c  | 34 +++++++++++++++++++
>   .../Library/PlatformDebugLibIoPort/DebugLib.c | 16 ---------
>   5 files changed, 36 insertions(+), 22 deletions(-)
>   create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c
> 
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> index c09f312ffb1d..94ab9105077a 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> @@ -24,6 +24,7 @@ [Defines]
>   #
>   
>   [Sources]
> +  DebugIoPortQemu.c
>     DebugLib.c
>     DebugLibDetect.c
>     DebugLibDetect.h
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> index ab27f6327a38..8f721d249dd5 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> @@ -24,6 +24,7 @@ [Defines]
>   #
>   
>   [Sources]
> +  DebugIoPortQemu.c
>     DebugLib.c
>     DebugLibDetect.h
>     DebugLibDetectRom.c
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> index 4677c85ac3c4..6d08909dbc58 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> @@ -12,12 +12,6 @@
>   
>   #include <Base.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
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c
> new file mode 100644
> index 000000000000..bf9119807a6c
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c
> @@ -0,0 +1,34 @@
> +/** @file
> +  Detection code for QEMU debug port.
> +
> +  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2012, Red Hat, Inc.<BR>
> +  Copyright (c) 2020, Citrix Systems, Inc.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/IoLib.h>
> +#include <Library/PcdLib.h>
> +#include "DebugLibDetect.h"
> +
> +//
> +// The constant value that is read from the debug I/O port
> +//
> +#define BOCHS_DEBUG_PORT_MAGIC    0xE9
> +
> +/**
> +  Return the result of detecting the debug I/O port device.
> +
> +  @retval TRUE   if the debug I/O port device was detected.
> +  @retval FALSE  otherwise
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +PlatformDebugLibIoPortDetect (
> +  VOID
> +  )
> +{
> +  return IoRead8 (PcdGet16 (PcdDebugIoPort)) == BOCHS_DEBUG_PORT_MAGIC;
> +}
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> index ec2e677afd8d..dffb20822d18 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> @@ -359,19 +359,3 @@ DebugPrintLevelEnabled (
>   {
>     return (BOOLEAN) ((ErrorLevel & PcdGet32(PcdFixedDebugPrintErrorLevel)) != 0);
>   }
> -
> -/**
> -  Return the result of detecting the debug I/O port device.
> -
> -  @retval TRUE   if the debug I/O port device was detected.
> -  @retval FALSE  otherwise
> -
> -**/
> -BOOLEAN
> -EFIAPI
> -PlatformDebugLibIoPortDetect (
> -  VOID
> -  )
> -{
> -  return IoRead8 (PcdGet16 (PcdDebugIoPort)) == BOCHS_DEBUG_PORT_MAGIC;
> -}
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 1/5] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT
  2020-04-23  9:53 ` [PATCH v2 1/5] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT Anthony PERARD
@ 2020-04-23 10:52   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 10:52 UTC (permalink / raw)
  To: devel, anthony.perard
  Cc: Laszlo Ersek, Ard Biesheuvel, Jordan Justen, Julien Grall

On 4/23/20 11:53 AM, Anthony PERARD wrote:
> Remove support for DEBUG_ON_SERIAL_PORT because OvmfXen can't be build
> with it due to a circular dependency:
>    DebugLib        : BaseDebugLibSerialPort ->
>    SerialPortLib   : XenConsoleSerialPortLib ->
>    XenHypercallLib : XenHypercallLib ->
>    DebugLib
> 
> Also, if that dependency is fixed, I think it would be harder to find
> which console the debug is sent to when running an HVM guest. The xen
> console isn't the serial console used by default. Furthermore,
> XenHypercallLib isn't initialised early enough, so we would loose
> debug output from the SEC phase and early PEI phase.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   OvmfPkg/OvmfXen.dsc | 40 +---------------------------------------
>   1 file changed, 1 insertion(+), 39 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 47ee8db8b884..4859faf1bff7 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -205,17 +205,14 @@ [LibraryClasses]
>     Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
>     TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>     RealTimeClockLib|OvmfPkg/Library/XenRealTimeClockLib/XenRealTimeClockLib.inf
> +  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
>   
>   [LibraryClasses.common]
>     BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>   
>   [LibraryClasses.common.SEC]
>     QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
>     DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> -!endif
>     ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
>     ExtractGuidedSectionLib|MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
>   !if $(SOURCE_DEBUG_ENABLE) == TRUE
> @@ -236,11 +233,6 @@ [LibraryClasses.common.PEI_CORE]
>     ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
> -  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> -!endif
>     PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
>   
>   [LibraryClasses.common.PEIM]
> @@ -252,11 +244,6 @@ [LibraryClasses.common.PEIM]
>     ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
>     OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>     PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
> -  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> -!endif
>     PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
>     ResourcePublicationLib|MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.inf
>     ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
> @@ -274,11 +261,6 @@ [LibraryClasses.common.DXE_CORE]
>     DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
>     MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
>     ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
> -  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> -!endif
>     ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
>   !if $(SOURCE_DEBUG_ENABLE) == TRUE
>     DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
> @@ -292,11 +274,6 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>     DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
>     MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>     ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
> -  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> -!endif
>     UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>     BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>     PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> @@ -308,11 +285,6 @@ [LibraryClasses.common.UEFI_DRIVER]
>     DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
>     MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>     ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
> -  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> -!endif
>     UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
>     PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>   
> @@ -322,11 +294,6 @@ [LibraryClasses.common.DXE_DRIVER]
>     MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>     ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
>     UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
> -  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> -!endif
>     PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>     PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
>     QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> @@ -344,11 +311,6 @@ [LibraryClasses.common.UEFI_APPLICATION]
>     HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>     MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>     ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> -!ifdef $(DEBUG_ON_SERIAL_PORT)
> -  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!else
> -  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> -!endif
>     PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>   
>   ################################################################################
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 2/5] OvmfPkg/PlatformDebugLibIoPort: Reword QEMU to hypervisor
  2020-04-23  9:53 ` [PATCH v2 2/5] OvmfPkg/PlatformDebugLibIoPort: Reword QEMU to hypervisor Anthony PERARD
@ 2020-04-24 17:48   ` Laszlo Ersek
  0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2020-04-24 17:48 UTC (permalink / raw)
  To: devel, anthony.perard; +Cc: Ard Biesheuvel, Jordan Justen, Julien Grall

On 04/23/20 11:53, Anthony PERARD wrote:
> We are going to reuse PlatformDebugLibIoPort to use debug IO port from
> hypervisors that aren't QEMU, so reword "QEMU" to "hypervisor" in the
> descriptions.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h    | 2 +-
>  OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c          | 2 +-
>  OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c    | 2 +-
>  OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> index 71a7f33aaf17..4677c85ac3c4 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> @@ -1,5 +1,5 @@
>  /** @file
> -  Base Debug library instance for QEMU debug port.
> +  Base Debug library instance for hypervisor debug port.
>    It uses PrintLib to send debug messages to a fixed I/O port.
>  
>    Copyright (c) 2017, Red Hat, Inc.<BR>
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> index 3dfa3126c3d0..ec2e677afd8d 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> @@ -1,5 +1,5 @@
>  /** @file
> -  Base Debug library instance for QEMU debug port.
> +  Base Debug library instance for hypervisor debug port.
>    It uses PrintLib to send debug messages to a fixed I/O port.
>  
>    Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
> index 2c659e7aea0a..8c466e64e0a6 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
> @@ -1,5 +1,5 @@
>  /** @file
> -  Detection code for QEMU debug port.
> +  Detection code for hypervisor debug port.
>    Non-SEC instance, caches the result of detection.
>  
>    Copyright (c) 2017, Red Hat, Inc.<BR>
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c
> index 03ba4a0a7b08..8cbdbd94fc0e 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetectRom.c
> @@ -1,5 +1,5 @@
>  /** @file
> -  Detection code for QEMU debug port.
> +  Detection code for hypervisor debug port.
>    SEC instance, cannot cache the result of detection.
>  
>    Copyright (c) 2017, Red Hat, Inc.<BR>
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 3/5] OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection
  2020-04-23  9:53 ` [PATCH v2 3/5] OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection Anthony PERARD
  2020-04-23 10:50   ` [edk2-devel] " Philippe Mathieu-Daudé
@ 2020-04-24 17:52   ` Laszlo Ersek
  1 sibling, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2020-04-24 17:52 UTC (permalink / raw)
  To: devel, anthony.perard; +Cc: Ard Biesheuvel, Jordan Justen, Julien Grall

On 04/23/20 11:53, Anthony PERARD wrote:
> Factor out debug port detection in PlatformDebugLibIoPort.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  .../PlatformDebugLibIoPort.inf                |  1 +
>  .../PlatformRomDebugLibIoPort.inf             |  1 +
>  .../PlatformDebugLibIoPort/DebugLibDetect.h   |  6 ----
>  .../PlatformDebugLibIoPort/DebugIoPortQemu.c  | 34 +++++++++++++++++++
>  .../Library/PlatformDebugLibIoPort/DebugLib.c | 16 ---------
>  5 files changed, 36 insertions(+), 22 deletions(-)
>  create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c
> 
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> index c09f312ffb1d..94ab9105077a 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> @@ -24,6 +24,7 @@ [Defines]
>  #
>  
>  [Sources]
> +  DebugIoPortQemu.c
>    DebugLib.c
>    DebugLibDetect.c
>    DebugLibDetect.h
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> index ab27f6327a38..8f721d249dd5 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> @@ -24,6 +24,7 @@ [Defines]
>  #
>  
>  [Sources]
> +  DebugIoPortQemu.c
>    DebugLib.c
>    DebugLibDetect.h
>    DebugLibDetectRom.c
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> index 4677c85ac3c4..6d08909dbc58 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> @@ -12,12 +12,6 @@
>  
>  #include <Base.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
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c
> new file mode 100644
> index 000000000000..bf9119807a6c
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c
> @@ -0,0 +1,34 @@
> +/** @file
> +  Detection code for QEMU debug port.
> +
> +  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2012, Red Hat, Inc.<BR>
> +  Copyright (c) 2020, Citrix Systems, Inc.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Library/IoLib.h>
> +#include <Library/PcdLib.h>
> +#include "DebugLibDetect.h"
> +
> +//
> +// The constant value that is read from the debug I/O port
> +//
> +#define BOCHS_DEBUG_PORT_MAGIC    0xE9
> +
> +/**
> +  Return the result of detecting the debug I/O port device.
> +
> +  @retval TRUE   if the debug I/O port device was detected.
> +  @retval FALSE  otherwise
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +PlatformDebugLibIoPortDetect (
> +  VOID
> +  )
> +{
> +  return IoRead8 (PcdGet16 (PcdDebugIoPort)) == BOCHS_DEBUG_PORT_MAGIC;
> +}
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> index ec2e677afd8d..dffb20822d18 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
> @@ -359,19 +359,3 @@ DebugPrintLevelEnabled (
>  {
>    return (BOOLEAN) ((ErrorLevel & PcdGet32(PcdFixedDebugPrintErrorLevel)) != 0);
>  }
> -
> -/**
> -  Return the result of detecting the debug I/O port device.
> -
> -  @retval TRUE   if the debug I/O port device was detected.
> -  @retval FALSE  otherwise
> -
> -**/
> -BOOLEAN
> -EFIAPI
> -PlatformDebugLibIoPortDetect (
> -  VOID
> -  )
> -{
> -  return IoRead8 (PcdGet16 (PcdDebugIoPort)) == BOCHS_DEBUG_PORT_MAGIC;
> -}
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 4/5] OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant
  2020-04-23  9:53 ` [PATCH v2 4/5] OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant Anthony PERARD
@ 2020-04-24 17:58   ` Laszlo Ersek
  2020-04-27 14:19     ` Anthony PERARD
  0 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2020-04-24 17:58 UTC (permalink / raw)
  To: devel, anthony.perard; +Cc: Ard Biesheuvel, Jordan Justen, Julien Grall

On 04/23/20 11:53, Anthony PERARD wrote:
> Introduce PlatformRomDebugLibIoPortNocheck which doesn't try to detect
> the debug IO port. Instead, debug logs are always written to the IO port.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  ...f => PlatformRomDebugLibIoPortNocheck.inf} | 15 ++++++-----
>  .../DebugIoPortNocheck.c                      | 25 +++++++++++++++++++
>  2 files changed, 32 insertions(+), 8 deletions(-)
>  copy OvmfPkg/Library/PlatformDebugLibIoPort/{PlatformRomDebugLibIoPort.inf => PlatformRomDebugLibIoPortNocheck.inf} (65%)
>  create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c
> 
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
> similarity index 65%
> copy from OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> copy to OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
> index 8f721d249dd5..34034e1eb887 100644
> --- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
> @@ -1,9 +1,8 @@
>  ## @file
> -#  Instance of Debug Library for the QEMU debug console port.
> +#  Instance of Debug Library for an hypervisor debug console port.
>  #  It uses Print Library to produce formatted output strings.
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> -#  Copyright (c) 2017, Red Hat, Inc.<BR>
> +#  Copyright (c) 2020, Citrix Systems, Inc.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -12,11 +11,11 @@
>  
>  [Defines]
>    INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = PlatformRomDebugLibIoPort
> -  FILE_GUID                      = CEB0D9D3-328F-4C24-8C02-28FA1986AE1B
> +  BASE_NAME                      = PlatformRomDebugLibIoPortNocheck
> +  FILE_GUID                      = 92AEB68E-C2CF-466E-9AB2-3F5E713F7DE6
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
> -  LIBRARY_CLASS                  = DebugLib|SEC
> +  LIBRARY_CLASS                  = DebugLib|SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER SMM_CORE DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION

(1) The simple way to say the same is to just drop the "|..." part :)

But I can fix this up for you.

>    CONSTRUCTOR                    = PlatformRomDebugLibIoPortConstructor
>  
>  #
> @@ -24,10 +23,10 @@ [Defines]
>  #
>  
>  [Sources]
> -  DebugIoPortQemu.c
> +  DebugIoPortNocheck.c
>    DebugLib.c
> -  DebugLibDetect.h
>    DebugLibDetectRom.c
> +  DebugLibDetect.h

(2) The re-ordering of "DebugLibDetect.h" is curious. Maybe you may have
an LC_COLLATE setting that sorts "." after "R"?

Anyway I can re-sort this when I merge the series.

>  
>  [Packages]
>    MdePkg/MdePkg.dec
> diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c
> new file mode 100644
> index 000000000000..0ef7920a8fb8
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c
> @@ -0,0 +1,25 @@
> +/** @file
> +  Dectection code for hypervisor debug port.
> +
> +  Copyright (c) 2020, Citrix Systems, Inc.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include "DebugLibDetect.h"
> +
> +/**
> +  Always return TRUE without detection as the debug I/O port is always
> +  present.
> +
> +  @retval TRUE   The debug I/O port is always present.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +PlatformDebugLibIoPortDetect (
> +  VOID
> +  )
> +{
> +  return TRUE;
> +}
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag
  2020-04-23  9:53 ` [PATCH v2 5/5] OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag Anthony PERARD
@ 2020-04-24 18:05   ` Laszlo Ersek
  2020-04-27 14:22     ` Anthony PERARD
  0 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2020-04-24 18:05 UTC (permalink / raw)
  To: devel, anthony.perard; +Cc: Ard Biesheuvel, Jordan Justen, Julien Grall

On 04/23/20 11:53, Anthony PERARD wrote:
> Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag to enable logging
> debug output to the Xen console.
> 
> This will work with both Xen HVM guest and Xen PVH guest whereas the
> default PlatformDebugLibIoPort works only in HVM when QEMU is present.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  OvmfPkg/OvmfXen.dsc | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 4859faf1bff7..0a8fd26990a3 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -205,14 +205,22 @@ [LibraryClasses]
>    Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
>    TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>    RealTimeClockLib|OvmfPkg/Library/XenRealTimeClockLib/XenRealTimeClockLib.inf
> +!ifdef $(DEBUG_ON_HYPERVISOR_CONSOLE)
> +  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
> +!else
>    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> +!endif
>  
>  [LibraryClasses.common]
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>  
>  [LibraryClasses.common.SEC]
>    QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> +!ifdef $(DEBUG_ON_HYPERVISOR_CONSOLE)
> +  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
> +!else
>    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> +!endif

(1) This part can be simplified with an !ifndef, instead, I think. (When
DEBUG_ON_HYPERVISOR_CONSOLE is defined, then the default resolution can
take effect for SEC too.)

If you wish I can change this for you as well. Alternatively, if you'd
rather repost, that's OK too. (And maybe Phil intends to review more of
the patches in this series; I'm not sure.)

>    ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
>    ExtractGuidedSectionLib|MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
>  !if $(SOURCE_DEBUG_ENABLE) == TRUE
> @@ -405,6 +413,11 @@ [PcdsFixedAtBuild]
>    #
>  !include NetworkPkg/NetworkPcds.dsc.inc
>  
> +!ifdef $(DEBUG_ON_HYPERVISOR_CONSOLE)
> +  ## Set Xen's debug IO port for PlatformDebugLibIoPort
> +  gUefiOvmfPkgTokenSpaceGuid.PcdDebugIoPort|0xe9
> +!endif
> +
>    # IRQs 5, 9, 10, 11 are level-triggered
>    gUefiOvmfPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20
>  
> 

With (1) updated:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 4/5] OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant
  2020-04-24 17:58   ` [edk2-devel] " Laszlo Ersek
@ 2020-04-27 14:19     ` Anthony PERARD
  0 siblings, 0 replies; 15+ messages in thread
From: Anthony PERARD @ 2020-04-27 14:19 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: devel, Ard Biesheuvel, Jordan Justen, Julien Grall

On Fri, Apr 24, 2020 at 07:58:48PM +0200, Laszlo Ersek wrote:
> On 04/23/20 11:53, Anthony PERARD wrote:
> > @@ -24,10 +23,10 @@ [Defines]
> >  #
> >  
> >  [Sources]
> > -  DebugIoPortQemu.c
> > +  DebugIoPortNocheck.c
> >    DebugLib.c
> > -  DebugLibDetect.h
> >    DebugLibDetectRom.c
> > +  DebugLibDetect.h
> 
> (2) The re-ordering of "DebugLibDetect.h" is curious. Maybe you may have
> an LC_COLLATE setting that sorts "." after "R"?

I think it's more likely that I've copied the .inf that uses
"DebugLibDetect.c", edited that line to add "Rom" and forgot to resort
the list.

> Anyway I can re-sort this when I merge the series.
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks.

-- 
Anthony PERARD

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 5/5] OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag
  2020-04-24 18:05   ` [edk2-devel] " Laszlo Ersek
@ 2020-04-27 14:22     ` Anthony PERARD
  0 siblings, 0 replies; 15+ messages in thread
From: Anthony PERARD @ 2020-04-27 14:22 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: devel, Ard Biesheuvel, Jordan Justen, Julien Grall

On Fri, Apr 24, 2020 at 08:05:53PM +0200, Laszlo Ersek wrote:
> On 04/23/20 11:53, Anthony PERARD wrote:
> >  [LibraryClasses.common.SEC]
> >    QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
> > +!ifdef $(DEBUG_ON_HYPERVISOR_CONSOLE)
> > +  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
> > +!else
> >    DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
> > +!endif
> 
> (1) This part can be simplified with an !ifndef, instead, I think. (When
> DEBUG_ON_HYPERVISOR_CONSOLE is defined, then the default resolution can
> take effect for SEC too.)

Sounds good.

> If you wish I can change this for you as well. Alternatively, if you'd
> rather repost, that's OK too. (And maybe Phil intends to review more of
> the patches in this series; I'm not sure.)

Feel free to edit on commit.

> With (1) updated:
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks,

-- 
Anthony PERARD

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v2 0/5] OvmfXen: Cleanup debug options
  2020-04-23  9:53 [PATCH v2 0/5] OvmfXen: Cleanup debug options Anthony PERARD
                   ` (4 preceding siblings ...)
  2020-04-23  9:53 ` [PATCH v2 5/5] OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag Anthony PERARD
@ 2020-04-28 21:23 ` Laszlo Ersek
  5 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2020-04-28 21:23 UTC (permalink / raw)
  To: devel, anthony.perard
  Cc: Ard Biesheuvel, Jordan Justen, Julien Grall,
	Philippe Mathieu-Daudé

On 04/23/20 11:53, Anthony PERARD wrote:
> Patch series available in this git branch:
> git://xenbits.xen.org/people/aperard/ovmf.git br.ovmfxen-debug-io-v2
>
> v2:
> - reuse PlatformDebugLibIoPort rather than duplicate it.
>
> Remove non working DEBUG_ON_SERIAL_PORT, then add a new LibIoPort which always
> writes to the IO port.
>
> Issues was discovered and discuss in this mail threads:
>     <e1f7e62c-adb4-eace-3828-7d73ae59ecd4@bsdio.com>
>     "OvmfPkg XenPkg: X64 DEBUG GCC5 -DDEBUG_ON_SERIAL_PORT=TRUE build is broken"
>
> Cheers,
>
> Anthony PERARD (5):
>   OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT
>   OvmfPkg/PlatformDebugLibIoPort: Reword QEMU to hypervisor
>   OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection
>   OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant
>   OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag
>
>  OvmfPkg/OvmfXen.dsc                           | 49 +++++--------------
>  .../PlatformDebugLibIoPort.inf                |  1 +
>  .../PlatformRomDebugLibIoPort.inf             |  1 +
>  ...f => PlatformRomDebugLibIoPortNocheck.inf} | 14 +++---
>  .../PlatformDebugLibIoPort/DebugLibDetect.h   |  8 +--
>  .../DebugIoPortNocheck.c                      | 25 ++++++++++
>  .../PlatformDebugLibIoPort/DebugIoPortQemu.c  | 34 +++++++++++++
>  .../Library/PlatformDebugLibIoPort/DebugLib.c | 18 +------
>  .../PlatformDebugLibIoPort/DebugLibDetect.c   |  2 +-
>  .../DebugLibDetectRom.c                       |  2 +-
>  10 files changed, 84 insertions(+), 70 deletions(-)
>  copy OvmfPkg/Library/PlatformDebugLibIoPort/{PlatformRomDebugLibIoPort.inf => PlatformRomDebugLibIoPortNocheck.inf} (65%)
>  create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortNocheck.c
>  create mode 100644 OvmfPkg/Library/PlatformDebugLibIoPort/DebugIoPortQemu.c
>

Merged as commit range 099dfbb29d8b..3a402f961143, via
<https://github.com/tianocore/edk2/pull/554>, with the following changes
(listed with git-range-diff):

> 1:  30e967c8caff ! 1:  8131bcf75724 OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT
>     @@ -18,6 +18,7 @@
>          Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Message-Id: <20200423095358.2518197-2-anthony.perard@citrix.com>
>     +    Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>      diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
>      --- a/OvmfPkg/OvmfXen.dsc
> 2:  f5074c29bed8 ! 2:  d81604376c2c OvmfPkg/PlatformDebugLibIoPort: Reword QEMU to hypervisor
>     @@ -8,6 +8,7 @@
>
>          Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>          Message-Id: <20200423095358.2518197-3-anthony.perard@citrix.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
>      --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
> 3:  f34d57687c4f ! 3:  b2d70c6189ac OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection
>     @@ -6,6 +6,8 @@
>
>          Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>          Message-Id: <20200423095358.2518197-4-anthony.perard@citrix.com>
>     +    Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
>      --- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
> 4:  c095623a5cfb ! 4:  37f050ea8228 OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant
>     @@ -7,6 +7,7 @@
>
>          Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>          Message-Id: <20200423095358.2518197-5-anthony.perard@citrix.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf b/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
>      new file mode 100644
>     @@ -30,7 +31,7 @@
>      +  FILE_GUID                      = 92AEB68E-C2CF-466E-9AB2-3F5E713F7DE6
>      +  MODULE_TYPE                    = BASE
>      +  VERSION_STRING                 = 1.0
>     -+  LIBRARY_CLASS                  = DebugLib|SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER SMM_CORE DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION
>     ++  LIBRARY_CLASS                  = DebugLib
>      +  CONSTRUCTOR                    = PlatformRomDebugLibIoPortConstructor
>      +
>      +#
>     @@ -40,8 +41,8 @@
>      +[Sources]
>      +  DebugIoPortNocheck.c
>      +  DebugLib.c
>     -+  DebugLibDetectRom.c
>      +  DebugLibDetect.h
>     ++  DebugLibDetectRom.c
>      +
>      +[Packages]
>      +  MdePkg/MdePkg.dec
> 5:  979f76d57653 ! 5:  c6f8d17f53f9 OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag
>     @@ -10,6 +10,7 @@
>
>          Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>          Message-Id: <20200423095358.2518197-6-anthony.perard@citrix.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
>      --- a/OvmfPkg/OvmfXen.dsc
>     @@ -29,9 +30,7 @@
>
>       [LibraryClasses.common.SEC]
>         QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSecLib.inf
>     -+!ifdef $(DEBUG_ON_HYPERVISOR_CONSOLE)
>     -+  DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPortNocheck.inf
>     -+!else
>     ++!ifndef $(DEBUG_ON_HYPERVISOR_CONSOLE)
>         DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebugLibIoPort.inf
>      +!endif
>         ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-04-28 21:23 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-23  9:53 [PATCH v2 0/5] OvmfXen: Cleanup debug options Anthony PERARD
2020-04-23  9:53 ` [PATCH v2 1/5] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT Anthony PERARD
2020-04-23 10:52   ` [edk2-devel] " Philippe Mathieu-Daudé
2020-04-23  9:53 ` [PATCH v2 2/5] OvmfPkg/PlatformDebugLibIoPort: Reword QEMU to hypervisor Anthony PERARD
2020-04-24 17:48   ` [edk2-devel] " Laszlo Ersek
2020-04-23  9:53 ` [PATCH v2 3/5] OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection Anthony PERARD
2020-04-23 10:50   ` [edk2-devel] " Philippe Mathieu-Daudé
2020-04-24 17:52   ` Laszlo Ersek
2020-04-23  9:53 ` [PATCH v2 4/5] OvmfPkg/PlatformDebugLibIoPort: Introduce a Nocheck variant Anthony PERARD
2020-04-24 17:58   ` [edk2-devel] " Laszlo Ersek
2020-04-27 14:19     ` Anthony PERARD
2020-04-23  9:53 ` [PATCH v2 5/5] OvmfPkg/OvmfXen: Introduce DEBUG_ON_HYPERVISOR_CONSOLE build flag Anthony PERARD
2020-04-24 18:05   ` [edk2-devel] " Laszlo Ersek
2020-04-27 14:22     ` Anthony PERARD
2020-04-28 21:23 ` [edk2-devel] [PATCH v2 0/5] OvmfXen: Cleanup debug options Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox