[edk2-devel] [Patch v4 3/3] NetworkPkg: Add package level include DSC file
Laszlo Ersek
lersek at redhat.com
Mon May 6 14:28:17 UTC 2019
On 05/06/19 15:24, Liming Gao wrote:
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek at redhat.com]
>> Sent: Monday, May 6, 2019 7:54 PM
>> To: Gao, Liming <liming.gao at intel.com>; devel at edk2.groups.io
>> Subject: Re: [edk2-devel] [Patch v4 3/3] NetworkPkg: Add package level include DSC file
>>
>> On 05/05/19 16:53, Gao, Liming wrote:
>>>> -----Original Message-----
>>>> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of Laszlo Ersek
>>>> Sent: Tuesday, April 30, 2019 10:42 PM
>>>> To: devel at edk2.groups.io; Gao, Liming <liming.gao at intel.com>
>>>> Subject: Re: [edk2-devel] [Patch v4 3/3] NetworkPkg: Add package level include DSC file
>>>>
>>>> Hi Liming,
>>>>
>>>> please read my email until my signature below:
>>>>
>>>> On 04/29/19 16:50, Liming Gao wrote:
>>>>> Platform DSC can include Network.dsc.inc to enable network features.
>>>>>
>>>>> Signed-off-by: Liming Gao <liming.gao at intel.com>
>>>>> ---
>>>>> NetworkPkg/Network.dsc.inc | 40 ++++++++++++++++++++++++++++++++++++++++
>>>>> NetworkPkg/NetworkPkg.dsc | 22 +---------------------
>>>>> 2 files changed, 41 insertions(+), 21 deletions(-)
>>>>> create mode 100644 NetworkPkg/Network.dsc.inc
>>>>>
>>>>> diff --git a/NetworkPkg/Network.dsc.inc b/NetworkPkg/Network.dsc.inc
>>>>> new file mode 100644
>>>>> index 0000000000..d51ee1b040
>>>>> --- /dev/null
>>>>> +++ b/NetworkPkg/Network.dsc.inc
>>>>> @@ -0,0 +1,40 @@
>>>>> +## @file
>>>>> +# Network DSC include file for Platform DSC
>>>>> +#
>>>>> +# This file includes all required information to enable Network features.
>>>>> +# It can be included to a platform DSC file by using "!include NetworkPkg/Network.dsc.inc".
>>>>> +#
>>>>> +# This file defines one build flag PLATFORMX64_ENABLE to support
>>>>> +# IA32 PEI and X64 DXE platform. Its default value is FALSE.
>>>>> +#
>>>>> +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>>>>> +#
>>>>> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>>>>> +#
>>>>> +##
>>>>> +
>>>>> +[Defines]
>>>>> +!include NetworkPkg/NetworkDefines.dsc.inc
>>>>> +
>>>>> +!ifndef PLATFORMX64_ENABLE
>>>>> + #
>>>>> + # PLATFORMX64_ENABLE is set to TRUE when PEI is IA32 and DXE is X64 platform
>>>>> + #
>>>>> + DEFINE PLATFORMX64_ENABLE = FALSE
>>>>> +!endif
>>>>> +
>>>>> +[PcdsFixedAtBuild]
>>>>> +!include NetworkPkg/NetworkPcds.dsc.inc
>>>>> +
>>>>> +[LibraryClasses]
>>>>> +!include NetworkPkg/NetworkLibs.dsc.inc
>>>>> +
>>>>> +!if $(PLATFORMX64_ENABLE) == TRUE
>>>>> +[Components.X64]
>>>>> +!include NetworkPkg/NetworkComponents.dsc.inc
>>>>> +
>>>>> +!else
>>>>> +[Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
>>>>> +!include NetworkPkg/NetworkComponents.dsc.inc
>>>>> +
>>>>> +!endif
>>>>
>>>> My point (9) in <https://edk2.groups.io/g/devel/message/39770> was
>>>> PLATFORMX64_ENABLE should be dropped completely, and we should use the
>>>> following instead:
>>>>
>>>> !if ("X64" in $(ARCH))
>>>> [Components.X64]
>>>> !include NetworkPkg/NetworkComponents.dsc.inc
>>>> !else
>>>> [Components.IA32, Components.ARM, Components.AARCH64]
>>>> !include NetworkPkg/NetworkComponents.dsc.inc
>>>> !endif
>>>>
>>>> I don't feel strongly about this patch, but I'd still like to hear your
>>>> opinion on this approach.
>>> This solution can't handle NetworkPkg.dsc. NetworkPkg.dsc includes IA32, X64, ARM and AARCH64.
>>> With this solution, NetworkPkg.dsc will only build drivers for X64 arch.
>>
>> How so? If you run "build -a ARM -p NetworkPkg/NetworkPkg.dsc ...", then
>> the second branch will be selected (the "in" operator will evaluate to
>> false), and the second branch !includes "NetworkComponents.dsc.inc" in
>> [Components.ARM].
>>
>> Can you provide an example command line where the above would break?
>>
>> Do you mean "build -a ARM -a X64 -p NetworkPkg/NetworkPkg.dsc ..."?
>
> Yes. I usually build -p NetworkPkg/NetworkPkg.dsc -a IA32 -a X64 and make sure the modules are built both for IA32 and X64.
OK, that certainly makes sense. A multi-arch build such as "-a IA32 -a
X64" may have a different meaning dependent on the presence of an FDF
file reference in the DSC file. I agree a separate flag is useful for
distinguishing IA32 PEI + X64 DXE special case from the rest of the
multi-arch builds.
Other than that: as I suggested under patch v3 2/3, point (11), I think
we should include TlsLib in "NetworkLibs.dsc.inc", and then we can
remove the TlsLib resolution too, from "NetworkPkg.dsc". I guess we
should await Jiaxin's and Siyuan's feedback on that.
Either way, for this patch:
Acked-by: Laszlo Ersek <lersek at redhat.com>
Thanks!
Laszlo
>>
>> Thanks
>> Laszlo
>>
>>> But, we expect NetworkPkg.dsc
>>> build the drivers for each ARCH.
>>>>
>>>> Thanks,
>>>> Laszlo
>>>>
>>>>> diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc
>>>>> index be9d9248e4..fe2fcf7b3c 100644
>>>>> --- a/NetworkPkg/NetworkPkg.dsc
>>>>> +++ b/NetworkPkg/NetworkPkg.dsc
>>>>> @@ -41,12 +41,6 @@
>>>>> DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
>>>>> SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
>>>>>
>>>>> - DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
>>>>> - NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
>>>>> - IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
>>>>> - UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
>>>>> - TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
>>>>> - HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
>>>>> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>>>>> OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>>>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>>>>> @@ -103,24 +97,10 @@
>>>>> ###################################################################################################
>>>>>
>>>>> [Components]
>>>>> - NetworkPkg/Ip6Dxe/Ip6Dxe.inf
>>>>> - NetworkPkg/TcpDxe/TcpDxe.inf
>>>>> - NetworkPkg/Udp6Dxe/Udp6Dxe.inf
>>>>> - NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
>>>>> - NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
>>>>> - NetworkPkg/DnsDxe/DnsDxe.inf
>>>>> - NetworkPkg/HttpDxe/HttpDxe.inf
>>>>> - NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
>>>>> - NetworkPkg/HttpBootDxe/HttpBootDxe.inf
>>>>> NetworkPkg/WifiConnectionManagerDxe/WifiConnectionManagerDxe.inf
>>>>> -
>>>>> NetworkPkg/Application/VConfig/VConfig.inf
>>>>>
>>>>> -[Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
>>>>> - NetworkPkg/IScsiDxe/IScsiDxe.inf
>>>>> - NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
>>>>> - NetworkPkg/TlsDxe/TlsDxe.inf
>>>>> - NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
>>>>> + !include NetworkPkg/Network.dsc.inc
>>>>>
>>>>> [BuildOptions]
>>>>> *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
>>>>>
>>>>
>>>>
>>>>
>>>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40042): https://edk2.groups.io/g/devel/message/40042
Mute This Topic: https://groups.io/mt/31384512/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list