* [PATCH 0/2] OvmfXen: Cleanup debug options
@ 2020-04-20 16:29 Anthony PERARD
2020-04-20 16:29 ` [PATCH 1/2] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT Anthony PERARD
2020-04-20 16:29 ` [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort Anthony PERARD
0 siblings, 2 replies; 7+ messages in thread
From: Anthony PERARD @ 2020-04-20 16:29 UTC (permalink / raw)
To: devel
Cc: Laszlo Ersek, Anthony Perard, Ard Biesheuvel, Jordan Justen,
Julien Grall
Patch series available in this git branch:
git://xenbits.xen.org/people/aperard/ovmf.git br.ovmfxen-debug-io-v1
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 (2):
OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT
OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort
OvmfPkg/OvmfXen.dsc | 49 +++++--------------
.../PlatformDebugLibIoPort.inf | 6 +--
.../DebugLibDetect.h | 20 --------
.../DebugLib.c | 16 ------
.../DebugLibDetect.c | 20 +-------
5 files changed, 17 insertions(+), 94 deletions(-)
copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/PlatformDebugLibIoPort.inf (76%)
copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.h (57%)
copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLib.c (94%)
copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.c (61%)
--
Anthony PERARD
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT
2020-04-20 16:29 [PATCH 0/2] OvmfXen: Cleanup debug options Anthony PERARD
@ 2020-04-20 16:29 ` Anthony PERARD
2020-04-21 13:22 ` Laszlo Ersek
2020-04-20 16:29 ` [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort Anthony PERARD
1 sibling, 1 reply; 7+ messages in thread
From: Anthony PERARD @ 2020-04-20 16:29 UTC (permalink / raw)
To: devel
Cc: Laszlo Ersek, Anthony Perard, Ard Biesheuvel, Jordan Justen,
Julien Grall
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>
---
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] 7+ messages in thread
* [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort
2020-04-20 16:29 [PATCH 0/2] OvmfXen: Cleanup debug options Anthony PERARD
2020-04-20 16:29 ` [PATCH 1/2] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT Anthony PERARD
@ 2020-04-20 16:29 ` Anthony PERARD
2020-04-21 13:54 ` Laszlo Ersek
1 sibling, 1 reply; 7+ messages in thread
From: Anthony PERARD @ 2020-04-20 16:29 UTC (permalink / raw)
To: devel
Cc: Laszlo Ersek, Anthony Perard, Ard Biesheuvel, Jordan Justen,
Julien Grall
Introduce XenDebugLibIoPort which is enabled with
DEBUG_ON_HYPERVISOR_CONSOLE which send the debug output to Xen's
console.
It's a copy PlatformDebugLibIoPort which always write to the IO port.
It works 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>
---
Notes:
Do you think it would be better to introduce a PCD to
PlatformDebugLibIoPort where we can disable the check
for the debug port and simply always write to it?
OvmfPkg/OvmfXen.dsc | 13 ++++++++++++
.../PlatformDebugLibIoPort.inf | 6 +++---
.../DebugLibDetect.h | 20 -------------------
.../DebugLib.c | 16 ---------------
.../DebugLibDetect.c | 20 ++-----------------
5 files changed, 18 insertions(+), 57 deletions(-)
copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/PlatformDebugLibIoPort.inf (76%)
copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.h (57%)
copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLib.c (94%)
copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.c (61%)
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 4859faf1bff7..8e4d70f21fc8 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/XenDebugLibIoPort/PlatformDebugLibIoPort.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/XenDebugLibIoPort/PlatformDebugLibIoPort.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)
+ ## This flag is used to control the destination port for PlatformDebugLibIoPort
+ gUefiOvmfPkgTokenSpaceGuid.PcdDebugIoPort|0xe9
+!endif
+
# IRQs 5, 9, 10, 11 are level-triggered
gUefiOvmfPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf b/OvmfPkg/Library/XenDebugLibIoPort/PlatformDebugLibIoPort.inf
similarity index 76%
copy from OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
copy to OvmfPkg/Library/XenDebugLibIoPort/PlatformDebugLibIoPort.inf
index c09f312ffb1d..141fe7498335 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
+++ b/OvmfPkg/Library/XenDebugLibIoPort/PlatformDebugLibIoPort.inf
@@ -12,11 +12,11 @@
[Defines]
INF_VERSION = 0x00010005
- BASE_NAME = PlatformDebugLibIoPort
- FILE_GUID = DF934DA3-CD31-49FE-AF50-B3C87C79325F
+ BASE_NAME = XenDebugLibIoPort
+ FILE_GUID = 201EE63F-79DC-4B8E-953C-FA794C0883A3
MODULE_TYPE = BASE
VERSION_STRING = 1.0
- LIBRARY_CLASS = DebugLib|PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER SMM_CORE DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION
+ LIBRARY_CLASS = DebugLib|SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER SMM_CORE DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION
CONSTRUCTOR = PlatformDebugLibIoPortConstructor
#
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h b/OvmfPkg/Library/XenDebugLibIoPort/DebugLibDetect.h
similarity index 57%
copy from OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
copy to OvmfPkg/Library/XenDebugLibIoPort/DebugLibDetect.h
index 71a7f33aaf17..d5ec4063c5be 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.h
+++ b/OvmfPkg/Library/XenDebugLibIoPort/DebugLibDetect.h
@@ -12,26 +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
- result.
-
- @retval TRUE if the debug I/O port device was detected.
- @retval FALSE otherwise
-
-**/
-BOOLEAN
-EFIAPI
-PlatformDebugLibIoPortDetect (
- VOID
- );
/**
Return whether the virtual machine has a debug I/O port. DebugLib.c
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/XenDebugLibIoPort/DebugLib.c
similarity index 94%
copy from OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
copy to OvmfPkg/Library/XenDebugLibIoPort/DebugLib.c
index 3dfa3126c3d0..b3952c9936da 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
+++ b/OvmfPkg/Library/XenDebugLibIoPort/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;
-}
diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c b/OvmfPkg/Library/XenDebugLibIoPort/DebugLibDetect.c
similarity index 61%
copy from OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
copy to OvmfPkg/Library/XenDebugLibIoPort/DebugLibDetect.c
index 2c659e7aea0a..04de924f3fa2 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
+++ b/OvmfPkg/Library/XenDebugLibIoPort/DebugLibDetect.c
@@ -1,6 +1,5 @@
/** @file
Detection code for QEMU debug port.
- Non-SEC instance, caches the result of detection.
Copyright (c) 2017, Red Hat, Inc.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -10,16 +9,6 @@
#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 must not do anything.
@@ -44,8 +33,7 @@ PlatformDebugLibIoPortConstructor (
}
/**
- 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.
+ 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
@@ -57,9 +45,5 @@ PlatformDebugLibIoPortFound (
VOID
)
{
- if (!mDebugIoPortChecked) {
- mDebugIoPortFound = PlatformDebugLibIoPortDetect ();
- mDebugIoPortChecked = TRUE;
- }
- return mDebugIoPortFound;
+ return TRUE;
}
--
Anthony PERARD
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT
2020-04-20 16:29 ` [PATCH 1/2] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT Anthony PERARD
@ 2020-04-21 13:22 ` Laszlo Ersek
0 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2020-04-21 13:22 UTC (permalink / raw)
To: Anthony PERARD, devel; +Cc: Ard Biesheuvel, Jordan Justen, Julien Grall
On 04/20/20 18:29, 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>
> ---
> 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: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort
2020-04-20 16:29 ` [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort Anthony PERARD
@ 2020-04-21 13:54 ` Laszlo Ersek
2020-04-22 10:18 ` Anthony PERARD
0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2020-04-21 13:54 UTC (permalink / raw)
To: Anthony PERARD, devel; +Cc: Ard Biesheuvel, Jordan Justen, Julien Grall
On 04/20/20 18:29, Anthony PERARD wrote:
> Introduce XenDebugLibIoPort which is enabled with
> DEBUG_ON_HYPERVISOR_CONSOLE which send the debug output to Xen's
> console.
>
> It's a copy PlatformDebugLibIoPort which always write to the IO port.
> It works 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>
> ---
>
> Notes:
> Do you think it would be better to introduce a PCD to
> PlatformDebugLibIoPort where we can disable the check
> for the debug port and simply always write to it?
>
> OvmfPkg/OvmfXen.dsc | 13 ++++++++++++
> .../PlatformDebugLibIoPort.inf | 6 +++---
> .../DebugLibDetect.h | 20 -------------------
> .../DebugLib.c | 16 ---------------
> .../DebugLibDetect.c | 20 ++-----------------
> 5 files changed, 18 insertions(+), 57 deletions(-)
> copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/PlatformDebugLibIoPort.inf (76%)
> copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.h (57%)
> copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLib.c (94%)
> copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.c (61%)
I think this approach is a good first approximation, but I think we can
refine it.
Rather than introducing a new directory, we should operate within
"OvmfPkg/Library/PlatformDebugLibIoPort".
(1) Please write a patch that replaces all instances of the string
"QEMU" with "hypervisor" in the following files:
- DebugLib.c
- DebugLibDetect.c
- DebugLibDetect.h
- DebugLibDetectRom.c
(2) Please write a patch for the following:
- move the PlatformDebugLibIoPortDetect() function definition to a
separate file called "DebugIoPortQemu.c",
- add the new C file to both existent INF files:
- PlatformDebugLibIoPort.inf
- PlatformRomDebugLibIoPort.inf
(keep the source file lists alphabetically sorted)
- from "DebugLibDetect.h", move the BOCHS_DEBUG_PORT_MAGIC macro
definition to "DebugIoPortQemu.c".
I think this patch can be called:
OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection
After this patch, we well have two independent dimensions, for any
particular INF file:
- in PlatformDebugLibIoPortFound(), the library instance may or may not
cache the result of debug port checking
- in PlatformDebugLibIoPortDetect(), the library instance may abstract
the debug port detection method
(3) Please write another patch:
- introduce "DebugIoPortNocheck.c",
- implement PlatformDebugLibIoPortDetect() as "return TRUE",
- introduce a new INF file "PlatformRomDebugLibIoPortNocheck.inf", for
linking together:
- DebugIoPortNocheck.c
- DebugLib.c
- DebugLibDetectRom.c
This library instance will not cache the result of the detection, and
the detection will return constant TRUE.
(4) Then, this library instance can be put to use in OvmfXen.dsc
(together with setting the proper PCD value). The library instance can
be used in all firmware phases / module types.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort
2020-04-21 13:54 ` Laszlo Ersek
@ 2020-04-22 10:18 ` Anthony PERARD
2020-04-22 16:10 ` Laszlo Ersek
0 siblings, 1 reply; 7+ messages in thread
From: Anthony PERARD @ 2020-04-22 10:18 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: devel, Ard Biesheuvel, Jordan Justen, Julien Grall
On Tue, Apr 21, 2020 at 03:54:26PM +0200, Laszlo Ersek wrote:
> (2) Please write a patch for the following:
>
> - move the PlatformDebugLibIoPortDetect() function definition to a
> separate file called "DebugIoPortQemu.c",
On this new file, should I copy the copyright lines from DebugLib.c or
should I add a new copyright line?
The rest sounds good, I'll prepare all the patches.
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort
2020-04-22 10:18 ` Anthony PERARD
@ 2020-04-22 16:10 ` Laszlo Ersek
0 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2020-04-22 16:10 UTC (permalink / raw)
To: Anthony PERARD; +Cc: devel, Ard Biesheuvel, Jordan Justen, Julien Grall
On 04/22/20 12:18, Anthony PERARD wrote:
> On Tue, Apr 21, 2020 at 03:54:26PM +0200, Laszlo Ersek wrote:
>> (2) Please write a patch for the following:
>>
>> - move the PlatformDebugLibIoPortDetect() function definition to a
>> separate file called "DebugIoPortQemu.c",
>
> On this new file, should I copy the copyright lines from DebugLib.c or
> should I add a new copyright line?
Both -- copy the old (C) notices, and add your own too, at the top.
> The rest sounds good, I'll prepare all the patches.
Thank you, I'm relieved you are OK with the request!
Laszlo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-22 16:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-20 16:29 [PATCH 0/2] OvmfXen: Cleanup debug options Anthony PERARD
2020-04-20 16:29 ` [PATCH 1/2] OvmfPkg/OvmfXen: Remove DEBUG_ON_SERIAL_PORT Anthony PERARD
2020-04-21 13:22 ` Laszlo Ersek
2020-04-20 16:29 ` [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort Anthony PERARD
2020-04-21 13:54 ` Laszlo Ersek
2020-04-22 10:18 ` Anthony PERARD
2020-04-22 16:10 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox