ACK: [PATCH 16/21] FADT: extend and add PM address block compliance tests
Alex Hung
alex.hung at canonical.com
Wed Feb 17 06:20:05 UTC 2016
On 2016-02-09 09:32 AM, Al Stone wrote:
> There are several address fields for power management in the FADT:
> PM1A_EVT_BLK
> PM1B_EVT_BLK
> PM1A_CNT_BLK
> PM1B_CNT_BLK
> PM2_CNT_BLK
> PM_TMR_BLK
> PM1_EVT_LEN
> PM1_CNT_LEN
> PM2_CNT_LEN
> PM_TMR_LEN
>
> The tests that existed before only touched a few of these, so extend
> the tests so that now all of them are checked for proper values. Most
> of the tests are very similar, hence they are grouped together into this
> one patch.
>
> Signed-off-by: Al Stone <al.stone at linaro.org>
> ---
> src/acpi/fadt/fadt.c | 397 +++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 336 insertions(+), 61 deletions(-)
>
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index 3c4cdda..3b5547f 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -884,20 +884,336 @@ static void acpi_table_check_fadt_pstate_cnt(fwts_framework *fw)
> return;
> }
>
> -static void acpi_table_check_fadt_pm_tmr(
> - fwts_framework *fw,
> - const fwts_acpi_table_fadt *fadt,
> - bool *passed)
> +static void acpi_table_check_fadt_pm1a_evt_blk(fwts_framework *fw)
> {
> - 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.");
> + bool both_zero;
> + bool both_nonzero;
> +
> + if (fadt->pm1a_evt_blk == 0 && fadt->x_pm1a_evt_blk.address == 0) {
> + both_zero = true;
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1aEvtBlkNotSet",
> + "FADT PM1A_EVT_BLK is a required field and must "
> + "have either a 32-bit or 64-bit address set.");
> + } else {
> + both_zero = false;
> + fwts_passed(fw,
> + "FADT required PM1A_EVT_BLK field is non-zero");
> + }
> +
> + if (fadt->pm1a_evt_blk != 0 && fadt->x_pm1a_evt_blk.address != 0) {
> + both_nonzero = true;
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1aEvtBlkBothtSet",
> + "FADT PM1A_EVT_BLK has both a 32-bit and a "
> + "64-bit address set; only one should be used.");
> + } else {
> + both_nonzero = false;
> + if (!both_zero)
> + fwts_passed(fw,
> + "FADT one required PM1A_EVT_BLK field "
> + "is non-zero");
> + }
> +
> + if (both_nonzero &&
> + ((uint64_t)fadt->pm1a_evt_blk == fadt->x_pm1a_evt_blk.address)) {
> + fwts_passed(fw,
> + "FADT 32- and 64-bit PM1A_EVT_BLK fields are "
> + "at least equal.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM1A_EVT_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. However, they are at least equal so "
> + "the kernel will at least have a usable value.");
> + } else {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1aEvtBlkNotSet",
> + "FADT PM1A_EVT_BLK is a required field and must "
> + "have either a 32-bit or 64-bit address set.");
> + if (!both_zero)
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM1A_EVT_BLK "
> + "fields are being used, but only one "
> + "should be non-zero. Since the fields "
> + "value are not equal the kernel cannot "
> + "unambiguously determine which value "
> + "is the correct one.");
> + }
> +}
> +
> +static void acpi_table_check_fadt_pm1b_evt_blk(fwts_framework *fw)
> +{
> + if (fadt->pm1b_evt_blk == 0 && fadt->x_pm1b_evt_blk.address == 0) {
> + fwts_skipped(fw, "FADT PM1B_EVT_BLK not being used.");
> return;
> }
>
> - if (fadt->pm_tmr_len != 4) {
> - *passed = false;
> + if ((fadt->pm1b_evt_blk != 0 && fadt->x_pm1b_evt_blk.address == 0) ||
> + (fadt->pm1b_evt_blk == 0 && fadt->x_pm1b_evt_blk.address != 0))
> + fwts_passed(fw,
> + "FADT only one of the 32-bit or 64-bit "
> + "PM1B_EVT_BLK fields is being used.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1bEvtBlkOnlyOneField",
> + "FADT PM1B_EVT_BLK field has both the 32-bit "
> + "and the 64-bit field set.");
> +
> + if ((uint64_t)fadt->pm1b_evt_blk == fadt->x_pm1b_evt_blk.address) {
> + fwts_passed(fw,
> + "FADT 32- and 64-bit PM1B_EVT_BLK fields are "
> + "at least equal.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM1B_EVT_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. However, they are at least equal so "
> + "the kernel will at least have a usable value.");
> + } else {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1bEvtBlkNotSet",
> + "FADT PM1A_EVT_BLK is a required field and must "
> + "have either a 32-bit or 64-bit address set.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM1B_EVT_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. Since the fields value are not equal "
> + "the kernel cannot unambiguously determine which "
> + "value is the correct one.");
> + }
> +}
> +
> +static void acpi_table_check_fadt_pm1a_cnt_blk(fwts_framework *fw)
> +{
> + if (fadt->pm1a_cnt_blk != 0 || fadt->x_pm1a_cnt_blk.address != 0)
> + fwts_passed(fw,
> + "FADT required PM1A_CNT_BLK field is non-zero");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1aCntBlkNotSet",
> + "FADT PM1A_CNT_BLK is a required field and must "
> + "have either a 32-bit or 64-bit address set.");
> +
> + if ((fadt->pm1a_cnt_blk != 0 && fadt->x_pm1a_cnt_blk.address == 0) ||
> + (fadt->pm1a_cnt_blk == 0 && fadt->x_pm1a_cnt_blk.address != 0))
> + fwts_passed(fw,
> + "FADT only one of the 32-bit or 64-bit "
> + "PM1A_CNT_BLK fields is being used.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1aCntBlkOnlyOneField",
> + "FADT PM1A_CNT_BLK field has both the 32-bit "
> + "and the 64-bit field set.");
> +
> + if ((uint64_t)fadt->pm1a_cnt_blk == fadt->x_pm1a_cnt_blk.address) {
> + fwts_passed(fw,
> + "FADT 32- and 64-bit PM1A_CNT_BLK fields are "
> + "at least equal.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM1A_CNT_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. However, they are at least equal so "
> + "the kernel will at least have a usable value.");
> + } else {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1aCntBlkNotSet",
> + "FADT PM1A_CNT_BLK is a required field and must "
> + "have either a 32-bit or 64-bit address set.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM1A_CNT_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. Since the fields value are not equal "
> + "the kernel cannot unambiguously determine which "
> + "value is the correct one.");
> + }
> +}
> +
> +static void acpi_table_check_fadt_pm1b_cnt_blk(fwts_framework *fw)
> +{
> + if (fadt->pm1b_cnt_blk == 0 && fadt->x_pm1b_cnt_blk.address == 0) {
> + fwts_skipped(fw, "FADT PM1B_CNT_BLK not being used.");
> + return;
> + }
> +
> + if ((fadt->pm1b_cnt_blk != 0 && fadt->x_pm1b_cnt_blk.address == 0) ||
> + (fadt->pm1b_cnt_blk == 0 && fadt->x_pm1b_cnt_blk.address != 0))
> + fwts_passed(fw,
> + "FADT only one of the 32-bit or 64-bit "
> + "PM1B_CNT_BLK fields is being used.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1bCntBlkOnlyOneField",
> + "FADT PM1B_CNT_BLK field has both the 32-bit "
> + "and the 64-bit field set.");
> +
> + if ((uint64_t)fadt->pm1b_cnt_blk == fadt->x_pm1b_cnt_blk.address) {
> + fwts_passed(fw,
> + "FADT 32- and 64-bit PM1B_CNT_BLK fields are "
> + "at least equal.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM1B_CNT_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. However, they are at least equal so "
> + "the kernel will at least have a usable value.");
> + } else {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm1bCntBlkNotSet",
> + "FADT PM1A_CNT_BLK is a required field and must "
> + "have either a 32-bit or 64-bit address set.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM1B_CNT_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. Since the fields value are not equal "
> + "the kernel cannot unambiguously determine which "
> + "value is the correct one.");
> + }
> +}
> +
> +static void acpi_table_check_fadt_pm2_cnt_blk(fwts_framework *fw)
> +{
> + if (fadt->pm2_cnt_blk == 0 && fadt->x_pm2_cnt_blk.address == 0) {
> + fwts_skipped(fw, "FADT PM2_CNT_BLK not being used.");
> + return;
> + }
> +
> + if ((fadt->pm2_cnt_blk != 0 && fadt->x_pm2_cnt_blk.address == 0) ||
> + (fadt->pm2_cnt_blk == 0 && fadt->x_pm2_cnt_blk.address != 0))
> + fwts_passed(fw,
> + "FADT only one of the 32-bit or 64-bit "
> + "PM2_CNT_BLK fields is being used.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm2CntBlkOnlyOneField",
> + "FADT PM2_CNT_BLK field has both the 32-bit "
> + "and the 64-bit field set.");
> +
> + if ((uint64_t)fadt->pm2_cnt_blk == fadt->x_pm2_cnt_blk.address) {
> + fwts_passed(fw,
> + "FADT 32- and 64-bit PM2_CNT_BLK fields are "
> + "at least equal.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM2_CNT_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. However, they are at least equal so "
> + "the kernel will at least have a usable value.");
> + } else {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm2CntBlkNotSet",
> + "FADT PM2_CNT_BLK is a required field and must "
> + "have either a 32-bit or 64-bit address set.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM2_CNT_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. Since the fields value are not equal "
> + "the kernel cannot unambiguously determine which "
> + "value is the correct one.");
> + }
> +}
> +
> +static void acpi_table_check_fadt_pm_tmr_blk(fwts_framework *fw)
> +{
> + if (fadt->pm_tmr_blk == 0 && fadt->x_pm_tmr_blk.address == 0) {
> + fwts_skipped(fw, "FADT PM_TMR_BLK not being used.");
> + return;
> + }
> +
> + if ((fadt->pm_tmr_blk != 0 && fadt->x_pm_tmr_blk.address == 0) ||
> + (fadt->pm_tmr_blk == 0 && fadt->x_pm_tmr_blk.address != 0))
> + fwts_passed(fw,
> + "FADT only one of the 32-bit or 64-bit "
> + "PM_TMR_BLK fields is being used.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm2CntBlkOnlyOneField",
> + "FADT PM_TMR_BLK field has both the 32-bit "
> + "and the 64-bit field set.");
> +
> + if ((uint64_t)fadt->pm_tmr_blk == fadt->x_pm_tmr_blk.address) {
> + fwts_passed(fw,
> + "FADT 32- and 64-bit PM_TMR_BLK fields are "
> + "at least equal.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM_TMR_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. However, they are at least equal so "
> + "the kernel will at least have a usable value.");
> + } else {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm2CntBlkNotSet",
> + "FADT PM1A_CNT_BLK is a required field and must "
> + "have either a 32-bit or 64-bit address set.");
> + fwts_advice(fw,
> + "Both FADT 32- and 64-bit PM_TMR_BLK "
> + "fields are being used, but only one should be "
> + "non-zero. Since the fields value are not equal "
> + "the kernel cannot unambiguously determine which "
> + "value is the correct one.");
> + }
> +}
> +
> +static void acpi_table_check_fadt_pm1_evt_len(fwts_framework *fw)
> +{
> + if (fadt->pm1_evt_len >= 4)
> + fwts_passed(fw, "FADT PM1_EVT_LEN >= 4.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTPm1EvtLenTooSmall",
> + "FADT PM1_EVT_LEN must be >= 4, but is %d.",
> + fadt->pm1_evt_len);
> + return;
> +}
> +
> +static void acpi_table_check_fadt_pm1_cnt_len(fwts_framework *fw)
> +{
> + if (fadt->pm1_cnt_len >= 2)
> + fwts_passed(fw, "FADT PM1_CNT_LEN >= 2.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTPm1CntLenTooSmall",
> + "FADT PM1_CNT_LEN must be >= 2, but is %d.",
> + fadt->pm1_cnt_len);
> + return;
> +}
> +
> +static void acpi_table_check_fadt_pm2_cnt_len(fwts_framework *fw)
> +{
> + if (fadt->pm2_cnt_blk == 0) {
> + if (fadt->pm2_cnt_len == 0)
> + fwts_passed(fw, "FADT PM2_CNT_LEN is zero and "
> + "PM2_CNT_BLK is not supported.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPm2CntLenInconsistent",
> + "FADT PM2_CNT_LEN must be zero because "
> + "PM2_CNT_BLK is not supported, but is %d.",
> + fadt->pm2_cnt_len);
> + return;
> + }
> +
> + if (fadt->pm2_cnt_len >= 1)
> + fwts_passed(fw, "FADT PM2_CNT_LEN >= 1.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "FADTPm2CntLenTooSmall",
> + "FADT PM2_CNT_LEN must be >= 1, but is %d.",
> + fadt->pm2_cnt_len);
> + return;
> +}
> +
> +static void acpi_table_check_fadt_pm_tmr_len(fwts_framework *fw)
> +{
> + if (fadt->pm_tmr_len == 0) {
> + if (fadt->pm_tmr_blk == 0)
> + fwts_passed(fw, "FADT PM_TMR_LEN is zero and "
> + "PM_TMR_BLK is not supported.");
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "FADTPmTmrLenInconsistent",
> + "FADT PM_TMR_LEN must be zero because "
> + "PM_TMR_BLK is not supported, but is %d.",
> + fadt->pm_tmr_len);
> + return;
> + }
> +
> + if (fadt->pm_tmr_len == 4)
> + fwts_passed(fw, "FADT PM_TMR_LEN is 4.");
> + else {
> fwts_failed(fw, LOG_LEVEL_MEDIUM,
> "FADTBadPMTMRLEN",
> "FADT PM_TMR_LEN is %" PRIu8
> @@ -958,55 +1274,6 @@ static void acpi_table_check_fadt_gpe(
> }
> }
>
> -static void acpi_table_check_fadt_addr(
> - fwts_framework *fw,
> - const char *name,
> - const uint32_t addr32,
> - const fwts_acpi_gas *addr64,
> - bool *passed)
> -{
> - /* Don't compare if addresses are zero */
> - if ((addr32 == 0) || (addr64->address == 0))
> - return;
> - if (addr32 == addr64->address)
> - return;
> -
> - *passed = false;
> - /*
> - * Since this can cause systems to misbehave
> - * if the kernel uses the incorrect address we
> - * make this LOG_LEVEL_HIGH
> - */
> - fwts_failed(fw, LOG_LEVEL_HIGH,
> - "FADTPmAddr32Addr64Different",
> - "FADT %s (32 bit address) 0x%" PRIx32 " is different from "
> - "X_%s (64 bit address) 0x%" PRIx64 ".",
> - name, addr32, name, addr64->address);
> -}
> -
> -static void acpi_table_check_fadt_pm_addr(
> - fwts_framework *fw,
> - const fwts_acpi_table_fadt *fadt,
> - bool *passed)
> -{
> - if (fadt->header.length < 148) {
> - /* No 64 bit PM addresses to sanity check */
> - return;
> - }
> - acpi_table_check_fadt_addr(fw, "PM1a_EVT_BLK", fadt->pm1a_evt_blk,
> - &fadt->x_pm1a_evt_blk, passed);
> - acpi_table_check_fadt_addr(fw, "PM1b_EVT_BLK", fadt->pm1b_evt_blk,
> - &fadt->x_pm1b_evt_blk, passed);
> - acpi_table_check_fadt_addr(fw, "PM1a_CNT_BLK", fadt->pm1a_cnt_blk,
> - &fadt->x_pm1a_cnt_blk, passed);
> - acpi_table_check_fadt_addr(fw, "PM1b_CNT_BLK", fadt->pm1b_cnt_blk,
> - &fadt->x_pm1b_cnt_blk, passed);
> - acpi_table_check_fadt_addr(fw, "PM2_CNT_BLK", fadt->pm2_cnt_blk,
> - &fadt->x_pm2_cnt_blk, passed);
> - acpi_table_check_fadt_addr(fw, "PM_TMR_BLK", fadt->pm_tmr_blk,
> - &fadt->x_pm_tmr_blk, passed);
> -}
> -
> static int fadt_test1(fwts_framework *fw)
> {
> bool passed = true;
> @@ -1031,9 +1298,17 @@ static int fadt_test1(fwts_framework *fw)
> acpi_table_check_fadt_acpi_disable(fw);
> acpi_table_check_fadt_s4bios_req(fw);
> acpi_table_check_fadt_pstate_cnt(fw);
> - acpi_table_check_fadt_pm_tmr(fw, fadt, &passed);
> + acpi_table_check_fadt_pm1a_evt_blk(fw);
> + acpi_table_check_fadt_pm1b_evt_blk(fw);
> + acpi_table_check_fadt_pm1a_cnt_blk(fw);
> + acpi_table_check_fadt_pm1b_cnt_blk(fw);
> + acpi_table_check_fadt_pm2_cnt_blk(fw);
> + acpi_table_check_fadt_pm_tmr_blk(fw);
> + acpi_table_check_fadt_pm1_evt_len(fw);
> + acpi_table_check_fadt_pm1_cnt_len(fw);
> + acpi_table_check_fadt_pm2_cnt_len(fw);
> + acpi_table_check_fadt_pm_tmr_len(fw);
> acpi_table_check_fadt_gpe(fw, fadt, &passed);
> - acpi_table_check_fadt_pm_addr(fw, fadt, &passed);
> fwts_log_info(fw, "FADT GPE1_BASE is %" PRIu8, fadt->gpe1_base);
>
> fwts_log_info(fw, "FADT FLUSH_SIZE is %" PRIu16,
>
Acked-by: Alex Hung <alex.hung at canonical.com>
More information about the fwts-devel
mailing list