[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 1/4/21 8:41 PM, David Woodhouse wrote:
> On Mon, 2021-01-04 at 18:44 -0500, boris.ostrovsky@xxxxxxxxxx wrote:
>> On 1/4/21 6:19 PM, David Woodhouse wrote:
>>> On Mon, 2021-01-04 at 18:04 -0500, boris.ostrovsky@xxxxxxxxxx
>>> wrote:
>>>> 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.
>>> native_cpu_die() calls that, and isn't that the function that gets
>>> installed if we don't install our own?
>> True.
>> Still, a Xen guest should call Xen-specific cpu_die() routine if
>> possible. Especially since (now) other cpu (i.e. non-IPI) ops will
>> call Xen versions.
> But as you said, the other three things that xen_hvm_cpu_die() does are
> all no-ops (or at least we'd have to make them no-ops; I haven't
> checked). I don't see the point in calling it, only for it to do
> nothing.

For maintenance purposes. When something gets added in initialization path 
(prepare_boot_cpu() and such) we likely want to tear it down in cpu_die(). 
Today both native and Xen cpu_die() ops work the same but things change. And 
chances are people will not test no_vector_callback case so we will then have 
to hunt down what broke when.

no_vector_callback is a pretty esoteric case already so making it take 
different code path from common case, when there is little or no penalty for 
doing the latter, seems undesirable to me.

>>>>>   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()?
>>> That would make the xen_nopvspin command line option disable PV
>>> spinlocks even under KVM.
>> xen_nopvspin is deprecated and nopvspin is recommended anyway so I think it 
>> should be OK, no?
> They do different things. If someone has an image (which might run on
> both Xen and KVM) which has xen_nopvspin, we don't suddenly want that
> to start disabling PV spinlocks on KVM too.

True, but this option is deprecated and will be removed in the future.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.