From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5016F209574EF for ; Tue, 27 Feb 2018 01:55:52 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Feb 2018 02:01:58 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.47,400,1515484800"; d="scan'208";a="33943210" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.13]) ([10.239.9.13]) by fmsmga001.fm.intel.com with ESMTP; 27 Feb 2018 02:01:56 -0800 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" References: <1519464575-38109-1-git-send-email-ming.huang@linaro.org> <1519464575-38109-2-git-send-email-ming.huang@linaro.org> <734D49CCEBEEF84792F5B80ED585239D5BBC26C6@SHSMSX104.ccr.corp.intel.com> From: "Ni, Ruiyu" Message-ID: Date: Tue, 27 Feb 2018 18:01:55 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [RFC v1 1/1] MdeModulePkg/Usb: Use Pcd for UsbBootIoBlocks 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: Tue, 27 Feb 2018 09:55:53 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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 >>> ; Dong, Eric >>> Cc: huangming23@huawei.com; ard.biesheuvel@linaro.org; Ming Huang >>> ; Gao, Liming ; >>> mengfanrong@huawei.com; guoheyi@huawei.com; >>> zhangjinsong2@huawei.com; Kinney, Michael D >>> ; 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 >>> --- >>> 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 >>> + >>> // >>> // 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