From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::22b; helo=mail-it0-x22b.google.com; envelope-from=ming.huang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::22b]) (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 B2F0822283519 for ; Sun, 11 Mar 2018 22:55:24 -0700 (PDT) Received: by mail-it0-x22b.google.com with SMTP id j7-v6so9886985ita.3 for ; Sun, 11 Mar 2018 23:01:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:from:to:cc:references:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=p369CGCVymWNv+MlhSBpVM5HsLuB5zi3IFbhvq2DyIo=; b=hvbmdHdqwKZN0gIWNJ+ghDKKsvMYzGE3zcbqOkMW2czmOOVOikQ/IR9WYMektRCWs4 mlgKBbR0EYKbeeRFHCMxA1jKGGqdFdJl8D6D1RDtVm3772LVrDgaWS8nhd1o0CjOqv2e l4ScPxl0HKsE/mYk2NiVAsEykeHB/9BnVdVgQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=p369CGCVymWNv+MlhSBpVM5HsLuB5zi3IFbhvq2DyIo=; b=sQvdpdPIP5wQfm4vmjPWnAvZSAABc2Q5K8c5zwbs1twK8nk4WM/3JegNF54Xn437KB fIAqZ9t0wDx2nBsnV45kHrWqgRTEM9weqZI4q5nDgw5hY2K6ZjED09Su6ESbCHCgD8Xi 6Sk20n6Kg+kQZtz75/mxcRqNEC2gh4WqKS87mgawH+VRIHRPx2u02SsVZOKVnrRK3PKf cpXwlZp9D1CIEWqhLYyAHN1mOYPWYMoQimpt5iiKpkjYDGhQ3WnWTcaFsWYB2masHuIQ 5UqDJqMq0d9mDzNudu1HVpFMxfmpdQt/389Pn8UNLKylMtCgbJ6HQQmRe6RZUAXk/XNo UC+w== X-Gm-Message-State: AElRT7G80y98U25JjZ75r/BVuiV92Uenjmy1pHZy5HomBbVCoZEwzV1Y ZmqvJ8+z4cIqCV6kGmjuY1JrHw== X-Google-Smtp-Source: AG47ELur6ih6ZScuJrFCh41S2745eQAROTksnHiTRDUpyL9zjGXZ9PNRDQA7LS8HI2uH8GVrHji0Rw== X-Received: by 10.36.134.2 with SMTP id u2mr6823686itd.28.1520834503922; Sun, 11 Mar 2018 23:01:43 -0700 (PDT) Received: from [10.125.1.254] ([104.237.86.199]) by smtp.gmail.com with ESMTPSA id l123sm2736116itl.43.2018.03.11.23.01.35 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 11 Mar 2018 23:01:43 -0700 (PDT) From: Ming Huang 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" 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> <8fb46727-76ea-6a86-d936-ea3cec9df3d8@linaro.org> Message-ID: <3d24a3f3-a704-b4bd-4d7d-6f1fdc46f78b@linaro.org> Date: Mon, 12 Mar 2018 14:01:24 +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: <8fb46727-76ea-6a86-d936-ea3cec9df3d8@linaro.org> 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: Mon, 12 Mar 2018 05:55:25 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US 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 >>>>> ; 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 >>> >>