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.web11.4937.1616042629579104444 for ; Wed, 17 Mar 2021 21:43:50 -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 ; Thu, 18 Mar 2021 12:43:41 +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: "'Laszlo Ersek'" , "'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> In-Reply-To: <7b41b3f7-b5dd-7fc6-3d75-32d9367b2902@redhat.com> Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW2VkazItcGxhdGZvcm1zXSBbcGF0Y2ggMDAvMzVdIENvbnN1bWUgUmVnaXN0ZXJGaWx0ZXJMaWJOdWxsIGluc3RhbmNl?= Date: Thu, 18 Mar 2021 12:43:42 +0800 Message-ID: <007301d71bb1$464e58e0$d2eb0aa0$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQItdfl10XSTJBOTUudN/bhmcVVhogFKI7HDAjOjCp4CFQnDZgJV4WZkqZ0cCwA= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn 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 ; devel@edk= 2.groups.io; > gaoliming@byosoft.com.cn; ardb@kernel.org; 'Andrew Fish' > > =E6=8A=84=E9=80=81: 'Leif Lindholm' ; Kinney, Michael= D > > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [edk2-platforms] [patch 00/35] Cons= ume > RegisterFilterLibNull instance >=20 > 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@intel.c= om; Laszlo Ersek > >>> ; Andrew Fish > >>> =E6=8A=84=E9=80=81: Leif Lindholm ; Michael D Kin= ney > >>> ; 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, whic= h > >>>> will be consumed by IoLib and BaseLib. > >>>> > >>>> This is the following update in edk2-platforms repo for the change > >>>> 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 library i= nstance, > >> and update all Platform DSC to include it. Then, for the future chang= e, no > >> change is required for platform DSC. > >> > >> Because this patch set updates every platform DSC, I suggest to intro= duce > >> MdePkg.dsc.inc file in this patch set. > > > > Hi Liming, > > > > I agree that add MdePkg.dsc.inc file to include the default library in= stance > and make it consumed by platform dsc will benefit future similar incompa= tible > changes. > > But I wonder to know whether we could do it in a separated task/topic,= as > > 1. It should be a code infrastructure design change/improvement in ed= k2. > > 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 enough t= o > 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. >=20 > (1) The file name should be "MdeLibs.dsc.inc", and it should be > structured similarly "NetworkLibs.dsc.inc" -- no [LibraryClasses] header > should be part of the file. >=20 I agree. > (2) The introduction of "MdeLibs.dsc.inc" is a big task, in my opinion. >=20 The full MdeLibs.dsc.inc is a big task. But, it should be a separate task.= = =20 For this patch set, the first version MdeLibs.dsc.inc can be added. It onl= y includes RegisterFilterLibNull library instance. And, this patch set will update every platform DSC to include MdeLibs.dsc.= inc.=20 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.i= nc on the first step. 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 simple > !include directive. Such a patch is unreviewable, as I'd have no way of > carefully comparing the before-after state, let alone a way of *pointing > out* (in comments) where exactly a problem was. >=20 > 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: >=20 > - 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. >=20 > Thanks > Laszlo >=20 >=20 > > > > > > Thanks, > > Dandan > >> > >> Thanks > >> Liming > >>> Is there really not a better way to provide a 'default' resolution f= or > >>> 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 use= d > >>> 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 instanc= e > >>>> Features/UserInterface: Consume RegisterFilterLibNull instance > >>>> 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 instanc= e > >>>> 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 insta= nce > >>>> Silicon/IntelSiliconPkg: Consume RegisterFilterLibNull instance > >>>> Silicon/KabylakeSiliconPkg: Consume RegisterFilterLibNull instanc= e > >>>> Silicon/QuarkSocPkg: Consume RegisterFilterLibNull instance > >>>> Silicon/TigerlakeSiliconPkg: Consume RegisterFilterLibNull instan= ce > >>>> 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.dsc | > 3 > >>> ++- > >>>> .../Debugging/BeepDebugFeaturePkg/Include/BeepDebugFeature.dsc > | > >> 3 > >>> ++- > >>>> .../PostCodeDebugFeaturePkg/Include/PostCodeDebugFeature.dsc > | 3 > >>> ++- > >>>> .../Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeature.dsc > | > >> 3 > >>> ++- > >>>> .../Intel/Network/NetworkFeaturePkg/Include/NetworkFeature.dsc | > 3 > >>> ++- > >>>> .../OutOfBandManagement/IpmiFeaturePkg/Include/IpmiFeature.dsc > | > >> 3 > >>> ++- > >>>> .../OutOfBandManagement/SpcrFeaturePkg/Include/SpcrFeature.dsc > | > >> 3 > >>> ++- > >>>> .../Intel/PowerManagement/S3FeaturePkg/Include/S3Feature.dsc > | 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 > >> > >