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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 21/03/18 05:46, Juergen Gross wrote:
> On 20/03/18 18:22, Andrew Cooper wrote:
>> 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.
> What about failing domain creation e.g. via -EAGAIN in case a
> microcode update happened in between? This would be easy by adding a
> microcode generation count which would have to be the same for start
> and end of the create domain hypercall.

Failing the hypercall is very complicated once domain_create() has
completed successfully.  See patch 11

(Although in writing this reply, I see that patch 11 is buggy).

Amongst other things, failing that late burns a domid, because there is
a period during which the domain has to live in the domain list.

I don't see the point of trying to retain a broken mechanism, especially
at the expense of error path complexity.

~Andrew

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