From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2] domctl: fix IRQ permission granting/revocation Date: Fri, 12 Dec 2014 16:34:45 +0000 Message-ID: <1418402085.16425.35.camel@eu.citrix.com> References: <548AD05D020000780004F329@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XzTBE-0000Z1-Qa for xen-devel@lists.xenproject.org; Fri, 12 Dec 2014 16:35:00 +0000 In-Reply-To: <548AD05D020000780004F329@mail.emea.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: Keir Fraser , Tim Deegan , Stefano Stabellini , Ian Jackson , xen-devel List-Id: xen-devel@lists.xenproject.org On Fri, 2014-12-12 at 10:24 +0000, Jan Beulich wrote: > Commit 545607eb3c ("x86: fix various issues with handling guest IRQs") > wasn't really consistent in one respect: The granting of access to an > IRQ shouldn't assume the pIRQ->IRQ translation to be the same in both > domains. In fact it is wrong to assume that a translation is already/ > still in place at the time access is being granted/revoked. > > What is wanted is to translate the incoming pIRQ to an IRQ for > the invoking domain (as the pIRQ is the only notion the invoking > domain has of the IRQ), and grant the subject domain access to > the resulting IRQ. > > Signed-off-by: Jan Beulich Acked-by: Ian Campbell > --- > v2: Also fix initial range check to use current->domain, adjust code > structure, and extend description (all requested by Ian). Along > the lines of the first mentioned change, also pass the Xen IRQ > number to the XSM hook (confirmed okay by Daniel). > Note that I would hope for this to make unnecessary Stefano's proposed > tools side change > http://lists.xenproject.org/archives/html/xen-devel/2014-12/msg00160.html. > > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -982,18 +982,21 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe > > case XEN_DOMCTL_irq_permission: > { > - unsigned int pirq = op->u.irq_permission.pirq; > + unsigned int pirq = op->u.irq_permission.pirq, irq; > int allow = op->u.irq_permission.allow_access; > > - if ( pirq >= d->nr_pirqs ) > + if ( pirq >= current->domain->nr_pirqs ) > + { > ret = -EINVAL; > - else if ( !pirq_access_permitted(current->domain, pirq) || > - xsm_irq_permission(XSM_HOOK, d, pirq, allow) ) > + break; > + } > + irq = pirq_access_permitted(current->domain, pirq); > + if ( !irq || xsm_irq_permission(XSM_HOOK, d, irq, allow) ) > ret = -EPERM; > else if ( allow ) > - ret = pirq_permit_access(d, pirq); > + ret = irq_permit_access(d, irq); > else > - ret = pirq_deny_access(d, pirq); > + ret = irq_deny_access(d, irq); > } > break; > > --- a/xen/include/xen/iocap.h > +++ b/xen/include/xen/iocap.h > @@ -28,22 +28,11 @@ > #define irq_access_permitted(d, i) \ > rangeset_contains_singleton((d)->irq_caps, i) > > -#define pirq_permit_access(d, i) ({ \ > - struct domain *d__ = (d); \ > - int i__ = domain_pirq_to_irq(d__, i); \ > - i__ > 0 ? rangeset_add_singleton(d__->irq_caps, i__)\ > - : -EINVAL; \ > -}) > -#define pirq_deny_access(d, i) ({ \ > - struct domain *d__ = (d); \ > - int i__ = domain_pirq_to_irq(d__, i); \ > - i__ > 0 ? rangeset_remove_singleton(d__->irq_caps, i__)\ > - : -EINVAL; \ > -}) > #define pirq_access_permitted(d, i) ({ \ > struct domain *d__ = (d); \ > - rangeset_contains_singleton(d__->irq_caps, \ > - domain_pirq_to_irq(d__, i));\ > + int irq__ = domain_pirq_to_irq(d__, i); \ > + irq__ > 0 && irq_access_permitted(d__, irq__) \ > + ? irq__ : 0; \ > }) > > #endif /* __XEN_IOCAP_H__ */ > > >