Re: [Xen-devel] [PATCH 09/57] ARM: VGIC: Move domain_max_vcpus() to be VGIC specific

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 05/03/18 16:03, Andre Przywara wrote:
> domain_max_vcpus(), which is used by generic Xen code, returns the
> maximum number of VCPUs for a domain, which on ARM is mostly limited by
> the VGIC model emulated (a (v)GICv2 can only handle 8 CPUs).
> Our current implementation lives in arch/arm/domain.c, but reaches into
> VGIC internal data structures.
> Move this function into vgic.c, to keep this VGIC internal.
While I understand the reason behind moving the code in vGIC, it feels a 
bit strange to have a domain function in vgic.c code.
How about renaming the function with a dummy helper in domain.h to 
accommodate common code? Maybe with a comment on top?
Cheers,

> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> ---
> Changelog RFC ... v1:
> - dump previous approach, move function to VGIC specific file instead
>
>   xen/arch/arm/domain.c | 14 --------------
>   xen/arch/arm/vgic.c   | 14 ++++++++++++++
>   2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index a010443bfd..8546443bad 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -968,20 +968,6 @@ void vcpu_block_unless_event_pending(struct vcpu *v)
>           vcpu_unblock(current);
>   }
>   
> -unsigned int domain_max_vcpus(const struct domain *d)
> -{
> -    /*
> -     * Since evtchn_init would call domain_max_vcpus for poll_mask
> -     * 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 )
> -        return MAX_VIRT_CPUS;
> -    else
> -        return min_t(unsigned int, MAX_VIRT_CPUS,
> -                     d->arch.vgic.handler->max_vcpus);
> -}
> -
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 34269bcf27..c3fdcebbde 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -665,6 +665,20 @@ void vgic_free_virq(struct domain *d, unsigned int virq)
>       clear_bit(virq, d->arch.vgic.allocated_irqs);
>   }
>   
> +unsigned int domain_max_vcpus(const struct domain *d)
> +{
> +    /*
> +     * Since evtchn_init would call domain_max_vcpus for poll_mask
> +     * 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 )
> +        return MAX_VIRT_CPUS;
> +    else
> +        return min_t(unsigned int, MAX_VIRT_CPUS,
> +                     d->arch.vgic.handler->max_vcpus);
> +}
> +
>   /*
>    * Local variables:
>    * mode: C
--
Julien Grall

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