Re: [edk2-platform][PATCH v1 1/1] Platform/RaspberryPi : Fix Usb2Hc SCT issues

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 12/24/20 8:39 PM, Samer El-Haj-Mahmoud wrote:
> REF: https://github.com/pftf/RPi4/issues/88
> 
> Fix SCT failures in the RPi USB2_HC_PROTOCOL by adding input parameter
> validation in Reset(), SetState(), GetState(), and
> SyncInterruptTransfer()
> 
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Andrei Warkentin <awarkentin@vmware.com>
> 
> Signed-off-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Pushed as ae6c236e7610..f8958b86e886


> ---
>  Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.c | 44 ++++++++++++++++++--
>  1 file changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.c b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.c
> index 4f6f60b1eb5f..0cd833c742cf 100644
> --- a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.c
> +++ b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.c
> @@ -1,5 +1,6 @@
>  /** @file
>   *
> +*   Copyright (c) 2020, ARM Limited. All rights reserved.
>   *  Copyright (c) 2017-2018, Andrey Warkentin <andrey.warkentin@gmail.com>
>   *  Copyright (c) 2015-2016, Linaro Limited. All rights reserved.
>   *
> @@ -498,6 +499,21 @@ DwHcReset (
>    DWUSB_OTGHC_DEV *DwHc;
>    DwHc = DWHC_FROM_THIS (This);
>  
> +  switch (Attributes) {
> +  case EFI_USB_HC_RESET_GLOBAL:
> +  case EFI_USB_HC_RESET_HOST_CONTROLLER:
> +    break;
> +
> +  case EFI_USB_HC_RESET_GLOBAL_WITH_DEBUG:
> +  case EFI_USB_HC_RESET_HOST_WITH_DEBUG:
> +    Status = EFI_UNSUPPORTED;
> +    goto Exit;
> +
> +  default:
> +    Status = EFI_INVALID_PARAMETER;
> +    goto Exit;
> +  }
> +
>    Status = gBS->CreateEvent (EVT_TIMER, 0, NULL, NULL, &TimeoutEvt);
>    ASSERT_EFI_ERROR (Status);
>    if (EFI_ERROR (Status)) {
> @@ -553,6 +569,10 @@ DwHcGetState (
>  
>    DwHc = DWHC_FROM_THIS (This);
>  
> +  if (State == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    *State = DwHc->DwHcState;
>  
>    return EFI_SUCCESS;
> @@ -565,13 +585,25 @@ DwHcSetState (
>    IN  EFI_USB_HC_STATE     State
>    )
>  {
> -  DWUSB_OTGHC_DEV *DwHc;
> +  DWUSB_OTGHC_DEV   *DwHc;
> +  EFI_STATUS        Status;
>  
>    DwHc = DWHC_FROM_THIS (This);
>  
> -  DwHc->DwHcState = State;
> +  switch (State) {
> +  case EfiUsbHcStateHalt:
> +  case EfiUsbHcStateOperational:
> +  case EfiUsbHcStateSuspend:
> +    DwHc->DwHcState = State;
> +    Status = EFI_SUCCESS;
> +    break;
>  
> -  return EFI_SUCCESS;
> +  default:
> +    Status = EFI_INVALID_PARAMETER;
> +    break;
> +  }
> +
> +  return Status;
>  }
>  
>  EFI_STATUS
> @@ -1173,6 +1205,12 @@ DwHcSyncInterruptTransfer (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> +  if (((DeviceSpeed == EFI_USB_SPEED_LOW) && (MaximumPacketLength != 8))  ||
> +      ((DeviceSpeed == EFI_USB_SPEED_FULL) && (MaximumPacketLength > 64)) ||
> +      ((DeviceSpeed == EFI_USB_SPEED_HIGH) && (MaximumPacketLength > 3072))) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    Status = gBS->CreateEvent (EVT_TIMER, 0, NULL, NULL, &TimeoutEvt);
>    if (EFI_ERROR (Status)) {
>      return Status;
>