Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform

2015-03-10 Thread Li, Aubrey
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

2015-03-10 Thread Ingo Molnar

* 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

2015-03-10 Thread Li, Aubrey
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

2015-03-10 Thread Ingo Molnar

* 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

2015-03-09 Thread Li, Aubrey
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

2015-03-09 Thread Li, Aubrey
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

2015-03-05 Thread Alan Cox
> 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

2015-03-05 Thread Alan Cox
> 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

2015-03-05 Thread Li, Aubrey
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

2015-03-05 Thread Ingo Molnar

* 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

2015-03-05 Thread Li, Aubrey
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

2015-03-05 Thread Li, Aubrey
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

2015-03-05 Thread Alan Cox
 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

2015-03-05 Thread Alan Cox
 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

2015-03-05 Thread Li, Aubrey
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

2015-03-05 Thread Li, Aubrey
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

2015-03-05 Thread Li, Aubrey
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

2015-03-05 Thread Ingo Molnar

* 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

2015-03-04 Thread Rafael J. Wysocki
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

2015-03-04 Thread Alan Cox
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

2015-03-04 Thread Alan Cox
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

2015-03-04 Thread Ingo Molnar

* 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

2015-03-04 Thread Arjan van de Ven

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

2015-03-04 Thread Rafael J. Wysocki
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

2015-03-04 Thread Borislav Petkov
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

2015-03-04 Thread Rafael J. Wysocki
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

2015-03-04 Thread Borislav Petkov
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

2015-03-04 Thread Arjan van de Ven


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

2015-03-04 Thread Rafael J. Wysocki
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

2015-03-04 Thread Alan Cox
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

2015-03-04 Thread Ingo Molnar

* 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

2015-03-04 Thread Alan Cox
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

2015-03-04 Thread Arjan van de Ven


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

2015-03-04 Thread Borislav Petkov
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

2015-03-04 Thread Arjan van de Ven

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

2015-03-04 Thread Rafael J. Wysocki
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

2015-03-04 Thread Borislav Petkov
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

2015-03-04 Thread Rafael J. Wysocki
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

2015-03-03 Thread Ingo Molnar

* 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

2015-03-03 Thread Li, Aubrey
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

2015-03-03 Thread Ingo Molnar

* 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

2015-03-03 Thread Li, Aubrey
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

2015-03-03 Thread Ingo Molnar

* 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

2015-03-03 Thread Ingo Molnar

* 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

2015-03-03 Thread Li, Aubrey
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

2015-03-03 Thread Li, Aubrey
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

2015-03-03 Thread Ingo Molnar

* 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

2015-03-03 Thread Ingo Molnar

* 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/