From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 BE38B21140806 for ; Tue, 18 Sep 2018 04:04:31 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E820783F42; Tue, 18 Sep 2018 11:04:30 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-209.rdu2.redhat.com [10.10.120.209]) by smtp.corp.redhat.com (Postfix) with ESMTP id 851F530912F5; Tue, 18 Sep 2018 11:04:29 +0000 (UTC) To: Jiaxin Wu , edk2-devel@lists.01.org Cc: Ye Ting , Shao Ming , Fu Siyuan References: <20180917054348.19228-1-Jiaxin.wu@intel.com> <20180917054348.19228-5-Jiaxin.wu@intel.com> From: Laszlo Ersek Message-ID: <9705cd37-9ce5-ee6d-1357-3b1dbd73670e@redhat.com> Date: Tue, 18 Sep 2018 13:04:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180917054348.19228-5-Jiaxin.wu@intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 18 Sep 2018 11:04:31 +0000 (UTC) Subject: Re: [Patch 4/5] MdeModulePkg/MdeModulePkg.dec: Define one PCD for PXE to specify MTFTP windowsize. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Sep 2018 11:04:31 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/17/18 07:43, Jiaxin Wu wrote: > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=886 > > This patch is to define one new PCD for PXE driver to specify MTFTP windowsize so as > to improve the PXE download performance. The default value is set to 4. > > Cc: Ye Ting > Cc: Fu Siyuan > Cc: Shao Ming > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Wu Jiaxin > --- > MdeModulePkg/MdeModulePkg.dec | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec > index 74a699cbb7..bfc63e5fcb 100644 > --- a/MdeModulePkg/MdeModulePkg.dec > +++ b/MdeModulePkg/MdeModulePkg.dec > @@ -1203,10 +1203,11 @@ > ## This setting can override the default TFTP block size. A value of 0 computes > # the default from MTU information. A non-zero value will be used as block size > # in bytes. > # @Prompt TFTP block size. > gEfiMdeModulePkgTokenSpaceGuid.PcdTftpBlockSize|0x0|UINT64|0x30001026 > + gEfiMdeModulePkgTokenSpaceGuid.PcdTftpWindowSize|0x4|UINT64|0x3000102A > > ## Maximum address that the DXE Core will allocate the EFI_SYSTEM_TABLE_POINTER > # structure. The default value for this PCD is 0, which means that the DXE Core > # will allocate the buffer from the EFI_SYSTEM_TABLE_POINTER structure on a 4MB > # boundary as close to the top of memory as feasible. If this PCD is set to a > The new PCD is missing documentation -- and not just in the DEC file, but also in the corresponding UNI file. Furthermore, given that the PCD is only used in the next patch (which is for NetworkPkg), the PCD should arguably be introduced to NetworkPkg. ("PcdTftpBlockSize" is different, that one is used in both "MdeModulePkg/Universal/Network/UefiPxeBcDxe/" and "NetworkPkg/UefiPxeBcDxe".) ... In fact, that brings me to my next question -- patch #5 only modifies "NetworkPkg/UefiPxeBcDxe"; it doesn't modify "MdeModulePkg/Universal/Network/UefiPxeBcDxe". The former module is usually included as a replacement for the latter, when the platform build enables IPv6. Is this intentional? I.e., is it the intent to *not* bring this feature to "MdeModulePkg/Universal/Network/UefiPxeBcDxe"? Thanks, Laszlo