Re: [PATCH v4 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing

["Wang, Jian J" <jian.j.wang@intel.com>]

I have summary in each patch email. 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?

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, October 09, 2017 11:56 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> 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 <star.zeng@intel.com>
> >> Cc: Eric Dong <eric.dong@intel.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Cc: Michael Kinney <michael.d.kinney@intel.com>
> >> Cc: Ayellet Wolman <ayellet.wolman@intel.com>
> >> Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> >> ---
> >>  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