On 10/11/19 02:14, Fu, Siyuan wrote: >> -----Original Message----- >> From: Laszlo Ersek <ler...@redhat.com> >> Sent: 2019年10月11日 0:06 >> To: Fu, Siyuan <siyuan...@intel.com>; devel@edk2.groups.io; Rabeda, >> Maciej <maciej.rab...@intel.com> >> Cc: Wu, Jiaxin <jiaxin...@intel.com> >> Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove >> ExitBootServices event >> >> On 10/10/19 11:29, Fu, Siyuan wrote: >>>> -----Original Message----- >>>> From: Laszlo Ersek <ler...@redhat.com> >>>> Sent: 2019年10月10日 16:06 >>>> To: Fu, Siyuan <siyuan...@intel.com>; devel@edk2.groups.io; Rabeda, >>>> Maciej <maciej.rab...@intel.com> >>>> Cc: Wu, Jiaxin <jiaxin...@intel.com> >>>> Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove >>>> ExitBootServices event >>>> >>>> On 10/10/19 05:32, Fu, Siyuan wrote: >>>>> Hi, Maciej >>>>> >>>>> Considering that this patch has to co-work with corresponding UNDI >> device >>>> driver >>>>> bug fix, in order to avoid potential compatibility problem, please add a >> PCD >>>> to >>>>> NetworkPkg for this fix, and set the default value to disable state (no >>>> behavior >>>>> change). The platform which need this fix could set the PCD to enable in >>>> its >>>>> platform DSC. >>>> >>>> Should the new PCD go into "NetworkPkg/NetworkPcds.dsc.inc", and be >>>> gated with a new build flag defined in >> "NetworkPkg/NetworkDefines.dsc.inc"? >>> >>> Currently not all the network package PCDs are listed in the >> NetworkPcds.dsc.inc, >>> But I do not oppose it if you think this PCD should be controlled by a build >> flag. >> >> I mainly asked from a consistency point of view. >> >> The inclusion of SnpDxe is already controlled by NETWORK_SNP_ENABLE, at >> least in platforms that utilize the NetworkPkg *.inc files. And the PCD >> in question would control the behavior of SnpDxe at ExitBootServices(), >> if I understand correctly. >> >> This is similar to NETWORK_HTTP_BOOT_ENABLE and >> NETWORK_ALLOW_HTTP_CONNECTIONS: >> - The former includes HttpDxe and HttpBootDxe (and some other drivers). >> - The latter controls PcdAllowHttpConnections, which is consumed by >> HttpDxe and HttpBootDxe. >> >> I don't feel too strongly about it, I just thought it worth raising. > > It's not an existing "consistency" in current network package. The current > macros > are mostly used for control more high level feature enable/disable, not such > kind > of bug fix, > For example, NETWORK_ISCSI_ENABLE includes IScsiDxe driver, while > PcdIScsiAIPNetworkBootPolicy is consumed by IScsiDriver, but not controlled by > a macro. And also other PXE, DHCP and PXE related PCDs.
Good point! So, I'm fine if the new feature PCD is not exposed with a build flag. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48809): https://edk2.groups.io/g/devel/message/48809 Mute This Topic: https://groups.io/mt/34444145/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
