Re: [PATCH v4 1/6] microcode/intel: extend microcode_update_match()

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

On Thu, Nov 29, 2018 at 10:00:34AM +0800, Chao Gao wrote:
> On Wed, Nov 28, 2018 at 11:58:06AM +0100, Roger Pau Monné wrote:
> >On Wed, Nov 28, 2018 at 01:34:11PM +0800, Chao Gao wrote:
> >>  static int microcode_sanity_check(void *mc)
> >> @@ -236,31 +259,13 @@ static int get_matching_microcode(const void *mc, unsigned int cpu)
> >>  {
> >>      struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
> >>      const struct microcode_header_intel *mc_header = mc;
> >> -    const struct extended_sigtable *ext_header;
> >>      unsigned long total_size = get_totalsize(mc_header);
> >> -    int ext_sigcount, i;
> >> -    struct extended_signature *ext_sig;
> >>      void *new_mc;
> >>  
> >> -    if ( microcode_update_match(cpu, mc_header,
> >> -                                mc_header->sig, mc_header->pf) )
> >> -        goto find;
> >> -
> >> -    if ( total_size <= (get_datasize(mc_header) + MC_HEADER_SIZE) )
> >> +    if ( microcode_update_match(mc, uci->cpu_sig.sig, uci->cpu_sig.pf,
> >> +                                uci->cpu_sig.rev) != NEW_UCODE )
> >>          return 0;
> >
> >Shouldn't you differentiate between the function returning OLD_UCODE
> >or MIS_UCODE? I would expect that trying to load a mismatched UCODE
> >would trigger some kind of message from Xen.
> 
> I don't differentiate these two cases. For both of them, we do nothing.
> Actually, I add a message "No newer or matched microcode found" in patch 4
> for them (Currently each cpu parses the file locally, if we add
> an error message, it will show up many times). However, if you are to load
> a corrupted file, another error will be prompted.

What I wanted to point out is that you don't need 3 different return
values if you only differentiate between two of them. Ie: a boolean
will achieve the same result here.

If those 3 different return values are indeed used by patch 4 then
that's fine, I didn't realize it.

Thanks, Roger.

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