Re: [Linaro-uefi] [edk2] [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks

13 Mar 2018

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 ming.huang@linaro.org; Ni, Ruiyu ruiyu.ni@intel.com; leif.lindholm@linaro.org; linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org; graeme.gregory@linaro.org; Zeng, Star star.zeng@intel.com; Dong, Eric eric.dong@intel.com; Kinney, Michael D michael.d.kinney@intel.com
Cc: huangming23@huawei.com; wanghuiqiang@huawei.com; ard.biesheuvel@linaro.org; zhangjinsong2@huawei.com; Gao, Liming liming.gao@intel.com; guoheyi@huawei.com; waip23@126.com; mengfanrong@huawei.com; huangdaode@hisilicon.com
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:ming.huang@linaro.org]
Sent: Sunday, March 11, 2018 11:01 PM
To: Ni, Ruiyu ruiyu.ni@intel.com;
leif.lindholm@linaro.org; linaro-uefi@lists.linaro.org; 
edk2-devel@lists.01.org; graeme.gregory@linaro.org; Zeng, Star 
star.zeng@intel.com; Dong, Eric eric.dong@intel.com
Cc: huangming23@huawei.com; wanghuiqiang@huawei.com; 
ard.biesheuvel@linaro.org; zhangjinsong2@huawei.com; Kinney, Michael D 
michael.d.kinney@intel.com; Gao, Liming liming.gao@intel.com; 
guoheyi@huawei.com; waip23@126.com; mengfanrong@huawei.com; 
huangdaode@hisilicon.com
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:

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.
...
...
...
...

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-
bounces@lists.01.org] On Behalf Of
...
...
...
...
...
Ming Huang
Sent: Saturday, February 24, 2018 5:30 PM
To: leif.lindholm@linaro.org; linaro-
uefi@lists.linaro.org; edk2-
...
...
...
...
...
devel@lists.01.org; graeme.gregory@linaro.org;
Zeng, Star
...
...
...
...
...
star.zeng@intel.com; Dong, Eric
eric.dong@intel.com
...
...
...
...
...
Cc: huangming23@huawei.com;
ard.biesheuvel@linaro.org; Ming Huang
...
...
...
...
...
ming.huang@linaro.org; Gao, Liming
liming.gao@intel.com;
...
...
...
...
...
mengfanrong@huawei.com; guoheyi@huawei.com; 
zhangjinsong2@huawei.com; Kinney, Michael D 
michael.d.kinney@intel.com; waip23@126.com; 
wanghuiqiang@huawei.com; huangdaode@hisilicon.com
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 ming.huang@linaro.org
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



...
...
...
...
...
+# @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
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

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

Re: [Linaro-uefi] [edk2] [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks

Signed-off-by: Ming Huang ming.huang@linaro.org

index 5ee50ac52a21..ca9240adbd5f 100644

index 26d15c7679bf..40426512f884 100644