On Tue, 2017-05-09 at 11:32 +0100, Julien Grall wrote:
> Hi Dario,
> 
Hey,

> On 05/09/2017 11:13 AM, Dario Faggioli wrote:
> > 
> > Which, FTR, is what we do in Xen for a bunch of things already,
> > i.e.,
> > softirqs and tasklets.
> 
> No, we don't switch to the idle vCPU to handle tasklets or softirqs. 
> They will be done before entering to the guest and still in the 
> hypervisor context.
> 
Mmm... I don't know who's "we" here, but even if it's "you ARM people",
you actually do.

In fact, this is common code:

static struct task_slice
csched_schedule(
    const struct scheduler *ops, s_time_t now, bool_t tasklet_work_scheduled)
{
    [...]
    /* Choices, choices:
     * - If we have a tasklet, we need to run the idle vcpu no matter what.
    [...]
    /* Tasklet work (which runs in idle VCPU context) overrides all else. */
    if ( tasklet_work_scheduled )
    {
        TRACE_0D(TRC_CSCHED_SCHED_TASKLET);
        snext = CSCHED_VCPU(idle_vcpu[cpu]);
        snext->pri = CSCHED_PRI_TS_BOOST;
    }
    [...]
}

And this is "your" idle loop:

void idle_loop(void)
{
    for ( ; ; )
    {
        [...]
        local_irq_disable();
        if ( cpu_is_haltable(smp_processor_id()) )
        {                            
            dsb(sy);
            wfi();
        }
        local_irq_enable();
        do_tasklet();
        do_softirq();
        [...]
    }
}

Actually, yes, it was a bit inaccurate of me to cite both softirqs and
tasklets, together, like I did. Softirqs indeed are checked and handled
before leaving Xen, as you say, as well as, in the idle loop, as shown
above.

But for tasklet (and, to be 100% precise, for vCPU context tasklet),
it's actually the case that we force the idle vCPU in execution to run
them.

> > And it should not be hard to give such code access to the context
> > of
> > the vCPU that was previously running (in x86, given we implement
> > what
> > we call lazy context switch, it's most likely still loaded in the
> > pCPU!).
> 
> I agree with Stefano, switching to the idle vCPU is a pretty bad
> idea.
> 
> the idle vCPU is a fake vCPU on ARM to stick with the common code
> (we 
> never leave the hypervisor). In the case of the EL0 app, we want to 
> change exception level to run the code with lower privilege.
> 
> Also IHMO, it should only be used when there are nothing to run and
> not 
> re-purposed for running EL0 app.
> 
It's already purposed for running when there is nothing to do _or_ when
there are tasklets.

I do see your point about privilege level, though. And I agree with
George that it looks very similar to when, in the x86 world, we tried
to put the infra together for switching to Ring3 to run some pieces of
Xen code.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)