From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (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 A1AD521E74910 for ; Thu, 31 Aug 2017 21:20:44 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Aug 2017 21:23:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,456,1498546800"; d="scan'208";a="1168132340" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga001.jf.intel.com with ESMTP; 31 Aug 2017 21:23:28 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 31 Aug 2017 21:23:26 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.117]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.168]) with mapi id 14.03.0319.002; Fri, 1 Sep 2017 12:23:24 +0800 From: "Wu, Hao A" To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" Thread-Topic: [PATCH v2] SourceLevelDebugPkg: Use Pcd for the revision of transfer protocol Thread-Index: AQHTIsfLxxifzU6JbkeQt9yLg+gVCqKe5jCAgACIKKA= Date: Fri, 1 Sep 2017 04:23:23 +0000 Message-ID: References: <20170901021230.21436-1-hao.a.wu@intel.com> <734D49CCEBEEF84792F5B80ED585239D5BA1E7E2@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5BA1E7E2@SHSMSX104.ccr.corp.intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2] SourceLevelDebugPkg: Use Pcd for the revision of transfer protocol X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Sep 2017 04:20:44 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Ni, Ruiyu > Sent: Friday, September 01, 2017 12:15 PM > To: Wu, Hao A; edk2-devel@lists.01.org > Subject: RE: [PATCH v2] SourceLevelDebugPkg: Use Pcd for the revision of > transfer protocol >=20 > Hao, > I have 2 comments regarding to the INF and DEC change. Check below. Thanks for the comments. I will refine them and send out a new version. Best Regards, Hao Wu >=20 > Thanks/Ray >=20 > > -----Original Message----- > > From: Wu, Hao A > > Sent: Friday, September 1, 2017 10:13 AM > > To: edk2-devel@lists.01.org > > Cc: Wu, Hao A ; Ni, Ruiyu > > Subject: [PATCH v2] SourceLevelDebugPkg: Use Pcd for the revision of > > transfer protocol > > > > V2 changes: > > Instead of using a global variable, use a Pcd for transfer protocol > > revision. > > > > Previously, the revison of the debug agent transfer protocol is > > reflected by a macro. > > > > This commit introduces a Pcd to reflect the revision in order to avoid = the > > comparision of two macros, which will generate a constant result > > detected by code checkers. > > > > Cc: Ruiyu Ni > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Hao Wu > > --- > > SourceLevelDebugPkg/Include/TransferProtocol.h | 3= +-- > > .../Library/DebugAgent/DebugAgentCommon/DebugAgent.c | 6 > > +++--- > > SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf | 1= + > > SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf | 1 > > + > > SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf | 1 > > + > > SourceLevelDebugPkg/SourceLevelDebugPkg.dec | 7= ++++++- > > SourceLevelDebugPkg/SourceLevelDebugPkg.uni | 6= +++++- > > 7 files changed, 18 insertions(+), 7 deletions(-) > > > > diff --git a/SourceLevelDebugPkg/Include/TransferProtocol.h > > b/SourceLevelDebugPkg/Include/TransferProtocol.h > > index ef7c891c39..5f9f35b5d7 100644 > > --- a/SourceLevelDebugPkg/Include/TransferProtocol.h > > +++ b/SourceLevelDebugPkg/Include/TransferProtocol.h > > @@ -2,7 +2,7 @@ > > Transfer protocol defintions used by debug agent and host. It is onl= y > > intended to be used by Debug related module implementation. > > > > - Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved. > > + Copyright (c) 2010 - 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 > > @@ -24,7 +24,6 @@ > > // > > #define DEBUG_AGENT_REVISION_03 ((0 << 16) | 03) > > #define DEBUG_AGENT_REVISION_04 ((0 << 16) | 04) > > -#define DEBUG_AGENT_REVISION DEBUG_AGENT_REVISION_04 > > #define DEBUG_AGENT_CAPABILITIES 0 > > > > // > > diff --git > > a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/Debug > > Agent.c > > b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/Debug > > Agent.c > > index f156fe24db..36b1ef924c 100644 > > --- > > a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/Debug > > Agent.c > > +++ > > b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/Debug > > Agent.c > > @@ -1564,7 +1564,7 @@ ReadMemoryAndSendResponsePacket ( > > // Compression/decompression support was added since revision 0.4. > > // Revision 0.3 shouldn't compress the packet. > > // > > - if (DEBUG_AGENT_REVISION >=3D DEBUG_AGENT_REVISION_04) { > > + if (PcdGet32(PcdTransferProtocolRevision) >=3D > > DEBUG_AGENT_REVISION_04) { > > // > > // Get the compressed data size without modifying the packet. > > // > > @@ -1711,7 +1711,7 @@ AttachHost ( > > } > > if (IncompatibilityFlag) { > > // > > - // If the incompatible Debug Packet received, the HOST should be r= unning > > transfer protocol before DEBUG_AGENT_REVISION. > > + // If the incompatible Debug Packet received, the HOST should be r= unning > > transfer protocol before PcdTransferProtocolRevision. > > // It could be UDK Debugger for Windows v1.1/v1.2 or for Linux v0.= 8/v1.2. > > // > > DebugPortWriteBuffer (Handle, (UINT8 *) mErrorMsgVersionAlert, > > AsciiStrLen (mErrorMsgVersionAlert)); > > @@ -2192,7 +2192,7 @@ CommandCommunication ( > > break; > > > > case DEBUG_COMMAND_GET_REVISION: > > - DebugAgentRevision.Revision =3D DEBUG_AGENT_REVISION; > > + DebugAgentRevision.Revision =3D PcdGet32(PcdTransferProtocolRevi= sion); > > DebugAgentRevision.Capabilities =3D DEBUG_AGENT_CAPABILITIES; > > Status =3D SendDataResponsePacket ((UINT8 *) &DebugAgentRevision= , > > (UINT16) sizeof (DEBUG_DATA_RESPONSE_GET_REVISION), DebugHeader); > > break; > > diff --git > > a/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf > > b/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf > > index ce36345bab..17b1ac5a89 100644 > > --- a/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf > > +++ b/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf > > @@ -101,4 +101,5 @@ > > gEfiMdePkgTokenSpaceGuid.PcdFSBClock = ## > > SOMETIMES_CONSUMES > > > > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdExceptionsIgnoredByDebugg > > er ## SOMETIMES_CONSUMES > > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugPortHandleBufferSize > > ## CONSUMES > > + gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdTransferProtocolRevision > > ## SOMETIMES_CONSUMES > > > > diff --git > > a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf > > b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf > > index 12c2a71b78..2f2bc6c162 100644 > > --- a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf > > +++ b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf > > @@ -91,4 +91,5 @@ > > gEfiMdePkgTokenSpaceGuid.PcdFSBClock = ## > > SOMETIMES_CONSUMES > > > > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdExceptionsIgnoredByDebugg > > er ## SOMETIMES_CONSUMES > > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugPortHandleBufferSize > > ## SOMETIMES_CONSUMES > > + gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdTransferProtocolRevision > > ## SOMETIMES_CONSUMES > > > > diff --git > > a/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf > > b/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf > > index 1fa5745b1c..df7ad75d68 100644 > > --- a/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf > > +++ b/SourceLevelDebugPkg/Library/DebugAgent/SmmDebugAgentLib.inf > > @@ -86,4 +86,5 @@ > > gEfiMdePkgTokenSpaceGuid.PcdFSBClock = ## > > SOMETIMES_CONSUMES > > # Skip Page Fault exception (14) by default in SMM > > > > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdExceptionsIgnoredByDebugg > > er|0x00004000 ## SOMETIMES_CONSUMES > > + gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdTransferProtocolRevision > > ## SOMETIMES_CONSUMES > 1. Why SOMETIMES_CONSUMES instead of CONSUMES? >=20 > > > > diff --git a/SourceLevelDebugPkg/SourceLevelDebugPkg.dec > > b/SourceLevelDebugPkg/SourceLevelDebugPkg.dec > > index 9579c3e006..18f9410539 100644 > > --- a/SourceLevelDebugPkg/SourceLevelDebugPkg.dec > > +++ b/SourceLevelDebugPkg/SourceLevelDebugPkg.dec > > @@ -6,7 +6,7 @@ > > # and host, PeCoffExtraActionLib instance to report symbol path inform= ation, > > # etc. > > # > > -# Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved. > > +# Copyright (c) 2010 - 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 that accompanies this > > distribution. > > # The full text of the license may be found at > > @@ -112,5 +112,10 @@ > > # @Prompt Configure debug device detection timeout value. > > > > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbXhciDebugDetectTimeout > > |3000000|UINT64|0x00000009 > > > > + ## Default revision of the debug agent transfer protocol. > > + # Debug packet compression and decompression is supported since > > revision 0.4. >=20 > 2. Please describe the PcdTransferProtocolRevision layout. Upper 2-byte f= or > major revision, lower-2-byte for minor revision. 0x0004 stands for 0.4. >=20 > > + # @Prompt Default revision of the debug agent transfer protocol. > > + > > gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdTransferProtocolRevision|0x4 > > |UINT32|0x0000000a > > + > > [UserExtensions.TianoCore."ExtraFiles"] > > SourceLevelDebugPkgExtra.uni > > diff --git a/SourceLevelDebugPkg/SourceLevelDebugPkg.uni > > b/SourceLevelDebugPkg/SourceLevelDebugPkg.uni > > index 533dafbfc8..d90a112e2c 100644 > > --- a/SourceLevelDebugPkg/SourceLevelDebugPkg.uni > > +++ b/SourceLevelDebugPkg/SourceLevelDebugPkg.uni > > @@ -8,7 +8,7 @@ > > // and host, PeCoffExtraActionLib instance to report symbol path > > information, > > // etc. > > // > > -// Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<= BR> > > +// Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<= BR> > > // > > // This program and the accompanying materials are licensed and made > > available under > > // the terms and conditions of the BSD License that accompanies this > > distribution. > > @@ -91,3 +91,7 @@ > > #string > > STR_gEfiSourceLevelDebugPkgTokenSpaceGuid_PcdUsbXhciDebugDetectTi > > meout_HELP #language en-US "Per XHCI spec, software shall impose a > > timeout between the detection of the Debug Host\n" > > = "connection > and the DbC > > Run transition to 1. This PCD specifies the timeout value in microsecon= d." > > > > +#string > > STR_gEfiSourceLevelDebugPkgTokenSpaceGuid_PcdTransferProtocolRevisio > > n_PROMPT #language en-US "Default revision of the debug agent transfer > > protocol." > > + > > +#string > > STR_gEfiSourceLevelDebugPkgTokenSpaceGuid_PcdTransferProtocolRevisio > > n_HELP #language en-US "Debug packet compression and decompression is > > supported since revision 0.4." > > + > > -- > > 2.12.0.windows.1