lkml.org 
[lkml]   [2012]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH ] tbfadt.c: output warning only when 64bit 32bit address of FACS/DSDT all have value but not equal to each other
From
Bob,
How about moving some checking code as following patch ?

From f46d539d81c763aa4e3e98f9fc1e94e0af48bd15 Mon Sep 17 00:00:00 2001
From: ethan.zhao <ethan.kernel@gmail.com>
Date: Sat, 17 Nov 2012 00:48:41 -0800
Subject: [PATCH]acpica/tbfadt.c Move some checking and 32bit to 64bit
assignment code from acpi_tb_validate_fadt() to
acpi_tb_convert_fadt to make the logic clearer and void multiple same
kind of warning.


Signed-off-by: ethan.zhao <ethan.kernel@gmail.com>
---
drivers/acpi/acpica/tbfadt.c | 51 ++++++++++++++---------------------------
1 files changed, 18 insertions(+), 33 deletions(-)

diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
index f23f512..dd18aba 100644
--- a/drivers/acpi/acpica/tbfadt.c
+++ b/drivers/acpi/acpica/tbfadt.c
@@ -388,18 +388,30 @@ static void acpi_tb_convert_fadt(void)
*/
if (!acpi_gbl_FADT.Xfacs) {
acpi_gbl_FADT.Xfacs = (u64) acpi_gbl_FADT.facs;
- } else if (acpi_gbl_FADT.facs &&
+ } else if ((acpi_gbl_FADT.facs && acpi_gbl_FADT.Xfacs) &&
(acpi_gbl_FADT.Xfacs != (u64) acpi_gbl_FADT.facs)) {
- ACPI_WARNING((AE_INFO,
- "32/64 FACS address mismatch in FADT - two FACS tables!"));
+ ACPI_BIOS_WARNING((AE_INFO,
+ "32/64X FACS address mismatch in FADT - "
+ "0x%8.8X/0x%8.8X%8.8X, using 32",
+ acpi_gbl_FADT.facs,
+ ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xfacs)));
+
+ acpi_gbl_FADT.Xfacs = (u64)acpi_gbl_FADT.facs;
+
}

if (!acpi_gbl_FADT.Xdsdt) {
acpi_gbl_FADT.Xdsdt = (u64) acpi_gbl_FADT.dsdt;
- } else if (acpi_gbl_FADT.dsdt &&
+ } else if ((acpi_gbl_FADT.dsdt && acpi_gbl_FADT.Xdsdt) &&
(acpi_gbl_FADT.Xdsdt != (u64) acpi_gbl_FADT.dsdt)) {
- ACPI_WARNING((AE_INFO,
- "32/64 DSDT address mismatch in FADT - two DSDT tables!"));
+ ACPI_BIOS_WARNING((AE_INFO,
+ "32/64X DSDT address mismatch in FADT - "
+ "0x%8.8X/0x%8.8X%8.8X, using 32",
+ acpi_gbl_FADT.dsdt,
+ ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xdsdt)));
+
+ acpi_gbl_FADT.Xdsdt = (u64)acpi_gbl_FADT.dsdt;
+
}

/*
@@ -507,33 +519,6 @@ static void acpi_tb_validate_fadt(void)
u8 length;
u32 i;

- /*
- * Check for FACS and DSDT address mismatches. An address mismatch between
- * the 32-bit and 64-bit address fields (FIRMWARE_CTRL/X_FIRMWARE_CTRL and
- * DSDT/X_DSDT) would indicate the presence of two FACS or two DSDT tables.
- */
- if (acpi_gbl_FADT.facs &&
- (acpi_gbl_FADT.Xfacs != (u64)acpi_gbl_FADT.facs)) {
- ACPI_BIOS_WARNING((AE_INFO,
- "32/64X FACS address mismatch in FADT - "
- "0x%8.8X/0x%8.8X%8.8X, using 32",
- acpi_gbl_FADT.facs,
- ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xfacs)));
-
- acpi_gbl_FADT.Xfacs = (u64)acpi_gbl_FADT.facs;
- }
-
- if (acpi_gbl_FADT.dsdt &&
- (acpi_gbl_FADT.Xdsdt != (u64)acpi_gbl_FADT.dsdt)) {
- ACPI_BIOS_WARNING((AE_INFO,
- "32/64X DSDT address mismatch in FADT - "
- "0x%8.8X/0x%8.8X%8.8X, using 32",
- acpi_gbl_FADT.dsdt,
- ACPI_FORMAT_UINT64(acpi_gbl_FADT.Xdsdt)));
-
- acpi_gbl_FADT.Xdsdt = (u64)acpi_gbl_FADT.dsdt;
- }
-
/* If Hardware Reduced flag is set, we are all done */

if (acpi_gbl_reduced_hardware) {
--
1.7.1

Thanks,
Ethan


On Fri, Nov 16, 2012 at 11:49 AM, Moore, Robert <robert.moore@intel.com> wrote:
> No decision(s) yet, we are still investigating
>
>
>> -----Original Message-----
>> From: Ethan Zhao [mailto:ethan.kernel@gmail.com]
>> Sent: Thursday, November 15, 2012 7:47 PM
>> To: Moore, Robert
>> Subject: Re: [PATCH ] tbfadt.c: output warning only when 64bit 32bit
>> address of FACS/DSDT all have value but not equal to each other
>>
>> Bob,
>> I read the discussion you have done
>>
>> https://www.acpica.org/bugzilla/show_bug.cgi?id=885
>>
>> Why method do you prefer ?
>>
>> Thanks,
>> Ethan
>>
>> On Fri, Nov 16, 2012 at 11:38 AM, Ethan Zhao <ethan.kernel@gmail.com>
>> wrote:
>> > Bob,
>> > In fact, you know if 32bit and 64bit address are all valid but not
>> > equal to each other, that does not follow ACPI 4&5 ,not follow 3 too.
>> > Do you mean we need to guess why they declare two different
>> > FACS/DSDT, and so kernel should collect some clue to figure out why ?
>> > that is amazing thing to keep system to work though its buggy BIOS.
>> >
>> > Ethan
>> >
>> >
>> > On Fri, Nov 16, 2012 at 11:27 AM, Moore, Robert <robert.moore@intel.com>
>> wrote:
>> >> Basically, the flow goes like this:
>> >>
>> >> 1) We convert the original FADT (multiple versions) to a common
>> internal FADT.
>> >> 2) Then we check the common internal FADT for errors/inconsistencies.
>> >>
>> >> In this way, we don't need to have different error checking for
>> different versions of the FADT, and this is probably not going to change.
>> >>
>> >> One thing we are going to add is to make a decision on whether to favor
>> a 32-bit address or a 64-bit address (if they are different) based upon
>> the age of the machine/BIOS. This is for Windows compatibility.
>> >>
>> >> Bob
>> >>
>> >>
>> >>> -----Original Message-----
>> >>> From: Ethan Zhao [mailto:ethan.kernel@gmail.com]
>> >>> Sent: Thursday, November 15, 2012 6:29 PM
>> >>> To: Moore, Robert
>> >>> Cc: Brown, Len; Zheng, Lv; LKML
>> >>> Subject: Re: [PATCH ] tbfadt.c: output warning only when 64bit 32bit
>> >>> address of FACS/DSDT all have value but not equal to each other
>> >>>
>> >>> Bob,
>> >>> Thanks for your detailed reviewing about the whole possible
>> >>> conditions.
>> >>> I.C. So that is impossible zero value for Xfacs /Xdsdt if
>> >>> facs/dsdt >0, for they are assigned in acpi_tb_convert_fadt(), I
>> >>> need to move my eyes one line code higher to see why it yelled twice
>> >>> but not using the 32bit address at once in acpi_tb_convert_fadt().
>> >>> Do you agree to move the checking code warning and into
>> >>> acpi_tb_convert_fadt to make the code more simple and clear ? Or
>> >>> just keep it for more rework, more bug?
>> >>>
>> >>>
>> >>> Thanks,
>> >>> Ethan


\
 
 \ /
  Last update: 2012-11-16 07:41    [W:0.039 / U:2.632 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site