On 04/25/16 17:17, Yao, Jiewen wrote: > Comments below: > >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: Monday, April 25, 2016 6:50 PM >> To: Yao, Jiewen <[email protected]> >> Cc: [email protected]; Gao, Liming <[email protected]>; Tian, >> Feng <[email protected]>; Zeng, Star <[email protected]>; Dong, Eric >> <[email protected]>; Justen, Jordan L <[email protected]> >> 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. > I will work out a branch in public repo tomorrow. Thank you; I plan to test it, and follow up here. Cheers Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
