Re: [Xen-devel] [PATCH v8 06/16] microcode: introduce a global cache of ucode patch

[Jan Beulich <JBeulich@xxxxxxxx>]

On 01.08.2019 12:22, Chao Gao wrote:
> +bool microcode_update_cache(struct microcode_patch *patch)
> +{
> +
> +    ASSERT(spin_is_locked(&microcode_mutex));
> +
> +    if ( !microcode_cache )
> +        microcode_cache = patch;
> +    else if ( microcode_ops->compare_patch(patch, microcode_cache) ==
> +                  NEW_UCODE )

Indentation is wrong here.

> +static struct microcode_patch *alloc_microcode_patch(
> +    const struct microcode_amd *mc_amd)
> +{
> +    struct microcode_patch *microcode_patch = xmalloc(struct 
> microcode_patch);
> +    struct microcode_amd *cache = xmalloc(struct microcode_amd);
> +    void *mpb = xmalloc_bytes(mc_amd->mpb_size);
> +    struct equiv_cpu_entry *equiv_cpu_table =
> +                                xmalloc_bytes(mc_amd->equiv_cpu_table_size);
> +
> +    if ( !microcode_patch || !cache || !mpb || !equiv_cpu_table )
> +    {
> +        xfree(microcode_patch);
> +        xfree(cache);
> +        xfree(mpb);
> +        xfree(equiv_cpu_table);
> +        return ERR_PTR(-ENOMEM);
> +    }
> +
> +    memcpy(mpb, mc_amd->mpb, mc_amd->mpb_size);
> +    cache->mpb = mpb;
> +    cache->mpb_size = mc_amd->mpb_size;
> +    memcpy(equiv_cpu_table, mc_amd->equiv_cpu_table,
> +           mc_amd->equiv_cpu_table_size);
> +    cache->equiv_cpu_table = equiv_cpu_table;
> +    cache->equiv_cpu_table_size = mc_amd->equiv_cpu_table_size;
> +    microcode_patch->mc_amd = cache;
> +
> +    return microcode_patch;
> +}

Why is it that everything needs to be copied here, rather than
simply shuffling one (or a few) pointer(s)? Can't the caller
simply install the argument it passes here as the new cache blob?

> +static enum microcode_match_result compare_patch(
> +    const struct microcode_patch *new, const struct microcode_patch *old)
> +{
> +    const struct microcode_header_intel *old_header = &old->mc_intel->hdr;
> +
> +    return microcode_update_match(&new->mc_intel->hdr, old_header->sig,
> +                                  old_header->pf, old_header->rev);

So this is exactly what I said on the earlier patch the function
cannot be used for. The way "pf" works precludes this, as said in
reply to an earlier version, and no-one corrected me (i.e. I'm in
no way excluding I'm misunderstanding something here).

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel