Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/10 16:06, Ingo Molnar wrote: > > * Li, Aubrey wrote: > - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' - clean up 'global_clock_event' handling: instead of a global variable, move its management into x86_platform_ops::get_clockevent() and set the method to hpet/pit/abp/etc. specific handlers that return the right clockevent device. - in your x86_reduced_hw_init() function add the hpet clockevent device to x86_platform_ops::get_clockevent, overriding the default PIT. >> >> how about this one? >> >> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c >> index b9e30da..70955d6 100644 >> --- a/arch/x86/kernel/acpi/boot.c >> +++ b/arch/x86/kernel/acpi/boot.c >> @@ -1541,6 +1541,16 @@ int __init early_acpi_boot_init(void) >> */ >> early_acpi_process_madt(); >> >> +/* >> + * Override x86_init functions and bypass legacy pic >> + * in hardware-reduced ACPI mode >> + */ >> +if (acpi_gbl_reduced_hardware) { >> +x86_init.timers.timer_init = x86_init_noop; >> +x86_init.irqs.pre_vector_init = x86_init_noop; >> +legacy_pic = _legacy_pic; >> +} > > Sounds good to me, assuming it builds, boots and works! :-) Yeah, it's verified on my ASUS-T100 machine. > > A couple of extra suggestions: > > 1) > > I'd suggest moving it into its own dedicated, appropriately named > static function, to make it clear that 'ACPI Reduced Hardware' init > happens there. > > 2) > > I'd also initialize it like this: > > x86_init.timers.timer_init = x86_init_noop; > x86_init.irqs.pre_vector_init = x86_init_noop; > legacy_pic = _legacy_pic; > > to make the noop patterns stand out better. Thanks for the suggestions, I'll send the patch out soon. > > 3) > > Once all is said and done, please also make acpi_gbl_reduced_hardware > a flag internal to the ACPI code, not exposed to and used by other > bits of x86 code. > >> If the above makes sense, I'll send poweroff and reboot change >> together in a seperate patch. > > Yeah, please send them in a single series though, so they form a > logical group. Execution flow: ->early_acpi_boot_init() -->acpi_reduced_hw_init() ->reboot_init() ->acpi_sleep_init() ->efi_shutdown_init() For poweroff, it will take no effect if we override pm_power_off during reduced hardware initialization, because acpi_sleep_init() will override it again to acpi_power_off. For reboot, it's really a quirk to have to force reboot mode to be EFI_RESET_WARM, so we can't just set the reboot type and done, there is also a logic that DMI quirks table takes precedence over EFI quirk. So, IMHO, we either need an EFI cross function called from ACPI to ask EFI to reboot and poweroff system in reduced hw mode, or we need a copy of acpi_gbl_reduced_hardware in EFI to do so. What do you think? Thanks, -Aubrey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey wrote: > >> - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' > >> > >> - clean up 'global_clock_event' handling: instead of a global > >>variable, move its management into x86_platform_ops::get_clockevent() > >>and set the method to hpet/pit/abp/etc. specific handlers that > >>return the right clockevent device. > >> > >> - in your x86_reduced_hw_init() function add the hpet clockevent > >>device to x86_platform_ops::get_clockevent, overriding the default > >>PIT. > > how about this one? > > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > index b9e30da..70955d6 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -1541,6 +1541,16 @@ int __init early_acpi_boot_init(void) >*/ > early_acpi_process_madt(); > > + /* > + * Override x86_init functions and bypass legacy pic > + * in hardware-reduced ACPI mode > + */ > + if (acpi_gbl_reduced_hardware) { > + x86_init.timers.timer_init = x86_init_noop; > + x86_init.irqs.pre_vector_init = x86_init_noop; > + legacy_pic = _legacy_pic; > + } Sounds good to me, assuming it builds, boots and works! :-) A couple of extra suggestions: 1) I'd suggest moving it into its own dedicated, appropriately named static function, to make it clear that 'ACPI Reduced Hardware' init happens there. 2) I'd also initialize it like this: x86_init.timers.timer_init = x86_init_noop; x86_init.irqs.pre_vector_init = x86_init_noop; legacy_pic = _legacy_pic; to make the noop patterns stand out better. 3) Once all is said and done, please also make acpi_gbl_reduced_hardware a flag internal to the ACPI code, not exposed to and used by other bits of x86 code. > If the above makes sense, I'll send poweroff and reboot change > together in a seperate patch. Yeah, please send them in a single series though, so they form a logical group. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/10 16:06, Ingo Molnar wrote: * Li, Aubrey aubrey...@linux.intel.com wrote: - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' - clean up 'global_clock_event' handling: instead of a global variable, move its management into x86_platform_ops::get_clockevent() and set the method to hpet/pit/abp/etc. specific handlers that return the right clockevent device. - in your x86_reduced_hw_init() function add the hpet clockevent device to x86_platform_ops::get_clockevent, overriding the default PIT. how about this one? diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index b9e30da..70955d6 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -1541,6 +1541,16 @@ int __init early_acpi_boot_init(void) */ early_acpi_process_madt(); +/* + * Override x86_init functions and bypass legacy pic + * in hardware-reduced ACPI mode + */ +if (acpi_gbl_reduced_hardware) { +x86_init.timers.timer_init = x86_init_noop; +x86_init.irqs.pre_vector_init = x86_init_noop; +legacy_pic = null_legacy_pic; +} Sounds good to me, assuming it builds, boots and works! :-) Yeah, it's verified on my ASUS-T100 machine. A couple of extra suggestions: 1) I'd suggest moving it into its own dedicated, appropriately named static function, to make it clear that 'ACPI Reduced Hardware' init happens there. 2) I'd also initialize it like this: x86_init.timers.timer_init = x86_init_noop; x86_init.irqs.pre_vector_init = x86_init_noop; legacy_pic = null_legacy_pic; to make the noop patterns stand out better. Thanks for the suggestions, I'll send the patch out soon. 3) Once all is said and done, please also make acpi_gbl_reduced_hardware a flag internal to the ACPI code, not exposed to and used by other bits of x86 code. If the above makes sense, I'll send poweroff and reboot change together in a seperate patch. Yeah, please send them in a single series though, so they form a logical group. Execution flow: -early_acpi_boot_init() --acpi_reduced_hw_init() -reboot_init() -acpi_sleep_init() -efi_shutdown_init() For poweroff, it will take no effect if we override pm_power_off during reduced hardware initialization, because acpi_sleep_init() will override it again to acpi_power_off. For reboot, it's really a quirk to have to force reboot mode to be EFI_RESET_WARM, so we can't just set the reboot type and done, there is also a logic that DMI quirks table takes precedence over EFI quirk. So, IMHO, we either need an EFI cross function called from ACPI to ask EFI to reboot and poweroff system in reduced hw mode, or we need a copy of acpi_gbl_reduced_hardware in EFI to do so. What do you think? Thanks, -Aubrey -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey aubrey...@linux.intel.com wrote: - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' - clean up 'global_clock_event' handling: instead of a global variable, move its management into x86_platform_ops::get_clockevent() and set the method to hpet/pit/abp/etc. specific handlers that return the right clockevent device. - in your x86_reduced_hw_init() function add the hpet clockevent device to x86_platform_ops::get_clockevent, overriding the default PIT. how about this one? diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index b9e30da..70955d6 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -1541,6 +1541,16 @@ int __init early_acpi_boot_init(void) */ early_acpi_process_madt(); + /* + * Override x86_init functions and bypass legacy pic + * in hardware-reduced ACPI mode + */ + if (acpi_gbl_reduced_hardware) { + x86_init.timers.timer_init = x86_init_noop; + x86_init.irqs.pre_vector_init = x86_init_noop; + legacy_pic = null_legacy_pic; + } Sounds good to me, assuming it builds, boots and works! :-) A couple of extra suggestions: 1) I'd suggest moving it into its own dedicated, appropriately named static function, to make it clear that 'ACPI Reduced Hardware' init happens there. 2) I'd also initialize it like this: x86_init.timers.timer_init = x86_init_noop; x86_init.irqs.pre_vector_init = x86_init_noop; legacy_pic = null_legacy_pic; to make the noop patterns stand out better. 3) Once all is said and done, please also make acpi_gbl_reduced_hardware a flag internal to the ACPI code, not exposed to and used by other bits of x86 code. If the above makes sense, I'll send poweroff and reboot change together in a seperate patch. Yeah, please send them in a single series though, so they form a logical group. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/5 20:42, Li, Aubrey wrote: > On 2015/3/5 19:36, Ingo Molnar wrote: >> >> * Li, Aubrey wrote: >> >>> On 2015/3/5 4:11, Ingo Molnar wrote: * Arjan van de Ven wrote: > On 3/4/2015 1:50 AM, Borislav Petkov wrote: >> On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. >>> >>> ideally, the presence of that flag in the firmware table will clear/set >>> more global settings, >>> for example, having that flag should cause the 8042 input code to not >>> probe for the 8042. >>> >>> for interrupts, there really ought to be a "apic first/only" mode, >>> which is then used on >>> all modern systems (not just hw reduced). >> >> Do we need some sort of platform-specific querying interfaces now too, >> similar to cpu_has()? I.e., platform_has()... >> >> if (platform_has(X86_PLATFORM_REDUCED_HW)) >> do stuff.. > > more like > > platform_has(X86_PLATFORM_PIT) > > etc, one for each legacy io item Precisely. The main problem is the generic, 'lumps everything together' nature of the acpi_gbl_reduced_hardware flag. (Like the big kernel lock lumped together all sorts of locking rules and semantics.) Properly split out, feature-ish or driver-ish interfaces for PIT and other legacy details are the proper approach to 'turn them off'. - x86_platform is a function pointer driven, driver-ish interface. - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish interface. Both are fine - for something as separate as the PIT (or the PIC) it might make more sense to go towards a 'driver' interface though, as modern drivers are (and will be) much different from the legacy PIT. Whichever method is used, low level platforms can just switch them on/off in their enumeration/detection routines, while the generic code will have them enabled by default. >>> >>> Whichever method is used, we will face a problem how to determine >>> PIT exists or not. >>> >>> When we enabled Bay Trail-T platform at the beginning, we were >>> trying to make the code as generic as possible, and it works >>> properly up to now. So we don't have a SUBARCH like >>> X86_SUBARCH_INTEL_MID to use the platform specific functions. And >>> for now I'm not quite sure it's a good idea to create one. >>> >>> If we make it as a flag driven, I don't know there is a flag in >>> firmware better than ACPI HW reduced flag(Of course it's not good >>> enough to cover all the cases). Or if we want to use platform info >>> to turn on/off this flag, we'll have to maintain a platform list, >>> which may be longer and more complicated than worth doing that. >> >> Well, it's not nearly so difficult, because you already have a >> platform flag: acpi_gbl_reduced_hardware. >> >> What I object against is to infest generic codepaths with unreadable, >> unrobust crap like: >> >> + if (acpi_gbl_reduced_hardware) { >> + pr_info("Using NULL legacy PIC\n"); >> + legacy_pic = _legacy_pic; >> + } else >> + legacy_pic->init(0); >> >> To solve that, add a small (early) init function (say >> 'x86_reduced_hw_init()') that sets up the right driver >> selections if acpi_gbl_reduced_hardware is set: >> >> - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' >> >> - clean up 'global_clock_event' handling: instead of a global >>variable, move its management into x86_platform_ops::get_clockevent() >>and set the method to hpet/pit/abp/etc. specific handlers that >>return the right clockevent device. >> >> - in your x86_reduced_hw_init() function add the hpet clockevent >>device to x86_platform_ops::get_clockevent, overriding the default >>PIT. >> how about this one? diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index b9e30da..70955d6 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -1541,6 +1541,16 @@ int __init early_acpi_boot_init(void) */ early_acpi_process_madt(); + /* +* Override x86_init functions and bypass legacy pic +* in hardware-reduced ACPI mode +*/ + if (acpi_gbl_reduced_hardware) { + x86_init.timers.timer_init = x86_init_noop; + x86_init.irqs.pre_vector_init = x86_init_noop; + legacy_pic = _legacy_pic; + } + return 0; } > >> - in x86_reduced_hw_init() set pm_power_off. >> >> - set 'reboot_type' and remove the acpi_gbl_reduced_hardware hack >>from efi_reboot_required(). >> > I'll do more investigation above items but I want to leave at least > these two as the quirk today unless I am convinced I can do that because > from my understanding, UEFI runtime
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/5 20:42, Li, Aubrey wrote: On 2015/3/5 19:36, Ingo Molnar wrote: * Li, Aubrey aubrey...@linux.intel.com wrote: On 2015/3/5 4:11, Ingo Molnar wrote: * Arjan van de Ven ar...@linux.intel.com wrote: On 3/4/2015 1:50 AM, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. more like platform_has(X86_PLATFORM_PIT) etc, one for each legacy io item Precisely. The main problem is the generic, 'lumps everything together' nature of the acpi_gbl_reduced_hardware flag. (Like the big kernel lock lumped together all sorts of locking rules and semantics.) Properly split out, feature-ish or driver-ish interfaces for PIT and other legacy details are the proper approach to 'turn them off'. - x86_platform is a function pointer driven, driver-ish interface. - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish interface. Both are fine - for something as separate as the PIT (or the PIC) it might make more sense to go towards a 'driver' interface though, as modern drivers are (and will be) much different from the legacy PIT. Whichever method is used, low level platforms can just switch them on/off in their enumeration/detection routines, while the generic code will have them enabled by default. Whichever method is used, we will face a problem how to determine PIT exists or not. When we enabled Bay Trail-T platform at the beginning, we were trying to make the code as generic as possible, and it works properly up to now. So we don't have a SUBARCH like X86_SUBARCH_INTEL_MID to use the platform specific functions. And for now I'm not quite sure it's a good idea to create one. If we make it as a flag driven, I don't know there is a flag in firmware better than ACPI HW reduced flag(Of course it's not good enough to cover all the cases). Or if we want to use platform info to turn on/off this flag, we'll have to maintain a platform list, which may be longer and more complicated than worth doing that. Well, it's not nearly so difficult, because you already have a platform flag: acpi_gbl_reduced_hardware. What I object against is to infest generic codepaths with unreadable, unrobust crap like: + if (acpi_gbl_reduced_hardware) { + pr_info(Using NULL legacy PIC\n); + legacy_pic = null_legacy_pic; + } else + legacy_pic-init(0); To solve that, add a small (early) init function (say 'x86_reduced_hw_init()') that sets up the right driver selections if acpi_gbl_reduced_hardware is set: - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' - clean up 'global_clock_event' handling: instead of a global variable, move its management into x86_platform_ops::get_clockevent() and set the method to hpet/pit/abp/etc. specific handlers that return the right clockevent device. - in your x86_reduced_hw_init() function add the hpet clockevent device to x86_platform_ops::get_clockevent, overriding the default PIT. how about this one? diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index b9e30da..70955d6 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -1541,6 +1541,16 @@ int __init early_acpi_boot_init(void) */ early_acpi_process_madt(); + /* +* Override x86_init functions and bypass legacy pic +* in hardware-reduced ACPI mode +*/ + if (acpi_gbl_reduced_hardware) { + x86_init.timers.timer_init = x86_init_noop; + x86_init.irqs.pre_vector_init = x86_init_noop; + legacy_pic = null_legacy_pic; + } + return 0; } - in x86_reduced_hw_init() set pm_power_off. - set 'reboot_type' and remove the acpi_gbl_reduced_hardware hack from efi_reboot_required(). I'll do more investigation above items but I want to leave at least these two as the quirk today unless I am convinced I can do that because from my understanding, UEFI runtime services should not be supported in reduced hw mode. If the above makes sense, I'll send poweroff and reboot change together in a seperate patch. Thanks, -Aubrey -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
> Besides, the "ACPI reduced hardware" case is kind of a red herring here, > because it most likely is not the only case when we'll want has_8259_pic() > to return 0 (quite likely, we'll want that on all BayTrail-based systems, > for example). No. Only those with ACPI reduced firmware. For others you do want to touch the 8259. It *is* about ACPI reduced. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
> I'll do more investigation above items but I want to leave at least > these two as the quirk today unless I am convinced I can do that because > from my understanding, UEFI runtime services should not be supported in > reduced hw mode. What actually matters in this space is what Microsoft does. The spec is unfortunately rather irrelevant. If MS do UEFI runtime services in reduced hw mode then the firmware and system will do so. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/5 19:36, Ingo Molnar wrote: > > * Li, Aubrey wrote: > >> On 2015/3/5 4:11, Ingo Molnar wrote: >>> >>> * Arjan van de Ven wrote: >>> On 3/4/2015 1:50 AM, Borislav Petkov wrote: > On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: >>> >>> Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code >>> is a mistake. >> >> ideally, the presence of that flag in the firmware table will clear/set >> more global settings, >> for example, having that flag should cause the 8042 input code to not >> probe for the 8042. >> >> for interrupts, there really ought to be a "apic first/only" mode, which >> is then used on >> all modern systems (not just hw reduced). > > Do we need some sort of platform-specific querying interfaces now too, > similar to cpu_has()? I.e., platform_has()... > > if (platform_has(X86_PLATFORM_REDUCED_HW)) > do stuff.. more like platform_has(X86_PLATFORM_PIT) etc, one for each legacy io item >>> >>> Precisely. The main problem is the generic, 'lumps everything >>> together' nature of the acpi_gbl_reduced_hardware flag. >>> >>> (Like the big kernel lock lumped together all sorts of locking rules >>> and semantics.) >>> >>> Properly split out, feature-ish or driver-ish interfaces for PIT and >>> other legacy details are the proper approach to 'turn them off'. >>> >>> - x86_platform is a function pointer driven, driver-ish interface. >>> >>> - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish >>>interface. >>> >>> Both are fine - for something as separate as the PIT (or the PIC) >>> it might make more sense to go towards a 'driver' interface >>> though, as modern drivers are (and will be) much different from >>> the legacy PIT. >>> >>> Whichever method is used, low level platforms can just switch them >>> on/off in their enumeration/detection routines, while the generic >>> code will have them enabled by default. >> >> Whichever method is used, we will face a problem how to determine >> PIT exists or not. >> >> When we enabled Bay Trail-T platform at the beginning, we were >> trying to make the code as generic as possible, and it works >> properly up to now. So we don't have a SUBARCH like >> X86_SUBARCH_INTEL_MID to use the platform specific functions. And >> for now I'm not quite sure it's a good idea to create one. >> >> If we make it as a flag driven, I don't know there is a flag in >> firmware better than ACPI HW reduced flag(Of course it's not good >> enough to cover all the cases). Or if we want to use platform info >> to turn on/off this flag, we'll have to maintain a platform list, >> which may be longer and more complicated than worth doing that. > > Well, it's not nearly so difficult, because you already have a > platform flag: acpi_gbl_reduced_hardware. > > What I object against is to infest generic codepaths with unreadable, > unrobust crap like: > > + if (acpi_gbl_reduced_hardware) { > + pr_info("Using NULL legacy PIC\n"); > + legacy_pic = _legacy_pic; > + } else > + legacy_pic->init(0); > > To solve that, add a small (early) init function (say > 'x86_reduced_hw_init()') that sets up the right driver > selections if acpi_gbl_reduced_hardware is set: > > - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' > > - clean up 'global_clock_event' handling: instead of a global >variable, move its management into x86_platform_ops::get_clockevent() >and set the method to hpet/pit/abp/etc. specific handlers that >return the right clockevent device. > > - in your x86_reduced_hw_init() function add the hpet clockevent >device to x86_platform_ops::get_clockevent, overriding the default >PIT. > > - in x86_reduced_hw_init() set pm_power_off. > > - set 'reboot_type' and remove the acpi_gbl_reduced_hardware hack >from efi_reboot_required(). > I'll do more investigation above items but I want to leave at least these two as the quirk today unless I am convinced I can do that because from my understanding, UEFI runtime services should not be supported in reduced hw mode. > etc. > > Just keep the generic init codepaths free of those random selections > based on global flags, okay? > Agree. Thanks, -Aubrey > Thanks, > > Ingo > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey wrote: > On 2015/3/5 4:11, Ingo Molnar wrote: > > > > * Arjan van de Ven wrote: > > > >> On 3/4/2015 1:50 AM, Borislav Petkov wrote: > >>> On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: > > > > Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code > > is a mistake. > > ideally, the presence of that flag in the firmware table will clear/set > more global settings, > for example, having that flag should cause the 8042 input code to not > probe for the 8042. > > for interrupts, there really ought to be a "apic first/only" mode, which > is then used on > all modern systems (not just hw reduced). > >>> > >>> Do we need some sort of platform-specific querying interfaces now too, > >>> similar to cpu_has()? I.e., platform_has()... > >>> > >>> if (platform_has(X86_PLATFORM_REDUCED_HW)) > >>> do stuff.. > >> > >> more like > >> > >> platform_has(X86_PLATFORM_PIT) > >> > >> etc, one for each legacy io item > > > > Precisely. The main problem is the generic, 'lumps everything > > together' nature of the acpi_gbl_reduced_hardware flag. > > > > (Like the big kernel lock lumped together all sorts of locking rules > > and semantics.) > > > > Properly split out, feature-ish or driver-ish interfaces for PIT and > > other legacy details are the proper approach to 'turn them off'. > > > > - x86_platform is a function pointer driven, driver-ish interface. > > > > - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish > >interface. > > > > Both are fine - for something as separate as the PIT (or the PIC) > > it might make more sense to go towards a 'driver' interface > > though, as modern drivers are (and will be) much different from > > the legacy PIT. > > > > Whichever method is used, low level platforms can just switch them > > on/off in their enumeration/detection routines, while the generic > > code will have them enabled by default. > > Whichever method is used, we will face a problem how to determine > PIT exists or not. > > When we enabled Bay Trail-T platform at the beginning, we were > trying to make the code as generic as possible, and it works > properly up to now. So we don't have a SUBARCH like > X86_SUBARCH_INTEL_MID to use the platform specific functions. And > for now I'm not quite sure it's a good idea to create one. > > If we make it as a flag driven, I don't know there is a flag in > firmware better than ACPI HW reduced flag(Of course it's not good > enough to cover all the cases). Or if we want to use platform info > to turn on/off this flag, we'll have to maintain a platform list, > which may be longer and more complicated than worth doing that. Well, it's not nearly so difficult, because you already have a platform flag: acpi_gbl_reduced_hardware. What I object against is to infest generic codepaths with unreadable, unrobust crap like: + if (acpi_gbl_reduced_hardware) { + pr_info("Using NULL legacy PIC\n"); + legacy_pic = _legacy_pic; + } else + legacy_pic->init(0); To solve that, add a small (early) init function (say 'x86_reduced_hw_init()') that sets up the right driver selections if acpi_gbl_reduced_hardware is set: - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' - clean up 'global_clock_event' handling: instead of a global variable, move its management into x86_platform_ops::get_clockevent() and set the method to hpet/pit/abp/etc. specific handlers that return the right clockevent device. - in your x86_reduced_hw_init() function add the hpet clockevent device to x86_platform_ops::get_clockevent, overriding the default PIT. - in x86_reduced_hw_init() set pm_power_off. - set 'reboot_type' and remove the acpi_gbl_reduced_hardware hack from efi_reboot_required(). etc. Just keep the generic init codepaths free of those random selections based on global flags, okay? Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/5 5:52, Rafael J. Wysocki wrote: > On Wednesday, March 04, 2015 08:21:01 PM Alan Cox wrote: >> On Wed, 2015-03-04 at 15:05 +0100, Borislav Petkov wrote: >>> On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: Sort of. What we need is a "do not touch PIC/PIT" bit for the code that tries to fall back to them in some cases (which may appear to work if the hardware is physically there, but it may confuse the platform). >>> >>> Can "some cases" detection be nicely put into a x86_platform >>> platform-specific method? >> >> In some cases they don't belong in x86, ACPI is also used for ARM64. >> >> However >> >> if ( has_8259_pic() ) >> >> is trivally 0, 1 or some platform or acpi provided method. > > And which is how that should have been implemented to start with IMO. > > Besides, the "ACPI reduced hardware" case is kind of a red herring here, > because it most likely is not the only case when we'll want has_8259_pic() > to return 0 (quite likely, we'll want that on all BayTrail-based systems, > for example). > BayTrail-based systems has BayTrail-I, BayTrail-M, BayTrail-D, BayTrail-T, BayTrail-T/CR. BayTrail-D is a desktop and BayTrail-M is a mobile/laptop and 8259 exists on both systems and I don't think we want to bypass it. ACPI reduced hardware is the best case in my mind unless you want to enumerate the platform one by one. can we make a global variable u8 has_8259; and initialize it by acpi reduced hardware flag? or a wrapper function? Thanks, -Aubrey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/5 4:11, Ingo Molnar wrote: > > * Arjan van de Ven wrote: > >> On 3/4/2015 1:50 AM, Borislav Petkov wrote: >>> On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: > > Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code > is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a "apic first/only" mode, which is then used on all modern systems (not just hw reduced). >>> >>> Do we need some sort of platform-specific querying interfaces now too, >>> similar to cpu_has()? I.e., platform_has()... >>> >>> if (platform_has(X86_PLATFORM_REDUCED_HW)) >>> do stuff.. >> >> more like >> >> platform_has(X86_PLATFORM_PIT) >> >> etc, one for each legacy io item > > Precisely. The main problem is the generic, 'lumps everything > together' nature of the acpi_gbl_reduced_hardware flag. > > (Like the big kernel lock lumped together all sorts of locking rules > and semantics.) > > Properly split out, feature-ish or driver-ish interfaces for PIT and > other legacy details are the proper approach to 'turn them off'. > > - x86_platform is a function pointer driven, driver-ish interface. > > - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish >interface. > > Both are fine - for something as separate as the PIT (or the PIC) it > might make more sense to go towards a 'driver' interface though, as > modern drivers are (and will be) much different from the legacy PIT. > > Whichever method is used, low level platforms can just switch them > on/off in their enumeration/detection routines, while the generic code > will have them enabled by default. Whichever method is used, we will face a problem how to determine PIT exists or not. When we enabled Bay Trail-T platform at the beginning, we were trying to make the code as generic as possible, and it works properly up to now. So we don't have a SUBARCH like X86_SUBARCH_INTEL_MID to use the platform specific functions. And for now I'm not quite sure it's a good idea to create one. If we make it as a flag driven, I don't know there is a flag in firmware better than ACPI HW reduced flag(Of course it's not good enough to cover all the cases). Or if we want to use platform info to turn on/off this flag, we'll have to maintain a platform list, which may be longer and more complicated than worth doing that. Thanks, -Aubrey > >> so we can clear it on hw reduced, but also in other cases. hw >> reduced is one way, but I'd be surprised if there weren't other ways >> (like quirks) where we'd want to do the same things > > Exactly. The key step is the proper, clean separation out of hardware > interfaces. > > Thanks, > > Ingo > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
Besides, the ACPI reduced hardware case is kind of a red herring here, because it most likely is not the only case when we'll want has_8259_pic() to return 0 (quite likely, we'll want that on all BayTrail-based systems, for example). No. Only those with ACPI reduced firmware. For others you do want to touch the 8259. It *is* about ACPI reduced. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
I'll do more investigation above items but I want to leave at least these two as the quirk today unless I am convinced I can do that because from my understanding, UEFI runtime services should not be supported in reduced hw mode. What actually matters in this space is what Microsoft does. The spec is unfortunately rather irrelevant. If MS do UEFI runtime services in reduced hw mode then the firmware and system will do so. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/5 19:36, Ingo Molnar wrote: * Li, Aubrey aubrey...@linux.intel.com wrote: On 2015/3/5 4:11, Ingo Molnar wrote: * Arjan van de Ven ar...@linux.intel.com wrote: On 3/4/2015 1:50 AM, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. more like platform_has(X86_PLATFORM_PIT) etc, one for each legacy io item Precisely. The main problem is the generic, 'lumps everything together' nature of the acpi_gbl_reduced_hardware flag. (Like the big kernel lock lumped together all sorts of locking rules and semantics.) Properly split out, feature-ish or driver-ish interfaces for PIT and other legacy details are the proper approach to 'turn them off'. - x86_platform is a function pointer driven, driver-ish interface. - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish interface. Both are fine - for something as separate as the PIT (or the PIC) it might make more sense to go towards a 'driver' interface though, as modern drivers are (and will be) much different from the legacy PIT. Whichever method is used, low level platforms can just switch them on/off in their enumeration/detection routines, while the generic code will have them enabled by default. Whichever method is used, we will face a problem how to determine PIT exists or not. When we enabled Bay Trail-T platform at the beginning, we were trying to make the code as generic as possible, and it works properly up to now. So we don't have a SUBARCH like X86_SUBARCH_INTEL_MID to use the platform specific functions. And for now I'm not quite sure it's a good idea to create one. If we make it as a flag driven, I don't know there is a flag in firmware better than ACPI HW reduced flag(Of course it's not good enough to cover all the cases). Or if we want to use platform info to turn on/off this flag, we'll have to maintain a platform list, which may be longer and more complicated than worth doing that. Well, it's not nearly so difficult, because you already have a platform flag: acpi_gbl_reduced_hardware. What I object against is to infest generic codepaths with unreadable, unrobust crap like: + if (acpi_gbl_reduced_hardware) { + pr_info(Using NULL legacy PIC\n); + legacy_pic = null_legacy_pic; + } else + legacy_pic-init(0); To solve that, add a small (early) init function (say 'x86_reduced_hw_init()') that sets up the right driver selections if acpi_gbl_reduced_hardware is set: - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' - clean up 'global_clock_event' handling: instead of a global variable, move its management into x86_platform_ops::get_clockevent() and set the method to hpet/pit/abp/etc. specific handlers that return the right clockevent device. - in your x86_reduced_hw_init() function add the hpet clockevent device to x86_platform_ops::get_clockevent, overriding the default PIT. - in x86_reduced_hw_init() set pm_power_off. - set 'reboot_type' and remove the acpi_gbl_reduced_hardware hack from efi_reboot_required(). I'll do more investigation above items but I want to leave at least these two as the quirk today unless I am convinced I can do that because from my understanding, UEFI runtime services should not be supported in reduced hw mode. etc. Just keep the generic init codepaths free of those random selections based on global flags, okay? Agree. Thanks, -Aubrey Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/5 4:11, Ingo Molnar wrote: * Arjan van de Ven ar...@linux.intel.com wrote: On 3/4/2015 1:50 AM, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. more like platform_has(X86_PLATFORM_PIT) etc, one for each legacy io item Precisely. The main problem is the generic, 'lumps everything together' nature of the acpi_gbl_reduced_hardware flag. (Like the big kernel lock lumped together all sorts of locking rules and semantics.) Properly split out, feature-ish or driver-ish interfaces for PIT and other legacy details are the proper approach to 'turn them off'. - x86_platform is a function pointer driven, driver-ish interface. - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish interface. Both are fine - for something as separate as the PIT (or the PIC) it might make more sense to go towards a 'driver' interface though, as modern drivers are (and will be) much different from the legacy PIT. Whichever method is used, low level platforms can just switch them on/off in their enumeration/detection routines, while the generic code will have them enabled by default. Whichever method is used, we will face a problem how to determine PIT exists or not. When we enabled Bay Trail-T platform at the beginning, we were trying to make the code as generic as possible, and it works properly up to now. So we don't have a SUBARCH like X86_SUBARCH_INTEL_MID to use the platform specific functions. And for now I'm not quite sure it's a good idea to create one. If we make it as a flag driven, I don't know there is a flag in firmware better than ACPI HW reduced flag(Of course it's not good enough to cover all the cases). Or if we want to use platform info to turn on/off this flag, we'll have to maintain a platform list, which may be longer and more complicated than worth doing that. Thanks, -Aubrey so we can clear it on hw reduced, but also in other cases. hw reduced is one way, but I'd be surprised if there weren't other ways (like quirks) where we'd want to do the same things Exactly. The key step is the proper, clean separation out of hardware interfaces. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/5 5:52, Rafael J. Wysocki wrote: On Wednesday, March 04, 2015 08:21:01 PM Alan Cox wrote: On Wed, 2015-03-04 at 15:05 +0100, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: Sort of. What we need is a do not touch PIC/PIT bit for the code that tries to fall back to them in some cases (which may appear to work if the hardware is physically there, but it may confuse the platform). Can some cases detection be nicely put into a x86_platform platform-specific method? In some cases they don't belong in x86, ACPI is also used for ARM64. However if ( has_8259_pic() ) is trivally 0, 1 or some platform or acpi provided method. And which is how that should have been implemented to start with IMO. Besides, the ACPI reduced hardware case is kind of a red herring here, because it most likely is not the only case when we'll want has_8259_pic() to return 0 (quite likely, we'll want that on all BayTrail-based systems, for example). BayTrail-based systems has BayTrail-I, BayTrail-M, BayTrail-D, BayTrail-T, BayTrail-T/CR. BayTrail-D is a desktop and BayTrail-M is a mobile/laptop and 8259 exists on both systems and I don't think we want to bypass it. ACPI reduced hardware is the best case in my mind unless you want to enumerate the platform one by one. can we make a global variable u8 has_8259; and initialize it by acpi reduced hardware flag? or a wrapper function? Thanks, -Aubrey -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey aubrey...@linux.intel.com wrote: On 2015/3/5 4:11, Ingo Molnar wrote: * Arjan van de Ven ar...@linux.intel.com wrote: On 3/4/2015 1:50 AM, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. more like platform_has(X86_PLATFORM_PIT) etc, one for each legacy io item Precisely. The main problem is the generic, 'lumps everything together' nature of the acpi_gbl_reduced_hardware flag. (Like the big kernel lock lumped together all sorts of locking rules and semantics.) Properly split out, feature-ish or driver-ish interfaces for PIT and other legacy details are the proper approach to 'turn them off'. - x86_platform is a function pointer driven, driver-ish interface. - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish interface. Both are fine - for something as separate as the PIT (or the PIC) it might make more sense to go towards a 'driver' interface though, as modern drivers are (and will be) much different from the legacy PIT. Whichever method is used, low level platforms can just switch them on/off in their enumeration/detection routines, while the generic code will have them enabled by default. Whichever method is used, we will face a problem how to determine PIT exists or not. When we enabled Bay Trail-T platform at the beginning, we were trying to make the code as generic as possible, and it works properly up to now. So we don't have a SUBARCH like X86_SUBARCH_INTEL_MID to use the platform specific functions. And for now I'm not quite sure it's a good idea to create one. If we make it as a flag driven, I don't know there is a flag in firmware better than ACPI HW reduced flag(Of course it's not good enough to cover all the cases). Or if we want to use platform info to turn on/off this flag, we'll have to maintain a platform list, which may be longer and more complicated than worth doing that. Well, it's not nearly so difficult, because you already have a platform flag: acpi_gbl_reduced_hardware. What I object against is to infest generic codepaths with unreadable, unrobust crap like: + if (acpi_gbl_reduced_hardware) { + pr_info(Using NULL legacy PIC\n); + legacy_pic = null_legacy_pic; + } else + legacy_pic-init(0); To solve that, add a small (early) init function (say 'x86_reduced_hw_init()') that sets up the right driver selections if acpi_gbl_reduced_hardware is set: - in x86_reduced_hw_init() set 'legacy_pic' to 'null_legacy_pic' - clean up 'global_clock_event' handling: instead of a global variable, move its management into x86_platform_ops::get_clockevent() and set the method to hpet/pit/abp/etc. specific handlers that return the right clockevent device. - in your x86_reduced_hw_init() function add the hpet clockevent device to x86_platform_ops::get_clockevent, overriding the default PIT. - in x86_reduced_hw_init() set pm_power_off. - set 'reboot_type' and remove the acpi_gbl_reduced_hardware hack from efi_reboot_required(). etc. Just keep the generic init codepaths free of those random selections based on global flags, okay? Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wednesday, March 04, 2015 08:21:01 PM Alan Cox wrote: > On Wed, 2015-03-04 at 15:05 +0100, Borislav Petkov wrote: > > On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: > > > Sort of. What we need is a "do not touch PIC/PIT" bit for the code that > > > tries to fall back to them in some cases (which may appear to work if > > > the hardware is physically there, but it may confuse the platform). > > > > Can "some cases" detection be nicely put into a x86_platform > > platform-specific method? > > In some cases they don't belong in x86, ACPI is also used for ARM64. > > However > > if ( has_8259_pic() ) > > is trivally 0, 1 or some platform or acpi provided method. And which is how that should have been implemented to start with IMO. Besides, the "ACPI reduced hardware" case is kind of a red herring here, because it most likely is not the only case when we'll want has_8259_pic() to return 0 (quite likely, we'll want that on all BayTrail-based systems, for example). -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wed, 2015-03-04 at 15:05 +0100, Borislav Petkov wrote: > On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: > > Sort of. What we need is a "do not touch PIC/PIT" bit for the code that > > tries to fall back to them in some cases (which may appear to work if > > the hardware is physically there, but it may confuse the platform). > > Can "some cases" detection be nicely put into a x86_platform > platform-specific method? In some cases they don't belong in x86, ACPI is also used for ARM64. However if ( has_8259_pic() ) is trivally 0, 1 or some platform or acpi provided method. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wed, 2015-03-04 at 10:50 +0100, Borislav Petkov wrote: > On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: > > > > > >Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code > > >is a mistake. > > > > ideally, the presence of that flag in the firmware table will clear/set > > more global settings, > > for example, having that flag should cause the 8042 input code to not probe > > for the 8042. > > > > for interrupts, there really ought to be a "apic first/only" mode, which is > > then used on > > all modern systems (not just hw reduced). > > Do we need some sort of platform-specific querying interfaces now too, > similar to cpu_has()? I.e., platform_has()... > > if (platform_has(X86_PLATFORM_REDUCED_HW)) > do stuff.. ACPI hw reduced is not an x86 specific concept so quite possibly yes. ACPI is the usual source for a variety of generic platform information such as absence and presence of prehistoric PC compatibility goo - increasingly so in fact. It tells you if the device is probably a tablet, if it is more efficient to idle via suspend/resume or by asking the cpu to idle. It tells you if low power modes are supported and so on. I don't think it makes sense to treat "ACPI reduced" as some kind of platform concept. You could easily get basically identical hardware that is or is not "hw reduced" depending upon firmware choices. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Arjan van de Ven wrote: > On 3/4/2015 1:50 AM, Borislav Petkov wrote: > >On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: > >>> > >>>Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code > >>>is a mistake. > >> > >>ideally, the presence of that flag in the firmware table will clear/set > >>more global settings, > >>for example, having that flag should cause the 8042 input code to not probe > >>for the 8042. > >> > >>for interrupts, there really ought to be a "apic first/only" mode, which is > >>then used on > >>all modern systems (not just hw reduced). > > > >Do we need some sort of platform-specific querying interfaces now too, > >similar to cpu_has()? I.e., platform_has()... > > > > if (platform_has(X86_PLATFORM_REDUCED_HW)) > > do stuff.. > > more like > > platform_has(X86_PLATFORM_PIT) > > etc, one for each legacy io item Precisely. The main problem is the generic, 'lumps everything together' nature of the acpi_gbl_reduced_hardware flag. (Like the big kernel lock lumped together all sorts of locking rules and semantics.) Properly split out, feature-ish or driver-ish interfaces for PIT and other legacy details are the proper approach to 'turn them off'. - x86_platform is a function pointer driven, driver-ish interface. - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish interface. Both are fine - for something as separate as the PIT (or the PIC) it might make more sense to go towards a 'driver' interface though, as modern drivers are (and will be) much different from the legacy PIT. Whichever method is used, low level platforms can just switch them on/off in their enumeration/detection routines, while the generic code will have them enabled by default. > so we can clear it on hw reduced, but also in other cases. hw > reduced is one way, but I'd be surprised if there weren't other ways > (like quirks) where we'd want to do the same things Exactly. The key step is the proper, clean separation out of hardware interfaces. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 3/4/2015 1:50 AM, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a "apic first/only" mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. more like platform_has(X86_PLATFORM_PIT) etc, one for each legacy io item so we can clear it on hw reduced, but also in other cases. hw reduced is one way, but I'd be surprised if there weren't other ways (like quirks) where we'd want to do the same things -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wednesday, March 04, 2015 03:05:55 PM Borislav Petkov wrote: > On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: > > Sort of. What we need is a "do not touch PIC/PIT" bit for the code that > > tries to fall back to them in some cases (which may appear to work if > > the hardware is physically there, but it may confuse the platform). > > Can "some cases" detection be nicely put into a x86_platform > platform-specific method? I guess so. The problem is that we just do that fallback unconditionally today which evidently doesn't always work. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: > Sort of. What we need is a "do not touch PIC/PIT" bit for the code that > tries to fall back to them in some cases (which may appear to work if > the hardware is physically there, but it may confuse the platform). Can "some cases" detection be nicely put into a x86_platform platform-specific method? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wednesday, March 04, 2015 10:50:11 AM Borislav Petkov wrote: > On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: > > > > > >Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code > > >is a mistake. > > > > ideally, the presence of that flag in the firmware table will clear/set > > more global settings, > > for example, having that flag should cause the 8042 input code to not probe > > for the 8042. > > > > for interrupts, there really ought to be a "apic first/only" mode, which is > > then used on > > all modern systems (not just hw reduced). > > Do we need some sort of platform-specific querying interfaces now too, > similar to cpu_has()? I.e., platform_has()... > > if (platform_has(X86_PLATFORM_REDUCED_HW)) > do stuff.. > > Hmmm. Sort of. What we need is a "do not touch PIC/PIT" bit for the code that tries to fall back to them in some cases (which may appear to work if the hardware is physically there, but it may confuse the platform). -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: > > > >Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code > >is a mistake. > > ideally, the presence of that flag in the firmware table will clear/set more > global settings, > for example, having that flag should cause the 8042 input code to not probe > for the 8042. > > for interrupts, there really ought to be a "apic first/only" mode, which is > then used on > all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. Hmmm. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a "apic first/only" mode, which is then used on all modern systems (not just hw reduced). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wednesday, March 04, 2015 08:21:01 PM Alan Cox wrote: On Wed, 2015-03-04 at 15:05 +0100, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: Sort of. What we need is a do not touch PIC/PIT bit for the code that tries to fall back to them in some cases (which may appear to work if the hardware is physically there, but it may confuse the platform). Can some cases detection be nicely put into a x86_platform platform-specific method? In some cases they don't belong in x86, ACPI is also used for ARM64. However if ( has_8259_pic() ) is trivally 0, 1 or some platform or acpi provided method. And which is how that should have been implemented to start with IMO. Besides, the ACPI reduced hardware case is kind of a red herring here, because it most likely is not the only case when we'll want has_8259_pic() to return 0 (quite likely, we'll want that on all BayTrail-based systems, for example). -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wed, 2015-03-04 at 15:05 +0100, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: Sort of. What we need is a do not touch PIC/PIT bit for the code that tries to fall back to them in some cases (which may appear to work if the hardware is physically there, but it may confuse the platform). Can some cases detection be nicely put into a x86_platform platform-specific method? In some cases they don't belong in x86, ACPI is also used for ARM64. However if ( has_8259_pic() ) is trivally 0, 1 or some platform or acpi provided method. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Arjan van de Ven ar...@linux.intel.com wrote: On 3/4/2015 1:50 AM, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. more like platform_has(X86_PLATFORM_PIT) etc, one for each legacy io item Precisely. The main problem is the generic, 'lumps everything together' nature of the acpi_gbl_reduced_hardware flag. (Like the big kernel lock lumped together all sorts of locking rules and semantics.) Properly split out, feature-ish or driver-ish interfaces for PIT and other legacy details are the proper approach to 'turn them off'. - x86_platform is a function pointer driven, driver-ish interface. - platform_has(X86_PLATFORM_IT) is a flag driven, feature-flag-ish interface. Both are fine - for something as separate as the PIT (or the PIC) it might make more sense to go towards a 'driver' interface though, as modern drivers are (and will be) much different from the legacy PIT. Whichever method is used, low level platforms can just switch them on/off in their enumeration/detection routines, while the generic code will have them enabled by default. so we can clear it on hw reduced, but also in other cases. hw reduced is one way, but I'd be surprised if there weren't other ways (like quirks) where we'd want to do the same things Exactly. The key step is the proper, clean separation out of hardware interfaces. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wed, 2015-03-04 at 10:50 +0100, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. ACPI hw reduced is not an x86 specific concept so quite possibly yes. ACPI is the usual source for a variety of generic platform information such as absence and presence of prehistoric PC compatibility goo - increasingly so in fact. It tells you if the device is probably a tablet, if it is more efficient to idle via suspend/resume or by asking the cpu to idle. It tells you if low power modes are supported and so on. I don't think it makes sense to treat ACPI reduced as some kind of platform concept. You could easily get basically identical hardware that is or is not hw reduced depending upon firmware choices. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. Hmmm. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 3/4/2015 1:50 AM, Borislav Petkov wrote: On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. more like platform_has(X86_PLATFORM_PIT) etc, one for each legacy io item so we can clear it on hw reduced, but also in other cases. hw reduced is one way, but I'd be surprised if there weren't other ways (like quirks) where we'd want to do the same things -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wednesday, March 04, 2015 10:50:11 AM Borislav Petkov wrote: On Wed, Mar 04, 2015 at 12:43:08AM -0800, Arjan van de Ven wrote: Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. ideally, the presence of that flag in the firmware table will clear/set more global settings, for example, having that flag should cause the 8042 input code to not probe for the 8042. for interrupts, there really ought to be a apic first/only mode, which is then used on all modern systems (not just hw reduced). Do we need some sort of platform-specific querying interfaces now too, similar to cpu_has()? I.e., platform_has()... if (platform_has(X86_PLATFORM_REDUCED_HW)) do stuff.. Hmmm. Sort of. What we need is a do not touch PIC/PIT bit for the code that tries to fall back to them in some cases (which may appear to work if the hardware is physically there, but it may confuse the platform). -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: Sort of. What we need is a do not touch PIC/PIT bit for the code that tries to fall back to them in some cases (which may appear to work if the hardware is physically there, but it may confuse the platform). Can some cases detection be nicely put into a x86_platform platform-specific method? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On Wednesday, March 04, 2015 03:05:55 PM Borislav Petkov wrote: On Wed, Mar 04, 2015 at 03:16:07PM +0100, Rafael J. Wysocki wrote: Sort of. What we need is a do not touch PIC/PIT bit for the code that tries to fall back to them in some cases (which may appear to work if the hardware is physically there, but it may confuse the platform). Can some cases detection be nicely put into a x86_platform platform-specific method? I guess so. The problem is that we just do that fallback unconditionally today which evidently doesn't always work. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey wrote: > On 2015/3/4 13:31, Ingo Molnar wrote: > > > > * Li, Aubrey wrote: > > > >> On 2015/3/4 13:08, Ingo Molnar wrote: > >>> > >>> * Li, Aubrey wrote: > >>> > On ACPI hardware reduced platform, the legacy PIC and PIT may not be > initialized even though they may be present in silicon. Touching > these legacy components causes unexpected result on system. > > On Bay Trail-T(ASUS-T100) platform, touching these legacy components > blocks platform hardware low idle power state(S0ix) during system > suspend. > So we should bypass them on ACPI hardware reduced platform. > > Suggested-by: Arjan van de Ven > Signed-off-by: Li Aubrey > Cc: Len Brown > Cc: Rafael J. Wysocki > --- > arch/x86/kernel/irqinit.c | 6 +- > arch/x86/kernel/time.c| 3 ++- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c > index 70e181e..9a64cc3 100644 > --- a/arch/x86/kernel/irqinit.c > +++ b/arch/x86/kernel/irqinit.c > @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) > #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) > init_bsp_APIC(); > #endif > -legacy_pic->init(0); > +if (acpi_gbl_reduced_hardware) { > +pr_info("Using NULL legacy PIC\n"); > +legacy_pic = _legacy_pic; > +} else > +legacy_pic->init(0); > > for (i = 0; i < nr_legacy_irqs(); i++) > irq_set_chip_and_handler(i, chip, handle_level_irq); > diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c > index 25adc0e..5ba94fa 100644 > --- a/arch/x86/kernel/time.c > +++ b/arch/x86/kernel/time.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) > /* Default timer init function */ > void __init hpet_time_init(void) > { > -if (!hpet_enable()) > +if (!hpet_enable() && !acpi_gbl_reduced_hardware) > setup_pit_timer(); > setup_default_timer_irq(); > } > >>> > >>> So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes > >>> various hardware drivers that have little relation to each other... > >>> > >>> Instead of having a proper platform init this flag hooks into various > >>> drivers and generic code, such as the efi reboot and shutdown code, > >>> and now the generic irq init code. > >>> > >>> For this IRQ init problem, why not add a proper callback to > >>> x86_platform_ops, define your own IRQ init function, initialize it in > >>> your platform init sequence and let it be called? That solves it > >>> without creating an ugly mix of different platform methods. > >>> > >>> For the EFI shutdown case, what's wrong with setting your own > >>> pm_power_off handler like most of the other platforms are doing? Plus > >>> the EFI code in drivers/firmware/efi/reboot.c should probably only set > >>> the shutdown handler if pm_power_off is still NULL. > >> > >> I think our goal is to make the code as generic as possible for all > >> x86 platform, rather than creating a new x86 branch, I added Alan > >> Cox for this strategy discussion. > >> > >> Do you have any inputs for the patch itself? > > > > Other than that the patch is unacceptable for an upstream merge in its > > current form for the reason I mentioned? No. > > So you are suggesting we extend a new x86 platform branch and > override the x86_platform and pm_power_off and reboot, like what > intel_mid does? Well, what I suggested above was to add an IRQ init method to x86_platform (and make use of it on your platform), and to use the existing pm_power_off method for the reboot quirk. Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/4 13:31, Ingo Molnar wrote: > > * Li, Aubrey wrote: > >> On 2015/3/4 13:08, Ingo Molnar wrote: >>> >>> * Li, Aubrey wrote: >>> On ACPI hardware reduced platform, the legacy PIC and PIT may not be initialized even though they may be present in silicon. Touching these legacy components causes unexpected result on system. On Bay Trail-T(ASUS-T100) platform, touching these legacy components blocks platform hardware low idle power state(S0ix) during system suspend. So we should bypass them on ACPI hardware reduced platform. Suggested-by: Arjan van de Ven Signed-off-by: Li Aubrey Cc: Len Brown Cc: Rafael J. Wysocki --- arch/x86/kernel/irqinit.c | 6 +- arch/x86/kernel/time.c| 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c index 70e181e..9a64cc3 100644 --- a/arch/x86/kernel/irqinit.c +++ b/arch/x86/kernel/irqinit.c @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) init_bsp_APIC(); #endif - legacy_pic->init(0); + if (acpi_gbl_reduced_hardware) { + pr_info("Using NULL legacy PIC\n"); + legacy_pic = _legacy_pic; + } else + legacy_pic->init(0); for (i = 0; i < nr_legacy_irqs(); i++) irq_set_chip_and_handler(i, chip, handle_level_irq); diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c index 25adc0e..5ba94fa 100644 --- a/arch/x86/kernel/time.c +++ b/arch/x86/kernel/time.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) /* Default timer init function */ void __init hpet_time_init(void) { - if (!hpet_enable()) + if (!hpet_enable() && !acpi_gbl_reduced_hardware) setup_pit_timer(); setup_default_timer_irq(); } >>> >>> So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes >>> various hardware drivers that have little relation to each other... >>> >>> Instead of having a proper platform init this flag hooks into various >>> drivers and generic code, such as the efi reboot and shutdown code, >>> and now the generic irq init code. >>> >>> For this IRQ init problem, why not add a proper callback to >>> x86_platform_ops, define your own IRQ init function, initialize it in >>> your platform init sequence and let it be called? That solves it >>> without creating an ugly mix of different platform methods. >>> >>> For the EFI shutdown case, what's wrong with setting your own >>> pm_power_off handler like most of the other platforms are doing? Plus >>> the EFI code in drivers/firmware/efi/reboot.c should probably only set >>> the shutdown handler if pm_power_off is still NULL. >> >> I think our goal is to make the code as generic as possible for all >> x86 platform, rather than creating a new x86 branch, I added Alan >> Cox for this strategy discussion. >> >> Do you have any inputs for the patch itself? > > Other than that the patch is unacceptable for an upstream merge in its > current form for the reason I mentioned? No. So you are suggesting we extend a new x86 platform branch and override the x86_platform and pm_power_off and reboot, like what intel_mid does? Thanks, -Aubrey > > Thanks, > > Ingo > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey wrote: > On 2015/3/4 13:08, Ingo Molnar wrote: > > > > * Li, Aubrey wrote: > > > >> On ACPI hardware reduced platform, the legacy PIC and PIT may not be > >> initialized even though they may be present in silicon. Touching > >> these legacy components causes unexpected result on system. > >> > >> On Bay Trail-T(ASUS-T100) platform, touching these legacy components > >> blocks platform hardware low idle power state(S0ix) during system suspend. > >> So we should bypass them on ACPI hardware reduced platform. > >> > >> Suggested-by: Arjan van de Ven > >> Signed-off-by: Li Aubrey > >> Cc: Len Brown > >> Cc: Rafael J. Wysocki > >> --- > >> arch/x86/kernel/irqinit.c | 6 +- > >> arch/x86/kernel/time.c| 3 ++- > >> 2 files changed, 7 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c > >> index 70e181e..9a64cc3 100644 > >> --- a/arch/x86/kernel/irqinit.c > >> +++ b/arch/x86/kernel/irqinit.c > >> @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) > >> #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) > >>init_bsp_APIC(); > >> #endif > >> - legacy_pic->init(0); > >> + if (acpi_gbl_reduced_hardware) { > >> + pr_info("Using NULL legacy PIC\n"); > >> + legacy_pic = _legacy_pic; > >> + } else > >> + legacy_pic->init(0); > >> > >>for (i = 0; i < nr_legacy_irqs(); i++) > >>irq_set_chip_and_handler(i, chip, handle_level_irq); > >> diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c > >> index 25adc0e..5ba94fa 100644 > >> --- a/arch/x86/kernel/time.c > >> +++ b/arch/x86/kernel/time.c > >> @@ -14,6 +14,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> > >> #include > >> #include > >> @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) > >> /* Default timer init function */ > >> void __init hpet_time_init(void) > >> { > >> - if (!hpet_enable()) > >> + if (!hpet_enable() && !acpi_gbl_reduced_hardware) > >>setup_pit_timer(); > >>setup_default_timer_irq(); > >> } > > > > So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes > > various hardware drivers that have little relation to each other... > > > > Instead of having a proper platform init this flag hooks into various > > drivers and generic code, such as the efi reboot and shutdown code, > > and now the generic irq init code. > > > > For this IRQ init problem, why not add a proper callback to > > x86_platform_ops, define your own IRQ init function, initialize it in > > your platform init sequence and let it be called? That solves it > > without creating an ugly mix of different platform methods. > > > > For the EFI shutdown case, what's wrong with setting your own > > pm_power_off handler like most of the other platforms are doing? Plus > > the EFI code in drivers/firmware/efi/reboot.c should probably only set > > the shutdown handler if pm_power_off is still NULL. > > I think our goal is to make the code as generic as possible for all > x86 platform, rather than creating a new x86 branch, I added Alan > Cox for this strategy discussion. > > Do you have any inputs for the patch itself? Other than that the patch is unacceptable for an upstream merge in its current form for the reason I mentioned? No. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/4 13:08, Ingo Molnar wrote: > > * Li, Aubrey wrote: > >> On ACPI hardware reduced platform, the legacy PIC and PIT may not be >> initialized even though they may be present in silicon. Touching >> these legacy components causes unexpected result on system. >> >> On Bay Trail-T(ASUS-T100) platform, touching these legacy components >> blocks platform hardware low idle power state(S0ix) during system suspend. >> So we should bypass them on ACPI hardware reduced platform. >> >> Suggested-by: Arjan van de Ven >> Signed-off-by: Li Aubrey >> Cc: Len Brown >> Cc: Rafael J. Wysocki >> --- >> arch/x86/kernel/irqinit.c | 6 +- >> arch/x86/kernel/time.c| 3 ++- >> 2 files changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c >> index 70e181e..9a64cc3 100644 >> --- a/arch/x86/kernel/irqinit.c >> +++ b/arch/x86/kernel/irqinit.c >> @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) >> #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) >> init_bsp_APIC(); >> #endif >> -legacy_pic->init(0); >> +if (acpi_gbl_reduced_hardware) { >> +pr_info("Using NULL legacy PIC\n"); >> +legacy_pic = _legacy_pic; >> +} else >> +legacy_pic->init(0); >> >> for (i = 0; i < nr_legacy_irqs(); i++) >> irq_set_chip_and_handler(i, chip, handle_level_irq); >> diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c >> index 25adc0e..5ba94fa 100644 >> --- a/arch/x86/kernel/time.c >> +++ b/arch/x86/kernel/time.c >> @@ -14,6 +14,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> #include >> @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) >> /* Default timer init function */ >> void __init hpet_time_init(void) >> { >> -if (!hpet_enable()) >> +if (!hpet_enable() && !acpi_gbl_reduced_hardware) >> setup_pit_timer(); >> setup_default_timer_irq(); >> } > > So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes > various hardware drivers that have little relation to each other... > > Instead of having a proper platform init this flag hooks into various > drivers and generic code, such as the efi reboot and shutdown code, > and now the generic irq init code. > > For this IRQ init problem, why not add a proper callback to > x86_platform_ops, define your own IRQ init function, initialize it in > your platform init sequence and let it be called? That solves it > without creating an ugly mix of different platform methods. > > For the EFI shutdown case, what's wrong with setting your own > pm_power_off handler like most of the other platforms are doing? Plus > the EFI code in drivers/firmware/efi/reboot.c should probably only set > the shutdown handler if pm_power_off is still NULL. I think our goal is to make the code as generic as possible for all x86 platform, rather than creating a new x86 branch, I added Alan Cox for this strategy discussion. Do you have any inputs for the patch itself? Thanks, -Aubrey > > Thanks, > > Ingo > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey wrote: > On ACPI hardware reduced platform, the legacy PIC and PIT may not be > initialized even though they may be present in silicon. Touching > these legacy components causes unexpected result on system. > > On Bay Trail-T(ASUS-T100) platform, touching these legacy components > blocks platform hardware low idle power state(S0ix) during system suspend. > So we should bypass them on ACPI hardware reduced platform. > > Suggested-by: Arjan van de Ven > Signed-off-by: Li Aubrey > Cc: Len Brown > Cc: Rafael J. Wysocki > --- > arch/x86/kernel/irqinit.c | 6 +- > arch/x86/kernel/time.c| 3 ++- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c > index 70e181e..9a64cc3 100644 > --- a/arch/x86/kernel/irqinit.c > +++ b/arch/x86/kernel/irqinit.c > @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) > #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) > init_bsp_APIC(); > #endif > - legacy_pic->init(0); > + if (acpi_gbl_reduced_hardware) { > + pr_info("Using NULL legacy PIC\n"); > + legacy_pic = _legacy_pic; > + } else > + legacy_pic->init(0); > > for (i = 0; i < nr_legacy_irqs(); i++) > irq_set_chip_and_handler(i, chip, handle_level_irq); > diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c > index 25adc0e..5ba94fa 100644 > --- a/arch/x86/kernel/time.c > +++ b/arch/x86/kernel/time.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) > /* Default timer init function */ > void __init hpet_time_init(void) > { > - if (!hpet_enable()) > + if (!hpet_enable() && !acpi_gbl_reduced_hardware) > setup_pit_timer(); > setup_default_timer_irq(); > } So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes various hardware drivers that have little relation to each other... Instead of having a proper platform init this flag hooks into various drivers and generic code, such as the efi reboot and shutdown code, and now the generic irq init code. For this IRQ init problem, why not add a proper callback to x86_platform_ops, define your own IRQ init function, initialize it in your platform init sequence and let it be called? That solves it without creating an ugly mix of different platform methods. For the EFI shutdown case, what's wrong with setting your own pm_power_off handler like most of the other platforms are doing? Plus the EFI code in drivers/firmware/efi/reboot.c should probably only set the shutdown handler if pm_power_off is still NULL. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey aubrey...@linux.intel.com wrote: On 2015/3/4 13:31, Ingo Molnar wrote: * Li, Aubrey aubrey...@linux.intel.com wrote: On 2015/3/4 13:08, Ingo Molnar wrote: * Li, Aubrey aubrey...@linux.intel.com wrote: On ACPI hardware reduced platform, the legacy PIC and PIT may not be initialized even though they may be present in silicon. Touching these legacy components causes unexpected result on system. On Bay Trail-T(ASUS-T100) platform, touching these legacy components blocks platform hardware low idle power state(S0ix) during system suspend. So we should bypass them on ACPI hardware reduced platform. Suggested-by: Arjan van de Ven ar...@linux.intel.com Signed-off-by: Li Aubrey aubrey...@linux.intel.com Cc: Len Brown len.br...@intel.com Cc: Rafael J. Wysocki rafael.j.wyso...@intel.com --- arch/x86/kernel/irqinit.c | 6 +- arch/x86/kernel/time.c| 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c index 70e181e..9a64cc3 100644 --- a/arch/x86/kernel/irqinit.c +++ b/arch/x86/kernel/irqinit.c @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) init_bsp_APIC(); #endif -legacy_pic-init(0); +if (acpi_gbl_reduced_hardware) { +pr_info(Using NULL legacy PIC\n); +legacy_pic = null_legacy_pic; +} else +legacy_pic-init(0); for (i = 0; i nr_legacy_irqs(); i++) irq_set_chip_and_handler(i, chip, handle_level_irq); diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c index 25adc0e..5ba94fa 100644 --- a/arch/x86/kernel/time.c +++ b/arch/x86/kernel/time.c @@ -14,6 +14,7 @@ #include linux/i8253.h #include linux/time.h #include linux/export.h +#include linux/acpi.h #include asm/vsyscall.h #include asm/x86_init.h @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) /* Default timer init function */ void __init hpet_time_init(void) { -if (!hpet_enable()) +if (!hpet_enable() !acpi_gbl_reduced_hardware) setup_pit_timer(); setup_default_timer_irq(); } So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes various hardware drivers that have little relation to each other... Instead of having a proper platform init this flag hooks into various drivers and generic code, such as the efi reboot and shutdown code, and now the generic irq init code. For this IRQ init problem, why not add a proper callback to x86_platform_ops, define your own IRQ init function, initialize it in your platform init sequence and let it be called? That solves it without creating an ugly mix of different platform methods. For the EFI shutdown case, what's wrong with setting your own pm_power_off handler like most of the other platforms are doing? Plus the EFI code in drivers/firmware/efi/reboot.c should probably only set the shutdown handler if pm_power_off is still NULL. I think our goal is to make the code as generic as possible for all x86 platform, rather than creating a new x86 branch, I added Alan Cox for this strategy discussion. Do you have any inputs for the patch itself? Other than that the patch is unacceptable for an upstream merge in its current form for the reason I mentioned? No. So you are suggesting we extend a new x86 platform branch and override the x86_platform and pm_power_off and reboot, like what intel_mid does? Well, what I suggested above was to add an IRQ init method to x86_platform (and make use of it on your platform), and to use the existing pm_power_off method for the reboot quirk. Using 'acpi_gbl_reduced_hardware' flag outside the ACPI code is a mistake. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/4 13:31, Ingo Molnar wrote: * Li, Aubrey aubrey...@linux.intel.com wrote: On 2015/3/4 13:08, Ingo Molnar wrote: * Li, Aubrey aubrey...@linux.intel.com wrote: On ACPI hardware reduced platform, the legacy PIC and PIT may not be initialized even though they may be present in silicon. Touching these legacy components causes unexpected result on system. On Bay Trail-T(ASUS-T100) platform, touching these legacy components blocks platform hardware low idle power state(S0ix) during system suspend. So we should bypass them on ACPI hardware reduced platform. Suggested-by: Arjan van de Ven ar...@linux.intel.com Signed-off-by: Li Aubrey aubrey...@linux.intel.com Cc: Len Brown len.br...@intel.com Cc: Rafael J. Wysocki rafael.j.wyso...@intel.com --- arch/x86/kernel/irqinit.c | 6 +- arch/x86/kernel/time.c| 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c index 70e181e..9a64cc3 100644 --- a/arch/x86/kernel/irqinit.c +++ b/arch/x86/kernel/irqinit.c @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) init_bsp_APIC(); #endif - legacy_pic-init(0); + if (acpi_gbl_reduced_hardware) { + pr_info(Using NULL legacy PIC\n); + legacy_pic = null_legacy_pic; + } else + legacy_pic-init(0); for (i = 0; i nr_legacy_irqs(); i++) irq_set_chip_and_handler(i, chip, handle_level_irq); diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c index 25adc0e..5ba94fa 100644 --- a/arch/x86/kernel/time.c +++ b/arch/x86/kernel/time.c @@ -14,6 +14,7 @@ #include linux/i8253.h #include linux/time.h #include linux/export.h +#include linux/acpi.h #include asm/vsyscall.h #include asm/x86_init.h @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) /* Default timer init function */ void __init hpet_time_init(void) { - if (!hpet_enable()) + if (!hpet_enable() !acpi_gbl_reduced_hardware) setup_pit_timer(); setup_default_timer_irq(); } So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes various hardware drivers that have little relation to each other... Instead of having a proper platform init this flag hooks into various drivers and generic code, such as the efi reboot and shutdown code, and now the generic irq init code. For this IRQ init problem, why not add a proper callback to x86_platform_ops, define your own IRQ init function, initialize it in your platform init sequence and let it be called? That solves it without creating an ugly mix of different platform methods. For the EFI shutdown case, what's wrong with setting your own pm_power_off handler like most of the other platforms are doing? Plus the EFI code in drivers/firmware/efi/reboot.c should probably only set the shutdown handler if pm_power_off is still NULL. I think our goal is to make the code as generic as possible for all x86 platform, rather than creating a new x86 branch, I added Alan Cox for this strategy discussion. Do you have any inputs for the patch itself? Other than that the patch is unacceptable for an upstream merge in its current form for the reason I mentioned? No. So you are suggesting we extend a new x86 platform branch and override the x86_platform and pm_power_off and reboot, like what intel_mid does? Thanks, -Aubrey Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
On 2015/3/4 13:08, Ingo Molnar wrote: * Li, Aubrey aubrey...@linux.intel.com wrote: On ACPI hardware reduced platform, the legacy PIC and PIT may not be initialized even though they may be present in silicon. Touching these legacy components causes unexpected result on system. On Bay Trail-T(ASUS-T100) platform, touching these legacy components blocks platform hardware low idle power state(S0ix) during system suspend. So we should bypass them on ACPI hardware reduced platform. Suggested-by: Arjan van de Ven ar...@linux.intel.com Signed-off-by: Li Aubrey aubrey...@linux.intel.com Cc: Len Brown len.br...@intel.com Cc: Rafael J. Wysocki rafael.j.wyso...@intel.com --- arch/x86/kernel/irqinit.c | 6 +- arch/x86/kernel/time.c| 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c index 70e181e..9a64cc3 100644 --- a/arch/x86/kernel/irqinit.c +++ b/arch/x86/kernel/irqinit.c @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) init_bsp_APIC(); #endif -legacy_pic-init(0); +if (acpi_gbl_reduced_hardware) { +pr_info(Using NULL legacy PIC\n); +legacy_pic = null_legacy_pic; +} else +legacy_pic-init(0); for (i = 0; i nr_legacy_irqs(); i++) irq_set_chip_and_handler(i, chip, handle_level_irq); diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c index 25adc0e..5ba94fa 100644 --- a/arch/x86/kernel/time.c +++ b/arch/x86/kernel/time.c @@ -14,6 +14,7 @@ #include linux/i8253.h #include linux/time.h #include linux/export.h +#include linux/acpi.h #include asm/vsyscall.h #include asm/x86_init.h @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) /* Default timer init function */ void __init hpet_time_init(void) { -if (!hpet_enable()) +if (!hpet_enable() !acpi_gbl_reduced_hardware) setup_pit_timer(); setup_default_timer_irq(); } So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes various hardware drivers that have little relation to each other... Instead of having a proper platform init this flag hooks into various drivers and generic code, such as the efi reboot and shutdown code, and now the generic irq init code. For this IRQ init problem, why not add a proper callback to x86_platform_ops, define your own IRQ init function, initialize it in your platform init sequence and let it be called? That solves it without creating an ugly mix of different platform methods. For the EFI shutdown case, what's wrong with setting your own pm_power_off handler like most of the other platforms are doing? Plus the EFI code in drivers/firmware/efi/reboot.c should probably only set the shutdown handler if pm_power_off is still NULL. I think our goal is to make the code as generic as possible for all x86 platform, rather than creating a new x86 branch, I added Alan Cox for this strategy discussion. Do you have any inputs for the patch itself? Thanks, -Aubrey Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey aubrey...@linux.intel.com wrote: On ACPI hardware reduced platform, the legacy PIC and PIT may not be initialized even though they may be present in silicon. Touching these legacy components causes unexpected result on system. On Bay Trail-T(ASUS-T100) platform, touching these legacy components blocks platform hardware low idle power state(S0ix) during system suspend. So we should bypass them on ACPI hardware reduced platform. Suggested-by: Arjan van de Ven ar...@linux.intel.com Signed-off-by: Li Aubrey aubrey...@linux.intel.com Cc: Len Brown len.br...@intel.com Cc: Rafael J. Wysocki rafael.j.wyso...@intel.com --- arch/x86/kernel/irqinit.c | 6 +- arch/x86/kernel/time.c| 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c index 70e181e..9a64cc3 100644 --- a/arch/x86/kernel/irqinit.c +++ b/arch/x86/kernel/irqinit.c @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) init_bsp_APIC(); #endif - legacy_pic-init(0); + if (acpi_gbl_reduced_hardware) { + pr_info(Using NULL legacy PIC\n); + legacy_pic = null_legacy_pic; + } else + legacy_pic-init(0); for (i = 0; i nr_legacy_irqs(); i++) irq_set_chip_and_handler(i, chip, handle_level_irq); diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c index 25adc0e..5ba94fa 100644 --- a/arch/x86/kernel/time.c +++ b/arch/x86/kernel/time.c @@ -14,6 +14,7 @@ #include linux/i8253.h #include linux/time.h #include linux/export.h +#include linux/acpi.h #include asm/vsyscall.h #include asm/x86_init.h @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) /* Default timer init function */ void __init hpet_time_init(void) { - if (!hpet_enable()) + if (!hpet_enable() !acpi_gbl_reduced_hardware) setup_pit_timer(); setup_default_timer_irq(); } So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes various hardware drivers that have little relation to each other... Instead of having a proper platform init this flag hooks into various drivers and generic code, such as the efi reboot and shutdown code, and now the generic irq init code. For this IRQ init problem, why not add a proper callback to x86_platform_ops, define your own IRQ init function, initialize it in your platform init sequence and let it be called? That solves it without creating an ugly mix of different platform methods. For the EFI shutdown case, what's wrong with setting your own pm_power_off handler like most of the other platforms are doing? Plus the EFI code in drivers/firmware/efi/reboot.c should probably only set the shutdown handler if pm_power_off is still NULL. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
* Li, Aubrey aubrey...@linux.intel.com wrote: On 2015/3/4 13:08, Ingo Molnar wrote: * Li, Aubrey aubrey...@linux.intel.com wrote: On ACPI hardware reduced platform, the legacy PIC and PIT may not be initialized even though they may be present in silicon. Touching these legacy components causes unexpected result on system. On Bay Trail-T(ASUS-T100) platform, touching these legacy components blocks platform hardware low idle power state(S0ix) during system suspend. So we should bypass them on ACPI hardware reduced platform. Suggested-by: Arjan van de Ven ar...@linux.intel.com Signed-off-by: Li Aubrey aubrey...@linux.intel.com Cc: Len Brown len.br...@intel.com Cc: Rafael J. Wysocki rafael.j.wyso...@intel.com --- arch/x86/kernel/irqinit.c | 6 +- arch/x86/kernel/time.c| 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c index 70e181e..9a64cc3 100644 --- a/arch/x86/kernel/irqinit.c +++ b/arch/x86/kernel/irqinit.c @@ -75,7 +75,11 @@ void __init init_ISA_irqs(void) #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC) init_bsp_APIC(); #endif - legacy_pic-init(0); + if (acpi_gbl_reduced_hardware) { + pr_info(Using NULL legacy PIC\n); + legacy_pic = null_legacy_pic; + } else + legacy_pic-init(0); for (i = 0; i nr_legacy_irqs(); i++) irq_set_chip_and_handler(i, chip, handle_level_irq); diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c index 25adc0e..5ba94fa 100644 --- a/arch/x86/kernel/time.c +++ b/arch/x86/kernel/time.c @@ -14,6 +14,7 @@ #include linux/i8253.h #include linux/time.h #include linux/export.h +#include linux/acpi.h #include asm/vsyscall.h #include asm/x86_init.h @@ -76,7 +77,7 @@ void __init setup_default_timer_irq(void) /* Default timer init function */ void __init hpet_time_init(void) { - if (!hpet_enable()) + if (!hpet_enable() !acpi_gbl_reduced_hardware) setup_pit_timer(); setup_default_timer_irq(); } So the whole acpi_gbl_reduced_hardware flaggery sucks as it mixes various hardware drivers that have little relation to each other... Instead of having a proper platform init this flag hooks into various drivers and generic code, such as the efi reboot and shutdown code, and now the generic irq init code. For this IRQ init problem, why not add a proper callback to x86_platform_ops, define your own IRQ init function, initialize it in your platform init sequence and let it be called? That solves it without creating an ugly mix of different platform methods. For the EFI shutdown case, what's wrong with setting your own pm_power_off handler like most of the other platforms are doing? Plus the EFI code in drivers/firmware/efi/reboot.c should probably only set the shutdown handler if pm_power_off is still NULL. I think our goal is to make the code as generic as possible for all x86 platform, rather than creating a new x86 branch, I added Alan Cox for this strategy discussion. Do you have any inputs for the patch itself? Other than that the patch is unacceptable for an upstream merge in its current form for the reason I mentioned? No. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/