On 09/22/17 13:50, Laszlo Ersek wrote:
> This patch looks great to me, I would like to request a few small
> updates:
>
> On 09/21/17 07:20, Jian J Wang wrote:
>> QemuVideoDxe driver will install VBE SHIM into page 0. If NULL pointer
>
> (1) please replace the word "install" with "link".
>
> The VBE Shim is technically installed into the "real-mode" C segment,
> only the int 0x10 vector lives in page 0.
>
>> 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.
>
> This is not a request, just a comment: I verified the default value in
> the .dec, and I see it is 0. So there's no need to post an additional
> patch for the OVMF DSC files, in order to set BIT7.
Actually, let me take a step back, and re-think the necessity of all
this work for QemuVideoDxe!
The facts are:
(1) The *only* purpose of the VBE Shim is to allow Windows 7 to boot in
pure UEFI mode (i.e. without a CSM).
(2) If I understand correctly, you guys have verified that Windows 7
cannot boot with the page0 protection enabled, *regardless* of what we
do in QemuVideoDxe. Can you confirm this please?
With the above in mind, let's consider the effects of the
"PcdNullPointerDetectionPropertyMask" bits:
* BIT0 clear:
- The page0 protection is completely disabled.
- This patch does nothing, in effect.
- The VBE Shim works.
- Windows 7 boots.
* BIT0 set, BIT7 also set:
- The page0 protection is disabled in the DXE core at the end of DXE.
- This patch does nothing, in effect.
- The VBE Shim works, because it is a UEFI driver, and it connects its
devices (and installs the shim) after End-of-Dxe, at which point
page0 protection is no longer in effect.
- Windows 7 boots fine, again because it is loaded after End-of-Dxe.
* BIT0 set, BIT7 clear:
- The page0 protection is never disabled until the OS (loader)
installs its own page tables.
- This patch enables the VBE Shim to work, by temporarily disabling
page0 protection.
- However, Windows 7 will fail to boot nonetheless, because it cannot
cope with page0 protection. (This is fact (2).)
Now, if you consider fact (1) as well: given that Windows 7 cannot boot
with page0 protection enabled *anyway*, why mess with the VBE Shim at
all?
How about the following patch instead:
> diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.c b/OvmfPkg/QemuVideoDxe/VbeShim.c
> index e45a08e8873f..8ba5522cde3c 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;
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
