From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aravind Gopalakrishnan Subject: Re: [PATCH] x86, amd_ucode: Support multiple container files appended together Date: Mon, 23 Jun 2014 15:24:49 -0500 Message-ID: <53A88D11.4060601@amd.com> References: <1403190841-22892-1-git-send-email-aravind.gopalakrishnan@amd.com> <53A408B8020000780001BE1A@mail.emea.novell.com> <53A45C19.2000900@amd.com> <53A7E640020000780001C43A@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53A7E640020000780001C43A@mail.emea.novell.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: Jan Beulich Cc: keir@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 6/23/2014 1:33 AM, Jan Beulich wrote: >>>> On 20.06.14 at 18:06, wrote: >> On 6/20/2014 3:11 AM, Jan Beulich wrote: >>>>>> On 19.06.14 at 17:14, 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. Thanks, -Aravind.