> -----Original Message----- > From: Ni, Ruiyu > Sent: Friday, September 01, 2017 12:15 PM > To: Wu, Hao A; [email protected] > Subject: RE: [PATCH v2] SourceLevelDebugPkg: Use Pcd for the revision of > transfer protocol > > 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 > > Thanks/Ray > > > -----Original Message----- > > From: Wu, Hao A > > Sent: Friday, September 1, 2017 10:13 AM > > To: [email protected] > > Cc: Wu, Hao A <[email protected]>; Ni, Ruiyu <[email protected]> > > 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 <[email protected]> > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Hao Wu <[email protected]> > > --- > > 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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

