LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Li, Aubrey" <aubrey.li@linux.intel.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>,
	Borislav Petkov <bp@alien8.de>,
	"alan@linux.intel.com" <alan@linux.intel.com>,
	"H. Peter Anvin" <hpa@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len.Brown@intel.com, x86@kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform
Date: Wed, 11 Mar 2015 12:14:49 +0800	[thread overview]
Message-ID: <54FFC139.7070603@linux.intel.com> (raw)
In-Reply-To: <20150310080628.GA20314@gmail.com>

On 2015/3/10 16:06, Ingo Molnar wrote:
> 
> * Li, Aubrey <aubrey.li@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

  reply	other threads:[~2015-03-11  4:14 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-04  3:23 Li, Aubrey
2015-03-04  5:08 ` Ingo Molnar
2015-03-04  5:26   ` Li, Aubrey
2015-03-04  5:31     ` Ingo Molnar
2015-03-04  6:04       ` Li, Aubrey
2015-03-04  7:37         ` Ingo Molnar
2015-03-04  8:43           ` Arjan van de Ven
2015-03-04  9:50             ` Borislav Petkov
2015-03-04 14:16               ` Rafael J. Wysocki
2015-03-04 14:05                 ` Borislav Petkov
2015-03-04 14:38                   ` Rafael J. Wysocki
2015-03-04 20:21                   ` Alan Cox
2015-03-04 21:52                     ` Rafael J. Wysocki
2015-03-05 11:26                       ` Li, Aubrey
2015-03-05 16:05                       ` Alan Cox
2015-03-04 14:36               ` Arjan van de Ven
2015-03-04 20:11                 ` Ingo Molnar
2015-03-05 11:13                   ` Li, Aubrey
2015-03-05 11:36                     ` Ingo Molnar
2015-03-05 12:42                       ` Li, Aubrey
2015-03-05 16:06                         ` Alan Cox
2015-03-09 23:26                         ` Li, Aubrey
2015-03-10  8:06                           ` Ingo Molnar
2015-03-11  4:14                             ` Li, Aubrey [this message]
2015-03-04 20:18               ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54FFC139.7070603@linux.intel.com \
    --to=aubrey.li@linux.intel.com \
    --cc=Len.Brown@intel.com \
    --cc=alan@linux.intel.com \
    --cc=arjan@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=hpa@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=rjw@rjwysocki.net \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH] x86: Bypass legacy PIC and PIT on ACPI hardware reduced platform' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).