Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6882a4e7-a733-5ef0-f32c-53763b3c84e0@intel.com>
Date:   Fri, 11 Nov 2022 13:39:27 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v2 06/14] x86/microcode/intel: Expose
 microcode_sanity_check()
Content-Language: en-US
To:     Borislav Petkov <bp@alien8.de>
CC:     <hdegoede@redhat.com>, <markgross@kernel.org>,
        <tglx@linutronix.de>, <mingo@redhat.com>,
        <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
        <gregkh@linuxfoundation.org>, <ashok.raj@intel.com>,
        <tony.luck@intel.com>, <linux-kernel@vger.kernel.org>,
        <platform-driver-x86@vger.kernel.org>, <patches@lists.linux.dev>,
        <ravi.v.shankar@intel.com>, <thiago.macieira@intel.com>,
        <athenas.jimenez.gonzalez@intel.com>, <sohil.mehta@intel.com>
References: <20221021203413.1220137-1-jithu.joseph@intel.com>
 <20221107225323.2733518-1-jithu.joseph@intel.com>
 <20221107225323.2733518-7-jithu.joseph@intel.com> <Y25dTPMQ5uwBDa9I@zn.tnic>
From:   "Joseph, Jithu" <jithu.joseph@intel.com>
In-Reply-To: <Y25dTPMQ5uwBDa9I@zn.tnic>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?azgrdnMyeGlxdFhFZGRNMGZSL3J1MEhYQVU2OG41VUJDNlUzcFVTOVhUK3R1?=
 =?utf-8?B?ZmRJMnVNc3BOMGoralZkN0F0YU1JYkFWbWNRQzlWVHJGRVorZnBSOEVZUXl3?=
 =?utf-8?B?L3RvNWpMWFV2bU9UZGtUQ3JVK0IzejVzQ3VPSmRyN3dxdlhoVERQT2hFd1Az?=
 =?utf-8?B?Sm9lQlFEMjB4MWlubmZNeTVkbDljYS95TDYwdy9qQ0RWenBZSzBWcFBMNXU3?=
 =?utf-8?B?Zy80a28xMWcydklxUzFFSzUzSGs1ejlacHdwUlFZbjliK2JUdEkxM1JSNmtz?=
 =?utf-8?B?bDgvYnp1ZGpmUWxqWHV5MXRJK1I0aGQzeUVCMlFrdGE5UW5XNUxGY1FKbXVB?=
 =?utf-8?B?U1dKRFQzQ0RIMkRsTnk2KysrSzA3djZuQVRoaFlNS3h1Z0JuOWhjMGs1YzJI?=
 =?utf-8?B?M295YVVwNXIyRk8zZHdBNnBINVJBYzl3bGt3ZlZmZ2s5SHBwS3VrQW5Nempp?=
 =?utf-8?B?c0h1bXlLRmE4Y0ZvUGNxQm5vUUhkZkdReSs5eUZMYnh1Sk11bHVvRW9LS3pz?=
 =?utf-8?B?ZjNZWmk5UlB2T3I1VmdrTk5oT1M5TCtIY2pSVXJOMko1QzJ5TE41eFBMQlgv?=
 =?utf-8?B?aCtYOVk4aDVRV3JaZkhRTzR6Ri9YN29QWXlRYjRvUVRtTS9wWGhOQVBOblFU?=
 =?utf-8?B?SFFoMnVzMzE1VUJiVEg2QUdKV2REcFJhVzRWcE5yQTNyeXFIRFBESTBaM1NW?=
 =?utf-8?B?dkp0UFpKS3A2V1dBL1ZpWVkvRUlnUk16UURoUW96NC96WENwdkNMVkFRTGN6?=
 =?utf-8?B?UlpaeGlzVDE3b3FLcUhDYlU2ZjFSN243Y3dmN3FsanVsclI2NzNsdVY0cjRH?=
 =?utf-8?B?QTI3ZGMyWVFsL0RORk8vaTJKRzVydWdidlBaNitJMStsVGVhWWV3RG5pRVFa?=
 =?utf-8?B?bnZ2NlNGY093MlR5MEpkTW1reG5kd3N3cm9rSzBHSXA1clpidFdCMjFuVmJp?=
 =?utf-8?B?QlJ2UVZiQWUzSUd3YnZsVjNNN01mYmJDYkJXVEF0NHo0NmlMVmNwNGJtSWQv?=
 =?utf-8?B?bjVJUjNqYWFodVJHMTVROTUzK2FXSkJMWEJ0bW1XSzJ3WWwyQzc2M3ZnRThS?=
 =?utf-8?B?UUIwRVV6cEhsdGtyMWhyd2I5elRJSDZzUmF0TCt3MURpZTVncmdjS2k1a3BF?=
 =?utf-8?B?NXlyMWVlaUljZjdtNzliOWhvWk45YitpYll1MGVucWo2b043SmZ6bkdyZGFN?=
 =?utf-8?B?dUNJN2dEUXNmTWkwTzhtUlNDVlZiVkVQdjZRN2RZUm1tbS9ZVEhuTXpURGdJ?=
 =?utf-8?B?dFRSR2t1T2NhL09ma2J4SkRrSlhhZ3ZrRkZBdCtsMmFMYlQveGUyZmFLdEw2?=
 =?utf-8?B?ZkdQR2V4R1o0OUM3RUE1QTZlN0poeXoxeXI3SGZRQzl2VEdlQzIvRGxkMEo0?=
 =?utf-8?B?a2J6QVRZVVVRcWwvVlNVK2tkMXgyWXgwRmNZTmQ3VmVuYjgyR016UEtSSUJ6?=
 =?utf-8?B?QlpIWitSemNrR09RUlJJeldjdkxOU3RDUGgzVEo3cURFcFFaUFRJRkdndVNi?=
 =?utf-8?B?Z1o3NGttczZRMmdrMnNRdWdMT1ZiQlB5MHMvdGZXYjZwZVN5cHdSYVp4NW1S?=
 =?utf-8?B?K0FZTlNwV3c5RkNnQzB1MnlvbmNHWjdkRElsK2k2ekFwU3NzRkhFWFlEQWFi?=
 =?utf-8?B?ZFRROUdLWCtMMzQzOE9rZ3B5TVhmUW5odHlzVE03SFRjRmNpZ0JTYkE0S3FC?=
 =?utf-8?B?T3R6SHFDSkcvNGQzNlJMVk56emp6UXpwU0NpZ1Mwa2xkNDZoWlZmb3NQUGhz?=
 =?utf-8?B?Y2pwUmgrUmpnTCtKVlg0dHdwR0dueUpOYXo2QXVLT2MwZlVxZFkwZEczRVEz?=
 =?utf-8?B?NlRnOVk0dm5UeG5tL2pxYkxkQ2IvZXd5M09MSDZnRVJOeldJdXhRcTZqSG0w?=
 =?utf-8?B?ek03eGhscVZ2MTBiaVN3LzQ0Nkc4cW1VYUVCUDUxSVdrWitGZ3AyU1E5Rnhh?=
 =?utf-8?B?TW1mbDV5WFcyUEJTcmNwYkNyRzh2d3NQN01iN3JSQW9nM282NS81L1pKS0VI?=
 =?utf-8?B?SlB6UkF4cEVZcVphblJXM01GcTIweXEyM09FWE9oTFl6bElVYmQxNTNYRGNO?=
 =?utf-8?B?ZkppaE8rRVAyclJibWJKMThjSmVrT1daQm85OVdMUTRnMUhkOTFiUExpUnUy?=
 =?utf-8?B?bnRueUtQcmVYZHVsQytrOW5yY25TdzEwS2JXK28xNll5ckRoMjJVNTRFbFhx?=
 =?utf-8?B?K0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 55af4c9f-a5b6-4bd1-9e40-08dac42d373b
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3768.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Nov 2022 21:39:30.9166
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 1uoifqve0g6olhqhQ4Kpp4KdPi4immo3rf4rdZ8ud9jLpt2idZvJallScHi2by7lAt2e+8ZCoI/eqwI8BBCtrA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6747
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 11/11/2022 6:33 AM, Borislav Petkov wrote:
> On Mon, Nov 07, 2022 at 02:53:15PM -0800, Jithu Joseph wrote:
>> IFS test image carries the same microcode header as regular Intel
>> microcode blobs. Microcode blobs  use header version of 1,
>> whereas IFS test images will use header version of 2.
>>
>> microcode_sanity_check() can be used by IFS driver to perform
>> sanity check of the IFS test images too.
>>
>> Refactor header version as a parameter, move it to cpu/intel.c
>> and expose this function. Qualify the function name with intel.
> 
> Same comments as before.

Will drop the last para


>> +#define MICROCODE_HEADER_VER_UCODE	1
> 
> "MICROCODE" ... "UCODE" - too much.
> 
> And "header version" sounds wrong when all you wanna say is, this header
> is of this or that *type*. So you simply do:
> 
> #define MC_HEADER_TYPE_MICROCODE	1
> #define MC_HEADER_TYPE_IFS		2
> 
> and that's it.

Will modify

> 
>>  #define get_totalsize(mc) \
>>  	(((struct microcode_intel *)mc)->hdr.datasize ? \
>> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
>> index b6f9210fb31a..f8a5a25ab502 100644
>> --- a/arch/x86/kernel/cpu/intel.c
>> +++ b/arch/x86/kernel/cpu/intel.c
>> @@ -245,6 +245,106 @@ int intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
>>  }
>>  EXPORT_SYMBOL_GPL(intel_find_matching_signature);
>>  
>> +int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_ver)
> 
> This is not how this is done:
> 
> 1st patch: *only* mechanical code move, no semantic or functional changes
> whatsoever.
> 
> 2nd patch: Add semantical/functional changes.
> 
> Also, in the second patch, pls put a kernel-doc comment over the
> function to explain what hdr_type - not hdr_ver - means here.

Thanks for the suggestion above,  will split to 2 patches


Jithu