>>> On 14.03.19 at 14:01, <chao....@intel.com> wrote: > On Wed, Mar 13, 2019 at 02:02:55AM -0600, Jan Beulich wrote: >>>>> On 13.03.19 at 08:54, <jbeul...@suse.com> wrote: >>>>>> On 13.03.19 at 06:02, <chao....@intel.com> 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@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
