Message ID | 20190103182825.32231-1-ard.biesheuvel@linaro.org |
---|---|
Headers | show |
Series | implement standalone MM versions of the variable runtime drivers | expand |
On Thu, 3 Jan 2019 at 19:28, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > This series proposed an alternative approach to the series sent out by > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > special PCD, as well as some other if() conditionals. > That would be [0] https://lists.01.org/pipermail/edk2-devel/2019-January/034542.html Also, I seem to have included a BaseTools/ patch in error. Apologies for the sloppiness. > The primary difference is that this series defines and implements > MmServicesTableLib in such a way that the traditional SMM drivers > can use it as well. This is appropriate, considering that the PI > spec has rebranded traditional SMM as one implementation of the generic > MM framework. > > Patch #1 is based on Jagadeesh's patch, and introduces the MmServicesTableLib > library class, but for all SMM flavours, not only for standalone MM. > > Patch #2 implements MmServicesTableLib for traditional SMM implementations. > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > driver that invoke boot services are separated from the core SMM pieces. > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM environment. > > Patches #5 and #6 do the same, respectively, for the variable runtime driver. > > This approach minimizes the delta, and thus the maintenance burden, between > the traditional SMM and standalone MM drivers, while not resorting to runtime > checks or other conditionals in the code to implement logic that should be > decided at build time. > > Note that this series only covers part of the work contributed by Jagadeesh. > This series focuses on the MdePkg and MdeModulePkg changes that affect shared > code. > > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Hao Wu <hao.a.wu@intel.com> > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > Cc: Achin Gupta <Achin.Gupta@arm.com> > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > > Ard Biesheuvel (5): > MdePkg: implement MmServicesTableLib based on traditional SMM > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version > MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses > MdeModulePkg/VariableRuntimeDxe: implement standalone MM version > > Jagadeesh Ujja (1): > MdePkg/Include: add MmServicesTableLib header file > > MdeModulePkg/MdeModulePkg.dsc | 1 + > .../FaultTolerantWrite.h | 22 ++- > .../FaultTolerantWriteDxe.c | 31 ++++ > .../FaultTolerantWriteSmm.c | 54 +++---- > .../FaultTolerantWriteSmm.inf | 5 +- > .../FaultTolerantWriteSmmCommon.h | 31 ++++ > .../FaultTolerantWriteSmmDxe.c | 1 + > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ > .../UpdateWorkingBlock.c | 10 +- > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ > .../RuntimeDxe/VariableStandaloneMm.inf | 135 ++++++++++++++++++ > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ > MdePkg/MdePkg.dec | 4 + > MdePkg/MdePkg.dsc | 1 + > 24 files changed, 916 insertions(+), 103 deletions(-) > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni > > -- > 2.17.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Ard: I agree this design is good. But, I need some time to evaluate its impact on our X86 platform. Could you wait for several days? Thanks Liming > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Friday, January 4, 2019 2:28 AM > To: edk2-devel@lists.01.org > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, > Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A > <hao.a.wu@intel.com>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam > Abraham <thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com> > Subject: [PATCH 0/6] implement standalone MM versions of the variable runtime drivers > > This series proposed an alternative approach to the series sent out by > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > special PCD, as well as some other if() conditionals. > > The primary difference is that this series defines and implements > MmServicesTableLib in such a way that the traditional SMM drivers > can use it as well. This is appropriate, considering that the PI > spec has rebranded traditional SMM as one implementation of the generic > MM framework. > > Patch #1 is based on Jagadeesh's patch, and introduces the MmServicesTableLib > library class, but for all SMM flavours, not only for standalone MM. > > Patch #2 implements MmServicesTableLib for traditional SMM implementations. > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > driver that invoke boot services are separated from the core SMM pieces. > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM environment. > > Patches #5 and #6 do the same, respectively, for the variable runtime driver. > > This approach minimizes the delta, and thus the maintenance burden, between > the traditional SMM and standalone MM drivers, while not resorting to runtime > checks or other conditionals in the code to implement logic that should be > decided at build time. > > Note that this series only covers part of the work contributed by Jagadeesh. > This series focuses on the MdePkg and MdeModulePkg changes that affect shared > code. > > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Hao Wu <hao.a.wu@intel.com> > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > Cc: Achin Gupta <Achin.Gupta@arm.com> > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > > Ard Biesheuvel (5): > MdePkg: implement MmServicesTableLib based on traditional SMM > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version > MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses > MdeModulePkg/VariableRuntimeDxe: implement standalone MM version > > Jagadeesh Ujja (1): > MdePkg/Include: add MmServicesTableLib header file > > MdeModulePkg/MdeModulePkg.dsc | 1 + > .../FaultTolerantWrite.h | 22 ++- > .../FaultTolerantWriteDxe.c | 31 ++++ > .../FaultTolerantWriteSmm.c | 54 +++---- > .../FaultTolerantWriteSmm.inf | 5 +- > .../FaultTolerantWriteSmmCommon.h | 31 ++++ > .../FaultTolerantWriteSmmDxe.c | 1 + > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ > .../UpdateWorkingBlock.c | 10 +- > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ > .../RuntimeDxe/VariableStandaloneMm.inf | 135 ++++++++++++++++++ > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ > MdePkg/MdePkg.dec | 4 + > MdePkg/MdePkg.dsc | 1 + > 24 files changed, 916 insertions(+), 103 deletions(-) > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni > > -- > 2.17.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Mon, 7 Jan 2019 at 13:44, Gao, Liming <liming.gao@intel.com> wrote: > > Ard: > I agree this design is good. But, I need some time to evaluate its impact on our X86 platform. Could you wait for several days? > Of course. Thanks, > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > Sent: Friday, January 4, 2019 2:28 AM > > To: edk2-devel@lists.01.org > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, > > Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A > > <hao.a.wu@intel.com>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam > > Abraham <thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com> > > Subject: [PATCH 0/6] implement standalone MM versions of the variable runtime drivers > > > > This series proposed an alternative approach to the series sent out by > > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > > special PCD, as well as some other if() conditionals. > > > > The primary difference is that this series defines and implements > > MmServicesTableLib in such a way that the traditional SMM drivers > > can use it as well. This is appropriate, considering that the PI > > spec has rebranded traditional SMM as one implementation of the generic > > MM framework. > > > > Patch #1 is based on Jagadeesh's patch, and introduces the MmServicesTableLib > > library class, but for all SMM flavours, not only for standalone MM. > > > > Patch #2 implements MmServicesTableLib for traditional SMM implementations. > > > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > > driver that invoke boot services are separated from the core SMM pieces. > > > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM environment. > > > > Patches #5 and #6 do the same, respectively, for the variable runtime driver. > > > > This approach minimizes the delta, and thus the maintenance burden, between > > the traditional SMM and standalone MM drivers, while not resorting to runtime > > checks or other conditionals in the code to implement logic that should be > > decided at build time. > > > > Note that this series only covers part of the work contributed by Jagadeesh. > > This series focuses on the MdePkg and MdeModulePkg changes that affect shared > > code. > > > > Cc: Laszlo Ersek <lersek@redhat.com> > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > Cc: Liming Gao <liming.gao@intel.com> > > Cc: Jian J Wang <jian.j.wang@intel.com> > > Cc: Hao Wu <hao.a.wu@intel.com> > > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > > Cc: Achin Gupta <Achin.Gupta@arm.com> > > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > > > > Ard Biesheuvel (5): > > MdePkg: implement MmServicesTableLib based on traditional SMM > > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses > > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version > > MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses > > MdeModulePkg/VariableRuntimeDxe: implement standalone MM version > > > > Jagadeesh Ujja (1): > > MdePkg/Include: add MmServicesTableLib header file > > > > MdeModulePkg/MdeModulePkg.dsc | 1 + > > .../FaultTolerantWrite.h | 22 ++- > > .../FaultTolerantWriteDxe.c | 31 ++++ > > .../FaultTolerantWriteSmm.c | 54 +++---- > > .../FaultTolerantWriteSmm.inf | 5 +- > > .../FaultTolerantWriteSmmCommon.h | 31 ++++ > > .../FaultTolerantWriteSmmDxe.c | 1 + > > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ > > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ > > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ > > .../UpdateWorkingBlock.c | 10 +- > > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- > > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ > > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- > > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- > > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ > > .../RuntimeDxe/VariableStandaloneMm.inf | 135 ++++++++++++++++++ > > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ > > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ > > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ > > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ > > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ > > MdePkg/MdePkg.dec | 4 + > > MdePkg/MdePkg.dsc | 1 + > > 24 files changed, 916 insertions(+), 103 deletions(-) > > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c > > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf > > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c > > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c > > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c > > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h > > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c > > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni > > > > -- > > 2.17.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 01/07/19 14:05, Ard Biesheuvel wrote: > On Mon, 7 Jan 2019 at 13:44, Gao, Liming <liming.gao@intel.com> wrote: >> >> Ard: >> I agree this design is good. But, I need some time to evaluate its impact on our X86 platform. Could you wait for several days? >> > > Of course. I think it would be prudent of me to at least regression-test this series. Adding it to my queue... It's likely that I won't be too quick. :/ Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 01/03/19 19:28, Ard Biesheuvel wrote: > This series proposed an alternative approach to the series sent out by > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > special PCD, as well as some other if() conditionals. > > The primary difference is that this series defines and implements > MmServicesTableLib in such a way that the traditional SMM drivers can > use it as well. This is appropriate, considering that the PI spec has > rebranded traditional SMM as one implementation of the generic MM > framework. > > Patch #1 is based on Jagadeesh's patch, and introduces the > MmServicesTableLib library class, but for all SMM flavours, not only > for standalone MM. > > Patch #2 implements MmServicesTableLib for traditional SMM > implementations. > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > driver that invoke boot services are separated from the core SMM > pieces. > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM > environment. > > Patches #5 and #6 do the same, respectively, for the variable runtime > driver. > > This approach minimizes the delta, and thus the maintenance burden, > between the traditional SMM and standalone MM drivers, while not > resorting to runtime checks or other conditionals in the code to > implement logic that should be decided at build time. > > Note that this series only covers part of the work contributed by > Jagadeesh. This series focuses on the MdePkg and MdeModulePkg changes > that affect shared code. > > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Hao Wu <hao.a.wu@intel.com> > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > Cc: Achin Gupta <Achin.Gupta@arm.com> > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > Cc: Sami Mujawar <Sami.Mujawar@arm.com> I tried building this, on top of commit a53a888de8f5: build \ -a IA32 \ -p OvmfPkg/OvmfPkgIa32.dsc \ -t GCC48 \ -b NOOPT \ -D HTTP_BOOT_ENABLE \ -D NETWORK_IP6_ENABLE \ -D SECURE_BOOT_ENABLE \ -D SMM_REQUIRE \ -D TLS_ENABLE \ --cmd-len=65536 \ --hash \ --genfds-multi-thread but it failed with: > OvmfPkg/OvmfPkgIa32.dsc(...): error 4000: Instance of library class [MmServicesTableLib] is not found > in [MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf] [IA32] > consumed by module [MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf] You did mention earlier that adding new lib class resolutions to several x86 DSC files would be necessary, so this is not unexpected. Can you please insert such a patch for OvmfPkg between patches #2 and #3? I've looked at the current OVMF DSC files, and SmmServicesTableLib is resolved for two module types, > [LibraryClasses.common.DXE_SMM_DRIVER] > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > > [LibraryClasses.common.SMM_CORE] > SmmServicesTableLib|MdeModulePkg/Library/PiSmmCoreSmmServicesTableLib/PiSmmCoreSmmServicesTableLib.inf I assume it should be enough, for this series, to update the DXE_SMM_DRIVER resolution only, and to leave SMM_CORE alone. (Because, my understanding is that the current, x86 specific MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf module, of type SMM_CORE, will not be refactored; instead, it is entirely supplanted -- in the affected platforms -- by the StandaloneMmPkg/Core/StandaloneMmCore.inf module, which is of type MM_CORE_STANDALONE.) But, it's still not clear to me (without trying) whether I should resolve MmServicesTableLib for DXE_SMM_DRIVER in addition to SmmServicesTableLib, or in its place. I'd prefer not experimenting with this myself; I'd just like to apply the series, and build & test it. :) Thanks, Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, 9 Jan 2019 at 10:44, Laszlo Ersek <lersek@redhat.com> wrote: > > On 01/03/19 19:28, Ard Biesheuvel wrote: > > This series proposed an alternative approach to the series sent out by > > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > > special PCD, as well as some other if() conditionals. > > > > The primary difference is that this series defines and implements > > MmServicesTableLib in such a way that the traditional SMM drivers can > > use it as well. This is appropriate, considering that the PI spec has > > rebranded traditional SMM as one implementation of the generic MM > > framework. > > > > Patch #1 is based on Jagadeesh's patch, and introduces the > > MmServicesTableLib library class, but for all SMM flavours, not only > > for standalone MM. > > > > Patch #2 implements MmServicesTableLib for traditional SMM > > implementations. > > > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > > driver that invoke boot services are separated from the core SMM > > pieces. > > > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM > > environment. > > > > Patches #5 and #6 do the same, respectively, for the variable runtime > > driver. > > > > This approach minimizes the delta, and thus the maintenance burden, > > between the traditional SMM and standalone MM drivers, while not > > resorting to runtime checks or other conditionals in the code to > > implement logic that should be decided at build time. > > > > Note that this series only covers part of the work contributed by > > Jagadeesh. This series focuses on the MdePkg and MdeModulePkg changes > > that affect shared code. > > > > Cc: Laszlo Ersek <lersek@redhat.com> > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > Cc: Liming Gao <liming.gao@intel.com> > > Cc: Jian J Wang <jian.j.wang@intel.com> > > Cc: Hao Wu <hao.a.wu@intel.com> > > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > > Cc: Achin Gupta <Achin.Gupta@arm.com> > > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > > I tried building this, on top of commit a53a888de8f5: > > build \ > -a IA32 \ > -p OvmfPkg/OvmfPkgIa32.dsc \ > -t GCC48 \ > -b NOOPT \ > -D HTTP_BOOT_ENABLE \ > -D NETWORK_IP6_ENABLE \ > -D SECURE_BOOT_ENABLE \ > -D SMM_REQUIRE \ > -D TLS_ENABLE \ > --cmd-len=65536 \ > --hash \ > --genfds-multi-thread > > but it failed with: > > > OvmfPkg/OvmfPkgIa32.dsc(...): error 4000: Instance of library class [MmServicesTableLib] is not found > > in [MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf] [IA32] > > consumed by module [MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf] > > You did mention earlier that adding new lib class resolutions to several > x86 DSC files would be necessary, so this is not unexpected. Can you > please insert such a patch for OvmfPkg between patches #2 and #3? > Ah yes, of course. > I've looked at the current OVMF DSC files, and SmmServicesTableLib is > resolved for two module types, > > > [LibraryClasses.common.DXE_SMM_DRIVER] > > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > > > > [LibraryClasses.common.SMM_CORE] > > SmmServicesTableLib|MdeModulePkg/Library/PiSmmCoreSmmServicesTableLib/PiSmmCoreSmmServicesTableLib.inf > > I assume it should be enough, for this series, to update the > DXE_SMM_DRIVER resolution only, and to leave SMM_CORE alone. > > (Because, my understanding is that the current, x86 specific > > MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf > > module, of type SMM_CORE, will not be refactored; instead, it is > entirely supplanted -- in the affected platforms -- by the > > StandaloneMmPkg/Core/StandaloneMmCore.inf > > module, which is of type MM_CORE_STANDALONE.) > > But, it's still not clear to me (without trying) whether I should > resolve MmServicesTableLib for DXE_SMM_DRIVER in addition to > SmmServicesTableLib, or in its place. I'd prefer not experimenting with > this myself; I'd just like to apply the series, and build & test it. :) > At the moment, you will need both resolutions for DXE_SMM_DRIVERS globally. Based on the outcome of the review of this series, this is something we will need to clean up going forward, but currently, even the drivers that are updated to use MmServicesTableLib pull in libraries that depend on SmmServicesTableLib. This should be a rather straight-forward search/replace operation [famous last words], and I can commit to dedicating some of my time to getting this fixed throughout, at least to the point where modules that consume MmServicesTableLib no longer have to supply a resolution for SmmServicesTableLib as well. So, I will include a patch in the next revision of the series. In the mean time, the hunk below should suffice to complete your regression testing. --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -391,6 +391,7 @@ HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf + MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf !ifdef $(DEBUG_ON_SERIAL_PORT) DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf !else _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Ard: Now, the impact is to update platform DSC to include MmServicesTableLib library instance. This change is acceptable for me. I suggest your create one BZ for this patch set. Besides, I can't apply for these patches in my machine. Could you share git branch to me? Then, I can further verify its functionality on SMM mode. Thanks Liming > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Monday, January 7, 2019 9:06 PM > To: Gao, Liming <liming.gao@intel.com> > Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D > <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Jagadeesh Ujja > <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam Abraham <thomas.abraham@arm.com>; > Sami Mujawar <Sami.Mujawar@arm.com> > Subject: Re: [PATCH 0/6] implement standalone MM versions of the variable runtime drivers > > On Mon, 7 Jan 2019 at 13:44, Gao, Liming <liming.gao@intel.com> wrote: > > > > Ard: > > I agree this design is good. But, I need some time to evaluate its impact on our X86 platform. Could you wait for several days? > > > > Of course. > > Thanks, > > > > -----Original Message----- > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > > Sent: Friday, January 4, 2019 2:28 AM > > > To: edk2-devel@lists.01.org > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; > Kinney, > > > Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A > > > <hao.a.wu@intel.com>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam > > > Abraham <thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com> > > > Subject: [PATCH 0/6] implement standalone MM versions of the variable runtime drivers > > > > > > This series proposed an alternative approach to the series sent out by > > > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > > > special PCD, as well as some other if() conditionals. > > > > > > The primary difference is that this series defines and implements > > > MmServicesTableLib in such a way that the traditional SMM drivers > > > can use it as well. This is appropriate, considering that the PI > > > spec has rebranded traditional SMM as one implementation of the generic > > > MM framework. > > > > > > Patch #1 is based on Jagadeesh's patch, and introduces the MmServicesTableLib > > > library class, but for all SMM flavours, not only for standalone MM. > > > > > > Patch #2 implements MmServicesTableLib for traditional SMM implementations. > > > > > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > > > driver that invoke boot services are separated from the core SMM pieces. > > > > > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM environment. > > > > > > Patches #5 and #6 do the same, respectively, for the variable runtime driver. > > > > > > This approach minimizes the delta, and thus the maintenance burden, between > > > the traditional SMM and standalone MM drivers, while not resorting to runtime > > > checks or other conditionals in the code to implement logic that should be > > > decided at build time. > > > > > > Note that this series only covers part of the work contributed by Jagadeesh. > > > This series focuses on the MdePkg and MdeModulePkg changes that affect shared > > > code. > > > > > > Cc: Laszlo Ersek <lersek@redhat.com> > > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > > Cc: Liming Gao <liming.gao@intel.com> > > > Cc: Jian J Wang <jian.j.wang@intel.com> > > > Cc: Hao Wu <hao.a.wu@intel.com> > > > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > > > Cc: Achin Gupta <Achin.Gupta@arm.com> > > > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > > > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > > > > > > Ard Biesheuvel (5): > > > MdePkg: implement MmServicesTableLib based on traditional SMM > > > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses > > > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version > > > MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses > > > MdeModulePkg/VariableRuntimeDxe: implement standalone MM version > > > > > > Jagadeesh Ujja (1): > > > MdePkg/Include: add MmServicesTableLib header file > > > > > > MdeModulePkg/MdeModulePkg.dsc | 1 + > > > .../FaultTolerantWrite.h | 22 ++- > > > .../FaultTolerantWriteDxe.c | 31 ++++ > > > .../FaultTolerantWriteSmm.c | 54 +++---- > > > .../FaultTolerantWriteSmm.inf | 5 +- > > > .../FaultTolerantWriteSmmCommon.h | 31 ++++ > > > .../FaultTolerantWriteSmmDxe.c | 1 + > > > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ > > > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ > > > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ > > > .../UpdateWorkingBlock.c | 10 +- > > > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- > > > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ > > > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- > > > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- > > > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ > > > .../RuntimeDxe/VariableStandaloneMm.inf | 135 ++++++++++++++++++ > > > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ > > > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ > > > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ > > > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ > > > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ > > > MdePkg/MdePkg.dec | 4 + > > > MdePkg/MdePkg.dsc | 1 + > > > 24 files changed, 916 insertions(+), 103 deletions(-) > > > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c > > > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf > > > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c > > > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c > > > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > > > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c > > > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h > > > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c > > > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > > > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni > > > > > > -- > > > 2.17.1 > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 01/09/19 11:28, Ard Biesheuvel wrote: > On Wed, 9 Jan 2019 at 10:44, Laszlo Ersek <lersek@redhat.com> wrote: >> But, it's still not clear to me (without trying) whether I should >> resolve MmServicesTableLib for DXE_SMM_DRIVER in addition to >> SmmServicesTableLib, or in its place. I'd prefer not experimenting with >> this myself; I'd just like to apply the series, and build & test it. :) >> > > At the moment, you will need both resolutions for DXE_SMM_DRIVERS > globally. Based on the outcome of the review of this series, this is > something we will need to clean up going forward, but currently, even > the drivers that are updated to use MmServicesTableLib pull in > libraries that depend on SmmServicesTableLib. > > This should be a rather straight-forward search/replace operation > [famous last words], and I can commit to dedicating some of my time to > getting this fixed throughout, at least to the point where modules > that consume MmServicesTableLib no longer have to supply a resolution > for SmmServicesTableLib as well. > > So, I will include a patch in the next revision of the series. Great, thank you. This is exactly the info I needed. > In the mean time, the hunk below should suffice to complete your regression > testing. > > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -391,6 +391,7 @@ > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > + MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > !ifdef $(DEBUG_ON_SERIAL_PORT) > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > !else > I'll replicate this to the other two DSC files [*], and then retry the tests. [*] SMM in OVMF has a non-intuitive restriction, in relation to X64 PEI. SMM "just works" in the IA32 and IA32X64 builds, however, in the X64 build, one has to disable S3 support on the QEMU command line, or else we hang the boot intentionally. See commit 5133d1f1d297 ("OvmfPkg: replace README fine print about X64 SMM S3 with PlatformPei check", 2015-11-30). For this reason, the IA32X64 build is considered the most-featureful, if -D SMM_REQUIRE is desired. For those that insist on the X64 build nevertheless, OvmfPkg/README documents the QEMU option that disables S3, on the Q35 machine type, which is required for SMM anyway: -global ICH9-LPC.disable_s3=1 When using libvirt, the matching knob is: https://libvirt.org/formatdomain.html#elementsPowerManagement <pm> <suspend-to-mem enabled='no'/> </pm> Personally, I focus my SMM testing on IA32 and IA32X64; I almost never test SMM in the X64 build. This is because S3 has historically proved very effective at triggering multiprocessing bugs in core SMM code, and in general UefiCpuPkg infrastructure. Thus, my SMM regression tests: https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt always include S3 suspend/resume, and that precludes the X64 build of OVMF. ... Sorry about the wall of text, I just wanted to explain why precisely the short hunk you pasted is unhelpful in this case. Obviously, it does answer my question, I can copy the class resolution to the other two DSC files, and in the final OvmfPkg patch, we should update all three DSC files. I'll be back with test results later. Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, 9 Jan 2019 at 14:56, Gao, Liming <liming.gao@intel.com> wrote: > > Ard: > Now, the impact is to update platform DSC to include MmServicesTableLib library instance. This change is acceptable for me. I suggest your create one BZ for this patch set. https://bugzilla.tianocore.org/show_bug.cgi?id=1442 > Besides, I can't apply for these patches in my machine. Could you share git branch to me? Then, I can further verify its functionality on SMM mode. > https://github.com/ardbiesheuvel/edk2/tree/variable-ftw-standalone-mm-conversion Note that I included the changes to add the MmServicesTableLib resolution to consumers of the FTW and variable drivers. Thanks, Ard. > Thanks > Liming > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > Sent: Monday, January 7, 2019 9:06 PM > > To: Gao, Liming <liming.gao@intel.com> > > Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D > > <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Jagadeesh Ujja > > <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam Abraham <thomas.abraham@arm.com>; > > Sami Mujawar <Sami.Mujawar@arm.com> > > Subject: Re: [PATCH 0/6] implement standalone MM versions of the variable runtime drivers > > > > On Mon, 7 Jan 2019 at 13:44, Gao, Liming <liming.gao@intel.com> wrote: > > > > > > Ard: > > > I agree this design is good. But, I need some time to evaluate its impact on our X86 platform. Could you wait for several days? > > > > > > > Of course. > > > > Thanks, > > > > > > -----Original Message----- > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > > > Sent: Friday, January 4, 2019 2:28 AM > > > > To: edk2-devel@lists.01.org > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; > > Kinney, > > > > Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A > > > > <hao.a.wu@intel.com>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam > > > > Abraham <thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com> > > > > Subject: [PATCH 0/6] implement standalone MM versions of the variable runtime drivers > > > > > > > > This series proposed an alternative approach to the series sent out by > > > > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > > > > special PCD, as well as some other if() conditionals. > > > > > > > > The primary difference is that this series defines and implements > > > > MmServicesTableLib in such a way that the traditional SMM drivers > > > > can use it as well. This is appropriate, considering that the PI > > > > spec has rebranded traditional SMM as one implementation of the generic > > > > MM framework. > > > > > > > > Patch #1 is based on Jagadeesh's patch, and introduces the MmServicesTableLib > > > > library class, but for all SMM flavours, not only for standalone MM. > > > > > > > > Patch #2 implements MmServicesTableLib for traditional SMM implementations. > > > > > > > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > > > > driver that invoke boot services are separated from the core SMM pieces. > > > > > > > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM environment. > > > > > > > > Patches #5 and #6 do the same, respectively, for the variable runtime driver. > > > > > > > > This approach minimizes the delta, and thus the maintenance burden, between > > > > the traditional SMM and standalone MM drivers, while not resorting to runtime > > > > checks or other conditionals in the code to implement logic that should be > > > > decided at build time. > > > > > > > > Note that this series only covers part of the work contributed by Jagadeesh. > > > > This series focuses on the MdePkg and MdeModulePkg changes that affect shared > > > > code. > > > > > > > > Cc: Laszlo Ersek <lersek@redhat.com> > > > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > > > Cc: Liming Gao <liming.gao@intel.com> > > > > Cc: Jian J Wang <jian.j.wang@intel.com> > > > > Cc: Hao Wu <hao.a.wu@intel.com> > > > > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > > > > Cc: Achin Gupta <Achin.Gupta@arm.com> > > > > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > > > > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > > > > > > > > Ard Biesheuvel (5): > > > > MdePkg: implement MmServicesTableLib based on traditional SMM > > > > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses > > > > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version > > > > MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses > > > > MdeModulePkg/VariableRuntimeDxe: implement standalone MM version > > > > > > > > Jagadeesh Ujja (1): > > > > MdePkg/Include: add MmServicesTableLib header file > > > > > > > > MdeModulePkg/MdeModulePkg.dsc | 1 + > > > > .../FaultTolerantWrite.h | 22 ++- > > > > .../FaultTolerantWriteDxe.c | 31 ++++ > > > > .../FaultTolerantWriteSmm.c | 54 +++---- > > > > .../FaultTolerantWriteSmm.inf | 5 +- > > > > .../FaultTolerantWriteSmmCommon.h | 31 ++++ > > > > .../FaultTolerantWriteSmmDxe.c | 1 + > > > > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ > > > > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ > > > > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ > > > > .../UpdateWorkingBlock.c | 10 +- > > > > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- > > > > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ > > > > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- > > > > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- > > > > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ > > > > .../RuntimeDxe/VariableStandaloneMm.inf | 135 ++++++++++++++++++ > > > > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ > > > > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ > > > > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ > > > > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ > > > > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ > > > > MdePkg/MdePkg.dec | 4 + > > > > MdePkg/MdePkg.dsc | 1 + > > > > 24 files changed, 916 insertions(+), 103 deletions(-) > > > > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c > > > > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf > > > > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c > > > > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c > > > > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > > > > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c > > > > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h > > > > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c > > > > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > > > > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni > > > > > > > > -- > > > > 2.17.1 > > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 01/09/19 16:04, Laszlo Ersek wrote: > On 01/09/19 11:28, Ard Biesheuvel wrote: >> In the mean time, the hunk below should suffice to complete your >> regression testing. I used: > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 3f3533e5c163..908450eda174 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -386,6 +386,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER] > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > + MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > !ifdef $(DEBUG_ON_SERIAL_PORT) > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > !else > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 6c08b2728d63..14166e946a93 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -391,6 +391,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER] > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > + MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > !ifdef $(DEBUG_ON_SERIAL_PORT) > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > !else > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 4072c839d73f..0cd5f76cccd9 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -391,6 +391,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER] > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > + MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > !ifdef $(DEBUG_ON_SERIAL_PORT) > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > !else On 01/09/19 16:04, Laszlo Ersek wrote: > I'll be back with test results later. Regression-tested-by: Laszlo Ersek <lersek@redhat.com> Thanks, Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, 9 Jan 2019 at 22:46, Laszlo Ersek <lersek@redhat.com> wrote: > > On 01/09/19 16:04, Laszlo Ersek wrote: > > On 01/09/19 11:28, Ard Biesheuvel wrote: > >> In the mean time, the hunk below should suffice to complete your > >> regression testing. > > I used: > > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > > index 3f3533e5c163..908450eda174 100644 > > --- a/OvmfPkg/OvmfPkgIa32.dsc > > +++ b/OvmfPkg/OvmfPkgIa32.dsc > > @@ -386,6 +386,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER] > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > > + MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > > !ifdef $(DEBUG_ON_SERIAL_PORT) > > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > > !else > > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > > index 6c08b2728d63..14166e946a93 100644 > > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > > @@ -391,6 +391,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER] > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > > + MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > > !ifdef $(DEBUG_ON_SERIAL_PORT) > > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > > !else > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > > index 4072c839d73f..0cd5f76cccd9 100644 > > --- a/OvmfPkg/OvmfPkgX64.dsc > > +++ b/OvmfPkg/OvmfPkgX64.dsc > > @@ -391,6 +391,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER] > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf > > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf > > + MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > > !ifdef $(DEBUG_ON_SERIAL_PORT) > > DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf > > !else > > On 01/09/19 16:04, Laszlo Ersek wrote: > > I'll be back with test results later. > > Regression-tested-by: Laszlo Ersek <lersek@redhat.com> > Wonderful! Thanks a lot Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
We'd better have a bugzilla to track this change. And since it will require platform change in platform dsc to add the new library mapping, we need add notes in https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Notes. Thanks, Star On 2019/1/4 2:28, Ard Biesheuvel wrote: > This series proposed an alternative approach to the series sent out by > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > special PCD, as well as some other if() conditionals. > > The primary difference is that this series defines and implements > MmServicesTableLib in such a way that the traditional SMM drivers > can use it as well. This is appropriate, considering that the PI > spec has rebranded traditional SMM as one implementation of the generic > MM framework. > > Patch #1 is based on Jagadeesh's patch, and introduces the MmServicesTableLib > library class, but for all SMM flavours, not only for standalone MM. > > Patch #2 implements MmServicesTableLib for traditional SMM implementations. > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > driver that invoke boot services are separated from the core SMM pieces. > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM environment. > > Patches #5 and #6 do the same, respectively, for the variable runtime driver. > > This approach minimizes the delta, and thus the maintenance burden, between > the traditional SMM and standalone MM drivers, while not resorting to runtime > checks or other conditionals in the code to implement logic that should be > decided at build time. > > Note that this series only covers part of the work contributed by Jagadeesh. > This series focuses on the MdePkg and MdeModulePkg changes that affect shared > code. > > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Hao Wu <hao.a.wu@intel.com> > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > Cc: Achin Gupta <Achin.Gupta@arm.com> > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > > Ard Biesheuvel (5): > MdePkg: implement MmServicesTableLib based on traditional SMM > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version > MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses > MdeModulePkg/VariableRuntimeDxe: implement standalone MM version > > Jagadeesh Ujja (1): > MdePkg/Include: add MmServicesTableLib header file > > MdeModulePkg/MdeModulePkg.dsc | 1 + > .../FaultTolerantWrite.h | 22 ++- > .../FaultTolerantWriteDxe.c | 31 ++++ > .../FaultTolerantWriteSmm.c | 54 +++---- > .../FaultTolerantWriteSmm.inf | 5 +- > .../FaultTolerantWriteSmmCommon.h | 31 ++++ > .../FaultTolerantWriteSmmDxe.c | 1 + > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ > .../UpdateWorkingBlock.c | 10 +- > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ > .../RuntimeDxe/VariableStandaloneMm.inf | 135 ++++++++++++++++++ > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ > MdePkg/MdePkg.dec | 4 + > MdePkg/MdePkg.dsc | 1 + > 24 files changed, 916 insertions(+), 103 deletions(-) > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Ard, FYI: There is minor change overlap to VariableDxe/Smm.c between this patch series and the patch series at https://lists.01.org/pipermail/edk2-devel/2019-January/034921.html ([PATCH 04/12]) I just sent. After one patch series is pushed, the other patch series will need a simple rebase. Thanks, Star On 2019/1/4 2:28, Ard Biesheuvel wrote: > This series proposed an alternative approach to the series sent out by > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > special PCD, as well as some other if() conditionals. > > The primary difference is that this series defines and implements > MmServicesTableLib in such a way that the traditional SMM drivers > can use it as well. This is appropriate, considering that the PI > spec has rebranded traditional SMM as one implementation of the generic > MM framework. > > Patch #1 is based on Jagadeesh's patch, and introduces the MmServicesTableLib > library class, but for all SMM flavours, not only for standalone MM. > > Patch #2 implements MmServicesTableLib for traditional SMM implementations. > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > driver that invoke boot services are separated from the core SMM pieces. > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM environment. > > Patches #5 and #6 do the same, respectively, for the variable runtime driver. > > This approach minimizes the delta, and thus the maintenance burden, between > the traditional SMM and standalone MM drivers, while not resorting to runtime > checks or other conditionals in the code to implement logic that should be > decided at build time. > > Note that this series only covers part of the work contributed by Jagadeesh. > This series focuses on the MdePkg and MdeModulePkg changes that affect shared > code. > > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Hao Wu <hao.a.wu@intel.com> > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > Cc: Achin Gupta <Achin.Gupta@arm.com> > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > > Ard Biesheuvel (5): > MdePkg: implement MmServicesTableLib based on traditional SMM > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service accesses > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM version > MdeModulePkg/VariableRuntimeDxe: factor out boot service accesses > MdeModulePkg/VariableRuntimeDxe: implement standalone MM version > > Jagadeesh Ujja (1): > MdePkg/Include: add MmServicesTableLib header file > > MdeModulePkg/MdeModulePkg.dsc | 1 + > .../FaultTolerantWrite.h | 22 ++- > .../FaultTolerantWriteDxe.c | 31 ++++ > .../FaultTolerantWriteSmm.c | 54 +++---- > .../FaultTolerantWriteSmm.inf | 5 +- > .../FaultTolerantWriteSmmCommon.h | 31 ++++ > .../FaultTolerantWriteSmmDxe.c | 1 + > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ > .../UpdateWorkingBlock.c | 10 +- > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ > .../RuntimeDxe/VariableStandaloneMm.inf | 135 ++++++++++++++++++ > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ > MdePkg/MdePkg.dec | 4 + > MdePkg/MdePkg.dsc | 1 + > 24 files changed, 916 insertions(+), 103 deletions(-) > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.c > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf > create mode 100644 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditionalMm.c > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > create mode 100644 MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Ard: I don't find the function issue in this patch. I have no other comments for the change in MdePkg. Reviewed-by: Liming Gao <liming.gao@intel.com>. For this patch set, if you push the change, please push the patches in MdePkg first, and tell me the revision. I will update our internal platform DSC to include new MmServicesTableLib library instance. After I am done, I will let you know. Then, you can continue to push the change in MdeModulePkg. Is it OK? I see you will continue to look add MmStandaloneEntryPointLib and MmServiceLib for MmStandalone driver. You can create another BZ for it. I will review them once you are done. Thanks Liming >-----Original Message----- >From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >Sent: Wednesday, January 09, 2019 11:30 PM >To: Gao, Liming <liming.gao@intel.com> >Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm ><leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; >Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; >Jagadeesh Ujja <jagadeesh.ujja@arm.com>; Achin Gupta ><Achin.Gupta@arm.com>; Thomas Panakamattam Abraham ><thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com> >Subject: Re: [PATCH 0/6] implement standalone MM versions of the variable >runtime drivers > >On Wed, 9 Jan 2019 at 14:56, Gao, Liming <liming.gao@intel.com> wrote: >> >> Ard: >> Now, the impact is to update platform DSC to include MmServicesTableLib >library instance. This change is acceptable for me. I suggest your create one BZ >for this patch set. > >https://bugzilla.tianocore.org/show_bug.cgi?id=1442 > >> Besides, I can't apply for these patches in my machine. Could you share git >branch to me? Then, I can further verify its functionality on SMM mode. >> > >https://github.com/ardbiesheuvel/edk2/tree/variable-ftw-standalone-mm- >conversion > >Note that I included the changes to add the MmServicesTableLib >resolution to consumers of the FTW and variable drivers. > >Thanks, >Ard. > > > >> Thanks >> Liming >> > -----Original Message----- >> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> > Sent: Monday, January 7, 2019 9:06 PM >> > To: Gao, Liming <liming.gao@intel.com> >> > Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Leif >Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D >> > <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, >Hao A <hao.a.wu@intel.com>; Jagadeesh Ujja >> > <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; >Thomas Panakamattam Abraham <thomas.abraham@arm.com>; >> > Sami Mujawar <Sami.Mujawar@arm.com> >> > Subject: Re: [PATCH 0/6] implement standalone MM versions of the >variable runtime drivers >> > >> > On Mon, 7 Jan 2019 at 13:44, Gao, Liming <liming.gao@intel.com> wrote: >> > > >> > > Ard: >> > > I agree this design is good. But, I need some time to evaluate its impact >on our X86 platform. Could you wait for several days? >> > > >> > >> > Of course. >> > >> > Thanks, >> > >> > > > -----Original Message----- >> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> > > > Sent: Friday, January 4, 2019 2:28 AM >> > > > To: edk2-devel@lists.01.org >> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek ><lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; >> > Kinney, >> > > > Michael D <michael.d.kinney@intel.com>; Gao, Liming ><liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A >> > > > <hao.a.wu@intel.com>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>; >Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam >> > > > Abraham <thomas.abraham@arm.com>; Sami Mujawar ><Sami.Mujawar@arm.com> >> > > > Subject: [PATCH 0/6] implement standalone MM versions of the >variable runtime drivers >> > > > >> > > > This series proposed an alternative approach to the series sent out by >> > > > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the >> > > > special PCD, as well as some other if() conditionals. >> > > > >> > > > The primary difference is that this series defines and implements >> > > > MmServicesTableLib in such a way that the traditional SMM drivers >> > > > can use it as well. This is appropriate, considering that the PI >> > > > spec has rebranded traditional SMM as one implementation of the >generic >> > > > MM framework. >> > > > >> > > > Patch #1 is based on Jagadeesh's patch, and introduces the >MmServicesTableLib >> > > > library class, but for all SMM flavours, not only for standalone MM. >> > > > >> > > > Patch #2 implements MmServicesTableLib for traditional SMM >implementations. >> > > > >> > > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM >> > > > driver that invoke boot services are separated from the core SMM >pieces. >> > > > >> > > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM >environment. >> > > > >> > > > Patches #5 and #6 do the same, respectively, for the variable runtime >driver. >> > > > >> > > > This approach minimizes the delta, and thus the maintenance burden, >between >> > > > the traditional SMM and standalone MM drivers, while not resorting to >runtime >> > > > checks or other conditionals in the code to implement logic that should >be >> > > > decided at build time. >> > > > >> > > > Note that this series only covers part of the work contributed by >Jagadeesh. >> > > > This series focuses on the MdePkg and MdeModulePkg changes that >affect shared >> > > > code. >> > > > >> > > > Cc: Laszlo Ersek <lersek@redhat.com> >> > > > Cc: Leif Lindholm <leif.lindholm@linaro.org> >> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> >> > > > Cc: Liming Gao <liming.gao@intel.com> >> > > > Cc: Jian J Wang <jian.j.wang@intel.com> >> > > > Cc: Hao Wu <hao.a.wu@intel.com> >> > > > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> >> > > > Cc: Achin Gupta <Achin.Gupta@arm.com> >> > > > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> >> > > > Cc: Sami Mujawar <Sami.Mujawar@arm.com> >> > > > >> > > > Ard Biesheuvel (5): >> > > > MdePkg: implement MmServicesTableLib based on traditional SMM >> > > > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service >accesses >> > > > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM >version >> > > > MdeModulePkg/VariableRuntimeDxe: factor out boot service >accesses >> > > > MdeModulePkg/VariableRuntimeDxe: implement standalone MM >version >> > > > >> > > > Jagadeesh Ujja (1): >> > > > MdePkg/Include: add MmServicesTableLib header file >> > > > >> > > > MdeModulePkg/MdeModulePkg.dsc | 1 + >> > > > .../FaultTolerantWrite.h | 22 ++- >> > > > .../FaultTolerantWriteDxe.c | 31 ++++ >> > > > .../FaultTolerantWriteSmm.c | 54 +++---- >> > > > .../FaultTolerantWriteSmm.inf | 5 +- >> > > > .../FaultTolerantWriteSmmCommon.h | 31 ++++ >> > > > .../FaultTolerantWriteSmmDxe.c | 1 + >> > > > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ >> > > > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ >> > > > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ >> > > > .../UpdateWorkingBlock.c | 10 +- >> > > > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- >> > > > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ >> > > > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- >> > > > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- >> > > > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ >> > > > .../RuntimeDxe/VariableStandaloneMm.inf | 135 >++++++++++++++++++ >> > > > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ >> > > > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ >> > > > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ >> > > > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ >> > > > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ >> > > > MdePkg/MdePkg.dec | 4 + >> > > > MdePkg/MdePkg.dsc | 1 + >> > > > 24 files changed, 916 insertions(+), 103 deletions(-) >> > > > create mode 100644 >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal >oneMm.c >> > > > create mode 100644 >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal >oneMm.inf >> > > > create mode 100644 >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditio >nalMm.c >> > > > create mode 100644 >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c >> > > > create mode 100644 >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf >> > > > create mode 100644 >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c >> > > > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h >> > > > create mode 100644 >MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c >> > > > create mode 100644 >MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf >> > > > create mode 100644 >MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni >> > > > >> > > > -- >> > > > 2.17.1 >> > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Mon, 14 Jan 2019 at 03:55, Gao, Liming <liming.gao@intel.com> wrote: > > Ard: > I don't find the function issue in this patch. I have no other comments for the change in MdePkg. Reviewed-by: Liming Gao <liming.gao@intel.com>. For this patch set, if you push the change, please push the patches in MdePkg first, and tell me the revision. I will update our internal platform DSC to include new MmServicesTableLib library instance. After I am done, I will let you know. Then, you can continue to push the change in MdeModulePkg. Is it OK? > Yes, that is fine. I will need to respin the remaining patches anyway. I have pushed the following patches b94aecb4ec94 MdePkg/Include: add MmServicesTableLib header file 17f5fd9291e0 MdePkg: implement MmServicesTableLib based on traditional SMM with Star's and Jian's feedback addressed, and your R-b's added. > I see you will continue to look add MmStandaloneEntryPointLib and MmServiceLib for MmStandalone driver. You can create another BZ for it. I will review them once you are done. > OK. > >-----Original Message----- > >From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > >Sent: Wednesday, January 09, 2019 11:30 PM > >To: Gao, Liming <liming.gao@intel.com> > >Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm > ><leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; > >Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; > >Jagadeesh Ujja <jagadeesh.ujja@arm.com>; Achin Gupta > ><Achin.Gupta@arm.com>; Thomas Panakamattam Abraham > ><thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com> > >Subject: Re: [PATCH 0/6] implement standalone MM versions of the variable > >runtime drivers > > > >On Wed, 9 Jan 2019 at 14:56, Gao, Liming <liming.gao@intel.com> wrote: > >> > >> Ard: > >> Now, the impact is to update platform DSC to include MmServicesTableLib > >library instance. This change is acceptable for me. I suggest your create one BZ > >for this patch set. > > > >https://bugzilla.tianocore.org/show_bug.cgi?id=1442 > > > >> Besides, I can't apply for these patches in my machine. Could you share git > >branch to me? Then, I can further verify its functionality on SMM mode. > >> > > > >https://github.com/ardbiesheuvel/edk2/tree/variable-ftw-standalone-mm- > >conversion > > > >Note that I included the changes to add the MmServicesTableLib > >resolution to consumers of the FTW and variable drivers. > > > >Thanks, > >Ard. > > > > > > > >> Thanks > >> Liming > >> > -----Original Message----- > >> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > >> > Sent: Monday, January 7, 2019 9:06 PM > >> > To: Gao, Liming <liming.gao@intel.com> > >> > Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Leif > >Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D > >> > <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, > >Hao A <hao.a.wu@intel.com>; Jagadeesh Ujja > >> > <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; > >Thomas Panakamattam Abraham <thomas.abraham@arm.com>; > >> > Sami Mujawar <Sami.Mujawar@arm.com> > >> > Subject: Re: [PATCH 0/6] implement standalone MM versions of the > >variable runtime drivers > >> > > >> > On Mon, 7 Jan 2019 at 13:44, Gao, Liming <liming.gao@intel.com> wrote: > >> > > > >> > > Ard: > >> > > I agree this design is good. But, I need some time to evaluate its impact > >on our X86 platform. Could you wait for several days? > >> > > > >> > > >> > Of course. > >> > > >> > Thanks, > >> > > >> > > > -----Original Message----- > >> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > >> > > > Sent: Friday, January 4, 2019 2:28 AM > >> > > > To: edk2-devel@lists.01.org > >> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek > ><lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; > >> > Kinney, > >> > > > Michael D <michael.d.kinney@intel.com>; Gao, Liming > ><liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A > >> > > > <hao.a.wu@intel.com>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>; > >Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam > >> > > > Abraham <thomas.abraham@arm.com>; Sami Mujawar > ><Sami.Mujawar@arm.com> > >> > > > Subject: [PATCH 0/6] implement standalone MM versions of the > >variable runtime drivers > >> > > > > >> > > > This series proposed an alternative approach to the series sent out by > >> > > > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > >> > > > special PCD, as well as some other if() conditionals. > >> > > > > >> > > > The primary difference is that this series defines and implements > >> > > > MmServicesTableLib in such a way that the traditional SMM drivers > >> > > > can use it as well. This is appropriate, considering that the PI > >> > > > spec has rebranded traditional SMM as one implementation of the > >generic > >> > > > MM framework. > >> > > > > >> > > > Patch #1 is based on Jagadeesh's patch, and introduces the > >MmServicesTableLib > >> > > > library class, but for all SMM flavours, not only for standalone MM. > >> > > > > >> > > > Patch #2 implements MmServicesTableLib for traditional SMM > >implementations. > >> > > > > >> > > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > >> > > > driver that invoke boot services are separated from the core SMM > >pieces. > >> > > > > >> > > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM > >environment. > >> > > > > >> > > > Patches #5 and #6 do the same, respectively, for the variable runtime > >driver. > >> > > > > >> > > > This approach minimizes the delta, and thus the maintenance burden, > >between > >> > > > the traditional SMM and standalone MM drivers, while not resorting to > >runtime > >> > > > checks or other conditionals in the code to implement logic that should > >be > >> > > > decided at build time. > >> > > > > >> > > > Note that this series only covers part of the work contributed by > >Jagadeesh. > >> > > > This series focuses on the MdePkg and MdeModulePkg changes that > >affect shared > >> > > > code. > >> > > > > >> > > > Cc: Laszlo Ersek <lersek@redhat.com> > >> > > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > >> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > >> > > > Cc: Liming Gao <liming.gao@intel.com> > >> > > > Cc: Jian J Wang <jian.j.wang@intel.com> > >> > > > Cc: Hao Wu <hao.a.wu@intel.com> > >> > > > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > >> > > > Cc: Achin Gupta <Achin.Gupta@arm.com> > >> > > > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > >> > > > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > >> > > > > >> > > > Ard Biesheuvel (5): > >> > > > MdePkg: implement MmServicesTableLib based on traditional SMM > >> > > > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service > >accesses > >> > > > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM > >version > >> > > > MdeModulePkg/VariableRuntimeDxe: factor out boot service > >accesses > >> > > > MdeModulePkg/VariableRuntimeDxe: implement standalone MM > >version > >> > > > > >> > > > Jagadeesh Ujja (1): > >> > > > MdePkg/Include: add MmServicesTableLib header file > >> > > > > >> > > > MdeModulePkg/MdeModulePkg.dsc | 1 + > >> > > > .../FaultTolerantWrite.h | 22 ++- > >> > > > .../FaultTolerantWriteDxe.c | 31 ++++ > >> > > > .../FaultTolerantWriteSmm.c | 54 +++---- > >> > > > .../FaultTolerantWriteSmm.inf | 5 +- > >> > > > .../FaultTolerantWriteSmmCommon.h | 31 ++++ > >> > > > .../FaultTolerantWriteSmmDxe.c | 1 + > >> > > > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ > >> > > > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ > >> > > > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ > >> > > > .../UpdateWorkingBlock.c | 10 +- > >> > > > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- > >> > > > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ > >> > > > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- > >> > > > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- > >> > > > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ > >> > > > .../RuntimeDxe/VariableStandaloneMm.inf | 135 > >++++++++++++++++++ > >> > > > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ > >> > > > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ > >> > > > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ > >> > > > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ > >> > > > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ > >> > > > MdePkg/MdePkg.dec | 4 + > >> > > > MdePkg/MdePkg.dsc | 1 + > >> > > > 24 files changed, 916 insertions(+), 103 deletions(-) > >> > > > create mode 100644 > >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal > >oneMm.c > >> > > > create mode 100644 > >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal > >oneMm.inf > >> > > > create mode 100644 > >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditio > >nalMm.c > >> > > > create mode 100644 > >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c > >> > > > create mode 100644 > >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > >> > > > create mode 100644 > >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c > >> > > > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h > >> > > > create mode 100644 > >MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c > >> > > > create mode 100644 > >MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > >> > > > create mode 100644 > >MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni > >> > > > > >> > > > -- > >> > > > 2.17.1 > >> > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Ard: Got it. I will update our internal platform dsc to include new MmServicesTableLib library. Besides, will you send the patch to update platform DSC files in edk2-platforms? If yes, please update DSCs in https://github.com/tianocore/edk2-platforms/tree/devel-MinPlatform. They both depend on edk2 master. Thanks Liming > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Monday, January 14, 2019 4:27 PM > To: Gao, Liming <liming.gao@intel.com> > Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D > <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Jagadeesh Ujja > <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam Abraham <thomas.abraham@arm.com>; > Sami Mujawar <Sami.Mujawar@arm.com> > Subject: Re: [PATCH 0/6] implement standalone MM versions of the variable runtime drivers > > On Mon, 14 Jan 2019 at 03:55, Gao, Liming <liming.gao@intel.com> wrote: > > > > Ard: > > I don't find the function issue in this patch. I have no other comments for the change in MdePkg. Reviewed-by: Liming Gao > <liming.gao@intel.com>. For this patch set, if you push the change, please push the patches in MdePkg first, and tell me the revision. I > will update our internal platform DSC to include new MmServicesTableLib library instance. After I am done, I will let you know. Then, > you can continue to push the change in MdeModulePkg. Is it OK? > > > > Yes, that is fine. I will need to respin the remaining patches anyway. > > I have pushed the following patches > > b94aecb4ec94 MdePkg/Include: add MmServicesTableLib header file > 17f5fd9291e0 MdePkg: implement MmServicesTableLib based on traditional SMM > > with Star's and Jian's feedback addressed, and your R-b's added. > > > > I see you will continue to look add MmStandaloneEntryPointLib and MmServiceLib for MmStandalone driver. You can create > another BZ for it. I will review them once you are done. > > > > OK. > > > >-----Original Message----- > > >From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > >Sent: Wednesday, January 09, 2019 11:30 PM > > >To: Gao, Liming <liming.gao@intel.com> > > >Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm > > ><leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; > > >Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; > > >Jagadeesh Ujja <jagadeesh.ujja@arm.com>; Achin Gupta > > ><Achin.Gupta@arm.com>; Thomas Panakamattam Abraham > > ><thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com> > > >Subject: Re: [PATCH 0/6] implement standalone MM versions of the variable > > >runtime drivers > > > > > >On Wed, 9 Jan 2019 at 14:56, Gao, Liming <liming.gao@intel.com> wrote: > > >> > > >> Ard: > > >> Now, the impact is to update platform DSC to include MmServicesTableLib > > >library instance. This change is acceptable for me. I suggest your create one BZ > > >for this patch set. > > > > > >https://bugzilla.tianocore.org/show_bug.cgi?id=1442 > > > > > >> Besides, I can't apply for these patches in my machine. Could you share git > > >branch to me? Then, I can further verify its functionality on SMM mode. > > >> > > > > > >https://github.com/ardbiesheuvel/edk2/tree/variable-ftw-standalone-mm- > > >conversion > > > > > >Note that I included the changes to add the MmServicesTableLib > > >resolution to consumers of the FTW and variable drivers. > > > > > >Thanks, > > >Ard. > > > > > > > > > > > >> Thanks > > >> Liming > > >> > -----Original Message----- > > >> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > >> > Sent: Monday, January 7, 2019 9:06 PM > > >> > To: Gao, Liming <liming.gao@intel.com> > > >> > Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Leif > > >Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D > > >> > <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, > > >Hao A <hao.a.wu@intel.com>; Jagadeesh Ujja > > >> > <jagadeesh.ujja@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; > > >Thomas Panakamattam Abraham <thomas.abraham@arm.com>; > > >> > Sami Mujawar <Sami.Mujawar@arm.com> > > >> > Subject: Re: [PATCH 0/6] implement standalone MM versions of the > > >variable runtime drivers > > >> > > > >> > On Mon, 7 Jan 2019 at 13:44, Gao, Liming <liming.gao@intel.com> wrote: > > >> > > > > >> > > Ard: > > >> > > I agree this design is good. But, I need some time to evaluate its impact > > >on our X86 platform. Could you wait for several days? > > >> > > > > >> > > > >> > Of course. > > >> > > > >> > Thanks, > > >> > > > >> > > > -----Original Message----- > > >> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > >> > > > Sent: Friday, January 4, 2019 2:28 AM > > >> > > > To: edk2-devel@lists.01.org > > >> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek > > ><lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; > > >> > Kinney, > > >> > > > Michael D <michael.d.kinney@intel.com>; Gao, Liming > > ><liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A > > >> > > > <hao.a.wu@intel.com>; Jagadeesh Ujja <jagadeesh.ujja@arm.com>; > > >Achin Gupta <Achin.Gupta@arm.com>; Thomas Panakamattam > > >> > > > Abraham <thomas.abraham@arm.com>; Sami Mujawar > > ><Sami.Mujawar@arm.com> > > >> > > > Subject: [PATCH 0/6] implement standalone MM versions of the > > >variable runtime drivers > > >> > > > > > >> > > > This series proposed an alternative approach to the series sent out by > > >> > > > Jagadeesh [0]. In particular, it gets rid of the InMm() calls and the > > >> > > > special PCD, as well as some other if() conditionals. > > >> > > > > > >> > > > The primary difference is that this series defines and implements > > >> > > > MmServicesTableLib in such a way that the traditional SMM drivers > > >> > > > can use it as well. This is appropriate, considering that the PI > > >> > > > spec has rebranded traditional SMM as one implementation of the > > >generic > > >> > > > MM framework. > > >> > > > > > >> > > > Patch #1 is based on Jagadeesh's patch, and introduces the > > >MmServicesTableLib > > >> > > > library class, but for all SMM flavours, not only for standalone MM. > > >> > > > > > >> > > > Patch #2 implements MmServicesTableLib for traditional SMM > > >implementations. > > >> > > > > > >> > > > Patch #3 refactors FaultTolerantWriteDxe so that the parts of the SMM > > >> > > > driver that invoke boot services are separated from the core SMM > > >pieces. > > >> > > > > > >> > > > Patch #4 implements FaultTolerantWriteSmm for the standalone MM > > >environment. > > >> > > > > > >> > > > Patches #5 and #6 do the same, respectively, for the variable runtime > > >driver. > > >> > > > > > >> > > > This approach minimizes the delta, and thus the maintenance burden, > > >between > > >> > > > the traditional SMM and standalone MM drivers, while not resorting to > > >runtime > > >> > > > checks or other conditionals in the code to implement logic that should > > >be > > >> > > > decided at build time. > > >> > > > > > >> > > > Note that this series only covers part of the work contributed by > > >Jagadeesh. > > >> > > > This series focuses on the MdePkg and MdeModulePkg changes that > > >affect shared > > >> > > > code. > > >> > > > > > >> > > > Cc: Laszlo Ersek <lersek@redhat.com> > > >> > > > Cc: Leif Lindholm <leif.lindholm@linaro.org> > > >> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > >> > > > Cc: Liming Gao <liming.gao@intel.com> > > >> > > > Cc: Jian J Wang <jian.j.wang@intel.com> > > >> > > > Cc: Hao Wu <hao.a.wu@intel.com> > > >> > > > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com> > > >> > > > Cc: Achin Gupta <Achin.Gupta@arm.com> > > >> > > > Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com> > > >> > > > Cc: Sami Mujawar <Sami.Mujawar@arm.com> > > >> > > > > > >> > > > Ard Biesheuvel (5): > > >> > > > MdePkg: implement MmServicesTableLib based on traditional SMM > > >> > > > MdeModulePkg/FaultTolerantWriteDxe: factor out boot service > > >accesses > > >> > > > MdeModulePkg/FaultTolerantWriteDxe: implement standalone MM > > >version > > >> > > > MdeModulePkg/VariableRuntimeDxe: factor out boot service > > >accesses > > >> > > > MdeModulePkg/VariableRuntimeDxe: implement standalone MM > > >version > > >> > > > > > >> > > > Jagadeesh Ujja (1): > > >> > > > MdePkg/Include: add MmServicesTableLib header file > > >> > > > > > >> > > > MdeModulePkg/MdeModulePkg.dsc | 1 + > > >> > > > .../FaultTolerantWrite.h | 22 ++- > > >> > > > .../FaultTolerantWriteDxe.c | 31 ++++ > > >> > > > .../FaultTolerantWriteSmm.c | 54 +++---- > > >> > > > .../FaultTolerantWriteSmm.inf | 5 +- > > >> > > > .../FaultTolerantWriteSmmCommon.h | 31 ++++ > > >> > > > .../FaultTolerantWriteSmmDxe.c | 1 + > > >> > > > .../FaultTolerantWriteStandaloneMm.c | 70 +++++++++ > > >> > > > .../FaultTolerantWriteStandaloneMm.inf | 90 ++++++++++++ > > >> > > > .../FaultTolerantWriteTraditionalMm.c | 94 ++++++++++++ > > >> > > > .../UpdateWorkingBlock.c | 10 +- > > >> > > > .../Variable/RuntimeDxe/TcgMorLockSmm.c | 18 +-- > > >> > > > .../Universal/Variable/RuntimeDxe/Variable.h | 50 +++++++ > > >> > > > .../Variable/RuntimeDxe/VariableSmm.c | 59 +++----- > > >> > > > .../Variable/RuntimeDxe/VariableSmm.inf | 5 +- > > >> > > > .../RuntimeDxe/VariableStandaloneMm.c | 69 +++++++++ > > >> > > > .../RuntimeDxe/VariableStandaloneMm.inf | 135 > > >++++++++++++++++++ > > >> > > > .../RuntimeDxe/VariableTraditionalMm.c | 114 +++++++++++++++ > > >> > > > MdePkg/Include/Library/MmServicesTableLib.h | 25 ++++ > > >> > > > .../MmServicesTableLib/MmServicesTableLib.c | 63 ++++++++ > > >> > > > .../MmServicesTableLib/MmServicesTableLib.inf | 45 ++++++ > > >> > > > .../MmServicesTableLib/MmServicesTableLib.uni | 22 +++ > > >> > > > MdePkg/MdePkg.dec | 4 + > > >> > > > MdePkg/MdePkg.dsc | 1 + > > >> > > > 24 files changed, 916 insertions(+), 103 deletions(-) > > >> > > > create mode 100644 > > >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal > > >oneMm.c > > >> > > > create mode 100644 > > >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandal > > >oneMm.inf > > >> > > > create mode 100644 > > >MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteTraditio > > >nalMm.c > > >> > > > create mode 100644 > > >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.c > > >> > > > create mode 100644 > > >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > > >> > > > create mode 100644 > > >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableTraditionalMm.c > > >> > > > create mode 100644 MdePkg/Include/Library/MmServicesTableLib.h > > >> > > > create mode 100644 > > >MdePkg/Library/MmServicesTableLib/MmServicesTableLib.c > > >> > > > create mode 100644 > > >MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf > > >> > > > create mode 100644 > > >MdePkg/Library/MmServicesTableLib/MmServicesTableLib.uni > > >> > > > > > >> > > > -- > > >> > > > 2.17.1 > > >> > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel