[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information


On 09/29/2018 12:48 AM, Andrew Cooper wrote:
On 29/09/18 00:45, Stefano Stabellini wrote:
On Sat, 29 Sep 2018, Andrew Cooper wrote:
On 28/09/18 21:35, Julien Grall wrote:

On 09/28/2018 12:11 AM, Stefano Stabellini wrote:
On Wed, 26 Sep 2018, Julien Grall wrote:
Hi Stefano,

On 09/25/2018 09:45 PM, Stefano Stabellini wrote:
On Tue, 4 Sep 2018, Andrew Cooper wrote:
On 04/09/18 20:35, Julien Grall wrote:

On 09/04/2018 08:21 PM, Julien Grall wrote:
A follow-up patch will require to know the number of vCPUs when
initializating the vGICv3 domain structure. However this
not available at domain creation. This is only known once
XEN_DOMCTL_max_vpus is called for that domain.

In order to get the max vCPUs around, delay the domain part of the
v3 initialization until the first vCPU of the domain is

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>


Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

This is nasty but I can't find a better way for Xen 4.11 and older.
is not necessary for unstable as the number of vCPUs is known at

Andrew, I have CCed you to know whether you have a better idea
place this call on Xen 4.11 and older.
I just noticed that d->max_vcpus is initialized after
arch_domain_create. So without this patch on Xen 4.12, it will
not work.

This is getting nastier because arch_domain_init is the one
the value returned by dom0_max_vcpus. So I am not entirely sure what
to do here.
The positioning after arch_domain_create() is unfortunate, but I
couldn’t manage better with ARM's current behaviour and Jan's
that the allocation of d->vcpu was common.  I'd prefer if the
could be broken and the allocation moved earlier.

One option might be to have an arch_check_domainconfig() (or
which is called very early on and can sanity check the values,
cross-checking the vgic and max_vcpus settings?  It could even be
responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the
real value.

As for your patch here, its a gross hack, but its probably the best
which can be done.
If that is what we have to do, it is as ugly as hell, but that is what
we'll do.
This is the best we can do with the current code base. I think it
would be
worth reworking the code to make it nicer. I will add it in my TODO

My only suggestion to marginally improve it would be instead of:

+    if ( v->vcpu_id == 0 )
+    {
+        rc = vgic_v3_real_domain_init(d);
+        if ( rc )
+            return rc;
+    }
to check on d->arch.vgic.rdist_regions instead:

         if ( d->arch.vgic.rdist_regions == NULL )
            // initialize domain
I would prefer to keep v->vcpu_id == 0 just in case we end up to
re-order the
allocation in the future.
I was suggesting to check on (rdist_regions == NULL) exactly for
potential re-ordering, in case in the future we end up calling
vcpu_vgic_init differently and somehow vcpu_init(vcpu1) is done before
before vcpu_init(vcpu0). Ideally we would like a way to check that
vgic_v3_real_domain_init has been called before and I thought
rdist_regions == NULL could do just that...
What I meant by re-ordering is we manage to allocate the
re-distributors before the vCPUs are created but still need
vgic_v3_real_domain_init for other purpose.

But vCPU initialization is potentially other issue.

Anyway. both way have drawbacks. Yet I still prefer checking on the
vCPU. It less likely vCPU0 will not be the first one initialized.
With the exception of the idle domain, all vcpus are strictly allocated
in packed ascending order.  Loads of other stuff will break if that
changed, so I wouldn't worry about it.

Furthermore, there is no obvious reason for this behaviour to ever change.
OK, let's go with Julien's patch. We need a new tag for this, something

Acked-but-disliked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

Do bear in mind that this patch is only for 4.11 and earlier.  I've
already fixed staging (i.e. 4.12) when it comes to knowing d->max_vcpus :)
I thought we agreed that patch is necessary for 4.12 as d->max_vcpus is initialized after arch_domain_init?

I am not planning to do the rework in short term. Did you do more work on around domain_create recently?


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.