Attention is currently required from: Patrick Rudolph. Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59321 )
Change subject: treewide: Replace spinlock calls with mutex ......................................................................
treewide: Replace spinlock calls with mutex
spinlock does not currently work nicely with coop-threads. If a `udelay` is called while in a critical section, it will context switch. If that second thread also tries to acquire the same lock then we deadlock since we never yield again. There is currently a hack in x86/spinlock.h where we disable coop threads when acquiring a spinlock. This hack only works for ramstage because in romstage we use the noop spinlock implementation. By replacing the spinlocks with mutex we no longer need to rely on the hack in the x86/spinlock.h.
I didn't convert the printk spinlock into a mutex. Since printk is the primary debugging tool, I wanted to keep it decoupled from threading mutex. This continues to allow debugging the threading code with printks.
BUG=b:179699789 TEST=Boot guybrush to OS
Signed-off-by: Raul E Rangel rrangel@chromium.org Change-Id: Ibee331a9ccd491e819c7f2c9a19bca7dc4379d9d --- M src/cpu/intel/microcode/microcode.c M src/cpu/x86/lapic/lapic_cpu_init.c M src/cpu/x86/mp_init.c M src/device/device.c M src/drivers/pc80/rtc/post.c 5 files changed, 28 insertions(+), 28 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/59321/1
diff --git a/src/cpu/intel/microcode/microcode.c b/src/cpu/intel/microcode/microcode.c index 469bd25..ea0cd80 100644 --- a/src/cpu/intel/microcode/microcode.c +++ b/src/cpu/intel/microcode/microcode.c @@ -9,9 +9,9 @@ #include <console/console.h> #include <cpu/x86/msr.h> #include <cpu/intel/microcode.h> -#include <smp/spinlock.h> +#include <mutex.h>
-DECLARE_SPIN_LOCK(microcode_lock) +struct mutex microcode_lock;
struct microcode { u32 hdrver; /* Header Version */ @@ -243,11 +243,11 @@ { const void *patch = intel_microcode_find();
- spin_lock(µcode_lock); + mutex_lock(µcode_lock);
intel_microcode_load_unlocked(patch);
- spin_unlock(µcode_lock); + mutex_unlock(µcode_lock); }
#if ENV_RAMSTAGE diff --git a/src/cpu/x86/lapic/lapic_cpu_init.c b/src/cpu/x86/lapic/lapic_cpu_init.c index c35888a..9af6519 100644 --- a/src/cpu/x86/lapic/lapic_cpu_init.c +++ b/src/cpu/x86/lapic/lapic_cpu_init.c @@ -13,9 +13,9 @@ #include <device/device.h> #include <device/path.h> #include <smp/atomic.h> -#include <smp/spinlock.h> #include <cpu/cpu.h> #include <cpu/intel/speedstep.h> +#include <mutex.h> #include <smp/node.h> #include <stdlib.h> #include <thread.h> @@ -215,7 +215,7 @@ * start_cpu returns. */
-DECLARE_SPIN_LOCK(start_cpu_lock); +struct mutex start_cpu_lock; static unsigned int last_cpu_index = 0; static void *stacks[CONFIG_MAX_CPUS]; volatile unsigned long secondary_stack; @@ -231,7 +231,7 @@ unsigned long count; int result;
- spin_lock(&start_cpu_lock); + mutex_lock(&start_cpu_lock);
/* Get the CPU's apicid */ apicid = cpu->path.apic.apic_id; @@ -277,7 +277,7 @@ } } secondary_stack = 0; - spin_unlock(&start_cpu_lock); + mutex_unlock(&start_cpu_lock); return result; }
@@ -286,7 +286,7 @@ { atomic_inc(&active_cpus);
- spin_lock(&start_cpu_lock); + mutex_lock(&start_cpu_lock);
#ifdef __SSE3__ /* @@ -300,7 +300,7 @@ #endif cpu_initialize(index);
- spin_unlock(&start_cpu_lock); + mutex_unlock(&start_cpu_lock);
atomic_dec(&active_cpus);
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index c99732f..156b963 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -18,8 +18,8 @@ #include <delay.h> #include <device/device.h> #include <device/path.h> +#include <mutex.h> #include <smp/atomic.h> -#include <smp/spinlock.h> #include <symbols.h> #include <timer.h> #include <thread.h> @@ -659,14 +659,14 @@ printk(BIOS_DEBUG, "Relocation complete.\n"); }
-DECLARE_SPIN_LOCK(smm_relocation_lock); +struct mutex smm_relocation_lock;
/* Send SMI to self with single user serialization. */ void smm_initiate_relocation(void) { - spin_lock(&smm_relocation_lock); + mutex_lock(&smm_relocation_lock); smm_initiate_relocation_parallel(); - spin_unlock(&smm_relocation_lock); + mutex_unlock(&smm_relocation_lock); }
struct mp_state { diff --git a/src/device/device.c b/src/device/device.c index 8337d55..e06e659 100644 --- a/src/device/device.c +++ b/src/device/device.c @@ -8,10 +8,10 @@ #include <device/device.h> #include <device/pci_def.h> #include <device/pci_ids.h> +#include <mutex.h> #include <post.h> #include <stdlib.h> #include <string.h> -#include <smp/spinlock.h> #if ENV_X86 #include <arch/ebda.h> #endif @@ -71,7 +71,7 @@ } }
-DECLARE_SPIN_LOCK(dev_lock) +struct mutex dev_lock;
/** * Allocate a new device structure. @@ -122,9 +122,9 @@ struct device *alloc_dev(struct bus *parent, struct device_path *path) { struct device *dev; - spin_lock(&dev_lock); + mutex_lock(&dev_lock); dev = __alloc_dev(parent, path); - spin_unlock(&dev_lock); + mutex_unlock(&dev_lock); return dev; }
@@ -138,11 +138,11 @@ struct device *alloc_find_dev(struct bus *parent, struct device_path *path) { struct device *child; - spin_lock(&dev_lock); + mutex_lock(&dev_lock); child = find_dev_path(parent, path); if (!child) child = __alloc_dev(parent, path); - spin_unlock(&dev_lock); + mutex_unlock(&dev_lock); return child; }
diff --git a/src/drivers/pc80/rtc/post.c b/src/drivers/pc80/rtc/post.c index aa8ea18..eff42ed 100644 --- a/src/drivers/pc80/rtc/post.c +++ b/src/drivers/pc80/rtc/post.c @@ -1,11 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <stdint.h> +#include <mutex.h> #include <post.h> #include <console/console.h> #include <device/device.h> #include <pc80/mc146818rtc.h> -#include <smp/spinlock.h>
#if CONFIG(USE_OPTION_TABLE) # include "option_table.h" @@ -35,14 +35,14 @@
#define CMOS_POST_EXTRA_DEV_PATH 0x01
-DECLARE_SPIN_LOCK(cmos_post_lock) +struct mutex cmos_post_lock;
int cmos_post_previous_boot(u8 *code, u32 *extra) { *code = 0; *extra = 0;
- spin_lock(&cmos_post_lock); + mutex_lock(&cmos_post_lock);
/* Get post code from other bank */ switch (cmos_read(CMOS_POST_BANK_OFFSET)) { @@ -56,7 +56,7 @@ break; }
- spin_unlock(&cmos_post_lock); + mutex_unlock(&cmos_post_lock);
/* Check last post code in previous boot against normal list */ switch (*code) { @@ -96,7 +96,7 @@
void cmos_post_code(u8 value) { - spin_lock(&cmos_post_lock); + mutex_lock(&cmos_post_lock);
switch (cmos_read(CMOS_POST_BANK_OFFSET)) { case CMOS_POST_BANK_0_MAGIC: @@ -107,12 +107,12 @@ break; }
- spin_unlock(&cmos_post_lock); + mutex_unlock(&cmos_post_lock); }
void cmos_post_extra(u32 value) { - spin_lock(&cmos_post_lock); + mutex_lock(&cmos_post_lock);
switch (cmos_read(CMOS_POST_BANK_OFFSET)) { case CMOS_POST_BANK_0_MAGIC: @@ -123,7 +123,7 @@ break; }
- spin_unlock(&cmos_post_lock); + mutex_unlock(&cmos_post_lock); }
void cmos_post_path(const struct device *dev)