Re: [PATCH 09/11] evtchn: implement EVTCHNOP_set_limit

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

On 16/09/13 11:33, Jan Beulich wrote:
>>>> On 16.09.13 at 12:00, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 16/09/13 08:07, Jan Beulich wrote:
>>>>>> On 13.09.13 at 18:56, David Vrabel <david.vrabel@citrix.com> wrote:
>>>> +static long evtchn_set_limit(const struct evtchn_set_limit *set_limit)
>>>> +{
>>>> +    struct domain *d;
>>>> +    unsigned max_port = set_limit->max_port;
>>>> +    long ret;
>>>> +
>>>> +    if ( max_port > EVTCHN_MAX_PORT_UNLIMITED )
>>>> +        return -EINVAL;
>>>> +
>>>> +    d = rcu_lock_domain_by_id(set_limit->domid);
>>>> +    if ( !d )
>>>> +        return -ESRCH;
>>>> +
>>>> +    ret = xsm_evtchn_set_limit(XSM_DM_PRIV, d);
>>>> +    if ( ret )
>>>> +        goto out;
>>>> +
>>>> +    spin_lock(&d->event_lock);
>>>> +
>>>> +    d->max_evtchn_port = max_port;
>>>
>>> So you allow this to be set even if the L2 ABI is in use. Does this
>>> make sense? Is this consistent?
>>
>> I think it would be confusing if guests could subvert the limit by using
>> a different ABI, even if it doesn't really make much difference from a
>> resource usage point of view.
> 
> Somehow I'm getting the impression we're no understanding one
> another. My questions were:
> - What's the point of permitting use of this function for a guest
>   using the 2-level ABI?

So any administrator set limit is consistently applied regardless of
which ABI a guest uses.

> - If you consider it valid to be used by a 2-level guest, will the
>   result be consistent (will the guest see the limit enforced, and is
>   there no implicit assumption somewhere that 2-level guests can
>   always use the so far statically limited number of event channels)?

The limit is enforced (see the checks in get_free_port()) for all ABIs:

     for ( port = 0; port_is_valid(d, port); port++ )
     {
         if (port > d->max_evtchn_port )
             return -ENOSPC;
         if ( evtchn_from_port(d, port)->state == ECS_FREE )
             return port;
     }

     if ( port == d->max_evtchns || port > d->max_evtchn_port )
         return -ENOSPC;

>>>> @@ -1189,6 +1229,11 @@ void evtchn_check_pollers(struct domain *d, unsigned port)
>>>>  
>>>>  int evtchn_init(struct domain *d)
>>>>  {
>>>> +    if ( is_control_domain(d) )
>>>> +        d->max_evtchn_port = EVTCHN_MAX_PORT_UNLIMITED;
>>>> +    else
>>>> +        d->max_evtchn_port = EVTCHN_MAX_PORT_DEFAULT;
>>>> +
>>>>      /* Default to N-level ABI. */
>>>>      evtchn_2l_init(d);
>>>
>>> Similarly here - you set limits that are not consistent with the default
>>> L2 ABI.
>>
>> I'm not sure why you think they are inconsistent, the limits set here
>> are such that there is no regression in the number of usable event
>> channels.  A guest is still limited by the maximum supported by any ABI.
>> i.e., the limit is min(d->max_evtchn_port, d->max_evtchns-1).
> 
> I asked because the limit for a 2-level Dom0 is now wrong. But you
> ought to read this in the context of the questions above, i.e. if all's
> consistent (and the max() you point out is indeed consistently
> enforced), then there is no issue.

The maximum supported by the ABI and the administratively set limit are
independent constraints.  get_free_port() makes sure neither constraint
is exceeded.

David