Re: [PATCH v2 1/1] evtchn: make EVTCHNOP_reset suitable for kexec

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

On 30/07/14 14:53, Jan Beulich wrote:
>>>> On 30.07.14 at 15:42, <david.vrabel@citrix.com> wrote:
>> On 30/07/14 14:26, Vitaly Kuznetsov wrote:
>>> --- a/xen/common/event_channel.c
>>> +++ b/xen/common/event_channel.c
>>> @@ -957,6 +957,17 @@ static long evtchn_reset(evtchn_reset_t *r)
>>>      for ( i = 0; port_is_valid(d, i); i++ )
>>>          (void)__evtchn_close(d, i);
>>>  
>>> +    if ( (d == current->domain) && d->evtchn_fifo )
>>> +    {
>>> +        /*
>>> +         * Guest domain called EVTCHNOP_reset with DOMID_SELF, destroying
>>> +         * FIFO event array and control blocks, resetting evtchn_port_ops to
>>> +         * evtchn_port_ops_2l.
>>> +         */
>>> +        evtchn_fifo_destroy(d);
>>> +        evtchn_2l_init(d);
>>
>> You need to take d->event_lock around this if, or the guest could try to
>> bind another event whilst the ABI is in an inconsistent state.
> 
> True, but then not just around this. Subsequent to any of the
> __evtchn_close() invocations above, a port could also get
> re-used (resulting in at least leaks). So I guess after taking the
> lock there would also need to be a loop checking that all ports
> are still closed (and return an error if they aren't).

I considered this but I think the only side effect would be losing a
pending event, and it would be sufficient to document that guests should
not bind events while calling EVTCHNOP_reset.

David