[PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[Kun Qin]

The interface proposed in this patch series intends to provide an
abstraction layer for DXE drivers to request certain memory regions to be
accessible from inside MM environment that applies total memory blockage.

This abstraction could pave way for models such as Standalone MM to
manage memory resources without having knowledge of DXE memory map inside
MM environment.

Example usages of it can be NVS region in Tcg2Smm and runtime variable
cache regions in VariableSmmRuntimeDxe.

Patch v1 branch: https://github.com/kuqin12/edk2/tree/unblock_mem_v1

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>

Kun Qin (1):
  MdeModulePkg: DxeMmUnblockMemoryLib: Added definition and null
    instance

 MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemoryLibNull.c   | 40 ++++++++++++++++++++
 MdeModulePkg/Include/Library/DxeMmUnblockMemoryLib.h                     | 40 ++++++++++++++++++++
 MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemoryLibNull.inf | 29 ++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                                            |  5 +++
 MdeModulePkg/MdeModulePkg.dsc                                            |  2 +
 5 files changed, 116 insertions(+)
 create mode 100644 MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemoryLibNull.c
 create mode 100644 MdeModulePkg/Include/Library/DxeMmUnblockMemoryLib.h
 create mode 100644 MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemoryLibNull.inf

-- 
2.30.0.windows.1

Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[Wu, Hao A]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
> Sent: Wednesday, February 3, 2021 6:16 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib
> Interface
> 
> The interface proposed in this patch series intends to provide an abstraction
> layer for DXE drivers to request certain memory regions to be accessible from
> inside MM environment that applies total memory blockage.
> 
> This abstraction could pave way for models such as Standalone MM to
> manage memory resources without having knowledge of DXE memory map
> inside MM environment.
> 
> Example usages of it can be NVS region in Tcg2Smm and runtime variable
> cache regions in VariableSmmRuntimeDxe.


My thought is that it might be more helpful if the whole implementation proposal
to address BZ-3168 can be provided before reviewing the interfaces for the new
library (or the library itself).

Hello Jiewen,

Do you have comments on the approach on implementing the BZ-3168
(https://bugzilla.tianocore.org/show_bug.cgi?id=3168) feature?
Thanks in advance.

Best Regards,
Hao Wu


> 
> Patch v1 branch: https://github.com/kuqin12/edk2/tree/unblock_mem_v1
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> 
> Kun Qin (1):
>   MdeModulePkg: DxeMmUnblockMemoryLib: Added definition and null
>     instance
> 
> 
> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
> ryLibNull.c   | 40 ++++++++++++++++++++
>  MdeModulePkg/Include/Library/DxeMmUnblockMemoryLib.h                     |
> 40 ++++++++++++++++++++
> 
> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
> ryLibNull.inf | 29 ++++++++++++++
>  MdeModulePkg/MdeModulePkg.dec                                            |  5 +++
>  MdeModulePkg/MdeModulePkg.dsc                                            |  2 +
>  5 files changed, 116 insertions(+)
>  create mode 100644
> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
> ryLibNull.c
>  create mode 100644
> MdeModulePkg/Include/Library/DxeMmUnblockMemoryLib.h
>  create mode 100644
> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
> ryLibNull.inf
> 
> --
> 2.30.0.windows.1
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[Sean]

Hao wu,

I agree that for reviewing this change that would provide more confidence.
The real issue is that there is no x64 mm standalone solution that
blocks memory access in edk2 today.  So implementing this interface in 
edk2 doesn't make sense.  It would just rot with no users, validation, 
or maintenance.

The interface is needed because this is a compatibility point in other 
drivers.  Without it, those other drivers need to be forked and then 
maintained and that is not in the best interest of anyone.

To push a new standalone x64 mm model up that blocks dxe memory space is 
not small task and we are working on it and are happy to contribute it 
to edk2 / open source but it will not happen quickly.  So step 1 is get 
a compatible abstraction/interface added.  Step 2 is work to upstream 
the core modules.

We may be able to share the core modules with you if you want to see 
them for review purposes but we need more dev work before upstreaming 
the change to edk2.

Thanks
Sean




On 2/4/2021 6:17 PM, Wu, Hao A wrote:
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
>> Sent: Wednesday, February 3, 2021 6:16 AM
>> To: devel@edk2.groups.io
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
>> Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
>> <jiewen.yao@intel.com>
>> Subject: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib
>> Interface
>>
>> The interface proposed in this patch series intends to provide an abstraction
>> layer for DXE drivers to request certain memory regions to be accessible from
>> inside MM environment that applies total memory blockage.
>>
>> This abstraction could pave way for models such as Standalone MM to
>> manage memory resources without having knowledge of DXE memory map
>> inside MM environment.
>>
>> Example usages of it can be NVS region in Tcg2Smm and runtime variable
>> cache regions in VariableSmmRuntimeDxe.
> 
> 
> My thought is that it might be more helpful if the whole implementation proposal
> to address BZ-3168 can be provided before reviewing the interfaces for the new
> library (or the library itself).
> 
> Hello Jiewen,
> 
> Do you have comments on the approach on implementing the BZ-3168
> (https://bugzilla.tianocore.org/show_bug.cgi?id=3168) feature?
> Thanks in advance.
> 
> Best Regards,
> Hao Wu
> 
> 
>>
>> Patch v1 branch: https://github.com/kuqin12/edk2/tree/unblock_mem_v1
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Hao A Wu <hao.a.wu@intel.com>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>
>> Kun Qin (1):
>>    MdeModulePkg: DxeMmUnblockMemoryLib: Added definition and null
>>      instance
>>
>>
>> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
>> ryLibNull.c   | 40 ++++++++++++++++++++
>>   MdeModulePkg/Include/Library/DxeMmUnblockMemoryLib.h                     |
>> 40 ++++++++++++++++++++
>>
>> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
>> ryLibNull.inf | 29 ++++++++++++++
>>   MdeModulePkg/MdeModulePkg.dec                                            |  5 +++
>>   MdeModulePkg/MdeModulePkg.dsc                                            |  2 +
>>   5 files changed, 116 insertions(+)
>>   create mode 100644
>> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
>> ryLibNull.c
>>   create mode 100644
>> MdeModulePkg/Include/Library/DxeMmUnblockMemoryLib.h
>>   create mode 100644
>> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
>> ryLibNull.inf
>>
>> --
>> 2.30.0.windows.1
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[Wu, Hao A]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> Sent: Friday, February 5, 2021 10:37 AM
> To: devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com>;
> kun.q@outlook.com; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 0/1] Introduce
> DxeMmUnblockMemoryLib Interface
> 
> Hao wu,
> 
> I agree that for reviewing this change that would provide more confidence.
> The real issue is that there is no x64 mm standalone solution that blocks
> memory access in edk2 today.  So implementing this interface in
> edk2 doesn't make sense.  It would just rot with no users, validation, or
> maintenance.
> 
> The interface is needed because this is a compatibility point in other drivers.
> Without it, those other drivers need to be forked and then maintained and
> that is not in the best interest of anyone.


Thanks Sean.
Sorry for a question, do you have an example for the drivers with potential
compatibility opens? It would be helpful to have a better understanding of the
purpose of this library.

Best Regards,
Hao Wu


> 
> To push a new standalone x64 mm model up that blocks dxe memory space
> is not small task and we are working on it and are happy to contribute it to
> edk2 / open source but it will not happen quickly.  So step 1 is get a
> compatible abstraction/interface added.  Step 2 is work to upstream the core
> modules.
> 
> We may be able to share the core modules with you if you want to see them
> for review purposes but we need more dev work before upstreaming the
> change to edk2.
> 
> Thanks
> Sean
> 
> 
> 
> 
> On 2/4/2021 6:17 PM, Wu, Hao A wrote:
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun
> >> Qin
> >> Sent: Wednesday, February 3, 2021 6:16 AM
> >> To: devel@edk2.groups.io
> >> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> >> <hao.a.wu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> >> <ray.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> >> Subject: [edk2-devel] [PATCH v1 0/1] Introduce
> DxeMmUnblockMemoryLib
> >> Interface
> >>
> >> The interface proposed in this patch series intends to provide an
> >> abstraction layer for DXE drivers to request certain memory regions
> >> to be accessible from inside MM environment that applies total memory
> blockage.
> >>
> >> This abstraction could pave way for models such as Standalone MM to
> >> manage memory resources without having knowledge of DXE memory
> map
> >> inside MM environment.
> >>
> >> Example usages of it can be NVS region in Tcg2Smm and runtime
> >> variable cache regions in VariableSmmRuntimeDxe.
> >
> >
> > My thought is that it might be more helpful if the whole
> > implementation proposal to address BZ-3168 can be provided before
> > reviewing the interfaces for the new library (or the library itself).
> >
> > Hello Jiewen,
> >
> > Do you have comments on the approach on implementing the BZ-3168
> > (https://bugzilla.tianocore.org/show_bug.cgi?id=3168) feature?
> > Thanks in advance.
> >
> > Best Regards,
> > Hao Wu
> >
> >
> >>
> >> Patch v1 branch:
> https://github.com/kuqin12/edk2/tree/unblock_mem_v1
> >>
> >> Cc: Jian J Wang <jian.j.wang@intel.com>
> >> Cc: Hao A Wu <hao.a.wu@intel.com>
> >> Cc: Eric Dong <eric.dong@intel.com>
> >> Cc: Ray Ni <ray.ni@intel.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >>
> >> Kun Qin (1):
> >>    MdeModulePkg: DxeMmUnblockMemoryLib: Added definition and null
> >>      instance
> >>
> >>
> >>
> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
> >> ryLibNull.c   | 40 ++++++++++++++++++++
> >>   MdeModulePkg/Include/Library/DxeMmUnblockMemoryLib.h
> |
> >> 40 ++++++++++++++++++++
> >>
> >>
> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
> >> ryLibNull.inf | 29 ++++++++++++++
> >>   MdeModulePkg/MdeModulePkg.dec                                            |  5 +++
> >>   MdeModulePkg/MdeModulePkg.dsc                                            |  2 +
> >>   5 files changed, 116 insertions(+)
> >>   create mode 100644
> >>
> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
> >> ryLibNull.c
> >>   create mode 100644
> >> MdeModulePkg/Include/Library/DxeMmUnblockMemoryLib.h
> >>   create mode 100644
> >>
> MdeModulePkg/Library/DxeMmUnblockMemoryLib/DxeMmUnblockMemo
> >> ryLibNull.inf
> >>
> >> --
> >> 2.30.0.windows.1
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[Kun Qin]

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

Hi Hao,

As mentioned in the cover letter and in BZ-3168, VariableStandaloneMm and Tcg2Smm would need this capability to unblock certain regions in order to access either variable runtime cache or NVS region patched into ACPI table, if non-MMRAM region is blocked for access.

Just as a preview (error handling needs better polishing), the example usages for the 2 drivers above could be found here:
1. Variable DXE SMM change that unblocks runtime cache regions is posted here: https://github.com/kuqin12/mu_basecore/commit/189f90318d1256c2e72a1a67d31e3176588d8e5b
2. Tcg2Smm change example, which breaks one module into 2 and then unblock NVS region, is posted here: https://github.com/kuqin12/mu_basecore/commit/c0784f95ffe31d4d65f6230657adfc63294a693d

The change should be relatively minor but critical for the standalone Mm model.

Thanks,
Kun

[-- Attachment #2: Type: text/html, Size: 1227 bytes --]

Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[Wu, Hao A]

[-- Attachment #1: Type: text/plain, Size: 1565 bytes --]

Thanks Kun,

If my understanding is correct, this proposed library will not have any consumer until the implementation of blocking the access of unregistered memory access in MM is made.
If this is the case, my take for this sent patch is that it is a RFC to the SMM drivers owners on the library API interface.
And the actual patch for adding this library will come together will the implementation to block unregistered memory access within MM.

Best Regards,
Hao Wu

From: Kun Qin <kun.q@outlook.com>
Sent: Friday, February 5, 2021 11:11 AM
To: Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

Hi Hao,

As mentioned in the cover letter and in BZ-3168, VariableStandaloneMm and Tcg2Smm would need this capability to unblock certain regions in order to access either variable runtime cache or NVS region patched into ACPI table, if non-MMRAM region is blocked for access.

Just as a preview (error handling needs better polishing), the example usages for the 2 drivers above could be found here:
1. Variable DXE SMM change that unblocks runtime cache regions is posted here: https://github.com/kuqin12/mu_basecore/commit/189f90318d1256c2e72a1a67d31e3176588d8e5b
2. Tcg2Smm change example, which breaks one module into 2 and then unblock NVS region, is posted here: https://github.com/kuqin12/mu_basecore/commit/c0784f95ffe31d4d65f6230657adfc63294a693d

The change should be relatively minor but critical for the standalone Mm model.

Thanks,
Kun

[-- Attachment #2: Type: text/html, Size: 4342 bytes --]

Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[Kun Qin]

[-- Attachment #1: Type: text/plain, Size: 386 bytes --]

Hi Hao,

My plan was to follow up with the driver changes regarding Tcg2Smm and VariableSmmRuntimeDxe once this interface is officially checked in. But if it is preferred to submit the patch for Tcg2Smm and VariableSmmRuntimeDxe to make better sense on how this interface will be consumed, I can send them out in v2. Please let me know how you would like to proceed.

Thanks,
Kun

[-- Attachment #2: Type: text/html, Size: 406 bytes --]

Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[Wu, Hao A]

[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]

Hello Kun,

Could you help to send the library interface together with its usage in modules in the series?
Thanks in advance.

Also, one more question is that are Tcg2Smm and VariableSmmRuntimeDxe the only modules need to be updated?
Or an investigation in the code base should be done to address all the potential modules affected?

Best Regards,
Hao Wu

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
Sent: Saturday, February 6, 2021 10:48 AM
To: Wu; Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

Hi Hao,

My plan was to follow up with the driver changes regarding Tcg2Smm and VariableSmmRuntimeDxe once this interface is officially checked in. But if it is preferred to submit the patch for Tcg2Smm and VariableSmmRuntimeDxe to make better sense on how this interface will be consumed, I can send them out in v2. Please let me know how you would like to proceed.

Thanks,
Kun


[-- Attachment #2: Type: text/html, Size: 3717 bytes --]

Re: [edk2-devel] [PATCH v1 0/1] Introduce DxeMmUnblockMemoryLib Interface

[Kun Qin]

[-- Attachment #1: Type: text/plain, Size: 195 bytes --]

Hi Hao,

These 2 modules seems to be the only ones need to be updated when we scrubbed and validated our code base.

I will prepare for the tcg2 and variable module patches.

Thanks,
Kun

[-- Attachment #2: Type: text/html, Size: 223 bytes --]