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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/03/18 16:58, Jan Beulich wrote:
>>>> On 19.03.18 at 20:13, <andrew.cooper3@xxxxxxxxxx> wrote:
>> It is not entirely clear why this interlock was introduced in c/s 8cbb5278e
>> "x86/AMD: Add support for AMD's OSVW feature in guests".
>>
>> At the time, svm_handle_osvw() could have seen an unexpected change in OSVW
>> (not the case now, due to the new CPUID Policy infrastructure), but even 
>> then,
>> it would have caused spurious changes in behaviour when handling
>> OSVW_{ID_LENGTH,STATUS} read requests on behalf of an already-running guest.
>>
>> There are plenty of other aspects of domain creation which depend on hardware
>> details which may change across a microcode load, but where not protected by
>> this interlock.
> Are there? We don't re-read CPUID (yet), for example. But of
> course it is also not really specified which aspects may change
> across microcode updates.
>
>> A host administrator choosing to perform late microcode loading has plenty of
>> other problems to worry about, and is it not unreasonable to expect them to
>> temporarily cease domain construction activities while the microcode loading
>> is in progress.
> But it is also not unreasonable to expect the hypervisor to guard
> against inconsistencies here. On the whole I'm not really
> convinced; I think I'd like to hear others' opinions.

The underlying problem is that this lock cannot say when merging
max_cpus into createdomain, because we cannot continue the hypercall
midway through.

As it doesn't currently protect createdomain, which amongst other things
contains init_domain_cpuid_policy() and init_domain_msr_policy() (the
most likely structures to be affected by microcode updates), I don't see
any purpose in keeping it for the minute area it does cover.

i.e. the behaviour is currently broken, and removing the lock doesn't
make it meaningfully more broken, but does let us resolve a whole class
of issues relating to partially-constructed domain objects.

Making things work consistently/sensibly after a microcode update is far
more complicated than just this, and strictly does require administrator
input because we can't go blindly substituting guest visible state.

~Andrew

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