From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aravind Gopalakrishnan Subject: Re: [PATCH V3] x86, amd_ucode: Support multiple container files appended together Date: Thu, 26 Jun 2014 11:37:52 -0500 Message-ID: <53AC4C60.7080206@amd.com> References: <53AC43EF.807@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53AC43EF.807@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky Cc: xen-devel@lists.xen.org, keir@xen.org, jbeulich@suse.com List-Id: xen-devel@lists.xenproject.org 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.