* [RFC v1 0/1] Add PcdUsbBootIoBlocks to UsbMassBoot
@ 2018-02-24 9:29 Ming Huang
2018-02-24 9:29 ` [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks Ming Huang
0 siblings, 1 reply; 11+ messages in thread
From: Ming Huang @ 2018-02-24 9:29 UTC (permalink / raw)
To: leif.lindholm, linaro-uefi, edk2-devel, graeme.gregory, star.zeng,
eric.dong
Cc: ard.biesheuvel, michael.d.kinney, liming.gao, guoheyi,
wanghuiqiang, huangming23, zhangjinsong2, mengfanrong, huangdaode,
waip23, Ming Huang
Adding PcdUsbBootIoBlocks to UsbMassBoot to fix booting failed from
USB issue in some platforms.
Ming Huang (1):
MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h | 7 +++++--
MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf | 4 ++++
MdeModulePkg/MdeModulePkg.dec | 4 ++++
MdeModulePkg/MdeModulePkg.uni | 4 ++++
4 files changed, 17 insertions(+), 2 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-02-24 9:29 [RFC v1 0/1] Add PcdUsbBootIoBlocks to UsbMassBoot Ming Huang
@ 2018-02-24 9:29 ` Ming Huang
2018-02-27 5:25 ` Ni, Ruiyu
0 siblings, 1 reply; 11+ messages in thread
From: Ming Huang @ 2018-02-24 9:29 UTC (permalink / raw)
To: leif.lindholm, linaro-uefi, edk2-devel, graeme.gregory, star.zeng,
eric.dong
Cc: ard.biesheuvel, michael.d.kinney, liming.gao, guoheyi,
wanghuiqiang, huangming23, zhangjinsong2, mengfanrong, huangdaode,
waip23, Ming Huang
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/UsbMassBoot.h | 7 +++++--
MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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/UsbMassStorageDxe.inf b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
index 26d15c7679bf..40426512f884 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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|BOOLEAN|0x30001055
+## The Max blocks of usb transfer. The default is 128.
+# @Prompt The Max blocks of usb transfer
+gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks|128|UINT32|0x0000010B
+
[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_PcdEdkiiFpdtStringRecordEnableOnly_HELP #language en-US "Control which FPDT record format will be used to store the performance entry.\n"
"On TRUE, the string FPDT record will be used to store every performance entry.\n"
"On FALSE, the different FPDT record will be used to store the different performance entries."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_PROMPT #language en-US "The Max blocks of usb transfer."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_HELP #language en-US "The Max blocks of usb transfer. The default is 128."
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-02-24 9:29 ` [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks Ming Huang
@ 2018-02-27 5:25 ` Ni, Ruiyu
2018-02-27 9:25 ` Ming Huang
0 siblings, 1 reply; 11+ messages in thread
From: Ni, Ruiyu @ 2018-02-27 5:25 UTC (permalink / raw)
To: Ming Huang, leif.lindholm@linaro.org,
linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org,
graeme.gregory@linaro.org, Zeng, Star, Dong, Eric
Cc: huangming23@huawei.com, ard.biesheuvel@linaro.org, Gao, Liming,
mengfanrong@huawei.com, guoheyi@huawei.com,
zhangjinsong2@huawei.com, Kinney, Michael D, waip23@126.com,
wanghuiqiang@huawei.com, huangdaode@hisilicon.com
I don't prefer to add PCD, unless we cannot find:
1. spec content to describe the max/min blocks
2. error handling when the blocks number is bigger than HW expects.
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/UsbMassBoot.h | 7
> +++++--
> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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/UsbMassStorageDxe.inf
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> index 26d15c7679bf..40426512f884 100644
> ---
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
> +++
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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|BOOLEAN|0
> x30001055
>
> +## The Max blocks of usb transfer. The default is 128.
> +# @Prompt The Max blocks of usb transfer
> +gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks|128|UINT32|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_PcdEdkiiFpdtStringRecordEnableO
> nly_HELP #language en-US "Control which FPDT record format will be used
> to store the performance entry.\n"
> "On TRUE, the string FPDT
> record will be used to store every performance entry.\n"
> "On FALSE, the different
> FPDT record will be used to store the different performance entries."
> +
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_PROMPT
> #language en-US "The Max blocks of usb transfer."
> +
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_HELP
> #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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-02-27 5:25 ` Ni, Ruiyu
@ 2018-02-27 9:25 ` Ming Huang
2018-02-27 10:01 ` Ni, Ruiyu
0 siblings, 1 reply; 11+ messages in thread
From: Ming Huang @ 2018-02-27 9:25 UTC (permalink / raw)
To: Ni, Ruiyu, leif.lindholm@linaro.org, linaro-uefi@lists.linaro.org,
edk2-devel@lists.01.org, graeme.gregory@linaro.org, Zeng, Star,
Dong, Eric
Cc: huangming23@huawei.com, ard.biesheuvel@linaro.org, Gao, Liming,
mengfanrong@huawei.com, guoheyi@huawei.com,
zhangjinsong2@huawei.com, Kinney, Michael D, waip23@126.com,
wanghuiqiang@huawei.com, huangdaode@hisilicon.com
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.
>
> 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/UsbMassBoot.h | 7
>> +++++--
>> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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/UsbMassStorageDxe.inf
>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>> index 26d15c7679bf..40426512f884 100644
>> ---
>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>> +++
>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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|BOOLEAN|0
>> x30001055
>>
>> +## The Max blocks of usb transfer. The default is 128.
>> +# @Prompt The Max blocks of usb transfer
>> +gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks|128|UINT32|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_PcdEdkiiFpdtStringRecordEnableO
>> nly_HELP #language en-US "Control which FPDT record format will be used
>> to store the performance entry.\n"
>> "On TRUE, the string FPDT
>> record will be used to store every performance entry.\n"
>> "On FALSE, the different
>> FPDT record will be used to store the different performance entries."
>> +
>> +#string
>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_PROMPT
>> #language en-US "The Max blocks of usb transfer."
>> +
>> +#string
>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_HELP
>> #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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-02-27 9:25 ` Ming Huang
@ 2018-02-27 10:01 ` Ni, Ruiyu
2018-03-02 6:36 ` Ming Huang
0 siblings, 1 reply; 11+ messages in thread
From: Ni, Ruiyu @ 2018-02-27 10:01 UTC (permalink / raw)
To: Ming Huang, leif.lindholm@linaro.org,
linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org,
graeme.gregory@linaro.org, Zeng, Star, Dong, Eric
Cc: huangming23@huawei.com, wanghuiqiang@huawei.com,
ard.biesheuvel@linaro.org, zhangjinsong2@huawei.com,
Kinney, Michael D, Gao, Liming, guoheyi@huawei.com,
waip23@126.com, mengfanrong@huawei.com, huangdaode@hisilicon.com
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.
>>
>> 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/UsbMassBoot.h | 7
>>> +++++--
>>> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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/UsbMassStorageDxe.inf
>>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>>> index 26d15c7679bf..40426512f884 100644
>>> ---
>>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>>> +++
>>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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|BOOLEAN|0
>>> x30001055
>>>
>>> +## The Max blocks of usb transfer. The default is 128.
>>> +# @Prompt The Max blocks of usb transfer
>>> +gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks|128|UINT32|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_PcdEdkiiFpdtStringRecordEnableO
>>> nly_HELP #language en-US "Control which FPDT record format will be used
>>> to store the performance entry.\n"
>>> "On TRUE, the string FPDT
>>> record will be used to store every performance entry.\n"
>>> "On FALSE, the different
>>> FPDT record will be used to store the different performance entries."
>>> +
>>> +#string
>>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_PROMPT
>>> #language en-US "The Max blocks of usb transfer."
>>> +
>>> +#string
>>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_HELP
>>> #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
>
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-02-27 10:01 ` Ni, Ruiyu
@ 2018-03-02 6:36 ` Ming Huang
2018-03-12 6:01 ` Ming Huang
0 siblings, 1 reply; 11+ messages in thread
From: Ming Huang @ 2018-03-02 6:36 UTC (permalink / raw)
To: Ni, Ruiyu, leif.lindholm@linaro.org, linaro-uefi@lists.linaro.org,
edk2-devel@lists.01.org, graeme.gregory@linaro.org, Zeng, Star,
Dong, Eric
Cc: huangming23@huawei.com, wanghuiqiang@huawei.com,
ard.biesheuvel@linaro.org, zhangjinsong2@huawei.com,
Kinney, Michael D, Gao, Liming, guoheyi@huawei.com,
waip23@126.com, mengfanrong@huawei.com, huangdaode@hisilicon.com
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-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/UsbMassBoot.h | 7
>>>> +++++--
>>>> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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/UsbMassStorageDxe.inf
>>>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>>>> index 26d15c7679bf..40426512f884 100644
>>>> ---
>>>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>>>> +++
>>>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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|BOOLEAN|0
>>>> x30001055
>>>>
>>>> +## The Max blocks of usb transfer. The default is 128.
>>>> +# @Prompt The Max blocks of usb transfer
>>>> +gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks|128|UINT32|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_PcdEdkiiFpdtStringRecordEnableO
>>>> nly_HELP #language en-US "Control which FPDT record format will be used
>>>> to store the performance entry.\n"
>>>> "On TRUE, the string FPDT
>>>> record will be used to store every performance entry.\n"
>>>> "On FALSE, the different
>>>> FPDT record will be used to store the different performance entries."
>>>> +
>>>> +#string
>>>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_PROMPT
>>>> #language en-US "The Max blocks of usb transfer."
>>>> +
>>>> +#string
>>>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_HELP
>>>> #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
>>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-03-02 6:36 ` Ming Huang
@ 2018-03-12 6:01 ` Ming Huang
2018-03-12 15:47 ` Kinney, Michael D
0 siblings, 1 reply; 11+ messages in thread
From: Ming Huang @ 2018-03-12 6:01 UTC (permalink / raw)
To: Ni, Ruiyu, leif.lindholm@linaro.org, linaro-uefi@lists.linaro.org,
edk2-devel@lists.01.org, graeme.gregory@linaro.org, Zeng, Star,
Dong, Eric
Cc: huangming23@huawei.com, wanghuiqiang@huawei.com,
ard.biesheuvel@linaro.org, zhangjinsong2@huawei.com,
Kinney, Michael D, Gao, Liming, guoheyi@huawei.com,
waip23@126.com, mengfanrong@huawei.com, huangdaode@hisilicon.com
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-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/UsbMassBoot.h | 7
>>>>> +++++--
>>>>> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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/UsbMassStorageDxe.inf
>>>>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>>>>> index 26d15c7679bf..40426512f884 100644
>>>>> ---
>>>>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>>>>> +++
>>>>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.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|BOOLEAN|0
>>>>> x30001055
>>>>>
>>>>> +## The Max blocks of usb transfer. The default is 128.
>>>>> +# @Prompt The Max blocks of usb transfer
>>>>> +gEfiMdeModulePkgTokenSpaceGuid.PcdUsbBootIoBlocks|128|UINT32|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_PcdEdkiiFpdtStringRecordEnableO
>>>>> nly_HELP #language en-US "Control which FPDT record format will be used
>>>>> to store the performance entry.\n"
>>>>> "On TRUE, the string FPDT
>>>>> record will be used to store every performance entry.\n"
>>>>> "On FALSE, the different
>>>>> FPDT record will be used to store the different performance entries."
>>>>> +
>>>>> +#string
>>>>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_PROMPT
>>>>> #language en-US "The Max blocks of usb transfer."
>>>>> +
>>>>> +#string
>>>>> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUsbBootIoBlocks_HELP
>>>>> #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
>>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-03-12 6:01 ` Ming Huang
@ 2018-03-12 15:47 ` Kinney, Michael D
2018-03-13 0:44 ` Zeng, Star
0 siblings, 1 reply; 11+ messages in thread
From: Kinney, Michael D @ 2018-03-12 15:47 UTC (permalink / raw)
To: Ming Huang, Ni, Ruiyu, leif.lindholm@linaro.org,
linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org,
graeme.gregory@linaro.org, Zeng, Star, Dong, Eric,
Kinney, Michael D
Cc: huangming23@huawei.com, wanghuiqiang@huawei.com,
ard.biesheuvel@linaro.org, zhangjinsong2@huawei.com, Gao, Liming,
guoheyi@huawei.com, waip23@126.com, mengfanrong@huawei.com,
huangdaode@hisilicon.com
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:
> >>>> 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-
> 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
> 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
> >>>>> 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
> >>>
> >>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-03-12 15:47 ` Kinney, Michael D
@ 2018-03-13 0:44 ` Zeng, Star
2018-03-13 1:38 ` Kinney, Michael D
0 siblings, 1 reply; 11+ messages in thread
From: Zeng, Star @ 2018-03-13 0:44 UTC (permalink / raw)
To: Kinney, Michael D, Ming Huang, Ni, Ruiyu,
leif.lindholm@linaro.org, linaro-uefi@lists.linaro.org,
edk2-devel@lists.01.org, graeme.gregory@linaro.org, Dong, Eric
Cc: huangming23@huawei.com, wanghuiqiang@huawei.com,
ard.biesheuvel@linaro.org, zhangjinsong2@huawei.com, Gao, Liming,
guoheyi@huawei.com, waip23@126.com, mengfanrong@huawei.com,
huangdaode@hisilicon.com, Zeng, Star
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:
> >>>> 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-
> 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
> 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
> >>>>> 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
> >>>
> >>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-03-13 0:44 ` Zeng, Star
@ 2018-03-13 1:38 ` Kinney, Michael D
2018-03-14 12:40 ` Ming Huang
0 siblings, 1 reply; 11+ messages in thread
From: Kinney, Michael D @ 2018-03-13 1:38 UTC (permalink / raw)
To: Zeng, Star, Ming Huang, Ni, Ruiyu, leif.lindholm@linaro.org,
linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org,
graeme.gregory@linaro.org, Dong, Eric, Kinney, Michael D
Cc: huangming23@huawei.com, wanghuiqiang@huawei.com,
ard.biesheuvel@linaro.org, zhangjinsong2@huawei.com, Gao, Liming,
guoheyi@huawei.com, waip23@126.com, mengfanrong@huawei.com,
huangdaode@hisilicon.com
Star,
Maybe we should evaluate block size of the target USB
device and limit the total transfer size to 64KB.
This would continue to use 128 for 512B blocks and
32 for 2K blocks.
Mike
> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, March 12, 2018 5:44 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; 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; Dong, Eric
> <eric.dong@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; Zeng, Star
> <star.zeng@intel.com>
> Subject: RE: [edk2] [RFC v1 1/1] MdeModulePkg/Usb: Use
> Pcd for UsbBootIoBlocks
>
> 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:
> > >>>> 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-
> > 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
> > 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
> > >>>>> 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
> > >>>
> > >>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks
2018-03-13 1:38 ` Kinney, Michael D
@ 2018-03-14 12:40 ` Ming Huang
0 siblings, 0 replies; 11+ messages in thread
From: Ming Huang @ 2018-03-14 12:40 UTC (permalink / raw)
To: Kinney, Michael D, Zeng, Star, Ni, Ruiyu,
leif.lindholm@linaro.org, linaro-uefi@lists.linaro.org,
edk2-devel@lists.01.org, graeme.gregory@linaro.org, Dong, Eric
Cc: huangming23@huawei.com, wanghuiqiang@huawei.com,
ard.biesheuvel@linaro.org, zhangjinsong2@huawei.com, Gao, Liming,
guoheyi@huawei.com, waip23@126.com, mengfanrong@huawei.com,
huangdaode@hisilicon.com
Mike,
Evaluating block size is a good idea!
The block size of the target USB device is 2048 in my platform.
I modify the UsbMassBoot.c with evaluating block size and it works.
I will send the patch later.
Thanks,
Ming
On 2018/3/13 9:38, Kinney, Michael D wrote:
> Star,
>
> Maybe we should evaluate block size of the target USB
> device and limit the total transfer size to 64KB.
>
> This would continue to use 128 for 512B blocks and
> 32 for 2K blocks.
>
> Mike
>
>> -----Original Message-----
>> From: Zeng, Star
>> Sent: Monday, March 12, 2018 5:44 PM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>; 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; Dong, Eric
>> <eric.dong@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; Zeng, Star
>> <star.zeng@intel.com>
>> Subject: RE: [edk2] [RFC v1 1/1] MdeModulePkg/Usb: Use
>> Pcd for UsbBootIoBlocks
>>
>> 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:
>>>>>>> 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-
>>> 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
>>> 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
>>>>>>>> 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
>>>>>>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-03-14 12:34 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-24 9:29 [RFC v1 0/1] Add PcdUsbBootIoBlocks to UsbMassBoot Ming Huang
2018-02-24 9:29 ` [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks Ming Huang
2018-02-27 5:25 ` Ni, Ruiyu
2018-02-27 9:25 ` Ming Huang
2018-02-27 10:01 ` Ni, Ruiyu
2018-03-02 6:36 ` Ming Huang
2018-03-12 6:01 ` Ming Huang
2018-03-12 15:47 ` Kinney, Michael D
2018-03-13 0:44 ` Zeng, Star
2018-03-13 1:38 ` Kinney, Michael D
2018-03-14 12:40 ` Ming Huang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox