public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2 UsbMassStorageDxe v1 0/1] Calculating the count of blocks to transfer
@ 2018-03-15 12:29 Ming Huang
  2018-03-15 12:29 ` [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS Ming Huang
  0 siblings, 1 reply; 5+ messages in thread
From: Ming Huang @ 2018-03-15 12:29 UTC (permalink / raw)
  To: leif.lindholm, linaro-uefi, edk2-devel, star.zeng, eric.dong
  Cc: ard.biesheuvel, michael.d.kinney, liming.gao, guoheyi,
	wanghuiqiang, huangming23, zhangjinsong2, mengfanrong, huangdaode,
	waip23, Ming Huang

Calculating the count of blocks to transfer by Evaluating block size
to fix booting failed from USB issue in some platforms.


Ming Huang (1):
  MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS

 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c | 16 ++++++++++++----
 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h |  4 ++--
 2 files changed, 14 insertions(+), 6 deletions(-)

-- 
1.9.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
  2018-03-15 12:29 [edk2 UsbMassStorageDxe v1 0/1] Calculating the count of blocks to transfer Ming Huang
@ 2018-03-15 12:29 ` Ming Huang
  2018-03-21 12:40   ` Zeng, Star
  0 siblings, 1 reply; 5+ messages in thread
From: Ming Huang @ 2018-03-15 12:29 UTC (permalink / raw)
  To: leif.lindholm, linaro-uefi, edk2-devel, 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 the block size of some USB devices are exceeded
512. So,the count blocks to transfer should be calculated by block
size of the USB devices.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c | 16 ++++++++++++----
 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h |  4 ++--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
index b84bfd2d7290..b38cb6116bf4 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
@@ -814,11 +814,13 @@ UsbBootReadBlocks (
   USB_BOOT_READ10_CMD       ReadCmd;
   EFI_STATUS                Status;
   UINT16                    Count;
+  UINT16                    CountMax;
   UINT32                    BlockSize;
   UINT32                    ByteSize;
   UINT32                    Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status    = EFI_SUCCESS;
 
   while (TotalBlock > 0) {
@@ -827,7 +829,7 @@ UsbBootReadBlocks (
     // on the device. We must split the total block because the READ10
     // command only has 16 bit transfer length (in the unit of block).
     //
-    Count     = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
+    Count     = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
     ByteSize  = (UINT32)Count * BlockSize;
 
     //
@@ -890,11 +892,13 @@ UsbBootWriteBlocks (
   USB_BOOT_WRITE10_CMD  WriteCmd;
   EFI_STATUS            Status;
   UINT16                Count;
+  UINT16                CountMax;
   UINT32                BlockSize;
   UINT32                ByteSize;
   UINT32                Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status    = EFI_SUCCESS;
 
   while (TotalBlock > 0) {
@@ -903,7 +907,7 @@ UsbBootWriteBlocks (
     // on the device. We must split the total block because the WRITE10
     // command only has 16 bit transfer length (in the unit of block).
     //
-    Count     = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
+    Count     = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
     ByteSize  = (UINT32)Count * BlockSize;
 
     //
@@ -966,18 +970,20 @@ UsbBootReadBlocks16 (
   UINT8                     ReadCmd[16];
   EFI_STATUS                Status;
   UINT16                    Count;
+  UINT16                    CountMax;
   UINT32                    BlockSize;
   UINT32                    ByteSize;
   UINT32                    Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status    = EFI_SUCCESS;
 
   while (TotalBlock > 0) {
     //
     // Split the total blocks into smaller pieces.
     //
-    Count     = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
+    Count     = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
     ByteSize  = (UINT32)Count * BlockSize;
 
     //
@@ -1040,18 +1046,20 @@ UsbBootWriteBlocks16 (
   UINT8                 WriteCmd[16];
   EFI_STATUS            Status;
   UINT16                Count;
+  UINT16                CountMax;
   UINT32                BlockSize;
   UINT32                ByteSize;
   UINT32                Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status    = EFI_SUCCESS;
 
   while (TotalBlock > 0) {
     //
     // Split the total blocks into smaller pieces.
     //
-    Count     = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
+    Count     = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
     ByteSize  = (UINT32)Count * BlockSize;
 
     //
diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
index 13a926035ceb..fc5449bde21e 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
@@ -65,9 +65,9 @@ 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 64KB.
 //
-#define USB_BOOT_IO_BLOCKS              128
+#define USB_BOOT_MAX_CARRY_SIZE         0x10000
 
 //
 // Retry mass command times, set by experience
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
  2018-03-15 12:29 ` [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS Ming Huang
@ 2018-03-21 12:40   ` Zeng, Star
  2018-03-21 14:22     ` Kinney, Michael D
  0 siblings, 1 reply; 5+ messages in thread
From: Zeng, Star @ 2018-03-21 12:40 UTC (permalink / raw)
  To: Ming Huang, Ni, Ruiyu, leif.lindholm@linaro.org,
	linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org, Dong, Eric
  Cc: ard.biesheuvel@linaro.org, Kinney, Michael D, Gao, Liming,
	guoheyi@huawei.com, wanghuiqiang@huawei.com,
	huangming23@huawei.com, zhangjinsong2@huawei.com,
	mengfanrong@huawei.com, huangdaode@hisilicon.com, waip23@126.com

Ming,

Basically, I am ok with the change in this patch. There are two comments.

1. Please add more background information about the specific device in the commit log. For example, you mentioned " like some virtual CD-ROM from BMC " in previous patch.
2. Please make sure building pass with the patch on different tool chains. I tried to build with the patch on VS2015, but met building failure like below.
warning C4244: '=': conversion from 'UINT32' to 'UINT16', possible loss of data


Ray,

Do you have other concern?


Thanks,
Star
-----Original Message-----
From: Ming Huang [mailto:ming.huang@linaro.org] 
Sent: Thursday, March 15, 2018 8:30 PM
To: leif.lindholm@linaro.org; linaro-uefi@lists.linaro.org; edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>
Cc: ard.biesheuvel@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; guoheyi@huawei.com; wanghuiqiang@huawei.com; huangming23@huawei.com; zhangjinsong2@huawei.com; mengfanrong@huawei.com; huangdaode@hisilicon.com; waip23@126.com; Ming Huang <ming.huang@linaro.org>
Subject: [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS

Booting from USB may fail while the macro USB_BOOT_IO_BLOCKS set to 128 because the block size of some USB devices are exceeded 512. So,the count blocks to transfer should be calculated by block size of the USB devices.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c | 16 ++++++++++++----  MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h |  4 ++--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
index b84bfd2d7290..b38cb6116bf4 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
@@ -814,11 +814,13 @@ UsbBootReadBlocks (
   USB_BOOT_READ10_CMD       ReadCmd;
   EFI_STATUS                Status;
   UINT16                    Count;
+  UINT16                    CountMax;
   UINT32                    BlockSize;
   UINT32                    ByteSize;
   UINT32                    Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status    = EFI_SUCCESS;
 
   while (TotalBlock > 0) {
@@ -827,7 +829,7 @@ UsbBootReadBlocks (
     // on the device. We must split the total block because the READ10
     // command only has 16 bit transfer length (in the unit of block).
     //
-    Count     = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
+    Count     = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
     ByteSize  = (UINT32)Count * BlockSize;
 
     //
@@ -890,11 +892,13 @@ UsbBootWriteBlocks (
   USB_BOOT_WRITE10_CMD  WriteCmd;
   EFI_STATUS            Status;
   UINT16                Count;
+  UINT16                CountMax;
   UINT32                BlockSize;
   UINT32                ByteSize;
   UINT32                Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status    = EFI_SUCCESS;
 
   while (TotalBlock > 0) {
@@ -903,7 +907,7 @@ UsbBootWriteBlocks (
     // on the device. We must split the total block because the WRITE10
     // command only has 16 bit transfer length (in the unit of block).
     //
-    Count     = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
+    Count     = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
     ByteSize  = (UINT32)Count * BlockSize;
 
     //
@@ -966,18 +970,20 @@ UsbBootReadBlocks16 (
   UINT8                     ReadCmd[16];
   EFI_STATUS                Status;
   UINT16                    Count;
+  UINT16                    CountMax;
   UINT32                    BlockSize;
   UINT32                    ByteSize;
   UINT32                    Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status    = EFI_SUCCESS;
 
   while (TotalBlock > 0) {
     //
     // Split the total blocks into smaller pieces.
     //
-    Count     = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
+    Count     = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
     ByteSize  = (UINT32)Count * BlockSize;
 
     //
@@ -1040,18 +1046,20 @@ UsbBootWriteBlocks16 (
   UINT8                 WriteCmd[16];
   EFI_STATUS            Status;
   UINT16                Count;
+  UINT16                CountMax;
   UINT32                BlockSize;
   UINT32                ByteSize;
   UINT32                Timeout;
 
   BlockSize = UsbMass->BlockIoMedia.BlockSize;
+  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
   Status    = EFI_SUCCESS;
 
   while (TotalBlock > 0) {
     //
     // Split the total blocks into smaller pieces.
     //
-    Count     = (UINT16)((TotalBlock < USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
+    Count     = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax);
     ByteSize  = (UINT32)Count * BlockSize;
 
     //
diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
index 13a926035ceb..fc5449bde21e 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
@@ -65,9 +65,9 @@ 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 64KB.
 //
-#define USB_BOOT_IO_BLOCKS              128
+#define USB_BOOT_MAX_CARRY_SIZE         0x10000
 
 //
 // Retry mass command times, set by experience
--
1.9.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
  2018-03-21 12:40   ` Zeng, Star
@ 2018-03-21 14:22     ` Kinney, Michael D
  2018-03-22  3:31       ` Ming Huang
  0 siblings, 1 reply; 5+ messages in thread
From: Kinney, Michael D @ 2018-03-21 14:22 UTC (permalink / raw)
  To: Zeng, Star, Ming Huang, Ni, Ruiyu, leif.lindholm@linaro.org,
	linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org, Dong, Eric,
	Kinney, Michael D
  Cc: ard.biesheuvel@linaro.org, Gao, Liming, guoheyi@huawei.com,
	wanghuiqiang@huawei.com, huangming23@huawei.com,
	zhangjinsong2@huawei.com, mengfanrong@huawei.com,
	huangdaode@hisilicon.com, waip23@126.com

One minor comment.

  #define USB_BOOT_MAX_CARRY_SIZE         0x10000

Should be

  #define USB_BOOT_MAX_CARRY_SIZE         SIZE_64KB

Mike

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, March 21, 2018 5:41 AM
> 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; Dong,
> Eric <eric.dong@intel.com>
> Cc: ard.biesheuvel@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; guoheyi@huawei.com;
> wanghuiqiang@huawei.com; huangming23@huawei.com;
> zhangjinsong2@huawei.com; mengfanrong@huawei.com;
> huangdaode@hisilicon.com; waip23@126.com
> Subject: RE: [edk2 UsbMassStorageDxe v1 1/1]
> MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
> 
> Ming,
> 
> Basically, I am ok with the change in this patch. There
> are two comments.
> 
> 1. Please add more background information about the
> specific device in the commit log. For example, you
> mentioned " like some virtual CD-ROM from BMC " in
> previous patch.
> 2. Please make sure building pass with the patch on
> different tool chains. I tried to build with the patch
> on VS2015, but met building failure like below.
> warning C4244: '=': conversion from 'UINT32' to
> 'UINT16', possible loss of data
> 
> 
> Ray,
> 
> Do you have other concern?
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Ming Huang [mailto:ming.huang@linaro.org]
> Sent: Thursday, March 15, 2018 8:30 PM
> To: leif.lindholm@linaro.org; linaro-
> uefi@lists.linaro.org; edk2-devel@lists.01.org; Zeng,
> Star <star.zeng@intel.com>; Dong, Eric
> <eric.dong@intel.com>
> Cc: ard.biesheuvel@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; guoheyi@huawei.com;
> wanghuiqiang@huawei.com; huangming23@huawei.com;
> zhangjinsong2@huawei.com; mengfanrong@huawei.com;
> huangdaode@hisilicon.com; waip23@126.com; Ming Huang
> <ming.huang@linaro.org>
> Subject: [edk2 UsbMassStorageDxe v1 1/1]
> MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
> 
> Booting from USB may fail while the macro
> USB_BOOT_IO_BLOCKS set to 128 because the block size of
> some USB devices are exceeded 512. So,the count blocks
> to transfer should be calculated by block size of the
> USB devices.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c |
> 16 ++++++++++++----
> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h |
> 4 ++--
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
> index b84bfd2d7290..b38cb6116bf4 100644
> ---
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
> +++
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
> @@ -814,11 +814,13 @@ UsbBootReadBlocks (
>    USB_BOOT_READ10_CMD       ReadCmd;
>    EFI_STATUS                Status;
>    UINT16                    Count;
> +  UINT16                    CountMax;
>    UINT32                    BlockSize;
>    UINT32                    ByteSize;
>    UINT32                    Timeout;
> 
>    BlockSize = UsbMass->BlockIoMedia.BlockSize;
> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>    Status    = EFI_SUCCESS;
> 
>    while (TotalBlock > 0) {
> @@ -827,7 +829,7 @@ UsbBootReadBlocks (
>      // on the device. We must split the total block
> because the READ10
>      // command only has 16 bit transfer length (in the
> unit of block).
>      //
> -    Count     = (UINT16)((TotalBlock <
> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
> +    Count     = (UINT16)((TotalBlock < CountMax) ?
> TotalBlock : CountMax);
>      ByteSize  = (UINT32)Count * BlockSize;
> 
>      //
> @@ -890,11 +892,13 @@ UsbBootWriteBlocks (
>    USB_BOOT_WRITE10_CMD  WriteCmd;
>    EFI_STATUS            Status;
>    UINT16                Count;
> +  UINT16                CountMax;
>    UINT32                BlockSize;
>    UINT32                ByteSize;
>    UINT32                Timeout;
> 
>    BlockSize = UsbMass->BlockIoMedia.BlockSize;
> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>    Status    = EFI_SUCCESS;
> 
>    while (TotalBlock > 0) {
> @@ -903,7 +907,7 @@ UsbBootWriteBlocks (
>      // on the device. We must split the total block
> because the WRITE10
>      // command only has 16 bit transfer length (in the
> unit of block).
>      //
> -    Count     = (UINT16)((TotalBlock <
> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
> +    Count     = (UINT16)((TotalBlock < CountMax) ?
> TotalBlock : CountMax);
>      ByteSize  = (UINT32)Count * BlockSize;
> 
>      //
> @@ -966,18 +970,20 @@ UsbBootReadBlocks16 (
>    UINT8                     ReadCmd[16];
>    EFI_STATUS                Status;
>    UINT16                    Count;
> +  UINT16                    CountMax;
>    UINT32                    BlockSize;
>    UINT32                    ByteSize;
>    UINT32                    Timeout;
> 
>    BlockSize = UsbMass->BlockIoMedia.BlockSize;
> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>    Status    = EFI_SUCCESS;
> 
>    while (TotalBlock > 0) {
>      //
>      // Split the total blocks into smaller pieces.
>      //
> -    Count     = (UINT16)((TotalBlock <
> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
> +    Count     = (UINT16)((TotalBlock < CountMax) ?
> TotalBlock : CountMax);
>      ByteSize  = (UINT32)Count * BlockSize;
> 
>      //
> @@ -1040,18 +1046,20 @@ UsbBootWriteBlocks16 (
>    UINT8                 WriteCmd[16];
>    EFI_STATUS            Status;
>    UINT16                Count;
> +  UINT16                CountMax;
>    UINT32                BlockSize;
>    UINT32                ByteSize;
>    UINT32                Timeout;
> 
>    BlockSize = UsbMass->BlockIoMedia.BlockSize;
> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>    Status    = EFI_SUCCESS;
> 
>    while (TotalBlock > 0) {
>      //
>      // Split the total blocks into smaller pieces.
>      //
> -    Count     = (UINT16)((TotalBlock <
> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
> +    Count     = (UINT16)((TotalBlock < CountMax) ?
> TotalBlock : CountMax);
>      ByteSize  = (UINT32)Count * BlockSize;
> 
>      //
> diff --git
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
> index 13a926035ceb..fc5449bde21e 100644
> ---
> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
> +++
> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
> @@ -65,9 +65,9 @@ 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 64KB.
>  //
> -#define USB_BOOT_IO_BLOCKS              128
> +#define USB_BOOT_MAX_CARRY_SIZE         0x10000
> 
>  //
>  // Retry mass command times, set by experience
> --
> 1.9.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
  2018-03-21 14:22     ` Kinney, Michael D
@ 2018-03-22  3:31       ` Ming Huang
  0 siblings, 0 replies; 5+ messages in thread
From: Ming Huang @ 2018-03-22  3:31 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, Dong, Eric
  Cc: ard.biesheuvel@linaro.org, Gao, Liming, guoheyi@huawei.com,
	wanghuiqiang@huawei.com, huangming23@huawei.com,
	zhangjinsong2@huawei.com, mengfanrong@huawei.com,
	huangdaode@hisilicon.com, waip23@126.com

Star & Mike,

I will fix the issues mentioned below and send v2 patch later.

Thanks,
Ming


On 2018/3/21 22:22, Kinney, Michael D wrote:
> One minor comment.
>
>   #define USB_BOOT_MAX_CARRY_SIZE         0x10000
>
> Should be
>
>   #define USB_BOOT_MAX_CARRY_SIZE         SIZE_64KB
>
> Mike
>
>> -----Original Message-----
>> From: Zeng, Star
>> Sent: Wednesday, March 21, 2018 5:41 AM
>> 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; Dong,
>> Eric <eric.dong@intel.com>
>> Cc: ard.biesheuvel@linaro.org; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Gao, Liming
>> <liming.gao@intel.com>; guoheyi@huawei.com;
>> wanghuiqiang@huawei.com; huangming23@huawei.com;
>> zhangjinsong2@huawei.com; mengfanrong@huawei.com;
>> huangdaode@hisilicon.com; waip23@126.com
>> Subject: RE: [edk2 UsbMassStorageDxe v1 1/1]
>> MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
>>
>> Ming,
>>
>> Basically, I am ok with the change in this patch. There
>> are two comments.
>>
>> 1. Please add more background information about the
>> specific device in the commit log. For example, you
>> mentioned " like some virtual CD-ROM from BMC " in
>> previous patch.
>> 2. Please make sure building pass with the patch on
>> different tool chains. I tried to build with the patch
>> on VS2015, but met building failure like below.
>> warning C4244: '=': conversion from 'UINT32' to
>> 'UINT16', possible loss of data
>>
>>
>> Ray,
>>
>> Do you have other concern?
>>
>>
>> Thanks,
>> Star
>> -----Original Message-----
>> From: Ming Huang [mailto:ming.huang@linaro.org]
>> Sent: Thursday, March 15, 2018 8:30 PM
>> To: leif.lindholm@linaro.org; linaro-
>> uefi@lists.linaro.org; edk2-devel@lists.01.org; Zeng,
>> Star <star.zeng@intel.com>; Dong, Eric
>> <eric.dong@intel.com>
>> Cc: ard.biesheuvel@linaro.org; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Gao, Liming
>> <liming.gao@intel.com>; guoheyi@huawei.com;
>> wanghuiqiang@huawei.com; huangming23@huawei.com;
>> zhangjinsong2@huawei.com; mengfanrong@huawei.com;
>> huangdaode@hisilicon.com; waip23@126.com; Ming Huang
>> <ming.huang@linaro.org>
>> Subject: [edk2 UsbMassStorageDxe v1 1/1]
>> MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS
>>
>> Booting from USB may fail while the macro
>> USB_BOOT_IO_BLOCKS set to 128 because the block size of
>> some USB devices are exceeded 512. So,the count blocks
>> to transfer should be calculated by block size of the
>> USB devices.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c |
>> 16 ++++++++++++----
>> MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h |
>> 4 ++--
>>  2 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git
>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
>> index b84bfd2d7290..b38cb6116bf4 100644
>> ---
>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
>> +++
>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
>> @@ -814,11 +814,13 @@ UsbBootReadBlocks (
>>    USB_BOOT_READ10_CMD       ReadCmd;
>>    EFI_STATUS                Status;
>>    UINT16                    Count;
>> +  UINT16                    CountMax;
>>    UINT32                    BlockSize;
>>    UINT32                    ByteSize;
>>    UINT32                    Timeout;
>>
>>    BlockSize = UsbMass->BlockIoMedia.BlockSize;
>> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>>    Status    = EFI_SUCCESS;
>>
>>    while (TotalBlock > 0) {
>> @@ -827,7 +829,7 @@ UsbBootReadBlocks (
>>      // on the device. We must split the total block
>> because the READ10
>>      // command only has 16 bit transfer length (in the
>> unit of block).
>>      //
>> -    Count     = (UINT16)((TotalBlock <
>> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
>> +    Count     = (UINT16)((TotalBlock < CountMax) ?
>> TotalBlock : CountMax);
>>      ByteSize  = (UINT32)Count * BlockSize;
>>
>>      //
>> @@ -890,11 +892,13 @@ UsbBootWriteBlocks (
>>    USB_BOOT_WRITE10_CMD  WriteCmd;
>>    EFI_STATUS            Status;
>>    UINT16                Count;
>> +  UINT16                CountMax;
>>    UINT32                BlockSize;
>>    UINT32                ByteSize;
>>    UINT32                Timeout;
>>
>>    BlockSize = UsbMass->BlockIoMedia.BlockSize;
>> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>>    Status    = EFI_SUCCESS;
>>
>>    while (TotalBlock > 0) {
>> @@ -903,7 +907,7 @@ UsbBootWriteBlocks (
>>      // on the device. We must split the total block
>> because the WRITE10
>>      // command only has 16 bit transfer length (in the
>> unit of block).
>>      //
>> -    Count     = (UINT16)((TotalBlock <
>> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
>> +    Count     = (UINT16)((TotalBlock < CountMax) ?
>> TotalBlock : CountMax);
>>      ByteSize  = (UINT32)Count * BlockSize;
>>
>>      //
>> @@ -966,18 +970,20 @@ UsbBootReadBlocks16 (
>>    UINT8                     ReadCmd[16];
>>    EFI_STATUS                Status;
>>    UINT16                    Count;
>> +  UINT16                    CountMax;
>>    UINT32                    BlockSize;
>>    UINT32                    ByteSize;
>>    UINT32                    Timeout;
>>
>>    BlockSize = UsbMass->BlockIoMedia.BlockSize;
>> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>>    Status    = EFI_SUCCESS;
>>
>>    while (TotalBlock > 0) {
>>      //
>>      // Split the total blocks into smaller pieces.
>>      //
>> -    Count     = (UINT16)((TotalBlock <
>> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
>> +    Count     = (UINT16)((TotalBlock < CountMax) ?
>> TotalBlock : CountMax);
>>      ByteSize  = (UINT32)Count * BlockSize;
>>
>>      //
>> @@ -1040,18 +1046,20 @@ UsbBootWriteBlocks16 (
>>    UINT8                 WriteCmd[16];
>>    EFI_STATUS            Status;
>>    UINT16                Count;
>> +  UINT16                CountMax;
>>    UINT32                BlockSize;
>>    UINT32                ByteSize;
>>    UINT32                Timeout;
>>
>>    BlockSize = UsbMass->BlockIoMedia.BlockSize;
>> +  CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize;
>>    Status    = EFI_SUCCESS;
>>
>>    while (TotalBlock > 0) {
>>      //
>>      // Split the total blocks into smaller pieces.
>>      //
>> -    Count     = (UINT16)((TotalBlock <
>> USB_BOOT_IO_BLOCKS) ? TotalBlock : USB_BOOT_IO_BLOCKS);
>> +    Count     = (UINT16)((TotalBlock < CountMax) ?
>> TotalBlock : CountMax);
>>      ByteSize  = (UINT32)Count * BlockSize;
>>
>>      //
>> diff --git
>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
>> index 13a926035ceb..fc5449bde21e 100644
>> ---
>> a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
>> +++
>> b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
>> @@ -65,9 +65,9 @@ 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 64KB.
>>  //
>> -#define USB_BOOT_IO_BLOCKS              128
>> +#define USB_BOOT_MAX_CARRY_SIZE         0x10000
>>
>>  //
>>  // Retry mass command times, set by experience
>> --
>> 1.9.1
>



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-03-22  3:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-15 12:29 [edk2 UsbMassStorageDxe v1 0/1] Calculating the count of blocks to transfer Ming Huang
2018-03-15 12:29 ` [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS Ming Huang
2018-03-21 12:40   ` Zeng, Star
2018-03-21 14:22     ` Kinney, Michael D
2018-03-22  3:31       ` Ming Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox