| Message ID | 20190704144233.27968-33-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Specific platform to run OVMF in Xen PVH and HVM guests | expand |
On 07/04/19 16:42, Anthony PERARD wrote: > On a Xen PVH guest, none of the existing serial or console interface > works, so we add a new one, based on XenConsoleSerialPortLib, and > implemented via SerialDxe. > > That is a simple console implementation that can works on both PVH > guest and HVM guests, even if it rarely going to be use on HVM. > > Have PlatformBootManagerLib look for the new console, when running as a > Xen guest. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689 > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > > Notes: > v3: > - removed PciSioSerialDxe and IsaSerialDxe from OvmfXen, since they > would not be used, maybe, to check. > - some coding style fix > > - not changed: PciSioSerialDxe: even if we add SerialDxe, we still needs > PciSioSerialDxe to have OVMF use the emulated serial port on HVM. > > v2: > - Use MdeModulePkg/Universal/SerialDxe instead of something new. > - Have PlatformInitializeConsole() look for it by using the > known-in-advance device path for the xen console in the > PLATFORM_CONSOLE_CONNECT_ENTRY. > > OvmfPkg/OvmfXen.dsc | 4 ++ > OvmfPkg/OvmfXen.fdf | 1 + > .../PlatformBootManagerLib.inf | 4 ++ > .../PlatformBootManagerLib/BdsPlatform.h | 1 + > .../PlatformBootManagerLib/BdsPlatform.c | 3 +- > .../PlatformBootManagerLib/PlatformData.c | 59 ++++++++++++++++++- > 6 files changed, 69 insertions(+), 3 deletions(-) > > diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc > index 1ecae3fb45..487bada64d 100644 > --- a/OvmfPkg/OvmfXen.dsc > +++ b/OvmfPkg/OvmfXen.dsc > @@ -586,6 +586,10 @@ [Components] > OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf > OvmfPkg/XenBusDxe/XenBusDxe.inf > OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf > + MdeModulePkg/Universal/SerialDxe/SerialDxe.inf { > + <LibraryClasses> > + SerialPortLib|OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf > + } > MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf > MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf > MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf > diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf > index fa0830a324..5c1a925d6a 100644 > --- a/OvmfPkg/OvmfXen.fdf > +++ b/OvmfPkg/OvmfXen.fdf > @@ -312,6 +312,7 @@ [FV.DXEFV] > INF OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf > INF OvmfPkg/XenBusDxe/XenBusDxe.inf > INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf > +INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf > > INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf > INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > index b2d3b4fb4d..646a1c522c 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > @@ -61,6 +61,10 @@ [Pcd] > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut > + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES > + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES > + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES > + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES > > [Pcd.IA32, Pcd.X64] > gEfiMdePkgTokenSpaceGuid.PcdFSBClock > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h > index 49a072b400..153e215101 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h > +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h > @@ -165,6 +165,7 @@ typedef struct { > #define CONSOLE_IN BIT1 > #define STD_ERROR BIT2 > extern PLATFORM_CONSOLE_CONNECT_ENTRY gPlatformConsole[]; > +extern PLATFORM_CONSOLE_CONNECT_ENTRY gXenPlatformConsole[]; > > // > // Platform BDS Functions > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > index 9ae590293a..ee6ee6608f 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > @@ -399,7 +399,8 @@ PlatformBootManagerBeforeConsole ( > // > EfiBootManagerDispatchDeferredImages (); > > - PlatformInitializeConsole (gPlatformConsole); > + PlatformInitializeConsole ( > + XenDetected() ? gXenPlatformConsole : gPlatformConsole); > PcdStatus = PcdSet16S (PcdPlatformBootTimeOut, > GetFrontPageTimeoutFromQemu ()); > ASSERT_RETURN_ERROR (PcdStatus); > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c > index 36aab784d7..a9b1fe274a 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c > +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c > @@ -9,18 +9,19 @@ > > #include "BdsPlatform.h" > #include <Guid/QemuRamfb.h> > +#include <Guid/SerialPortLibVendor.h> > > // > // Debug Agent UART Device Path structure > // > -#pragma pack(1) > +#pragma pack (1) > typedef struct { > VENDOR_DEVICE_PATH VendorHardware; > UART_DEVICE_PATH Uart; > VENDOR_DEVICE_PATH TerminalType; > EFI_DEVICE_PATH_PROTOCOL End; > } VENDOR_UART_DEVICE_PATH; > -#pragma pack() > +#pragma pack () > > // > // USB Keyboard Device Path structure > @@ -43,6 +44,18 @@ typedef struct { > } VENDOR_RAMFB_DEVICE_PATH; > #pragma pack () > > +// > +// Xen Console Device Path structure > +// > +#pragma pack(1) > +typedef struct { > + VENDOR_DEVICE_PATH VendorHardware; > + UART_DEVICE_PATH Uart; > + VENDOR_DEVICE_PATH TerminalType; > + EFI_DEVICE_PATH_PROTOCOL End; > +} XEN_CONSOLE_DEVICE_PATH; > +#pragma pack() > + This version of the patch addresses all of my v2 review comments (either by code changes or by explanations in the Notes section) -- thanks for that. However, when you arrived at my reuqest (6) in <http://mid.mail-archive.com/7d6adf5d-baca-7e9c-68ef-2f8479bbd902@redhat.com>, and searched the source file for "pack(" -- in order to insert a space character before the opening paren --, the match was *not* around the new XEN_CONSOLE_DEVICE_PATH structure. Instead, it was around the preexistent VENDOR_UART_DEVICE_PATH structure. And so you fixed the style for the old code, and not the new code. But: that's actually useful. Because now that I'm looking at VENDOR_UART_DEVICE_PATH, it seems that we don't need the new type XEN_CONSOLE_DEVICE_PATH at all. Is that right? So: (1) Please drop XEN_CONSOLE_DEVICE_PATH. (2) Please replace the comment Debug Agent UART Device Path structure with Vendor UART Device Path structure on VENDOR_UART_DEVICE_PATH. (3) Please preserve the "misplaced" whitespace fix, for "pack(", around VENDOR_UART_DEVICE_PATH. (4) Please use VENDOR_UART_DEVICE_PATH as the type of gXenConsoleDevicePath. With those: Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo > ACPI_HID_DEVICE_PATH gPnpPs2KeyboardDeviceNode = gPnpPs2Keyboard; > ACPI_HID_DEVICE_PATH gPnp16550ComPortDeviceNode = gPnp16550ComPort; > UART_DEVICE_PATH gUartDeviceNode = gUart; > @@ -141,6 +154,37 @@ STATIC VENDOR_RAMFB_DEVICE_PATH gQemuRamfbDevicePath = { > gEndEntire > }; > > +STATIC XEN_CONSOLE_DEVICE_PATH gXenConsoleDevicePath = { > + { > + { > + HARDWARE_DEVICE_PATH, > + HW_VENDOR_DP, > + { > + (UINT8) (sizeof (VENDOR_DEVICE_PATH)), > + (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8) > + } > + }, > + EDKII_SERIAL_PORT_LIB_VENDOR_GUID > + }, > + { > + { > + MESSAGING_DEVICE_PATH, > + MSG_UART_DP, > + { > + (UINT8) (sizeof (UART_DEVICE_PATH)), > + (UINT8) ((sizeof (UART_DEVICE_PATH)) >> 8) > + } > + }, > + 0, > + FixedPcdGet64 (PcdUartDefaultBaudRate), > + FixedPcdGet8 (PcdUartDefaultDataBits), > + FixedPcdGet8 (PcdUartDefaultParity), > + FixedPcdGet8 (PcdUartDefaultStopBits), > + }, > + gPcAnsiTerminal, > + gEndEntire > +}; > + > // > // Predefined platform default console device path > // > @@ -163,6 +207,17 @@ PLATFORM_CONSOLE_CONNECT_ENTRY gPlatformConsole[] = { > } > }; > > +PLATFORM_CONSOLE_CONNECT_ENTRY gXenPlatformConsole[] = { > + { > + (EFI_DEVICE_PATH_PROTOCOL *)&gXenConsoleDevicePath, > + (CONSOLE_OUT | CONSOLE_IN | STD_ERROR) > + }, > + { > + NULL, > + 0 > + } > +}; > + > // > // Predefined platform connect sequence > // >
On Wed, Jul 10, 2019 at 12:48:57PM +0200, Laszlo Ersek wrote: > On 07/04/19 16:42, Anthony PERARD wrote: > > On a Xen PVH guest, none of the existing serial or console interface > > works, so we add a new one, based on XenConsoleSerialPortLib, and > > implemented via SerialDxe. > > > > That is a simple console implementation that can works on both PVH > > guest and HVM guests, even if it rarely going to be use on HVM. > > > > Have PlatformBootManagerLib look for the new console, when running as a > > Xen guest. > > > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689 > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > --- > > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c > > index 36aab784d7..a9b1fe274a 100644 > > --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c > > +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c > > @@ -9,18 +9,19 @@ > > > > #include "BdsPlatform.h" > > #include <Guid/QemuRamfb.h> > > +#include <Guid/SerialPortLibVendor.h> > > > > // > > // Debug Agent UART Device Path structure > > // > > -#pragma pack(1) > > +#pragma pack (1) > > typedef struct { > > VENDOR_DEVICE_PATH VendorHardware; > > UART_DEVICE_PATH Uart; > > VENDOR_DEVICE_PATH TerminalType; > > EFI_DEVICE_PATH_PROTOCOL End; > > } VENDOR_UART_DEVICE_PATH; > > -#pragma pack() > > +#pragma pack () > > > > // > > // USB Keyboard Device Path structure > > @@ -43,6 +44,18 @@ typedef struct { > > } VENDOR_RAMFB_DEVICE_PATH; > > #pragma pack () > > > > +// > > +// Xen Console Device Path structure > > +// > > +#pragma pack(1) > > +typedef struct { > > + VENDOR_DEVICE_PATH VendorHardware; > > + UART_DEVICE_PATH Uart; > > + VENDOR_DEVICE_PATH TerminalType; > > + EFI_DEVICE_PATH_PROTOCOL End; > > +} XEN_CONSOLE_DEVICE_PATH; > > +#pragma pack() > > + > > This version of the patch addresses all of my v2 review comments (either > by code changes or by explanations in the Notes section) -- thanks for that. > > However, when you arrived at my reuqest (6) in > <http://mid.mail-archive.com/7d6adf5d-baca-7e9c-68ef-2f8479bbd902@redhat.com>, > and searched the source file for "pack(" -- in order to insert a space > character before the opening paren --, the match was *not* around the > new XEN_CONSOLE_DEVICE_PATH structure. Instead, it was around the > preexistent VENDOR_UART_DEVICE_PATH structure. And so you fixed the > style for the old code, and not the new code. > > But: that's actually useful. Because now that I'm looking at > VENDOR_UART_DEVICE_PATH, it seems that we don't need the new type > XEN_CONSOLE_DEVICE_PATH at all. Is that right? So: > > (1) Please drop XEN_CONSOLE_DEVICE_PATH. > > (2) Please replace the comment > > Debug Agent UART Device Path structure > > with > > Vendor UART Device Path structure > > on VENDOR_UART_DEVICE_PATH. > > (3) Please preserve the "misplaced" whitespace fix, for "pack(", around > VENDOR_UART_DEVICE_PATH. > > (4) Please use VENDOR_UART_DEVICE_PATH as the type of gXenConsoleDevicePath. > > With those: > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> I'm going to add the following to the commit message: Since we use VENDOR_UART_DEVICE_PATH, fix its description and coding style.
On 07/22/19 19:06, Anthony PERARD wrote: > On Wed, Jul 10, 2019 at 12:48:57PM +0200, Laszlo Ersek wrote: >> On 07/04/19 16:42, Anthony PERARD wrote: >>> On a Xen PVH guest, none of the existing serial or console interface >>> works, so we add a new one, based on XenConsoleSerialPortLib, and >>> implemented via SerialDxe. >>> >>> That is a simple console implementation that can works on both PVH >>> guest and HVM guests, even if it rarely going to be use on HVM. >>> >>> Have PlatformBootManagerLib look for the new console, when running as a >>> Xen guest. >>> >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689 >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >>> --- > >>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c >>> index 36aab784d7..a9b1fe274a 100644 >>> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c >>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c >>> @@ -9,18 +9,19 @@ >>> >>> #include "BdsPlatform.h" >>> #include <Guid/QemuRamfb.h> >>> +#include <Guid/SerialPortLibVendor.h> >>> >>> // >>> // Debug Agent UART Device Path structure >>> // >>> -#pragma pack(1) >>> +#pragma pack (1) >>> typedef struct { >>> VENDOR_DEVICE_PATH VendorHardware; >>> UART_DEVICE_PATH Uart; >>> VENDOR_DEVICE_PATH TerminalType; >>> EFI_DEVICE_PATH_PROTOCOL End; >>> } VENDOR_UART_DEVICE_PATH; >>> -#pragma pack() >>> +#pragma pack () >>> >>> // >>> // USB Keyboard Device Path structure >>> @@ -43,6 +44,18 @@ typedef struct { >>> } VENDOR_RAMFB_DEVICE_PATH; >>> #pragma pack () >>> >>> +// >>> +// Xen Console Device Path structure >>> +// >>> +#pragma pack(1) >>> +typedef struct { >>> + VENDOR_DEVICE_PATH VendorHardware; >>> + UART_DEVICE_PATH Uart; >>> + VENDOR_DEVICE_PATH TerminalType; >>> + EFI_DEVICE_PATH_PROTOCOL End; >>> +} XEN_CONSOLE_DEVICE_PATH; >>> +#pragma pack() >>> + >> >> This version of the patch addresses all of my v2 review comments (either >> by code changes or by explanations in the Notes section) -- thanks for that. >> >> However, when you arrived at my reuqest (6) in >> <http://mid.mail-archive.com/7d6adf5d-baca-7e9c-68ef-2f8479bbd902@redhat.com>, >> and searched the source file for "pack(" -- in order to insert a space >> character before the opening paren --, the match was *not* around the >> new XEN_CONSOLE_DEVICE_PATH structure. Instead, it was around the >> preexistent VENDOR_UART_DEVICE_PATH structure. And so you fixed the >> style for the old code, and not the new code. >> >> But: that's actually useful. Because now that I'm looking at >> VENDOR_UART_DEVICE_PATH, it seems that we don't need the new type >> XEN_CONSOLE_DEVICE_PATH at all. Is that right? So: >> >> (1) Please drop XEN_CONSOLE_DEVICE_PATH. >> >> (2) Please replace the comment >> >> Debug Agent UART Device Path structure >> >> with >> >> Vendor UART Device Path structure >> >> on VENDOR_UART_DEVICE_PATH. >> >> (3) Please preserve the "misplaced" whitespace fix, for "pack(", around >> VENDOR_UART_DEVICE_PATH. >> >> (4) Please use VENDOR_UART_DEVICE_PATH as the type of gXenConsoleDevicePath. >> >> With those: >> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > I'm going to add the following to the commit message: > > Since we use VENDOR_UART_DEVICE_PATH, fix its description and > coding style. > > Thanks! Laszlo
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc index 1ecae3fb45..487bada64d 100644 --- a/OvmfPkg/OvmfXen.dsc +++ b/OvmfPkg/OvmfXen.dsc @@ -586,6 +586,10 @@ [Components] OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf OvmfPkg/XenBusDxe/XenBusDxe.inf OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf + MdeModulePkg/Universal/SerialDxe/SerialDxe.inf { + <LibraryClasses> + SerialPortLib|OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf + } MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf index fa0830a324..5c1a925d6a 100644 --- a/OvmfPkg/OvmfXen.fdf +++ b/OvmfPkg/OvmfXen.fdf @@ -312,6 +312,7 @@ [FV.DXEFV] INF OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf INF OvmfPkg/XenBusDxe/XenBusDxe.inf INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf +INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf index b2d3b4fb4d..646a1c522c 100644 --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf @@ -61,6 +61,10 @@ [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity ## CONSUMES + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits ## CONSUMES [Pcd.IA32, Pcd.X64] gEfiMdePkgTokenSpaceGuid.PcdFSBClock diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h index 49a072b400..153e215101 100644 --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h @@ -165,6 +165,7 @@ typedef struct { #define CONSOLE_IN BIT1 #define STD_ERROR BIT2 extern PLATFORM_CONSOLE_CONNECT_ENTRY gPlatformConsole[]; +extern PLATFORM_CONSOLE_CONNECT_ENTRY gXenPlatformConsole[]; // // Platform BDS Functions diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c index 9ae590293a..ee6ee6608f 100644 --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c @@ -399,7 +399,8 @@ PlatformBootManagerBeforeConsole ( // EfiBootManagerDispatchDeferredImages (); - PlatformInitializeConsole (gPlatformConsole); + PlatformInitializeConsole ( + XenDetected() ? gXenPlatformConsole : gPlatformConsole); PcdStatus = PcdSet16S (PcdPlatformBootTimeOut, GetFrontPageTimeoutFromQemu ()); ASSERT_RETURN_ERROR (PcdStatus); diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c index 36aab784d7..a9b1fe274a 100644 --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c @@ -9,18 +9,19 @@ #include "BdsPlatform.h" #include <Guid/QemuRamfb.h> +#include <Guid/SerialPortLibVendor.h> // // Debug Agent UART Device Path structure // -#pragma pack(1) +#pragma pack (1) typedef struct { VENDOR_DEVICE_PATH VendorHardware; UART_DEVICE_PATH Uart; VENDOR_DEVICE_PATH TerminalType; EFI_DEVICE_PATH_PROTOCOL End; } VENDOR_UART_DEVICE_PATH; -#pragma pack() +#pragma pack () // // USB Keyboard Device Path structure @@ -43,6 +44,18 @@ typedef struct { } VENDOR_RAMFB_DEVICE_PATH; #pragma pack () +// +// Xen Console Device Path structure +// +#pragma pack(1) +typedef struct { + VENDOR_DEVICE_PATH VendorHardware; + UART_DEVICE_PATH Uart; + VENDOR_DEVICE_PATH TerminalType; + EFI_DEVICE_PATH_PROTOCOL End; +} XEN_CONSOLE_DEVICE_PATH; +#pragma pack() + ACPI_HID_DEVICE_PATH gPnpPs2KeyboardDeviceNode = gPnpPs2Keyboard; ACPI_HID_DEVICE_PATH gPnp16550ComPortDeviceNode = gPnp16550ComPort; UART_DEVICE_PATH gUartDeviceNode = gUart; @@ -141,6 +154,37 @@ STATIC VENDOR_RAMFB_DEVICE_PATH gQemuRamfbDevicePath = { gEndEntire }; +STATIC XEN_CONSOLE_DEVICE_PATH gXenConsoleDevicePath = { + { + { + HARDWARE_DEVICE_PATH, + HW_VENDOR_DP, + { + (UINT8) (sizeof (VENDOR_DEVICE_PATH)), + (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8) + } + }, + EDKII_SERIAL_PORT_LIB_VENDOR_GUID + }, + { + { + MESSAGING_DEVICE_PATH, + MSG_UART_DP, + { + (UINT8) (sizeof (UART_DEVICE_PATH)), + (UINT8) ((sizeof (UART_DEVICE_PATH)) >> 8) + } + }, + 0, + FixedPcdGet64 (PcdUartDefaultBaudRate), + FixedPcdGet8 (PcdUartDefaultDataBits), + FixedPcdGet8 (PcdUartDefaultParity), + FixedPcdGet8 (PcdUartDefaultStopBits), + }, + gPcAnsiTerminal, + gEndEntire +}; + // // Predefined platform default console device path // @@ -163,6 +207,17 @@ PLATFORM_CONSOLE_CONNECT_ENTRY gPlatformConsole[] = { } }; +PLATFORM_CONSOLE_CONNECT_ENTRY gXenPlatformConsole[] = { + { + (EFI_DEVICE_PATH_PROTOCOL *)&gXenConsoleDevicePath, + (CONSOLE_OUT | CONSOLE_IN | STD_ERROR) + }, + { + NULL, + 0 + } +}; + // // Predefined platform connect sequence //
On a Xen PVH guest, none of the existing serial or console interface works, so we add a new one, based on XenConsoleSerialPortLib, and implemented via SerialDxe. That is a simple console implementation that can works on both PVH guest and HVM guests, even if it rarely going to be use on HVM. Have PlatformBootManagerLib look for the new console, when running as a Xen guest. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689 Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v3: - removed PciSioSerialDxe and IsaSerialDxe from OvmfXen, since they would not be used, maybe, to check. - some coding style fix - not changed: PciSioSerialDxe: even if we add SerialDxe, we still needs PciSioSerialDxe to have OVMF use the emulated serial port on HVM. v2: - Use MdeModulePkg/Universal/SerialDxe instead of something new. - Have PlatformInitializeConsole() look for it by using the known-in-advance device path for the xen console in the PLATFORM_CONSOLE_CONNECT_ENTRY. OvmfPkg/OvmfXen.dsc | 4 ++ OvmfPkg/OvmfXen.fdf | 1 + .../PlatformBootManagerLib.inf | 4 ++ .../PlatformBootManagerLib/BdsPlatform.h | 1 + .../PlatformBootManagerLib/BdsPlatform.c | 3 +- .../PlatformBootManagerLib/PlatformData.c | 59 ++++++++++++++++++- 6 files changed, 69 insertions(+), 3 deletions(-)