| 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
| ||
|
Date: Fri, 16 Nov 2012 06:02:21 +0000
From: "Moore, Robert" <robert.moore@...el.com>
To: Ethan Zhao <ethan.kernel@...il.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
As I mentioned, we are going to be rewriting this part of the code, so I would rather wait to until then to make changes. However, this looks like a reasonable approach to the error check, at first glance.
Thanks,
Bob
> -----Original Message-----
> From: Ethan Zhao [mailto:ethan.kernel@...il.com]
> Sent: Thursday, November 15, 2012 9:47 PM
> To: Moore, Robert
> Cc: 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,
> 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