From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 09B2381ADB for ; Mon, 9 Jan 2017 10:07:24 -0800 (PST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP; 09 Jan 2017 10:07:23 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,340,1477983600"; d="scan'208";a="1110133421" Received: from orsmsx108.amr.corp.intel.com ([10.22.240.6]) by fmsmga002.fm.intel.com with ESMTP; 09 Jan 2017 10:07:23 -0800 Received: from orsmsx160.amr.corp.intel.com (10.22.226.43) by ORSMSX108.amr.corp.intel.com (10.22.240.6) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 9 Jan 2017 10:07:23 -0800 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.184]) by ORSMSX160.amr.corp.intel.com ([169.254.13.139]) with mapi id 14.03.0248.002; Mon, 9 Jan 2017 10:07:23 -0800 From: "Kinney, Michael D" To: Laszlo Ersek , "edk2-devel@ml01.01.org" , "Kinney, Michael D" CC: "Justen, Jordan L" , "Fan, Jeff" Thread-Topic: [edk2] [Patch] OvmgPkg/PlatformBootManagerLib: Add Debug Agent console Thread-Index: AQHSajmzNpUHOltFCUeeND67O7yQ4qEwnaMA///TRwA= Date: Mon, 9 Jan 2017 18:07:22 +0000 Message-ID: References: <1483939865-24012-1-git-send-email-michael.d.kinney@intel.com> <51fada6f-2adb-5a7d-2577-7271b14eaa8e@redhat.com> In-Reply-To: <51fada6f-2adb-5a7d-2577-7271b14eaa8e@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjU5OTIwNDMtZDY0ZC00MTAwLTlkMmEtZmYwYjhjOThlZjEzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ik1hYk5OSkdYRGpVWnErY3UyWTdIMVVneDlMeVBoT0xHUUZQa2w3cktOOXM9In0= x-originating-ip: [10.22.254.139] MIME-Version: 1.0 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 18:07:24 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo, Thanks for the feedback. I have send v2 and v3. Please ignore v2. I missed the STD_ERROR flag in v2 and fixed in v3. On a related topic, if you look at: QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformData.c You will see in the function InitializePlatformBootManagerLib() Updates the device path global variables based on UART and terminal PCD settings. =20 Do you think this applies to OVMF? Thanks, Mike > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Monday, January 9, 2017 4:44 AM > To: Kinney, Michael D ; edk2-devel@ml01.01.or= g > Cc: Justen, Jordan L ; Fan, Jeff > Subject: Re: [edk2] [Patch] OvmgPkg/PlatformBootManagerLib: Add Debug Age= nt > console >=20 > 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 th= e BSD > License > > which accompanies this distribution. The full text of the license m= ay 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 =3D { > > + { > > + { > > + 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 =3D (EFI_DEVICE_PATH_PROTOCOL *)&gDebugAgentUartDevicePat= h; > > + > > + // > > + // Print Device Path > > + // > > + DevPathStr =3D ConvertDevicePathToText (DevicePath, FALSE, FALSE); > > + if (DevPathStr !=3D 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/PlatformBootManager= Lib.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.<= BR> > > +# Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<= BR> > > # This program and the accompanying materials > > # are licensed and made available under the terms and conditions of t= he 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] > > >=20 > I'm going to suggest / request almost the same thing here as I did > (independently) last week: >=20 > https://lists.01.org/pipermail/edk2-devel/2017-January/006190.html >=20 > In other words, given that the complete device path for the DebugAgent > console is constant and known in advance, >=20 > (1) please move the definitions of VENDOR_UART_DEVICE_PATH and > "gDebugAgentUartDevicePath" to the file >=20 > OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c >=20 > and >=20 > (2) rather than inserting additional logic to the > PrepareLpcBridgeDevicePath() callback function, please add the address > of "gDebugAgentUartDevicePath" to "gPlatformConsole" in > "PlatformData.c", with a >=20 > CONSOLE_IN | CONSOLE_OUT | STD_ERROR >=20 > bitmask. This will cause the PlatformInitializeConsole() function to > update the console variables appropriately, when necessary. >=20 > (The debug output won't contain the textual rendering of the device > path, but that's fine (it's a constant devpath).) >=20 > (3) In the "gDebugAgentUartDevicePath" initializer, feel free to reuse > the "gPcAnsiTerminal" macro for shortening the "TerminalType" field's > initializer. >=20 > Thanks! > Laszlo