Re: [Xen-devel] [PATCH RFC v1 66/74] xen/shim: allow DomU to have as many vcpus as available

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.18 at 14:06, <wei.liu2@xxxxxxxxxx> wrote:
> From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> 
> Since the shim VCPUOP_{up/down} hypercall is wired to the plug/unplug
> of CPUs to the shim itself, start the shim DomU with only the BSP
> online, and let the guest bring up other CPUs as it needs them.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

What are the ramifications of not making this change? Shouldn't
the shim's pCPU count (pCPU as viewed from its own perspective)
simply always match its client's vCPU count?

> @@ -153,6 +162,23 @@ unsigned int __init dom0_max_vcpus(void)
>      unsigned int i, max_vcpus, limit;
>      nodeid_t node;
>  
> +    if ( pv_shim )
> +    {
> +        nodes_setall(dom0_nodes);
> +
> +        /*
> +         * When booting in shim mode APs are not started until the guest 
> brings
> +         * other vCPUs up.
> +         */
> +        cpumask_set_cpu(0, &dom0_cpus);
> +
> +        /*
> +         * On PV shim mode allow the guest to have as many CPUs as available.
> +         */

Style (single line comment).

> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -695,7 +695,8 @@ int __init dom0_construct_pv(struct domain *d,
>      for ( i = 0; i < XEN_LEGACY_MAX_VCPUS; i++ )
>          shared_info(d, vcpu_info[i].evtchn_upcall_mask) = 1;
>  
> -    printk("Dom0 has maximum %u VCPUs\n", d->max_vcpus);
> +    printk("%s has maximum %u VCPUs\n", pv_shim ? "DomU" : "Dom0",

"Dom%c ..." perhaps?

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1580,20 +1580,28 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  
>      do_presmp_initcalls();
>  
> -    for_each_present_cpu ( i )
> +    if ( !pv_shim )
>      {
> -        /* Set up cpu_to_node[]. */
> -        srat_detect_node(i);
> -        /* Set up node_to_cpumask based on cpu_to_node[]. */
> -        numa_add_cpu(i);        
> -
> -        if ( (num_online_cpus() < max_cpus) && !cpu_online(i) )
> +        for_each_present_cpu ( i )
>          {
> -            int ret = cpu_up(i);
> -            if ( ret != 0 )
> -                printk("Failed to bring up CPU %u (error %d)\n", i, ret);
> +            /* Set up cpu_to_node[]. */
> +            srat_detect_node(i);
> +            /* Set up node_to_cpumask based on cpu_to_node[]. */
> +            numa_add_cpu(i);
> +
> +            if ( (num_online_cpus() < max_cpus) && !cpu_online(i) )
> +            {
> +                int ret = cpu_up(i);
> +                if ( ret != 0 )
> +                    printk("Failed to bring up CPU %u (error %d)\n", i, ret);
> +            }
>          }
>      }
> +    /*
> +     * NB: when running as a PV shim VCPUOP_up/down is wired to the shim
> +     * physical cpu_add/remove functions, so launch the guest with only
> +     * the BSP online and let it bring up the other CPUs as required.
> +     */

I think this comment would better go immediately ahead of the if()
you introduce.

Jan

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