[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 6/26/2014 11:01 AM, Boris Ostrovsky wrote:

            +
            +        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]?
Neither will work as we will need to first advance buf 4 bytes.. (but I get your point)
Regardless, this is not necessary.
Let me clarify (below)
         *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?

But we don't actually need to parse the second container for reasons given in the previous thread.

Example scenarios-
1. Order of containers in initrd: f15h container, container for remaining families.
  - equiv_cpu_id match for first container (surely you are on a f15h model)
    - parse this container, find correct patch and try to apply
- No need to parse second container as it will not have patches for f15h. - no match for 1st container, but match on 2nd, then current processor is in set [f10h,f14h]
    - parse this container, find correct patch and try to apply
  - no match on both.
   - borked containers. So, abort.

2. Order of containers: container for families from f10h-f14h; container for f15h - equiv_cpu_id match for 1st container (surely you are on some family in the set of [f10h, f14h])
    - Parse container, find patch, try to apply
- No need to parse second container as it will not have patches for f10h-f14h
 - found match on 2nd, then current processor is a f15h model,
    - parse container, find patch, try to apply
 - no match
    - borked container. So abort.

The process(,scripts) to create containers ensure that we only have patches for f10h-f14h on 'microcode_amd.bin' and patches for f15h models (could be different model/stepping values) on microcode_amd_fam15h.bin

So it is safe to assume that we we only have a matching equiv_id in only one of the containers.

-Aravind.


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


 


Rackspace

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