From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.groups.io with SMTP id smtpd.web12.7744.1570786909175713431 for ; Fri, 11 Oct 2019 02:41:49 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CEF5485538; Fri, 11 Oct 2019 09:41:48 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-177.rdu2.redhat.com [10.10.120.177]) by smtp.corp.redhat.com (Postfix) with ESMTP id CC4226092F; Fri, 11 Oct 2019 09:41:47 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices event To: "Fu, Siyuan" , "devel@edk2.groups.io" , "Rabeda, Maciej" Cc: "Wu, Jiaxin" References: <20191008161629.14668-1-maciej.rabeda@intel.com> <20191008161629.14668-2-maciej.rabeda@intel.com> <26e6a08a-0fcf-f903-8411-f47bd56991cb@redhat.com> From: "Laszlo Ersek" Message-ID: Date: Fri, 11 Oct 2019 11:41:46 +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: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 11 Oct 2019 09:41:48 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 10/11/19 02:14, Fu, Siyuan wrote: >> -----Original Message----- >> From: Laszlo Ersek >> Sent: 2019=E5=B9=B410=E6=9C=8811=E6=97=A5 0:06 >> To: Fu, Siyuan ; devel@edk2.groups.io; Rabeda, >> Maciej >> Cc: Wu, Jiaxin >> 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 >>>> Sent: 2019=E5=B9=B410=E6=9C=8810=E6=97=A5 16:06 >>>> To: Fu, Siyuan ; devel@edk2.groups.io; Rabeda, >>>> Maciej >>>> Cc: Wu, Jiaxin >>>> 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 ena= ble 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, a= t >> least in platforms that utilize the NetworkPkg *.inc files. And the PC= D >> 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. >=20 > It's not an existing "consistency" in current network package. The curr= ent 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=20 > PcdIScsiAIPNetworkBootPolicy is consumed by IScsiDriver, but not contro= lled 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