[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86, amd_ucode: Support multiple container files appended together

On 6/23/2014 1:33 AM, Jan Beulich wrote:
On 20.06.14 at 18:06, <aravind.gopalakrishnan@xxxxxxx> wrote:
On 6/20/2014 3:11 AM, Jan Beulich wrote:
On 19.06.14 at 17:14, <aravind.gopalakrishnan@xxxxxxx> wrote:
@@ -379,8 +457,16 @@ static int cpu_request_microcode(int cpu, const void *buf, 
size_t bufsize)
           save_error = get_ucode_from_buffer_amd(
               mc_amd, buf, bufsize, &applied_offset);
+ /*
+         * If there happens to be multiple container files and if patch
+         * update succeeded on first container itself, a stale error val
+         * is returned from get_ucode_from_buffer_amd. So, force
+         * error = 0 here as we have already succeeded in the update.
+         */
           if ( save_error )
               error = save_error;
+        else
+            error = 0;
First of all I don't think this change is logically a part of $subject (as
it also alters behavior for single container files). With that the
question then is why the change is really correct. And if it was really
correct, you could just drop the if() instead.

The change is needed (as stated in the comments) because we end up
returning err val (EINVAL) like so-
DEBUG: cpu_request_microcode :  no save_err;error=-22

even though update succeeded-
(XEN) microcode: CPU0 found a matching microcode update with version
0x600063d (current=0x6000626)
(XEN) microcode: CPU0 updated from revision 0x6000626 to 0x600063d

That said, I also understand your point that we can just drop the if().
So, I'll change this as per your suggestion.

The fact that we have to make this change as a _consequence_ of adding
multiple container file support,
might explain why it was done later right?
I can make this a separate patch as well. Let me know what you think.
I think I follow what you're saying, in which case this clearly shouldn't
be a separate patch. But behavior for single-container files still should
remain unaltered, so some change other than folding the if and else
branches is going to be needed afaict.

I have addressed your concern in a V2.


Xen-devel mailing list



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