Re: [Xen-devel] [PATCH] xen/domctl: Drop vcpu_alloc_lock

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.09.18 at 21:15, <andrew.cooper3@xxxxxxxxxx> wrote:
> Since its introduction in c/s 8cbb5278e "x86/AMD: Add support for AMD's OSVW
> feature in guests", the OSVW data has been corrected to be per-domain rather
> than per-vcpu, and is initialised during XEN_DOMCTL_createdomain.
> 
> Furthermore, because XENPF_microcode_update uses hypercall continuations to
> move between CPUs, it drops the vcpu_alloc_lock mid update, meaning that it
> didn't provided the interlock guarantee that the OSVW patch was looking for in
> the first place.
> 
> This interlock serves no purpose, so take the opportunity to drop it and
> remove a global spinlock from the hypervisor.

The interlock didn't work as intended, I agree, but "serves no purpose"
is wrong imo. Rather than blindly dropping the logic, I'd have expected
for it to be fixed: Despite the movement into XEN_DOMCTL_createdomain
there's still a race between ucode updates and domain creation.

I see you've rushed the patch in (perhaps to avoid objections, given
that you've proposed this removal before, and I didn't really like it),
so I guess we need to take it from there now. I certainly agree that
runtime ucode updates aren't in good shape anyway, but I certainly
don't think making a bad situation worse really helps. In any event -
I would have expected you to give a little more time between patch
submission and patch committing, especially when the subject was
previously controversial, no matter that you've has the formally
necessary ack-s (which, if you had pointed out the previous
controversy, might not have been given that easily/quickly).

FAOD I'm not suggesting to revert the patch at this point in time.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel