Re: [Patch 4/5] MdeModulePkg/MdeModulePkg.dec: Define one PCD for PXE to specify MTFTP windowsize.

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

> Subject: Re: [edk2] [Patch 4/5] MdeModulePkg/MdeModulePkg.dec: Define
> one PCD for PXE to specify MTFTP windowsize.
> 
> 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 <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Shao Ming <ming.shao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > ---
> >  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|0x30001
> 026
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdTftpWindowSize|0x4|UINT64|0x30
> 00102A
> >
> >    ## 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.
> 

Agree. I will fix that in the version patch.

> 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".)
> 

Agree, I will move the new PCD into the networkpkg.

> ... 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"?
> 

Yes, we only add the new feature into the combo driver. 


> Thanks,
> Laszlo