Re: [PATCH 3/5] xen/domain: Audit config->max_vcpus during {, arch_}check_domain_config()

[Julien Grall <julien.grall@arm.com>]

Hi Andrew,

On 05/10/2018 15:54, Andrew Cooper wrote:
> The purpose of this is to move the auduting to be earlier than
> arch_domain_create().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> 
> The max_vcpus setting for GIC_V3 is somewhat confusing.  The current GIC_V3
> driver claims to support 4096 cpus, while the newer GIC_V3 driver uses 255.

The maximum number of vCPUs supported for GICv3 depends on the number of 
affinity levels supported by the vGIC emulation.

> ---
>   xen/arch/arm/domain.c | 18 ++++++++++++++++++
>   xen/arch/x86/domain.c |  6 ++++++
>   xen/common/domain.c   |  3 +++
>   3 files changed, 27 insertions(+)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 43593a4..9676893 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -601,6 +601,8 @@ void vcpu_switch_to_aarch64_mode(struct vcpu *v)
>   
>   int arch_check_domain_config(struct xen_domctl_createdomain *config)
>   {
> +    unsigned int max_vcpus = 0;
> +
>       /* Fill in the native GIC version, passed back to the toolstack. */
>       if ( config->arch.gic_version == XEN_DOMCTL_CONFIG_GIC_NATIVE )
>       {
> @@ -619,6 +621,22 @@ int arch_check_domain_config(struct xen_domctl_createdomain *config)
>           }
>       }
>   
> +    /* Calculate the maximum number of vcpus from the selected GIC version... */
> +    switch ( config->arch.gic_version )
> +    {
> +    case GIC_V2: max_vcpus = 8;   break;
> +    case GIC_V3: max_vcpus = 255; break;
> +
> +    default:
> +        return -EOPNOTSUPP;
> +    }

I would prefer to keep those values in a separate helper implemented by 
each vGIC.

> +
> +    /* ... clipped at the maximum value Xen has been configured for. */
> +    max_vcpus = min(max_vcpus, MAX_VIRT_CPUS + 0u);

+ 0U feels a bit odd to read. It would be better to append u in 
MAX_VIRT_CPUS.

> +
> +    if ( config->max_vcpus > max_vcpus )
> +        return -EINVAL;
> +
>       return 0;
>   }
>   
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 26cab7c..19023d4 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -403,6 +403,12 @@ void arch_vcpu_destroy(struct vcpu *v)
>   
>   int arch_check_domain_config(struct xen_domctl_createdomain *config)
>   {
> +    unsigned int max_vcpus = ((config->flags & XEN_DOMCTL_CDF_hvm_guest)
> +                              ? HVM_MAX_VCPUS : MAX_VIRT_CPUS);
> +
> +    if ( config->max_vcpus > max_vcpus )
> +        return -EINVAL;
> +
>       return 0;
>   }
>   
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 236c2ad..9882550 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -297,6 +297,9 @@ static int check_domain_config(struct xen_domctl_createdomain *config)
>                              XEN_DOMCTL_CDF_xs_domain) )
>           return -EINVAL;
>   
> +    if ( config->max_vcpus < 1 )
> +        return -EINVAL;
> +
>       return arch_check_domain_config(config);
>   }
>   
> 

Cheers,

-- 
Julien Grall

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