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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan


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