Re: [Xen-devel] [PATCH V2] x86, amd_ucode: Skip microcode updates for final levels

[Aravind Gopalakrishnan <aravind.gopalakrishnan@xxxxxxx>]

On 8/3/2015 12:22 PM, Boris Ostrovsky wrote:
> On 08/03/2015 12:35 PM, Aravind Gopalakrishnan wrote:
> > Some of older[Fam10h] systems require that certain number of
> > applied microcode patch levels should not be overwritten by
> > the microcode loader. Otherwise, system hangs are known to occur.
> >
> > The 'final_levels' of patch ids have been obtained empirically.
> > Refer bug https://bugzilla.suse.com/show_bug.cgi?id=913996
> > for details of the issue.
> >
> > The short version is that people have predominantly noticed
> > system hang issues when trying to update microcode levels
> > beyond the patch IDs below.
> > [0x01000098, 0x0100009f, 0x010000af]
> >
> >
> >
> >   +/*
> > + * The 'final_levels' of patch ids have been obtained empirically.
> > + * Refer bug https://bugzilla.suse.com/show_bug.cgi?id=913996
> > + * for details of the issue. The short version is that people
> > + * have predominantly noticed system hang issues when trying to
> > + * update microcode levels beyond the patch IDs below.
> > + * From internal discussions, we gathered that OS/hypervisor
> > + * cannot reliably perform microcode updates beyond these levels
> > + * due to hardware issues. Therefore, we need to abort microcode
> > + * update process if we hit any of these levels.
> > + */
> > +static unsigned int final_levels[] = {
> > +    0x01000098,
> > +    0x0100009f,
> > +    0x010000af
> > +};
> > +
> > +static bool_t check_final_patch_levels(int cpu)
> > +{
> > +    /*
> > +     * Check the current patch levels on the cpu. If they are equal to
> > +     * any of the 'final_levels', then we should not update the 
> > microcode
> > +     * patch on the cpu as system will hang otherwise.
> > +     */
> > +    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
> > +    unsigned int i;
> > +
> > +    for ( i = 0; i < ARRAY_SIZE(final_levels); i++ )
> > +        if ( uci->cpu_sig.rev == final_levels[i] )
> > +            return 1;
> (I should have asked this when you posted v1, sorry)
>
> Are these final_levels[] fam10h-specific? Are we guaranteed that other 
> families won't have them? I think family check would be good here. Or 
> at least a comment stating that these levels can only be observed on 
> family 10h (the only mention of family right now is in the commit 
> message)

Yep. These are Fam10h specific microcode patches. And yeah, the HW 
problem seems to exist only on these Fam10h specific processor variants. 
So, it's pretty isolated.
And why need a family check? The microcode patches are tied to a 
specific family/model anyway right?
It's only ever going to apply successfully if the equivalent id's match..

And sure, I can massage the comment like so-
" ... The short version is that people who are using certain Fam10h 
systems are noticing system hang issues when tryingupdate microcode..."
Would that be ok?

Thanks,
-Aravind.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel