| Message ID | 1445002020-12672-1-git-send-email-Vincent.Wan@amd.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On Fri, Oct 16, 2015 at 09:27:00AM -0400, Vincent Wan wrote: > Detecting platform supports i8042 or not, AMD resorted to > BIOS's FADT i8042 flag. > > Signed-off-by: Vincent Wan <Vincent.Wan@amd.com> > --- > drivers/input/serio/i8042-x86ia64io.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h > index c115565..bf3a605 100644 > --- a/drivers/input/serio/i8042-x86ia64io.h > +++ b/drivers/input/serio/i8042-x86ia64io.h > @@ -9,6 +9,7 @@ > > #ifdef CONFIG_X86 > #include <asm/x86_init.h> > +#include <linux/acpi.h> > #endif > > /* > @@ -1047,6 +1048,11 @@ static int __init i8042_platform_init(void) > /* Just return if pre-detection shows no i8042 controller exist */ > if (!x86_platform.i8042_detect()) > return -ENODEV; > + > + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { Why the vendor check if you're accessing a bit defined in the ACPI spec? > + if (!(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042)) > + return -ENODEV; > + }
On Fri, Oct 16, 2015 at 04:58:09PM +0800, Borislav Petkov wrote: > On Fri, Oct 16, 2015 at 09:27:00AM -0400, Vincent Wan wrote: > > Detecting platform supports i8042 or not, AMD resorted to > > BIOS's FADT i8042 flag. > > > > Signed-off-by: Vincent Wan <Vincent.Wan@amd.com> > > --- > > drivers/input/serio/i8042-x86ia64io.h | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h > > index c115565..bf3a605 100644 > > --- a/drivers/input/serio/i8042-x86ia64io.h > > +++ b/drivers/input/serio/i8042-x86ia64io.h > > @@ -9,6 +9,7 @@ > > > > #ifdef CONFIG_X86 > > #include <asm/x86_init.h> > > +#include <linux/acpi.h> > > #endif > > > > /* > > @@ -1047,6 +1048,11 @@ static int __init i8042_platform_init(void) > > /* Just return if pre-detection shows no i8042 controller exist */ > > if (!x86_platform.i8042_detect()) > > return -ENODEV; > > + > > + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { > > Why the vendor check if you're accessing a bit defined in the ACPI spec? > Yeah, I have the same concern with AMD vendor check. :) Aaron, could you please help to check the impact at intel platfomrs if we remove this check? Thanks, Rui -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-10-16 16:58 GMT+08:00 Borislav Petkov <bp@alien8.de>: > On Fri, Oct 16, 2015 at 09:27:00AM -0400, Vincent Wan wrote: >> Detecting platform supports i8042 or not, AMD resorted to >> BIOS's FADT i8042 flag. >> >> Signed-off-by: Vincent Wan <Vincent.Wan@amd.com> >> --- >> drivers/input/serio/i8042-x86ia64io.h | 6 ++++++ >> 1 file changed, 6 insertions(+) >> /* >> @@ -1047,6 +1048,11 @@ static int __init i8042_platform_init(void) >> /* Just return if pre-detection shows no i8042 controller exist */ >> if (!x86_platform.i8042_detect()) >> return -ENODEV; >> + >> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { > > Why the vendor check if you're accessing a bit defined in the ACPI spec? From intel's 'x86_platform.i8042_detect' implementation, I doubt if their BIOS is providing this i8024 flag. So I have to implement my codes carefully. > >> + if (!(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042)) >> + return -ENODEV; >> + } > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply.
On Fri, Oct 16, 2015 at 05:35:40PM +0800, Wan ZongShun wrote: > 2015-10-16 16:58 GMT+08:00 Borislav Petkov <bp@alien8.de>: > > On Fri, Oct 16, 2015 at 09:27:00AM -0400, Vincent Wan wrote: > >> Detecting platform supports i8042 or not, AMD resorted to > >> BIOS's FADT i8042 flag. > >> > >> Signed-off-by: Vincent Wan <Vincent.Wan@amd.com> > >> --- > >> drivers/input/serio/i8042-x86ia64io.h | 6 ++++++ > >> 1 file changed, 6 insertions(+) > > >> /* > >> @@ -1047,6 +1048,11 @@ static int __init i8042_platform_init(void) > >> /* Just return if pre-detection shows no i8042 controller exist */ > >> if (!x86_platform.i8042_detect()) > >> return -ENODEV; > >> + > >> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { > > > > Why the vendor check if you're accessing a bit defined in the ACPI spec? > > From intel's 'x86_platform.i8042_detect' implementation, I doubt if > their BIOS is providing this i8024 flag. Why would you doubt that - it is at least in ACPI v4, if not earlier. If you still doubt that, go and check it or ask Intel people. > So I have to implement my codes carefully. What are you people talking about?! It is in the ACPI spec - this bit is either set or not. If it is not set, then that's a problem. But then it is the problem of this one BIOS. Vendor checks have nothing to do in vendor-agnostic code. Besides, there's intel_mid_i8042_detect() which is platform-specific and Intel can supply a specific ->detect() function if, in the very distant chance, they don't implement that bit. Still no need for a vendor check!
On Fri, Oct 16, 2015 at 12:21:55PM +0200, Borislav Petkov wrote: > On Fri, Oct 16, 2015 at 05:35:40PM +0800, Wan ZongShun wrote: > > 2015-10-16 16:58 GMT+08:00 Borislav Petkov <bp@alien8.de>: > > > On Fri, Oct 16, 2015 at 09:27:00AM -0400, Vincent Wan wrote: > > >> Detecting platform supports i8042 or not, AMD resorted to > > >> BIOS's FADT i8042 flag. > > >> > > >> Signed-off-by: Vincent Wan <Vincent.Wan@amd.com> > > >> --- > > >> drivers/input/serio/i8042-x86ia64io.h | 6 ++++++ > > >> 1 file changed, 6 insertions(+) > > > > >> /* > > >> @@ -1047,6 +1048,11 @@ static int __init i8042_platform_init(void) > > >> /* Just return if pre-detection shows no i8042 controller exist */ > > >> if (!x86_platform.i8042_detect()) > > >> return -ENODEV; > > >> + > > >> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { > > > > > > Why the vendor check if you're accessing a bit defined in the ACPI spec? > > > > From intel's 'x86_platform.i8042_detect' implementation, I doubt if > > their BIOS is providing this i8024 flag. > > Why would you doubt that - it is at least in ACPI v4, if not earlier. If > you still doubt that, go and check it or ask Intel people. > > > So I have to implement my codes carefully. > > What are you people talking about?! > > It is in the ACPI spec - this bit is either set or not. If it is not Well, the fact that is is in a spec does not mean that vendors follow it (and BTW I do not think that AMD as a CPU vendor can vouch for a random notebook or desktop vendor that acquired and then hacked on some version of some BIOS from some BIOS vendor so I agree that this check is a no-go). Historically we did not trust BIOS data with regard to i8042 on x86. Maybe we should now using certain date cutoff (anything manufactured past 201[2345?]). Does Windows respect this flag? If it does then we could also trust it, and not only on AMD but for all x86 CPUs. Thanks.
> Subject: Re: [PATCH] input: i8042: add quirk to implement i8042 detect for AMD > > On Fri, Oct 16, 2015 at 12:21:55PM +0200, Borislav Petkov wrote: > > On Fri, Oct 16, 2015 at 05:35:40PM +0800, Wan ZongShun wrote: > > > 2015-10-16 16:58 GMT+08:00 Borislav Petkov <bp@alien8.de>: > > > > On Fri, Oct 16, 2015 at 09:27:00AM -0400, Vincent Wan wrote: > > > >> Detecting platform supports i8042 or not, AMD resorted to BIOS's > > > >> FADT i8042 flag. > > > >> > > > >> Signed-off-by: Vincent Wan <Vincent.Wan@amd.com> > > > >> --- > > > >> drivers/input/serio/i8042-x86ia64io.h | 6 ++++++ > > > >> 1 file changed, 6 insertions(+) > > > > > > >> /* > > > >> @@ -1047,6 +1048,11 @@ static int __init i8042_platform_init(void) > > > >> /* Just return if pre-detection shows no i8042 controller exist */ > > > >> if (!x86_platform.i8042_detect()) > > > >> return -ENODEV; > > > >> + > > > >> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { > > > > > > > > Why the vendor check if you're accessing a bit defined in the ACPI spec? > > > > > > From intel's 'x86_platform.i8042_detect' implementation, I doubt if > > > their BIOS is providing this i8024 flag. > > > > Why would you doubt that - it is at least in ACPI v4, if not earlier. > > If you still doubt that, go and check it or ask Intel people. > > > > > So I have to implement my codes carefully. > > > > What are you people talking about?! > > > > It is in the ACPI spec - this bit is either set or not. If it is not > > Well, the fact that is is in a spec does not mean that vendors follow it (and > BTW I do not think that AMD as a CPU vendor can vouch for a random notebook > or desktop vendor that acquired and then hacked on some version of some BIOS > from some BIOS vendor so I agree that this check is a no-go). > > Historically we did not trust BIOS data with regard to i8042 on x86. > Maybe we should now using certain date cutoff (anything manufactured past > 201[2345?]). > > Does Windows respect this flag? If it does then we could also trust it, and not > only on AMD but for all x86 CPUs. Dmitry ,I found a document in MS MSDN, link: http://download.microsoft.com/download/3/2/c/32cb69db-71e3-447e-a91b-4070cdc4548a/WLP22-10_a.doc I copy a piece of comments from this doc here, seems MS is requiring this flag for legacy free system from BIOS. If you think it is ok, I will send v2 patch later to add detect function for all x86 CPUs. " A4.1 - Legacy-Free PC System - Windows Compatibility A4.1.1 - ACPI legacy-free support is reported as described in "ACPI Changes for Legacy Free" Any x86-based system designs that reduce the amount of legacy ISR support in conjunction with other legacy removal efforts (such as 8042 removal) must still provide the necessary ISRs required to boot systems using BIOS. The minimum requirements include support for ISR 8h, 13h, and 19h (all functions), and ISR 15h, function E820h. To achieve this, Fixed ACPI Description Table (FADT) settings must be supported and correctly implemented, including support for reporting legacy-free and hard reset/boot capabilities, as described in the specification at http://go.microsoft.com/fwlink/?LinkId=36697 * A4.1.1.1 LEGACY_DEVICES flag is set to 0 in the ACPI FADT as defined in ACPI section 5.2.1. * A4.1.1.2 ACPI reset mechanism as defined in ACPI section 4.7.5. * A4.1.1.3 8042 flag is set to 0 in systems that do not include an 8042 controller; value is set to 1 in a mobile or desktop system that includes an 8042 controller. * A4.1.1.4 Debug Port Table in the BIOS, as described in ACPI section 5.2.11. " > > Thanks. > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h index c115565..bf3a605 100644 --- a/drivers/input/serio/i8042-x86ia64io.h +++ b/drivers/input/serio/i8042-x86ia64io.h @@ -9,6 +9,7 @@ #ifdef CONFIG_X86 #include <asm/x86_init.h> +#include <linux/acpi.h> #endif /* @@ -1047,6 +1048,11 @@ static int __init i8042_platform_init(void) /* Just return if pre-detection shows no i8042 controller exist */ if (!x86_platform.i8042_detect()) return -ENODEV; + + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { + if (!(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042)) + return -ENODEV; + } #endif /*
Detecting platform supports i8042 or not, AMD resorted to BIOS's FADT i8042 flag. Signed-off-by: Vincent Wan <Vincent.Wan@amd.com> --- drivers/input/serio/i8042-x86ia64io.h | 6 ++++++ 1 file changed, 6 insertions(+)