Messages in this thread | | | Date | Tue, 21 Jan 2014 15:34:58 +0100 | From | Jan Kiszka <> | Subject | Re: x86: Inconsistent xAPIC synchronization in arch_irq_work_raise? |
| |
On 2014-01-21 15:11, Jan Kiszka wrote: > On 2014-01-21 15:01, Peter Zijlstra wrote: >> On Tue, Jan 21, 2014 at 02:02:06PM +0100, Jan Kiszka wrote: >>> Hi all, >>> >>> while trying to plug a race in the CPU hotplug code on xAPIC systems, I >>> was analyzing IPI transmission patterns. The handlers in >>> arch/x86/include/asm/ipi.h first wait for ICR, then send. In contrast, >>> arch_irq_work_raise sends the self-IPI directly and then waits. This >>> looks inconsistent. Is it intended? >>> >>> BTW, the races are in wakeup_secondary_cpu_via_init and >>> wakeup_secondary_cpu_via_nmi (lacking IRQ disable around ICR accesses). >>> There we also send first, then wait for completion. But I guess that is >>> due to the code originally only being used during boot. Will send fixes >>> for those once the sync pattern is clear to me. >> >> Could be I had no clue what I was doing and copy/pasted the code until >> it compiled and ran. >> >> In fact, I've got no clue what an ICR is. > > Old xAPIC requires you to only send IPIs, when the APIC signals it is > done with sending the previous one. Therefore we wait for availability > in the other IPI transmission services before writing to ICR. > > OK, then I will write a separate patch for arch_irq_work_raise to switch > the ordering.
Hmm, missed that we do have synchronization already via apic->send_IPI_self -> default_send_IPI_self -> __default_send_IPI_shortcut. So the closing wait would only be relevant if we need to settle the APIC because we may have interrupted a wait_icr_idle + write_icr sequence. But shouldnt those sequences be atomic (except for the problematic wakeup_secondary path)?
Still confused: What is the official locking model around wait_icr_idle, write(ICR), and also write(IRC2)? IRQ disable around ICR2+ICR accesses and preempt_disable around wait + write? That is also important to fix the SMP boot-up code properly.
Jan
-- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
| |