回复: 回复: [edk2-devel] [edk2-platforms] [patch 00/35] Consume RegisterFilterLibNull instance
gaoliming
gaoliming at byosoft.com.cn
Fri Mar 19 01:20:41 UTC 2021
Ray:
For now, this one MdeLibs.dsc.inc is introduced to mainly resolve the future incompatible change.
For long term, this one MdeLibs.dsc.inc includes the default library instance from MdePkg. The platform DSC can still specify its library instance after include MdeLibs.dsc.inc.
Thanks
Liming
> -----邮件原件-----
> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Ni, Ray
> 发送时间: 2021年3月18日 21:42
> 收件人: devel at edk2.groups.io; lersek at redhat.com; gaoliming
> <gaoliming at byosoft.com.cn>; Bi, Dandan <dandan.bi at intel.com>;
> ardb at kernel.org; 'Andrew Fish' <afish at apple.com>
> 抄送: 'Leif Lindholm' <leif at nuviainc.com>; Kinney, Michael D
> <michael.d.kinney at intel.com>
> 主题: Re: 回复: [edk2-devel] [edk2-platforms] [patch 00/35] Consume
> RegisterFilterLibNull instance
>
> I know that NetworkPkg already provided a case of DSC header file.
> But NetworkPkg provides drivers and the dependent libs. Multiple instances
> of a single lib class are not common in NetworkPkg. So, for NetworkPkg, the
> consumer can simplify include the DSC header files.
>
>
> 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 chosen).
>
> 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 instances
> are used in the platform.
>
> Still, I am open to see what the final MdePkg DSC header will be like.
>
> > -----Original Message-----
> > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Laszlo
> Ersek
> > Sent: Thursday, March 18, 2021 9:25 PM
> > To: gaoliming <gaoliming at byosoft.com.cn>; Bi, Dandan
> <dandan.bi at intel.com>; devel at edk2.groups.io; ardb at kernel.org;
> > 'Andrew Fish' <afish at apple.com>
> > Cc: 'Leif Lindholm' <leif at nuviainc.com>; Kinney, Michael D
> <michael.d.kinney at intel.com>
> > Subject: Re: 回复: [edk2-devel] [edk2-platforms] [patch 00/35] Consume
> RegisterFilterLibNull instance
> >
> > On 03/18/21 05:43, gaoliming wrote:
> > > Laszlo and Dandan:
> > >
> > >> -----邮件原件-----
> > >> 发件人: Laszlo Ersek <lersek at redhat.com>
> > >> 发送时间: 2021年3月18日 1:08
> > >> 收件人: Bi, Dandan <dandan.bi at intel.com>; devel at edk2.groups.io;
> > >> gaoliming at byosoft.com.cn; ardb at kernel.org; 'Andrew Fish'
> > >> <afish at apple.com>
> > >> 抄送: 'Leif Lindholm' <leif at nuviainc.com>; Kinney, Michael D
> > >> <michael.d.kinney at intel.com>
> > >> 主题: Re: [edk2-devel] [edk2-platforms] [patch 00/35] Consume
> > >> RegisterFilterLibNull instance
> > >>
> > >> On 03/17/21 16:05, Bi, Dandan wrote:
> > >>>> -----Original Message-----
> > >>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > >>>> gaoliming
> > >>>> Sent: Wednesday, March 17, 2021 11:05 AM
> > >>>> To: devel at edk2.groups.io; ardb at kernel.org; Bi, Dandan
> > >>>> <dandan.bi at intel.com>; 'Laszlo Ersek' <lersek at redhat.com>;
> 'Andrew
> > >> Fish'
> > >>>> <afish at apple.com>
> > >>>> Cc: 'Leif Lindholm' <leif at nuviainc.com>; Kinney, Michael D
> > >>>> <michael.d.kinney at intel.com>
> > >>>> Subject: 回复: [edk2-devel] [edk2-platforms] [patch 00/35] Consume
> > >>>> RegisterFilterLibNull instance
> > >>>>
> > >>>> Ard and Dandan:
> > >>>>
> > >>>>> -----邮件原件-----
> > >>>>> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Ard
> > >>>> Biesheuvel
> > >>>>> 发送时间: 2021年3月16日 23:01
> > >>>>> 收件人: devel at edk2.groups.io; dandan.bi at intel.com; Laszlo Ersek
> > >>>>> <lersek at redhat.com>; Andrew Fish <afish at apple.com>
> > >>>>> 抄送: Leif Lindholm <leif at nuviainc.com>; Michael D Kinney
> > >>>>> <michael.d.kinney at intel.com>; Liming Gao
> <gaoliming at byosoft.com.cn>
> > >>>>> 主题: Re: [edk2-devel] [edk2-platforms] [patch 00/35] Consume
> > >>>>> RegisterFilterLibNull instance
> > >>>>>
> > >>>>> On Tue, 16 Mar 2021 at 15:56, Dandan Bi <dandan.bi at intel.com>
> wrote:
> > >>>>>>
> > >>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246
> > >>>>>> 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 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 <leif at nuviainc.com>
> > >>>>>> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> > >>>>>> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> > >>>>>>
> > >>>>>
> > >>>>> 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
> instance,
> > >>>> and update all Platform DSC to include it. Then, for the future change,
> 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 library
> 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/topic,
> as
> > >>> 1. It should be a code infrastructure design change/improvement in
> 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 enough
> 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] header
> > >> should be part of the file.
> > >>
> > > I agree.
> > >
> > >> (2) The introduction of "MdeLibs.dsc.inc" is a big task, in my opinion.
> > >>
> > > 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 use
> > 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 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.
> > >>
> > >> 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' resolution 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 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
> 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 instance
> > >>>>>> 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
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>
> > >
> > >
> > >
> >
> >
> >
> >
> >
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73029): https://edk2.groups.io/g/devel/message/73029
Mute This Topic: https://groups.io/mt/81445700/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