[edk2] [PATCH] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.

Marvin Häuser posted 1 patch 5 years, 10 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c   | 46 +++++++++++++++++---
SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf |  4 ++
SourceLevelDebugPkg/SourceLevelDebugPkg.dec                                       | 12 +++++
3 files changed, 55 insertions(+), 7 deletions(-)
[edk2] [PATCH] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.
Posted by Marvin Häuser 5 years, 10 months ago
Currently, DebugCommunicationLibUsb uses the hardcoded endpoints 0x82
and 0x01 to communicate with the EHCI Debug Device. These, however,
are not standardized and may vary across different hardware.
To solve this problem, two PCDs have been introduced to configure the
in and out endpoints of the EHCI Debug Device. These may be set to 0
to retrieve the endpoints from the USB Device Descriptor directly.
To ensure maximum compatibility, the PCD defaults have been set to
the former hardcoded values.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
---
 SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c   | 46 +++++++++++++++++---
 SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf |  4 ++
 SourceLevelDebugPkg/SourceLevelDebugPkg.dec                                       | 12 +++++
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c
index d996f80f59e3..bb4be7b0710f 100644
--- a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c
+++ b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c
@@ -583,6 +583,8 @@ NeedReinitializeHardware(
   5. configure the usb debug device to debug mode.
 
   @param  Handle           Debug port handle.
+  @param  DebugInEndpoint  IN Endpoint of the usb debug device.
+  @param  DebugOutEndpoint OUT Endpoint of the usb debug device.
 
   @retval TRUE             The usb debug port hardware configuration is changed.
   @retval FALSE            The usb debug port hardware configuration is not changed.
@@ -591,7 +593,9 @@ NeedReinitializeHardware(
 RETURN_STATUS
 EFIAPI
 InitializeUsbDebugHardware (
-  IN USB_DEBUG_PORT_HANDLE *Handle
+  IN  USB_DEBUG_PORT_HANDLE *Handle,
+  OUT UINT8                 *DebugInEndpoint, OPTIONAL
+  OUT UINT8                 *DebugOutEndpoint OPTIONAL
 )
 {
   RETURN_STATUS             Status;
@@ -722,6 +726,14 @@ InitializeUsbDebugHardware (
       return RETURN_DEVICE_ERROR;
     }
 
+    if (DebugInEndpoint != NULL) {
+      *DebugInEndpoint = UsbDebugPortDescriptor.DebugInEndpoint;
+    }
+
+    if (DebugOutEndpoint != NULL) {
+      *DebugOutEndpoint = UsbDebugPortDescriptor.DebugOutEndpoint;
+    }
+
     //
     // enable the usb debug feature.
     //
@@ -790,7 +802,7 @@ DebugPortReadBuffer (
   }
 
   if (NeedReinitializeHardware(UsbDebugPortHandle)) {
-    Status = InitializeUsbDebugHardware (UsbDebugPortHandle);
+    Status = InitializeUsbDebugHardware (UsbDebugPortHandle, NULL, NULL);
     if (RETURN_ERROR(Status)) {
       return 0;
     }
@@ -845,6 +857,8 @@ DebugPortWriteBuffer (
   UINT8                     Sent;
   UINTN                     Total;
   UINT8                     ReceivedPid;
+  UINT8                     OutEndpoint;
+  UINT8                     StaticOutEndpoint;
 
   if (NumberOfBytes == 0 || Buffer == NULL) {
     return 0;
@@ -864,12 +878,20 @@ DebugPortWriteBuffer (
   }
 
   if (NeedReinitializeHardware(UsbDebugPortHandle)) {
-    Status = InitializeUsbDebugHardware (UsbDebugPortHandle);
+    Status = InitializeUsbDebugHardware (UsbDebugPortHandle, NULL, &OutEndpoint);
     if (RETURN_ERROR(Status)) {
       return 0;
     }
   }
 
+  StaticOutEndpoint = PcdGet8 (PcdUsbDebugPortOutEndpoint);
+  //
+  // If the endpoint has been configured statically, use it.
+  //
+  if (StaticOutEndpoint != 0) {
+    OutEndpoint = StaticOutEndpoint;
+  }
+
   UsbDebugPortRegister = (USB_DEBUG_PORT_REGISTER *)((UINTN)UsbDebugPortHandle->UsbDebugPortMemoryBase + UsbDebugPortHandle->DebugPortOffset);
 
   while ((Total < NumberOfBytes)) {
@@ -879,7 +901,7 @@ DebugPortWriteBuffer (
       Sent = (UINT8)(NumberOfBytes - Total);
     }
 
-    Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent, OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle);
+    Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent, OUTPUT_PID, 0x7F, OutEndpoint, UsbDebugPortHandle->BulkOutToggle);
 
     if (RETURN_ERROR(Status)) {
       return Total;
@@ -924,6 +946,8 @@ DebugPortPollBuffer (
   UINT8                     Length;
   UINT8                     Index;
   RETURN_STATUS             Status;
+  UINT8                     InEndpoint;
+  UINT8                     StaticInEndpoint;
 
   //
   // If Handle is NULL, it means memory is ready for use.
@@ -936,12 +960,20 @@ DebugPortPollBuffer (
   }
 
   if (NeedReinitializeHardware(UsbDebugPortHandle)) {
-    Status = InitializeUsbDebugHardware(UsbDebugPortHandle);
+    Status = InitializeUsbDebugHardware(UsbDebugPortHandle, &InEndpoint, NULL);
     if (RETURN_ERROR(Status)) {
       return FALSE;
     }
   }
 
+  StaticInEndpoint = PcdGet8 (PcdUsbDebugPortInEndpoint);
+  //
+  // If the endpoint has been configured statically, use it.
+  //
+  if (StaticInEndpoint != 0) {
+    InEndpoint = StaticInEndpoint;
+  }
+
   //
   // If the data buffer is not empty, then return TRUE directly.
   // else initialize a usb read transaction and read data to the data buffer.
@@ -959,7 +991,7 @@ DebugPortPollBuffer (
     UsbDebugPortRegister->SendPid  = DATA1_PID;
   }
   UsbDebugPortRegister->UsbAddress  = 0x7F;
-  UsbDebugPortRegister->UsbEndPoint = 0x82 & 0x0F;
+  UsbDebugPortRegister->UsbEndPoint = InEndpoint & 0x0F;
 
   //
   // Clearing W/R bit to indicate it's a READ operation
@@ -1078,7 +1110,7 @@ DebugPortInitialize (
 
   if (NeedReinitializeHardware(&Handle)) {
     DEBUG ((EFI_D_ERROR, "UsbDbg: Start EHCI debug port initialization!\n"));
-    Status = InitializeUsbDebugHardware (&Handle);
+    Status = InitializeUsbDebugHardware (&Handle, NULL, NULL);
     if (RETURN_ERROR(Status)) {
       DEBUG ((EFI_D_ERROR, "UsbDbg: Failed, please check if USB debug cable is plugged into EHCI debug port correctly!\n"));
       goto Exit;
diff --git a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf
index 028b04afbf00..8d3a7e477802 100644
--- a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf
+++ b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.inf
@@ -43,6 +43,10 @@ [Pcd]
   # The pci address of ehci host controller, in which usb debug feature is enabled.
   # The format of pci address please refer to SourceLevelDebugPkg.dec
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress              ## CONSUMES
+  # The endpoint that should be used for read transactions from the usb debug device.
+  gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortInEndpoint         ## CONSUMES
+  # The endpoint that should be used for write transactions to the usb debug device.
+  gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortOutEndpoint        ## CONSUMES
   # The value of data buffer size used for USB debug port handle.
   # It should be equal to sizeof (USB_DEBUG_PORT_HANDLE).
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugPortHandleBufferSize|23   ## SOMETIMES_CONSUMES
diff --git a/SourceLevelDebugPkg/SourceLevelDebugPkg.dec b/SourceLevelDebugPkg/SourceLevelDebugPkg.dec
index b89e9c6ad601..76410444f385 100644
--- a/SourceLevelDebugPkg/SourceLevelDebugPkg.dec
+++ b/SourceLevelDebugPkg/SourceLevelDebugPkg.dec
@@ -72,6 +72,18 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # @Expression  0x80000001 | (gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress & 0xF0000FFF) == 0
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress|0x000EF000|UINT32|0x00000003
 
+  ## The endpoint that should be used for read transactions from the usb debug device.
+  #  0:     Determine the endpoint dynamically.
+  #  other: The endpoint that should be used.
+  # @Prompt Configure the endpoint to read from the usb debug device.
+  gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortInEndpoint|0x82|UINT8|0x0000000b
+
+  ## The endpoint that should be used for write transactions to the usb debug device.
+  #  0:     Determine the endpoint dynamically.
+  #  other: The endpoint that should be used.
+  # @Prompt Configure the endpoint to write to the usb debug device.
+  gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortOutEndpoint|0x01|UINT8|0x0000000c
+
   ## The mask of exception numbers whose handlers would be ignored and cannot be replaced or 
   #  hooked by Debug Agent Library. Masking INT1/INT3 is invalid.
   # @Prompt Configure exception numbers not to be hooked by Debug Agent.
-- 
2.17.1.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint configuration.
Posted by Marvin Häuser 5 years, 10 months ago
Sorry, I made a mistake overlooking that the Initialization function is only called under certain circumstances.
I will submit a V2 soon.

Thanks,
Marvin

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Marvin
> Häuser
> Sent: Tuesday, June 12, 2018 2:34 PM
> To: edk2-devel@lists.01.org
> Cc: hao.a.wu@intel.com
> Subject: [edk2] [PATCH]
> SourceLevelDebugPkg/DebugCommunicationLibUsb: Add endpoint
> configuration.
> 
> Currently, DebugCommunicationLibUsb uses the hardcoded endpoints 0x82
> and 0x01 to communicate with the EHCI Debug Device. These, however, are
> not standardized and may vary across different hardware.
> To solve this problem, two PCDs have been introduced to configure the in
> and out endpoints of the EHCI Debug Device. These may be set to 0 to
> retrieve the endpoints from the USB Device Descriptor directly.
> To ensure maximum compatibility, the PCD defaults have been set to the
> former hardcoded values.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
> ---
> 
> SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommun
> icationLibUsb.c   | 46 +++++++++++++++++---
> 
> SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommun
> icationLibUsb.inf |  4 ++
>  SourceLevelDebugPkg/SourceLevelDebugPkg.dec                                       | 12
> +++++
>  3 files changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> unicationLibUsb.c
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> unicationLibUsb.c
> index d996f80f59e3..bb4be7b0710f 100644
> ---
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> unicationLibUsb.c
> +++
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> unic
> +++ ationLibUsb.c
> @@ -583,6 +583,8 @@ NeedReinitializeHardware(
>    5. configure the usb debug device to debug mode.
> 
>    @param  Handle           Debug port handle.
> +  @param  DebugInEndpoint  IN Endpoint of the usb debug device.
> +  @param  DebugOutEndpoint OUT Endpoint of the usb debug device.
> 
>    @retval TRUE             The usb debug port hardware configuration is changed.
>    @retval FALSE            The usb debug port hardware configuration is not
> changed.
> @@ -591,7 +593,9 @@ NeedReinitializeHardware(  RETURN_STATUS  EFIAPI
> InitializeUsbDebugHardware (
> -  IN USB_DEBUG_PORT_HANDLE *Handle
> +  IN  USB_DEBUG_PORT_HANDLE *Handle,
> +  OUT UINT8                 *DebugInEndpoint, OPTIONAL
> +  OUT UINT8                 *DebugOutEndpoint OPTIONAL
>  )
>  {
>    RETURN_STATUS             Status;
> @@ -722,6 +726,14 @@ InitializeUsbDebugHardware (
>        return RETURN_DEVICE_ERROR;
>      }
> 
> +    if (DebugInEndpoint != NULL) {
> +      *DebugInEndpoint = UsbDebugPortDescriptor.DebugInEndpoint;
> +    }
> +
> +    if (DebugOutEndpoint != NULL) {
> +      *DebugOutEndpoint = UsbDebugPortDescriptor.DebugOutEndpoint;
> +    }
> +
>      //
>      // enable the usb debug feature.
>      //
> @@ -790,7 +802,7 @@ DebugPortReadBuffer (
>    }
> 
>    if (NeedReinitializeHardware(UsbDebugPortHandle)) {
> -    Status = InitializeUsbDebugHardware (UsbDebugPortHandle);
> +    Status = InitializeUsbDebugHardware (UsbDebugPortHandle, NULL,
> + NULL);
>      if (RETURN_ERROR(Status)) {
>        return 0;
>      }
> @@ -845,6 +857,8 @@ DebugPortWriteBuffer (
>    UINT8                     Sent;
>    UINTN                     Total;
>    UINT8                     ReceivedPid;
> +  UINT8                     OutEndpoint;
> +  UINT8                     StaticOutEndpoint;
> 
>    if (NumberOfBytes == 0 || Buffer == NULL) {
>      return 0;
> @@ -864,12 +878,20 @@ DebugPortWriteBuffer (
>    }
> 
>    if (NeedReinitializeHardware(UsbDebugPortHandle)) {
> -    Status = InitializeUsbDebugHardware (UsbDebugPortHandle);
> +    Status = InitializeUsbDebugHardware (UsbDebugPortHandle, NULL,
> + &OutEndpoint);
>      if (RETURN_ERROR(Status)) {
>        return 0;
>      }
>    }
> 
> +  StaticOutEndpoint = PcdGet8 (PcdUsbDebugPortOutEndpoint);  //  // If
> + the endpoint has been configured statically, use it.
> +  //
> +  if (StaticOutEndpoint != 0) {
> +    OutEndpoint = StaticOutEndpoint;
> +  }
> +
>    UsbDebugPortRegister = (USB_DEBUG_PORT_REGISTER
> *)((UINTN)UsbDebugPortHandle->UsbDebugPortMemoryBase +
> UsbDebugPortHandle->DebugPortOffset);
> 
>    while ((Total < NumberOfBytes)) {
> @@ -879,7 +901,7 @@ DebugPortWriteBuffer (
>        Sent = (UINT8)(NumberOfBytes - Total);
>      }
> 
> -    Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total, Sent,
> OUTPUT_PID, 0x7F, 0x01, UsbDebugPortHandle->BulkOutToggle);
> +    Status = UsbDebugPortOut(UsbDebugPortRegister, Buffer + Total,
> + Sent, OUTPUT_PID, 0x7F, OutEndpoint,
> + UsbDebugPortHandle->BulkOutToggle);
> 
>      if (RETURN_ERROR(Status)) {
>        return Total;
> @@ -924,6 +946,8 @@ DebugPortPollBuffer (
>    UINT8                     Length;
>    UINT8                     Index;
>    RETURN_STATUS             Status;
> +  UINT8                     InEndpoint;
> +  UINT8                     StaticInEndpoint;
> 
>    //
>    // If Handle is NULL, it means memory is ready for use.
> @@ -936,12 +960,20 @@ DebugPortPollBuffer (
>    }
> 
>    if (NeedReinitializeHardware(UsbDebugPortHandle)) {
> -    Status = InitializeUsbDebugHardware(UsbDebugPortHandle);
> +    Status = InitializeUsbDebugHardware(UsbDebugPortHandle,
> + &InEndpoint, NULL);
>      if (RETURN_ERROR(Status)) {
>        return FALSE;
>      }
>    }
> 
> +  StaticInEndpoint = PcdGet8 (PcdUsbDebugPortInEndpoint);  //  // If
> + the endpoint has been configured statically, use it.
> +  //
> +  if (StaticInEndpoint != 0) {
> +    InEndpoint = StaticInEndpoint;
> +  }
> +
>    //
>    // If the data buffer is not empty, then return TRUE directly.
>    // else initialize a usb read transaction and read data to the data buffer.
> @@ -959,7 +991,7 @@ DebugPortPollBuffer (
>      UsbDebugPortRegister->SendPid  = DATA1_PID;
>    }
>    UsbDebugPortRegister->UsbAddress  = 0x7F;
> -  UsbDebugPortRegister->UsbEndPoint = 0x82 & 0x0F;
> +  UsbDebugPortRegister->UsbEndPoint = InEndpoint & 0x0F;
> 
>    //
>    // Clearing W/R bit to indicate it's a READ operation @@ -1078,7 +1110,7
> @@ DebugPortInitialize (
> 
>    if (NeedReinitializeHardware(&Handle)) {
>      DEBUG ((EFI_D_ERROR, "UsbDbg: Start EHCI debug port initialization!\n"));
> -    Status = InitializeUsbDebugHardware (&Handle);
> +    Status = InitializeUsbDebugHardware (&Handle, NULL, NULL);
>      if (RETURN_ERROR(Status)) {
>        DEBUG ((EFI_D_ERROR, "UsbDbg: Failed, please check if USB debug cable
> is plugged into EHCI debug port correctly!\n"));
>        goto Exit;
> diff --git
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> unicationLibUsb.inf
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> unicationLibUsb.inf
> index 028b04afbf00..8d3a7e477802 100644
> ---
> a/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> unicationLibUsb.inf
> +++
> b/SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugComm
> unic
> +++ ationLibUsb.inf
> @@ -43,6 +43,10 @@ [Pcd]
>    # The pci address of ehci host controller, in which usb debug feature is
> enabled.
>    # The format of pci address please refer to SourceLevelDebugPkg.dec
>    gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress              ##
> CONSUMES
> +  # The endpoint that should be used for read transactions from the usb
> debug device.
> +  gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortInEndpoint
> ## CONSUMES
> +  # The endpoint that should be used for write transactions to the usb
> debug device.
> +  gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortOutEndpoint
> ## CONSUMES
>    # The value of data buffer size used for USB debug port handle.
>    # It should be equal to sizeof (USB_DEBUG_PORT_HANDLE).
> 
> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugPortHandleBufferSize|
> 23   ## SOMETIMES_CONSUMES
> diff --git a/SourceLevelDebugPkg/SourceLevelDebugPkg.dec
> b/SourceLevelDebugPkg/SourceLevelDebugPkg.dec
> index b89e9c6ad601..76410444f385 100644
> --- a/SourceLevelDebugPkg/SourceLevelDebugPkg.dec
> +++ b/SourceLevelDebugPkg/SourceLevelDebugPkg.dec
> @@ -72,6 +72,18 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>    # @Expression  0x80000001 |
> (gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress &
> 0xF0000FFF) == 0
> 
> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbEhciPciAddress|0x000EF0
> 00|UINT32|0x00000003
> 
> +  ## The endpoint that should be used for read transactions from the usb
> debug device.
> +  #  0:     Determine the endpoint dynamically.
> +  #  other: The endpoint that should be used.
> +  # @Prompt Configure the endpoint to read from the usb debug device.
> +
> +
> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortInEndpoint|0x
> 82|U
> + INT8|0x0000000b
> +
> +  ## The endpoint that should be used for write transactions to the usb
> debug device.
> +  #  0:     Determine the endpoint dynamically.
> +  #  other: The endpoint that should be used.
> +  # @Prompt Configure the endpoint to write to the usb debug device.
> +
> +
> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdUsbDebugPortOutEndpoint|0
> x01|
> + UINT8|0x0000000c
> +
>    ## The mask of exception numbers whose handlers would be ignored and
> cannot be replaced or
>    #  hooked by Debug Agent Library. Masking INT1/INT3 is invalid.
>    # @Prompt Configure exception numbers not to be hooked by Debug
> Agent.
> --
> 2.17.1.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel