Comments below: > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Monday, April 25, 2016 6:50 PM > To: Yao, Jiewen <jiewen....@intel.com> > Cc: edk2-de...@ml01.01.org; Gao, Liming <liming....@intel.com>; Tian, > Feng <feng.t...@intel.com>; Zeng, Star <star.z...@intel.com>; Dong, Eric > <eric.d...@intel.com>; Justen, Jordan L <jordan.l.jus...@intel.com> > Subject: Re: [PATCH 00/12] Enhance SMM Communication by using fixed > comm buffer. > > Hi Jiewen, > > On 04/22/16 10:04, Jiewen Yao wrote: > > This series patches are generate to meet Microsoft WSMT table definition > on > > FIXED_COMM_BUFFERS requirement. > > > > Before this series patches, the DXE or OS module can use any non-SMM > memory > > as communication buffer to exchange data with SMM agent. Microsoft > WSMT table > > has requirement to support fixed communication buffer - so that SMM > agent > > can only support communication buffer with type > EfiReservedMemoryType/ > > EfiRuntimeServicesCode/EfiRuntimeServicesData/EfiACPIMemoryNVS, > which will > > not be used by OS during runtime. > > So we clean up all SMM handler to only use these memory regions for > SMM > > communication, and enhance check in SmmMemLib to catch the violation. > > > > This series patches are validated on real platforms with SMM enabled. > > > > This series patches are validated on OVMF ia32-x64 with SMM enabled. > > I tried to test this, but the patches fail to apply with "git am". > > (1) Can you please push your work to a branch in your public repo, so I > can fetch it?
[Jiewen] Yes, I will do that tomorrow. > > Also, I have one remark and one question: > > (2) Please don't add UNI files that have UCS-2 encoding. To my > knowledge, Jordan converted all UNI files in the tree to UTF-8. Can you > please update the UNI files to UTF-8? You can use Jordan's script for it: > > python BaseTools/Scripts/ConvertUni.py --help [Jiewen] Yes, agree. That is my fault. I guess it is because the new module was generated long time ago, before UNI file patch. So the new module copied some old UCS-2 encoding files. I will fix that. Thanks to catch that. > > (3) In the cover letter and in the commit message of patch #12, new > requirements for platforms are spelled out: > > > The assumption is that a platform reports valid SMM communication > > buffer at EndOfDxe, because EndOfDxe is last hook point that SMM code > > can call-out to get memory map information. > > > > A platform MUST finish SMM communication buffer allocation before > > EndOfDxe. If a DXE or OS driver need do communication after EndOfDxe, > > it can either allocate SMM communication buffer before EndOfDxe and > > save it, or consume EDKII_PI_SMM_COMMUNICATION_REGION_TABLE > table to > > get general fixed comm buffer. > > Given that your testing with OVMF succeeded, am I right to assume that > VariableSmmRuntimeDxe conforms to the above requirement, without any > modifications? [Jiewen] Yes. VariableSmmRuntimeDxe follows the rule. So no modification is needed. We reviewed code and did validation. We caught SmmProfileApp/Fpdt/PerfLib/OpalDxe, then each module owner fixed it. I notice OVMF does not include any of these modules, so I think no change is needed. If you can double confirm, that would be great. I will work out a branch in public repo tomorrow. > > Thanks > Laszlo > > > > Cc: "Gao, Liming" <liming....@intel.com> > > Cc: "Tian, Feng" <feng.t...@intel.com> > > Cc: "Zeng, Star" <star.z...@intel.com> > > Cc: "Dong, Eric" <eric.d...@intel.com> > > Cc: "Laszlo Ersek" <ler...@redhat.com> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: "Yao, Jiewen" <jiewen....@intel.com> > > Reviewed-by: "Tian, Feng" <feng.t...@intel.com> > > Reviewed-by: "Gao, Liming" <liming....@intel.com> > > > > Jiewen Yao (12): > > MdeModulePkg: Add > EDKII_PI_SMM_COMMUNICATION_REGION_TABLE definition. > > MdeModulePkg: Add new driver to publish > > EDKII_PI_SMM_COMMUNICATION_REGION_TABLE. > > MdeModulePkg-MemoryProfile(1): Add > > SMRAM_PROFILE_COMMAND_GET_PROFILE_DATA_BY_OFFSET > definition. > > MdeModulePkg-MemoryProfile(2): Add > > SMRAM_PROFILE_COMMAND_GET_PROFILE_DATA_BY_OFFSET in > PiSmmCore. > > MdeModulePkg-MemoryProfile(3): Use > > SMRAM_PROFILE_COMMAND_GET_PROFILE_DATA_BY_OFFSET in > > MemoryProfileInfo. > > MdeModulePkg-FPDT(1): Add > > SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA_BY_OFFSET > definition. > > MdeModulePkg-FPDT(2): Add > > SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA_BY_OFFSET in > FpdtSmm Handler. > > MdeModulePkg-FPDT(3): Use > > SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA_BY_OFFSET in > FpdtDxe. > > MdeModulePkg-FPDT(4): Use > > SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA_BY_OFFSET in > PerfLib. > > SecurityPkg-Opal(1): Use fixed SMM communication buffer in OPAL > > password lib. > > SecurityPkg-Opal(2): Enhance AHCI Bar MMIO region check. > > MdePkg-SmmMemLib: Enhance SmmIsBufferOutsideSmmValid() check > for fixed > > comm buffer. > > > > .../MemoryProfileInfo/MemoryProfileInfo.c | 103 ++++++--- > > .../MemoryProfileInfo/MemoryProfileInfo.inf | 3 +- > > MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 247 > +++++++++++++++++---- > > MdeModulePkg/Include/Guid/FirmwarePerformance.h | 8 +- > > MdeModulePkg/Include/Guid/MemoryProfile.h | 27 ++- > > .../Include/Guid/PiSmmCommunicationRegionTable.h | 63 ++++++ > > .../DxeSmmPerformanceLib/DxeSmmPerformanceLib.c | 154 > ++++++++++--- > > .../DxeSmmPerformanceLib/DxeSmmPerformanceLib.inf | 3 +- > > MdeModulePkg/MdeModulePkg.dec | 2 + > > MdeModulePkg/MdeModulePkg.dsc | 1 + > > .../FirmwarePerformanceDxe.c | 118 > ++++++---- > > .../FirmwarePerformanceDxe.inf | 3 +- > > .../FirmwarePerformanceSmm.c | 63 > +++--- > > .../SmmCommunicationBuffer.c | 99 > +++++++++ > > .../SmmCommunicationBuffer.inf | 62 ++++++ > > .../SmmCommunicationBuffer.uni | Bin 0 -> > 2794 bytes > > .../SmmCommunicationBufferExtra.uni | Bin 0 -> > 1372 bytes > > MdePkg/Library/SmmMemLib/SmmMemLib.c | 180 > ++++++++++++++- > > MdePkg/Library/SmmMemLib/SmmMemLib.inf | 6 +- > > .../OpalPasswordSupportLib.c | 32 ++- > > .../OpalPasswordSupportLib.inf | 4 + > > .../OpalPasswordSupportNotify.h | 2 +- > > .../Tcg/Opal/OpalPasswordSmm/OpalAhciMode.c | 30 ++- > > .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.c | 51 +++++ > > .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.h | 5 +- > > .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.inf | 2 +- > > 26 files changed, 1071 insertions(+), 197 deletions(-) > > create mode 100644 > MdeModulePkg/Include/Guid/PiSmmCommunicationRegionTable.h > > create mode 100644 > MdeModulePkg/Universal/SmmCommunicationBuffer/SmmCommunication > Buffer.c > > create mode 100644 > MdeModulePkg/Universal/SmmCommunicationBuffer/SmmCommunication > Buffer.inf > > create mode 100644 > MdeModulePkg/Universal/SmmCommunicationBuffer/SmmCommunication > Buffer.uni > > create mode 100644 > MdeModulePkg/Universal/SmmCommunicationBuffer/SmmCommunication > BufferExtra.uni > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
