Skip to content

Commit

Permalink
x86/ucode: Rework AMD's microcode_fits()
Browse files Browse the repository at this point in the history
This function is overloaded, creating complexity; 3 of 4 callers already only
want it for it's "applicable to this CPU or not" answer, and handle revision
calculations separately.

Change it to be microcode_fits_cpu(), returning a simple boolean.  The
checking of the equiv table can be simplified substantially too; A mapping
will only be inserted if it's correct for the CPU, so any nonzero equiv.sig
suffices to know that equiv.id is correct.

Drop compare_header() too, which is simiarly overloaded, and use
compare_revisions() directly.

Notably, this removes a path where cpu_request_microcode() inspects
currently-loaded microcode revision, just to discard the answer.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
  • Loading branch information
andyhhp committed Nov 12, 2024
1 parent 502478b commit 39360c3
Showing 1 changed file with 24 additions and 25 deletions.
49 changes: 24 additions & 25 deletions xen/arch/x86/cpu/microcode/amd.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,36 +182,31 @@ static enum microcode_match_result compare_revisions(
return OLD_UCODE;
}

static enum microcode_match_result microcode_fits(
const struct microcode_patch *patch)
{
unsigned int cpu = smp_processor_id();
const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);

if ( equiv.sig != sig->sig ||
equiv.id != patch->processor_rev_id )
return MIS_UCODE;

return compare_revisions(sig->rev, patch->patch_id);
}

static enum microcode_match_result compare_header(
const struct microcode_patch *new, const struct microcode_patch *old)
/*
* Check whether this microcode patch is applicable for the current CPU.
*
* AMD microcode blobs only have the "equivalent CPU identifier" which is a 16
* bit contraction of the 32 bit Family/Model/Stepping.
*
* We expect to only be run after scan_equiv_cpu_table() has found a valid
* mapping for the current CPU. If this is violated, the 0 in equiv.id will
* cause the patch to be rejected too.
*/
static bool microcode_fits_cpu(const struct microcode_patch *patch)
{
if ( new->processor_rev_id != old->processor_rev_id )
return MIS_UCODE;
ASSERT(equiv.sig);

return compare_revisions(old->patch_id, new->patch_id);
return equiv.id == patch->processor_rev_id;
}

static enum microcode_match_result cf_check compare_patch(
const struct microcode_patch *new, const struct microcode_patch *old)
{
/* Both patches to compare are supposed to be applicable to local CPU. */
ASSERT(microcode_fits(new) != MIS_UCODE);
ASSERT(microcode_fits(old) != MIS_UCODE);
ASSERT(microcode_fits_cpu(new));
ASSERT(microcode_fits_cpu(old));

return compare_header(new, old);
return compare_revisions(old->patch_id, new->patch_id);
}

static int cf_check apply_microcode(const struct microcode_patch *patch,
Expand All @@ -221,12 +216,14 @@ static int cf_check apply_microcode(const struct microcode_patch *patch,
unsigned int cpu = smp_processor_id();
struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
uint32_t rev, old_rev = sig->rev;
enum microcode_match_result result = microcode_fits(patch);
enum microcode_match_result result;
bool ucode_force = flags & XENPF_UCODE_FORCE;

if ( result == MIS_UCODE )
if ( !microcode_fits_cpu(patch) )
return -EINVAL;

result = compare_revisions(old_rev, patch->patch_id);

/*
* Allow application of the same revision to pick up SMT-specific changes
* even if the revision of the other SMT thread is already up-to-date.
Expand Down Expand Up @@ -396,8 +393,10 @@ static struct microcode_patch *cf_check cpu_request_microcode(
* If the new ucode covers current CPU, compare ucodes and store the
* one with higher revision.
*/
if ( (microcode_fits(mc->patch) != MIS_UCODE) &&
(!saved || (compare_header(mc->patch, saved) == NEW_UCODE)) )
if ( microcode_fits_cpu(mc->patch) &&
(!saved ||
compare_revisions(saved->patch_id,
mc->patch->patch_id) == NEW_UCODE) )
{
saved = mc->patch;
saved_size = mc->len;
Expand Down

0 comments on commit 39360c3

Please # to comment.