From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 483BB817A9 for ; Mon, 9 Jan 2017 04:43:37 -0800 (PST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B0A8F83F43; Mon, 9 Jan 2017 12:43:37 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-74.phx2.redhat.com [10.3.116.74]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v09ChZRc024482; Mon, 9 Jan 2017 07:43:36 -0500 To: Michael Kinney , edk2-devel@ml01.01.org References: <1483939865-24012-1-git-send-email-michael.d.kinney@intel.com> Cc: Jordan Justen , Jeff Fan From: Laszlo Ersek Message-ID: <51fada6f-2adb-5a7d-2577-7271b14eaa8e@redhat.com> Date: Mon, 9 Jan 2017 13:43:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <1483939865-24012-1-git-send-email-michael.d.kinney@intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 09 Jan 2017 12:43:37 +0000 (UTC) Subject: Re: [Patch] OvmgPkg/PlatformBootManagerLib: Add Debug Agent console X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Jan 2017 12:43:37 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 01/09/17 06:31, Michael Kinney wrote: > The Debug Agent in the SourceLevelDebugPkg can multiplex > both source level debug messages and console messages on > the same UART. WHen this is done, the Debug Agent owns > the UART device and an additional device handle with a > Serial I/O Protocol is produced with a VenHw device path > node. > > In order for a platform to provide a UART based console > when the Debug Agent is using the same UART device, the > PlatformBootManagerLib must consider the SerialI/O > Protocol produces by the Debug Agent as one of the > supported consoles. > > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Jeff Fan > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Michael Kinney > --- > .../Library/PlatformBootManagerLib/BdsPlatform.c | 82 +++++++++++++++++++++- > .../PlatformBootManagerLib.inf | 3 +- > 2 files changed, 83 insertions(+), 2 deletions(-) > > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > index cc35630..0d94840 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > @@ -1,7 +1,7 @@ > /** @file > Platform BDS customizations. > > - Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > which accompanies this distribution. The full text of the license may be found at > @@ -16,7 +16,17 @@ > #include > #include > #include > +#include > > +// > +// Debug Agent UART Device Path structure > +// > +typedef struct { > + VENDOR_DEVICE_PATH VendorHardware; > + UART_DEVICE_PATH Uart; > + VENDOR_DEVICE_PATH TerminalType; > + EFI_DEVICE_PATH_PROTOCOL End; > +} VENDOR_UART_DEVICE_PATH; > > // > // Global data > @@ -28,6 +38,53 @@ VOID *mEmuVariableEventReg; > EFI_EVENT mEmuVariableEvent; > BOOLEAN mDetectVgaOnly; > UINT16 mHostBridgeDevId; > +VENDOR_UART_DEVICE_PATH gDebugAgentUartDevicePath = { > + { > + { > + HARDWARE_DEVICE_PATH, > + HW_VENDOR_DP, > + { > + (UINT8) (sizeof (VENDOR_DEVICE_PATH)), > + (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8) > + } > + }, > + EFI_DEBUG_AGENT_GUID, > + }, > + { > + { > + MESSAGING_DEVICE_PATH, > + MSG_UART_DP, > + { > + (UINT8) (sizeof (UART_DEVICE_PATH)), > + (UINT8) ((sizeof (UART_DEVICE_PATH)) >> 8) > + } > + }, > + 0, // Reserved > + 0, // BaudRate - Default > + 0, // DataBits - Default > + 0, // Parity - Default > + 0, // StopBits - Default > + }, > + { > + { > + MESSAGING_DEVICE_PATH, > + MSG_VENDOR_DP, > + { > + (UINT8)(sizeof (VENDOR_DEVICE_PATH)), > + (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8) > + } > + }, > + DEVICE_PATH_MESSAGING_PC_ANSI > + }, > + { > + END_DEVICE_PATH_TYPE, > + END_ENTIRE_DEVICE_PATH_SUBTYPE, > + { > + END_DEVICE_PATH_LENGTH, > + 0 > + } > + } > +}; > > // > // Table of host IRQs matching PCI IRQs A-D > @@ -532,6 +589,29 @@ Returns: > EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL); > EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL); > > + // > + // Register Debug Agent > + // > + DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)&gDebugAgentUartDevicePath; > + > + // > + // Print Device Path > + // > + DevPathStr = ConvertDevicePathToText (DevicePath, FALSE, FALSE); > + if (DevPathStr != NULL) { > + DEBUG(( > + EFI_D_INFO, > + "BdsPlatform.c+%d: DevPath: %s\n", > + __LINE__, > + DevPathStr > + )); > + FreePool(DevPathStr); > + } > + > + EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL); > + EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL); > + EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL); > + > return EFI_SUCCESS; > } > > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > index 4a6bece..f9e35c9 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Platform BDS customizations library. > # > -# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
> # This program and the accompanying materials > # are licensed and made available under the terms and conditions of the BSD License > # which accompanies this distribution. The full text of the license may be found at > @@ -36,6 +36,7 @@ > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec > + SourceLevelDebugPkg/SourceLevelDebugPkg.dec > OvmfPkg/OvmfPkg.dec > > [LibraryClasses] > I'm going to suggest / request almost the same thing here as I did (independently) last week: https://lists.01.org/pipermail/edk2-devel/2017-January/006190.html In other words, given that the complete device path for the DebugAgent console is constant and known in advance, (1) please move the definitions of VENDOR_UART_DEVICE_PATH and "gDebugAgentUartDevicePath" to the file OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c and (2) rather than inserting additional logic to the PrepareLpcBridgeDevicePath() callback function, please add the address of "gDebugAgentUartDevicePath" to "gPlatformConsole" in "PlatformData.c", with a CONSOLE_IN | CONSOLE_OUT | STD_ERROR bitmask. This will cause the PlatformInitializeConsole() function to update the console variables appropriately, when necessary. (The debug output won't contain the textual rendering of the device path, but that's fine (it's a constant devpath).) (3) In the "gDebugAgentUartDevicePath" initializer, feel free to reuse the "gPcAnsiTerminal" macro for shortening the "TerminalType" field's initializer. Thanks! Laszlo