Re: [PATCH RFC V2 42/45] xen/sched: add fall back to idle vcpu when scheduling item

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

>>> On 16.05.19 at 15:51, <jgross@suse.com> wrote:
> On 16/05/2019 15:05, Jan Beulich wrote:
>>>>> On 06.05.19 at 08:56, <jgross@suse.com> wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -154,6 +154,24 @@ static void idle_loop(void)
>>>      }
>>>  }
>>>  
>>> +/*
>>> + * Idle loop for siblings of active schedule items.
>>> + * We don't do any standard idle work like tasklets, page scrubbing or
>>> + * livepatching.
>>> + * Use default_idle() in order to simulate v->is_urgent.
>> 
>> I guess I'm missing a part of the description which explains all this:
>> What's wrong with doing scrubbing work, for example? Why is
>> doing tasklet work not okay, but softirqs are? What is the deal with
>> v->is_urgent, i.e. what justifies not entering a decent power
>> saving mode here on Intel, but doing so on AMD?
> 
> One of the reasons for using core scheduling is to avoid running vcpus
> of different domains on the same core in order to minimize the chances
> for side channel attacks to data of other domains. Not allowing
> scrubbing or tasklets here is due to avoid accessing data of other
> domains.

So how is running softirqs okay then? And how is scrubbing accessing
other domains' data?

> As with core scheduling we can be sure the other thread is active
> (otherwise we would schedule the idle item) and hoping for saving power
> by using mwait is moot.

Saving power may be indirect, by the CPU re-arranging
resource assignment between threads when one goes idle.
I have no idea whether they do this when entering C1, or
only when entering deeper C states.

And anyway - I'm still none the wiser as to the v->is_urgent
relationship.

>>> --- a/xen/include/asm-x86/smp.h
>>> +++ b/xen/include/asm-x86/smp.h
>>> @@ -76,6 +76,9 @@ void set_nr_sockets(void);
>>>  /* Representing HT and core siblings in each socket. */
>>>  extern cpumask_t **socket_cpumask;
>>>  
>>> +#define get_cpu_current(cpu) \
>>> +    (get_cpu_info_from_stack((unsigned long)stack_base[cpu])->current_vcpu)
>> 
>> Yet another, slightly different notion of "current". If "current"
>> itself is not suitable (I can't immediately see why that would be,
>> but I also didn't look at all the scheduler specific changes earlier
>> in this series), why isn't per_cpu(curr_vcpu, cpu) either?
> 
> current is always the vcpu running on the current physical cpu.
> curr_vcpu is the vcpu which was the one running in guest mode last
> (this avoids the need to save/restore context in case a vcpu is
> blocked for a short time without another guest vcpu running on the
> physical cpu in between), so with current being idle the two can
> differ.
> 
> Here I need "current" from another physical cpu which is not easily
> available.

Oh, right - I should have been able to spot this.

Jan



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