Re: [PATCH 5/5] x86/xen: Don't register PV spinlock IPI when it isn't going to be used

[boris.ostrovsky@xxxxxxxxxx]

On 1/4/21 5:37 PM, David Woodhouse wrote:
>
> @@ -33,9 +33,11 @@ static void __init xen_hvm_smp_prepare_cpus(unsigned int 
> max_cpus)
>       int cpu;
>  
>       native_smp_prepare_cpus(max_cpus);
> -     WARN_ON(xen_smp_intr_init(0));
>  
> -     xen_init_lock_cpu(0);
> +     if (xen_have_vector_callback) {
> +             WARN_ON(xen_smp_intr_init(0));
> +             xen_init_lock_cpu(0);


By now you have nopvspin set so you might as well leave xen_init_lock_cpu(0) as 
is. (and then move the check inside xen_smp_intr_init())


> +     }
>  
>       for_each_possible_cpu(cpu) {
>               if (cpu == 0)
> @@ -64,14 +66,17 @@ static void xen_hvm_cpu_die(unsigned int cpu)
>  
>  void __init xen_hvm_smp_init(void)
>  {
> -     if (!xen_have_vector_callback)
> +     smp_ops.smp_prepare_boot_cpu = xen_hvm_smp_prepare_boot_cpu;
> +     smp_ops.smp_prepare_cpus = xen_hvm_smp_prepare_cpus;
> +     smp_ops.smp_cpus_done = xen_smp_cpus_done;
> +
> +     if (!xen_have_vector_callback) {
> +             nopvspin = true;
>               return;
> +     }
>  
> -     smp_ops.smp_prepare_cpus = xen_hvm_smp_prepare_cpus;
>       smp_ops.smp_send_reschedule = xen_smp_send_reschedule;
>       smp_ops.cpu_die = xen_hvm_cpu_die;


Why not xen_hvm_cpu_die too? common_cpu_die() sounds like something we should 
do, and the other three we call there will be nops.


>       smp_ops.send_call_func_ipi = xen_smp_send_call_function_ipi;
>       smp_ops.send_call_func_single_ipi = 
> xen_smp_send_call_function_single_ipi;
> -     smp_ops.smp_prepare_boot_cpu = xen_hvm_smp_prepare_boot_cpu;
> -     smp_ops.smp_cpus_done = xen_smp_cpus_done;
>  }
>
>> Also, for the spinlock changes specifically --- I wonder whether it
>> would be better to reverse initial value of xen_pvspin and set it to
>> 'true' only if initialization succeeds.
> I looked at that but it would need to be tristate, since the
> 'xen_nopvspin' command line option clears it from its default of being
> enabled.


Ah, right. How about setting nopvspin in xen_parse_nopvspin()?


-boris