Re: [PATCH v12 1/9] x86: add generic resource (e.g. MSR) access hypercall

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/07/14 08:45, Jan Beulich wrote:
>>>> On 15.07.14 at 12:00, <andrew.cooper3@citrix.com> wrote:
>> Hmm - I am very sorry for pushing at continue_hypercall_on_cpu(), but
>> looking at the code, there is no possible way it can be made to work in
>> its current form.  It will BUG() at the second attempt to nest, making
>> it inappropriate to use.
> Not sure why - look at microcode.c's use of it, which also makes
> use of it more than once for a single hypercall.
>
> Jan
>

Eugh.  That highlights several more issues.

Any two continue_hypercall_on_cpu()'s which target the same cpu may fall
over the BUG_ON(info->nest != 0).  A microcode update and this proposed
hypercall run a very real risk of intersecting.

The vcpu_pause_nosync(curr); in continue_hypercall_on_cpu() is bogus
given the do_microcode_update() example, as it will be pausing the wrong
vcpu.

As a result, the vcpu which made the hypercall will be unpaused after
the first continue_hypercall_on_cpu() returns, which is before the
hypercall has actually completed.

Irrespective of that, copying data back to guest context from the
continue_hypercall_tasklet is in the wrong vcpu context, and likely even
the wrong domain context.

Finally, we have seen in the past that using AMD SVM with mismatched
microcode patch levels can result in hardware lockup.  The only safe way
to apply microcode in these circumstances would be to rendezvous in Xen
and update every core at once, ensuring that no SVM operations are being
performed on sibling processors.

~Andrew