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

[Li, Aubrey]

On 2015/3/10 16:06, Ingo Molnar wrote:
> 
> * Li, Aubrey <aubrey.li@xxxxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/