[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 32/35] OvmfPkg/PlatformBootManagerLib: Use a Xen console for ConOut/ConIn
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@xxxxxxxxxx> > > --- > > 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@xxxxxxxxxx>, > 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@xxxxxxxxxx> I'm going to add the following to the commit message: Since we use VENDOR_UART_DEVICE_PATH, fix its description and coding style. -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |