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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 04/05/2023 9:17 am, Jan Beulich wrote:
> On 04.05.2023 10:08, 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".
> Okay, with the case at least suitably mentioned
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks,

>> 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. 
> Right, but iirc we did expose it to guests earlier on. And I've used it as
> a known example only anyway. Who knows what vendors decide to make disappear
> the next time round ...

It's true that we used to expose the CPUID bit to guests, but IIRC we
never virtualised it correctly which was my justification for hiding the
feature bit when I was doing the original work to rationalise what
guests saw.

 
Removing LWP was an extraordinary situation, and AMD didn't do it lightly.

What they did was sacrifice a fairly expensive LWP errata workaround to
make space ucode space for IBPB instead.  They hid the CPUID bit (and
specifically by modifying the CPUID mask MSR only) because there were no
known production users of LWP, and a consequence of the patch, an errata
got unfixed.

On real AMD systems which used to enumerate LWP, and subsequently ceased
to, it was only "normal virt" levels of feature hiding.  The LWP CPUID
leaf still has real data in it, and you can still use %xcr0.lwp, and the
LWP MSRs still function.

Software which was genuinely using LWP, not tickling the errata case,
and late loaded this patch wouldn't actually have anything malfunction
underfoot.

>> Crashing on S3
> I may guess you meant to continue "... is bad anyway"?

Oops.  I was clearly in a rush for my morning meeting.  What I meant was
"we probably shouldn't crash on S3 resume in this known-ok case".

~Andrew