Re: [PATCH v3 01/24] xen: Extend DOMCTL createdomain to support arch configuration

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

On 20/02/15 15:15, Ian Campbell wrote:
> On Tue, 2015-01-13 at 14:25 +0000, Julien Grall wrote:
>> On ARM the virtual GIC may differ between each guest (emulated GIC version,
>> number of SPIs...). Those informations are already known at the domain creation
> "This information is already known at domain creation..."
>
>> and can never change.
>>
>> For now only the gic_version is set. In long run, there will be more parameters
> "In the long run".
>
>> such as the number of SPIs. All will be required to be set at the same time.
>>
>> A new arch-specific structure arch_domainconfig has been created, the x86
>> one doesn't have any specific configuration, a dummy structure
>> (C-spec compliant) has been created to factorize the code on the toolstack.
> I'm not sure what you mean by factorize here.
>
>> Some external tools (qemu, xenstore) may require to create a domain. Rather
> "may be required" perhaps? I'm not 100% sure what you are trying to say.
>
>> than asking them to take care of the arch-specific domain configuration, let
>> the current function (xc_domain_create) to chose a default configuration and
> s/to//
>
>> introduce a new one (xc_domain_create_config).
>>
>> This patch also drop the previously DOMCTL arm_configure_domain introduced
> "drops the previously introduced DOMCTL arm...".
>
>> in Xen 4.5, as it has been made useless.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
>> Cc: Keir Fraser <keir@xen.org>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: George Dunlap <george.dunlap@eu.citrix.com>
>>
>> ---
>>     This is a follow-up of http://lists.xen.org/archives/html/xen-devel/2014-11/msg00522.html
>>
>>     TODO: What about migration? For now the configuration lives in internal
>>     libxl structure. We need a way to pass the domain configuration to the
>>     other end.
> Things like the GIC version and number of SPIs would normally end up
> being encoded in the hvm save records, i.e. the blob which Xen provides
> to the toolstack to be included in the save stream. That would then be
> alongside the actual related interrupt architectural state etc (e.g.
> pending, active, masked etc).
>
> That's problematic here because you can't pass that blob to the create
> function and the toolstack cannot really parse the blob to figure out
> the details to pass.
>
> Looking back at msg00522.html it does seem like migration is a good
> motivation for doing it as a separate hypercall as you originally did,
> so initial create would be
> 	DOMCTL_create
> 	DOMCTL_set_the_variables
> and restore would be
> 	DOMCTL_create
> 	DOMCTL_restore_from_blob
>
> In both cases this needn't preclude requiring the call to be made before
> unpause or that it is only called exactly once.
>
> Write once HVM params would be another option here.
>
> Jan, do you find any of that convincing as to the need for doing this
> outside the the create domctl? Based on msg00522 is seems you would
> prefer some HVM params over a new domctl?
>
>>     I'm not sure if we should care of this right now as migration doesn't
>>     yet exists on ARM.
> It's a domctl so we aren't exactly painting ourselves into a corner, but
> it would be good to have some idea that we aren't going down a blind
> alley at least. Especially if the way out of the alley is to come back
> to where we are now...
>
> (There doesn't seem much point in looking at the code until we conclude
> the approach is correct, so I haven't)

GIC version and SPIs should absolutely be part of the migration stream. 
They are domain architectural state.

I have a similar issue with my plans for cpuid improvements in x86, and
was going to introduce a domain architectural state near the head of the
migration stream.  (There is currently a chicken & egg problem in x86
with the hvm params, the cpu state and the cpuid policy.  In Xen, we
currently check the cpu state for plausibility because it is not
currently possible to load the policy before xc_domain_restore() is
complete.)


At first glance, extending createdomian would look like a sensible
action to ensure that these bits of state get set appropriately.  A
consequence would be that the createdomain hypercall must move down into
xc_domain_restore(), which causes problems in the XSM case (where the
permissions are along the lines of "sub builder $X is permitted to issue
hypercalls on domid $Y).

An alternative is instead to move more of domain construction into the
xc_domain_restore(), so architectural state from the stream can be used
to construct the domain.  (For x86 cpuid, I believe I need to set part
of the architectural state before the set_max_vcpus hypercall, and I
certainly need to propagate maxphysaddr from the source before I can
validate any memory frames in the stream.)

This would result in a leaning towards separate hypercalls.

~Andrew