From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web08.307.1616116878330225621 for ; Thu, 18 Mar 2021 18:21:20 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Fri, 19 Mar 2021 09:20:39 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , , , "'Bi, Dandan'" , , "'Andrew Fish'" Cc: "'Leif Lindholm'" , "'Kinney, Michael D'" References: <20210316145428.35616-1-dandan.bi@intel.com> <00c601d71ada$42fdc900$c8f95b00$@byosoft.com.cn> <7b41b3f7-b5dd-7fc6-3d75-32d9367b2902@redhat.com> <007301d71bb1$464e58e0$d2eb0aa0$@byosoft.com.cn> <6094a2fc-86f9-2d11-b322-80279aa5ed6e@redhat.com> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiDlm57lpI06IFtlZGsyLWRldmVsXSBbZWRrMi1wbGF0Zm9ybXNdIFtwYXRjaCAwMC8zNV0gQ29uc3VtZSBSZWdpc3RlckZpbHRlckxpYk51bGwgaW5zdGFuY2U=?= Date: Fri, 19 Mar 2021 09:20:41 +0800 Message-ID: <009601d71c5e$140626d0$3c127470$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQItdfl10XSTJBOTUudN/bhmcVVhogFKI7HDAjOjCp4CFQnDZgJV4WZkArEaPQEA0421pQJ5D4A2qW6amHA= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Ray: For now, this one MdeLibs.dsc.inc is introduced to mainly resolve the fu= ture incompatible change.=20 For long term, this one MdeLibs.dsc.inc includes the default library ins= tance from MdePkg. The platform DSC can still specify its library instance = after include MdeLibs.dsc.inc.=20 Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io = =E4=BB=A3=E8=A1=A8 Ni, Ray > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B43=E6=9C=8818=E6=97=A5= 21:42 > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; lersek@redhat.com; ga= oliming > ; Bi, Dandan ; > ardb@kernel.org; 'Andrew Fish' > =E6=8A=84=E9=80=81: 'Leif Lindholm' ; Kinney, Michael= D > > =E4=B8=BB=E9=A2=98: Re: =E5=9B=9E=E5=A4=8D: [edk2-devel] [edk2-platforms= ] [patch 00/35] Consume > RegisterFilterLibNull instance >=20 > I know that NetworkPkg already provided a case of DSC header file. > But NetworkPkg provides drivers and the dependent libs. Multiple instanc= es > of a single lib class are not common in NetworkPkg. So, for NetworkPkg, = the > consumer can simplify include the DSC header files. >=20 >=20 > But for MdePkg, there are so many lib instances for a single lib class. = I am not > sure how we create the DSC header file (which lib instance should be cho= sen). >=20 > Maybe having a DSC header file can avoid changing lots of code in each > platform DSC *today*. But imagine that we look at the platform DSC file = after > couple years/months, we might be more frustrated about which lib instanc= es > are used in the platform. >=20 > Still, I am open to see what the final MdePkg DSC header will be like. >=20 > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Laszlo > Ersek > > Sent: Thursday, March 18, 2021 9:25 PM > > To: gaoliming ; Bi, Dandan > ; devel@edk2.groups.io; ardb@kernel.org; > > 'Andrew Fish' > > Cc: 'Leif Lindholm' ; Kinney, Michael D > > > Subject: Re: =E5=9B=9E=E5=A4=8D: [edk2-devel] [edk2-platforms] [patch = 00/35] Consume > RegisterFilterLibNull instance > > > > On 03/18/21 05:43, gaoliming wrote: > > > Laszlo and Dandan: > > > > > >> -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > >> =E5=8F=91=E4=BB=B6=E4=BA=BA: Laszlo Ersek > > >> =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B43=E6=9C=8818=E6= =97=A5 1:08 > > >> =E6=94=B6=E4=BB=B6=E4=BA=BA: Bi, Dandan ; deve= l@edk2.groups.io; > > >> gaoliming@byosoft.com.cn; ardb@kernel.org; 'Andrew Fish' > > >> > > >> =E6=8A=84=E9=80=81: 'Leif Lindholm' ; Kinney, Mi= chael D > > >> > > >> =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [edk2-platforms] [patch 00/35]= Consume > > >> RegisterFilterLibNull instance > > >> > > >> On 03/17/21 16:05, Bi, Dandan wrote: > > >>>> -----Original Message----- > > >>>> From: devel@edk2.groups.io On Behalf Of > > >>>> gaoliming > > >>>> Sent: Wednesday, March 17, 2021 11:05 AM > > >>>> To: devel@edk2.groups.io; ardb@kernel.org; Bi, Dandan > > >>>> ; 'Laszlo Ersek' ; > 'Andrew > > >> Fish' > > >>>> > > >>>> Cc: 'Leif Lindholm' ; Kinney, Michael D > > >>>> > > >>>> Subject: =E5=9B=9E=E5=A4=8D: [edk2-devel] [edk2-platforms] [patch= 00/35] Consume > > >>>> RegisterFilterLibNull instance > > >>>> > > >>>> Ard and Dandan: > > >>>> > > >>>>> -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > >>>>> =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io =E4=BB=A3=E8=A1=A8 Ard > > >>>> Biesheuvel > > >>>>> =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B43=E6=9C=8816= =E6=97=A5 23:01 > > >>>>> =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; dandan.bi@int= el.com; Laszlo Ersek > > >>>>> ; Andrew Fish > > >>>>> =E6=8A=84=E9=80=81: Leif Lindholm ; Michael D= Kinney > > >>>>> ; Liming Gao > > > >>>>> =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [edk2-platforms] [patch 00/= 35] Consume > > >>>>> RegisterFilterLibNull instance > > >>>>> > > >>>>> On Tue, 16 Mar 2021 at 15:56, Dandan Bi > wrote: > > >>>>>> > > >>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3246 > > >>>>>> RFC: https://edk2.groups.io/g/devel/message/72530 > > >>>>>> > > >>>>>> > > >>>>>> Add RegisterFilterLibNull in dsc files in edk2-platforms repo, = which > > >>>>>> will be consumed by IoLib and BaseLib. > > >>>>>> > > >>>>>> This is the following update in edk2-platforms repo for the cha= nge > > >>>>>> in edk2, which will add RegisterFilterLib dependency for IoLib = and > > >>>>>> BaseLib to > > >>>>> filter/trace > > >>>>>> port IO/MMIO/MSR access. > > >>>>>> https://edk2.groups.io/g/devel/message/72754 > > >>>>>> > > >>>>>> Cc: Leif Lindholm > > >>>>>> Cc: Michael D Kinney > > >>>>>> Cc: Liming Gao > > >>>>>> > > >>>>> > > >>>>> It is a bit disappointing that we have to update every platform = in > > >>>>> existence again to apply a change to a core module. > > >>>>> > > >>>> > > >>>> I suggest to add MdePkg.dsc.inc file to include the default libra= ry > instance, > > >>>> and update all Platform DSC to include it. Then, for the future c= hange, > no > > >>>> change is required for platform DSC. > > >>>> > > >>>> Because this patch set updates every platform DSC, I suggest to > introduce > > >>>> MdePkg.dsc.inc file in this patch set. > > >>> > > >>> Hi Liming, > > >>> > > >>> I agree that add MdePkg.dsc.inc file to include the default librar= y > instance > > >> and make it consumed by platform dsc will benefit future similar > incompatible > > >> changes. > > >>> But I wonder to know whether we could do it in a separated task/to= pic, > as > > >>> 1. It should be a code infrastructure design change/improvement i= n > edk2. > > >>> 2. Personally I don't hope the new solution will have much impact= on > my > > >> current schedule, but it seems have. > > >>> And we may need to: > > >>> 1). Clarify the default library instances which should be added in > > >> MdePkg.dsc.inc > > >>> The library instances in MdePkg.dsc.inc should be generic enou= gh > to > > >> be widely included in platform dsc files. > > >>> 2). Update dsc files in edk2 and edk2-platforms repo to include > > >> MdePkg.dsc.inc and cleanup the default Lib instance in dsc files. > > >> > > >> (1) The file name should be "MdeLibs.dsc.inc", and it should be > > >> structured similarly "NetworkLibs.dsc.inc" -- no [LibraryClasses] h= eader > > >> should be part of the file. > > >> > > > I agree. > > > > > >> (2) The introduction of "MdeLibs.dsc.inc" is a big task, in my opin= ion. > > >> > > > The full MdeLibs.dsc.inc is a big task. But, it should be a separate= task. > > > > > > For this patch set, the first version MdeLibs.dsc.inc can be added. = It only > includes RegisterFilterLibNull library instance. > > > And, this patch set will update every platform DSC to include > MdeLibs.dsc.inc. > > > The future incompatible change can update MdeLibs.dsc.inc only, and > avoid the change in each platform DSC. > > > I would like to resolve the potential incompatible change by > MdeLibs.dsc.inc on the first step. > > > > That's a good idea! > > > > Dandan is touching up a bunch of DSC files already, so we might even u= se > > this opportunity to introduce "MdeLibs.dsc.inc". And, at this time, > > "MdeLibs.dsc.inc" would be really small, and not hard to review for > > individual platforms -- in particular it wouldn't attempt to *replace* > > existent lib class resolutions. So indeed this looks like a nice > > approach to me. > > > > Thanks! > > Laszlo > > > > > > > > Thanks > > > Liming > > >> As I stated earlier, I wouldn't like to review a patch for OvmfPkg = that > > >> replaces (say) 50-100 lines of library class resolutions with a sim= ple > > >> !include directive. Such a patch is unreviewable, as I'd have no wa= y of > > >> carefully comparing the before-after state, let alone a way of *poi= nting > > >> out* (in comments) where exactly a problem was. > > >> > > >> So I think such an include file would require a patch set, and the = lib > > >> class resolutions should be migrated in small *topical* steps. Such= as: > > >> > > >> - introduce the DSC include file as empty > > >> - add the !include directive to platform DSCs > > >> - add a small set of libraries (with some topical coherence) to the > > >> include file, > > >> - remove the same set of resolutions from platform DSCs, > > >> - repeat the last two steps until all "topics" have been covered. > > >> > > >> Thanks > > >> Laszlo > > >> > > >> > > >>> > > >>> > > >>> Thanks, > > >>> Dandan > > >>>> > > >>>> Thanks > > >>>> Liming > > >>>>> Is there really not a better way to provide a 'default' resoluti= on for > > >>>>> a library class? Maybe a change to the .DEC format, so that the = file > > >>>>> which defines the library class can provide a resolution that is= used > > >>>>> if none is provided by the .DSC file? > > >>>>> > > >>>>> > > >>>>> > > >>>>>> Dandan Bi (35): > > >>>>>> Drivers/ASIX: Consume RegisterFilterLibNull instance > > >>>>>> Drivers/DisplayLink: Consume RegisterFilterLibNull instance > > >>>>>> Drivers/OptionRomPkg: Consume RegisterFilterLibNull instance > > >>>>>> Features/Debugging: Consume RegisterFilterLibNull instance > > >>>>>> Features/Network: Consume RegisterFilterLibNull instance > > >>>>>> Features/OutOfBandManagement: Consume > RegisterFilterLibNull > > >>>>> instance > > >>>>>> Features/PowerManagement: Consume RegisterFilterLibNull > > >> instance > > >>>>>> Features/SystemInformation: Consume RegisterFilterLibNull > instance > > >>>>>> Features/UserInterface: Consume RegisterFilterLibNull instanc= e > > >>>>>> Platform/AMD: Consume RegisterFilterLibNull instance > > >>>>>> Platform/ARM: Consume RegisterFilterLibNull instance > > >>>>>> Platform/BeagleBoard: Consume RegisterFilterLibNull instance > > >>>>>> Platform/BoardModulePkg: Consume RegisterFilterLibNull > instance > > >>>>>> Platform/MinPlatformPkg: Consume RegisterFilterLibNull > instance > > >>>>>> Platform/QuarkPlatformPkg: Consume RegisterFilterLibNull > instance > > >>>>>> Platform/Vlv2TbltDevicePkg: Consume RegisterFilterLibNull > instance > > >>>>>> Platform/LeMaker: Consume RegisterFilterLibNull instance > > >>>>>> Platform/Qemu: Consume RegisterFilterLibNull instance > > >>>>>> Platform/RaspberryPi: Consume RegisterFilterLibNull instance > > >>>>>> Platform/RISC-V: Consume RegisterFilterLibNull instance > > >>>>>> Platform/SiFive: Consume RegisterFilterLibNull instance > > >>>>>> Platform/Socionext: Consume RegisterFilterLibNull instance > > >>>>>> Platform/SoftIron: Consume RegisterFilterLibNull instance > > >>>>>> Silicon/Hisilicon: Consume RegisterFilterLibNull instance > > >>>>>> Silicon/CoffeelakeSiliconPkg: Consume RegisterFilterLibNull > instance > > >>>>>> Silicon/IntelSiliconPkg: Consume RegisterFilterLibNull instan= ce > > >>>>>> Silicon/KabylakeSiliconPkg: Consume RegisterFilterLibNull > instance > > >>>>>> Silicon/QuarkSocPkg: Consume RegisterFilterLibNull instance > > >>>>>> Silicon/TigerlakeSiliconPkg: Consume RegisterFilterLibNull > instance > > >>>>>> Silicon/Marvell: Consume RegisterFilterLibNull instance > > >>>>>> Silicon/NXP: Consume RegisterFilterLibNull instance > > >>>>>> Silicon/Openmoko: Consume RegisterFilterLibNull instance > > >>>>>> Silicon/RISC_V: Consume RegisterFilterLibNull instance > > >>>>>> Silicon/Synopsys/DesignWare: Consume RegisterFilterLibNull > > >> instance > > >>>>>> Silicon/TexasInstruments: Consume RegisterFilterLibNull > instance > > >>>>>> > > >>>>>> Drivers/ASIX/Asix.dsc > > >>>>> | 1 + > > >>>>>> Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc > | 1 > > >> + > > >>>>>> Drivers/OptionRomPkg/OptionRomPkg.dsc > > >>>>> | 3 ++- > > >>>>>> .../Debugging/AcpiDebugFeaturePkg/Include/AcpiDebugFeature.d > sc | > > >> 3 > > >>>>> ++- > > >>>>>> .../Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature > .dsc > > >> | > > >>>> 3 > > >>>>> ++- > > >>>>>> .../PostCodeDebugFeaturePkg/Include/PostCodeDebugFeature.ds > c > > >> | 3 > > >>>>> ++- > > >>>>>> .../Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature > .dsc > > >> | > > >>>> 3 > > >>>>> ++- > > >>>>>> .../Intel/Network/NetworkFeaturePkg/Include/NetworkFeature.d > sc | > > >> 3 > > >>>>> ++- > > >>>>>> .../OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature > .dsc > > >> | > > >>>> 3 > > >>>>> ++- > > >>>>>> .../OutOfBandManagement/SpcrFeaturePkg/Include/SpcrFeature > .dsc > > >> | > > >>>> 3 > > >>>>> ++- > > >>>>>> .../Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.ds > c > > >> | 3 > > >>>>> ++- > > >>>>>> .../SmbiosFeaturePkg/Include/SmbiosFeature.dsc > > >> | 3 > > >>>>> ++- > > >>>>>> .../Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc > | 3 > > >> ++- > > >>>>>> .../UserAuthFeaturePkg/Include/UserAuthFeature.dsc > > >> | 3 > > >>>>> ++- > > >>>>>> .../Include/VirtualKeyboardFeature.dsc > > >> | 3 > > >>>>> ++- > > >>>>>> Platform/AMD/OverdriveBoard/OverdriveBoard.dsc > > >>>>> | 1 + > > >>>>>> Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc > > >>>>> | 1 + > > >>>>>> Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc > > >>>>> | 1 + > > >>>>>> Platform/BeagleBoard/BeagleBoardPkg/BeagleBoardPkg.dsc > > >> | > > >>>>> 3 ++- > > >>>>>> Platform/Intel/BoardModulePkg/BoardModulePkg.dsc > > >> | > > >>>>> 3 ++- > > >>>>>> Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc > > >> | 3 > > >>>>> ++- > > >>>>>> Platform/Intel/QuarkPlatformPkg/Quark.dsc > > >> | > > >>>>> 1 + > > >>>>>> Platform/Intel/QuarkPlatformPkg/QuarkMin.dsc > > >> | > > >>>>> 1 + > > >>>>>> Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc > | > > >> 3 > > >>>>> ++- > > >>>>>> Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc > | > > >> 3 > > >>>>> ++- > > >>>>>> Platform/LeMaker/CelloBoard/CelloBoard.dsc > > >> | > > >>>>> 1 + > > >>>>>> Platform/Qemu/SbsaQemu/SbsaQemu.dsc > > >>>>> | 1 + > > >>>>>> Platform/RISC-V/PlatformPkg/RiscVPlatformPkg.dsc > > >> | 1 > > >>>>> + > > >>>>>> Platform/RaspberryPi/RPi3/RPi3.dsc > > >> | > > >>>>> 3 ++- > > >>>>>> Platform/RaspberryPi/RPi4/RPi4.dsc > > >> | > > >>>>> 3 ++- > > >>>>>> > Platform/SiFive/U5SeriesPkg/FreedomU500VC707Board/U500.dsc > > >> | > > >>>>> 1 + > > >>>>>> .../U5SeriesPkg/FreedomU540HiFiveUnleashedBoard/U540.dsc > > >> | > > >>>>> 1 + > > >>>>>> Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc > > >> | 1 > > >>>>> + > > >>>>>> > Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc > > >> | 1 > > >>>>> + > > >>>>>> Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc > > >> | 1 > > >>>>> + > > >>>>>> Silicon/Hisilicon/Hisilicon.dsc.inc > | 1 > > >> + > > >>>>>> Silicon/Intel/CoffeelakeSiliconPkg/CoffeelakeSiliconPkg.dsc = | > 1 + > > >>>>>> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc = | > 3 > > >> ++- > > >>>>>> Silicon/Intel/KabylakeSiliconPkg/KabylakeSiliconPkg.dsc > | 3 > > >> ++- > > >>>>>> Silicon/Intel/QuarkSocPkg/QuarkSocPkg.dsc > > >> | > > >>>>> 3 ++- > > >>>>>> Silicon/Intel/TigerlakeSiliconPkg/TigerlakeSiliconPkg.dsc = | 1 > + > > >>>>>> Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > > >> | > > >>>>> 1 + > > >>>>>> Silicon/NXP/NxpQoriqLs.dsc.inc > > >> | > > >>>>> 1 + > > >>>>>> Silicon/Openmoko/Openmoko.dsc > > >>>>> | 1 + > > >>>>>> Silicon/RISC-V/ProcessorPkg/RiscVProcessorPkg.dsc > | > > >> 1 + > > >>>>>> Silicon/Synopsys/DesignWare/DesignWare.dsc > > >> | > > >>>>> 1 + > > >>>>>> Silicon/TexasInstruments/Omap35xxPkg/Omap35xxPkg.dsc > > >>>>> | 1 + > > >>>>>> 47 files changed, 70 insertions(+), 23 deletions(-) > > >>>>>> > > >>>>>> -- > > >>>>>> 2.18.0.windows.1 > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>> > > > > > > > > > > > > > > > > > > > >=20 >=20 >=20 >=20 >=20