lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 16 Nov 2012 13:46:46 +0800
From:	Ethan Zhao <ethan.kernel@...il.com>
To:	"Moore, Robert" <robert.moore@...el.com>
Cc:	LKML <linux-kernel@...r.kernel.org>
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,
     How about moving some checking code as following patch ?

     From f46d539d81c763aa4e3e98f9fc1e94e0af48bd15 Mon Sep 17 00:00:00 2001
From: ethan.zhao <ethan.kernel@...il.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@...il.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@...el.com> wrote:
> No decision(s) yet, we are still investigating
>
>
>> -----Original Message-----
>> From: Ethan Zhao [mailto:ethan.kernel@...il.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@...il.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@...el.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@...il.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ