Re: [PATCH 4/6] MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version

["Zeng, Star" <star.zeng@intel.com>]

On 2019/1/11 0:23, Ard Biesheuvel wrote:
> On Thu, 10 Jan 2019 at 14:03, Laszlo Ersek <lersek@redhat.com> 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 <star.zeng@intel.com> 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 <ard.biesheuvel@linaro.org>
>>>>>>> ---
>>>>>>>
>>>>>>> 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


>