linux-kernel - [PATCH]: local_irq_save/restore when issuing IPI in early bootup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20100812134252.23717.50702.sendpatchset@prarit.bos.redhat.com>
Date:	Thu, 12 Aug 2010 09:42:53 -0400
From:	Prarit Bhargava <prarit@...hat.com>
To:	linux-kernel@...r.kernel.org, x86@...nel.org, akataria@...are.com,
	sassmann@...hat.com
Cc:	Prarit Bhargava <prarit@...hat.com>
Subject: [PATCH]: local_irq_save/restore when issuing IPI in early bootup

When sending out an IPI interrupts are masked off.  In the cpu wakeup code, this
is not the case.  Make the code consistent.

Alok Kataria noticed the following hang:

1.  vcpu1 is doing wakeup_secondary_cpu_via_init() is booting all the other
processors.  It tries to bring each cpu up by sending an IPI which is done
by writing the APIC ICR.

The call path is wakeup_secondary_cpu_via_init() which calls apic_icr_write->
native_apic_icr_write().  native_apic_icr_write() writes APIC_ICR2 and APIC_ICR
in order to modify the ICRHI and ICRLO values.

The ICRHI is set to the number of online cpus.

The write to APIC_ICR does not happen because

2.  vcpu1's physical cpu *may* take a timer interrupt which does
lapic_timer_broadcast(), which calls send_IPI_mask().  send_IPI_mask()
calls default_send_IPI_mask_sequence_phys() which changes the ICRHI value to
the number of online vcpus -- which is one less than what it was set to in
step 1.

3.  return to thread in step 1 ... ICRHI is different and one less
than the previous value.  This results in the IPI being sent TWICE to the
same vcpu (CPU X-1) instead of once to CPU X-1 and once to CPU X.

It may be that native_apic_icr_write() actually needs it's own irq save/restore,
however, I cannot see a code path that requires it.  A larger irq save/restore
seems more appropriate around the IPI code which seems adequate given that
other code does the same thing (see calls to __default_send_IPI_dest_field() )
and is not broken.

Signed-off-by: Prarit Bhargava <prarit@...hat.com>

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 5162095..39f871c 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -556,6 +556,7 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
 {
 	unsigned long send_status, accept_status = 0;
 	int maxlvt, num_starts, j;
+	unsigned long flags;

 	maxlvt = lapic_get_maxlvt();

@@ -576,6 +577,7 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
 	/*
 	 * Send IPI
 	 */
+	local_irq_save(flags);
 	apic_icr_write(APIC_INT_LEVELTRIG | APIC_INT_ASSERT | APIC_DM_INIT,
 		       phys_apicid);

@@ -592,6 +594,7 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)

 	pr_debug("Waiting for send to finish...\n");
 	send_status = safe_apic_wait_icr_idle();
+	local_irq_restore(flags);

 	mb();
 	atomic_set(&init_deasserted, 1);
@@ -633,6 +636,7 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
 		/* Target chip */
 		/* Boot on the stack */
 		/* Kick the second */
+		local_irq_save(flags);
 		apic_icr_write(APIC_DM_STARTUP | (start_eip >> 12),
 			       phys_apicid);

@@ -645,6 +649,7 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)

 		pr_debug("Waiting for send to finish...\n");
 		send_status = safe_apic_wait_icr_idle();
+		local_irq_restore(flags);

 		/*
 		 * Give the other CPU some time to accept the IPI.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/