David Hendricks has submitted this change. ( https://review.coreboot.org/c/coreboot/+/53983 )
Change subject: AGESA f15tn: Drop `IDSOPT_ASSERT_ENABLED` ......................................................................
AGESA f15tn: Drop `IDSOPT_ASSERT_ENABLED`
The `ASSERT` macro is already defined in `src/include/assert.h`, and AGESA's definition is never used. On Asus A88XM-E, toggling the value of the `IDSOPT_ASSERT_ENABLED` macro does not change the resulting binary when using reproducible builds. Attempting to use AGESA's definition of the `ASSERT` macro results in build errors:
In file included from src/vendorcode/amd/agesa/f15tn/Proc/CPU/Feature/cpuDmi.c:56: src/vendorcode/amd/agesa/f15tn/Proc/CPU/Feature/cpuDmi.c: In function 'GetType4Type7Info': src/vendorcode/amd/agesa/f15tn/Include/Ids.h:371:33: error: statement with no effect [-Werror=unused-value] #define ASSERT(conditional) ((conditional) ? 0 : IdsAssert (STOP_CODE));
Given that coreboot's definition of `ASSERT` is more useful, drop AGESA's broken definition and the useless `IDSOPT_ASSERT_ENABLED` macro. Also remove the `IdsAssert` function, as it is no longer used anywhere.
Tested with BUILD_TIMELESS=1, Asus A88XM-E remains identical.
Change-Id: Ia4e5dbfd3d2e5cec979b8b16fbc11d1ca8a0661e Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/53983 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/mainboard/amd/parmer/OptionsIds.h M src/mainboard/amd/thatcher/OptionsIds.h M src/mainboard/asus/a88xm-e/OptionsIds.h M src/mainboard/asus/f2a85-m/OptionsIds.h M src/mainboard/hp/pavilion_m6_1035dx/OptionsIds.h M src/mainboard/lenovo/g505s/OptionsIds.h M src/mainboard/msi/ms7721/OptionsIds.h M src/vendorcode/amd/agesa/f15tn/Include/Ids.h M src/vendorcode/amd/agesa/f15tn/MainPage.h M src/vendorcode/amd/agesa/f15tn/Proc/IDS/Debug/IdsDebug.c 10 files changed, 0 insertions(+), 94 deletions(-)
Approvals: build bot (Jenkins): Verified Michał Żygowski: Looks good to me, approved
diff --git a/src/mainboard/amd/parmer/OptionsIds.h b/src/mainboard/amd/parmer/OptionsIds.h index 130d852..d109d2f 100644 --- a/src/mainboard/amd/parmer/OptionsIds.h +++ b/src/mainboard/amd/parmer/OptionsIds.h @@ -23,7 +23,6 @@ * IDSOPT_CONTROL_ENABLED * IDSOPT_TRACING_ENABLED * IDSOPT_PERF_ANALYSIS - * IDSOPT_ASSERT_ENABLED * IDSOPT_CAR_CORRUPTION_CHECK_ENABLED * **/ @@ -33,6 +32,5 @@ //#define IDSOPT_TRACING_ENABLED TRUE #define IDSOPT_TRACING_CONSOLE_SERIALPORT TRUE //#define IDSOPT_PERF_ANALYSIS TRUE -#define IDSOPT_ASSERT_ENABLED TRUE
#endif diff --git a/src/mainboard/amd/thatcher/OptionsIds.h b/src/mainboard/amd/thatcher/OptionsIds.h index 130d852..d109d2f 100644 --- a/src/mainboard/amd/thatcher/OptionsIds.h +++ b/src/mainboard/amd/thatcher/OptionsIds.h @@ -23,7 +23,6 @@ * IDSOPT_CONTROL_ENABLED * IDSOPT_TRACING_ENABLED * IDSOPT_PERF_ANALYSIS - * IDSOPT_ASSERT_ENABLED * IDSOPT_CAR_CORRUPTION_CHECK_ENABLED * **/ @@ -33,6 +32,5 @@ //#define IDSOPT_TRACING_ENABLED TRUE #define IDSOPT_TRACING_CONSOLE_SERIALPORT TRUE //#define IDSOPT_PERF_ANALYSIS TRUE -#define IDSOPT_ASSERT_ENABLED TRUE
#endif diff --git a/src/mainboard/asus/a88xm-e/OptionsIds.h b/src/mainboard/asus/a88xm-e/OptionsIds.h index 33b0232..77b239d 100644 --- a/src/mainboard/asus/a88xm-e/OptionsIds.h +++ b/src/mainboard/asus/a88xm-e/OptionsIds.h @@ -21,7 +21,6 @@ * IDSOPT_CONTROL_ENABLED * IDSOPT_TRACING_ENABLED * IDSOPT_PERF_ANALYSIS - * IDSOPT_ASSERT_ENABLED * IDSOPT_CAR_CORRUPTION_CHECK_ENABLED **/
@@ -30,6 +29,5 @@ //#define IDSOPT_TRACING_ENABLED TRUE #define IDSOPT_TRACING_CONSOLE_SERIALPORT TRUE //#define IDSOPT_PERF_ANALYSIS TRUE -#define IDSOPT_ASSERT_ENABLED TRUE
#endif diff --git a/src/mainboard/asus/f2a85-m/OptionsIds.h b/src/mainboard/asus/f2a85-m/OptionsIds.h index 130d852..d109d2f 100644 --- a/src/mainboard/asus/f2a85-m/OptionsIds.h +++ b/src/mainboard/asus/f2a85-m/OptionsIds.h @@ -23,7 +23,6 @@ * IDSOPT_CONTROL_ENABLED * IDSOPT_TRACING_ENABLED * IDSOPT_PERF_ANALYSIS - * IDSOPT_ASSERT_ENABLED * IDSOPT_CAR_CORRUPTION_CHECK_ENABLED * **/ @@ -33,6 +32,5 @@ //#define IDSOPT_TRACING_ENABLED TRUE #define IDSOPT_TRACING_CONSOLE_SERIALPORT TRUE //#define IDSOPT_PERF_ANALYSIS TRUE -#define IDSOPT_ASSERT_ENABLED TRUE
#endif diff --git a/src/mainboard/hp/pavilion_m6_1035dx/OptionsIds.h b/src/mainboard/hp/pavilion_m6_1035dx/OptionsIds.h index 130d852..d109d2f 100644 --- a/src/mainboard/hp/pavilion_m6_1035dx/OptionsIds.h +++ b/src/mainboard/hp/pavilion_m6_1035dx/OptionsIds.h @@ -23,7 +23,6 @@ * IDSOPT_CONTROL_ENABLED * IDSOPT_TRACING_ENABLED * IDSOPT_PERF_ANALYSIS - * IDSOPT_ASSERT_ENABLED * IDSOPT_CAR_CORRUPTION_CHECK_ENABLED * **/ @@ -33,6 +32,5 @@ //#define IDSOPT_TRACING_ENABLED TRUE #define IDSOPT_TRACING_CONSOLE_SERIALPORT TRUE //#define IDSOPT_PERF_ANALYSIS TRUE -#define IDSOPT_ASSERT_ENABLED TRUE
#endif diff --git a/src/mainboard/lenovo/g505s/OptionsIds.h b/src/mainboard/lenovo/g505s/OptionsIds.h index 130d852..d109d2f 100644 --- a/src/mainboard/lenovo/g505s/OptionsIds.h +++ b/src/mainboard/lenovo/g505s/OptionsIds.h @@ -23,7 +23,6 @@ * IDSOPT_CONTROL_ENABLED * IDSOPT_TRACING_ENABLED * IDSOPT_PERF_ANALYSIS - * IDSOPT_ASSERT_ENABLED * IDSOPT_CAR_CORRUPTION_CHECK_ENABLED * **/ @@ -33,6 +32,5 @@ //#define IDSOPT_TRACING_ENABLED TRUE #define IDSOPT_TRACING_CONSOLE_SERIALPORT TRUE //#define IDSOPT_PERF_ANALYSIS TRUE -#define IDSOPT_ASSERT_ENABLED TRUE
#endif diff --git a/src/mainboard/msi/ms7721/OptionsIds.h b/src/mainboard/msi/ms7721/OptionsIds.h index 130d852..d109d2f 100644 --- a/src/mainboard/msi/ms7721/OptionsIds.h +++ b/src/mainboard/msi/ms7721/OptionsIds.h @@ -23,7 +23,6 @@ * IDSOPT_CONTROL_ENABLED * IDSOPT_TRACING_ENABLED * IDSOPT_PERF_ANALYSIS - * IDSOPT_ASSERT_ENABLED * IDSOPT_CAR_CORRUPTION_CHECK_ENABLED * **/ @@ -33,6 +32,5 @@ //#define IDSOPT_TRACING_ENABLED TRUE #define IDSOPT_TRACING_CONSOLE_SERIALPORT TRUE //#define IDSOPT_PERF_ANALYSIS TRUE -#define IDSOPT_ASSERT_ENABLED TRUE
#endif diff --git a/src/vendorcode/amd/agesa/f15tn/Include/Ids.h b/src/vendorcode/amd/agesa/f15tn/Include/Ids.h index 95d726d..09a2341 100644 --- a/src/vendorcode/amd/agesa/f15tn/Include/Ids.h +++ b/src/vendorcode/amd/agesa/f15tn/Include/Ids.h @@ -223,10 +223,6 @@ #define IDSOPT_HEAP_CHECKING FALSE #endif
-#ifndef IDSOPT_ASSERT_ENABLED - #define IDSOPT_ASSERT_ENABLED FALSE -#endif - #ifndef IDSOPT_ERROR_TRAP_ENABLED #define IDSOPT_ERROR_TRAP_ENABLED FALSE #endif @@ -264,7 +260,6 @@ #undef IDSOPT_TRACING_ENABLED #undef IDSOPT_PERF_ANALYSIS #undef IDSOPT_HEAP_CHECKING - #undef IDSOPT_ASSERT_ENABLED #undef IDSOPT_ERROR_TRAP_ENABLED #undef IDSOPT_CAR_CORRUPTION_CHECK_ENABLED #undef IDSOPT_DEBUG_CODE_ENABLED @@ -277,7 +272,6 @@ #define IDSOPT_TRACING_ENABLED FALSE #define IDSOPT_PERF_ANALYSIS FALSE #define IDSOPT_HEAP_CHECKING FALSE - #define IDSOPT_ASSERT_ENABLED FALSE #define IDSOPT_ERROR_TRAP_ENABLED FALSE #define IDSOPT_CAR_CORRUPTION_CHECK_ENABLED FALSE #define IDSOPT_DEBUG_CODE_ENABLED FALSE @@ -345,34 +339,6 @@ #define STOP_HERE STOP_HERE_Needs_To_Be_Removed //"WARNING: Debug code needs to be removed for production builds." #endif
-/** - * @def ASSERT - * Test an assertion that the given statement is True. - * - * The statement is evaluated to a boolean value. If the statement is True, - * then no action is taken (no error). If the statement is False, a error stop - * is generated to halt the program. Used for testing for fatal errors that - * must be resolved before production. This is used to do parameter checks, - * bounds checking, range checks and 'sanity' checks. - * - * @param[in] conditional Assert that evaluating this conditional results in TRUE. - * - **/ -#ifndef ASSERT - #if IDSOPT_ASSERT_ENABLED == TRUE - #ifdef STOP_CODE - #undef STOP_CODE - #endif - #define STOP_CODE (((UINT32)FILECODE)*0x10000ul + \ - ((__LINE__) % 10) + (((__LINE__ / 10) % 10)*0x10) + \ - (((__LINE__ / 100) % 10)*0x100) + (((__LINE__ / 1000) % 10)*0x1000)) - - #define ASSERT(conditional) ((conditional) ? 0 : IdsAssert (STOP_CODE)); - #else - #define ASSERT(conditional) - #endif -#endif - #if IDSOPT_CAR_CORRUPTION_CHECK_ENABLED == TRUE #undef IDSOPT_ERROR_TRAP_ENABLED #define IDSOPT_ERROR_TRAP_ENABLED TRUE @@ -1251,23 +1217,6 @@ );
/** - * IDS Backend Function for ASSERT - * - * Halt execution with stop code display. Stop Code is displayed on port 80, with rotation so that - * it is visible on 8, 16, or 32 bit display. The stop code is alternated with 0xDEAD on the display, - * to help distinguish the stop code from a post code loop. - * Additional features may be available if using simulation. - * - * @param[in] FileCode File code(define in FILECODE.h) mix with assert Line num. - * - * @retval TRUE No error -**/ -BOOLEAN -IdsAssert ( - IN UINT32 FileCode - ); - -/** * The engine code for ASSERT MACRO * * Halt execution with stop code display. Stop Code is displayed on port 80, with rotation so that diff --git a/src/vendorcode/amd/agesa/f15tn/MainPage.h b/src/vendorcode/amd/agesa/f15tn/MainPage.h index 503aa4a..52094c5 100644 --- a/src/vendorcode/amd/agesa/f15tn/MainPage.h +++ b/src/vendorcode/amd/agesa/f15tn/MainPage.h @@ -90,7 +90,6 @@ * @code * #define IDSOPT_IDS_ENABLED TRUE * #define IDSOPT_ERROR_TRAP_ENABLED TRUE - * #define IDSOPT_ASSERT_ENABLED TRUE * @endcode * <li> Edit and modify the option selections in those two files to meet the needs of the specific platform. * <li> Set the environment variable AGESA_ROOT to the root folder of the AGESA code. diff --git a/src/vendorcode/amd/agesa/f15tn/Proc/IDS/Debug/IdsDebug.c b/src/vendorcode/amd/agesa/f15tn/Proc/IDS/Debug/IdsDebug.c index b5c26ae..45b25a4 100644 --- a/src/vendorcode/amd/agesa/f15tn/Proc/IDS/Debug/IdsDebug.c +++ b/src/vendorcode/amd/agesa/f15tn/Proc/IDS/Debug/IdsDebug.c @@ -78,34 +78,6 @@ }
/** - * IDS Backend Function for ASSERT - * - * Halt execution with stop code display. Stop Code is displayed on port 80, with rotation so that - * it is visible on 8, 16, or 32 bit display. The stop code is alternated with 0xDEAD on the display, - * to help distinguish the stop code from a post code loop. - * Additional features may be available if using simulation. - * - * @param[in] FileCode File code(define in FILECODE.h) mix with assert Line num. - * - * @retval TRUE No error - **/ -BOOLEAN -IdsAssert ( - IN UINT32 FileCode - ) -{ - UINT32 file; - UINT32 line; - - file = (FileCode >> 16); - line = (FileCode & 0xFFFF); - IDS_HDT_CONSOLE (MAIN_FLOW, "ASSERT on File[%x] Line[%x]\n", (UINTN) file, (UINTN) line); - IDS_HDT_CONSOLE_FLUSH_BUFFER (NULL); - IDS_HDT_CONSOLE_ASSERT (FileCode); - return TRUE; -} - -/** * IDS Backend Function for Memory timeout control * * This function is used to override Memory timeout control.