1. Patchew
  2. EDK2
  3. [edk2] [PATCH v4 0/6] Add NULL pointer detection feature
  4. View patch

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

Jian J Wang posted 6 patches 7 years, 2 months ago
[edk2] [PATCH v4 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing
Posted by Jian J Wang 7 years, 2 months ago 
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;
-- 
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing
Posted by Laszlo Ersek 7 years, 2 months ago 
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.

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing
Posted by Laszlo Ersek 7 years, 2 months ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing
Posted by Wang, Jian J 7 years, 2 months ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing
Posted by Laszlo Ersek 7 years, 2 months ago 
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 <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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing
Posted by Wang, Jian J 7 years, 2 months ago 
Got it. Thank you very much for the advice and information.

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, October 10, 2017 4:13 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/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 <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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.