[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 12:37 PM, Aravind Gopalakrishnan wrote:
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.
The trouble is that if I have two files in my hands, both called
microcode_amd_fam15h.bin, I don't know which one is the more up-to-date
(would be nice to have a tool to print file info).
Of course, one can argue that in that case I shouldn't be using neither
but HW makes its own verification of patches so in principle trying both
should be safe (provided that SW tries not to load older revision).
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
