Date:   Wed, 1 Feb 2023 18:51:44 -0800
From:   Ashok Raj <ashok.raj@intel.com>
To:     Borislav Petkov <bp@alien8.de>
CC:     Dave Hansen <dave.hansen@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Tony Luck <tony.luck@intel.com>,
        Alison Schofield <alison.schofield@intel.com>,
        Reinette Chatre <reinette.chatre@intel.com>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Stefan Talpalaru <stefantalpalaru@yahoo.com>,
        David Woodhouse <dwmw2@infradead.org>,
        "Benjamin Herrenschmidt" <benh@kernel.crashing.org>,
        Jonathan Corbet <corbet@lwn.net>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Peter Zilstra <peterz@infradead.org>,
        Andy Lutomirski <luto@kernel.org>,
        Andrew Cooper <Andrew.Cooper3@citrix.com>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        Martin Pohlack <mpohlack@amazon.de>,
        Ashok Raj <ashok.raj@intel.com>
Subject: Re: [Patch v3 Part2 4/9] x86/microcode: Do not call
 apply_microcode() on sibling threads
Message-ID: <Y9skuAuxozZLDrGB@a4bf019067fa.jf.intel.com>
References: <20230130213955.6046-1-ashok.raj@intel.com>
 <20230130213955.6046-5-ashok.raj@intel.com>
 <bfd85699-4b7a-c606-266a-cea7ff336950@intel.com>
 <Y9rqeqMjDOYJKmLZ@zn.tnic>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Y9rqeqMjDOYJKmLZ@zn.tnic>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?CXNaOFUFstbNeCbE0A2CL/ogikFfH9Ve0W65U7SO5oquw41YzQbAMCFudRr1?=
 =?us-ascii?Q?8k97gV40SDUvZAdrutxxcTBfEifJ7zPTDVJzf0Lzm25dNkGGB35KYM7XvEP4?=
 =?us-ascii?Q?6QW0/nguTSdwp5hfGb76cLvsmtQXzW7hB6MDwl0Eg1mfA3bccJqKe3XoBhQq?=
 =?us-ascii?Q?LI7sRCigNitzosv2TF3NHr7uY6gZUQwDOE/ljNJZfWV7G5h5qu5MZVZJbuQl?=
 =?us-ascii?Q?Z+DpRbhpGeBeGaf1Qe+bNrDZe7K3Nyz71CHKeUeglolgSH/43ficzj/DxGSj?=
 =?us-ascii?Q?8caC2rX/Eg3V9DXrt9YJv0PoGXY56xf9xsxbnRa/j++7udZAI6LugBkTYxTR?=
 =?us-ascii?Q?3nUJQ5B/Z/QjbTyCorsyPHLoWgc0HovMUD/iDvbSXyv3blWrgkc6vvxZR+DV?=
 =?us-ascii?Q?WQ58ZxW72EdeLo01n1264Wz08LABmmiqWX/ge2WLWFeMq6wQAms8VcOtsWiL?=
 =?us-ascii?Q?DC+vtzEBirAccdEI17CuZNBo3Qg7WdQSMdJlzi2Jbi72kZHILEG8GFVP1+Ar?=
 =?us-ascii?Q?s8IbPF5utI2f/feJimUiPVJKs9hCzzc7SRRSuvwp63VR2Cer3q0Aw3tP9/SL?=
 =?us-ascii?Q?qgQlhjYYAvD0ZOTvf62EUcDh2oSBeKXDA0AJa/ytfqekCGqV94LUxQC4jga7?=
 =?us-ascii?Q?kAkkVllf8GfQNsDQ+0MbfK1ve1Q+X1QYgyUaYEhvslWwC21+YfM/7zgcpPJ3?=
 =?us-ascii?Q?RvWNjkYGQFnYLgkc9GlG0Kld4yHU6AQB69/CfR+GZtZcUWC5n/lMNR1STuqx?=
 =?us-ascii?Q?NCxXtSMCKgeb8Gum7sdWm7QdENAYAkIL4SFWFj8yE7pZ9Ziku+v9T271hrav?=
 =?us-ascii?Q?W0Ty4ZdD93DpbJC291AW801209/o7EVRVUR5AMzU3kqAWlCvGVK/qomAeDG6?=
 =?us-ascii?Q?BnUKKg7McVRvAXvv8RX95/F5OoSWeJB5tovCbSnuTJr0er5vU/L2JrN7bkOL?=
 =?us-ascii?Q?KK8hID9qK33rrGGIs9NNHI2bZ0UJzNdbCUGsAo9AjeorS+mRoVo9qZMZrnf1?=
 =?us-ascii?Q?07a8I28sqVDFgC9cgMxZXTTLEJGGFrSUOvw1+P/eW6Hpl/cDxxRCRgjFWhax?=
 =?us-ascii?Q?ZKXa3xRotPTHRvfb1z+mchsmVk4GLssaWpVhvksX6wvnwSsiGrXaGnIjODoz?=
 =?us-ascii?Q?DJx3F60eRGzF6S4F08Y7DfmW8+kvJtEuuAKlR3JTzQE5BBwo/xcaav53uHTg?=
 =?us-ascii?Q?oZGxqStXJqOFymZFsUOS4ie84bLTT8VUQiu9vRdWpUBT/1wFWbPA4MlQbvE7?=
 =?us-ascii?Q?h7wDHNfLkJQivdKDJeT79WF0x/8dGilz7bId8rHCJoJuNjlO4UsY/61NjtFN?=
 =?us-ascii?Q?yKjGc54l+nUoC3fvk1mVBQ07cc3zL9RYVTeqJke7V7Q5v3REf81UaJLcw1og?=
 =?us-ascii?Q?DuP7X4soRI2wbzZqnhNMh2G6Auzv6wUK22QI6R4OVohcrPawB4hzBg0X3VSN?=
 =?us-ascii?Q?Iw9GTArw39mIA0t4m/yVAfU+dOzYCWWHtWAVbgWaJ1Rc5mJhJ+k5Lb46S+/n?=
 =?us-ascii?Q?4Yb9U2BEWgGvtAu+zLPnPENA3jDJCmOHTOu8lM5Q3NLPmEoc8GSMb3XV3ZpZ?=
 =?us-ascii?Q?x7qk/tFxo2UkThridihbNLDwM/WxFbi2xix7o9qOBJFwqGgBZqo7R2WwzPlz?=
 =?us-ascii?Q?Vg=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e283214c-f04d-4f80-ac02-08db04c87e90
X-MS-Exchange-CrossTenant-AuthSource: SJ1PR11MB6201.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2023 02:52:16.8937
 (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: ksHh0o0we6rnzoickUHMBmLBq64x05XUW+WNp1KD8S+w/2u9Wy3QYWg+fL2SNIW9maGAU3ArWjb+Nktw58vlhw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB8028
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Feb 01, 2023 at 11:40:58PM +0100, Borislav Petkov wrote:
> On Wed, Feb 01, 2023 at 02:21:18PM -0800, Dave Hansen wrote:
> > That works great, unless T0 experiences an error.  In that case, T0 will
> > jump out of __reload_late() after failing to do the update.  T1 will
> > come bumbling along after it and will enter ->apply_microcode(),
> > blissfully unaware of T0's failure.  T1 will assume that it is supposed
> > to do T0's job, noting "rev < mc->hdr.rev".  T1 will write the MSR while
> > T0 is off doing god knows what.
> > 
> > T1 should not even be attempting to do ->apply_microcode() because T0 is
> > not quiescent.
> 
> Yah, thanks for explaining properly.
> 
> So, if T0 fails, then we will say that it failed. The ->apply_microcode()
> call on T1 was never meant to apply any microcode - just to update the
> cached data.

The commit log "attempted" to convey that, replace primary with T0, and
secondary with T1.

> 
> Now, if T0 fails, then it doesn't matter what T1 does - you have a
> bigger problem:
> 
> A subset of the cores is running with new microcode while other subset
> with the old one. Now this is a shit situation I don't want to be in.
> 
> And I don't have a good way out of it.

T1 shouldn't be sent down the apply_microcode() path, but instead
just gather and update the per-cpu info reflecting the current revision.

Patch 3 and 4 attempted to that. 

In addition....to ensure cores being out of sync within themselves

At wait_for_siblings: Each thread can check their rev against the BSP (yes
BSP can be removed, but we can elect a core leader) and if they are
different we can either warn/taint or pull the plug and panic.


> 
> Revert to the old patch? Maybe...
> 
> Retry to application on all again with the hope that it works this time?
> 
> What if some core touches a MSR being added with the new microcode
> patch?
> 
> Late loading is a big PITA. As we've been preaching for a while now.
> 

Cheers,
Ashok