Mike, There was some discussion at https://lists.01.org/pipermail/edk2-devel/2017-January/006707.html. Seeming, there is no detailed information about why it was changed to 128 according to the commit log.
Thanks, Star -----Original Message----- From: Kinney, Michael D Sent: Monday, March 12, 2018 11:47 PM To: Ming Huang <[email protected]>; Ni, Ruiyu <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Zeng, Star <[email protected]>; Dong, Eric <[email protected]>; Kinney, Michael D <[email protected]> Cc: [email protected]; [email protected]; [email protected]; [email protected]; Gao, Liming <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected] Subject: RE: [edk2] [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks Does anyone know wow was the original #define of 128 selected? If we are seeing compatibility issues with 128 and we have greater compatibility with 64, what would be the impact of just changing the #define to 64? Thanks, Mike > -----Original Message----- > From: Ming Huang [mailto:[email protected]] > Sent: Sunday, March 11, 2018 11:01 PM > To: Ni, Ruiyu <[email protected]>; > [email protected]; [email protected]; > [email protected]; [email protected]; Zeng, Star > <[email protected]>; Dong, Eric <[email protected]> > Cc: [email protected]; [email protected]; > [email protected]; [email protected]; Kinney, Michael D > <[email protected]>; Gao, Liming <[email protected]>; > [email protected]; [email protected]; [email protected]; > [email protected] > Subject: Re: [edk2] [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for > UsbBootIoBlocks > > Any comments about this patch? > > > On 2018/3/2 14:36, Ming Huang wrote: > > > > On 2018/2/27 18:01, Ni, Ruiyu wrote: > >> On 2/27/2018 5:25 PM, Ming Huang wrote: > >>> > >>> On 2018/2/27 13:25, Ni, Ruiyu wrote: > >>>> I don't prefer to add PCD, unless we cannot find: > >>>> 1. spec content to describe the max/min blocks > >>> There is no spec about the max/min blocks in my > mind. I had checked this in > >>> several pdf document like > >>> Universal Serial Bus Mass Storage Class > Specification Overview, > >>> Universal Serial Bus Mass Storage Specification For > Bootability, > >>> Universal Serial Bus Mass Storage Class Bulk-Only > Transport. > >>>> 2. error handling when the blocks number is bigger > than HW expects. > >>> Where should error handling add to? Error handing > can't add to HW (end-point device), > >>> because HW is not in our control scope. > >> I mean maybe spec describes an error status could be > returned from HW when using 128. So that we can use 64, 32, and > smaller value until HW is happy. > >> > >> I am curious how the other USB storage drivers handle > this. > >> PCD is a static way. Dynamic way is more preferred. > >> > > When using 128, after waiting > 5x5(USB_BOOT_COMMAND_RETRY=5, > USB_BOOT_GENERAL_CMD_TIMEOUT=5) seconds, > > the UsbBootReadBlocks ->UsbBootExecCmdWithRetry retrun > TimeOut. I don't know why HW return Timeout. > > Booting time is to long if using Dynamic way to fix > the issue. > > When using 64, It works and booting from HW succeed. > > May be using PCD is a simple and effective mean. > > > > Thanks > > Ming > >>>> Thanks/Ray > >>>> > >>>>> -----Original Message----- > >>>>> From: edk2-devel [mailto:edk2-devel- > [email protected]] On Behalf Of > >>>>> Ming Huang > >>>>> Sent: Saturday, February 24, 2018 5:30 PM > >>>>> To: [email protected]; linaro- > [email protected]; edk2- > >>>>> [email protected]; [email protected]; > Zeng, Star > >>>>> <[email protected]>; Dong, Eric > <[email protected]> > >>>>> Cc: [email protected]; > [email protected]; Ming Huang > >>>>> <[email protected]>; Gao, Liming > <[email protected]>; > >>>>> [email protected]; [email protected]; > >>>>> [email protected]; Kinney, Michael D > >>>>> <[email protected]>; [email protected]; > >>>>> [email protected]; [email protected] > >>>>> Subject: [edk2] [RFC v1 1/1] MdeModulePkg/Usb: Use > Pcd for > >>>>> UsbBootIoBlocks > >>>>> > >>>>> Booting from USB may fail while the macro > USB_BOOT_IO_BLOCKS set to 128 > >>>>> because 128 blocks is exceeded the maximun blocks > of some USB > >>>>> devices,like some virtual CD-ROM from BMC. So, > give a chance to set the > >>>>> value of USB_BOOT_IO_BLOCKS by adding a Pcd. > >>>>> > >>>>> Contributed-under: TianoCore Contribution > Agreement 1.1 > >>>>> Signed-off-by: Ming Huang <[email protected]> > >>>>> --- > >>>>> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBo > ot.h | 7 > >>>>> +++++-- > >>>>> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassSt > orageDxe.inf | 4 > >>>>> ++++ > >>>>> MdeModulePkg/MdeModulePkg.dec > | 4 ++++ > >>>>> MdeModulePkg/MdeModulePkg.uni > | 4 ++++ > >>>>> 4 files changed, 17 insertions(+), 2 deletions(- > ) > >>>>> > >>>>> diff --git > a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h > >>>>> > b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h > >>>>> index 5ee50ac52a21..ca9240adbd5f 100644 > >>>>> --- > a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h > >>>>> +++ > b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h > >>>>> @@ -16,6 +16,8 @@ WITHOUT WARRANTIES OR > REPRESENTATIONS OF ANY > >>>>> KIND, EITHER EXPRESS OR IMPLIED. > >>>>> #ifndef _EFI_USB_MASS_BOOT_H_ > >>>>> #define _EFI_USB_MASS_BOOT_H_ > >>>>> > >>>>> +#include <Library/PcdLib.h> > >>>>> + > >>>>> // > >>>>> // The opcodes of various USB boot commands: > >>>>> // INQUIRY/REQUEST_SENSE are "No Timeout > Commands" as specified @@ > >>>>> -66,9 +68,10 @@ WITHOUT WARRANTIES OR > REPRESENTATIONS OF ANY > >>>>> KIND, EITHER EXPRESS OR IMPLIED. > >>>>> #define > USB_PDT_SIMPLE_DIRECT 0x0E ///< Simplified direct > access > >>>>> device > >>>>> > >>>>> // > >>>>> -// Other parameters, Max carried size is 512B * > 128 = 64KB > >>>>> +// Other parameters, Max carried size is depanded > on Pcd. > >>>>> +// The default of PcdUsbBootIoBlocks is 128. 512B > * 128 = 64KB > >>>>> // > >>>>> -#define USB_BOOT_IO_BLOCKS 128 > >>>>> +#define > USB_BOOT_IO_BLOCKS (FixedPcdGet32 > >>>>> (PcdUsbBootIoBlocks)) > >>>>> > >>>>> // > >>>>> // Retry mass command times, set by experience > diff --git > >>>>> > a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageD > xe.inf > >>>>> > b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageD > xe.inf > >>>>> index 26d15c7679bf..40426512f884 100644 > >>>>> --- > >>>>> > a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageD > xe.inf > >>>>> +++ > >>>>> > b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageD > xe.inf > >>>>> @@ -60,6 +60,7 @@ [Sources] > >>>>> UsbMassDiskInfo.c > >>>>> > >>>>> [Packages] > >>>>> + MdeModulePkg/MdeModulePkg.dec > >>>>> MdePkg/MdePkg.dec > >>>>> > >>>>> [LibraryClasses] > >>>>> @@ -83,5 +84,8 @@ [Protocols] > >>>>> # EVENT_TYPE_RELATIVE_TIMER ## CONSUMES > >>>>> # > >>>>> > >>>>> +[FixedPcd] > >>>>> > + gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks > >>>>> + > >>>>> [UserExtensions.TianoCore."ExtraFiles"] > >>>>> UsbMassStorageDxeExtra.uni > >>>>> diff --git a/MdeModulePkg/MdeModulePkg.dec > >>>>> b/MdeModulePkg/MdeModulePkg.dec index > 455979386e3f..fc40745315a0 > >>>>> 100644 > >>>>> --- a/MdeModulePkg/MdeModulePkg.dec > >>>>> +++ b/MdeModulePkg/MdeModulePkg.dec > >>>>> @@ -999,6 +999,10 @@ [PcdsFixedAtBuild] > >>>>> # @Prompt Enable UEFI Stack Guard. > >>>>> > >>>>> > gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BO > OLEAN|0 > >>>>> x30001055 > >>>>> > >>>>> +## The Max blocks of usb transfer. The default is > 128. > >>>>> +# @Prompt The Max blocks of usb transfer > >>>>> > +gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks|128|U > INT32|0x0 > >>>>> 000010B > >>>>> + > >>>>> [PcdsFixedAtBuild, PcdsPatchableInModule] > >>>>> ## Dynamic type PCD can be registered callback > function for Pcd setting > >>>>> action. > >>>>> # PcdMaxPeiPcdCallBackNumberPerPcdEntry > indicates the maximum > >>>>> number of callback function diff --git > a/MdeModulePkg/MdeModulePkg.uni > >>>>> b/MdeModulePkg/MdeModulePkg.uni index > f3fa616438b0..c996d6b4ebe0 > >>>>> 100644 > >>>>> --- a/MdeModulePkg/MdeModulePkg.uni > >>>>> +++ b/MdeModulePkg/MdeModulePkg.uni > >>>>> @@ -1243,3 +1243,7 @@ > >>>>> #string > >>>>> > STR_gEfiMdeModulePkgTokenSpaceGuid_PcdEdkiiFpdtStringRec > ordEnableO > >>>>> nly_HELP #language en-US "Control which FPDT > record format will be used > >>>>> to store the performance entry.\n" > >>>>> > "O n TRUE, the > string FPDT > >>>>> record will be used to store every performance > entry.\n" > >>>>> > "O n FALSE, the > different > >>>>> FPDT record will be used to store the different > performance entries." > >>>>> + > >>>>> +#string > >>>>> > STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_PR > OMPT > >>>>> #language en-US "The Max blocks of usb transfer." > >>>>> + > >>>>> +#string > >>>>> > STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_HE > LP > >>>>> #language en-US "The Max blocks of usb transfer. > The default is 128." > >>>>> -- > >>>>> 1.9.1 > >>>>> > >>>>> _______________________________________________ > >>>>> edk2-devel mailing list > >>>>> [email protected] > >>>>> https://lists.01.org/mailman/listinfo/edk2-devel > >>> _______________________________________________ > >>> edk2-devel mailing list > >>> [email protected] > >>> https://lists.01.org/mailman/listinfo/edk2-devel > >>> > >> _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
