[PATCH] x86/apic: Justification for disabling IO APIC before Local APIC

[PATCH] x86/apic: Justification for disabling IO APIC before Local APIC

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

Since erratum AVR31 in "Intel Atom Processor C2000 Product Family
Specification Update" is published, I add a justification comment for
disabling IO APIC before Local APIC (commit 522e6646).

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/reboot.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index da3c599..c752cb4 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -558,6 +558,17 @@ void native_machine_shutdown(void)
 {
 	/* Stop the cpus and apics */
 #ifdef CONFIG_X86_IO_APIC
+	/*
+	 * Disabling IO APIC before local APIC is a workaround for
+	 * erratum AVR31 in "Intel Atom Processor C2000 Product Family
+	 * Specification Update". In this situation, interrupts that target
+	 * a Logical Processor whose Local APIC is either in the process of
+	 * being hardware disabled or software disabled are neither delivered
+	 * nor discarded. When this erratum occurs, the processor may hang.
+	 *
+	 * Even without the erratum, it still makes sense to quiet IO APIC
+	 * before disabling Local APIC.
+	 */
 	disable_IO_APIC();
 #endif
 
-- 
1.8.1.2

[tip:x86/urgent] x86/apic, doc: Justification for disabling IO APIC before Local APIC

[tip-bot for Fenghua Yu]

Commit-ID:  2885432aaf15c1b7e65c787bfe7c5fec428296f0
Gitweb:     http://git.kernel.org/tip/2885432aaf15c1b7e65c787bfe7c5fec428296f0
Author:     Fenghua Yu <fenghua.yu@intel.com>
AuthorDate: Wed, 4 Dec 2013 16:07:49 -0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Wed, 4 Dec 2013 19:33:21 -0800

x86/apic, doc: Justification for disabling IO APIC before Local APIC

Since erratum AVR31 in "Intel Atom Processor C2000 Product Family
Specification Update" is now published, I added a justification
comment for disabling IO APIC before Local APIC, as changed in commit:

522e66464467 x86/apic: Disable I/O APIC before shutdown of the local APIC

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Link: http://lkml.kernel.org/r/1386202069-51515-1-git-send-email-fenghua.yu@intel.com
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/kernel/reboot.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index da3c599..c752cb4 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -558,6 +558,17 @@ void native_machine_shutdown(void)
 {
 	/* Stop the cpus and apics */
 #ifdef CONFIG_X86_IO_APIC
+	/*
+	 * Disabling IO APIC before local APIC is a workaround for
+	 * erratum AVR31 in "Intel Atom Processor C2000 Product Family
+	 * Specification Update". In this situation, interrupts that target
+	 * a Logical Processor whose Local APIC is either in the process of
+	 * being hardware disabled or software disabled are neither delivered
+	 * nor discarded. When this erratum occurs, the processor may hang.
+	 *
+	 * Even without the erratum, it still makes sense to quiet IO APIC
+	 * before disabling Local APIC.
+	 */
 	disable_IO_APIC();
 #endif
 

Re: [PATCH] x86/apic: Justification for disabling IO APIC before Local APIC

[Ingo Molnar]

* Fenghua Yu <fenghua.yu@intel.com> wrote:

> From: Fenghua Yu <fenghua.yu@intel.com>
> 
> Since erratum AVR31 in "Intel Atom Processor C2000 Product Family
> Specification Update" is published, I add a justification comment for
> disabling IO APIC before Local APIC (commit 522e6646).
> 
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  arch/x86/kernel/reboot.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index da3c599..c752cb4 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -558,6 +558,17 @@ void native_machine_shutdown(void)
>  {
>  	/* Stop the cpus and apics */
>  #ifdef CONFIG_X86_IO_APIC
> +	/*
> +	 * Disabling IO APIC before local APIC is a workaround for
> +	 * erratum AVR31 in "Intel Atom Processor C2000 Product Family
> +	 * Specification Update". In this situation, interrupts that target
> +	 * a Logical Processor whose Local APIC is either in the process of
> +	 * being hardware disabled or software disabled are neither delivered
> +	 * nor discarded. When this erratum occurs, the processor may hang.
> +	 *
> +	 * Even without the erratum, it still makes sense to quiet IO APIC
> +	 * before disabling Local APIC.
> +	 */
>  	disable_IO_APIC();
>  #endif

Looks good to me, except that patch titles should start with verbs, 
i.e. something like:

  x86/apic/doc: Add justification for disabling IO APIC before Local APIC

Which makes for a much more fluid reading of shortlogs etc.

Unless you sentences without verbs.
          ^-------------------------------------------like

;-)

Thanks,

	Ingo

RE: [PATCH] x86/apic: Justification for disabling IO APIC before Local APIC

[Yu, Fenghua]

> From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
> Molnar
> Sent: Thursday, December 05, 2013 12:55 AM
> 
> 
> * Fenghua Yu <fenghua.yu@intel.com> wrote:
> 
> > From: Fenghua Yu <fenghua.yu@intel.com>
> >
> > Since erratum AVR31 in "Intel Atom Processor C2000 Product Family
> > Specification Update" is published, I add a justification comment for
> > disabling IO APIC before Local APIC (commit 522e6646).
> >
> 
> Looks good to me, except that patch titles should start with verbs,
> i.e. something like:
> 
>   x86/apic/doc: Add justification for disabling IO APIC before Local
> APIC
> 
> Which makes for a much more fluid reading of shortlogs etc.
> 
> Unless you sentences without verbs.
>           ^-------------------------------------------like
> 
> ;-)
> 
> Thanks,
> 
> 	Ingo

Thank you very much for your comment, Ingo.

-Fenghua