|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [edk2] [PATCH RFC 12/14] OvmfPkg/PlatformBootManagerLib: Use a Xen console for ConOut/ConIn
On 12/08/16 16:33, Anthony PERARD wrote:
> and add OvmfPkg/XenConsoleIo/XenConsoleIo to XenOvmf platform.
>
> It actually look for gEfiSerialIoProtocolGuid.
> ---
> .../Library/PlatformBootManagerLib/BdsPlatform.c | 33
> ++++++++++++++++++++++
> .../PlatformBootManagerLib.inf | 2 ++
> OvmfPkg/XenOvmf.dsc | 4 +++
> OvmfPkg/XenOvmf.fdf | 1 +
> 4 files changed, 40 insertions(+)
>
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index bd64cc3..b8972f7 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -904,6 +904,31 @@ DetectAndPreparePlatformPciDevicePaths (
> return VisitAllPciInstances (DetectAndPreparePlatformPciDevicePath);
> }
>
> +#include <Protocol/SerialIo.h>
> +EFI_STATUS
> +EFIAPI
> +add_serial (
> + IN EFI_HANDLE DeviceHandle,
> + IN VOID *Instance,
> + IN VOID *Context
> + )
> +{
> + EFI_DEVICE_PATH_PROTOCOL *DevicePath = NULL;
> +
> + DevicePath = DevicePathFromHandle(DeviceHandle);
> + if (DevicePath == NULL) {
> + return EFI_NOT_FOUND;
> + }
> +
> + DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL
> *)&gTerminalTypeDeviceNode);
> + DEBUG((EFI_D_ERROR, "%a %d: full path: %s\n", __FUNCTION__, __LINE__,
> + ConvertDevicePathToText(DevicePath, TRUE, FALSE)
> + ));
> + EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
> + EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
> + EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL);
> + return EFI_SUCCESS;
> +}
>
> VOID
> PlatformInitializeConsole (
> @@ -931,6 +956,14 @@ Arguments:
> GetEfiGlobalVariable2 (EFI_CON_OUT_VARIABLE_NAME, (VOID **) &VarConout,
> NULL);
> GetEfiGlobalVariable2 (EFI_CON_IN_VARIABLE_NAME, (VOID **) &VarConin,
> NULL);
>
> + // do xen console
> + //VISIT_PCI_INSTANCE_CALLBACK CallBackFunction
> + VisitAllInstancesOfProtocol (
> + &gEfiSerialIoProtocolGuid,
> + add_serial,
> + (VOID*)NULL
> + );
> +
> if (VarConout == NULL || VarConin == NULL) {
> //
> // Do platform specific PCI Device check and add them to ConOut, ConIn,
> ErrOut
> diff --git
> a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 4a6bece..74ab6b1 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -73,6 +73,8 @@
> gEfiLoadedImageProtocolGuid # PROTOCOL SOMETIMES_PRODUCED
> gEfiFirmwareVolume2ProtocolGuid # PROTOCOL SOMETIMES_CONSUMED
>
> + gEfiSerialIoProtocolGuid
> +
> [Guids]
> gEfiXenInfoGuid
> gEfiEndOfDxeEventGroupGuid
> diff --git a/OvmfPkg/XenOvmf.dsc b/OvmfPkg/XenOvmf.dsc
> index 31a2185..8bce996 100644
> --- a/OvmfPkg/XenOvmf.dsc
> +++ b/OvmfPkg/XenOvmf.dsc
> @@ -590,6 +590,10 @@
> OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
> OvmfPkg/XenBusDxe/XenBusDxe.inf
> OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> + OvmfPkg/XenConsoleIo/XenConsoleIo.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/XenOvmf.fdf b/OvmfPkg/XenOvmf.fdf
> index f6876d7..a40d186 100644
> --- a/OvmfPkg/XenOvmf.fdf
> +++ b/OvmfPkg/XenOvmf.fdf
> @@ -223,6 +223,7 @@ INF OvmfPkg/BlockMmioToBlockIoDxe/BlockIo.inf
> INF OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
> INF OvmfPkg/XenBusDxe/XenBusDxe.inf
> INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> +INF OvmfPkg/XenConsoleIo/XenConsoleIo.inf
>
> !if $(SECURE_BOOT_ENABLE) == TRUE
> INF
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
>
Okay, so this patch forced me to review the current additions of serial
port devpaths to ConIn, ConOut, ErrOut in
OvmfPkg/Library/PlatformBootManagerLib/
I think I understand what you intend to do. I'd like to propose an
alternative I feel would be better.
You are introducing a new driver called "XenConsoleIo" which I assume is
a DXE_DRIVER module that produces gEfiSerialIoProtocolGuid. The reason I
can only "assume" is that you apparently forgot to include the driver in
the patch, with "git add". Nonetheless, without having seen the driver,
I claim that it should be unnecessary.
Namely, MdeModulePkg provides a generic platform serial driver, under
MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
This driver is already used in the ARM Xen guest platform:
ArmVirtPkg/ArmVirtXen.dsc
with a dependency on
OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf
which you also employ above.
Delegating the actual serial port access to the SerialPortLib class, the
SerialDxe driver produces one instance of EFI_SERIAL_IO_PROTOCOL. The
device path protocol instance that it installs on the handle is
constant, and known to other edk2 modules.
(
In turn, the TerminalDxe driver layers on top of the
EFI_SERIAL_IO_PROTOCOL instance, and produces simple text in/out
protocols on a child handle. The device path node appended specifically
for the child handle determines the terminal type (e.g., vt100, pc-ansi,
etc) to emulate. The terminal type can be selected by the platform by
adding a devpath to the Con* variables that ends with the desired device
path node, or else the TerminalDxe driver can fall back to a default
type. But, I digress.
)
What this all boils down to is:
(1) Rather than introducing a new driver (again, not having it seen),
please include "MdeModulePkg/Universal/SerialDxe/SerialDxe.inf" in the
XenOvmf DSC and FDF files.
You can resolve the SerialPortLib class to XenConsoleSerialPortLib
either for this one module only, or generally (same as in
"ArmVirtPkg/ArmVirtXen.dsc").
(I assume that in a Xen HVM guest, both the emulated serial port and the
paravirt xen console are available )
(2) In the code, we shouldn't locate the appropriate handle dynamically.
The full device path is known statically, in advance. Please refer to
"mSerialConsole" in
ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
Therefore I suggest that we add a "gXenPlatformConsole" array to
OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
with an initializer that effects the following:
- gXenPlatformConsole[1] is entirely zeroed,
- gXenPlatformConsole[0].ConnectType = CONSOLE_IN |
CONSOLE_OUT |
STD_ERROR;
- gXenPlatformConsole[0].DevicePath should be what's seen in
"mSerialConsole" above, but with the terminal type GUID pre-filled to
PC-ANSI ("gPcAnsiTerminal").
Then, we shouldn't touch the PlatformInitializeConsole() function;
instead modify the PlatformInitializeConsole() call in
PlatformBootManagerBeforeConsole() like this:
PlatformInitializeConsole (
XenDetected() ? gXenPlatformConsole : gPlatformConsole);
This will cause PlatformInitializeConsole() to do the right thing, and
it won't introduce special handling for gEfiSerialIoProtocolGuid that
would superfluously run on QEMU as well.
Furthermore, in a Xen HVM guest, I reckon this will multiplex terminal
IO to both emulated serial and Xen console.
Thanks
Laszlo
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |