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

Re: [Xen-devel] [RFC PATCH 08/49] ARM: VGIC: move max_vcpus VGIC limit to struct arch_domain



Hi,

On 02/28/2018 12:32 PM, Andre Przywara wrote:
Hi,

On 09/02/18 19:27, Julien Grall wrote:
Hi,

On 02/09/2018 02:38 PM, Andre Przywara wrote:
The VGIC model used for a domain (GICv2 or GICv3) determines the maximum
number of VCPUs for that guest, as GICv2 can only handle 8 processors.
In the moment we carry this per-VGIC-model limit in the vgic_ops,
alongside the model specific functions. That makes some sense, but
exposes some current VGIC implementation details to generic Xen code.
Add a new arch specific field in our domain structure to hold this
vcpu limit,
and initialize it when we set the ops. This allows us to plug in the new
VGIC later without also needing to carry some ops structure.

Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
---
   xen/arch/arm/domain.c        | 5 ++---
   xen/arch/arm/vgic.c          | 3 ++-
   xen/include/asm-arm/domain.h | 1 +
   3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index a010443bfd..9ad4cd0a6e 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -975,11 +975,10 @@ unsigned int domain_max_vcpus(const struct
domain *d)
        * allocation when the vgic_ops haven't been initialised yet,
        * we return MAX_VIRT_CPUS if d->arch.vgic.handler is null.
        */
-    if ( !d->arch.vgic.handler )
+    if ( !d->arch.max_vcpus )
           return MAX_VIRT_CPUS;
       else
-        return min_t(unsigned int, MAX_VIRT_CPUS,
-                     d->arch.vgic.handler->max_vcpus);
+        return d->arch.max_vcpus;
   }
     /*
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 9921769b15..5f47aa84a9 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -166,7 +166,8 @@ int domain_vgic_init(struct domain *d, unsigned
int nr_spis)
     void register_vgic_ops(struct domain *d, const struct vgic_ops *ops)
   {
-   d->arch.vgic.handler = ops;
+    d->arch.vgic.handler = ops;
+    d->arch.max_vcpus = ops->max_vcpus;
   }
     void domain_vgic_free(struct domain *d)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 1dd9683d25..2fef32eaee 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -149,6 +149,7 @@ struct arch_domain
   #ifdef CONFIG_SBSA_VUART_CONSOLE
       struct vpl011 vpl011;
   #endif
+    unsigned int max_vcpus;

Now, you have max_vcpus defined in both arch_domain and domain. Which
makes the code very confusing to read. This is becoming apparent in the
check if (d->arch.max_vcpus > d->max_vcpus).

True, though I was just copying the name from the vgic_ops ;-)
I could suggest arch.vgic_vcpu_limit instead, but...

If you plan to ditch the ops, then I would prefer a check on the vGIC
version. Even if it means carrying vGIC specific implementation in
generic Xen code.

So what about just providing per-VGIC implementations of
domain_max_vcpus()? That seems to be the only user of this information,
AFAICS? So I move this from xen/arch/arm/domain.c to
xen/arch/arm{/vgic,}/vgic.c, where we can do all kind of internal VGIC
tricks to learn this bit of information.

Sounds good to me.

Cheers,

--
Julien Grall

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

 


Rackspace

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