Re: [Xen-devel] [PATCH] evtchn: make EVTCHNOP_reset suitable for kexec

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-07-25 at 18:38 +0200, Vitaly Kuznetsov wrote:
> "Jan Beulich" <JBeulich@xxxxxxxx> writes:
> 
> >>>> On 25.07.14 at 17:48, <vkuznets@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx <kfraser@xxxxxxxxxxxxxxxxxxxxx>
>  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@xxxxxxxxxxxxx>
>     Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx>
> 
> 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.


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