[GIT PULL] x86/apic changes for v3.14

[GIT PULL] x86/apic changes for v3.14

[Ingo Molnar]

Linus,

Please pull the latest x86-apic-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-apic-for-linus

   # HEAD: 5b4d1dbc24bb6fd7179ada0f47be34e27e64decb x86, apic: Make disabled_cpu_apicid static read_mostly, fix typos

Two main changes:

 - Improve local APIC Error Status Register reporting robustness.

 - Add the 'disable_cpu_apicid=x' boot parameter for kexec booting.

 Thanks,

	Ingo

------------------>
H. Peter Anvin (1):
      x86, apic: Make disabled_cpu_apicid static read_mostly, fix typos

HATAYAMA Daisuke (1):
      x86, apic, kexec: Add disable_cpu_apicid kernel parameter

Richard Weinberger (1):
      x86/apic: Read Error Status Register correctly


 Documentation/kernel-parameters.txt |  9 +++++
 arch/x86/kernel/apic/apic.c         | 66 ++++++++++++++++++++++++++++++++-----
 2 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index b9e9bd8..6fdbf8c 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -774,6 +774,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	disable=	[IPV6]
 			See Documentation/networking/ipv6.txt.
 
+	disable_cpu_apicid= [X86,APIC,SMP]
+			Format: <int>
+			The number of initial APIC ID for the
+			corresponding CPU to be disabled at boot,
+			mostly used for the kdump 2nd kernel to
+			disable BSP to wake up multiple CPUs without
+			causing system reset or hang due to sending
+			INIT from AP to BSP.
+
 	disable_ddw     [PPC/PSERIES]
 			Disable Dynamic DMA Window support. Use this if
 			to workaround buggy firmware.
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index d278736..7f26c9a 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -75,6 +75,13 @@ unsigned int max_physical_apicid;
 physid_mask_t phys_cpu_present_map;
 
 /*
+ * Processor to be disabled specified by kernel parameter
+ * disable_cpu_apicid=<int>, mostly used for the kdump 2nd kernel to
+ * avoid undefined behaviour caused by sending INIT from AP to BSP.
+ */
+static unsigned int disabled_cpu_apicid __read_mostly = BAD_APICID;
+
+/*
  * Map cpu index to physical APIC ID
  */
 DEFINE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid, BAD_APICID);
@@ -1968,7 +1975,7 @@ __visible void smp_trace_spurious_interrupt(struct pt_regs *regs)
  */
 static inline void __smp_error_interrupt(struct pt_regs *regs)
 {
-	u32 v0, v1;
+	u32 v;
 	u32 i = 0;
 	static const char * const error_interrupt_reason[] = {
 		"Send CS error",		/* APIC Error Bit 0 */
@@ -1982,21 +1989,20 @@ static inline void __smp_error_interrupt(struct pt_regs *regs)
 	};
 
 	/* First tickle the hardware, only then report what went on. -- REW */
-	v0 = apic_read(APIC_ESR);
 	apic_write(APIC_ESR, 0);
-	v1 = apic_read(APIC_ESR);
+	v = apic_read(APIC_ESR);
 	ack_APIC_irq();
 	atomic_inc(&irq_err_count);
 
-	apic_printk(APIC_DEBUG, KERN_DEBUG "APIC error on CPU%d: %02x(%02x)",
-		    smp_processor_id(), v0 , v1);
+	apic_printk(APIC_DEBUG, KERN_DEBUG "APIC error on CPU%d: %02x",
+		    smp_processor_id(), v);
 
-	v1 = v1 & 0xff;
-	while (v1) {
-		if (v1 & 0x1)
+	v &= 0xff;
+	while (v) {
+		if (v & 0x1)
 			apic_printk(APIC_DEBUG, KERN_CONT " : %s", error_interrupt_reason[i]);
 		i++;
-		v1 >>= 1;
+		v >>= 1;
 	}
 
 	apic_printk(APIC_DEBUG, KERN_CONT "\n");
@@ -2115,6 +2121,39 @@ int generic_processor_info(int apicid, int version)
 				phys_cpu_present_map);
 
 	/*
+	 * boot_cpu_physical_apicid is designed to have the apicid
+	 * returned by read_apic_id(), i.e, the apicid of the
+	 * currently booting-up processor. However, on some platforms,
+	 * it is temporarily modified by the apicid reported as BSP
+	 * through MP table. Concretely:
+	 *
+	 * - arch/x86/kernel/mpparse.c: MP_processor_info()
+	 * - arch/x86/mm/amdtopology.c: amd_numa_init()
+	 * - arch/x86/platform/visws/visws_quirks.c: MP_processor_info()
+	 *
+	 * This function is executed with the modified
+	 * boot_cpu_physical_apicid. So, disabled_cpu_apicid kernel
+	 * parameter doesn't work to disable APs on kdump 2nd kernel.
+	 *
+	 * Since fixing handling of boot_cpu_physical_apicid requires
+	 * another discussion and tests on each platform, we leave it
+	 * for now and here we use read_apic_id() directly in this
+	 * function, generic_processor_info().
+	 */
+	if (disabled_cpu_apicid != BAD_APICID &&
+	    disabled_cpu_apicid != read_apic_id() &&
+	    disabled_cpu_apicid == apicid) {
+		int thiscpu = num_processors + disabled_cpus;
+
+		pr_warning("APIC: Disabling requested cpu."
+			   " Processor %d/0x%x ignored.\n",
+			   thiscpu, apicid);
+
+		disabled_cpus++;
+		return -ENODEV;
+	}
+
+	/*
 	 * If boot cpu has not been detected yet, then only allow upto
 	 * nr_cpu_ids - 1 processors and keep one slot free for boot cpu
 	 */
@@ -2592,3 +2631,12 @@ static int __init lapic_insert_resource(void)
  * that is using request_resource
  */
 late_initcall(lapic_insert_resource);
+
+static int __init apic_set_disabled_cpu_apicid(char *arg)
+{
+	if (!arg || !get_option(&arg, &disabled_cpu_apicid))
+		return -EINVAL;
+
+	return 0;
+}
+early_param("disable_cpu_apicid", apic_set_disabled_cpu_apicid);

Re: [GIT PULL] x86/apic changes for v3.14

[Maciej W. Rozycki]

On Mon, 20 Jan 2014, Ingo Molnar wrote:

> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index d278736..7f26c9a 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1982,21 +1989,20 @@ static inline void __smp_error_interrupt(struct pt_regs *regs)
>  	};
>  
>  	/* First tickle the hardware, only then report what went on. -- REW */
> -	v0 = apic_read(APIC_ESR);
>  	apic_write(APIC_ESR, 0);
> -	v1 = apic_read(APIC_ESR);
> +	v = apic_read(APIC_ESR);
>  	ack_APIC_irq();
>  	atomic_inc(&irq_err_count);
>  
> -	apic_printk(APIC_DEBUG, KERN_DEBUG "APIC error on CPU%d: %02x(%02x)",
> -		    smp_processor_id(), v0 , v1);
> +	apic_printk(APIC_DEBUG, KERN_DEBUG "APIC error on CPU%d: %02x",
> +		    smp_processor_id(), v);
>  
> -	v1 = v1 & 0xff;
> -	while (v1) {
> -		if (v1 & 0x1)
> +	v &= 0xff;
> +	while (v) {
> +		if (v & 0x1)
>  			apic_printk(APIC_DEBUG, KERN_CONT " : %s", error_interrupt_reason[i]);
>  		i++;
> -		v1 >>= 1;
> +		v >>= 1;
>  	}
>  
>  	apic_printk(APIC_DEBUG, KERN_CONT "\n");

 Sorry to come up with this so late, I've been cleaning up my mailbox from 
old mailing traffic and came across your change only now.

 I'm afraid this interferes with an old Pentium APIC erratum:

"3AP. Writes to Error Register Clears Register

PROBLEM: The APIC Error register is intended to only be read.  If there is 
a write to this register the data in the APIC Error register will be 
cleared and lost.

IMPLICATION: There is a possibility of clearing the Error register status 
since the write to the register is not specifically blocked.

WORKAROUND: Writes should not occur to the Pentium processor APIC Error 
register.

STATUS: For the steppings affected see the Summary Table of Changes at the 
beginning of this section."

The steppings affected are actually: B1, B3 and B5.  Do we want to keep 
supporting them?  I think yes, we already handle the erratum elsewhere 
(lapic_setup_esr).  So how about:

	if (lapic_get_maxlvt() > 3)	/* Due to the Pentium erratum 3AP. */
		apic_write(APIC_ESR, 0);
	v = apic_read(APIC_ESR);

instead?  I can make a patch if that would make your life easier.

 There's room for optimisation here, but I think it's not worth the effort 
as this is a slow path, APIC error interrupts are not supposed to happen 
and are I believe extremely uncommon with FSB message delivery.

  Maciej

Re: [GIT PULL] x86/apic changes for v3.14

[Ingo Molnar]

* Maciej W. Rozycki <macro@linux-mips.org> wrote:

> IMPLICATION: There is a possibility of clearing the Error register 
> status since the write to the register is not specifically blocked.
> 
> WORKAROUND: Writes should not occur to the Pentium processor APIC 
> Error register.
> 
> STATUS: For the steppings affected see the Summary Table of Changes 
> at the beginning of this section."
> 
> The steppings affected are actually: B1, B3 and B5.  Do we want to 
> keep supporting them?  I think yes, we already handle the erratum 
> elsewhere (lapic_setup_esr).  So how about:
> 
> 	if (lapic_get_maxlvt() > 3)	/* Due to the Pentium erratum 3AP. */
> 		apic_write(APIC_ESR, 0);
> 	v = apic_read(APIC_ESR);
> 
> instead?  I can make a patch if that would make your life easier.

Sure, a patch would be helpful.

>  There's room for optimisation here, but I think it's not worth the 
> effort as this is a slow path, APIC error interrupts are not 
> supposed to happen and are I believe extremely uncommon with FSB 
> message delivery.

Agreed.

Thanks,

	Ingo