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

[Laszlo Ersek <lersek@redhat.com>]

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