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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3653 bytes --]

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,
            +                               &equiv_cpu_id) )
            +                break;
            +
            +        /*
            +         * Could happen as we advance 'offset' early
            +         * in install_equiv_cpu_table
            +         */
            +        if ( offset > bufsize )
            +        {
            +            printk(KERN_ERR "microcode: Microcode buffer
            overrun\n");
            +            return -EINVAL;
            +        }
            +
            +        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?

    Thanks,
    -Aravind.



[-- Attachment #1.2: Type: text/html, Size: 5347 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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