[PATCH] evtchn: make EVTCHNOP_reset suitable for kexec

[PATCH] evtchn: make EVTCHNOP_reset suitable for kexec

[Vitaly Kuznetsov]

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.

[The idea was suggested by Ian Campbell and Andrew Cooper]

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 xen/common/event_channel.c | 17 +++++++++++++++--
 xen/common/event_fifo.c    |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index db952af..46a0ef0 100644
--- 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);
+    }
+
+    if (d->evtchn_fifo)
+        evtchn_fifo_destroy(d);
 
     rc = 0;
 
diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index 1fce3f1..51b4ff6 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -451,6 +451,7 @@ static void cleanup_event_array(struct domain *d)
     for ( i = 0; i < EVTCHN_FIFO_MAX_EVENT_ARRAY_PAGES; i++ )
         unmap_guest_page(d->evtchn_fifo->event_array[i]);
     xfree(d->evtchn_fifo);
+    d->evtchn_fifo = NULL;
 }
 
 static void setup_ports(struct domain *d)
-- 
1.9.3

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

[Andrew Cooper]

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.
>
> [The idea was suggested by Ian Campbell and Andrew Cooper]
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

This patch is looking very much better than its predecessor.

The xenstore and console event channels are set up by the toolstack for
the domain, and not expected to be set up by the domain itself;  They
are fine to be exempt here.

However, all other event channels to dom0 are not special in the
slightest, and should be reset.  They can be re-negotiated with the
backends via the usual xenstore methods.

> ---
>  xen/common/event_channel.c | 17 +++++++++++++++--
>  xen/common/event_fifo.c    |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index db952af..46a0ef0 100644
> --- 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++ )

What about channel 0?  What if there is an invalid port preceding a
subsequent valid one?

~Andrew

> +    {
> +        /*
> +         * 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);
> +    }
> +
> +    if (d->evtchn_fifo)
> +        evtchn_fifo_destroy(d);
>  
>      rc = 0;
>  
> diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
> index 1fce3f1..51b4ff6 100644
> --- a/xen/common/event_fifo.c
> +++ b/xen/common/event_fifo.c
> @@ -451,6 +451,7 @@ static void cleanup_event_array(struct domain *d)
>      for ( i = 0; i < EVTCHN_FIFO_MAX_EVENT_ARRAY_PAGES; i++ )
>          unmap_guest_page(d->evtchn_fifo->event_array[i]);
>      xfree(d->evtchn_fifo);
> +    d->evtchn_fifo = NULL;
>  }
>  
>  static void setup_ports(struct domain *d)

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

[Vitaly Kuznetsov]

Andrew Cooper <andrew.cooper3@citrix.com> writes:

> 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.
>>
>> [The idea was suggested by Ian Campbell and Andrew Cooper]
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>
> This patch is looking very much better than its predecessor.
>

Thanks!

> The xenstore and console event channels are set up by the toolstack for
> the domain, and not expected to be set up by the domain itself;  They
> are fine to be exempt here.
>
> However, all other event channels to dom0 are not special in the
> slightest, and should be reset.  They can be re-negotiated with the
> backends via the usual xenstore methods.

I agree. Unfortunately I wasn't able to figure out how to distinguish
between store/console channels and all other interdomain channels bound
to dom0 (I know toolstack has this information, but how would we check
that in hypervisor?) Any suggestions are more than welcome!

>> ---
>>  xen/common/event_channel.c | 17 +++++++++++++++--
>>  xen/common/event_fifo.c    |  1 +
>>  2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
>> index db952af..46a0ef0 100644
>> --- 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++ )
>
> What about channel 0?

I was under an imression that port = 0 is a special ECS_RESERVED one (we
set it up in evtchn_init()). We need to either skip it or re-initialize
it after we close it.

>  What if there is an invalid port preceding a subsequent valid one?

I *think* that never happens (and the original code relies on
that, I just added port=0 skip). Please correct me if I'm wrong.

>
> ~Andrew
>
>> +    {
>> +        /*
>> +         * 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);
>> +    }
>> +
>> +    if (d->evtchn_fifo)
>> +        evtchn_fifo_destroy(d);
>>  
>>      rc = 0;
>>  
>> diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
>> index 1fce3f1..51b4ff6 100644
>> --- a/xen/common/event_fifo.c
>> +++ b/xen/common/event_fifo.c
>> @@ -451,6 +451,7 @@ static void cleanup_event_array(struct domain *d)
>>      for ( i = 0; i < EVTCHN_FIFO_MAX_EVENT_ARRAY_PAGES; i++ )
>>          unmap_guest_page(d->evtchn_fifo->event_array[i]);
>>      xfree(d->evtchn_fifo);
>> +    d->evtchn_fifo = NULL;
>>  }
>>  
>>  static void setup_ports(struct domain *d)
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

-- 
  Vitaly

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

[Andrew Cooper]

On 25/07/14 17:25, Vitaly Kuznetsov wrote:
> Andrew Cooper <andrew.cooper3@citrix.com> writes:
>
>> 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.
>>>
>>> [The idea was suggested by Ian Campbell and Andrew Cooper]
>>>
>>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> This patch is looking very much better than its predecessor.
>>
> Thanks!
>
>> The xenstore and console event channels are set up by the toolstack for
>> the domain, and not expected to be set up by the domain itself;  They
>> are fine to be exempt here.
>>
>> However, all other event channels to dom0 are not special in the
>> slightest, and should be reset.  They can be re-negotiated with the
>> backends via the usual xenstore methods.
> I agree. Unfortunately I wasn't able to figure out how to distinguish
> between store/console channels and all other interdomain channels bound
> to dom0 (I know toolstack has this information, but how would we check
> that in hypervisor?) Any suggestions are more than welcome!

Hmm - HVM guests are easy; the information is in the hvm params.

PV guests however are going to require some creative thinking.

The start_info page contains the required information, but Xen has
nothing to do with this page; it is just another guest page which
happens to have some magic values placed there by the toolstack.  In the
case of a domain kexec, there is no guarentee that the start_info page
still contains valid data, so even if Xen could map the page, its
content cant be relied upon.

I can't think of a reasonable way of doing this which doesn't involve
modifying the toolstack to explicitly provide that information for PV
guests.

~Andrew

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

[Jan Beulich]

>>> On 25.07.14 at 19:06, <andrew.cooper3@citrix.com> wrote:
> I can't think of a reasonable way of doing this which doesn't involve
> modifying the toolstack to explicitly provide that information for PV
> guests.

It wouldn't be the neatest solution, but how about simply skipping
the first 2 interdomain ones in that case? Another (unpleasant)
alternative would be latch the values from the start info page e.g.
when the domain first gets un-paused.

Jan

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

[Jan Beulich]

>>> On 25.07.14 at 17:48, <vkuznets@redhat.com> wrote:
> @@ -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);
> +    }

You can't alter the behavior of an existing hypercall like this. Did
you at all check why it closes all channels, i.e. for what purpose
it got introduced?

And apart from that blindly leaving all interdomain channels intact
doesn't seem reasonable either.

Jan

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

[Andrew Cooper]

On 25/07/14 17:09, Jan Beulich wrote:
>>>> On 25.07.14 at 17:48, <vkuznets@redhat.com> wrote:
>> @@ -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);
>> +    }
> You can't alter the behavior of an existing hypercall like this. Did
> you at all check why it closes all channels, i.e. for what purpose
> it got introduced?
>
> And apart from that blindly leaving all interdomain channels intact
> doesn't seem reasonable either.
>
> Jan
>

I agree with your comment regarding interdomain channels.

However, there absolutely nothing a domain can do to recover from a
closed xenstore or console connection, as it is state set up by the
toolstack at domain build time, rather than state set up during early boot.

Whatever the intention of this hypercall originally, I can't see how a
domain could possibly recover having used it.

~Andrew

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

[Andrew Cooper]

On 25/07/14 17:16, Andrew Cooper wrote:
> On 25/07/14 17:09, Jan Beulich wrote:
>>>>> On 25.07.14 at 17:48, <vkuznets@redhat.com> wrote:
>>> @@ -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);
>>> +    }
>> You can't alter the behavior of an existing hypercall like this. Did
>> you at all check why it closes all channels, i.e. for what purpose
>> it got introduced?
>>
>> And apart from that blindly leaving all interdomain channels intact
>> doesn't seem reasonable either.
>>
>> Jan
>>
> I agree with your comment regarding interdomain channels.
>
> However, there absolutely nothing a domain can do to recover from a
> closed xenstore or console connection, as it is state set up by the
> toolstack at domain build time, rather than state set up during early boot.
>
> Whatever the intention of this hypercall originally, I can't see how a
> domain could possibly recover having used it.
>
> ~Andrew

Right - digging into the history shows that this was introduced 7 years
ago as a mixed toolstack or use-on-self hypercall.  In the toolstack
case, clearing all channels unconditionally is fine as the toolstack can
reset the xenstore/console event channels.

However, when using it with DOMID_SELF, the domain will clobber itself
irrevocably.

I think it would be acceptable to reset the console and store event
channels if and only if current->domain != d

~Andrew

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

[Jan Beulich]

>>> On 25.07.14 at 18:23, <andrew.cooper3@citrix.com> wrote:
> Right - digging into the history shows that this was introduced 7 years
> ago as a mixed toolstack or use-on-self hypercall.  In the toolstack
> case, clearing all channels unconditionally is fine as the toolstack can
> reset the xenstore/console event channels.
> 
> However, when using it with DOMID_SELF, the domain will clobber itself
> irrevocably.
> 
> I think it would be acceptable to reset the console and store event
> channels if and only if current->domain != d

To that I agree.

Jan

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

[Vitaly Kuznetsov]

"Jan Beulich" <JBeulich@suse.com> writes:

>>>> On 25.07.14 at 17:48, <vkuznets@redhat.com> wrote:
>> @@ -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);
>> +    }
>
> You can't alter the behavior of an existing hypercall like this. Did
> you at all check why it closes all channels, i.e. for what purpose
> it got introduced?

Originally I though about introducing new hypercall to cleans up all
control blocks when FIFO-based event channel ABI is being used (see
"[PATCH RFC] evtchn: introduce EVTCHNOP_fifo_destroy hypercall"
email). Andrew and Ian convinced me to reuse EVTCHNOP_reset to be
ABI-agnostic here.

I'm not sure how to check the purpose of introduction but I did check
that EVTCHNOP_reset is not being used neither in Linux kernel nor in
http://xenbits.xensource.com/ext/win-pvdrivers/. The original commit
which introduces it is:
 commit 115209d91bcd3734ddaaf58a4a1cdbb4c44cd4fa
 Author: kfraser@localhost.localdomain <kfraser@localhost.localdomain>
 Date:   Fri Jan 19 17:20:57 2007 +0000

    [XEN] New event-channel reset operation.
    Plumbed through libxenctrl to python.
    
    From: Andrei Petrov <andrei.petrov@xensource.com>
    Signed-off-by: Keir Fraser <keir@xensource.com>

It would be great if you can point out EVTCHNOP_reset usages I'm not
aware of. We can revise the descision of reusing EVTCHNOP_reset in case
we break things.

>
> And apart from that blindly leaving all interdomain channels intact
> doesn't seem reasonable either.

I agree here, see my reply to Andrew. We need to preserve console/store
channels only and any suggestion on how to distinguish them from other
interdomain mappings to Dom0 on hypervisor side are welcome.

>
> Jan
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

-- 
  Vitaly

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

[Ian Campbell]

On Fri, 2014-07-25 at 18:38 +0200, Vitaly Kuznetsov wrote:
> "Jan Beulich" <JBeulich@suse.com> writes:
> 
> >>>> On 25.07.14 at 17:48, <vkuznets@redhat.com> wrote:
> >> @@ -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);
> >> +    }
> >
> > You can't alter the behavior of an existing hypercall like this. Did
> > you at all check why it closes all channels, i.e. for what purpose
> > it got introduced?
> 
> Originally I though about introducing new hypercall to cleans up all
> control blocks when FIFO-based event channel ABI is being used (see
> "[PATCH RFC] evtchn: introduce EVTCHNOP_fifo_destroy hypercall"
> email). Andrew and Ian convinced me to reuse EVTCHNOP_reset to be
> ABI-agnostic here.
> 
> I'm not sure how to check the purpose of introduction but I did check
> that EVTCHNOP_reset is not being used neither in Linux kernel nor in
> http://xenbits.xensource.com/ext/win-pvdrivers/. The original commit
> which introduces it is:
>  commit 115209d91bcd3734ddaaf58a4a1cdbb4c44cd4fa
>  Author: kfraser@localhost.localdomain <kfraser@localhost.localdomain>
>  Date:   Fri Jan 19 17:20:57 2007 +0000
> 
>     [XEN] New event-channel reset operation.
>     Plumbed through libxenctrl to python.
>     
>     From: Andrei Petrov <andrei.petrov@xensource.com>
>     Signed-off-by: Keir Fraser <keir@xensource.com>
> 
> It would be great if you can point out EVTCHNOP_reset usages I'm not
> aware of. We can revise the descision of reusing EVTCHNOP_reset in case
> we break things.

Hrm, it's a long time ago but thinking back I *think* this was added for
the use of the toolstack, e.g. on slow/non-cooperative suspend resume
xend would use it. This works in that case because the toolstack can
then setup a new console etc.

I suspect that the in-guest usecase may not have been considered back
then.

> > And apart from that blindly leaving all interdomain channels intact
> > doesn't seem reasonable either.
> 
> I agree here, see my reply to Andrew. We need to preserve console/store
> channels only and any suggestion on how to distinguish them from other
> interdomain mappings to Dom0 on hypervisor side are welcome.

Rather than evtchn_reset perhaps the guest explicitly unbind whichever
evtchns it doesn't want to carry over the kexec?

Ian.

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

[David Vrabel]

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