From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web09.300.1616000915942289096 for ; Wed, 17 Mar 2021 10:08:36 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Wb4izoPH; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1616000911; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3dpY7vs9+eMN5cr3oFcdnBXd9G6ywbxgRGlve9Js5wE=; b=Wb4izoPH2Z1AY5HNit3xVtWSV8musB5ZyDex3g4GLya+YYRIDzYFrGwVBo9Fupq0sy5ei+ b2n6o7N7bHAtka/sw47sRVzJgnvbqYcjPE0EmXqH3qQe6n7mQi5Eoc7Dk8+FvMsB/Kbvi1 OUPN80KSSWq9KSw6HleFDEBUWQU65EE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-321-SpVw6v_SMPy_smRnHx_s8w-1; Wed, 17 Mar 2021 13:08:21 -0400 X-MC-Unique: SpVw6v_SMPy_smRnHx_s8w-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 1FBB6180FCA3; Wed, 17 Mar 2021 17:08:19 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-112.ams2.redhat.com [10.36.113.112]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2229610016F8; Wed, 17 Mar 2021 17:08:16 +0000 (UTC) Subject: Re: [edk2-devel] [edk2-platforms] [patch 00/35] Consume RegisterFilterLibNull instance To: "Bi, Dandan" , "devel@edk2.groups.io" , "gaoliming@byosoft.com.cn" , "ardb@kernel.org" , 'Andrew Fish' Cc: 'Leif Lindholm' , "Kinney, Michael D" References: <20210316145428.35616-1-dandan.bi@intel.com> <00c601d71ada$42fdc900$c8f95b00$@byosoft.com.cn> From: "Laszlo Ersek" Message-ID: <7b41b3f7-b5dd-7fc6-3d75-32d9367b2902@redhat.com> Date: Wed, 17 Mar 2021 18:08:16 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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: 回复: [edk2-devel] [edk2-platforms] [patch 00/35] Consume >> RegisterFilterLibNull instance >> >> Ard and Dandan: >> >>> -----邮件原件----- >>> 发件人: devel@edk2.groups.io 代表 Ard >> Biesheuvel >>> 发送时间: 2021年3月16日 23:01 >>> 收件人: devel@edk2.groups.io; dandan.bi@intel.com; Laszlo Ersek >>> ; Andrew Fish >>> 抄送: Leif Lindholm ; Michael D Kinney >>> ; Liming Gao >>> 主题: 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=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 >>>> 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 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. (2) The introduction of "MdeLibs.dsc.inc" is a big task, in my opinion. 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 >>>> >>>> >>>> >>>> >>>> >>>> >>> >>> >>> >>> >> >> >> >> >> >> >> >