From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.93; helo=mga11.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6DFB8203555E2 for ; Mon, 13 Nov 2017 17:09:33 -0800 (PST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Nov 2017 17:13:39 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,392,1505804400"; d="scan'208";a="1926773" Received: from orsmsx102.amr.corp.intel.com ([10.22.225.129]) by orsmga003.jf.intel.com with ESMTP; 13 Nov 2017 17:13:39 -0800 Received: from orsmsx114.amr.corp.intel.com (10.22.240.10) by ORSMSX102.amr.corp.intel.com (10.22.225.129) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 13 Nov 2017 17:13:39 -0800 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.80]) by ORSMSX114.amr.corp.intel.com ([169.254.8.48]) with mapi id 14.03.0319.002; Mon, 13 Nov 2017 17:13:38 -0800 From: "Kinney, Michael D" To: "Zeng, Star" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Dong, Eric" Thread-Topic: [Patch] MdeModulePkg/UsbMassStorageDxe: Verify Get Max LUN value Thread-Index: AQHTXGQP1JqLDAs1rUGacB2uJcE1S6MSmpgA///v9oCAAQchAP//fqhQ Date: Tue, 14 Nov 2017 01:13:38 +0000 Message-ID: References: <20171108221906.6544-1-michael.d.kinney@intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B9B495E@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B9B49A0@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B9B4C88@shsmsx102.ccr.corp.intel.com> In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B9B4C88@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZGJiMDkxNzQtMWNmOS00NjU1LWI2N2MtOTU4NzIyODRiMDViIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJVMDNva3BxRUlyZFl4YktKRlJWVTlFa1BYOUVkY0VlQ1lPeW15SnZtV0ZONVhjNjhXdEtKc25jXC9iVXFNZXJLWiJ9 dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Subject: Re: [Patch] MdeModulePkg/UsbMassStorageDxe: Verify Get Max LUN value X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Nov 2017 01:09:33 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Star, I may drop this patch. The range check is valid, but=20 may want to change to an error condition instead of=20 forcing to 0 if out of range. I just found that this device is returning different values for the max LUN. I saw examples today of a single LUN device returning 4. This appears to be due to=20 the request for max LUN returning a buffer with 0 DataLength, and the 1 byte data buffer is being returned with random data. I believe the root cause is a USB Control Transfer with Direction of EfiUsbDataIn and the length of=20 data written is less than the amount of data requested. In this case 1 byte is requested, and 0 bytes are written. The USB I/O Protocol ControlTransfer() API has a DataLength parameter as an IN, but there is no method to know how=20 much data was written. The USB HC Protocol ControlTransfer() API has DataLength as an IN/OUT, so the amount of data=20 written is known at the HC level. I am thinking the USB HC ControlTransfer() API needs to return an error if the amount of data written is less than requested. I will work on a new patch with this approach. Best regards, Mike > -----Original Message----- > From: Zeng, Star > Sent: Monday, November 13, 2017 4:48 PM > To: Kinney, Michael D ; edk2- > devel@lists.01.org > Cc: Dong, Eric ; Zeng, Star > > Subject: RE: [Patch] MdeModulePkg/UsbMassStorageDxe: > Verify Get Max LUN value >=20 > That is fine. >=20 > Reviewed-by: Star Zeng >=20 > Thanks, > Star > -----Original Message----- > From: Kinney, Michael D > Sent: Tuesday, November 14, 2017 1:08 AM > To: Zeng, Star ; edk2- > devel@lists.01.org; Kinney, Michael D > > Cc: Dong, Eric ; Zeng, Star > > Subject: RE: [Patch] MdeModulePkg/UsbMassStorageDxe: > Verify Get Max LUN value >=20 > Star, >=20 > I do not recall the exact value I saw. >=20 > However, the spec states that one byte is returned with > the max LUN value. It does not state that the byte is > composed of bitfields with the lower bits being the max > LUN, so I prefer the value check instead if the and mask. >=20 > Thanks, >=20 > Mike >=20 >=20 > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel- > bounces@lists.01.org] > > On Behalf Of Zeng, Star > > Sent: Monday, November 13, 2017 2:03 AM > > To: Kinney, Michael D ; > edk2- > > devel@lists.01.org > > Cc: Dong, Eric ; Zeng, Star > > > Subject: Re: [edk2] [Patch] > > MdeModulePkg/UsbMassStorageDxe: Verify Get Max LUN > value > > > > Mike, > > > > Just curious about what is the value returned for > MaxLun in the case > > you met before this patch? > > > > Could *MaxLun &=3D USB_BOT_MAX_LUN work? > > > > > > Thanks, > > Star > > -----Original Message----- > > From: Zeng, Star > > Sent: Monday, November 13, 2017 5:45 PM > > To: Kinney, Michael D ; > edk2- > > devel@lists.01.org > > Cc: Dong, Eric ; Zeng, Star > > > Subject: RE: [Patch] MdeModulePkg/UsbMassStorageDxe: > > Verify Get Max LUN value > > > > Reviewed-by: Star Zeng > > > > How about also adding the compatibility info(now in > commit log) to the > > code comments? > > For example, > > > > + // > > + // If 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 > > + // which means no LUN is associated with the > device. > > + // It improves compatibility with USB FLASH drives > > that have a single LUN, > > + // but return invalid maximum LUN values. > > + // > > > > Thanks, > > Star > > -----Original Message----- > > From: Kinney, Michael D > > Sent: Thursday, November 9, 2017 6:19 AM > > To: edk2-devel@lists.01.org > > Cc: Zeng, Star ; Dong, Eric > > > Subject: [Patch] MdeModulePkg/UsbMassStorageDxe: Verify > Get Max LUN > > value > > > > 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 have a > > single LUN, but return invalid maximum LUN values. > > > > Cc: Star Zeng > > Cc: Eric Dong > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Michael D Kinney > > > > --- > > MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c | > 10 > > +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git > > a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c > > b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c > > index 4bb7222b89..c7436cf036 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.
> > +Copyright (c) 2007 - 2017, Intel Corporation. All > rights > > reserved.
> > 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,14 @@ UsbBotGetMaxLun ( > > 1, > > &Result > > ); > > + if (!EFI_ERROR (Status) && *MaxLun > > USB_BOT_MAX_LUN) > > { > > + // > > + // If 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 > > + // which means no LUN is associated with the > device. > > + // > > + *MaxLun =3D 0; > > + } > > > > return Status; > > } > > -- > > 2.14.2.windows.3 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel