Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45352 )
Change subject: nb/intel/haswell: Guard DMIBAR/EPBAR macro parameters ......................................................................
nb/intel/haswell: Guard DMIBAR/EPBAR macro parameters
Add brackets around the parameters to avoid operation order problems.
Tested with BUILD_TIMELESS=1, Asrock B85M Pro4 remains identical.
Change-Id: I5e1a02ba2ebf468f0d80b7f1838766280b6b7b22 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/haswell/haswell.h 1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/45352/1
diff --git a/src/northbridge/intel/haswell/haswell.h b/src/northbridge/intel/haswell/haswell.h index 00cc885..f3f525d 100644 --- a/src/northbridge/intel/haswell/haswell.h +++ b/src/northbridge/intel/haswell/haswell.h @@ -57,9 +57,9 @@ * EPBAR - Egress Port Root Complex Register Block */
-#define EPBAR8(x) *((volatile u8 *)(DEFAULT_EPBAR + x)) -#define EPBAR16(x) *((volatile u16 *)(DEFAULT_EPBAR + x)) -#define EPBAR32(x) *((volatile u32 *)(DEFAULT_EPBAR + x)) +#define EPBAR8(x) *((volatile u8 *)(DEFAULT_EPBAR + (x))) +#define EPBAR16(x) *((volatile u16 *)(DEFAULT_EPBAR + (x))) +#define EPBAR32(x) *((volatile u32 *)(DEFAULT_EPBAR + (x)))
#define EPPVCCAP1 0x004 /* 32bit */ #define EPPVCCAP2 0x008 /* 32bit */ @@ -88,9 +88,9 @@ * DMIBAR */
-#define DMIBAR8(x) *((volatile u8 *)(DEFAULT_DMIBAR + x)) -#define DMIBAR16(x) *((volatile u16 *)(DEFAULT_DMIBAR + x)) -#define DMIBAR32(x) *((volatile u32 *)(DEFAULT_DMIBAR + x)) +#define DMIBAR8(x) *((volatile u8 *)(DEFAULT_DMIBAR + (x))) +#define DMIBAR16(x) *((volatile u16 *)(DEFAULT_DMIBAR + (x))) +#define DMIBAR32(x) *((volatile u32 *)(DEFAULT_DMIBAR + (x)))
#define DMIVCECH 0x000 /* 32bit */ #define DMIPVCCAP1 0x004 /* 32bit */
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45352 )
Change subject: nb/intel/haswell: Guard DMIBAR/EPBAR macro parameters ......................................................................
Patch Set 1:
(6 comments)
https://review.coreboot.org/c/coreboot/+/45352/1/src/northbridge/intel/haswe... File src/northbridge/intel/haswell/haswell.h:
https://review.coreboot.org/c/coreboot/+/45352/1/src/northbridge/intel/haswe... PS1, Line 60: #define EPBAR8(x) *((volatile u8 *)(DEFAULT_EPBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/1/src/northbridge/intel/haswe... PS1, Line 61: #define EPBAR16(x) *((volatile u16 *)(DEFAULT_EPBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/1/src/northbridge/intel/haswe... PS1, Line 62: #define EPBAR32(x) *((volatile u32 *)(DEFAULT_EPBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/1/src/northbridge/intel/haswe... PS1, Line 91: #define DMIBAR8(x) *((volatile u8 *)(DEFAULT_DMIBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/1/src/northbridge/intel/haswe... PS1, Line 92: #define DMIBAR16(x) *((volatile u16 *)(DEFAULT_DMIBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/1/src/northbridge/intel/haswe... PS1, Line 93: #define DMIBAR32(x) *((volatile u32 *)(DEFAULT_DMIBAR + (x))) Macros with complex values should be enclosed in parentheses
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45352 )
Change subject: nb/intel/haswell: Guard DMIBAR/EPBAR macro parameters ......................................................................
Patch Set 2:
(6 comments)
https://review.coreboot.org/c/coreboot/+/45352/2/src/northbridge/intel/haswe... File src/northbridge/intel/haswell/haswell.h:
https://review.coreboot.org/c/coreboot/+/45352/2/src/northbridge/intel/haswe... PS2, Line 60: #define EPBAR8(x) *((volatile u8 *)(DEFAULT_EPBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/2/src/northbridge/intel/haswe... PS2, Line 61: #define EPBAR16(x) *((volatile u16 *)(DEFAULT_EPBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/2/src/northbridge/intel/haswe... PS2, Line 62: #define EPBAR32(x) *((volatile u32 *)(DEFAULT_EPBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/2/src/northbridge/intel/haswe... PS2, Line 91: #define DMIBAR8(x) *((volatile u8 *)(DEFAULT_DMIBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/2/src/northbridge/intel/haswe... PS2, Line 92: #define DMIBAR16(x) *((volatile u16 *)(DEFAULT_DMIBAR + (x))) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/45352/2/src/northbridge/intel/haswe... PS2, Line 93: #define DMIBAR32(x) *((volatile u32 *)(DEFAULT_DMIBAR + (x))) Macros with complex values should be enclosed in parentheses
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45352 )
Change subject: nb/intel/haswell: Guard DMIBAR/EPBAR macro parameters ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45352 )
Change subject: nb/intel/haswell: Guard DMIBAR/EPBAR macro parameters ......................................................................
nb/intel/haswell: Guard DMIBAR/EPBAR macro parameters
Add brackets around the parameters to avoid operation order problems.
Tested with BUILD_TIMELESS=1, Asrock B85M Pro4 remains identical.
Change-Id: I5e1a02ba2ebf468f0d80b7f1838766280b6b7b22 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45352 Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/haswell/haswell.h 1 file changed, 6 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/northbridge/intel/haswell/haswell.h b/src/northbridge/intel/haswell/haswell.h index 00cc885..f3f525d 100644 --- a/src/northbridge/intel/haswell/haswell.h +++ b/src/northbridge/intel/haswell/haswell.h @@ -57,9 +57,9 @@ * EPBAR - Egress Port Root Complex Register Block */
-#define EPBAR8(x) *((volatile u8 *)(DEFAULT_EPBAR + x)) -#define EPBAR16(x) *((volatile u16 *)(DEFAULT_EPBAR + x)) -#define EPBAR32(x) *((volatile u32 *)(DEFAULT_EPBAR + x)) +#define EPBAR8(x) *((volatile u8 *)(DEFAULT_EPBAR + (x))) +#define EPBAR16(x) *((volatile u16 *)(DEFAULT_EPBAR + (x))) +#define EPBAR32(x) *((volatile u32 *)(DEFAULT_EPBAR + (x)))
#define EPPVCCAP1 0x004 /* 32bit */ #define EPPVCCAP2 0x008 /* 32bit */ @@ -88,9 +88,9 @@ * DMIBAR */
-#define DMIBAR8(x) *((volatile u8 *)(DEFAULT_DMIBAR + x)) -#define DMIBAR16(x) *((volatile u16 *)(DEFAULT_DMIBAR + x)) -#define DMIBAR32(x) *((volatile u32 *)(DEFAULT_DMIBAR + x)) +#define DMIBAR8(x) *((volatile u8 *)(DEFAULT_DMIBAR + (x))) +#define DMIBAR16(x) *((volatile u16 *)(DEFAULT_DMIBAR + (x))) +#define DMIBAR32(x) *((volatile u32 *)(DEFAULT_DMIBAR + (x)))
#define DMIVCECH 0x000 /* 32bit */ #define DMIPVCCAP1 0x004 /* 32bit */
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45352 )
Change subject: nb/intel/haswell: Guard DMIBAR/EPBAR macro parameters ......................................................................
Patch Set 3:
Automatic boot test returned (PASS/FAIL/TOTAL): 8/1/9 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19833 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19832 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/19831 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19830 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/19829 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19837 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/19836 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/19835 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/19834
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
build bot (Jenkins) (Code Review)
-
Tim Wawrzynczak (Code Review)