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

Re: [Xen-devel] [PATCH v4] x86/AMD: Add support for AMD's OSVW feature in guests

On 02/03/12 11:25, Jan Beulich wrote:
On 03.02.12 at 17:13, Boris Ostrovsky<boris.ostrovsky@xxxxxxx>  wrote:
On 02/03/12 02:53, Jan Beulich wrote:
On 02.02.12 at 21:29, Boris Ostrovsky<boris.ostrovsky@xxxxxxx>   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

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

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.


Xen-devel mailing list



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