UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index c31160735a..a3b62f7787 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -571,7 +571,7 @@ ReclaimPages (
//
// First, find the leaf entry has the smallest access record value
//
- for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1; Pml5Index++) {
+ for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1); Pml5Index++) {
if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & IA32_PG_PMNT) != 0) {
//
// If the PML5 entry is not present or is masked, skip it
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#43533): https://edk2.groups.io/g/devel/message/43533
Mute This Topic: https://groups.io/mt/32422914/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Eric Dong <eric.dong@intel.com> > -----Original Message----- > From: Ni, Ray > Sent: Thursday, July 11, 2019 9:17 AM > To: devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Dong, Eric > <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com> > Subject: [PATCH] UefiCpuPkg/PiSmmCpu: Fix GCC7/GCC8 build failure > > Signed-off-by: Ray Ni <ray.ni@intel.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index c31160735a..a3b62f7787 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -571,7 +571,7 @@ ReclaimPages ( > // > // First, find the leaf entry has the smallest access record value > // > - for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / > sizeof (*Pml4)) : 1; Pml5Index++) { > + for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / > sizeof (*Pml4)) : 1); Pml5Index++) { > if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & > IA32_PG_PMNT) != 0) { > // > // If the PML5 entry is not present or is masked, skip it > -- > 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43547): https://edk2.groups.io/g/devel/message/43547 Mute This Topic: https://groups.io/mt/32422914/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hello Ray, On 07/11/19 03:16, Ray Ni wrote: > Signed-off-by: Ray Ni <ray.ni@intel.com> > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index c31160735a..a3b62f7787 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -571,7 +571,7 @@ ReclaimPages ( > // > // First, find the leaf entry has the smallest access record value > // > - for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1; Pml5Index++) { > + for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1); Pml5Index++) { > if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & IA32_PG_PMNT) != 0) { > // > // If the PML5 entry is not present or is masked, skip it > This is not a GCC7/GCC8 build failure, but a genuine bug in the code that GCC7/GCC8 helpfully reported. The conditional operator ?: has weaker precedence than the relational operator <, and so the patch incurs a behavioral change -- thus, it is a bugfix. If we were only adding the parentheses to reinforce the operator bingings that are already in place, i.e., preserving the behavior, *then* we could call this a "build failure". (1) Please update the subject accordingly, for example: UefiCpuPkg/PiSmmCpuDxeSmm: ReclaimPages: fix incorrect operator binding (71 characters) (2) I'd suggest also adding, to the commit message body: Fixes: 7365eb2c8cf1d7112330d09918c0c67e8d0b827a With those: Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43595): https://edk2.groups.io/g/devel/message/43595 Mute This Topic: https://groups.io/mt/32422914/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 07/11/19 12:02, Laszlo Ersek wrote: > Hello Ray, > > On 07/11/19 03:16, Ray Ni wrote: >> Signed-off-by: Ray Ni <ray.ni@intel.com> >> Cc: Michael D Kinney <michael.d.kinney@intel.com> >> Cc: Eric Dong <eric.dong@intel.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> --- >> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> index c31160735a..a3b62f7787 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> @@ -571,7 +571,7 @@ ReclaimPages ( >> // >> // First, find the leaf entry has the smallest access record value >> // >> - for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1; Pml5Index++) { >> + for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1); Pml5Index++) { >> if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & IA32_PG_PMNT) != 0) { >> // >> // If the PML5 entry is not present or is masked, skip it >> > > This is not a GCC7/GCC8 build failure, but a genuine bug in the code > that GCC7/GCC8 helpfully reported. > > The conditional operator ?: has weaker precedence than the relational > operator <, and so the patch incurs a behavioral change -- thus, it is a > bugfix. > > If we were only adding the parentheses to reinforce the operator > bingings that are already in place, i.e., preserving the behavior, > *then* we could call this a "build failure". > > > (1) Please update the subject accordingly, for example: > > UefiCpuPkg/PiSmmCpuDxeSmm: ReclaimPages: fix incorrect operator binding > > (71 characters) > > > (2) I'd suggest also adding, to the commit message body: > > Fixes: 7365eb2c8cf1d7112330d09918c0c67e8d0b827a > > With those: > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> IMPORTANT: please do not push this patch before you revert and reapply 7365eb2c8cf1; otherwise we'll get into a huge mess. Please see here: https://edk2.groups.io/g/devel/message/43599 (Namely, if you apply this patch on top of 7365eb2c8cf1, then you won't be able to revert 7365eb2c8cf1, which is a pre-requisite for reverting 7c5010c7f88b.) Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43601): https://edk2.groups.io/g/devel/message/43601 Mute This Topic: https://groups.io/mt/32422914/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.