From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:400e:c00::236; helo=mail-pf0-x236.google.com; envelope-from=ming.huang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pf0-x236.google.com (mail-pf0-x236.google.com [IPv6:2607:f8b0:400e:c00::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5140E22146736 for ; Wed, 21 Mar 2018 20:25:05 -0700 (PDT) Received: by mail-pf0-x236.google.com with SMTP id y186so2855977pfb.2 for ; Wed, 21 Mar 2018 20:31:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=MXD+rWJYW7TKE8t2FYoveVhHFy1VFCyyys3qm4vaQl4=; b=jHqLwx3TqWCQUjdwEC6L7+2W3wxB553P2m2YRS2Q2ofijczUV5IEfUjy7QWzHaeDUV OkRgllSK9xw1MSe+XLJkr7MBobjIRTD+mH696WQdrrXnTIWKa6kEsv5RlcULReYhl7AO DAqRFFTiCGrwoytyHwHRhUgAyNqvLx3wkZ3v4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=MXD+rWJYW7TKE8t2FYoveVhHFy1VFCyyys3qm4vaQl4=; b=emDp23ZhzuG52uqj5lc5kZyRlSMa6WdUjATmphoiy9BwFo+4hc6wyHI0occJ90kwwN z/O8sdLrYZHKVZiJqBNeeZyDfpqL3H2zJwkg2fLxd4gnHGMb7HkUu1EPCUzhPGmwwK9Z U7bylBvYs9UWnJHOabOe7vPMyJS6S47U1RM4+p2OD1L95k8+y4XASGnXOReUP8mpo/w+ k3my0GWu28gJj8sfGLQ5YNWjpgXY0TPSRroGkChsFNWYeROvbROaSv0rOOkd1nO9f/1e rzD3jaACOKdLZF1nraOGDO3ytb5aQr+Todj/Q+jLyXGUjAKGwCvkwRW0iXwxUq2agkw5 GJVA== X-Gm-Message-State: AElRT7EqTpaIXMg4yYY4Opxiudv8TEj5qoobxAEHhqtli/XmQDpjSIbD 3ZS8caqjDpBEk2YgOUqPHTOPcQ== X-Google-Smtp-Source: AG47ELuLvRSnhQWuzX0HYrc+z1la8eq3L3zH80CXrXPPvls6DDQPLLvqkpzz2VkHkpJCcOw02jtghg== X-Received: by 10.101.92.76 with SMTP id v12mr16617437pgr.50.1521689496763; Wed, 21 Mar 2018 20:31:36 -0700 (PDT) Received: from [10.196.0.18] ([45.56.155.60]) by smtp.gmail.com with ESMTPSA id b64sm11598408pfl.148.2018.03.21.20.31.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 21 Mar 2018 20:31:35 -0700 (PDT) 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" References: <1521116980-120594-1-git-send-email-ming.huang@linaro.org> <1521116980-120594-2-git-send-email-ming.huang@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103BA71F3C@shsmsx102.ccr.corp.intel.com> From: Ming Huang Message-ID: Date: Thu, 22 Mar 2018 11:31:20 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [edk2 UsbMassStorageDxe v1 1/1] MdeModulePkg/Usb: Replace macro USB_BOOT_IO_BLOCKS X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Mar 2018 03:25:06 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US 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 ; 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 >> 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 ; 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 Huang >> >> 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 >> --- >> 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 >