Hao, I have 2 comments regarding to the INF and DEC change. Check below. Thanks/Ray
> -----Original Message----- > From: Wu, Hao A > Sent: Friday, September 1, 2017 10:13 AM > To: edk2-devel@lists.01.org > Cc: Wu, Hao A <hao.a...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com> > 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 <ruiyu...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Hao Wu <hao.a...@intel.com> > --- > 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 only > intended to be used by Debug related module implementation. > > - 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 > which accompanies this distribution. The full text of the license may 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 >= DEBUG_AGENT_REVISION_04) { > + if (PcdGet32(PcdTransferProtocolRevision) >= > 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 running > transfer protocol before DEBUG_AGENT_REVISION. > + // If the incompatible Debug Packet received, the HOST should be running > 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 = DEBUG_AGENT_REVISION; > + DebugAgentRevision.Revision = PcdGet32(PcdTransferProtocolRevision); > DebugAgentRevision.Capabilities = DEBUG_AGENT_CAPABILITIES; > Status = 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? > > 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 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. > # 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. 2. Please describe the PcdTransferProtocolRevision layout. Upper 2-byte for major revision, lower-2-byte for minor revision. 0x0004 stands for 0.4. > + # @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 microsecond." > > +#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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel