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

[David Woodhouse <dwmw2@xxxxxxxxxxxxx>]

On Mon, 2021-01-04 at 14:06 -0500, boris.ostrovsky@xxxxxxxxxx wrote:
> On 1/4/21 12:32 PM, David Woodhouse wrote:
> > On Mon, 2021-01-04 at 12:06 -0500, boris.ostrovsky@xxxxxxxxxx wrote
> > :
> > > > @@ -115,7 +115,7 @@ PV_CALLEE_SAVE_REGS_THUNK(xen_vcpu_stolen);
> > > >    void __init xen_init_spinlocks(void)
> > > >    {
> > > >         /*  Don't need to use pvqspinlock code if there is only
> > > > 1 vCPU. */
> > > > -     if (num_possible_cpus() == 1 || nopvspin)
> > > > +     if (num_possible_cpus() == 1 || nopvspin ||
> > > > !xen_have_vector_callback)
> > > 
> > > xen_init_spinlock() will not be called without vector callbacks
> > > so
> > > this test is not really necessary.
> > 
> > Right, that's just paranoia to make the conditions consistent and
> > safe.
> 
> 
> OK, but we still need to do something about virt_spin_lock_key.

Right, I suppose we should just call xen_init_spinlocks() and then my
defensive check stops being defensive and does what we need, including
fixing virt_spin_lock_key.

Normally it's xen_hvm_smp_prepare_boot_cpu() which calls
xen_init_spinlocks(), and *also* calls xen_vcpu_setup() for CPU0...
which brings me back to your other observation.

I think we *should* be calling xen_vcpu_setup() for all CPUs, even when
there's no vector callback. We can still have a per-vCPU vcpu_info page
if we want it. It was relatively harmless that we didn't do it for
CPU0, but it was wrong not to do so.

So I think this fixes both. Will test:

--- a/arch/x86/xen/smp_hvm.c
+++ b/arch/x86/xen/smp_hvm.c
@@ -64,6 +64,8 @@ static void xen_hvm_cpu_die(unsigned int cpu)
 
 void __init xen_hvm_smp_init(void)
 {
+       smp_ops.smp_prepare_boot_cpu = xen_hvm_smp_prepare_boot_cpu;
+
        if (!xen_have_vector_callback)
                return;
 
@@ -72,6 +74,5 @@ void __init xen_hvm_smp_init(void)
        smp_ops.cpu_die = xen_hvm_cpu_die;
        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;
 }

Attachment: smime.p7s
Description: S/MIME cryptographic signature