Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Fri, 1 Dec 2023 12:33:34 -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: <ZWpDHgNjhQKLodF6@a4bf019067fa.jf.intel.com>
References: <20231115210212.9981-1-bp@alien8.de>
 <20231115210212.9981-3-bp@alien8.de>
 <ZWjVt5dNRjbcvlzR@a4bf019067fa.jf.intel.com>
 <20231201163928.GCZWoMQNAAQSA9U12D@fat_crate.local>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231201163928.GCZWoMQNAAQSA9U12D@fat_crate.local>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ILyGLq8N8nYVE+Ah/ebQo57goLyBQLko3kskYq4t+BMKn97dxqfdijEnGXFv?=
 =?us-ascii?Q?jLnom3xdY3dKMPHRLXnmjUxniT93syPOtbbjHKVJIeYaZPhZVA+vEQbkQWM6?=
 =?us-ascii?Q?spBCaxa+WOYvujHnsFn5XyB48ZWYuuten9sXqIMO7pthZscFACEzYM5TqYSn?=
 =?us-ascii?Q?PlkUaSkcvZdKHjAozToH/ijEJrlChfxalrhgz9c/qr0iv86emecMtP/B3mE8?=
 =?us-ascii?Q?Vf2HJbycsiSkIBLM3Adi9xhZIwpX3h8bVoMeAZxRC+J1AVoVge/8hPClBZKB?=
 =?us-ascii?Q?rfQ63EWyZ20y4q+hPDz/0f1pbf+UdkXUbcRyyCB8NM0wqZEpNcYBAfQmbY3b?=
 =?us-ascii?Q?xByjipsrkoZ1GS2J/uVafxp3zJl3i4tU3U5sl7QAY90Crb63fIZqAB2JrOXo?=
 =?us-ascii?Q?xf8g50915UniBzGuDo9x7MmJlCFxz73+1ROjCHqz9QSbwQbHQn+UeZgVN8qB?=
 =?us-ascii?Q?aW3gzarGBH0Zn8OL1A4bubNiLLomJaEeoNXTKqAl8zw8YY53ftYArYBDA+Uq?=
 =?us-ascii?Q?O16cfvhGv5mRIAG1F0GzDAA8kjApa8ytkjEthNNbAzUWS68q/KAWV90MKrRy?=
 =?us-ascii?Q?VinpTQRbhuHN9BZwoFKSP6Wjhm58VfqOZv+Jx1TUETEj/z6r2KH0vcLxuv9B?=
 =?us-ascii?Q?ClxIk7vzYKVPFVQyh/NsmxCQY5yNt32iPRXgfYMNrMkTEGyVsVEjH0hDNp8+?=
 =?us-ascii?Q?nvsW2gAvd3EHRoXpHyNh+dkIh4d4nuGGYACKksDHFadHCLK58OSRAygePDKk?=
 =?us-ascii?Q?bAcyo6L9KQa3ApPKKQ8rDR2Aj6xvIZ8wsv9VVE2L911NMs9OIxKpkqZQGv/4?=
 =?us-ascii?Q?ai6W7rWcZd0DOJO/BHimtsL29xzxQWYpfGRIH+DYpCBEjE4MGR8D7vypNk7q?=
 =?us-ascii?Q?zVZE2oZd7RdpAuwEnohCH3y7TJF7NIx9TO7KLuiNf/sd0WUwe0hu5RwjcyrD?=
 =?us-ascii?Q?YVYEnddtAZ0vbq+NTGywntk7NHxyHNKCPQN20qoOfLt/UVinwHUZOzTs/AtR?=
 =?us-ascii?Q?zW/9cGD9gTN75ENx3dFMqkms05yq9AjWSwKDWdZ/uSESb0W40eD+O7PzBoCb?=
 =?us-ascii?Q?r718LeSyNhomWg+pziUX4QadK3LLaiG7ygNIgkMB71Ea/l1YwGKo9cJG3UE7?=
 =?us-ascii?Q?t/ff8xRp5AKbTRBqUFdsdGtu2ZbyR0S2XrwjOQeqxLG/kXoDIrSF7tfoBGq2?=
 =?us-ascii?Q?xUM61d+WF2wHdAGsXHK2LjgtpjwmPgwCJIYHRuQdj85tv8jp+2m0uJ1UFnIE?=
 =?us-ascii?Q?k91leTaCFVCwXDHsUj1HBjHAkmIIgE+3DBb/r6D8DfkLpQZG2Q8AQxyx+FfM?=
 =?us-ascii?Q?JPwxOza7Ph7uiXJR37SYPACOT5clDKlg0iyQkw7g3fcOGnm4UKe0MVOwg807?=
 =?us-ascii?Q?qe5hniDKJY53g+ntHCADSCTf44scQEQerv+WEFQZq34ej1rB9Yz2Q1gHxDse?=
 =?us-ascii?Q?uO/AXLcz5iHX+/bjvbvo4zULzF2uIp3t12BFh7mx0l5xWcVYOnYmsVoWd4JA?=
 =?us-ascii?Q?mwiOxKnOV9accUEDab43HHcWIqi1F4TDjBwzlkwrIiakvtDmsSWH81KNc+QB?=
 =?us-ascii?Q?30/Eq20e+ZocEb7A66RZsq5Xd09c9HwELSLRVyqi?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c3233a5b-8e82-435e-deef-08dbf2accd9a
X-MS-Exchange-CrossTenant-AuthSource: SJ1PR11MB6201.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Dec 2023 20:33:40.4439
 (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: /OJ86Feyw5CRQ7wSo0DUq7Ond4sqTv2cWoDBzXWGHgwg7qkIGkuzihF+gcBY8NZdnq2XABpZZIrTpNCvTtPpVg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5583
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 pete.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 (pete.vger.email [0.0.0.0]); Fri, 01 Dec 2023 12:34:02 -0800 (PST)

On Fri, Dec 01, 2023 at 05:39:28PM +0100, Borislav Petkov wrote:
> On Thu, Nov 30, 2023 at 10:34:31AM -0800, Ashok Raj wrote:
> > new_rev is always assigned even if there was no microcode to apply.
> 
> That is wrong.
> 
> In my defence, I don't have an Intel machine which has up-to-date
> microcode in the BIOS and the blob has the same microcode revision. All
> my Intel test boxes do update microcode.
> 
> > - On AMD, the ucode is loaded even if the current revision matches what is
> >   being loaded.
> 
> From the commit message of the patch you're replying to:
> 
> "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."
> 
> The logic is, we want to dump before-after everytime there was
> a successful early upload.
> 
> > Currently, it's displayed as below:
> > 
> > [  113.395868] microcode: Current revision: 0x21000170
> > [  113.404244] microcode: Updated early from: 0x21000170
> 
> There's something else weird going on though. I would expect that that
> machine should not update microcode if it cannot find newer. Maybe the
> scan_microcode() logic is a bit weird still.
> 
> Please run this debug + fix patch and send me full dmesg from that
> machine.
> 

I'll get a dmesg shortly once i get my test system back.

What I meant was 

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;
}

ed->new_rev is always assigned, just not for the UCODE_UPDATED case. Hence
even if we had no microcode to update ed->new_rev ends up being non-zero.