From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web12.6760.1617800708477199324 for ; Wed, 07 Apr 2021 06:05:08 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Sic7pEFN; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1617800707; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xGQRL/DuDohJjg9cLjxV4u6s9Zwd5xRA87yExDcQKtI=; b=Sic7pEFNLmUB9cVuct+8agyRez4ehCMJANMBelVKBBhkiloIAz8kD7yCgJhmjtCwLoI8OB PEXN81CScTiq2jDUTsE/++ahMdaH+nUSZYUYoglg+gs+pwIKi5D8OY4mvkMbgffIA1vJ7A UEpkFeIM4tl65x7dhHghOmU0huo8kKA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-261-xFE6_ukbP_yajGk0sS9XQQ-1; Wed, 07 Apr 2021 09:05:02 -0400 X-MC-Unique: xFE6_ukbP_yajGk0sS9XQQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id EBAC818D6A22; Wed, 7 Apr 2021 13:05:00 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-38.ams2.redhat.com [10.36.112.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id 482B919C46; Wed, 7 Apr 2021 13:05:00 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 0/4] UefiCpuPkg: Add MicrocodeLib for loading microcode To: devel@edk2.groups.io, ray.ni@intel.com References: <10006.1617763383898351049@groups.io> From: "Laszlo Ersek" Message-ID: <879b535b-a744-8e0a-418b-62f8b7fd5402@redhat.com> Date: Wed, 7 Apr 2021 15:04:59 +0200 MIME-Version: 1.0 In-Reply-To: <10006.1617763383898351049@groups.io> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 04/07/21 04:43, Ni, Ray wrote: > On Tue, Apr 6, 2021 at 08:03 PM, Laszlo Ersek wrote: > >> >> (1) I think we should use a new TianoCore feature request BZ for this >> feature, and the commit messages should link it. (I understand the >> library only factors out existent logic, but still.) > > sure. https://bugzilla.tianocore.org/show_bug.cgi?id=3303 > >> >> (2) As I understand it, a platform can provide microcode in three ways: >> - via the "microcode patch" GUIDed HOB (PEI and DXE phases both), >> - via the "shadow microcode" PPI (PEI phase only), >> - via the PcdCpuMicrocodePatch* PCDs (PEI and DXE phases both). >> >> If a platform uses none of these methods (for example, OVMF does not), >> then I think it would benefit from a Null instance of the new >> MicrocodeLib class. >> >> Would you consider introducing a Null instance too, and using that one >> in the OVMF DSC files? >> > > No. I don't think it's necessary for a NULL instance. > Because: > 1. MicrocodePatch GUIDed HOB is only produced when "shadow microcode" PPI or "PcdCpuMicrocodePatch *" exists. > I will further simplify today's MpInitLib to skip loading microcode when this HOB exists (because DXE re-load is unnecessary). > This is captured by https://bugzilla.tianocore.org/show_bug.cgi?id=3155. > > 2. Today's logic only calls the MicrocodeLib API when "shadow microcode" PPI or "PcdCpuMicrocodePatch *" exists. > Even NULL instance is provided, it's not called. > > 3. MicrocodeLib calls ASSERT() when the supplied microcode binary is NULL. > If the logic in MpInitLib is changed by accident to allow the call to MicrocodeLib even in OVMF, the assertion can catch this. I was thinking that a Null instance would be useful for eliminating dead code from the binary (because: the PPI check is dynamic, unlike a compile-time PCD check, so it can not be optimized out). But it's not critical. Thanks Laszlo