summaryrefslogtreecommitdiff
path: root/src/include/cpu
diff options
context:
space:
mode:
authorFurquan Shaikh <furquan@google.com>2021-03-08 22:08:42 -0800
committerFurquan Shaikh <furquan@google.com>2021-03-12 17:33:01 +0000
commitf14c05f14493f8960559172b4653f33e26357703 (patch)
treec0c1ab2091b3068fded3a5652a0d488c1b06af1e /src/include/cpu
parent105d91e11476c8889fc7c5f0048c325fef3288fd (diff)
downloadcoreboot-f14c05f14493f8960559172b4653f33e26357703.tar.xz
cpu/intel/microcode: Fix caching logic in intel_microcode_find
CB:49896 added support in `intel_microcode_find()` to cache the found microcode for faster subsequent accesses. This works okay when the function succeeds in finding the microcode on BSP. However, if for any reason, `cpu_microcode_blob.bin` does not contain a valid microcode for the given processor, then the logic ends up attempting to find microcode again and again every time it is called (because `ucode_updates` is set to NULL on failed find, thus retriggering the whole find sequence every time). This leads to a weird race condition when multiple APs are running in parallel and executing this function. A snippet of the issues observed in the scenario described above: ``` ... microcode: Update skipped, already up-to-date ... Microcode header corrupted! ... ``` 1. AP reports that microcode update is being skipped since the current version matches the version in CBFS (even though there is no matching microcode update in CBFS). 2. AP reports microcode header is corrupted because it thinks that the data size reported in the microcode is larger than the file read from CBFS. Above issues occur because each time an AP calls `intel_microcode_find()`, it might end up seeing some intermittent state of `ucode_updates` and taking incorrect action. This change fixes this race condition by separating the logic for finding microcode into an internal function `find_cbfs_microcode()` and maintaining the caching logic in `intel_microcode_find()` using a boolean flag `microcode_checked`. BUG=b:182232187 TEST=Verified that `intel_microcode_find()` no longer makes repeated attempts to find microcode from CBFS if it failed the first time. Change-Id: I8600c830ba029e5cb9c0d7e0f1af18d87c61ad3a Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/51371 Reviewed-by: Patrick Rudolph Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Diffstat (limited to 'src/include/cpu')
-rw-r--r--src/include/cpu/intel/microcode.h6
1 files changed, 5 insertions, 1 deletions
diff --git a/src/include/cpu/intel/microcode.h b/src/include/cpu/intel/microcode.h
index a0ab7fd13b..d977e5836f 100644
--- a/src/include/cpu/intel/microcode.h
+++ b/src/include/cpu/intel/microcode.h
@@ -7,7 +7,11 @@
void intel_update_microcode_from_cbfs(void);
/* Find a microcode that matches the revision and platform family returning
* NULL if none found. The found microcode is cached for faster access on
- * subsequent calls of this function. */
+ * subsequent calls of this function.
+ *
+ * Since this function caches the found microcode (NULL or a valid microcode
+ * pointer), it is expected to be run from BSP before starting any other APs.
+ * It is not multithread safe otherwise. */
const void *intel_microcode_find(void);
/* It is up to the caller to determine if parallel loading is possible as