* [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
* 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
* [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 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