On 10/8/24 11:56, Jason Andryuk wrote:
On 2024-10-06 17:49, Daniel P. Smith wrote:
To track if the microcode boot module was loaded, a copy of the boot
module is
kept. The size element of this copy is set to zero as the indicator
that the
microcode was loaded. A side effect is that the modules have to be
rescanned to
find the boot module post-relocation, so the cache copy can be created.
Use the consumed boot module flag to track the loading of the
microcode boot
module. This removes the need to manipulate the boot module size
element, no
longer requiring the copy, thus allowing it to be replaced by a
reference. As a
result it is no longer necessary to rescan the boot modules after
relocation
has occurred.
Signed-off-by: Daniel P. Smith <dpsm...@apertussolutions.com>
---
xen/arch/x86/cpu/microcode/core.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/xen/arch/x86/cpu/microcode/core.c
b/xen/arch/x86/cpu/microcode/core.c
index 7bcc17e0ab2f..5b42aad2fdd0 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -826,14 +826,14 @@ int __init microcode_init_cache(
if ( !ucode_ops.apply_microcode )
return -ENODEV;
- if ( ucode_scan )
- /* Need to rescan the modules because they might have been
relocated */
+ /* Scan if microcode was not detected earlier */
+ if ( !ucode_mod )
ucode_scan is a user-controlled variable (ucode=scan=$bool), so I think
it still needs to be respected.
The ucode_scan was introduced due to the complex situation attempting to
be addressed. The microcode needs to be loaded earlier before it is
possible to safely store a cached copy. Multiboot's module_t had no
method of state tracking, identification and consumption. To address
this short coming, the early loading made a copy of the module so it
could use the mod_end field as a flag without breaking the relocation
logic later. And now because it made a copy instead of holding a
reference, when the relocation occurs, the mod_start is no longer valid.
I am not sure why the scan was a user exposed flag, but with boot_module
having identification and state, it is no longer necessary to hold a
copy and a reference can now be used. Since it is now a reference, when
the relocation occurs, there is no longer a need to rescan because of a
relocation. I did leave a rescan if there wasn't microcode detected
during the early load. Though, honestly that probably should go since it
should be the exact same modules that were scanned during early load.
microcode_scan_module(module_map, bi);
- if ( ucode_mod.size )
- rc = early_update_cache(bootstrap_map_bm(&ucode_mod),
- ucode_mod.size);
- else if ( ucode_blob.size )
+ if ( ucode_mod && !(ucode_mod->flags & BOOTMOD_FLAG_X86_CONSUMED) )
+ rc = early_update_cache(bootstrap_map_bm(ucode_mod),
+ ucode_mod->size);
+ else if ( ucode_mod && ucode_blob.size )
ucode_blob seems independent of ucode_mod, so I don't see why this
didn't stay `else if ( ucode_blob.size )`
From my inspection, looks like that should have been an '||" and not a
'&&'. The reason being is that the function will fall back to ucode_mod
if ucode_blob is not set.
v/r,
dps
