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

Re: [RFC PATCH 00/10] Preemption in hypervisor (ARM only)

On 23/02/2021 12:06, Volodymyr Babchuk wrote:

Hi Julien,

Hi Volodymyr,

Julien Grall writes:
On 23/02/2021 02:34, Volodymyr Babchuk wrote:
... just rescheduling the vCPU. It will also give the opportunity for
the guest to handle interrupts.

If you don't return to the guest, then risk to get an RCU sched stall
on that the vCPU (some hypercalls can take really really long).

Ah yes, you are right. I'd only wish that hypervisor saved context of
hypercall on it's side...

I have example of OP-TEE before my eyes. They have special return code
"task was interrupted" and they use separate call "continue execution of
interrupted task", which takes opaque context handle as a
parameter. With this approach state of interrupted call never leaks to > rest 
of the system.

Feel free to suggest a new approach for the hypercals.

This approach itself have obvious
problems: code that executes hypercall is responsible for preemption,
preemption checks are infrequent (because they are costly by
themselves), hypercall execution state is stored in guest-controlled
area, we rely on guest's good will to continue the hypercall.

Why is it a problem to rely on guest's good will? The hypercalls
should be preempted at a boundary that is safe to continue.

Yes, and it imposes restrictions on how to write hypercall
In other words, there are much more places in hypercall handler code
where it can be preempted than where hypercall continuation can be
used. For example, you can preempt hypercall that holds a mutex, but of
course you can't create an continuation point in such place.

I disagree, you can create continuation point in such place. Although it will be more complex because you have to make sure you break the work in a restartable place.

I would also like to point out that preemption also have some drawbacks.
With RT in mind, you have to deal with priority inversion (e.g. a lower priority vCPU held a mutex that is required by an higher priority).

Outside of RT, you have to be careful where mutex are held. In your earlier answer, you suggested to held mutex for the memory allocation. If you do that, then it means a domain A can block allocation for domain B as it helds the mutex.

This can lead to quite serious problem if domain A cannot run (because it exhausted its credit) for a long time.

All this
imposes restrictions on which hypercalls can be preempted, when they
can be preempted and how to write hypercall handlers. Also, it
requires very accurate coding and already led to at least one
vulnerability - XSA-318. Some hypercalls can not be preempted at all,
like the one mentioned in [1].
Absence of hypervisor threads/vCPUs. Hypervisor owns only idle
which are supposed to run when the system is idle. If hypervisor needs
to execute own tasks that are required to run right now, it have no
other way than to execute them on current vCPU. But scheduler does not
know that hypervisor executes hypervisor task and accounts spent time
to a domain. This can lead to domain starvation.
Also, absence of hypervisor threads leads to absence of high-level
synchronization primitives like mutexes, conditional variables,
completions, etc. This leads to two problems: we need to use spinlocks
everywhere and we have problems when porting device drivers from linux
Proposed solution
It is quite obvious that to fix problems above we need to allow
preemption in hypervisor mode. I am not familiar with x86 side, but
for the ARM it was surprisingly easy to implement. Basically, vCPU
context in hypervisor mode is determined by its stack at general
purpose registers. And __context_switch() function perfectly switches
them when running in hypervisor mode. So there are no hard
restrictions, why it should be called only in leave_hypervisor() path.
The obvious question is: when we should to try to preempt running
vCPU?  And answer is: when there was an external event. This means
that we should try to preempt only when there was an interrupt request
where we are running in hypervisor mode. On ARM, in this case function
do_trap_irq() is called. Problem is that IRQ handler can be called
when vCPU is already in atomic state (holding spinlock, for
example). In this case we should try to preempt right after leaving
atomic state. This is basically all the idea behind this PoC.
Now, about the series composition.
    sched: core: save IRQ state during locking
    sched: rt: save IRQ state during locking
    sched: credit2: save IRQ state during locking
    preempt: use atomic_t to for preempt_count
    arm: setup: disable preemption during startup
    arm: context_switch: allow to run with IRQs already disabled
prepare the groundwork for the rest of PoC. It appears that not all
code is ready to be executed in IRQ state, and schedule() now can be
called at end of do_trap_irq(), which technically is considered IRQ
handler state. Also, it is unwise to try preempt things when we are
still booting, so ween to enable atomic context during the boot

I am really surprised that this is the only changes necessary in
Xen. For a first approach, we may want to be conservative when the
preemption is happening as I am not convinced that all the places are
safe to preempt.

Well, I can't say that I ran extensive tests, but I played with this for
some time and it seemed quite stable. Of course, I had some problems
with RTDS...

As I see it, Xen is already supports SMP, so all places where races are
possible should already be covered by spinlocks or taken into account by
some other means.
That's correct for shared resources. I am more worried for any hypercalls that expected to run more or less continuously (e.g not taking into account interrupt) on the same pCPU.

Places which may not be safe to preempt are clustered around task
management code itself: schedulers, xen entry/exit points, vcpu
creation/destruction and such.

For example, for sure we do not want to destroy vCPU which was preempted
in hypervisor mode. I didn't covered this case, by the way.

    preempt: add try_preempt() function
    sched: core: remove ASSERT_NOT_IN_ATOMIC and disable preemption[!]
    arm: traps: try to preempt before leaving IRQ handler
are basically the core of this PoC. try_preempt() function tries to
preempt vCPU when either called by IRQ handler and when leaving atomic
state. Scheduler now enters atomic state to ensure that it will not
preempt self. do_trap_irq() calls try_preempt() to initiate preemption.

AFAICT, try_preempt() will deal with the rescheduling. But how about
softirqs? Don't we want to handle them in try_preempt() as well?

Well, yes and no. We have the following softirqs:

  TIMER_SOFTIRQ - should be called, I believe
  RCU_SOFTIRQ - I'm not sure about this, but probably no

When would you call RCU callback then?

  NEW_TLBFLUSH_CLOCK_PERIOD_SOFTIRQ - should be moved to a separate
  thread, maybe?
  TASKLET_SOFTIRQ - should be moved to a separate thread >
So, looks like only timers should be handled for sure.


My main intention is to begin discussion of hypervisor
preemption. As
I showed, it is doable right away and provides some immediate
benefits. I do understand that proper implementation requires much
more efforts. But we are ready to do this work if community is
interested in it.
Just to reiterate main benefits:
1. More controllable latency. On embedded systems customers care
such things.

Is the plan to only offer preemptible Xen?

Sorry, didn't get the question.

What's your plan for the preemption support? Will an admin be able to configure Xen to be either preemptible or not?


Julien Grall



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