| Message ID | 1341585020-24134-3-git-send-email-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Jul 6, 2012 at 10:30 PM, Colin King <colin.king@canonical.com> wrote: > From: Colin Ian King <colin.king@canonical.com> > > We only should really compare board type and preferred PM profile if we > are totally sure about the the types. If either are CHASSIS_OTHER then > don't bother being pedantic and ship the check. Also, because CERT can't > figure out if this error is bad or not, add some advice text to fully > explain what this error really means. > > To make this more friendly, I've added the a textual description of the > chassis type and PM preferred profiles rather than just dump out some > undecipherable magic numbers. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/dmi/dmi_decode/dmi_decode.c | 131 +++++++++++++++++++++++++-------------- > 1 file changed, 84 insertions(+), 47 deletions(-) > > diff --git a/src/dmi/dmi_decode/dmi_decode.c b/src/dmi/dmi_decode/dmi_decode.c > index f8c37ed..67dfea2 100644 > --- a/src/dmi/dmi_decode/dmi_decode.c > +++ b/src/dmi/dmi_decode/dmi_decode.c > @@ -73,6 +73,7 @@ typedef struct { > } fwts_dmi_version; > > typedef struct { > + const char *name; > uint8_t original; > uint8_t mapped; > } fwts_chassis_type_map; > @@ -90,48 +91,48 @@ static const char *uuid_patterns[] = { > }; > > static const fwts_chassis_type_map fwts_dmi_chassis_type[] = { > - { FWTS_SMBIOS_CHASSIS_INVALID, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_OTHER, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_UNKNOWN, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_DESKTOP, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_LOW_PROFILE_DESKTOP, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_PIZZA_BOX, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_MINI_TOWER, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_TOWER, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_PORTABLE, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_LAPTOP, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_NOTEBOOK, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_HANDHELD, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_DOCKING_STATION, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_ALL_IN_ONE, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_SUB_NOTEBOOK, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_SPACE_SAVING, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_LUNCH_BOX, CHASSIS_DESKTOP | CHASSIS_MOBILE}, > - { FWTS_SMBIOS_CHASSIS_MAIN_SERVER_CHASSIS, CHASSIS_SERVER }, > - { FWTS_SMBIOS_CHASSIS_EXPANISON_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_SUB_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_BUS_EXPANSION_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_PERIPHERAL_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_RAID_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_RACK_MOUNT_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_SEALED_CASE_PC, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_MULTI_SYSTEM_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_COMPACT_PCI, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_ADVANCED_TCA, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_BLADE, CHASSIS_SERVER }, > - { FWTS_SMBIOS_CHASSIS_BLASE_ENCLOSURE, CHASSIS_SERVER } > + { "Invalid", FWTS_SMBIOS_CHASSIS_INVALID, CHASSIS_OTHER }, > + { "Other", FWTS_SMBIOS_CHASSIS_OTHER, CHASSIS_OTHER }, > + { "Unknown", FWTS_SMBIOS_CHASSIS_UNKNOWN, CHASSIS_OTHER }, > + { "Desktop", FWTS_SMBIOS_CHASSIS_DESKTOP, CHASSIS_DESKTOP }, > + { "Low Profile Desktop",FWTS_SMBIOS_CHASSIS_LOW_PROFILE_DESKTOP, CHASSIS_DESKTOP }, > + { "Pizza Box", FWTS_SMBIOS_CHASSIS_PIZZA_BOX, CHASSIS_DESKTOP }, > + { "Mini Tower", FWTS_SMBIOS_CHASSIS_MINI_TOWER, CHASSIS_DESKTOP }, > + { "Chassis Tower", FWTS_SMBIOS_CHASSIS_TOWER, CHASSIS_DESKTOP }, > + { "Portable", FWTS_SMBIOS_CHASSIS_PORTABLE, CHASSIS_MOBILE }, > + { "Laptop", FWTS_SMBIOS_CHASSIS_LAPTOP, CHASSIS_MOBILE }, > + { "Notebook", FWTS_SMBIOS_CHASSIS_NOTEBOOK, CHASSIS_MOBILE }, > + { "Handheld", FWTS_SMBIOS_CHASSIS_HANDHELD, CHASSIS_MOBILE }, > + { "Docking Station", FWTS_SMBIOS_CHASSIS_DOCKING_STATION, CHASSIS_DESKTOP }, > + { "All In One", FWTS_SMBIOS_CHASSIS_ALL_IN_ONE, CHASSIS_DESKTOP }, > + { "Sub Notebook", FWTS_SMBIOS_CHASSIS_SUB_NOTEBOOK, CHASSIS_MOBILE }, > + { "Space Saving", FWTS_SMBIOS_CHASSIS_SPACE_SAVING, CHASSIS_DESKTOP }, > + { "Lunch Box", FWTS_SMBIOS_CHASSIS_LUNCH_BOX, CHASSIS_DESKTOP | CHASSIS_MOBILE}, > + { "Server Chassis", FWTS_SMBIOS_CHASSIS_MAIN_SERVER_CHASSIS, CHASSIS_SERVER }, > + { "Expansion Chassis", FWTS_SMBIOS_CHASSIS_EXPANISON_CHASSIS, CHASSIS_OTHER }, > + { "Sub Chassis", FWTS_SMBIOS_CHASSIS_SUB_CHASSIS, CHASSIS_OTHER }, > + { "Bus Expansion Chassis", FWTS_SMBIOS_CHASSIS_BUS_EXPANSION_CHASSIS, CHASSIS_OTHER }, > + { "Peripheral Chassis", FWTS_SMBIOS_CHASSIS_PERIPHERAL_CHASSIS, CHASSIS_OTHER }, > + { "Raid Chassis", FWTS_SMBIOS_CHASSIS_RAID_CHASSIS, CHASSIS_OTHER }, > + { "Rack Mount Chassis", FWTS_SMBIOS_CHASSIS_RACK_MOUNT_CHASSIS, CHASSIS_OTHER }, > + { "Sealed Case PC", FWTS_SMBIOS_CHASSIS_SEALED_CASE_PC, CHASSIS_DESKTOP }, > + { "Multi System Chassis",FWTS_SMBIOS_CHASSIS_MULTI_SYSTEM_CHASSIS, CHASSIS_OTHER }, > + { "Compact PCI", FWTS_SMBIOS_CHASSIS_COMPACT_PCI, CHASSIS_OTHER }, > + { "Advanced TCA", FWTS_SMBIOS_CHASSIS_ADVANCED_TCA, CHASSIS_OTHER }, > + { "Blade", FWTS_SMBIOS_CHASSIS_BLADE, CHASSIS_SERVER }, > + { "Enclosure", FWTS_SMBIOS_CHASSIS_BLASE_ENCLOSURE, CHASSIS_SERVER } > }; > > static const fwts_chassis_type_map fwts_acpi_pm_profile_type[] = { > - { FWTS_FACP_UNSPECIFIED, CHASSIS_OTHER }, > - { FWTS_FACP_DESKTOP, CHASSIS_DESKTOP }, > - { FWTS_FACP_MOBILE, CHASSIS_MOBILE }, > - { FWTS_FACP_WORKSTATION, CHASSIS_WORKSTATION }, > - { FWTS_FACP_ENTERPRISE_SERVER, CHASSIS_SERVER }, > - { FWTS_FACP_SOHO_SERVER, CHASSIS_SERVER | CHASSIS_DESKTOP }, > - { FWTS_FACP_APPLIANCE_PC, CHASSIS_DESKTOP }, > - { FWTS_FACP_PERFORMANCE_SERVER, CHASSIS_SERVER }, > - { FWTS_FACP_TABLET, CHASSIS_MOBILE } > + { "Unspecified", FWTS_FACP_UNSPECIFIED, CHASSIS_OTHER }, > + { "Desktop", FWTS_FACP_DESKTOP, CHASSIS_DESKTOP }, > + { "Mobile", FWTS_FACP_MOBILE, CHASSIS_MOBILE }, > + { "Workstation", FWTS_FACP_WORKSTATION, CHASSIS_WORKSTATION }, > + { "Enterprise Server", FWTS_FACP_ENTERPRISE_SERVER, CHASSIS_SERVER }, > + { "SOHO Server", FWTS_FACP_SOHO_SERVER, CHASSIS_SERVER | CHASSIS_DESKTOP }, > + { "Appliance PC", FWTS_FACP_APPLIANCE_PC, CHASSIS_DESKTOP }, > + { "Performance Server", FWTS_FACP_PERFORMANCE_SERVER, CHASSIS_SERVER }, > + { "Tablet", FWTS_FACP_TABLET, CHASSIS_MOBILE } > }; > > /* Remapping table from buggy version numbers to correct values */ > @@ -300,8 +301,10 @@ static void dmi_decode_entry(fwts_framework *fw, > int failed_count = fw->minor_tests.failed; > int battery_count; > int ret; > + int both_ok; > fwts_acpi_table_info *acpi_table; > fwts_acpi_table_fadt *fadt; > + bool advice_given = false; > > switch (hdr->type) { > case 0: /* 7.1 */ > @@ -359,6 +362,7 @@ static void dmi_decode_entry(fwts_framework *fw, > if (acpi_table == NULL) > break; > fadt = (fwts_acpi_table_fadt *)acpi_table->data; > + > if (fadt->preferred_pm_profile >= > (sizeof(fwts_acpi_pm_profile_type) / sizeof(fwts_chassis_type_map))) { > fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY, > @@ -371,18 +375,51 @@ static void dmi_decode_entry(fwts_framework *fw, > (sizeof(fwts_dmi_chassis_type) / sizeof(fwts_chassis_type_map))) { > fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY, > "Incorrect Chassis Type " > - "SMBIOS Type 3 reports 0x%x ", > + "SMBIOS Type 3 reports 0x%x", > data[5]); > break; > } > - if (!(fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].mapped & > - fwts_dmi_chassis_type[data[5]].mapped)) { > + > + /* > + * LP: #1021674 > + * We should only check profile and chassis types when we know for sure > + * that we can compare valid types togther. So it is only valid to do > + * a check if both aren't CHASSIS_OTHER types > + */ > + both_ok = (fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].mapped != CHASSIS_OTHER) & > + (fwts_dmi_chassis_type[data[5]].mapped != CHASSIS_OTHER); > + > + if ( both_ok && > + !(fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].mapped & > + fwts_dmi_chassis_type[data[5]].mapped)) { > fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY, > - "Unmatched Chassis Type " > - "SMBIOS Type 3 reports 0x%x " > - "ACPI FACP reports 0x%x", > + "Unmatched Chassis Type: " > + "SMBIOS Type 3 reports 0x%x '%s' " > + "ACPI FACP reports 0x%x '%s'", > data[5], > - fadt->preferred_pm_profile); > + fwts_dmi_chassis_type[data[5]].name, > + fadt->preferred_pm_profile, > + fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].name); > + /* > + * Make it a bit more wordy to explain the ramifications > + */ > + fwts_advice(fw, > + "The SMBIOS System Enclosure/Chassis type is defined as " > + "0x%x '%s' where as the ACPI FACP reports the preferred " > + "power management profile is 0x%x '%s' so we possibly " > + "have conflicting definitions of the machine's PM profile " > + "for the type of machine. " > + "See Table 16 of section 4.5.1 of the SMBIOS specification " > + "and Table 5-34 of section 5.2.9 of the ACPI specification " > + "for more details. " > + "This kind of mismatch may lead to incorrect power " > + "management handling for the type of workload expected " > + "for this hardware.", > + data[5], > + fwts_dmi_chassis_type[data[5]].name, > + fadt->preferred_pm_profile, > + fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].name); > + advice_given = true; > } > dmi_min_max_mask_uint8_check(fw, table, addr, "Chassis Lock", hdr, 0x5, 0x0, 0x1, 0x7, 0x1); > dmi_str_check(fw, table, addr, "Version", hdr, 0x6); > @@ -956,7 +993,7 @@ static void dmi_decode_entry(fwts_framework *fw, > } > if (fw->minor_tests.failed == failed_count) > fwts_passed(fw, "Entry @ 0x%8.8x '%s'", addr, table); > - else if (hdr->type <= 42) > + else if (!advice_given && hdr->type <= 42) > fwts_advice(fw, > "It may be worth checking against section 7.%d of the " > "System Management BIOS (SMBIOS) Reference Specification " > -- > 1.7.10.4 > Acked-by: Keng-Yu Lin <kengyu@canonical.com>
On 07/06/2012 10:30 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > We only should really compare board type and preferred PM profile if we > are totally sure about the the types. If either are CHASSIS_OTHER then > don't bother being pedantic and ship the check. Also, because CERT can't > figure out if this error is bad or not, add some advice text to fully > explain what this error really means. > > To make this more friendly, I've added the a textual description of the > chassis type and PM preferred profiles rather than just dump out some > undecipherable magic numbers. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/dmi/dmi_decode/dmi_decode.c | 131 +++++++++++++++++++++++++-------------- > 1 file changed, 84 insertions(+), 47 deletions(-) > > diff --git a/src/dmi/dmi_decode/dmi_decode.c b/src/dmi/dmi_decode/dmi_decode.c > index f8c37ed..67dfea2 100644 > --- a/src/dmi/dmi_decode/dmi_decode.c > +++ b/src/dmi/dmi_decode/dmi_decode.c > @@ -73,6 +73,7 @@ typedef struct { > } fwts_dmi_version; > > typedef struct { > + const char *name; > uint8_t original; > uint8_t mapped; > } fwts_chassis_type_map; > @@ -90,48 +91,48 @@ static const char *uuid_patterns[] = { > }; > > static const fwts_chassis_type_map fwts_dmi_chassis_type[] = { > - { FWTS_SMBIOS_CHASSIS_INVALID, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_OTHER, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_UNKNOWN, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_DESKTOP, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_LOW_PROFILE_DESKTOP, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_PIZZA_BOX, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_MINI_TOWER, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_TOWER, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_PORTABLE, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_LAPTOP, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_NOTEBOOK, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_HANDHELD, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_DOCKING_STATION, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_ALL_IN_ONE, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_SUB_NOTEBOOK, CHASSIS_MOBILE }, > - { FWTS_SMBIOS_CHASSIS_SPACE_SAVING, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_LUNCH_BOX, CHASSIS_DESKTOP | CHASSIS_MOBILE}, > - { FWTS_SMBIOS_CHASSIS_MAIN_SERVER_CHASSIS, CHASSIS_SERVER }, > - { FWTS_SMBIOS_CHASSIS_EXPANISON_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_SUB_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_BUS_EXPANSION_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_PERIPHERAL_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_RAID_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_RACK_MOUNT_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_SEALED_CASE_PC, CHASSIS_DESKTOP }, > - { FWTS_SMBIOS_CHASSIS_MULTI_SYSTEM_CHASSIS, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_COMPACT_PCI, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_ADVANCED_TCA, CHASSIS_OTHER }, > - { FWTS_SMBIOS_CHASSIS_BLADE, CHASSIS_SERVER }, > - { FWTS_SMBIOS_CHASSIS_BLASE_ENCLOSURE, CHASSIS_SERVER } > + { "Invalid", FWTS_SMBIOS_CHASSIS_INVALID, CHASSIS_OTHER }, > + { "Other", FWTS_SMBIOS_CHASSIS_OTHER, CHASSIS_OTHER }, > + { "Unknown", FWTS_SMBIOS_CHASSIS_UNKNOWN, CHASSIS_OTHER }, > + { "Desktop", FWTS_SMBIOS_CHASSIS_DESKTOP, CHASSIS_DESKTOP }, > + { "Low Profile Desktop",FWTS_SMBIOS_CHASSIS_LOW_PROFILE_DESKTOP, CHASSIS_DESKTOP }, > + { "Pizza Box", FWTS_SMBIOS_CHASSIS_PIZZA_BOX, CHASSIS_DESKTOP }, > + { "Mini Tower", FWTS_SMBIOS_CHASSIS_MINI_TOWER, CHASSIS_DESKTOP }, > + { "Chassis Tower", FWTS_SMBIOS_CHASSIS_TOWER, CHASSIS_DESKTOP }, > + { "Portable", FWTS_SMBIOS_CHASSIS_PORTABLE, CHASSIS_MOBILE }, > + { "Laptop", FWTS_SMBIOS_CHASSIS_LAPTOP, CHASSIS_MOBILE }, > + { "Notebook", FWTS_SMBIOS_CHASSIS_NOTEBOOK, CHASSIS_MOBILE }, > + { "Handheld", FWTS_SMBIOS_CHASSIS_HANDHELD, CHASSIS_MOBILE }, > + { "Docking Station", FWTS_SMBIOS_CHASSIS_DOCKING_STATION, CHASSIS_DESKTOP }, > + { "All In One", FWTS_SMBIOS_CHASSIS_ALL_IN_ONE, CHASSIS_DESKTOP }, > + { "Sub Notebook", FWTS_SMBIOS_CHASSIS_SUB_NOTEBOOK, CHASSIS_MOBILE }, > + { "Space Saving", FWTS_SMBIOS_CHASSIS_SPACE_SAVING, CHASSIS_DESKTOP }, > + { "Lunch Box", FWTS_SMBIOS_CHASSIS_LUNCH_BOX, CHASSIS_DESKTOP | CHASSIS_MOBILE}, > + { "Server Chassis", FWTS_SMBIOS_CHASSIS_MAIN_SERVER_CHASSIS, CHASSIS_SERVER }, > + { "Expansion Chassis", FWTS_SMBIOS_CHASSIS_EXPANISON_CHASSIS, CHASSIS_OTHER }, > + { "Sub Chassis", FWTS_SMBIOS_CHASSIS_SUB_CHASSIS, CHASSIS_OTHER }, > + { "Bus Expansion Chassis", FWTS_SMBIOS_CHASSIS_BUS_EXPANSION_CHASSIS, CHASSIS_OTHER }, > + { "Peripheral Chassis", FWTS_SMBIOS_CHASSIS_PERIPHERAL_CHASSIS, CHASSIS_OTHER }, > + { "Raid Chassis", FWTS_SMBIOS_CHASSIS_RAID_CHASSIS, CHASSIS_OTHER }, > + { "Rack Mount Chassis", FWTS_SMBIOS_CHASSIS_RACK_MOUNT_CHASSIS, CHASSIS_OTHER }, > + { "Sealed Case PC", FWTS_SMBIOS_CHASSIS_SEALED_CASE_PC, CHASSIS_DESKTOP }, > + { "Multi System Chassis",FWTS_SMBIOS_CHASSIS_MULTI_SYSTEM_CHASSIS, CHASSIS_OTHER }, > + { "Compact PCI", FWTS_SMBIOS_CHASSIS_COMPACT_PCI, CHASSIS_OTHER }, > + { "Advanced TCA", FWTS_SMBIOS_CHASSIS_ADVANCED_TCA, CHASSIS_OTHER }, > + { "Blade", FWTS_SMBIOS_CHASSIS_BLADE, CHASSIS_SERVER }, > + { "Enclosure", FWTS_SMBIOS_CHASSIS_BLASE_ENCLOSURE, CHASSIS_SERVER } > }; > > static const fwts_chassis_type_map fwts_acpi_pm_profile_type[] = { > - { FWTS_FACP_UNSPECIFIED, CHASSIS_OTHER }, > - { FWTS_FACP_DESKTOP, CHASSIS_DESKTOP }, > - { FWTS_FACP_MOBILE, CHASSIS_MOBILE }, > - { FWTS_FACP_WORKSTATION, CHASSIS_WORKSTATION }, > - { FWTS_FACP_ENTERPRISE_SERVER, CHASSIS_SERVER }, > - { FWTS_FACP_SOHO_SERVER, CHASSIS_SERVER | CHASSIS_DESKTOP }, > - { FWTS_FACP_APPLIANCE_PC, CHASSIS_DESKTOP }, > - { FWTS_FACP_PERFORMANCE_SERVER, CHASSIS_SERVER }, > - { FWTS_FACP_TABLET, CHASSIS_MOBILE } > + { "Unspecified", FWTS_FACP_UNSPECIFIED, CHASSIS_OTHER }, > + { "Desktop", FWTS_FACP_DESKTOP, CHASSIS_DESKTOP }, > + { "Mobile", FWTS_FACP_MOBILE, CHASSIS_MOBILE }, > + { "Workstation", FWTS_FACP_WORKSTATION, CHASSIS_WORKSTATION }, > + { "Enterprise Server", FWTS_FACP_ENTERPRISE_SERVER, CHASSIS_SERVER }, > + { "SOHO Server", FWTS_FACP_SOHO_SERVER, CHASSIS_SERVER | CHASSIS_DESKTOP }, > + { "Appliance PC", FWTS_FACP_APPLIANCE_PC, CHASSIS_DESKTOP }, > + { "Performance Server", FWTS_FACP_PERFORMANCE_SERVER, CHASSIS_SERVER }, > + { "Tablet", FWTS_FACP_TABLET, CHASSIS_MOBILE } > }; > > /* Remapping table from buggy version numbers to correct values */ > @@ -300,8 +301,10 @@ static void dmi_decode_entry(fwts_framework *fw, > int failed_count = fw->minor_tests.failed; > int battery_count; > int ret; > + int both_ok; > fwts_acpi_table_info *acpi_table; > fwts_acpi_table_fadt *fadt; > + bool advice_given = false; > > switch (hdr->type) { > case 0: /* 7.1 */ > @@ -359,6 +362,7 @@ static void dmi_decode_entry(fwts_framework *fw, > if (acpi_table == NULL) > break; > fadt = (fwts_acpi_table_fadt *)acpi_table->data; > + > if (fadt->preferred_pm_profile >= > (sizeof(fwts_acpi_pm_profile_type) / sizeof(fwts_chassis_type_map))) { > fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY, > @@ -371,18 +375,51 @@ static void dmi_decode_entry(fwts_framework *fw, > (sizeof(fwts_dmi_chassis_type) / sizeof(fwts_chassis_type_map))) { > fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY, > "Incorrect Chassis Type " > - "SMBIOS Type 3 reports 0x%x ", > + "SMBIOS Type 3 reports 0x%x", > data[5]); > break; > } > - if (!(fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].mapped & > - fwts_dmi_chassis_type[data[5]].mapped)) { > + > + /* > + * LP: #1021674 > + * We should only check profile and chassis types when we know for sure > + * that we can compare valid types togther. So it is only valid to do > + * a check if both aren't CHASSIS_OTHER types > + */ > + both_ok = (fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].mapped != CHASSIS_OTHER) & > + (fwts_dmi_chassis_type[data[5]].mapped != CHASSIS_OTHER); > + > + if ( both_ok && > + !(fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].mapped & > + fwts_dmi_chassis_type[data[5]].mapped)) { > fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY, > - "Unmatched Chassis Type " > - "SMBIOS Type 3 reports 0x%x " > - "ACPI FACP reports 0x%x", > + "Unmatched Chassis Type: " > + "SMBIOS Type 3 reports 0x%x '%s' " > + "ACPI FACP reports 0x%x '%s'", > data[5], > - fadt->preferred_pm_profile); > + fwts_dmi_chassis_type[data[5]].name, > + fadt->preferred_pm_profile, > + fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].name); > + /* > + * Make it a bit more wordy to explain the ramifications > + */ > + fwts_advice(fw, > + "The SMBIOS System Enclosure/Chassis type is defined as " > + "0x%x '%s' where as the ACPI FACP reports the preferred " > + "power management profile is 0x%x '%s' so we possibly " > + "have conflicting definitions of the machine's PM profile " > + "for the type of machine. " > + "See Table 16 of section 4.5.1 of the SMBIOS specification " > + "and Table 5-34 of section 5.2.9 of the ACPI specification " > + "for more details. " > + "This kind of mismatch may lead to incorrect power " > + "management handling for the type of workload expected " > + "for this hardware.", > + data[5], > + fwts_dmi_chassis_type[data[5]].name, > + fadt->preferred_pm_profile, > + fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].name); > + advice_given = true; > } > dmi_min_max_mask_uint8_check(fw, table, addr, "Chassis Lock", hdr, 0x5, 0x0, 0x1, 0x7, 0x1); > dmi_str_check(fw, table, addr, "Version", hdr, 0x6); > @@ -956,7 +993,7 @@ static void dmi_decode_entry(fwts_framework *fw, > } > if (fw->minor_tests.failed == failed_count) > fwts_passed(fw, "Entry @ 0x%8.8x '%s'", addr, table); > - else if (hdr->type <= 42) > + else if (!advice_given && hdr->type <= 42) > fwts_advice(fw, > "It may be worth checking against section 7.%d of the " > "System Management BIOS (SMBIOS) Reference Specification " > Acked-by: Ivan Hu<ivan.hu@canonical.com>
diff --git a/src/dmi/dmi_decode/dmi_decode.c b/src/dmi/dmi_decode/dmi_decode.c index f8c37ed..67dfea2 100644 --- a/src/dmi/dmi_decode/dmi_decode.c +++ b/src/dmi/dmi_decode/dmi_decode.c @@ -73,6 +73,7 @@ typedef struct { } fwts_dmi_version; typedef struct { + const char *name; uint8_t original; uint8_t mapped; } fwts_chassis_type_map; @@ -90,48 +91,48 @@ static const char *uuid_patterns[] = { }; static const fwts_chassis_type_map fwts_dmi_chassis_type[] = { - { FWTS_SMBIOS_CHASSIS_INVALID, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_OTHER, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_UNKNOWN, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_DESKTOP, CHASSIS_DESKTOP }, - { FWTS_SMBIOS_CHASSIS_LOW_PROFILE_DESKTOP, CHASSIS_DESKTOP }, - { FWTS_SMBIOS_CHASSIS_PIZZA_BOX, CHASSIS_DESKTOP }, - { FWTS_SMBIOS_CHASSIS_MINI_TOWER, CHASSIS_DESKTOP }, - { FWTS_SMBIOS_CHASSIS_TOWER, CHASSIS_DESKTOP }, - { FWTS_SMBIOS_CHASSIS_PORTABLE, CHASSIS_MOBILE }, - { FWTS_SMBIOS_CHASSIS_LAPTOP, CHASSIS_MOBILE }, - { FWTS_SMBIOS_CHASSIS_NOTEBOOK, CHASSIS_MOBILE }, - { FWTS_SMBIOS_CHASSIS_HANDHELD, CHASSIS_MOBILE }, - { FWTS_SMBIOS_CHASSIS_DOCKING_STATION, CHASSIS_DESKTOP }, - { FWTS_SMBIOS_CHASSIS_ALL_IN_ONE, CHASSIS_DESKTOP }, - { FWTS_SMBIOS_CHASSIS_SUB_NOTEBOOK, CHASSIS_MOBILE }, - { FWTS_SMBIOS_CHASSIS_SPACE_SAVING, CHASSIS_DESKTOP }, - { FWTS_SMBIOS_CHASSIS_LUNCH_BOX, CHASSIS_DESKTOP | CHASSIS_MOBILE}, - { FWTS_SMBIOS_CHASSIS_MAIN_SERVER_CHASSIS, CHASSIS_SERVER }, - { FWTS_SMBIOS_CHASSIS_EXPANISON_CHASSIS, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_SUB_CHASSIS, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_BUS_EXPANSION_CHASSIS, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_PERIPHERAL_CHASSIS, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_RAID_CHASSIS, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_RACK_MOUNT_CHASSIS, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_SEALED_CASE_PC, CHASSIS_DESKTOP }, - { FWTS_SMBIOS_CHASSIS_MULTI_SYSTEM_CHASSIS, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_COMPACT_PCI, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_ADVANCED_TCA, CHASSIS_OTHER }, - { FWTS_SMBIOS_CHASSIS_BLADE, CHASSIS_SERVER }, - { FWTS_SMBIOS_CHASSIS_BLASE_ENCLOSURE, CHASSIS_SERVER } + { "Invalid", FWTS_SMBIOS_CHASSIS_INVALID, CHASSIS_OTHER }, + { "Other", FWTS_SMBIOS_CHASSIS_OTHER, CHASSIS_OTHER }, + { "Unknown", FWTS_SMBIOS_CHASSIS_UNKNOWN, CHASSIS_OTHER }, + { "Desktop", FWTS_SMBIOS_CHASSIS_DESKTOP, CHASSIS_DESKTOP }, + { "Low Profile Desktop",FWTS_SMBIOS_CHASSIS_LOW_PROFILE_DESKTOP, CHASSIS_DESKTOP }, + { "Pizza Box", FWTS_SMBIOS_CHASSIS_PIZZA_BOX, CHASSIS_DESKTOP }, + { "Mini Tower", FWTS_SMBIOS_CHASSIS_MINI_TOWER, CHASSIS_DESKTOP }, + { "Chassis Tower", FWTS_SMBIOS_CHASSIS_TOWER, CHASSIS_DESKTOP }, + { "Portable", FWTS_SMBIOS_CHASSIS_PORTABLE, CHASSIS_MOBILE }, + { "Laptop", FWTS_SMBIOS_CHASSIS_LAPTOP, CHASSIS_MOBILE }, + { "Notebook", FWTS_SMBIOS_CHASSIS_NOTEBOOK, CHASSIS_MOBILE }, + { "Handheld", FWTS_SMBIOS_CHASSIS_HANDHELD, CHASSIS_MOBILE }, + { "Docking Station", FWTS_SMBIOS_CHASSIS_DOCKING_STATION, CHASSIS_DESKTOP }, + { "All In One", FWTS_SMBIOS_CHASSIS_ALL_IN_ONE, CHASSIS_DESKTOP }, + { "Sub Notebook", FWTS_SMBIOS_CHASSIS_SUB_NOTEBOOK, CHASSIS_MOBILE }, + { "Space Saving", FWTS_SMBIOS_CHASSIS_SPACE_SAVING, CHASSIS_DESKTOP }, + { "Lunch Box", FWTS_SMBIOS_CHASSIS_LUNCH_BOX, CHASSIS_DESKTOP | CHASSIS_MOBILE}, + { "Server Chassis", FWTS_SMBIOS_CHASSIS_MAIN_SERVER_CHASSIS, CHASSIS_SERVER }, + { "Expansion Chassis", FWTS_SMBIOS_CHASSIS_EXPANISON_CHASSIS, CHASSIS_OTHER }, + { "Sub Chassis", FWTS_SMBIOS_CHASSIS_SUB_CHASSIS, CHASSIS_OTHER }, + { "Bus Expansion Chassis", FWTS_SMBIOS_CHASSIS_BUS_EXPANSION_CHASSIS, CHASSIS_OTHER }, + { "Peripheral Chassis", FWTS_SMBIOS_CHASSIS_PERIPHERAL_CHASSIS, CHASSIS_OTHER }, + { "Raid Chassis", FWTS_SMBIOS_CHASSIS_RAID_CHASSIS, CHASSIS_OTHER }, + { "Rack Mount Chassis", FWTS_SMBIOS_CHASSIS_RACK_MOUNT_CHASSIS, CHASSIS_OTHER }, + { "Sealed Case PC", FWTS_SMBIOS_CHASSIS_SEALED_CASE_PC, CHASSIS_DESKTOP }, + { "Multi System Chassis",FWTS_SMBIOS_CHASSIS_MULTI_SYSTEM_CHASSIS, CHASSIS_OTHER }, + { "Compact PCI", FWTS_SMBIOS_CHASSIS_COMPACT_PCI, CHASSIS_OTHER }, + { "Advanced TCA", FWTS_SMBIOS_CHASSIS_ADVANCED_TCA, CHASSIS_OTHER }, + { "Blade", FWTS_SMBIOS_CHASSIS_BLADE, CHASSIS_SERVER }, + { "Enclosure", FWTS_SMBIOS_CHASSIS_BLASE_ENCLOSURE, CHASSIS_SERVER } }; static const fwts_chassis_type_map fwts_acpi_pm_profile_type[] = { - { FWTS_FACP_UNSPECIFIED, CHASSIS_OTHER }, - { FWTS_FACP_DESKTOP, CHASSIS_DESKTOP }, - { FWTS_FACP_MOBILE, CHASSIS_MOBILE }, - { FWTS_FACP_WORKSTATION, CHASSIS_WORKSTATION }, - { FWTS_FACP_ENTERPRISE_SERVER, CHASSIS_SERVER }, - { FWTS_FACP_SOHO_SERVER, CHASSIS_SERVER | CHASSIS_DESKTOP }, - { FWTS_FACP_APPLIANCE_PC, CHASSIS_DESKTOP }, - { FWTS_FACP_PERFORMANCE_SERVER, CHASSIS_SERVER }, - { FWTS_FACP_TABLET, CHASSIS_MOBILE } + { "Unspecified", FWTS_FACP_UNSPECIFIED, CHASSIS_OTHER }, + { "Desktop", FWTS_FACP_DESKTOP, CHASSIS_DESKTOP }, + { "Mobile", FWTS_FACP_MOBILE, CHASSIS_MOBILE }, + { "Workstation", FWTS_FACP_WORKSTATION, CHASSIS_WORKSTATION }, + { "Enterprise Server", FWTS_FACP_ENTERPRISE_SERVER, CHASSIS_SERVER }, + { "SOHO Server", FWTS_FACP_SOHO_SERVER, CHASSIS_SERVER | CHASSIS_DESKTOP }, + { "Appliance PC", FWTS_FACP_APPLIANCE_PC, CHASSIS_DESKTOP }, + { "Performance Server", FWTS_FACP_PERFORMANCE_SERVER, CHASSIS_SERVER }, + { "Tablet", FWTS_FACP_TABLET, CHASSIS_MOBILE } }; /* Remapping table from buggy version numbers to correct values */ @@ -300,8 +301,10 @@ static void dmi_decode_entry(fwts_framework *fw, int failed_count = fw->minor_tests.failed; int battery_count; int ret; + int both_ok; fwts_acpi_table_info *acpi_table; fwts_acpi_table_fadt *fadt; + bool advice_given = false; switch (hdr->type) { case 0: /* 7.1 */ @@ -359,6 +362,7 @@ static void dmi_decode_entry(fwts_framework *fw, if (acpi_table == NULL) break; fadt = (fwts_acpi_table_fadt *)acpi_table->data; + if (fadt->preferred_pm_profile >= (sizeof(fwts_acpi_pm_profile_type) / sizeof(fwts_chassis_type_map))) { fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY, @@ -371,18 +375,51 @@ static void dmi_decode_entry(fwts_framework *fw, (sizeof(fwts_dmi_chassis_type) / sizeof(fwts_chassis_type_map))) { fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY, "Incorrect Chassis Type " - "SMBIOS Type 3 reports 0x%x ", + "SMBIOS Type 3 reports 0x%x", data[5]); break; } - if (!(fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].mapped & - fwts_dmi_chassis_type[data[5]].mapped)) { + + /* + * LP: #1021674 + * We should only check profile and chassis types when we know for sure + * that we can compare valid types togther. So it is only valid to do + * a check if both aren't CHASSIS_OTHER types + */ + both_ok = (fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].mapped != CHASSIS_OTHER) & + (fwts_dmi_chassis_type[data[5]].mapped != CHASSIS_OTHER); + + if ( both_ok && + !(fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].mapped & + fwts_dmi_chassis_type[data[5]].mapped)) { fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY, - "Unmatched Chassis Type " - "SMBIOS Type 3 reports 0x%x " - "ACPI FACP reports 0x%x", + "Unmatched Chassis Type: " + "SMBIOS Type 3 reports 0x%x '%s' " + "ACPI FACP reports 0x%x '%s'", data[5], - fadt->preferred_pm_profile); + fwts_dmi_chassis_type[data[5]].name, + fadt->preferred_pm_profile, + fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].name); + /* + * Make it a bit more wordy to explain the ramifications + */ + fwts_advice(fw, + "The SMBIOS System Enclosure/Chassis type is defined as " + "0x%x '%s' where as the ACPI FACP reports the preferred " + "power management profile is 0x%x '%s' so we possibly " + "have conflicting definitions of the machine's PM profile " + "for the type of machine. " + "See Table 16 of section 4.5.1 of the SMBIOS specification " + "and Table 5-34 of section 5.2.9 of the ACPI specification " + "for more details. " + "This kind of mismatch may lead to incorrect power " + "management handling for the type of workload expected " + "for this hardware.", + data[5], + fwts_dmi_chassis_type[data[5]].name, + fadt->preferred_pm_profile, + fwts_acpi_pm_profile_type[fadt->preferred_pm_profile].name); + advice_given = true; } dmi_min_max_mask_uint8_check(fw, table, addr, "Chassis Lock", hdr, 0x5, 0x0, 0x1, 0x7, 0x1); dmi_str_check(fw, table, addr, "Version", hdr, 0x6); @@ -956,7 +993,7 @@ static void dmi_decode_entry(fwts_framework *fw, } if (fw->minor_tests.failed == failed_count) fwts_passed(fw, "Entry @ 0x%8.8x '%s'", addr, table); - else if (hdr->type <= 42) + else if (!advice_given && hdr->type <= 42) fwts_advice(fw, "It may be worth checking against section 7.%d of the " "System Management BIOS (SMBIOS) Reference Specification "