From 0108cce0a4d60d00a34255f0ad17df3d01a05911 Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Fri, 5 Nov 2010 13:42:58 +0000 Subject: [PATCH] Adjust the order of operations in spinlock_enter() and spinlock_exit() to work properly with single-stepping in a kernel debugger. Specifically, these routines have always disabled interrupts before increasing the nesting count and restored the prior state of interrupts after decreasing the nesting count to avoid problems with a nested interrupt not disabling interrupts when acquiring a spin lock. However, trap interrupts for single-stepping can still occur even when interrupts are disabled. Now the saved state of interrupts is not saved in the thread until after interrupts have been disabled and the nesting count has been increased. Similarly, the saved state from the thread cannot be read once the nesting count has been decreased to zero. To fix this, use temporary variables to store interrupt state and shuffle it between the thread's MD area and the appropriate registers. In cooperation with: bde MFC after: 1 month --- sys/amd64/amd64/machdep.c | 14 ++++++++++---- sys/arm/arm/machdep.c | 14 ++++++++++---- sys/i386/i386/machdep.c | 14 ++++++++++---- sys/ia64/ia64/machdep.c | 14 ++++++++++---- sys/mips/mips/machdep.c | 14 ++++++++++---- sys/pc98/pc98/machdep.c | 14 ++++++++++---- sys/powerpc/aim/machdep.c | 14 ++++++++++---- sys/powerpc/booke/machdep.c | 14 ++++++++++---- sys/sparc64/sparc64/machdep.c | 9 ++++++--- sys/sun4v/sun4v/machdep.c | 13 +++++++------ 10 files changed, 93 insertions(+), 41 deletions(-) diff --git a/sys/amd64/amd64/machdep.c b/sys/amd64/amd64/machdep.c index eba788f374b..738427f25fb 100644 --- a/sys/amd64/amd64/machdep.c +++ b/sys/amd64/amd64/machdep.c @@ -1762,11 +1762,15 @@ void spinlock_enter(void) { struct thread *td; + register_t flags; td = curthread; - if (td->td_md.md_spinlock_count == 0) - td->td_md.md_saved_flags = intr_disable(); - td->td_md.md_spinlock_count++; + if (td->td_md.md_spinlock_count == 0) { + flags = intr_disable(); + td->td_md.md_spinlock_count = 1; + td->td_md.md_saved_flags = flags; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -1774,12 +1778,14 @@ void spinlock_exit(void) { struct thread *td; + register_t flags; td = curthread; critical_exit(); + flags = td->td_md.md_saved_flags; td->td_md.md_spinlock_count--; if (td->td_md.md_spinlock_count == 0) - intr_restore(td->td_md.md_saved_flags); + intr_restore(flags); } /* diff --git a/sys/arm/arm/machdep.c b/sys/arm/arm/machdep.c index 32348d7933b..2d26f685105 100644 --- a/sys/arm/arm/machdep.c +++ b/sys/arm/arm/machdep.c @@ -493,11 +493,15 @@ void spinlock_enter(void) { struct thread *td; + register_t cspr; td = curthread; - if (td->td_md.md_spinlock_count == 0) - td->td_md.md_saved_cspr = disable_interrupts(I32_bit | F32_bit); - td->td_md.md_spinlock_count++; + if (td->td_md.md_spinlock_count == 0) { + cspr = disable_interrupts(I32_bit | F32_bit); + td->td_md.md_spinlock_count = 1; + td->td_md.md_saved_cspr = cspr; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -505,12 +509,14 @@ void spinlock_exit(void) { struct thread *td; + register_t cspr; td = curthread; critical_exit(); + cspr = td->td_md.md_saved_cspr; td->td_md.md_spinlock_count--; if (td->td_md.md_spinlock_count == 0) - restore_interrupts(td->td_md.md_saved_cspr); + restore_interrupts(cspr); } /* diff --git a/sys/i386/i386/machdep.c b/sys/i386/i386/machdep.c index 4901b38ff35..0fbd519ef6a 100644 --- a/sys/i386/i386/machdep.c +++ b/sys/i386/i386/machdep.c @@ -2997,11 +2997,15 @@ void spinlock_enter(void) { struct thread *td; + register_t flags; td = curthread; - if (td->td_md.md_spinlock_count == 0) - td->td_md.md_saved_flags = intr_disable(); - td->td_md.md_spinlock_count++; + if (td->td_md.md_spinlock_count == 0) { + flags = intr_disable(); + td->td_md.md_spinlock_count = 1; + td->td_md.md_saved_flags = flags; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -3009,12 +3013,14 @@ void spinlock_exit(void) { struct thread *td; + register_t flags; td = curthread; critical_exit(); + flags = td->td_md.md_saved_flags; td->td_md.md_spinlock_count--; if (td->td_md.md_spinlock_count == 0) - intr_restore(td->td_md.md_saved_flags); + intr_restore(flags); } #if defined(I586_CPU) && !defined(NO_F00F_HACK) diff --git a/sys/ia64/ia64/machdep.c b/sys/ia64/ia64/machdep.c index ed00a0499aa..119c36f03b5 100644 --- a/sys/ia64/ia64/machdep.c +++ b/sys/ia64/ia64/machdep.c @@ -513,11 +513,15 @@ void spinlock_enter(void) { struct thread *td; + int intr; td = curthread; - if (td->td_md.md_spinlock_count == 0) - td->td_md.md_saved_intr = intr_disable(); - td->td_md.md_spinlock_count++; + if (td->td_md.md_spinlock_count == 0) { + intr = intr_disable(); + td->td_md.md_spinlock_count = 1; + td->td_md.md_saved_intr = intr; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -525,12 +529,14 @@ void spinlock_exit(void) { struct thread *td; + int intr; td = curthread; critical_exit(); + intr = td->td_md.md_saved_intr; td->td_md.md_spinlock_count--; if (td->td_md.md_spinlock_count == 0) - intr_restore(td->td_md.md_saved_intr); + intr_restore(intr); } void diff --git a/sys/mips/mips/machdep.c b/sys/mips/mips/machdep.c index 435b9b4d67a..314bfd12115 100644 --- a/sys/mips/mips/machdep.c +++ b/sys/mips/mips/machdep.c @@ -450,11 +450,15 @@ void spinlock_enter(void) { struct thread *td; + register_t intr; td = curthread; - if (td->td_md.md_spinlock_count == 0) - td->td_md.md_saved_intr = intr_disable(); - td->td_md.md_spinlock_count++; + if (td->td_md.md_spinlock_count == 0) { + intr = intr_disable(); + td->td_md.md_spinlock_count = 1; + td->td_md.md_saved_intr = intr; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -462,12 +466,14 @@ void spinlock_exit(void) { struct thread *td; + register_t intr; td = curthread; critical_exit(); + intr = td->td_md.md_saved_intr; td->td_md.md_spinlock_count--; if (td->td_md.md_spinlock_count == 0) - intr_restore(td->td_md.md_saved_intr); + intr_restore(intr); } /* diff --git a/sys/pc98/pc98/machdep.c b/sys/pc98/pc98/machdep.c index e01ef681fcf..942aa45b477 100644 --- a/sys/pc98/pc98/machdep.c +++ b/sys/pc98/pc98/machdep.c @@ -2340,11 +2340,15 @@ void spinlock_enter(void) { struct thread *td; + register_t flags; td = curthread; - if (td->td_md.md_spinlock_count == 0) - td->td_md.md_saved_flags = intr_disable(); - td->td_md.md_spinlock_count++; + if (td->td_md.md_spinlock_count == 0) { + flags = intr_disable(); + td->td_md.md_spinlock_count = 1; + td->td_md.md_saved_flags = flags; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -2352,12 +2356,14 @@ void spinlock_exit(void) { struct thread *td; + register_t flags; td = curthread; critical_exit(); + flags = td->td_md.md_saved_flags; td->td_md.md_spinlock_count--; if (td->td_md.md_spinlock_count == 0) - intr_restore(td->td_md.md_saved_flags); + intr_restore(flags); } #if defined(I586_CPU) && !defined(NO_F00F_HACK) diff --git a/sys/powerpc/aim/machdep.c b/sys/powerpc/aim/machdep.c index 9dec1f75944..0331b502780 100644 --- a/sys/powerpc/aim/machdep.c +++ b/sys/powerpc/aim/machdep.c @@ -751,11 +751,15 @@ void spinlock_enter(void) { struct thread *td; + register_t msr; td = curthread; - if (td->td_md.md_spinlock_count == 0) - td->td_md.md_saved_msr = intr_disable(); - td->td_md.md_spinlock_count++; + if (td->td_md.md_spinlock_count == 0) { + msr = intr_disable(); + td->td_md.md_spinlock_count = 1; + td->td_md.md_saved_msr = msr; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -763,12 +767,14 @@ void spinlock_exit(void) { struct thread *td; + register_t msr; td = curthread; critical_exit(); + msr = td->td_md.md_saved_msr; td->td_md.md_spinlock_count--; if (td->td_md.md_spinlock_count == 0) - intr_restore(td->td_md.md_saved_msr); + intr_restore(msr); } /* diff --git a/sys/powerpc/booke/machdep.c b/sys/powerpc/booke/machdep.c index c4b80cc665e..d21629243a2 100644 --- a/sys/powerpc/booke/machdep.c +++ b/sys/powerpc/booke/machdep.c @@ -516,11 +516,15 @@ void spinlock_enter(void) { struct thread *td; + register_t msr; td = curthread; - if (td->td_md.md_spinlock_count == 0) - td->td_md.md_saved_msr = intr_disable(); - td->td_md.md_spinlock_count++; + if (td->td_md.md_spinlock_count == 0) { + msr = intr_disable(); + td->td_md.md_spinlock_count = 1; + td->td_md.md_saved_msr = msr; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -528,12 +532,14 @@ void spinlock_exit(void) { struct thread *td; + register_t msr; td = curthread; critical_exit(); + msr = td->td_md.md_saved_msr; td->td_md.md_spinlock_count--; if (td->td_md.md_spinlock_count == 0) - intr_restore(td->td_md.md_saved_msr); + intr_restore(msr); } /* Shutdown the CPU as much as possible. */ diff --git a/sys/sparc64/sparc64/machdep.c b/sys/sparc64/sparc64/machdep.c index 82643ab4530..0a40bad0b5f 100644 --- a/sys/sparc64/sparc64/machdep.c +++ b/sys/sparc64/sparc64/machdep.c @@ -224,9 +224,10 @@ spinlock_enter(void) if (td->td_md.md_spinlock_count == 0) { pil = rdpr(pil); wrpr(pil, 0, PIL_TICK); + td->td_md.md_spinlock_count = 1; td->td_md.md_saved_pil = pil; - } - td->td_md.md_spinlock_count++; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -234,12 +235,14 @@ void spinlock_exit(void) { struct thread *td; + register_t pil; td = curthread; critical_exit(); + pil = td->td_md.md_saved_pil; td->td_md.md_spinlock_count--; if (td->td_md.md_spinlock_count == 0) - wrpr(pil, td->td_md.md_saved_pil, 0); + wrpr(pil, pil, 0); } static phandle_t diff --git a/sys/sun4v/sun4v/machdep.c b/sys/sun4v/sun4v/machdep.c index 3a80bee6950..43496e3012f 100644 --- a/sys/sun4v/sun4v/machdep.c +++ b/sys/sun4v/sun4v/machdep.c @@ -269,9 +269,10 @@ spinlock_enter(void) td = curthread; if (td->td_md.md_spinlock_count == 0) { pil = intr_disable(); + td->td_md.md_spinlock_count = 1; td->td_md.md_saved_pil = pil; - } - td->td_md.md_spinlock_count++; + } else + td->td_md.md_spinlock_count++; critical_enter(); } @@ -279,14 +280,14 @@ void spinlock_exit(void) { struct thread *td; + register_t pil; td = curthread; critical_exit(); + pil = td->td_md.md_saved_pil; td->td_md.md_spinlock_count--; - if (td->td_md.md_spinlock_count == 0) { - intr_restore(td->td_md.md_saved_pil); - } - + if (td->td_md.md_spinlock_count == 0) + intr_restore(pil); } unsigned