[V2][PATCH 2/2] acpi: acpitables: FADT: Ignore fields at offset 46 through 108 for HW_REDUCED_ACPI
Colin Ian King
colin.king at canonical.com
Tue Apr 7 10:21:55 UTC 2015
On 07/04/15 06:44, Heyi Guo wrote:
> According to ACPI spec 5.1, section 5.2.9, "If the HW_REDUCED_ACPI flag in the table is set, OSPM will ignore fields related to the ACPI HW register interface: Fields at offsets 46 through 108 and 148 through 232, as well as FADT Flag bits 1, 2, 3,7,8,12,13, 14, 16 and 17).", add precondition of checking SMI_CMD, PM_TMR_LEN, etc. As the code becomes a little complex, split it into small functions for each important field check.
>
> Signed-off-by: Heyi Guo <heyi.guo at linaro.org>
> ---
> src/acpi/acpitables/acpitables.c | 90 +++++++++++++++++++++++++++++++---------
> 1 file changed, 70 insertions(+), 20 deletions(-)
>
> diff --git a/src/acpi/acpitables/acpitables.c b/src/acpi/acpitables/acpitables.c
> index e5fffff..52b6659 100644
> --- a/src/acpi/acpitables/acpitables.c
> +++ b/src/acpi/acpitables/acpitables.c
> @@ -69,12 +69,10 @@ static void acpi_table_check_hpet(fwts_framework *fw, fwts_acpi_table_info *tabl
>
> }
>
> -static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *table)
> +static void acpi_table_check_fadt_firmware_control(fwts_framework *fw, fwts_acpi_table_fadt *fadt)
> {
> - fwts_acpi_table_fadt *fadt = (fwts_acpi_table_fadt*)table->data;
> -
> if (fadt->firmware_control == 0) {
> - if (table->length >= 140) {
> + 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 "
> @@ -88,7 +86,7 @@ static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
> "bug and needs to be fixed.");
> }
> } else {
> - if (table->length >= 140) {
> + 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",
> @@ -103,10 +101,14 @@ static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
> }
> }
> }
> +}
>
> +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 (table->length >= 148) {
> +
> + 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."
> @@ -121,6 +123,16 @@ static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
> "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
> @@ -153,6 +165,15 @@ static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
> "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",
> @@ -161,6 +182,18 @@ static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
> "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);
> @@ -175,6 +208,36 @@ static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
> "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
> *
> @@ -208,20 +271,7 @@ static void acpi_table_check_fadt(fwts_framework *fw, fwts_acpi_table_info *tabl
> fwts_warning(fw, "FADT CENTURY is zero, RTC does not support centenary feature is not supported.");
> */
>
> - if (table->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.");
> - }
> + acpi_table_check_fadt_reset(fw, fadt);
> }
>
> static void acpi_table_check_rsdp(fwts_framework *fw, fwts_acpi_table_info *table)
>
Acked-by: Colin Ian King <colin.king at canonical.com>
More information about the fwts-devel
mailing list