[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 09/11] evtchn: implement EVTCHNOP_set_limit
On 16/09/13 08:07, Jan Beulich wrote: >>>> On 13.09.13 at 18:56, David Vrabel <david.vrabel@xxxxxxxxxx> 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. >> @@ -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). However, I'm going to change it so the hypervisor always sets the limit to unlimited. The toolstack should be responsible for setting any limits (and picking a sensible default). >> --- a/xen/include/public/event_channel.h >> +++ b/xen/include/public/event_channel.h >> @@ -308,6 +308,9 @@ struct evtchn_set_limit { >> }; >> typedef struct evtchn_set_limit evtchn_set_limit_t; >> >> +#define EVTCHN_MAX_PORT_UNLIMITED ((1u << 31) - 1) >> +#define EVTCHN_MAX_PORT_DEFAULT (NR_EVENT_CHANNELS - 1) > > Does the former really need to be part of the ABI? And does it > really need to be 2^31-1 (rather than 2^32-1)? The hypervisor uses int for port in places (e.g., get_free_port() where it returns a port number or a negative error code). I will remove UNLIMITED from the ABI and set_limit will map any limit > UNLIMITED to UNLIMITED. At some point some one should go a change all the uses for port number to unsigned but I think this is work independent of this series. David _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |