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.przyw...@linaro.org>
---
  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).

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.

This would also avoid to grow the struct domain just for a "constant field" used mostly at guest creation.

Cheers,

--
Julien Grall

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

Reply via email to