From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH 09/11] evtchn: implement EVTCHNOP_set_limit Date: Mon, 16 Sep 2013 11:00:11 +0100 Message-ID: <5236D6AB.5050708@citrix.com> References: <1379091373-30293-1-git-send-email-david.vrabel@citrix.com> <1379091373-30293-10-git-send-email-david.vrabel@citrix.com> <5236CA6E02000078000F37FF@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VLVbN-00043I-EN for xen-devel@lists.xenproject.org; Mon, 16 Sep 2013 10:00:17 +0000 In-Reply-To: <5236CA6E02000078000F37FF@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Daniel De Graaf , Keir Fraser List-Id: xen-devel@lists.xenproject.org On 16/09/13 08:07, Jan Beulich wrote: >>>> On 13.09.13 at 18:56, David Vrabel 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