1. Patchew
  2. EDK2
  3. View series

[edk2] [PATCH v2] SourceLevelDebugPkg: Use Pcd for the revision of transfer protocol

Hao Wu posted 1 patch 6 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
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(-)
[edk2] [PATCH v2] SourceLevelDebugPkg: Use Pcd for the revision of transfer protocol
Posted by Hao Wu 6 years, 7 months ago 
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.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@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/DebugAgent.c b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.c
index f156fe24db..36b1ef924c 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.c
+++ b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.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.PcdExceptionsIgnoredByDebugger  ## 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.PcdExceptionsIgnoredByDebugger  ## 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.PcdExceptionsIgnoredByDebugger|0x00004000  ## SOMETIMES_CONSUMES
+  gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdTransferProtocolRevision                ## SOMETIMES_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.
+  # @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_PcdUsbXhciDebugDetectTimeout_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_PcdTransferProtocolRevision_PROMPT  #language en-US "Default revision of the debug agent transfer protocol."
+
+#string STR_gEfiSourceLevelDebugPkgTokenSpaceGuid_PcdTransferProtocolRevision_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
Re: [edk2] [PATCH v2] SourceLevelDebugPkg: Use Pcd for the revision of transfer protocol
Posted by Ni, Ruiyu 6 years, 7 months ago 
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.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@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.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@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
Re: [edk2] [PATCH v2] SourceLevelDebugPkg: Use Pcd for the revision of transfer protocol
Posted by Wu, Hao A 6 years, 7 months ago 
> -----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
> 
> 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: edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@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.ni@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu <hao.a.wu@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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.