Patchwork [2/3] dmi_decode: Only be pedantic if we are sure about the board type (LP:#1021674)

login
register
mail settings
Submitter Colin King
Date July 6, 2012, 2:30 p.m.
Message ID <1341585020-24134-3-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/169478/
State Accepted
Headers show

Comments

Colin King - July 6, 2012, 2:30 p.m.
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(-)
Keng-Yu Lin - July 9, 2012, 8:03 a.m.
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>
Ivan Hu - July 9, 2012, 9:03 a.m.
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>

Patch

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 "