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

[Aravind Gopalakrishnan <aravind.gopalakrishnan@xxxxxxx>]

On 7/30/2015 12:01 PM, Andrew Cooper wrote:
> On 30/07/15 17:23, Aravind Gopalakrishnan wrote:
> > Some of older[Fam10h] systems require that the microcode versions
> > that it comes up with should not be updated by the microcode driver.
> > Otherwise, system hangs are known to occur.
> >
> > In this patch, we check for those microcode versions and abort the
> > update process if existing microcode level is already applied by
> > the BIOS.
> >
> > A linux version of the patch has already made it into tip-
> > http://marc.info/?l=linux-kernel&m=143703405627170
> >
> > Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@xxxxxxx>
> > ---
> >   xen/arch/x86/microcode_amd.c | 31 +++++++++++++++++++++++++++++++
> >   1 file changed, 31 insertions(+)
> >
> > diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
> > index f79b397..c958a47 100644
> > --- a/xen/arch/x86/microcode_amd.c
> > +++ b/xen/arch/x86/microcode_amd.c
> > @@ -347,6 +347,30 @@ static int container_fast_forward(const void *data, size_t 
> > size_left, size_t *of
> >       return 0;
> >   }
> >   
> Please include the same comment as the Linux patch, explaining that
> these microcode versions can't be updated from.
Ok, will do that.

> I would also like to see some documentation from AMD concerning this.
(hopefully) answering Boris' question too here-

So, the patch id values have only been obtained empirically.
The Linux patch provides the bug reference for 
this:https://bugzilla.suse.com/show_bug.cgi?id=913996
(It's a fairly long thread but the gist of it is that people 
predominantly seem to be experiencing system hang issues
when they try to update microcode levels from these patch ids: 
0x01000098,  0x0100009f, 0x010000af)
From discussing about it internally, we gathered that OS/hypervisor 
cannot reliably perform microcode updates beyond these specified levels
due to HW issues.

> > +static unsigned int final_levels[] = {
> > +    0x01000098,
> > +    0x0100009f,
> > +    0x010000af,
> > +    0
> > +};
> > +
> > +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);
> > +    int i;
> unsigned
Will fix this.

> > +
> > +    for ( i = 0; final_levels[i]; i++ )
> ARRAY_SIZE(), and drop the 0 on the end of the list.

Ok, will fix.

> > +        if ( uci->cpu_sig.rev == final_levels[i] )
> > +            return 1;
> > +
> > +    return 0;
> > +}
> > +
> >   static int cpu_request_microcode(int cpu, const void *buf, size_t bufsize)
> >   {
> >       struct microcode_amd *mc_amd, *mc_old;
> > @@ -369,6 +393,13 @@ static int cpu_request_microcode(int cpu, const void *buf, 
> > size_t bufsize)
> >           goto out;
> >       }
> >   
> > +    if ( check_final_patch_levels(cpu) )
> > +    {
> > +        pr_debug("microcode: Cannot update microcode patch on the cpu as we hit a 
> > final level\n");
> pr_debug() is compiled out completely.  I would suggest
> printk(XENLOG_INFO instead.
Ok, Will fix this and send out a V2 with the changes.

Thanks,
-Aravind.

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