ACK: [PATCH 11/30] acpi: move FADT test from acpitables into existing FADT test

ivanhu ivan.hu at canonical.com
Wed Jun 24 06:41:51 UTC 2015



On 2015年06月18日 16:49, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> Re-organise FADT test, move it out of acpitables and into
> the existing acpi FADT test
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>   src/Makefile.am                  |   1 +
>   src/acpi/acpitables/acpitables.c | 206 -------------------------
>   src/acpi/fadt/fadt.c             | 314 ++++++++++++++++++++++++++++++++++++++-
>   3 files changed, 311 insertions(+), 210 deletions(-)
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 487e083..3829d09 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -62,6 +62,7 @@ fwts_SOURCES = main.c 				\
>   	acpi/s3/s3.c 				\
>   	acpi/s3power/s3power.c 			\
>   	acpi/s4/s4.c 				\
> +	acpi/sbst/sbst.c			\
>   	acpi/slit/slit.c 			\
>   	acpi/spcr/spcr.c 			\
>   	acpi/spmi/spmi.c 			\
> diff --git a/src/acpi/acpitables/acpitables.c b/src/acpi/acpitables/acpitables.c
> index 60d2af4..5af9aa2 100644
> --- a/src/acpi/acpitables/acpitables.c
> +++ b/src/acpi/acpitables/acpitables.c
> @@ -25,211 +25,6 @@
>   
>   #include "fwts.h"
>   
> -static void acpi_table_check_fadt_firmware_control(fwts_framework *fw, fwts_acpi_table_fadt *fadt)
> -{
> -	if (fadt->firmware_control == 0) {
> -		if (fadt->header.length >= 140) {
> -			if ((fadt->x_firmware_ctrl == 0) && !(fwts_acpi_is_reduced_hardware(fadt))) {
> -				fwts_failed(fw, LOG_LEVEL_CRITICAL, "FADTFACSZero", "FADT 32 bit FIRMWARE_CONTROL and 64 bit X_FIRMWARE_CONTROL (FACS address) are null.");
> -				fwts_advice(fw, "The 32 bit FIRMWARE_CTRL or 64 bit X_FIRMWARE_CTRL should point to a valid "
> -						"Firmware ACPI Control Structure (FACS) when ACPI hardware reduced mode is not set. "
> -						"This is a firmware bug and needs to be fixed.");
> -			}
> -		} else {
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADT32BitFACSNull", "FADT 32 bit FIRMWARE_CONTROL is null.");
> -			fwts_advice(fw, "The ACPI version 1.0 FADT has a NULL FIRMWARE_CTRL and it needs to be defined "
> -					"to point to a valid Firmware ACPI Control Structure (FACS). This is a firmware "
> -					"bug and needs to be fixed.");
> -		}
> -	} else {
> -		if (fadt->header.length >= 140) {
> -			if (fadt->x_firmware_ctrl != 0) {
> -				if (((uint64_t)fadt->firmware_control != fadt->x_firmware_ctrl)) {
> -					fwts_failed(fw, LOG_LEVEL_MEDIUM, "FwCtrl32and64Differ",
> -						"FIRMWARE_CONTROL is 0x%" PRIx32 " and differs "
> -						"from X_FIRMWARE_CONTROL 0x%" PRIx64,
> -						fadt->firmware_control,
> -						fadt->x_firmware_ctrl);
> -					fwts_advice(fw, "One would expect the 32 bit FIRMWARE_CTRL and 64 bit X_FIRMWARE_CTRL "
> -							"pointers to point to the same FACS, however they don't which is clearly ambiguous and wrong. "
> -							"The kernel works around this by using the 64 bit X_FIRMWARE_CTRL pointer to the FACS. ");
> -				}
> -			}
> -		}
> -	}
> -}
> -
> -static void acpi_table_check_fadt_dsdt(fwts_framework *fw, fwts_acpi_table_fadt *fadt)
> -{
> -	if (fadt->dsdt == 0)
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTDSTNull", "FADT DSDT address is null.");
> -
> -	if (fadt->header.length >= 148) {
> -		if (fadt->x_dsdt == 0) {
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTXDSTDNull", "FADT X_DSDT address is null.");
> -			fwts_advice(fw, "An ACPI 2.0 FADT is being used however the 64 bit X_DSDT is null."
> -					"The kernel will fall back to using the 32 bit DSDT pointer instead.");
> -		} else if ((uint64_t)fadt->dsdt != fadt->x_dsdt) {
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADT32And64Mismatch",
> -				"FADT 32 bit DSDT (0x%" PRIx32 ") does not point to same "
> -				"physical address as 64 bit X_DSDT (0x%" PRIx64 ").",
> -				fadt->dsdt, fadt->x_dsdt);
> -			fwts_advice(fw, "One would expect the 32 bit DSDT and 64 bit X_DSDT "
> -					"pointers to point to the same DSDT, however they don't which is clearly ambiguous and wrong. "
> -					"The kernel works around this by using the 64 bit X_DSDT pointer to the DSDT. ");
> -		}
> -	}
> -}
> -
> -
> -static void acpi_table_check_fadt_smi(fwts_framework *fw, fwts_acpi_table_fadt *fadt)
> -{
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> -		if (fadt->smi_cmd != 0)
> -			fwts_warning(fw, "FADT SMI_CMD is not zero but should be in reduced hardware mode.");
> -		return;
> -	}
> -
> -	/*
> -	 *  Section 5.2.9 (Fixed ACPI Description Table) of the ACPI 5.0
> -	 *  specification states that if SMI_CMD is zero then it is
> -	 *  a system that does not support System Management Mode, so
> -	 *  in that case, don't check SCI_INT being valid.
> -	 */
> -	if (fadt->smi_cmd != 0) {
> -		if (fadt->sci_int == 0) {
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSCIIRQZero", "FADT SCI Interrupt is 0x00, should be defined.");
> -		}
> -	} else {
> -		if ((fadt->acpi_enable == 0) &&
> -		    (fadt->acpi_disable == 0) &&
> -		    (fadt->s4bios_req == 0) &&
> -		    (fadt->pstate_cnt == 0) &&
> -		    (fadt->cst_cnt == 0)) {
> -			/* Not an error, but intentional, but feedback this finding anyhow */
> -			fwts_log_info(fw, "The FADT SMI_CMD is zero, system does not support System Management Mode.");
> -		}
> -		else {
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTSMICMDZero",
> -					"FADT SMI_CMD is 0x00, however, one or more of ACPI_ENABLE, ACPI_DISABLE, "
> -					"S4BIOS_REQ, PSTATE_CNT and CST_CNT are defined which means SMI_CMD should be "
> -					"defined otherwise SMI commands cannot be sent.");
> -			fwts_advice(fw, "The configuration seems to suggest that SMI command should be defined to "
> -					"allow the kernel to trigger system management interrupts via the SMD_CMD port. "
> -					"The fact that SMD_CMD is zero which is invalid means that SMIs are not possible "
> -					"through the normal ACPI mechanisms. This means some firmware based machine "
> -					"specific functions will not work.");
> -		}
> -	}
> -}
> -
> -static void acpi_table_check_fadt_pm_tmr(fwts_framework *fw, fwts_acpi_table_fadt *fadt)
> -{
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> -		if (fadt->pm_tmr_len != 0)
> -			fwts_warning(fw, "FADT PM_TMR_LEN is not zero but should be in reduced hardware mode.");
> -		return;
> -	}
> -
> -	if (fadt->pm_tmr_len != 4) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTBadPMTMRLEN",
> -			"FADT PM_TMR_LEN is %" PRIu8 ", should be 4.", fadt->pm_tmr_len);
> -		fwts_advice(fw, "FADT field PM_TMR_LEN defines the number of bytes decoded by PM_TMR_BLK. "
> -				"This fields value must be 4. If it is not the correct size then the kernel "
> -				"will not request a region for the pm timer block. ");
> -	}
> -}
> -
> -static void acpi_table_check_fadt_gpe(fwts_framework *fw, fwts_acpi_table_fadt *fadt)
> -{
> -	if (fwts_acpi_is_reduced_hardware(fadt)) {
> -		if (fadt->gpe0_blk_len != 0)
> -			fwts_warning(fw, "FADT GPE0_BLK_LEN is not zero but should be in reduced hardware mode.");
> -		if (fadt->gpe1_blk_len != 0)
> -			fwts_warning(fw, "FADT GPE1_BLK_LEN is not zero but should be in reduced hardware mode.");
> -		return;
> -	}
> -
> -	if (fadt->gpe0_blk_len & 1) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTBadGPEBLKLEN", "FADT GPE0_BLK_LEN is %" PRIu8
> -			", should a multiple of 2.", fadt->gpe0_blk_len);
> -		fwts_advice(fw, "The FADT GPE_BLK_LEN should be a multiple of 2. Because it isn't, the ACPI driver will "
> -				"not map in the GPE0 region. This could mean that General Purpose Events will not "
> -				"function correctly (for example lid or ac-power events).");
> -	}
> -	if (fadt->gpe1_blk_len & 1) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTBadGPE1BLKLEN", "FADT GPE1_BLK_LEN is %" PRIu8
> -			", should a multiple of 2.", fadt->gpe1_blk_len);
> -		fwts_advice(fw, "The FADT GPE_BLK_LEN should be a multiple of 2. Because it isn't, the ACPI driver will "
> -				"not map in the GPE1 region. This could mean that General Purpose Events will not "
> -				"function correctly (for example lid or ac-power events).");
> -	}
> -}
> -
> -static void acpi_table_check_fadt_reset(fwts_framework *fw, fwts_acpi_table_fadt *fadt)
> -{
> -	if (fadt->header.length>=129) {
> -		if ((fadt->reset_reg.address_space_id != 0) &&
> -		    (fadt->reset_reg.address_space_id != 1) &&
> -		    (fadt->reset_reg.address_space_id != 2)) {
> -			fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTBadRESETREG",
> -				"FADT RESET_REG address space ID was %" PRIu8 ", must be System Memory space (0), "
> -				"System I/O space (1), or PCI configuration space (2).",
> -				fadt->reset_reg.address_space_id);
> -			fwts_advice(fw, "If the FADT RESET_REG address space ID is not set correctly then ACPI writes "
> -					"to this register *may* nor work correctly, meaning a reboot via this mechanism may not work.");
> -		}
> -		if ((fadt->reset_value == 0) && (fadt->reset_reg.address != 0))
> -			fwts_warning(fw, "FADT RESET_VALUE is zero, which may be incorrect, it is usually non-zero.");
> -	}
> -}
> -
> -static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *table)
> -{
> -	fwts_acpi_table_fadt *fadt = (fwts_acpi_table_fadt*)table->data;
> -
> -	acpi_table_check_fadt_firmware_control(fw, fadt);
> -	acpi_table_check_fadt_dsdt(fw, fadt);
> -	acpi_table_check_fadt_smi(fw, fadt);
> -	acpi_table_check_fadt_pm_tmr(fw, fadt);
> -	acpi_table_check_fadt_gpe(fw, fadt);
> -
> -	/*
> -	 * Bug LP: /833644
> -	 *
> -	 *   Remove these tests, really need to put more intelligence into it
> -	 *   perhaps in the cstates test rather than here. For the moment we
> - 	 *   shall remove this warning as it's giving users false alarms
> -	 *   See: https://bugs.launchpad.net/ubuntu/+source/fwts/+bug/833644
> -	 */
> -	/*
> -	if (fadt->p_lvl2_lat > 100) {
> -		fwts_warning(fw, "FADT P_LVL2_LAT is %" PRIi16 ", a value > 100 indicates a "
> -			"system not to support a C2 state.", fadt->p_lvl2_lat);
> -		fwts_advice(fw, "The FADT P_LVL2_LAT setting specifies the C2 latency in microseconds. The ACPI specification "
> -				"states that a value > 100 indicates that C2 is not supported and hence the "
> -				"ACPI processor idle routine will not use C2 power states.");
> -	}
> -	if (fadt->p_lvl3_lat > 1000) {
> -		fwts_warning(fw, "FADT P_LVL3_LAT is %" PRIu16 ", a value > 1000 indicates a "
> -			"system not to support a C3 state.", fadt->p_lvl3_lat);
> -		fwts_advice(fw, "The FADT P_LVL2_LAT setting specifies the C3 latency in microseconds. The ACPI specification "
> -				"states that a value > 1000 indicates that C3 is not supported and hence the "
> -				"ACPI processor idle routine will not use C3 power states.");
> -	}
> -	*/
> -	/*
> -	if (fadt->day_alrm == 0)
> -		fwts_warning(fw, "FADT DAY_ALRM is zero, OS will not be able to program day of month alarm.");
> -	if (fadt->mon_alrm == 0)
> -		fwts_warning(fw, "FADT MON_ALRM is zero, OS will not be able to program month of year alarm.");
> -	if (fadt->century == 0)
> -		fwts_warning(fw, "FADT CENTURY is zero, RTC does not support centenary feature is not supported.");
> -	*/
> -
> -	acpi_table_check_fadt_reset(fw, fadt);
> -}
> -
>   typedef void (*check_func)(fwts_framework *fw, fwts_acpi_table_info *table);
>   
>   typedef struct {
> @@ -238,7 +33,6 @@ typedef struct {
>   } acpi_table_check_table;
>   
>   static acpi_table_check_table check_table[] = {
> -	{ "FACP", acpi_table_check_fadt },
>   	{ NULL  , NULL },
>   } ;
>   
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index af0b192..baa1bf8 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -60,8 +60,313 @@ static int fadt_init(fwts_framework *fw)
>   	return FWTS_OK;
>   }
>   
> +static void acpi_table_check_fadt_firmware_control(
> +	fwts_framework *fw,
> +	const fwts_acpi_table_fadt *fadt,
> +	bool *passed)
> +{
> +	if (fadt->firmware_control == 0) {
> +		if (fadt->header.length >= 140) {
> +			if ((fadt->x_firmware_ctrl == 0) &&
> +			    !(fwts_acpi_is_reduced_hardware(fadt))) {
> +				*passed = false;
> +				fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +					"FADTFACSZero",
> +					"FADT 32 bit FIRMWARE_CONTROL and "
> +					"64 bit X_FIRMWARE_CONTROL (FACS "
> +					"address) are null.");
> +				fwts_advice(fw,
> +					"The 32 bit FIRMWARE_CTRL or 64 "
> +					"bit X_FIRMWARE_CTRL should point "
> +					"to a valid Firmware ACPI Control "
> +					"Structure (FACS) when ACPI hardware "
> +					"reduced mode is not set. ");
> +			}
> +		} else {
> +			*passed = false;
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				"FADT32BitFACSNull",
> +				"FADT 32 bit FIRMWARE_CONTROL is null.");
> +			fwts_advice(fw,
> +				"The ACPI version 1.0 FADT has a NULL "
> +				"FIRMWARE_CTRL and it needs to be defined "
> +				"to point to a valid Firmware ACPI Control "
> +				"Structure (FACS).");
> +		}
> +	} else {
> +		if (fadt->header.length >= 140) {
> +			if (fadt->x_firmware_ctrl != 0) {
> +				if (((uint64_t)fadt->firmware_control != fadt->x_firmware_ctrl)) {
> +					*passed = false;
> +					fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +						"FwCtrl32and64Differ",
> +						"FIRMWARE_CONTROL is 0x%" PRIx32
> +						" and differs from X_FIRMWARE_CONTROL "
> +						"0x%" PRIx64,
> +						fadt->firmware_control,
> +						fadt->x_firmware_ctrl);
> +					fwts_advice(fw,
> +						"One would expect the 32 bit FIRMWARE_CTRL "
> +						"and 64 bit X_FIRMWARE_CTRL pointers to "
> +						"point to the same FACS, however they don't "
> +						"which is clearly ambiguous and wrong. "
> +						"The kernel works around this by using the "
> +						"64 bit X_FIRMWARE_CTRL pointer to the FACS. ");
> +				}
> +			}
> +		}
> +	}
> +}
> +
> +static void acpi_table_check_fadt_dsdt(
> +	fwts_framework *fw,
> +	const fwts_acpi_table_fadt *fadt,
> +	bool *passed)
> +{
> +	if (fadt->dsdt == 0) {
> +		*passed = false;
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +			"FADTDSTNull",
> +			"FADT DSDT address is null.");
> +	}
> +
> +	if (fadt->header.length >= 148) {
> +		if (fadt->x_dsdt == 0) {
> +			*passed = false;
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				"FADTXDSTDNull",
> +				"FADT X_DSDT address is null.");
> +			fwts_advice(fw,
> +				"An ACPI 2.0 FADT is being used however "
> +				"the 64 bit X_DSDT is null."
> +				"The kernel will fall back to using "
> +				"the 32 bit DSDT pointer instead.");
> +		} else if ((uint64_t)fadt->dsdt != fadt->x_dsdt) {
> +			*passed = false;
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				"FADT32And64Mismatch",
> +				"FADT 32 bit DSDT (0x%" PRIx32 ") "
> +				"does not point to same "
> +				"physical address as 64 bit X_DSDT "
> +				"(0x%" PRIx64 ").",
> +				fadt->dsdt, fadt->x_dsdt);
> +			fwts_advice(fw,
> +				"One would expect the 32 bit DSDT and "
> +				"64 bit X_DSDT pointers to point to the "
> +				"same DSDT, however they don't which is "
> +				"clearly ambiguous and wrong. "
> +				"The kernel works around this by using the "
> +				"64 bit X_DSDT pointer to the DSDT. ");
> +		}
> +	}
> +}
> +
> +
> +static void acpi_table_check_fadt_smi(
> +	fwts_framework *fw,
> +	const fwts_acpi_table_fadt *fadt,
> +	bool *passed)
> +{
> +	if (fwts_acpi_is_reduced_hardware(fadt)) {
> +		if (fadt->smi_cmd != 0) {
> +			fwts_warning(fw, "FADT SMI_CMD is not zero "
> +				"but should be in reduced hardware mode.");
> +		}
> +		return;
> +	}
> +
> +	/*
> +	 *  Section 5.2.9 (Fixed ACPI Description Table) of the ACPI 5.0
> +	 *  specification states that if SMI_CMD is zero then it is
> +	 *  a system that does not support System Management Mode, so
> +	 *  in that case, don't check SCI_INT being valid.
> +	 */
> +	if (fadt->smi_cmd != 0) {
> +		if (fadt->sci_int == 0) {
> +			*passed = false;
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				"FADTSCIIRQZero",
> +				"FADT SCI Interrupt is 0x00, should be defined.");
> +		}
> +	} else {
> +		if ((fadt->acpi_enable == 0) &&
> +		    (fadt->acpi_disable == 0) &&
> +		    (fadt->s4bios_req == 0) &&
> +		    (fadt->pstate_cnt == 0) &&
> +		    (fadt->cst_cnt == 0)) {
> +			/* Not an error, but intentional, but feedback this finding anyhow */
> +			fwts_log_info(fw, "The FADT SMI_CMD is zero, system "
> +				"does not support System Management Mode.");
> +		}
> +		else {
> +			*passed = false;
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				"FADTSMICMDZero",
> +				"FADT SMI_CMD is 0x00, however, one or more of "
> +				"ACPI_ENABLE, ACPI_DISABLE, S4BIOS_REQ, PSTATE_CNT "
> +				"and CST_CNT are defined which means SMI_CMD should "
> +				"be defined otherwise SMI commands cannot be sent.");
> +			fwts_advice(fw,
> +				"The configuration seems to suggest that SMI command "
> +				"should be defined to allow the kernel to trigger "
> +				"system management interrupts via the SMD_CMD port. "
> +				"The fact that SMD_CMD is zero which is invalid means "
> +				"that SMIs are not possible through the normal ACPI "
> +				"mechanisms. This means some firmware based machine "
> +				"specific functions will not work.");
> +		}
> +	}
> +}
> +
> +static void acpi_table_check_fadt_pm_tmr(
> +	fwts_framework *fw,
> +	const fwts_acpi_table_fadt *fadt,
> +	bool *passed)
> +{
> +	if (fwts_acpi_is_reduced_hardware(fadt)) {
> +		if (fadt->pm_tmr_len != 0)
> +			fwts_warning(fw, "FADT PM_TMR_LEN is not zero "
> +				"but should be in reduced hardware mode.");
> +		return;
> +	}
> +
> +	if (fadt->pm_tmr_len != 4) {
> +		*passed = false;
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +			"FADTBadPMTMRLEN",
> +			"FADT PM_TMR_LEN is %" PRIu8
> +			", should be 4.", fadt->pm_tmr_len);
> +		fwts_advice(fw,
> +			"FADT field PM_TMR_LEN defines the number "
> +			"of bytes decoded by PM_TMR_BLK. "
> +			"This fields value must be 4. If it is not "
> +			"the correct size then the kernel will not "
> +			"request a region for the pm timer block. ");
> +	}
> +}
> +
> +static void acpi_table_check_fadt_gpe(
> +	fwts_framework *fw,
> +	const fwts_acpi_table_fadt *fadt,
> +	bool *passed)
> +{
> +	if (fwts_acpi_is_reduced_hardware(fadt)) {
> +		if (fadt->gpe0_blk_len != 0) {
> +			fwts_warning(fw, "FADT GPE0_BLK_LEN is not zero "
> +				"but should be in reduced hardware mode.");
> +		}
> +		if (fadt->gpe1_blk_len != 0) {
> +			fwts_warning(fw, "FADT GPE1_BLK_LEN is not zero but "
> +				"should be in reduced hardware mode.");
> +		}
> +		return;
> +	}
> +
> +	if (fadt->gpe0_blk_len & 1) {
> +		*passed = false;
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +			"FADTBadGPEBLKLEN",
> +			"FADT GPE0_BLK_LEN is %" PRIu8
> +			", should a multiple of 2.",
> +			fadt->gpe0_blk_len);
> +		fwts_advice(fw,
> +			"The FADT GPE_BLK_LEN should be a multiple of 2. "
> +			"Because it isn't, the ACPI driver will not map in "
> +			"the GPE0 region. This could mean that General "
> +			"Purpose Events will not function correctly (for "
> +			"example lid or ac-power events).");
> +	}
> +	if (fadt->gpe1_blk_len & 1) {
> +		*passed = false;
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +			"FADTBadGPE1BLKLEN",
> +			"FADT GPE1_BLK_LEN is %" PRIu8
> +			", should a multiple of 2.",
> +			fadt->gpe1_blk_len);
> +		fwts_advice(fw,
> +			"The FADT GPE1_BLK_LEN should be a multiple of 2. "
> +			"Because it isn't, the ACPI driver will not map in "
> +			"the GPE1 region. This could mean that General "
> +			"Purpose Events will not function correctly (for "
> +			"example lid or ac-power events).");
> +	}
> +}
> +
> +static void acpi_table_check_fadt_reset(
> +	fwts_framework *fw,
> +	const fwts_acpi_table_fadt *fadt,
> +	bool *passed)
> +{
> +	if (fadt->header.length>=129) {
> +		if ((fadt->reset_reg.address_space_id != 0) &&
> +		    (fadt->reset_reg.address_space_id != 1) &&
> +		    (fadt->reset_reg.address_space_id != 2)) {
> +			*passed = false;
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +				"FADTBadRESETREG",
> +				"FADT RESET_REG address space ID was %"
> +				PRIu8 ", must be System Memory space (0), "
> +				"System I/O space (1), or PCI configuration "
> +				"space (2).",
> +				fadt->reset_reg.address_space_id);
> +			fwts_advice(fw,
> +				"If the FADT RESET_REG address space ID is "
> +				"not set correctly then ACPI writes "
> +				"to this register *may* nor work correctly, "
> +				"meaning a reboot via this mechanism may not work.");
> +		}
> +		if ((fadt->reset_value == 0) &&
> +		    (fadt->reset_reg.address != 0))
> +			fwts_warning(fw, "FADT RESET_VALUE is zero, which "
> +				"may be incorrect, it is usually non-zero.");
> +	}
> +}
> +
>   static int fadt_test1(fwts_framework *fw)
>   {
> +	bool passed = true;
> +
> +	acpi_table_check_fadt_firmware_control(fw, fadt, &passed);
> +	acpi_table_check_fadt_dsdt(fw, fadt, &passed);
> +	acpi_table_check_fadt_smi(fw, fadt, &passed);
> +	acpi_table_check_fadt_pm_tmr(fw, fadt, &passed);
> +	acpi_table_check_fadt_gpe(fw, fadt, &passed);
> +
> +	/*
> +	 * Bug LP: #833644
> +	 *
> +	 *   Remove these tests, really need to put more intelligence into it
> +	 *   perhaps in the cstates test rather than here. For the moment we
> + 	 *   shall remove this warning as it's giving users false alarms
> +	 *   See: https://bugs.launchpad.net/ubuntu/+source/fwts/+bug/833644
> +	 */
> +	/*
> +	if (fadt->p_lvl2_lat > 100) {
> +		fwts_warning(fw, "FADT P_LVL2_LAT is %" PRIi16 ", a value > 100 indicates a "
> +			"system not to support a C2 state.", fadt->p_lvl2_lat);
> +		fwts_advice(fw, "The FADT P_LVL2_LAT setting specifies the C2 latency in microseconds. The ACPI specification "
> +				"states that a value > 100 indicates that C2 is not supported and hence the "
> +				"ACPI processor idle routine will not use C2 power states.");
> +	}
> +	if (fadt->p_lvl3_lat > 1000) {
> +		fwts_warning(fw, "FADT P_LVL3_LAT is %" PRIu16 ", a value > 1000 indicates a "
> +			"system not to support a C3 state.", fadt->p_lvl3_lat);
> +		fwts_advice(fw, "The FADT P_LVL2_LAT setting specifies the C3 latency in microseconds. The ACPI specification "
> +				"states that a value > 1000 indicates that C3 is not supported and hence the "
> +				"ACPI processor idle routine will not use C3 power states.");
> +	}
> +	*/
> +	acpi_table_check_fadt_reset(fw, fadt, &passed);
> +
> +	if (passed)
> +		fwts_passed(fw, "No issues found in FADT table.");
> +
> +	return FWTS_OK;
> +}
> +
> +
> +static int fadt_test2(fwts_framework *fw)
> +{
>   	uint32_t port, width, val32;
>   	int ret = FWTS_OK;
>   
> @@ -142,7 +447,7 @@ static int fadt_test1(fwts_framework *fw)
>   	return FWTS_OK;
>   }
>   
> -static int fadt_test2(fwts_framework *fw)
> +static int fadt_test3(fwts_framework *fw)
>   {
>   	if ((fadt->header.revision == 1) || (fadt->header.length < 244)) {
>   		fwts_skipped(fw, "Header size indicates an ACPI 1.0 FADT, skipping test.");
> @@ -183,13 +488,14 @@ static int fadt_test2(fwts_framework *fw)
>   }
>   
>   static fwts_framework_minor_test fadt_tests[] = {
> -	{ fadt_test1, "Test FADT SCI_EN bit is enabled." },
> -	{ fadt_test2, "Test FADT reset register." },
> +	{ fadt_test1, "Test FADT ACPI Description Table tests." },
> +	{ fadt_test2, "Test FADT SCI_EN bit is enabled." },
> +	{ fadt_test3, "Test FADT reset register." },
>   	{ NULL, NULL }
>   };
>   
>   static fwts_framework_ops fadt_ops = {
> -	.description = "FADT SCI_EN enabled tests.",
> +	.description = "FADT Fixed ACPI Description Table tests.",
>   	.init        = fadt_init,
>   	.minor_tests = fadt_tests
>   };
Acked-by: Ivan Hu<ivan.hu at canonical.com>



More information about the fwts-devel mailing list