From: "gaoliming" <gaoliming@byosoft.com.cn>
To: "'Laszlo Ersek'" <lersek@redhat.com>,
"'Bi, Dandan'" <dandan.bi@intel.com>, <devel@edk2.groups.io>,
<ardb@kernel.org>, "'Andrew Fish'" <afish@apple.com>
Cc: "'Leif Lindholm'" <leif@nuviainc.com>,
"'Kinney, Michael D'" <michael.d.kinney@intel.com>
Subject: 回复: [edk2-devel] [edk2-platforms] [patch 00/35] Consume RegisterFilterLibNull instance
Date: Thu, 18 Mar 2021 12:43:42 +0800 [thread overview]
Message-ID: <007301d71bb1$464e58e0$d2eb0aa0$@byosoft.com.cn> (raw)
In-Reply-To: <7b41b3f7-b5dd-7fc6-3d75-32d9367b2902@redhat.com>
Laszlo and Dandan:
> -----邮件原件-----
> 发件人: Laszlo Ersek <lersek@redhat.com>
> 发送时间: 2021年3月18日 1:08
> 收件人: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io;
> gaoliming@byosoft.com.cn; ardb@kernel.org; 'Andrew Fish'
> <afish@apple.com>
> 抄送: 'Leif Lindholm' <leif@nuviainc.com>; Kinney, Michael D
> <michael.d.kinney@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@edk2.groups.io <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
> >> <dandan.bi@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; 'Andrew
> Fish'
> >> <afish@apple.com>
> >> Cc: 'Leif Lindholm' <leif@nuviainc.com>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>
> >> Subject: 回复: [edk2-devel] [edk2-platforms] [patch 00/35] Consume
> >> RegisterFilterLibNull instance
> >>
> >> Ard and Dandan:
> >>
> >>> -----邮件原件-----
> >>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> >> Biesheuvel
> >>> 发送时间: 2021年3月16日 23:01
> >>> 收件人: devel@edk2.groups.io; dandan.bi@intel.com; Laszlo Ersek
> >>> <lersek@redhat.com>; Andrew Fish <afish@apple.com>
> >>> 抄送: Leif Lindholm <leif@nuviainc.com>; Michael D Kinney
> >>> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@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@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@nuviainc.com>
> >>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >>>> Cc: Liming Gao <gaoliming@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.
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.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
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >
next prev parent reply other threads:[~2021-03-18 4:43 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-16 14:53 [edk2-platforms] [patch 00/35] Consume RegisterFilterLibNull instance Dandan Bi
2021-03-16 14:53 ` [edk2-platforms] [patch 01/35] Drivers/ASIX: " Dandan Bi
2021-03-16 14:53 ` [edk2-platforms] [patch 02/35] Drivers/DisplayLink: " Dandan Bi
2021-03-16 14:53 ` [edk2-platforms] [patch 03/35] Drivers/OptionRomPkg: " Dandan Bi
2021-03-16 14:53 ` [edk2-platforms] [patch 04/35] Features/Debugging: " Dandan Bi
2021-03-16 14:53 ` [edk2-platforms] [patch 05/35] Features/Network: " Dandan Bi
2021-03-16 14:53 ` [edk2-platforms] [patch 06/35] Features/OutOfBandManagement: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 07/35] Features/PowerManagement: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 08/35] Features/SystemInformation: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 09/35] Features/UserInterface: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 10/35] Platform/AMD: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 11/35] Platform/ARM: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 12/35] Platform/BeagleBoard: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 13/35] Platform/BoardModulePkg: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 14/35] Platform/MinPlatformPkg: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 15/35] Platform/QuarkPlatformPkg: " Dandan Bi
2021-03-16 15:04 ` Steele, Kelly
2021-03-16 14:54 ` [edk2-platforms] [patch 16/35] Platform/Vlv2TbltDevicePkg: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 17/35] Platform/LeMaker: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 18/35] Platform/Qemu: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 19/35] Platform/RaspberryPi: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 20/35] Platform/RISC-V: " Dandan Bi
2021-03-17 8:14 ` Abner Chang
2021-03-16 14:54 ` [edk2-platforms] [patch 21/35] Platform/SiFive: " Dandan Bi
2021-03-17 8:14 ` Abner Chang
2021-03-16 14:54 ` [edk2-platforms] [patch 22/35] Platform/Socionext: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 23/35] Platform/SoftIron: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 24/35] Silicon/Hisilicon: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 25/35] Silicon/CoffeelakeSiliconPkg: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 26/35] Silicon/IntelSiliconPkg: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 27/35] Silicon/KabylakeSiliconPkg: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 28/35] Silicon/QuarkSocPkg: " Dandan Bi
2021-03-16 15:05 ` Steele, Kelly
2021-03-16 14:54 ` [edk2-platforms] [patch 29/35] Silicon/TigerlakeSiliconPkg: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 30/35] Silicon/Marvell: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 31/35] Silicon/NXP: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 32/35] Silicon/Openmoko: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 33/35] Silicon/RISC_V: " Dandan Bi
2021-03-17 8:14 ` Abner Chang
2021-03-16 14:54 ` [edk2-platforms] [patch 34/35] Silicon/Synopsys/DesignWare: " Dandan Bi
2021-03-16 14:54 ` [edk2-platforms] [patch 35/35] Silicon/TexasInstruments: " Dandan Bi
2021-03-16 15:00 ` [edk2-devel] [edk2-platforms] [patch 00/35] " Ard Biesheuvel
2021-03-16 16:23 ` Laszlo Ersek
2021-03-17 3:04 ` 回复: " gaoliming
2021-03-17 15:05 ` Dandan Bi
2021-03-17 17:08 ` Laszlo Ersek
2021-03-18 4:43 ` gaoliming [this message]
2021-03-18 13:25 ` 回复: " Laszlo Ersek
2021-03-18 13:42 ` Ni, Ray
2021-03-19 1:20 ` 回复: " gaoliming
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='007301d71bb1$464e58e0$d2eb0aa0$@byosoft.com.cn' \
--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