Issues in fadt check item?

Alex Hung alex.hung at canonical.com
Wed Dec 7 17:38:24 UTC 2016


I agree with Jeffrey that's actually not a fix: ACPI 6.1 explicitly states
PM1b_CNT_BLK to meet "If the X_PM1b_CNT_BLK field contains a non-zero value
then this field must be zero." The same also applies to PM2_CNT_BLK.

Many systems are shipped before ACPI 6.1's release, and this failure is odd
for them.  I think it is a case this error can be ignored.  However, you
should try to meet ACPI 6.1 spec if a target system is being developed.

On Wed, Dec 7, 2016 at 7:36 AM, Jeffrey Hugo <jhugo at codeaurora.org> wrote:

> Hi Eric
>
>
> On 12/7/2016 5:51 AM, Dong, Eric wrote:
>
>> Hi,
>>
>>
>>
>> When I use fwts to check uefi bios code, it reports some issues related
>> to fadt check item. After check the uefi code and acpi spec, I didn’t
>> find error for uefi code. So I download fwts code (fwts_16.11.00) and i
>> think some bugs in the fadt check item.
>>
>> One issue like below:
>>
>> static void acpi_table_check_fadt_pm2_cnt_blk(fwts_framework *fw)
>>
>> {
>>
>>                 if (fadt->pm2_cnt_blk == 0 && fadt->header.length < 208) {
>>
>>                                 fwts_skipped(fw, "FADT PM2_CNT_BLK not
>> being used.");
>>
>>                                 return;
>>
>>                 }
>>
>>
>>
>>                 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.");
>>
>>                 }
>>
>> }
>>
>> }
>>
>>
>>
>> I think the fix is: remove the red mark code and add green mark code.
>> what do you think?
>>
>>
> I'm curious, can you describe the scenario or scenarios in which you feel
> the current code fails?
>
> Having looked at the ACPI spec and the code you've identified in your
> mail, I don't yet see an issue with the current code that you've
> identified, and I actually think your "fix" is incorrect at this time.
>
>
>>
>> Thanks,
>>
>> Eric
>>
>>
>>
>>
>
> --
> Jeffrey Hugo
> Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies,
> Inc.
> Qualcomm Technologies, Inc. is a member of the
> Code Aurora Forum, a Linux Foundation Collaborative Project.
>
> --
> fwts-devel mailing list
> fwts-devel at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm
> an/listinfo/fwts-devel
>



-- 
Cheers,
Alex Hung
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20161207/95c1c8d1/attachment.html>


More information about the fwts-devel mailing list