Re: [Xen-devel] pvops microcode support for AMD FAM >= 15

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.12.12 at 11:08, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> (Putting debian-kernel to bcc, since I don't imagine they are interested
> in the details of this discussion, I'll reraise the result with the
> Debian Xen maintainer when we have one)
> 
> On Wed, 2012-12-05 at 17:53 +0000, Ian Campbell wrote:
>> On Wed, 2012-12-05 at 17:27 +0000, Boris Ostrovsky wrote:
>> > On 12/05/2012 12:02 PM, Jan Beulich wrote:
>> > > But all of this shouldn't lead to equivalent ID mismatches, should
>> > > it? It ought to simply find nothing to update...
>> > 
>> > 
>> > The patch file (/lib/firmware/amd-ucode/microcode_amd_fam15h.bin) may 
>> > contain more than one patch. The driver goes over this file patch by 
>> > patch and tries to see whether to apply it.
>> > 
>> > I think what happened in Ian's case was that the patch file contained 
>> > two patches --- one for this processor (ID 6012) and another for a 
>> > different processor (ID 6101). (Both are family 15h but different revs).
>> > 
>> > The driver applied the first patch on core 0. Then, on core 1, the code 
>> > tried the first patch (at file offset 60) and noticed that it is already 
>> > applied. So it continued to the next patch (at offset 2660) which is not 
>> > meant for this processor, thus generating the "does not match" message.
> 
> OOI what would have happened if the two patches were in the opposite
> order? Would CPU0 have seen the ID 6101 patch first and aborted?

That would work well.

The problem is that cpu_request_microcode() returns the result
of its last call to microcode_fits(), no matter whether a prior one
already returned success (>= 0).

Something like

                                                   &offset)) == 0 )
     {
         error = microcode_fits(mc_amd, cpu);
-        if (error <= 0)
+        if (error < 0)
+            error = 0;
+        if (error == 0)
             continue;
 
         error = apply_microcode(cpu);

would apparently be needed. Or we could of course make
microcode_fits() return a bool_t in the first place.

But then again it would probably be nice to indeed return
failure from cpu_request_microcode() if _no_ suitable
microcode was found at all. Question is whether one blob
can contain more than one update for a given equivalent ID.
If not, bailing from the loop even if microcode_fits() returns
zero might be the right solution (and presumably the latter
then shouldn't return zero when no equivalent ID was
found).

But no matter what solution we pick, we need to review this
carefully in the context of the earlier regressions we had in
this area.

Jan


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