Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Thu, 30 Nov 2023 10:34:31 -0800
From:   Ashok Raj <ashok.raj@intel.com>
To:     Borislav Petkov <bp@alien8.de>
CC:     X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
        "Linus Torvalds" <torvalds@linux-foundation.org>,
        Ashok Raj <ashok.raj@intel.com>
Subject: Re: [PATCH 2/2] x86/microcode: Rework early revisions reporting
Message-ID: <ZWjVt5dNRjbcvlzR@a4bf019067fa.jf.intel.com>
References: <20231115210212.9981-1-bp@alien8.de>
 <20231115210212.9981-3-bp@alien8.de>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231115210212.9981-3-bp@alien8.de>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?DuLe8OjvdM0mSmf4rdYHydamsrrbT+1dOu3IZBMWICIcqbj25TDznfyuznQX?=
 =?us-ascii?Q?Dflm6hBbL1OaFI7JeW89sBLQU3/xe6DwYNUL95Jzcsn3QlhF0MEMebzQvLZ6?=
 =?us-ascii?Q?xYpmta7Irt7WY2aUyC3Mz2G9IrR7nlVZQHTvSk34CoIwqnYS4jwl7GlYCrPE?=
 =?us-ascii?Q?lSlkeU8a5uJGhzfbcooV6RLZbOQmrww+nbDTlfYLwF3pMtC+DUylru7MBNXj?=
 =?us-ascii?Q?+X/yDo93zgG0nKOUQ9HHwK6kEZf/hmqOASU/DKJzP7nr03+uutT4/paSVGFA?=
 =?us-ascii?Q?+g7EHlD9/pCho8eaMip3anpydZFTlqoJERsN7qgzuzOaRmf9oVqK88xR6llD?=
 =?us-ascii?Q?m6RMiyukglUN7GnEsF30VT2G8ArbY4UyseWE1u4hdAxnCppyQNCI35llTHAi?=
 =?us-ascii?Q?EeCCgL48WVBiKKxEWkK6ZQ/2MiK+2BQfA8wFn6bid4ALp8cVyLBZ0Q4/gqji?=
 =?us-ascii?Q?ik0JDXCEo1N+c56oFXvHnuvyf/bS5YF2tw9K60kkTamaLBDwGP2YoyZ6xPEB?=
 =?us-ascii?Q?sL08zgqTzDAyf3uvIxf6IlEgrTBHV2DN8FrGl91Usl2gggn2imYBSryVsCLJ?=
 =?us-ascii?Q?3t1f39K7LO31JhK5s2TcALGXT54nNQXHvyMGqsPzGWzbWeD4vCoHn7KDejTB?=
 =?us-ascii?Q?dPma4CbI1xZpdOd9Qh3Cthca6mb4YPkmEvg/N0uKNdQm1x6KIfkvSJhoNXJD?=
 =?us-ascii?Q?KdsRMYWvY1KBpZi7dJi0iKDRaBaW/bapiTtzcw0cu3+42LlNzvPzV9/FjEJE?=
 =?us-ascii?Q?qUxecc7ULojE1b83TEt1w+iVywiDshswJB/sc/Gd0aN3ePbK8kgLo4ZEpNim?=
 =?us-ascii?Q?jyPaRzvZ5+Gwx6ZARrd4/Y7L69+xIEAhnHrwsw8NMrAKHsLIF7yOsvHTOvjx?=
 =?us-ascii?Q?XBThrP+Dldnd4lFvuRg4oFcdyuPuxFOrfyNtlJ/BY3K20zIr5T8TBgF62CWH?=
 =?us-ascii?Q?dGHzyf6nTetc/NdhCFC/Hr7a/HKCMzLZoVZZbTSoV47VQ2XZO3CBihWXPXk8?=
 =?us-ascii?Q?0qoOdWY4AYT7m+G0xLLAL0ShDGYMekkCr0CK0AV88adZDAvOd8Oo1PKf+Z37?=
 =?us-ascii?Q?IkcTKQfs3Xb6z7P0y1Yy2PVrWSo6XZs1dTd+HVBDzssj+CaxwEkIjQ/i7f8q?=
 =?us-ascii?Q?u+3gELadFxtDclQI6hQ+bbLzeoySapz5K9oOTjMt7sfhAvRlDO+8DEzwVGWC?=
 =?us-ascii?Q?Ozivu+UND3po2x3hEumBlGEmcXK7o1zu90LXM7S62W5tc4BehCc07VDnTNa/?=
 =?us-ascii?Q?eaN3UuENIGm6bhJTDkrwAjvOHp59RS6bSwFiQCDWBvB6feGu+Ao2IpnWO1/f?=
 =?us-ascii?Q?7PJNeHgIbN+3JO8in7j0j+YJtp9Z9XqR2lcReNjPR7d88K3wo4dMyfBEHxDx?=
 =?us-ascii?Q?vkCBz+8n8jMDy0RSWTnWiElDAHXrh35vvcX3156hnGjhrCG20MgzrKCo454j?=
 =?us-ascii?Q?9jN7zf0NERqFMTMQzuPkrqCVpN5p1KKKl2nBUuN/qmG5LnwsgTZPo4qi5gZ0?=
 =?us-ascii?Q?OAk/Hwb6dOVvqHJS3m00lL4nT1AmzRdRHBXm/UAhukaHmcKHEqAzJAdVYPKA?=
 =?us-ascii?Q?a5gci7rWWlRhwZMqQ8FkK8OR99+/9LpR/PWU5VhS?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ddbd28be-cf13-4d05-9ee1-08dbf1d302cd
X-MS-Exchange-CrossTenant-AuthSource: SJ1PR11MB6201.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2023 18:34:39.5472
 (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: XLFH+o62Y9vizlpESlbgutweNVXnVW13uuCeaKPsrSHRzeugc8LVB7sr7ENfCTSqyiNmF/GshAEam1imQwqPwQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR11MB8336
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Thu, 30 Nov 2023 10:35:09 -0800 (PST)

Hi Boris,

On Wed, Nov 15, 2023 at 10:02:12PM +0100, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <bp@alien8.de>
> 
> The AMD side of the loader issues the microcode revision for each
> logical thread on the system, which can become really noisy on huge
> machines. And doing that doesn't make a whole lot of sense - the
> microcode revision is already in /proc/cpuinfo.
> 
> So in case one is interested in the theoretical support of mixed silicon
> steppings on AMD, one can check there.
> 
> What is also missing on the AMD side - something which people have
> requested before - is showing the microcode revision the CPU had
> *before* the early update.
> 
> So abstract that up in the main code and have the BSP on each vendor
> provide those revision numbers.
> 
> Then, dump them only once on driver init.
> 
> On Intel, do not dump the patch date - it is not needed.
> 
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Link: https://lore.kernel.org/r/CAHk-=wg=%2B8rceshMkB4VnKxmRccVLtBLPBawnewZuuqyx5U=3A@mail.gmail.com
> ---
>  arch/x86/kernel/cpu/microcode/amd.c      | 39 +++++++-----------------
>  arch/x86/kernel/cpu/microcode/core.c     | 12 ++++++--
>  arch/x86/kernel/cpu/microcode/intel.c    | 17 +++++------
>  arch/x86/kernel/cpu/microcode/internal.h | 14 ++++++---
>  4 files changed, 38 insertions(+), 44 deletions(-)
> 

[snip]

>  void load_ucode_ap(void)
> @@ -826,6 +828,12 @@ static int __init microcode_init(void)
>  	if (!microcode_ops)
>  		return -ENODEV;
>  
> +	pr_info_once("Current revision: 0x%08x\n", (early_data.new_rev ?: early_data.old_rev));
> +
> +	if (early_data.new_rev)
> +		pr_info_once("Updated early from: 0x%08x\n",
> +			     early_data.old_rev);

See below, new_rev is always assigned. The above message appears even when
no new microcode was applied.

> +
>  	microcode_pdev = platform_device_register_simple("microcode", -1, NULL, 0);
>  	if (IS_ERR(microcode_pdev))
>  		return PTR_ERR(microcode_pdev);
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 6024feb98d29..070426b9895f 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -339,16 +339,9 @@ static enum ucode_state __apply_microcode(struct ucode_cpu_info *uci,
>  static enum ucode_state apply_microcode_early(struct ucode_cpu_info *uci)
>  {
>  	struct microcode_intel *mc = uci->mc;
> -	enum ucode_state ret;
> -	u32 cur_rev, date;
> +	u32 cur_rev;
>  
> -	ret = __apply_microcode(uci, mc, &cur_rev);
> -	if (ret == UCODE_UPDATED) {
> -		date = mc->hdr.date;
> -		pr_info_once("updated early: 0x%x -> 0x%x, date = %04x-%02x-%02x\n",
> -			     cur_rev, mc->hdr.rev, date & 0xffff, date >> 24, (date >> 16) & 0xff);
> -	}
> -	return ret;
> +	return __apply_microcode(uci, mc, &cur_rev);
>  }
>  
>  static __init bool load_builtin_intel_microcode(struct cpio_data *cp)
> @@ -413,13 +406,17 @@ static int __init save_builtin_microcode(void)
>  early_initcall(save_builtin_microcode);
>  
>  /* Load microcode on BSP from initrd or builtin blobs */
> -void __init load_ucode_intel_bsp(void)
> +void __init load_ucode_intel_bsp(struct early_load_data *ed)
>  {
>  	struct ucode_cpu_info uci;
>  
> +	ed->old_rev = intel_get_microcode_revision();
> +
>  	uci.mc = get_microcode_blob(&uci, false);
>  	if (uci.mc && apply_microcode_early(&uci) == UCODE_UPDATED)
>  		ucode_patch_va = UCODE_BSP_LOADED;
> +
> +	ed->new_rev = uci.cpu_sig.rev;

new_rev is always assigned even if there was no microcode to apply.

Feel free to squash this patch if it makes sense.

From: Ashok Raj <ashok.raj@intel.com>
Date: Tue, 28 Nov 2023 15:30:31 -0800
Subject: [PATCH] x86/microcode: Suppress early load message when the revision
 is unchanged

After early loading, early_data.old_rev is always updated, that results in
printing a message even if the revision is unchanged.

Currently, it's displayed as below:

[  113.395868] microcode: Current revision: 0x21000170
[  113.404244] microcode: Updated early from: 0x21000170

This should happen on both AMD and Intel. Although for different reasons.

- On AMD, the ucode is loaded even if the current revision matches what is
  being loaded.
- On Intel, load_ucode_intel_bsp() assigns new_rev unconditionally. So it's
  never 0.

Suppress the "Updated early" message when revision is unchanged.

Fixes: 080990aa3344 ("x86/microcode: Rework early revisions reporting")
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
---
 arch/x86/kernel/cpu/microcode/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 232026a239a6..18e61ecab005 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -830,7 +830,7 @@ static int __init microcode_init(void)
 
 	pr_info_once("Current revision: 0x%08x\n", (early_data.new_rev ?: early_data.old_rev));
 
-	if (early_data.new_rev)
+	if (early_data.new_rev && early_data.new_rev != early_data.old_rev)
 		pr_info_once("Updated early from: 0x%08x\n", early_data.old_rev);
 
 	microcode_pdev = platform_device_register_simple("microcode", -1, NULL, 0);
-- 
2.39.2