Re: [PATCH v3 12/12] xen/domain: Allocate d->vcpu[] in domain_create()

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

On 30/08/18 20:46, Julien Grall wrote:
> Hi Andrew,
>
> On 08/29/2018 03:40 PM, Andrew Cooper wrote:
>> For ARM, the call to arch_domain_create() needs to have completed before
>> domain_max_vcpus() will return the correct upper bound.
>>
>> For each arch's dom0's, drop the temporary max_vcpus parameter, and
>> allocation
>> of dom0->vcpu.
>>
>> With d->max_vcpus now correctly configured before evtchn_init(), the
>> poll mask
>> can be constructed suitably for the domain, rather than for the
>> worst-case
>> setting.
>>
>> Due to the evtchn_init() fixes, it no longer calls
>> domain_max_vcpus(), and
>> ARM's two implementations of vgic_max_vcpus() no longer need work
>> around the
>> out-of-order call.
>>
>>  From this point on, d->max_vcpus and d->vcpus[] are valid for any
>> domain which
>> can be looked up by domid.
>>
>> The XEN_DOMCTL_max_vcpus hypercall is modified to reject any call
>> attempt with
>> max != d->max_vcpus, which does match the older semantics (not that
>> it is
>> obvious from the code).  The logic to allocate d->vcpu[] is dropped,
>> but at
>> this point the hypercall still needs making to allocate each vcpu.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> With one request below, for the ARM + Common code:
>
> Acked-by: Julien Grall <julien.grall@arm.com>
>
> FAOD, I agree with your position to avoid the renaming.
>
> [...]
>
>> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
>> index 58e51b2..8a803b3 100644
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -547,6 +547,13 @@ long
>> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>>           break;
>>       }
>>   +    /*
>> +     * Note: The parameter passed to XEN_DOMCTL_max_vcpus must match
>> the value
>> +     * passed to XEN_DOMCTL_createdomain.  This hypercall is in the
>> process of
>> +     * being removed (once the failure paths in domain_create() have
>> been
>> +     * improved), but is still required in the short term to
>> allocate the
>> +     * vcpus themselves.
>> +     */
>
> This comment might be more useful in the public header. This is
> usually the first place I would look for description of a domctl.

Ok - will do.

That said, it is very likely that the next person to read this will be
me when I finally remove it (hopefully in 4.12).

~Andrew

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