From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v4] x86/AMD: Add support for AMD's OSVW feature in guests Date: Fri, 3 Feb 2012 11:48:33 -0500 Message-ID: <4F2C0FE1.3030304@amd.com> References: <789bbf4f478b0e81d712.1328113814@wotan.osrc.amd.com> <4F2A9C1C0200007800070823@nat28.tlf.novell.com> <4F2AF21C.3090305@amd.com> <4F2BA08C0200007800070AC9@nat28.tlf.novell.com> <4F2C079E.6070404@amd.com> <4F2C18A40200007800071223@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4F2C18A40200007800071223@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: Christoph.Egger@amd.com, xen-devel@lists.xensource.com, keir@xen.org List-Id: xen-devel@lists.xenproject.org On 02/03/12 11:25, Jan Beulich wrote: >>>> On 03.02.12 at 17:13, Boris Ostrovsky wrote: >> On 02/03/12 02:53, Jan Beulich wrote: >>>>>> On 02.02.12 at 21:29, Boris Ostrovsky wrote: >>>> On 02/02/12 08:22, Jan Beulich wrote: >>>>> As I was about to apply this to my local tree to give it a try, I had >>>>> to realize that the microcode integration is still not correct: There >>>>> is (at least from an abstract perspective) no guarantee for >>>>> cpu_request_microcode() to be called on all CPUs, yet you want >>>>> svm_host_osvw_init() to be re-run on all of them. If you choose >>>>> to not deal with this in a formally correct way, it should be stated >>>>> so in a code comment (to lower the risk of surprises when someone >>>>> touches that code) that this is not possible in practice because >>>>> collect_cpu_info() won't currently fail for CPUs of interest. >>>> >>>> What if svm_host_osvw_init() is called from collect_cpu_info()? There >>>> may be cases when svm_host_osvw_init() is called multiple times for the >>>> same cpu but that should be harmless (and the routine will be renamed to >>>> svm_host_osvw_update()). >>> >>> Wouldn't that result in workaround bits that might get cleared with >>> the pending microcode update to get (and remain) set, as they're >>> being or-ed together over all invocations of the function after any >>> svm_host_osvw_reset()? >> >> >> I think that would be an OK but not optimal situation: more bits will >> end up being set than necessary, meaning that workarounds will need to >> be applied where they may not be required. But that should not affect >> correctness. I am not sure it's worth optimizing for this case since I >> think onlining a core while doing a microcode update is a rather >> uncommon occurrence. > > How is that related to CPU onlining? If you put the call into > collect_cpu_info(), that'll affect all CPUs currently online as well > (and, as I said, negate the effect of possibly cleared OSVW bits > that the pending microcode updated might have). The only case > where calling svm_host_osvw_init() from collect_cpu_info() would > be correct is if the latter returns an error. Which, as I noted > before, can only happen for non-AMD or pre-Fam10 CPUs (on > all of which calling the function is pointless). So we're back to me > asking that this simply be explained in a code comment in lieu of > a proper abstraction of the situation. OK, I misunderstood your original comment then. I thought you were talking about a race between microcode update and onlining a core. -boris