Re: [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset

[Vitaly Kuznetsov <vkuznets@redhat.com>]

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

>>>> On 03.06.15 at 15:35, <vkuznets@redhat.com> wrote:
>> --- a/xen/common/event_channel.c
>> +++ b/xen/common/event_channel.c
>> @@ -274,11 +274,13 @@ static long evtchn_bind_interdomain(evtchn_bind_interdomain_t *bind)
>>  
>>      lchn->u.interdomain.remote_dom  = rd;
>>      lchn->u.interdomain.remote_port = rport;
>> +    lchn->u.interdomain.opened_by   = current->domain;
>>      lchn->state                     = ECS_INTERDOMAIN;
>>      evtchn_port_init(ld, lchn);
>>      
>>      rchn->u.interdomain.remote_dom  = ld;
>>      rchn->u.interdomain.remote_port = lport;
>> +    rchn->u.interdomain.opened_by   = current->domain;
>>      rchn->state                     = ECS_INTERDOMAIN;
>
> For one ld == current->domain. And I don't think you need to store
> domain pointers here; storing the domain ID would seem sufficient
> (with the nice benefit of not growing struct evtchn). Plus
> "opened_by" doesn't really reflect what is being done here - the
> event channels are being bound, not opened.
>
>> @@ -933,26 +935,30 @@ int evtchn_unmask(unsigned int port)
>>  }
>>  
>>  
>> -static long evtchn_reset(evtchn_reset_t *r)
>> +void evtchn_reset(struct domain *d, bool_t soft_reset)
>>  {
>> -    domid_t dom = r->dom;
>> -    struct domain *d;
>> -    int i, rc;
>> -
>> -    d = rcu_lock_domain_by_any_id(dom);
>> -    if ( d == NULL )
>> -        return -ESRCH;
>> -
>> -    rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
>> -    if ( rc )
>> -        goto out;
>> +    int i;
>
> unsigned int
>
>> +    struct evtchn *chn;
>
> const (and perhaps moved inside the loop below)
>
>> +    /*
>> +     * ECS_INTERDOMAIN channels with port number suitable for the 2-level ABI
>> +     * opened by other domains should remain opened as the domain doing soft
>> +     * reset won't be able to reopen them.
>
> One question of course is - does this really apply to _all_ interdomain
> event channels (rather than just to the xenstore and xenconsole ones)?

The wild guess here is that if a channel was bound by someone else the
domain doing this reset operation won't probably be able (and allowed)
to bind such interdomain connectivity so we need to keep it, from the
hypervisor PoV xenstore and xenconsole channels are not special. We,
however, won't probably need that in case we go for the
toolstack-assisted approach (and as far as I understand there are no
objections against such approach for now but I'll have to prototype it
so we can make a decision.)

-- 
  Vitaly