From mboxrd@z Thu Jan  1 00:00:00 1970
From: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
Subject: Re: [PATCH V3] x86,
 amd_ucode: Support multiple container files appended together
Date: Thu, 26 Jun 2014 10:03:43 -0500
Message-ID: <53AC364F.7060302@amd.com>
References: <1403724849-14360-1-git-send-email-aravind.gopalakrishnan@amd.com>
	<53AC1CC2.2070105@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <53AC1CC2.2070105@oracle.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: keir@xen.org, jbeulich@suse.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

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..
Then again, if it works fine right now, would we really want to change 
this behavior?

Thanks,
-Aravind.