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

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel