Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755999AbbDJLw5 (ORCPT ); Fri, 10 Apr 2015 07:52:57 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:32032 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753793AbbDJLwy (ORCPT ); Fri, 10 Apr 2015 07:52:54 -0400 Date: Fri, 10 Apr 2015 13:54:56 +0200 From: Quentin Casasnovas To: Borislav Petkov Cc: Quentin Casasnovas , X86 ML , LKML Subject: Re: [PATCH 07/13] x86/microcode/intel: Rename update_match_revision() Message-ID: <20150410115455.GA23346@chrystal.uk.oracle.com> References: <1424774232-5981-1-git-send-email-bp@alien8.de> <1424774232-5981-8-git-send-email-bp@alien8.de> <20150224162318.GG4565@chrystal.uk.oracle.com> <20150410111218.GC28074@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150410111218.GC28074@pd.tnic> User-Agent: Mutt/1.5.22 (2013-10-16) X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2036 Lines: 66 On Fri, Apr 10, 2015 at 01:12:18PM +0200, Borislav Petkov wrote: > On Tue, Feb 24, 2015 at 05:23:18PM +0100, Quentin Casasnovas wrote: > > Minor nit-pick, if you reverse your inequality, you don't need for the > > ternary operator. > > Yeah, so I started looking at that and it seems the rabbit hole goes > deeper. > > Let's look at the call to revision_is_newer() in _save_mc(): > > save_mc: > > new_rev = mc_hdr->rev; > > ... > > if (!revision_is_newer(mc_hdr, new_rev)) > -> Ha good catch! BTW, I could not find that the 'rev' argument to get_matching_sig() was used at all.. > > --- > From: Borislav Petkov > Date: Fri, 10 Apr 2015 12:50:57 +0200 > Subject: [PATCH] x86/microcode/intel: Get rid of revision_is_newer() > > ... > > diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c > index cd47a510a3f1..63b0a2e059ee 100644 > --- a/arch/x86/kernel/cpu/microcode/intel_lib.c > +++ b/arch/x86/kernel/cpu/microcode/intel_lib.c > @@ -154,13 +154,13 @@ int get_matching_sig(unsigned int csig, int cpf, int rev, void *mc) > /* > * Returns 1 if update has been found, 0 otherwise. > */ > -int get_matching_microcode(unsigned int csig, int cpf, int rev, void *mc) > +int get_matching_microcode(unsigned int csig, int cpf, int new_rev, void *mc) If we're to rename 'rev', maybe calling it 'cpu_rev' would make it more obvious where this variable is coming from? > { > struct microcode_header_intel *mc_hdr = mc; > > - if (!revision_is_newer(mc_hdr, rev)) > + if (mc_hdr->rev <= new_rev) > return 0; > > - return get_matching_sig(csig, cpf, rev, mc); > + return get_matching_sig(csig, cpf, new_rev, mc); > } > EXPORT_SYMBOL_GPL(get_matching_microcode); Anyway you patch looks good to me! Quentin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/