[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH 2/7] xen: introduce _evtchn_alloc_unbound
On 08.01.2022 01:49, Stefano Stabellini wrote: > @@ -284,11 +285,32 @@ void evtchn_free(struct domain *d, struct evtchn *chn) > xsm_evtchn_close_post(chn); > } > > -static int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc) > +struct evtchn *_evtchn_alloc_unbound(struct domain *d, domid_t remote_dom) Function names want to be the other way around, to be in line with naming rules of the C spec: The static function may be underscore- prefixed, while the non-static one may not. > { > struct evtchn *chn; > + int port; > + > + if ( (port = get_free_port(d)) < 0 ) > + return ERR_PTR(port); > + chn = evtchn_from_port(d, port); > + > + evtchn_write_lock(chn); > + > + chn->state = ECS_UNBOUND; > + if ( (chn->u.unbound.remote_domid = remote_dom) == DOMID_SELF ) > + chn->u.unbound.remote_domid = current->domain->domain_id; I think the resolving of DOMID_SELF should remain in the caller, as I'm pretty sure your planned new user(s) can't sensibly pass that value. > + evtchn_port_init(d, chn); > + > + evtchn_write_unlock(chn); > + > + return chn; > +} > + > +static int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc) > +{ > + struct evtchn *chn = NULL; I don't think the initializer is needed. > @@ -297,27 +319,22 @@ static int evtchn_alloc_unbound(evtchn_alloc_unbound_t > *alloc) > > spin_lock(&d->event_lock); > > - if ( (port = get_free_port(d)) < 0 ) > - ERROR_EXIT_DOM(port, d); > - chn = evtchn_from_port(d, port); > + chn = _evtchn_alloc_unbound(d, alloc->remote_dom); > + if ( IS_ERR(chn) ) > + { > + rc = PTR_ERR(chn); > + ERROR_EXIT_DOM(rc, d); > + } > > rc = xsm_evtchn_unbound(XSM_TARGET, d, chn, alloc->remote_dom); > if ( rc ) > goto out; > > - evtchn_write_lock(chn); > - > - chn->state = ECS_UNBOUND; This cannot be pulled ahead of the XSM check (or in general anything potentially resulting in an error), as check_free_port() relies on ->state remaining ECS_FREE until it is known that the calling function can't fail anymore. > - if ( (chn->u.unbound.remote_domid = alloc->remote_dom) == DOMID_SELF ) > - chn->u.unbound.remote_domid = current->domain->domain_id; > - evtchn_port_init(d, chn); > - > - evtchn_write_unlock(chn); > - > - alloc->port = port; > + alloc->port = chn->port; > > out: > - check_free_port(d, port); > + if ( chn != NULL ) > + check_free_port(d, chn->port); Without the initializer above it'll then be more obvious that the condition here needs to be !IS_ERR(chn). Also (nit) please prefer the shorter "if ( chn )". Overall I wonder in how far it would be possible to instead re-use PV shim's "backdoor" into port allocation: evtchn_allocate_port() was specifically made available for it, iirc. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |