dmi: dmi_decode: check type 3 chassis type with acpi preferred_pm_profile

Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 src/dmi/dmi_decode/dmi_decode.c |   88 +++++++++++++++++++++++++++++++++++++++
 src/lib/include/fwts_acpi.h     |   10 ++++
 src/lib/include/fwts_smbios.h   |   31 ++++++++++++++
 3 files changed, 129 insertions(+), 0 deletions(-)

On 31/01/12 13:49, Alex Hung wrote:
> Signed-off-by: Alex Hung<alex.hung@canonical.com>
> ---
>   src/dmi/dmi_decode/dmi_decode.c |   88 +++++++++++++++++++++++++++++++++++++++
>   src/lib/include/fwts_acpi.h     |   10 ++++
>   src/lib/include/fwts_smbios.h   |   31 ++++++++++++++
>   3 files changed, 129 insertions(+), 0 deletions(-)
>
> diff --git a/src/dmi/dmi_decode/dmi_decode.c b/src/dmi/dmi_decode/dmi_decode.c
> index c922ee4..169e85c 100644
> --- a/src/dmi/dmi_decode/dmi_decode.c
> +++ b/src/dmi/dmi_decode/dmi_decode.c
> @@ -55,6 +55,12 @@
>   #define GET_UINT32(x) (uint32_t)(*(const uint32_t *)(x))
>   #define GET_UINT64(x) (uint64_t)(*(const uint64_t *)(x))
>
> +#define CHASSIS_OTHER			0x00
> +#define CHASSIS_DESKTOP			0x01
> +#define CHASSIS_WORKSTATION		0x02
> +#define CHASSIS_MOBILE			0x04
> +#define CHASSIS_SERVER			0x08
> +
>   typedef struct {
>   	const char *label;
>   	const char *field;
> @@ -66,6 +72,11 @@ typedef struct {
>   	uint16_t   new;
>   } fwts_dmi_version;
>
> +typedef struct {
> +	uint8_t   original;
> +	uint8_t   mapped;
> +} fwts_chassis_type_map;
> +
>   static const fwts_dmi_pattern dmi_patterns[] = {
>   	{ "DMISerialNumber",	"Serial Number", 	"0123456789" },
>   	{ "DMIAssetTag",	"Asset Tag",		"1234567890" },
> @@ -78,6 +89,51 @@ static const char *uuid_patterns[] = {
>   	NULL,
>   };
>
> +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 }
> +};
> +
> +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 }
> +};
> +

Didn't know TABLET was a define device - I learned something new :-)

>   /* Remapping table from buggy version numbers to correct values */
>   static const fwts_dmi_version dmi_versions[] = {
>   	{ 0x021f, 0x0203 },
> @@ -244,6 +300,8 @@ static void dmi_decode_entry(fwts_framework *fw,
>   	int	failed_count = fw->minor_tests.failed;
>   	int	battery_count;
>   	int	ret;
> +	fwts_acpi_table_info *acpi_table;
> +	fwts_acpi_table_fadt *fadt;
>
>   	switch (hdr->type) {
>   		case 0: /* 7.1 */
> @@ -296,6 +354,36 @@ static void dmi_decode_entry(fwts_framework *fw,
>   				break;
>   			dmi_str_check(fw, table, addr, "Manufacturer", hdr, 0x4);
>   			dmi_min_max_mask_uint8_check(fw, table, addr, "Chassis Type", hdr, 0x5, 0x1, 0x1d, 0x0, 0x7f);
> +			if (fwts_acpi_find_table(fw, "FACP", 0,&acpi_table) != FWTS_OK)
> +				break;
> +			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))) {

Hrm, the size of calculations here calculate the number of items in 
fwts_acpi_pm_profile_type - however the array is indexed from element 
zero, so I think the comparison should be >= rather than > to avoid 
falling off the array if fadt->preferred_pm_profile = 8

> +				fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY,
> +					"Incorrect Chassis Type "
> +					"ACPI FACP reports %x",
> +					fadt->preferred_pm_profile);
> +				break;
> +			}
> +			if (data[5]>
> +				(sizeof(fwts_acpi_pm_profile_type) / sizeof(fwts_chassis_type_map))) {

Shouldn't that be sizeof(fwts_dmi_chassis_type)?

Also, the same array indexing issue as above, e.g. make the comparison 
"data[5] >=" rather than "data[5] >".

> +				fwts_failed(fw, LOG_LEVEL_HIGH, DMI_INVALID_HARDWARE_ENTRY,
> +					"Incorrect Chassis Type "
> +					"SMBIOS Type 3 reports %x ",
> +					data[5]);
> +				break;
> +			}
> +			if (!(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 %x "
> +					"ACPI FACP reports %x",
> +					data[5],
> +					fadt->preferred_pm_profile);
> +			}
>   			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);
>   			dmi_str_check(fw, table, addr, "Serial Number", hdr, 0x7);
> diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
> index d9f74c4..5b85dc5 100644
> --- a/src/lib/include/fwts_acpi.h
> +++ b/src/lib/include/fwts_acpi.h
> @@ -22,6 +22,16 @@
>
>   #define FWTS_ACPI_TABLES_PATH   "/sys/firmware/acpi/tables"
>
> +#define FWTS_FACP_UNSPECIFIED		(0x00)
> +#define FWTS_FACP_DESKTOP		(0x01)
> +#define FWTS_FACP_MOBILE		(0x02)
> +#define FWTS_FACP_WORKSTATION		(0x03)
> +#define FWTS_FACP_ENTERPRISE_SERVER	(0x04)
> +#define FWTS_FACP_SOHO_SERVER		(0x05)
> +#define FWTS_FACP_APPLIANCE_PC		(0x06)
> +#define FWTS_FACP_PERFORMANCE_SERVER	(0x07)
> +#define FWTS_FACP_TABLET		(0x08)
> +
>   #define FWTS_FACP_IAPC_BOOT_ARCH_LEGACY_DEVICES		(0x0001)
>   #define FWTS_FACP_IAPC_BOOT_ARCH_8042			(0x0002)
>   #define FWTS_FACP_IAPC_BOOT_ARCH_VGA_NOT_PRESENT	(0x0004)
> diff --git a/src/lib/include/fwts_smbios.h b/src/lib/include/fwts_smbios.h
> index b63fe36..98479b5 100644
> --- a/src/lib/include/fwts_smbios.h
> +++ b/src/lib/include/fwts_smbios.h
> @@ -27,6 +27,37 @@
>   #define FWTS_SMBIOS_REGION_END       (0x000fffff)
>   #define FWTS_SMBIOS_REGION_SIZE      (FWTS_SMBIOS_REGION_END - FWTS_SMBIOS_REGION_START)
>
> +#define FWTS_SMBIOS_CHASSIS_INVALID			(0x00)
> +#define FWTS_SMBIOS_CHASSIS_OTHER			(0X01)
> +#define FWTS_SMBIOS_CHASSIS_UNKNOWN			(0x02)
> +#define FWTS_SMBIOS_CHASSIS_DESKTOP			(0x03)
> +#define FWTS_SMBIOS_CHASSIS_LOW_PROFILE_DESKTOP		(0x04)
> +#define FWTS_SMBIOS_CHASSIS_PIZZA_BOX			(0x05)
> +#define FWTS_SMBIOS_CHASSIS_MINI_TOWER			(0x06)
> +#define FWTS_SMBIOS_CHASSIS_TOWER			(0x07)
> +#define FWTS_SMBIOS_CHASSIS_PORTABLE			(0x08)
> +#define FWTS_SMBIOS_CHASSIS_LAPTOP			(0x09)
> +#define FWTS_SMBIOS_CHASSIS_NOTEBOOK			(0x0A)
> +#define FWTS_SMBIOS_CHASSIS_HANDHELD			(0x0B)
> +#define FWTS_SMBIOS_CHASSIS_DOCKING_STATION		(0x0C)
> +#define FWTS_SMBIOS_CHASSIS_ALL_IN_ONE			(0x0D)
> +#define FWTS_SMBIOS_CHASSIS_SUB_NOTEBOOK		(0x0E)
> +#define FWTS_SMBIOS_CHASSIS_SPACE_SAVING		(0x0F)
> +#define FWTS_SMBIOS_CHASSIS_LUNCH_BOX			(0x10)
> +#define FWTS_SMBIOS_CHASSIS_MAIN_SERVER_CHASSIS		(0x11)
> +#define FWTS_SMBIOS_CHASSIS_EXPANISON_CHASSIS		(0x12)
> +#define FWTS_SMBIOS_CHASSIS_SUB_CHASSIS			(0x13)
> +#define FWTS_SMBIOS_CHASSIS_BUS_EXPANSION_CHASSIS	(0x14)
> +#define FWTS_SMBIOS_CHASSIS_PERIPHERAL_CHASSIS		(0x15)
> +#define FWTS_SMBIOS_CHASSIS_RAID_CHASSIS		(0x16)
> +#define FWTS_SMBIOS_CHASSIS_RACK_MOUNT_CHASSIS		(0x17)
> +#define FWTS_SMBIOS_CHASSIS_SEALED_CASE_PC		(0x18)
> +#define FWTS_SMBIOS_CHASSIS_MULTI_SYSTEM_CHASSIS	(0x19)
> +#define FWTS_SMBIOS_CHASSIS_COMPACT_PCI			(0x1A)
> +#define FWTS_SMBIOS_CHASSIS_ADVANCED_TCA		(0x1B)
> +#define FWTS_SMBIOS_CHASSIS_BLADE			(0x1C)
> +#define FWTS_SMBIOS_CHASSIS_BLASE_ENCLOSURE		(0x1D)
> +
>   typedef enum {
>   	FWTS_SMBIOS_UNKNOWN = -1,
>   	FWTS_SMBIOS_DMI_LEGACY = 0,


Colin

Hi there,

While discussing some boot speed issues with the kernel with Andy 
Whitcroft we spotted that that the PCI config space is being set up by 
the BIOS and this can be sub-optimally configuring things like the SATA 
capabilities.

For example, bit 27 in HBA Capabilities for SATA devices controls the 
"Staggered Spin-up (SSS)" setting - this really makes sense on servers 
and hence should be set to zero for netbooks, laptops etc.  However, we 
observed it being incorrectly set on particular platform which can only 
have one drive in it, so SSS makes no sense.

So, perhaps we should add this to the next round of fwts blueprints - 
check for incorrect or sub-optimal incorrectly configured PCI config 
space settings for wide range of devices.

Any thoughts?

Colin

On 02/01/2012 12:54 AM, Colin Ian King wrote:
> Hi there,
>
> While discussing some boot speed issues with the kernel with Andy 
> Whitcroft we spotted that that the PCI config space is being set up by 
> the BIOS and this can be sub-optimally configuring things like the 
> SATA capabilities.
>
> For example, bit 27 in HBA Capabilities for SATA devices controls the 
> "Staggered Spin-up (SSS)" setting - this really makes sense on servers 
> and hence should be set to zero for netbooks, laptops etc.  However, 
> we observed it being incorrectly set on particular platform which can 
> only have one drive in it, so SSS makes no sense.
>
SSS looks very familiar. I recall that Intel's BIOS utility (selfcheck 
or selftest for Windows) checks this bit and it suggests a setting (I 
don't remember whether it was 1 or 0).

PS. Both Intel and AMD also have utilities that checks hardware 
registers - thought they make mistakes too.

I think it is a good idea to check hardware configuration but we will 
need to be careful. If fwts suggest different configuration from 
hardware vendors (i.e. Intel, AMD and nVidia), it will cause confusion.

> So, perhaps we should add this to the next round of fwts blueprints - 
> check for incorrect or sub-optimal incorrectly configured PCI config 
> space settings for wide range of devices.
>
> Any thoughts?
>
> Colin
>
>
>

The below is a fragment of a result from Intel's Selftest utility.

- This particular version checks for 12352 settings with Pantherpoint 
(PCH with Ivy Bridge), and it is based on BIOS Writer Guide 0.9.

     Stats    : Total:12352 Error:64 Warn:55
     
********************************************************************************* 

     Pantherpoint BIOS Spec 0.9
      BWG Version - 0.9

- It suggests that SSS needs to be enabled but BIOS disables it.
                                                         
Actual               Expected
      
--------------------------------------------------------------------------------- 

     AHCI Generic Host Control Registers
      
--------------------------------------------------------------------------------- 

     00h CAP - Host Capabilities Register
     E     [27] Supports Staggered Spin-up (SSS)         
0x0                  0x1
     See BIOS Spec 14.1.4 Initialize Registers in AHCI Memory-Mapped Space
     Attribute: R/WO
     Default: 1
     Indicates whether the SATA controller supports staggered spin-up on 
its ports, for use in balancing power spikes. This value is loaded by 
platform BIOS prior to OS initialization.
     0 = Staggered spin-up not supported.
     1 = Staggered spin-up supported.

- It shows pass if BIOS sets it right
     PASS  [4] SMI_LOCK                                  
0x1                  0x1
     See BIOS Spec 3.6 Flash Security Recommendation-- This 
recommendation is only for production BIOS
     Attribute: R/WO
     Default: 0
     When this bit is set, writes to the GLB_SMI_EN bit (PMBASE + 30h, 
bit 0)  will have no effect.
     Once the SMI_LOCK bit is set, writes of 0 to SMI_LOCK bit will have 
no effect (i.e., once set, this bit can only be cleared by PLTRST#).

- It also shows warning, i.e. when ASPM is not enabled (but this depends 
on the attached device).
     E8h PECR1 - PCI Express* Configuration Register 1
     W     [1] PECR1 Field 2                             
0x0                  0x1
     Both L0s and L1 Entry Supported.  This bit should be set to 1b

However, this utility only checks registers from CPU or chipset. No ACPI 
tables or any other interfaces are analyzed - that's where our fwts 
helps. ;-)

On 02/01/2012 05:17 PM, Alex Hung wrote:
> On 02/01/2012 12:54 AM, Colin Ian King wrote:
>> Hi there,
>>
>> While discussing some boot speed issues with the kernel with Andy 
>> Whitcroft we spotted that that the PCI config space is being set up 
>> by the BIOS and this can be sub-optimally configuring things like the 
>> SATA capabilities.
>>
>> For example, bit 27 in HBA Capabilities for SATA devices controls the 
>> "Staggered Spin-up (SSS)" setting - this really makes sense on 
>> servers and hence should be set to zero for netbooks, laptops etc.  
>> However, we observed it being incorrectly set on particular platform 
>> which can only have one drive in it, so SSS makes no sense.
>>
> SSS looks very familiar. I recall that Intel's BIOS utility (selfcheck 
> or selftest for Windows) checks this bit and it suggests a setting (I 
> don't remember whether it was 1 or 0).
>
> PS. Both Intel and AMD also have utilities that checks hardware 
> registers - thought they make mistakes too.
>
> I think it is a good idea to check hardware configuration but we will 
> need to be careful. If fwts suggest different configuration from 
> hardware vendors (i.e. Intel, AMD and nVidia), it will cause confusion.
>
>> So, perhaps we should add this to the next round of fwts blueprints - 
>> check for incorrect or sub-optimal incorrectly configured PCI config 
>> space settings for wide range of devices.
>>
>> Any thoughts?
>>
>> Colin
>>
>>
>>
>
>

On 02/02/12 03:53, Alex Hung wrote:
> The below is a fragment of a result from Intel's Selftest utility.

Oh, I don't know about Selftest - where can I get that from? - and what 
can it do?  Is it another tool that we should be using on projects by 
default to catch issues?

>
> - This particular version checks for 12352 settings with Pantherpoint
> (PCH with Ivy Bridge), and it is based on BIOS Writer Guide 0.9.
>
> Stats : Total:12352 Error:64 Warn:55
> *********************************************************************************
>
> Pantherpoint BIOS Spec 0.9
> BWG Version - 0.9
>
> - It suggests that SSS needs to be enabled but BIOS disables it.
> Actual Expected
> ---------------------------------------------------------------------------------
>
> AHCI Generic Host Control Registers
> ---------------------------------------------------------------------------------
>
> 00h CAP - Host Capabilities Register
> E [27] Supports Staggered Spin-up (SSS) 0x0 0x1
> See BIOS Spec 14.1.4 Initialize Registers in AHCI Memory-Mapped Space
> Attribute: R/WO
> Default: 1
> Indicates whether the SATA controller supports staggered spin-up on its
> ports, for use in balancing power spikes. This value is loaded by
> platform BIOS prior to OS initialization.
> 0 = Staggered spin-up not supported.
> 1 = Staggered spin-up supported.
>
> - It shows pass if BIOS sets it right
> PASS [4] SMI_LOCK 0x1 0x1
> See BIOS Spec 3.6 Flash Security Recommendation-- This recommendation is
> only for production BIOS
> Attribute: R/WO
> Default: 0
> When this bit is set, writes to the GLB_SMI_EN bit (PMBASE + 30h, bit 0)
> will have no effect.
> Once the SMI_LOCK bit is set, writes of 0 to SMI_LOCK bit will have no
> effect (i.e., once set, this bit can only be cleared by PLTRST#).
>
> - It also shows warning, i.e. when ASPM is not enabled (but this depends
> on the attached device).
> E8h PECR1 - PCI Express* Configuration Register 1
> W [1] PECR1 Field 2 0x0 0x1
> Both L0s and L1 Entry Supported. This bit should be set to 1b
>

> However, this utility only checks registers from CPU or chipset. No ACPI
> tables or any other interfaces are analyzed - that's where our fwts
> helps. ;-)

Indeed :-)

So do you think we should realistically add BIOS configured PCI config 
space checks into fwts as valuable set of tests - or does Selftest cover 
enough of what we need to check?  Perhaps this is up for Precise+1 
discussion at UDS

Colin

>
> On 02/01/2012 05:17 PM, Alex Hung wrote:
>> On 02/01/2012 12:54 AM, Colin Ian King wrote:
>>> Hi there,
>>>
>>> While discussing some boot speed issues with the kernel with Andy
>>> Whitcroft we spotted that that the PCI config space is being set up
>>> by the BIOS and this can be sub-optimally configuring things like the
>>> SATA capabilities.
>>>
>>> For example, bit 27 in HBA Capabilities for SATA devices controls the
>>> "Staggered Spin-up (SSS)" setting - this really makes sense on
>>> servers and hence should be set to zero for netbooks, laptops etc.
>>> However, we observed it being incorrectly set on particular platform
>>> which can only have one drive in it, so SSS makes no sense.
>>>
>> SSS looks very familiar. I recall that Intel's BIOS utility (selfcheck
>> or selftest for Windows) checks this bit and it suggests a setting (I
>> don't remember whether it was 1 or 0).
>>
>> PS. Both Intel and AMD also have utilities that checks hardware
>> registers - thought they make mistakes too.
>>
>> I think it is a good idea to check hardware configuration but we will
>> need to be careful. If fwts suggest different configuration from
>> hardware vendors (i.e. Intel, AMD and nVidia), it will cause confusion.
>>
>>> So, perhaps we should add this to the next round of fwts blueprints -
>>> check for incorrect or sub-optimal incorrectly configured PCI config
>>> space settings for wide range of devices.
>>>
>>> Any thoughts?
>>>
>>> Colin
>>>
>>>
>>>
>>
>>
>
>

On 02/02/2012 07:57 PM, Colin Ian King wrote:
> On 02/02/12 03:53, Alex Hung wrote:
>> The below is a fragment of a result from Intel's Selftest utility.
>
> Oh, I don't know about Selftest - where can I get that from? - and 
> what can it do?  Is it another tool that we should be using on 
> projects by default to catch issues?
It can be downloaded from Intel IBL, but an Intel account is needed. It 
only checks the registers in PCI(e) configuration space, CPU MSR or in 
memory space, based on Intel's recommendation. However, it only runs on 
Windows as far as I am aware of.

AMD has a similar utility (called BTS - BIOS Test Suite?).
>
>>
>> - This particular version checks for 12352 settings with Pantherpoint
>> (PCH with Ivy Bridge), and it is based on BIOS Writer Guide 0.9.
>>
>> Stats : Total:12352 Error:64 Warn:55
>> ********************************************************************************* 
>>
>>
>> Pantherpoint BIOS Spec 0.9
>> BWG Version - 0.9
>>
>> - It suggests that SSS needs to be enabled but BIOS disables it.
>> Actual Expected
>> --------------------------------------------------------------------------------- 
>>
>>
>> AHCI Generic Host Control Registers
>> --------------------------------------------------------------------------------- 
>>
>>
>> 00h CAP - Host Capabilities Register
>> E [27] Supports Staggered Spin-up (SSS) 0x0 0x1
>> See BIOS Spec 14.1.4 Initialize Registers in AHCI Memory-Mapped Space
>> Attribute: R/WO
>> Default: 1
>> Indicates whether the SATA controller supports staggered spin-up on its
>> ports, for use in balancing power spikes. This value is loaded by
>> platform BIOS prior to OS initialization.
>> 0 = Staggered spin-up not supported.
>> 1 = Staggered spin-up supported.
>>
>> - It shows pass if BIOS sets it right
>> PASS [4] SMI_LOCK 0x1 0x1
>> See BIOS Spec 3.6 Flash Security Recommendation-- This recommendation is
>> only for production BIOS
>> Attribute: R/WO
>> Default: 0
>> When this bit is set, writes to the GLB_SMI_EN bit (PMBASE + 30h, bit 0)
>> will have no effect.
>> Once the SMI_LOCK bit is set, writes of 0 to SMI_LOCK bit will have no
>> effect (i.e., once set, this bit can only be cleared by PLTRST#).
>>
>> - It also shows warning, i.e. when ASPM is not enabled (but this depends
>> on the attached device).
>> E8h PECR1 - PCI Express* Configuration Register 1
>> W [1] PECR1 Field 2 0x0 0x1
>> Both L0s and L1 Entry Supported. This bit should be set to 1b
>>
>
>> However, this utility only checks registers from CPU or chipset. No ACPI
>> tables or any other interfaces are analyzed - that's where our fwts
>> helps. ;-)
>
> Indeed :-)
>
> So do you think we should realistically add BIOS configured PCI config 
> space checks into fwts as valuable set of tests - or does Selftest 
> cover enough of what we need to check?  Perhaps this is up for 
> Precise+1 discussion at UDS
>
> Colin
Agreed. It is a good idea to take a closer look at selftest and worth of 
a good discussion.
>
>>
>> On 02/01/2012 05:17 PM, Alex Hung wrote:
>>> On 02/01/2012 12:54 AM, Colin Ian King wrote:
>>>> Hi there,
>>>>
>>>> While discussing some boot speed issues with the kernel with Andy
>>>> Whitcroft we spotted that that the PCI config space is being set up
>>>> by the BIOS and this can be sub-optimally configuring things like the
>>>> SATA capabilities.
>>>>
>>>> For example, bit 27 in HBA Capabilities for SATA devices controls the
>>>> "Staggered Spin-up (SSS)" setting - this really makes sense on
>>>> servers and hence should be set to zero for netbooks, laptops etc.
>>>> However, we observed it being incorrectly set on particular platform
>>>> which can only have one drive in it, so SSS makes no sense.
>>>>
>>> SSS looks very familiar. I recall that Intel's BIOS utility (selfcheck
>>> or selftest for Windows) checks this bit and it suggests a setting (I
>>> don't remember whether it was 1 or 0).
>>>
>>> PS. Both Intel and AMD also have utilities that checks hardware
>>> registers - thought they make mistakes too.
>>>
>>> I think it is a good idea to check hardware configuration but we will
>>> need to be careful. If fwts suggest different configuration from
>>> hardware vendors (i.e. Intel, AMD and nVidia), it will cause confusion.
>>>
>>>> So, perhaps we should add this to the next round of fwts blueprints -
>>>> check for incorrect or sub-optimal incorrectly configured PCI config
>>>> space settings for wide range of devices.
>>>>
>>>> Any thoughts?
>>>>
>>>> Colin
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>

On 02/02/12 13:09, Alex Hung wrote:
> On 02/02/2012 07:57 PM, Colin Ian King wrote:
>> On 02/02/12 03:53, Alex Hung wrote:
>>> The below is a fragment of a result from Intel's Selftest utility.
>>
>> Oh, I don't know about Selftest - where can I get that from? - and
>> what can it do? Is it another tool that we should be using on projects
>> by default to catch issues?
> It can be downloaded from Intel IBL, but an Intel account is needed. It
> only checks the registers in PCI(e) configuration space, CPU MSR or in
> memory space, based on Intel's recommendation. However, it only runs on
> Windows as far as I am aware of.
>
> AMD has a similar utility (called BTS - BIOS Test Suite?).
>>
>>>
>>> - This particular version checks for 12352 settings with Pantherpoint
>>> (PCH with Ivy Bridge), and it is based on BIOS Writer Guide 0.9.
>>>
>>> Stats : Total:12352 Error:64 Warn:55
>>> *********************************************************************************
>>>
>>>
>>> Pantherpoint BIOS Spec 0.9
>>> BWG Version - 0.9
>>>
>>> - It suggests that SSS needs to be enabled but BIOS disables it.
>>> Actual Expected
>>> ---------------------------------------------------------------------------------
>>>
>>>
>>> AHCI Generic Host Control Registers
>>> ---------------------------------------------------------------------------------
>>>
>>>
>>> 00h CAP - Host Capabilities Register
>>> E [27] Supports Staggered Spin-up (SSS) 0x0 0x1
>>> See BIOS Spec 14.1.4 Initialize Registers in AHCI Memory-Mapped Space
>>> Attribute: R/WO
>>> Default: 1
>>> Indicates whether the SATA controller supports staggered spin-up on its
>>> ports, for use in balancing power spikes. This value is loaded by
>>> platform BIOS prior to OS initialization.
>>> 0 = Staggered spin-up not supported.
>>> 1 = Staggered spin-up supported.
>>>
>>> - It shows pass if BIOS sets it right
>>> PASS [4] SMI_LOCK 0x1 0x1
>>> See BIOS Spec 3.6 Flash Security Recommendation-- This recommendation is
>>> only for production BIOS
>>> Attribute: R/WO
>>> Default: 0
>>> When this bit is set, writes to the GLB_SMI_EN bit (PMBASE + 30h, bit 0)
>>> will have no effect.
>>> Once the SMI_LOCK bit is set, writes of 0 to SMI_LOCK bit will have no
>>> effect (i.e., once set, this bit can only be cleared by PLTRST#).
>>>
>>> - It also shows warning, i.e. when ASPM is not enabled (but this depends
>>> on the attached device).
>>> E8h PECR1 - PCI Express* Configuration Register 1
>>> W [1] PECR1 Field 2 0x0 0x1
>>> Both L0s and L1 Entry Supported. This bit should be set to 1b
>>>
>>
>>> However, this utility only checks registers from CPU or chipset. No ACPI
>>> tables or any other interfaces are analyzed - that's where our fwts
>>> helps. ;-)
>>
>> Indeed :-)
>>
>> So do you think we should realistically add BIOS configured PCI config
>> space checks into fwts as valuable set of tests - or does Selftest
>> cover enough of what we need to check? Perhaps this is up for
>> Precise+1 discussion at UDS
>>
>> Colin
> Agreed. It is a good idea to take a closer look at selftest and worth of
> a good discussion.

Can we put it down as an action point:
	* Investigate features of selftest with possibility of adding these 
type of tests to fwts

..and have the list of features ready for discussion at UDS?

Colin
	

>>

>>>
>>> On 02/01/2012 05:17 PM, Alex Hung wrote:
>>>> On 02/01/2012 12:54 AM, Colin Ian King wrote:
>>>>> Hi there,
>>>>>
>>>>> While discussing some boot speed issues with the kernel with Andy
>>>>> Whitcroft we spotted that that the PCI config space is being set up
>>>>> by the BIOS and this can be sub-optimally configuring things like the
>>>>> SATA capabilities.
>>>>>
>>>>> For example, bit 27 in HBA Capabilities for SATA devices controls the
>>>>> "Staggered Spin-up (SSS)" setting - this really makes sense on
>>>>> servers and hence should be set to zero for netbooks, laptops etc.
>>>>> However, we observed it being incorrectly set on particular platform
>>>>> which can only have one drive in it, so SSS makes no sense.
>>>>>
>>>> SSS looks very familiar. I recall that Intel's BIOS utility (selfcheck
>>>> or selftest for Windows) checks this bit and it suggests a setting (I
>>>> don't remember whether it was 1 or 0).
>>>>
>>>> PS. Both Intel and AMD also have utilities that checks hardware
>>>> registers - thought they make mistakes too.
>>>>
>>>> I think it is a good idea to check hardware configuration but we will
>>>> need to be careful. If fwts suggest different configuration from
>>>> hardware vendors (i.e. Intel, AMD and nVidia), it will cause confusion.
>>>>
>>>>> So, perhaps we should add this to the next round of fwts blueprints -
>>>>> check for incorrect or sub-optimal incorrectly configured PCI config
>>>>> space settings for wide range of devices.
>>>>>
>>>>> Any thoughts?
>>>>>
>>>>> Colin
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>