[PATCH 2/3] dmi_decode: Only be pedantic if we are sure about the board type (LP:#1021674)
Colin King
colin.king at canonical.com
Fri Jul 6 14:30:19 UTC 2012
From: Colin Ian King <colin.king at 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 at 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
More information about the fwts-devel
mailing list