Comments below: > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Tuesday, April 26, 2016 12:32 AM > 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. > > On 04/25/16 17:17, Yao, Jiewen wrote: > > 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. > > >> (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. > > Great, thanks. > > > 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. > > Right, when I wrote my previous email, I first went over the patches > quickly, to see which of them affected OvmfPkg. I identified patch #4 > and patch #12, so I agree there's nothing more to do for OvmfPkg's sake. > > I have one general remark / request here. Given the fact that edk2 uses > quite long file names (and a somewhat deep directory structure), the > diffstat that git generates, when formatting the patches, is often > truncated. For example, in patch #1, the file name is only: > > .../Include/Guid/PiSmmCommunicationRegionTable.h > > and not > > MdeModulePkg/Include/Guid/PiSmmCommunicationRegionTable.h > > This is quite disturbing when someone tries to figure out quickly what > files a patch modifies. > > It can be remedied, of course. The following two options work well: > > git format-patch --stat=1000 --stat-graph-width=20 [other options] > > I have not found a possibility yet to make these settings permanent > (beyond capturing them in a git alias command), unfortunately. > > If you (and everyone else indeed) could adopt these options for > formatting edk2 patches, that would be great -- the difference these > options make for the readability of the diffstat is significant.
[Jiewen] Yes, that is very good suggestion. I have similar concern before, but I do not know how to let show full name. I added "--stat=1000 --stat-graph-width=20" to our BKM template. > > > I will work out a branch in public repo tomorrow. > > Thank you; I plan to test it, and follow up here. [Jiewen] Thanks great help from "Wu, Hao A" <hao.a...@intel.com>, I created a branch. You can get it from https://github.com/jyao1/edk2/tree/SmmComm-jyao1 I made 3 update based on feedback so far. 1) Patch 09/12. The previous commit message is wrong and is fixed now. No real code change. Only commit log change. 2) Patch 02/12. I rename the driver name from SmmCommunicationBuffer to SmmCommunicationBufferDxe. It follows EDKII driver naming style. No real code change. Only file name change. 3) Patch 02/12. I fixed UNI file to UTF8 format. (The tool is great). No content change. Only format change. > > Cheers > Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
