Re: [PATCH v5 2/8] microcode/intel: extend microcode_update_match()

["Roger Pau Monné" <roger.pau@citrix.com>]

On Mon, Jan 28, 2019 at 03:06:44PM +0800, Chao Gao wrote:
> to a more generic function. Then, this function can compare two given
> microcodes' signature/revision as well. Comparing two microcodes is
> used to update the global microcode cache (introduced by the later
> patches in this series) when a new microcode is given.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
> Changes in v5:
>  - constify the extended_signature
>  - use named enum type for the return value of microcode_update_match
> ---
>  xen/arch/x86/microcode_intel.c  | 43 ++++++++++++++++++-----------------------
>  xen/include/asm-x86/microcode.h |  6 ++++++
>  2 files changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
> index 4f69f4a..1ed573a 100644
> --- a/xen/arch/x86/microcode_intel.c
> +++ b/xen/arch/x86/microcode_intel.c
> @@ -127,14 +127,24 @@ static int collect_cpu_info(unsigned int cpu_num, struct cpu_signature *csig)
>      return 0;
>  }
>  
> -static inline int microcode_update_match(
> -    unsigned int cpu_num, const struct microcode_header_intel *mc_header,
> -    int sig, int pf)
> +static enum microcode_match_result microcode_update_match(
> +    const void *mc, unsigned int sig, unsigned int pf, unsigned int rev)

Why are you passing this as a void pointer? The only caller is already
passing this as a mc_header pointer.

>  {
> -    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu_num);
> +    const struct microcode_header_intel *mc_header = mc;
> +    const struct extended_sigtable *ext_header;
> +    const struct extended_signature *ext_sig;
> +    unsigned int i;
> +
> +    if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
> +        return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;

The code above implies that a microcode blob can only have a single
version of microcode for each model, I guess this is OK and guaranteed
by Intel?

With at least the first comment fixed:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

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