Re: [PATCH 3/3] x86/smt: Support for enabling/disabling SMT at runtime

["Jan Beulich" <JBeulich@suse.com>]

>>> On 03.04.19 at 13:33, <andrew.cooper3@citrix.com> wrote:
> On 03/04/2019 11:44, Jan Beulich wrote:
>>>>> On 03.04.19 at 12:17, <andrew.cooper3@citrix.com> wrote:
>>> On 03/04/2019 10:33, Jan Beulich wrote:
>>>>>>> On 02.04.19 at 21:57, <andrew.cooper3@citrix.com> wrote:
>>>>> Slightly RFC.  I'm not very happy with the contination situation, but -EBUSY
>>>>> is the preexisting style and it seems like it is the only option from tasklet
>>>>> context.
>>>> Well, offloading the re-invocation to the caller isn't really nice.
>>>> Looking at the code, is there any reason why couldn't use
>>>> the usual -ERESTART / hypercall_create_continuation()? This
>>>> would require a little bit of re-work, in particular to allow
>>>> passing the vCPU into hypercall_create_continuation(), but
>>>> beyond that I can't see any immediate obstacles. Though
>>>> clearly I wouldn't make this a prereq requirement for the work
>>>> here.
>>> The problem isn't really the ERESTART.  We could do some plumbing and
>>> make it work, but the real problem is that I can't stash the current cpu
>>> index in the sysctl data block across the continuation point.
>>>
>>> At the moment, the loop depends on, once all CPUs are in the correct
>>> state, getting through the for_each_present_cpu() loop without taking a
>>> further continuation.
>> But these are two orthogonal things: One is how to invoke the
>> continuation, and the other is where the continuation is to
>> resume from. I think the former is more important to address,
>> as it affects how the tools side code needs to look like.
> 
> Right, but -EBUSY is consistent with how the single online/offline ops
> function at the moment, which is why I reused it here.

Right, and which also is why I don't think this has to be taken care
of right now.

>>>>> +    for_each_present_cpu ( cpu )
>>>>> +    {
>>>>> +        if ( cpu == 0 )
>>>>> +            continue;
>>>> Is this special case really needed? If so, perhaps worth a brief
>>>> comment?
>>> Trying to down cpu 0 is a hard -EINVAL.
>> But here we're on the CPU-up path. Plus, for eventually supporting
>> the offlining of CPU 0, it would feel slightly better if you used
>> smp_processor_id() here.
> 
> Are there any processors where you can actually take CPU 0 offline?  Its
> certainly not possible on any Intel or AMD CPUs.
> 
> While I can appreciate the theoretical end goal, it isn't a reality and
> I see no signs of that changing.  Xen very definitely cannot take CPU 0
> offline, nor can hardware, and I don't see any value in jumping through
> hoops for an end goal which doesn't exist.

Interesting. Why was it then that x86 Linux took quite some
steps to make it possible (see BOOTPARAM_HOTPLUG_CPU0
Kconfig option as a possible anchor to locate pieces, which
even has a description of conditions that need to be met in
order for this to be possible)? IOW I'd appreciate clarification
on what it is exactly that you think prevents offlining CPU0
(from a pure hardware / firmware perspective), besides the
PIC and suspend/resume aspects (neither of which has to be
in use) mentioned there.

Jan



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