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...@intel.com>
> Cc: Fu Siyuan <siyuan...@intel.com>
> Cc: Shao Ming <ming.s...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin...@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|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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to