Re: [PATCH] evtchn: make EVTCHNOP_reset suitable for kexec

[David Vrabel <david.vrabel@citrix.com>]

On 25/07/14 16:48, Vitaly Kuznetsov wrote:
> It would be nice to allow guests to close all event channels in
> ABI-agnostic way on startup. This would allow a guest to perform
> full cleanup in case of kexec/kdump. EVTCHNOP_reset looks suitable
> for this purpose. However it has two issues:
> 
> - current implementation unconditionally closes all event channels
>   including store/console channels mapped to Dom0. There is no way
>   for a guest to restore these channels after they were closed.
> 
> - control blocks for vcpus need cleanup when FIFO ABI is being used.
> 
> With this change a guest can simply do EVTCHNOP_reset before its
> init in both 2-level and FIFO cases. Unfortunately we'll need to
> put something like "xen_version >= 4.5" check before doing it as
> if we do it with the old implementation the guest will get stuck.
> 
> The issue can also be solved by introducing a new EVTCHNOP
> operation but it seems that EVTCHNOP_reset was originally designed
> for such reset and it's not being used at this moment.
[...]
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -944,6 +944,7 @@ static long evtchn_reset(evtchn_reset_t *r)
>  {
>      domid_t dom = r->dom;
>      struct domain *d;
> +    struct evtchn *chn;
>      int i, rc;
>  
>      d = rcu_lock_domain_by_any_id(dom);
> @@ -954,8 +955,20 @@ static long evtchn_reset(evtchn_reset_t *r)
>      if ( rc )
>          goto out;
>  
> -    for ( i = 0; port_is_valid(d, i); i++ )
> -        (void)__evtchn_close(d, i);
> +    for ( i = 1; port_is_valid(d, i); i++ )
> +    {
> +        /*
> +         * Leave all interdomain connections to Dom0 untouched as we need to
> +         * preserve store/console channels.
> +         */
> +        chn = evtchn_from_port(d, i);
> +        if ( chn->state != ECS_INTERDOMAIN ||
> +             chn->u.interdomain.remote_dom->domain_id != 0 )
> +            (void)__evtchn_close(d, i);

I don't think we want to special case certain event channels in Xen like
this -- this will need to be in the guest or toolstack (probably the guest).

> +    }
> +
> +    if (d->evtchn_fifo)
> +        evtchn_fifo_destroy(d);

I don't think it is safe to call evtchn_fifo_destroy() while there are
any event channels bound to the domain since another domain may be
simultaneously raising the event and it does so while holding a
different lock.

It would also need to consider the pending state of any bound event
channels and ensure they are marked pending in the 2-level bitmap, so
that events are not lost.

There is also the question of what to do with a bound event channel with
a port number greater than 4095 (or 1023 for a 32-bit x86 VM).

I think these three reasons means it is only safe to switch back to
2-level if no event channels are bound.

I think the guest can rebind the XenStore and console event channels
itself, after having done the reset.  It will need to query Xen prior to
the reset to find the other end of the two event channels.

David