Re: [PATCH] x86/ucode: Refresh raw CPU policy after microcode load

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: "Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Cc: Wei Liu <wl@xen.org>, Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] x86/ucode: Refresh raw CPU policy after microcode load
Date: Thu, 4 May 2023 10:28:32 +0100	[thread overview]
Message-ID: <575505d8-b6bd-be14-a70c-547d4229ad90@citrix.com> (raw)
In-Reply-To: <7e159a5b-b7be-c582-7300-154cec7a8e91@suse.com>

On 04/05/2023 10:07 am, Jan Beulich wrote:
> On 04.05.2023 10:22, Roger Pau Monné wrote:
>> On Thu, May 04, 2023 at 09:08:02AM +0100, Andrew Cooper wrote:
>>> On 04/05/2023 8:08 am, Jan Beulich wrote:
>>>> On 03.05.2023 20:58, Andrew Cooper wrote:
>>>>> Loading microcode can cause new features to appear.
>>>> Or disappear (LWP)? While I don't think we want to panic() in this
>>>> case (we do on the S3 resume path when recheck_cpu_features() fails
>>>> on the BSP), it would seem to me that we want to go a step further
>>>> than you do and at least warn when a feature went amiss. Or is that
>>>> too much of a scope-creeping request right here?
>>> You're correct that I ought to discuss the disappear case.  But like
>>> livepatching, it's firmly in the realm of "the end user takes
>>> responsibility for trying this in their test system before running it in
>>> production".
>>>
>>> For LWP specifically, we ought to explicitly permit its disappearance in
>>> recheck_cpu_features(), because this specific example is known to exist,
>>> and known safe as Xen never used or virtualised LWP functionality. 
>>> Crashing on S3
>> Printing disappearing features might be a nice bonus, but could be
>> done from the tool that loads the microcode itself, no need to do such
>> work in the hypervisor IMO.

Xen is really the only entity in a position to judge when stuff has gone
away, so this can't really be deferred to another tool.

We have the X86_FEATURE_* names during boot for parsing the
{dom0-}cpuid= command line options, but we don't keep this beyond init

> Except that the system may not last long enough for the (or any) tool
> to actually make it to producing such output, let alone any human
> actually observing it (when that output isn't necessarily going to
> some remote system).

Yeah, `xl dmesg`/serial/whatever is the one place liable for anything
like this to get noticed.

~Andrew