From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E71FA211B63E0 for ; Thu, 10 Jan 2019 18:19:24 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Jan 2019 18:19:24 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,463,1539673200"; d="scan'208";a="125154430" Received: from shzintpr03.sh.intel.com (HELO [10.7.209.21]) ([10.239.4.100]) by orsmga002.jf.intel.com with ESMTP; 10 Jan 2019 18:19:22 -0800 To: Ard Biesheuvel , Laszlo Ersek Cc: Hao Wu , Michael D Kinney , "edk2-devel@lists.01.org" , Liming Gao , star.zeng@intel.com References: <20190103182825.32231-1-ard.biesheuvel@linaro.org> <20190103182825.32231-6-ard.biesheuvel@linaro.org> <4a353237-1191-a831-1b6e-e981b7c59a7d@intel.com> <539a4ec0-eb09-588b-2a24-4b1f450f587b@intel.com> <91581880-e3ef-31d6-40eb-1d52f5f0cdc1@redhat.com> From: "Zeng, Star" Message-ID: Date: Fri, 11 Jan 2019 10:18:51 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Jan 2019 02:19:26 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 2019/1/11 0:23, Ard Biesheuvel wrote: > On Thu, 10 Jan 2019 at 14:03, Laszlo Ersek wrote: >> >> On 01/10/19 08:59, Zeng, Star wrote: >>> On 2019/1/10 15:33, Ard Biesheuvel wrote: >>>> On Thu, 10 Jan 2019 at 08:30, Zeng, Star wrote: >>>>> >>>>> Hi Ard, >>>>> >>>>> Another minor feedback. >>>>> >>>>> On 2019/1/10 14:47, Zeng, Star wrote: >>>>>> Hi Ard, >>>>>> >>>>>> Some minor feedback added inline. >>>>>> >>>>>> On 2019/1/4 2:28, Ard Biesheuvel wrote: >>>>>>> Implement a new version of the fault tolerant write driver that can >>>>>>> be used in the context of a standalone MM implementation. >>>>>>> >>>>>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>>>>> Signed-off-by: Ard Biesheuvel >>>>>>> --- >>>>>>> >>>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c >>>>>>> >>>>>>> | 70 +++++++++++++++ >>>>>>> >>>>>>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf >>>>>>> >>>>>>> | 90 ++++++++++++++++++++ >>>>>>> 2 files changed, 160 insertions(+) >>>>> >>>>> Please add it into MdeModulePkg.dsc for package build verification. >>>>> >>>> >>>> Hello Star, >>>> >>>> Thanks for all the feedback. I will respond in more detail later. >>>> >>>> However, to the point raised here: it is not possible to add these >>>> drivers to MdeModulePkg.dsc unless we add a dummy implementation of >>>> StandaloneMmDriverEntryPoint to MdeModulePkg. Do you think we should >>>> do that? >>> >>> Oh, good information. >>> To have full code building coverage for the package, personally I think >>> we can move StandaloneMmDriverEntryPoint library class and instance into >>> MdePkg, and even the MmServicesTableLib for MM_STANDALONE, they should >>> be generic enough. >>> >>> I do not want to block this patch set because of this. So let's discuss >>> this in parallel as separated topic. >>> >>> Mike, Liming, Laszlo, Jian and Hao,\ >>> What's your opinion? >> >> It should be possible to build all library instances in a central >> Package (well, all Packages really), using the Package's DSC file. To my >> understanding, libraries built like this are not expected to be used in >> actual (shipped) drivers / applications, nor is their indiscriminate >> distribution (as LIBs) expected. For example, shipping a BaseXxxLibNull >> library instance in binary form seems quite useless. >> >> With that in mind, I think a Null instance for the entry point in >> question makes sense, under MdeModulePkg. >> > > I will look into this a bit deeper next week. I think it makes sense > for the core PI architected pieces to all live in MdePkg rather than > StandaloneMmPkg. For instance, MmServicesTableLib for standalone MM > should live there, MmEntryPoint should live there (and have > traditional and standalone MM implementation) and perhaps some other > core pieces as well. > > This may be a slippery slope, so I will dedicate some time to look > into this carefully, at least with the goal to make the > FaultTolerantWrite and Variable driver buildable from within > MdeModulePkg. Make sense to me. You'd better to submit a bugzilla to track this after this patchset is pushed. Thanks, Star >