On 10/10/17 03:50, Wang, Jian J wrote: > I have summary in each patch email.
Sure, you have a commit message, but you didn't point out any v3->v4 changes. (If there are no v3->v4 changes, then please pick up the R-b's received for v3.) > I removed the CC of some patches because > there's no update from v3 to v4. I thought this could remind you of this > situation. > What's the recommended way? Keep the CC as-was and just add summaries in > cover letter? Maintainers should always be CC'd, even if there are no changes relative to the previous version. Personally I prefer summarizing the changes in the cover letter, and also explaining the changes in more detail -- or pointing out the lack of any changes -- on each individual patch. If you have a bit of time for reading, I recommend the following wiki article: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers In it, I had written down most everything that I have in mind about the edk2 development process. The following steps pertain to the current discussion (i.e., picking up feedback tags, adding Maintainer CC's, and pointing out changes per patch): https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-18 https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-28 https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-30 Thanks, Laszlo > >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: Monday, October 09, 2017 11:56 PM >> To: Wang, Jian J <[email protected]>; [email protected] >> Cc: Kinney, Michael D <[email protected]>; Wolman, Ayellet >> <[email protected]>; Yao, Jiewen <[email protected]>; Dong, Eric >> <[email protected]>; Zeng, Star <[email protected]> >> Subject: Re: [edk2] [PATCH v4 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL >> pointer detection during VBE SHIM installing >> >> On 10/09/17 17:54, Laszlo Ersek wrote: >>> On 10/09/17 16:17, Jian J Wang wrote: >>>> QemuVideoDxe driver will link VBE SHIM into page 0. If NULL pointer >>>> detection is enabled, this driver will fail to load. NULL pointer detection >>>> bypassing code is added to prevent such problem during boot. >>>> >>>> Please note that Windows 7 will try to access VBE SHIM during boot if it's >>>> installed, and then cause boot failure. This can be fixed by setting BIT7 >>>> of PcdNullPointerDetectionPropertyMask to disable NULL pointer detection >>>> after EndOfDxe. As far as we know, there's no other OSs has such issue. >>>> >>>> Cc: Star Zeng <[email protected]> >>>> Cc: Eric Dong <[email protected]> >>>> Cc: Jiewen Yao <[email protected]> >>>> Cc: Michael Kinney <[email protected]> >>>> Cc: Ayellet Wolman <[email protected]> >>>> Suggested-by: Ayellet Wolman <[email protected]> >>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> Signed-off-by: Jian J Wang <[email protected]> >>>> --- >>>> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 1 + >>>> OvmfPkg/QemuVideoDxe/VbeShim.c | 14 ++++++++++++++ >>>> 2 files changed, 15 insertions(+) >>>> >>>> diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf >> b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf >>>> index 577e07b0a8..ff68c99e96 100644 >>>> --- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf >>>> +++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf >>>> @@ -77,3 +77,4 @@ >>>> [Pcd] >>>> gOptionRomPkgTokenSpaceGuid.PcdDriverSupportedEfiVersion >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId >>>> + >> gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask >>>> diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.c >> b/OvmfPkg/QemuVideoDxe/VbeShim.c >>>> index e45a08e887..8ba5522cde 100644 >>>> --- a/OvmfPkg/QemuVideoDxe/VbeShim.c >>>> +++ b/OvmfPkg/QemuVideoDxe/VbeShim.c >>>> @@ -75,6 +75,20 @@ InstallVbeShim ( >>>> UINTN Printed; >>>> VBE_MODE_INFO *VbeModeInfo; >>>> >>>> + if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7)) == >>>> BIT0) >> { >>>> + DEBUG (( >>>> + DEBUG_WARN, >>>> + "%a: page 0 protected, not installing VBE shim\n", >>>> + __FUNCTION__ >>>> + )); >>>> + DEBUG (( >>>> + DEBUG_WARN, >>>> + "%a: page 0 protection prevents Windows 7 from booting anyway\n", >>>> + __FUNCTION__ >>>> + )); >>>> + return; >>>> + } >>>> + >>>> Segment0 = 0x00000; >>>> SegmentC = 0xC0000; >>>> SegmentF = 0xF0000; >>>> >>> >>> If this patch is entirely identical to the previous version (v3), then >>> you should have please picked up the review tags from Jordan and myself, >>> the ones that you got for v3: >>> >>> http://mid.mail- >> archive.com/150696711831.2454.16712170525103415248@jljusten-skl >>> >>> http://mid.mail-archive.com/d1a20be5-8dbf-8ce6-1738- >> [email protected] >>> >>> This way we can quickly filter out already reviewed patches, and avoid >>> re-reviewing when there are no changes. >>> >>> >>> Your cover letter v4 0/6 also does not summarize the changes relative to >>> v3; in the future please don't forget about that. >> >> ... personal CC's for OvmfPkg maintainers and reviewers are also missing >> from this patch. Please check "Maintainers.txt" every time. >> >> Thanks >> Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
