Re: [Patch V3 2/2] MdeModulePkg/UsbMassStorageDxe: Check Get Max LUN status/value

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Star,

I agree.  I will update patch once more to make sure
Return status is meaningful.  I will also remove ASSERT
For Context NULL and return EFI_INVALID_PARAMETER
Instead if either Context or MaxLun are NULL.

Mike

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, November 16, 2017 3:03 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: RE: [Patch V3 2/2]
> MdeModulePkg/UsbMassStorageDxe: Check Get Max LUN
> status/value
> 
> Mike,
> 
> After this patch, the return status of UsbBotGetMaxLun
> seems confusing and useless. Fortunately, there is no
> one using the return status of it.
> What do you think?
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Thursday, November 16, 2017 11:58 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric
> <eric.dong@intel.com>
> Subject: [Patch V3 2/2] MdeModulePkg/UsbMassStorageDxe:
> Check Get Max LUN status/value
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=767
> 
> If a USB Mass Storage device does not support the Get
> Max LUN command, then the USB I/O Protocol
> ControlTransfer() service may return an error.  If an
> error is returned for this command, then assume that
> the device does not support multiple LUNs and return a
> maximum LUN value of 0.
> 
> The USB Mass Storage Class Specification states that a
> maximum LUN value larger than 0x0F is invalid.  Add a
> check to make sure this maximum LUN value is in this
> valid range, and if it is not, then assume that the
> device does not support multiple LUNs and return a
> maximum LUN value of 0.
> 
> This change improves compatibility with USB FLASH
> drives that do not support the Get Max LUN command or
> return an invalid maximum LUN value.
> 
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> ---
>  MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c |
> 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
> index 4bb7222b89..6afe2ae235 100644
> ---
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
> +++
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c
> @@ -2,7 +2,7 @@
>    Implementation of the USB mass storage Bulk-Only
> Transport protocol,
>    according to USB Mass Storage Class Bulk-Only
> Transport, Revision 1.0.
> 
> -Copyright (c) 2007 - 2011, Intel Corporation. All
> rights reserved.<BR>
> +Copyright (c) 2007 - 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 @@ -576,6 +576,18 @@ UsbBotGetMaxLun (
>                              1,
>                              &Result
>                              );
> +  if (EFI_ERROR (Status) || *MaxLun > USB_BOT_MAX_LUN)
> {
> +    //
> +    // If the Get LUN request returns an error or the
> MaxLun is larger than
> +    // the maximum LUN value (0x0f) supported by the
> USB Mass Storage Class
> +    // Bulk-Only Transport Spec, then set MaxLun to 0.
> +    //
> +    // This improves compatibility with USB FLASH
> drives that have a single LUN
> +    // and either do not return a max LUN value or
> return an invalid maximum LUN
> +    // value.
> +    //
> +    *MaxLun = 0;
> +  }
> 
>    return Status;
>  }
> --
> 2.14.2.windows.3