From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1C5A821F3C18D for ; Tue, 10 Oct 2017 01:10:04 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 099754ACA7; Tue, 10 Oct 2017 08:13:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 099754ACA7 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-87.rdu2.redhat.com [10.10.120.87]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1556A18B54; Tue, 10 Oct 2017 08:13:29 +0000 (UTC) To: "Wang, Jian J" , "edk2-devel@lists.01.org" Cc: "Kinney, Michael D" , "Wolman, Ayellet" , "Yao, Jiewen" , "Dong, Eric" , "Zeng, Star" References: <20171009141722.992-1-jian.j.wang@intel.com> <20171009141722.992-7-jian.j.wang@intel.com> <4936a3a6-e4ef-ad6c-d915-1ee45c9387b6@redhat.com> From: Laszlo Ersek Message-ID: <2678dcd3-6408-0b56-dfda-278c7990a936@redhat.com> Date: Tue, 10 Oct 2017 10:13:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 10 Oct 2017 08:13:32 +0000 (UTC) Subject: Re: [PATCH v4 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Oct 2017 08:10:05 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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:lersek@redhat.com] >> Sent: Monday, October 09, 2017 11:56 PM >> To: Wang, Jian J ; edk2-devel@lists.01.org >> Cc: Kinney, Michael D ; Wolman, Ayellet >> ; Yao, Jiewen ; Dong, Eric >> ; Zeng, Star >> 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 >>>> Cc: Eric Dong >>>> Cc: Jiewen Yao >>>> Cc: Michael Kinney >>>> Cc: Ayellet Wolman >>>> Suggested-by: Ayellet Wolman >>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> Signed-off-by: Jian J Wang >>>> --- >>>> 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- >> d03b330047cc@redhat.com >>> >>> 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