public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Liming Gao" <liming.gao@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"lersek@redhat.com" <lersek@redhat.com>,
	"Wu, Jiaxin" <jiaxin.wu@intel.com>
Subject: Re: [edk2-devel] [Patch v4 3/3] NetworkPkg: Add package level include DSC file
Date: Mon, 6 May 2019 14:30:51 +0000	[thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E441F36@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <0df78b22-af7b-d878-7ee5-35a54594b88b@redhat.com>

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
> Sent: Monday, May 6, 2019 10:28 PM
> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: Re: [edk2-devel] [Patch v4 3/3] NetworkPkg: Add package level include DSC file
> 
> On 05/06/19 15:24, Liming Gao wrote:
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Monday, May 6, 2019 7:54 PM
> >> To: Gao, Liming <liming.gao@intel.com>; devel@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@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
> >>>> Sent: Tuesday, April 30, 2019 10:42 PM
> >>>> To: devel@edk2.groups.io; Gao, Liming <liming.gao@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@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.
> 
Yes. After confirm with Jiaxin or Siyuan, I will send v5 version path. Thanks for your comments. 

> Either way, for this patch:
> 
> Acked-by: Laszlo Ersek <lersek@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
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>
> >
> >
> >
> >
> 
> 
> 


      reply	other threads:[~2019-05-06 14:30 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-29 14:50 [Patch v4 0/3] Add package level include DSC/FDF in NetworkPkg Liming Gao
2019-04-29 14:50 ` [Patch v4 1/3] NetworkPkg DSC: Add the required ARM library to pass ARM build Liming Gao
2019-04-30 13:40   ` [edk2-devel] " Laszlo Ersek
2019-04-29 14:50 ` [Patch v4 2/3] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg Liming Gao
2019-04-30 14:35   ` [edk2-devel] " Laszlo Ersek
2019-04-29 14:50 ` [Patch v4 3/3] NetworkPkg: Add package level include DSC file Liming Gao
2019-04-30 14:42   ` [edk2-devel] " Laszlo Ersek
2019-05-05 14:53     ` Liming Gao
2019-05-06 11:53       ` Laszlo Ersek
2019-05-06 13:24         ` Liming Gao
2019-05-06 14:28           ` Laszlo Ersek
2019-05-06 14:30             ` Liming Gao [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A89E2EF3DFEDB4C8BFDE51014F606A14E441F36@SHSMSX104.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox