[prev in list] [next in list] [prev in thread] [next in thread]
List: fwts-devel
Subject: ACK: [PATCH] bios: mpcheck: make LEVEL macro an inlined helper function
From: ivanhu <ivan.hu () canonical ! com>
Date: 2021-04-21 7:11:38
Message-ID: 49cf660f-011c-9551-7251-a2132a6c0255 () canonical ! com
[Download RAW message or body]
On 4/12/21 5:23 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Replace LEVEL macro with a more stronly typed helper function.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> src/bios/multiproc/mpcheck.c | 51 +++++++++++++++++++-----------------
> 1 file changed, 27 insertions(+), 24 deletions(-)
>
> diff --git a/src/bios/multiproc/mpcheck.c b/src/bios/multiproc/mpcheck.c
> index 793381b4..82f1ae71 100644
> --- a/src/bios/multiproc/mpcheck.c
> +++ b/src/bios/multiproc/mpcheck.c
> @@ -25,7 +25,10 @@
> static fwts_mp_data mp_data;
> static bool fwts_mp_not_used = false;
>
> -#define LEVEL(level) fwts_mp_not_used ? LOG_LEVEL_LOW : level
> +static inline fwts_log_level fwts_level(const fwts_log_level level)
> +{
> + return fwts_mp_not_used ? LOG_LEVEL_LOW : level;
> +}
>
> static bool mpcheck_find_bus(const uint8_t id, const int depth)
> {
> @@ -86,7 +89,7 @@ static int mpcheck_test_cpu_entries(fwts_framework *fw)
> fwts_list_data(fwts_mp_processor_entry *, entry2);
>
> if (cpu_entry1->local_apic_id == cpu_entry2->local_apic_id) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPCPUEntryLAPICId",
> "CPU Entry %d (@0x%8.8" PRIx32 ")"
> " and %d (@0x%8.8" PRIx32 ") "
> @@ -105,7 +108,7 @@ static int mpcheck_test_cpu_entries(fwts_framework *fw)
> /*
> if ((cpu_entry1->local_apic_version != 0x11) &&
> (cpu_entry1->local_apic_version != 0x14)) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPCPUEntryLAPICVersion",
> "CPU Entry %d (@0x%8.8" PRIx32 ") has an "
> "invalid Local APIC Version %2.2x, should be "
> @@ -120,7 +123,7 @@ static int mpcheck_test_cpu_entries(fwts_framework *fw)
>
> if ((cpu_entry1->cpu_flags >> 1) & 1) {
> if (bootstrap_cpu != -1) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPCPUEntryBootCPU",
> "CPU Entry %d (@0x%8.8" PRIx32 ") "
> "is marked as a boot CPU but CPU entry "
> @@ -135,7 +138,7 @@ static int mpcheck_test_cpu_entries(fwts_framework *fw)
> }
>
> if (!usable_cpu_found) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPCPUEntryUsable",
> "CPU entries 0..%d were not marked as usable. "
> "There should be at least one usable CPU.",
> @@ -192,7 +195,7 @@ static int mpcheck_test_bus_entries(fwts_framework *fw)
> break;
> }
> if (bus_types[i] == NULL) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPBusEntryBusType",
> "Bus Entry %d (@0x%8.8" PRIx32 ") has an "
> "unrecognised bus type: %6.6s",
> @@ -201,7 +204,7 @@ static int mpcheck_test_bus_entries(fwts_framework *fw)
> if (prev_bus_id == -1) {
> prev_bus_id = bus_entry->bus_id;
> if (prev_bus_id != 0) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPBusEntryLAPICId",
> "Bus Entry %d (@0x%8.8" PRIx32 ") has a "
> "Local APIC ID 0x%2.2x and should be 0x00.",
> @@ -211,7 +214,7 @@ static int mpcheck_test_bus_entries(fwts_framework *fw)
> }
> } else {
> if (bus_entry->bus_id < prev_bus_id) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPBusEntryBusId",
> "Bus Entry %d (@0x%8.8" PRIx32 ") has a "
> "Bus ID 0x%2.2x and should be greater "
> @@ -245,7 +248,7 @@ static int mpcheck_test_io_apic_entries(fwts_framework *fw)
> fwts_mp_io_apic_entry *io_apic_entry = fwts_list_data(fwts_mp_io_apic_entry *, entry);
>
> if (io_apic_entry->address == 0) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPIOAPICNullAddr",
> "IO APIC Entry %d (@0x%8.8" PRIx32 ") has an "
> "invalid NULL address, should be non-zero.",
> @@ -260,7 +263,7 @@ static int mpcheck_test_io_apic_entries(fwts_framework *fw)
> }
>
> if (!enabled) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPIOAPICEnabled",
> "None of the %d IO APIC entries were enabled, "
> "at least one must be enabled.", n);
> @@ -314,7 +317,7 @@ static int mpcheck_test_io_interrupt_entries(fwts_framework *fw)
> fwts_list_data(fwts_mp_io_interrupt_entry *, entry);
>
> if (io_interrupt_entry->type > 3) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPIOIRQType",
> "IO Interrupt Entry %d (@0x%8.8" PRIx32 ") has a "
> "Type 0x%2.2" PRIx8 " and should be 0x00..0x03.",
> @@ -323,7 +326,7 @@ static int mpcheck_test_io_interrupt_entries(fwts_framework *fw)
> failed = true;
> }
> if (!mpcheck_find_io_apic(io_interrupt_entry->destination_io_apic_id)) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPIOAPICId",
> "IO Interrupt Entry %d (@0x%8.8" PRIx32 ") has a "
> "Destination IO APIC ID 0x%2.2" PRIx8 " "
> @@ -355,7 +358,7 @@ static int mpcheck_test_local_interrupt_entries(fwts_framework *fw)
> fwts_mp_local_interrupt_entry *local_interrupt_entry =
> fwts_list_data(fwts_mp_local_interrupt_entry *, entry);
> if (local_interrupt_entry->type > 3) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPLocalIRQType",
> "Local Interrupt Entry %d (@0x%8.8" PRIx32 ") has a "
> "Type 0x%2.2" PRIx8 " and should be 0x00..0x03.",
> @@ -365,7 +368,7 @@ static int mpcheck_test_local_interrupt_entries(fwts_framework *fw)
> }
> #if 0
> if (!mpcheck_find_io_apic(local_interrupt_entry->destination_local_apic_id)) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_HIGH),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_HIGH),
> "MPLocalIRQDestIRQAPIDId",
> "Local Interrupt Entry %d (@0x%8.8" PRIx32 ") has a"
> "Destination IO APIC ID 0x%2.2" PRIx8 " "
> @@ -399,7 +402,7 @@ static int mpcheck_test_sys_addr_entries(fwts_framework *fw)
> fwts_list_data(fwts_mp_system_address_space_entry *, entry);
>
> if (!mpcheck_find_bus(sys_addr_entry->bus_id, 0)) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPSysAddrSpaceBusId",
> "System Address Space Mapping Entry %d (@0x%8.8" PRIx32 ") "
> "has an Bus ID 0x%2.2" PRIx8 " "
> @@ -408,7 +411,7 @@ static int mpcheck_test_sys_addr_entries(fwts_framework *fw)
> failed = true;
> }
> if (sys_addr_entry->address_type > 3) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPSysAddrSpaceType",
> "System Address Space Mapping Entry %d (@0x%8.8" PRIx32 ") "
> "has an incorrect Address Type: %2.2" PRIx8 ", "
> @@ -418,7 +421,7 @@ static int mpcheck_test_sys_addr_entries(fwts_framework *fw)
> failed = true;
> }
> if (sys_addr_entry->address_length == 0) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPSysAddrSpaceAddrLength",
> "System Address Space Mapping Entry %d "
> "(@0x%8.8" PRIx32 ") has a "
> @@ -449,7 +452,7 @@ static int mpcheck_test_bus_hierarchy_entries(fwts_framework *fw)
> fwts_mp_bus_hierarchy_entry *bus_hierarchy_entry =
> fwts_list_data(fwts_mp_bus_hierarchy_entry *, entry);
> if (bus_hierarchy_entry->length != 8) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPBusHieraracyLength",
> "Bus Hierarchy Entry %d (@x%8.8" PRIx32 ") "
> "length was 0x%2.2" PRIx8 ", it should be 0x08.",
> @@ -458,7 +461,7 @@ static int mpcheck_test_bus_hierarchy_entries(fwts_framework *fw)
> failed = true;
> }
> if (!mpcheck_find_bus(bus_hierarchy_entry->parent_bus, 0)) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPBusHierarchyParents",
> "Bus Hierarchy Entry %d (@x%8.8" PRIx32 ") "
> "did not have parents that "
> @@ -490,7 +493,7 @@ static int mpcheck_test_compat_bus_address_space_entries(fwts_framework *fw)
> fwts_list_data(fwts_mp_compat_bus_address_space_entry*, entry);
>
> if (compat_bus_entry->length != 8) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPCompatBusLength",
> "Compatible Bus Address Space Entry %d "
> "(@x%8.8" PRIx32 ") "
> @@ -500,7 +503,7 @@ static int mpcheck_test_compat_bus_address_space_entries(fwts_framework *fw)
> failed = true;
> }
> if (compat_bus_entry->range_list > 1) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPCompatBusRangeList",
> "Compatible Bus Address Space Entry %d "
> "(@x%8.8" PRIx32 ") Range List was 0x%8.8" PRIx32
> @@ -580,7 +583,7 @@ static int mpcheck_test_header(fwts_framework *fw)
> bool failed = false;
>
> if (strncmp((char*)mp_data.header->signature, FWTS_MP_HEADER_SIGNATURE, 4)) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPHeaderSig",
> "MP header signature should be %s, got %4.4s.",
> FWTS_MP_HEADER_SIGNATURE, mp_data.header->signature);
> @@ -588,14 +591,14 @@ static int mpcheck_test_header(fwts_framework *fw)
> }
>
> if ((mp_data.header->spec_rev != 1) && (mp_data.header->spec_rev != 4)) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPHeaderRevision",
> "MP header spec revision should be 1 or 4, got %" PRIx8 ".",
> mp_data.header->spec_rev);
> failed = true;
> }
> if (mp_data.header->lapic_address == 0) {
> - fwts_failed(fw, LEVEL(LOG_LEVEL_MEDIUM),
> + fwts_failed(fw, fwts_level(LOG_LEVEL_MEDIUM),
> "MPHeaderLAPICAddrNull",
> "MP header LAPIC address is NULL.");
> failed = true;
>
Acked-by: Ivan Hu <ivan.hu@canonical.com>
--
fwts-devel mailing list
fwts-devel@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic