[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V3] x86, amd_ucode: Support multiple container files appended together

On 06/26/2014 11:03 AM, Aravind Gopalakrishnan wrote:
On 6/26/2014 8:14 AM, Boris Ostrovsky wrote:

+ÂÂÂ /*
+ÂÂÂÂ * Multiple container file support:
+ÂÂÂÂ * 1. check if this container file has equiv_cpu_id match
+ÂÂÂÂ * 2. If not, fast-fwd to next container file
+ÂÂÂÂ */
+ÂÂÂ while ( offset < bufsize )
+ÂÂÂ {
+ÂÂÂÂÂÂÂ error = install_equiv_cpu_table(mc_amd, buf, &offset);
+ÂÂÂÂÂÂÂ if ( !error &&
+ÂÂÂÂÂÂÂÂÂÂÂÂ find_equiv_cpu_id(mc_amd->equiv_cpu_table, current_cpu_id,
+ÂÂÂÂÂÂÂÂ * Could happen as we advance 'offset' early
+ÂÂÂÂÂÂÂÂ * in install_equiv_cpu_table
+ÂÂÂÂÂÂÂ if ( offset > bufsize )
+ÂÂÂÂÂÂÂÂÂÂÂ printk(KERN_ERR "microcode: Microcode buffer overrun\n");
+ÂÂÂÂÂÂÂ error = container_fast_forward(buf, bufsize - offset, &offset);
+ÂÂÂÂÂÂÂ if ( error )
+ÂÂÂÂÂÂÂÂÂÂÂ printk(KERN_ERR "microcode: CPU%d incorrect or corrupt container file\n"
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "microcode: Failed to update patch level. "
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "Current lvl:%#x\n", cpu, uci->cpu_sig.rev);
+ÂÂÂÂÂÂÂÂÂÂÂ goto out;
+ÂÂÂ }

I just realized that I don't understand something: if you have two merged container files, both having an entry for current processor in the equivalence table but only the second file having the actual patch (or at least the patch with higher version), will we get to load that patch?

We will not come across such a situation:
One container has patch files for families 10h,11h,12h,14h and the other has patches for 15h.
So there will be an equiv_cpu_id match in (at least and) only *one* of the containers.
(If there ever are patches for 16h, then I suspect it will have a separate container of it's own. So there will not be any clashes with older containers)

Besides, the patches themselves are not incremental, i.e;
If there is a newer patch, then it handles all errata/bugs that were handled by the previous patch and then some.
Which means, you will not have a container that has two patches for the same processor.
So, *strictly speaking*, even the loop to get_ucode_from_buffer_amd() till the end of buffer is not necessary once we
have already applied a patch.

But, to change this behavior now will need more logic rework..

I don't think there is a whole lot of work/testing (especially since I am not the one doing it ;-)) --- you just need to skip header and equivalence table of the next container.

if ( mpbuf->type != UCODE_UCODE_TYPE )
    if ( *(const uint32_t *)buf == UCODE_MAGIC )
        struct mpbhdr *mpbuf = (const struct mpbhdr *)&buf[2]; // or [1]?
        *offset += mpbuf->len + CONT_HDR_SIZE + 4;  // I think 4, to skip MAGIC 
                                                    // and next word.
        // Now we've fast-forwarded past header and eq. table
        return 0; // or, goto beginning?

Will that work?
Then again, if it works fine right now, would we really want to change this behavior?


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.