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.web12.8769.1616073940172767105 for ; Thu, 18 Mar 2021 06:25:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=NCxMXxA1; 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=1616073939; 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=FRCtdhYJeYheK300jIlcxrztrvSqLP2o0JW/arYDKPg=; b=NCxMXxA1yozXAWvk9L3sizVMyvC2IKkOG3ulVdnMv94E3YDjkBlO8zrzpLchZrXVo14vfm 4n57mnUUdo+01v5/8PkqPQuU10cFj9W89A0U3xIt3kmi+w2s3Nq09/PyoeCUmObUNmtq84 VuG1PKyqllA9bfTTVIhoDzOF0OBFeo4= 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-187-TYxoxiHDNwaNIjRfxTQt0w-1; Thu, 18 Mar 2021 09:25:24 -0400 X-MC-Unique: TYxoxiHDNwaNIjRfxTQt0w-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 794F48162B; Thu, 18 Mar 2021 13:25:22 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-176.ams2.redhat.com [10.36.115.176]) by smtp.corp.redhat.com (Postfix) with ESMTP id 88D4D60843; Thu, 18 Mar 2021 13:25:19 +0000 (UTC) Subject: =?UTF-8?B?UmU6IOWbnuWkjTogW2VkazItZGV2ZWxdIFtlZGsyLXBsYXRmb3Jtc10gW3BhdGNoIDAwLzM1XSBDb25zdW1lIFJlZ2lzdGVyRmlsdGVyTGliTnVsbCBpbnN0YW5jZQ==?= To: gaoliming , "'Bi, Dandan'" , devel@edk2.groups.io, 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> <7b41b3f7-b5dd-7fc6-3d75-32d9367b2902@redhat.com> <007301d71bb1$464e58e0$d2eb0aa0$@byosoft.com.cn> From: "Laszlo Ersek" Message-ID: <6094a2fc-86f9-2d11-b322-80279aa5ed6e@redhat.com> Date: Thu, 18 Mar 2021 14:25:18 +0100 MIME-Version: 1.0 In-Reply-To: <007301d71bb1$464e58e0$d2eb0aa0$@byosoft.com.cn> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 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/18/21 05:43, gaoliming wrote: > Laszlo and Dandan: > >> -----邮件原件----- >> 发件人: Laszlo Ersek >> 发送时间: 2021年3月18日 1:08 >> 收件人: Bi, Dandan ; devel@edk2.groups.io; >> gaoliming@byosoft.com.cn; ardb@kernel.org; 'Andrew Fish' >> >> 抄送: 'Leif Lindholm' ; Kinney, Michael D >> >> 主题: 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: 回复: [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. >> > 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.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 >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>> > > >