Re: [Xen-devel] [PATCH v6 11/12] x86/microcode: Synchronize late microcode loading

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.03.19 at 14:01, <chao.gao@xxxxxxxxx> wrote:
> On Wed, Mar 13, 2019 at 02:02:55AM -0600, Jan Beulich wrote:
>>>>> On 13.03.19 at 08:54, <JBeulich@xxxxxxxx> wrote:
>>>>>> On 13.03.19 at 06:02, <chao.gao@xxxxxxxxx> wrote:
>>>> On Tue, Mar 12, 2019 at 05:07:51PM -0700, Raj, Ashok wrote:
>>>>>On Mon, Mar 11, 2019 at 03:57:35PM +0800, Chao Gao wrote:
>>>>>> +    if ( cpu == cpumask_first(per_cpu(cpu_sibling_mask, cpu)) )
>>>>>> +        ret = microcode_update_cpu();
>>>>>
>>>>>Does ret have any useful things on if the update failed? Doesn't seem 
>>>>>to be used before you overwrite later in collect_cpu_info()?
>>>> 
>>>> It has the reason of failure on error. Actally, there are two reasons:
>>>> one is no patch of newer revision, the other is we tried to update but
>>>> the microcode revision didn't change. I can check this return value and
>>>> print more informative message to admin. And furthermore, for the
>>>> latter, we can remove the ucode patch from caches to address Roger's
>>>> concern expressed in comments to patch 4 & 5.
>>> 
>>> Btw, I'm not sure removing such ucode from the cache is appropriate:
>>> It may well apply elsewhere, unless there's a clear indication that the
>>> blob is broken.
> 
> Yes. Got it. Can we just assume we won't encounter that ucode update
> succeeded only on part of cpus and warn a reboot is needed if it happened?
> We definitely want to tolerate some kinds of hardware misbehavior. But
> for such cases which are unlikely to happen, I prefer to improve this code
> when we meet this kind of issue.

Okay, for both this and ...

>>> So perhaps there needs to be special casing of -EIO,
>>> which gets returned when the ucode rev reported by the CPU after
>>> the update does not match expectations.
>>
>>An to go one step further, perhaps we should also store more than
>>just the newest variant for a given pf. If the newest fails to apply
>>but there is another one newer than what's on a CPU, updating to
>>that may work, and once that intermediate update worked, the
>>update to the newest version may then work too.

... see also Andrew's reply. As long as no-one is aware of mixed-
stepping systems out there in the wild, I'm fine with simplifying
things where possible.

> Intel SDM doesn't mention this dependency (to apply an ucode relies on a
> specific old ucode applied). Perhaps we can also assume we won't fall
> into this case.

Please see Linux'es
arch/x86/kernel/cpu/microcode/intel.c:is_blacklisted()
for one of the possible problematic situations here. Of course
the SDM wouldn't mention it, only the errata document for this
specific model would.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel