From mboxrd@z Thu Jan 1 00:00:00 1970 From: DuanZhenzhong Subject: Re: [PATCH 4.1] x86: fix emuirq regression from XSA-21 fix Date: Tue, 25 Jun 2013 15:44:10 +0800 Message-ID: <51C94A4A.8050904@oracle.com> References: <20130514142013.GA10173@konrad-lan.dumpdata.com> <5195944A.3050608@oracle.com> <20130520175706.GA27973@phenom.dumpdata.com> <20130520203855.GA30616@phenom.dumpdata.com> <519B474E.4000202@citrix.com> <20130521134059.GE492@phenom.dumpdata.com> <51AECC3A.7060803@oracle.com> <51C26F98.8070103@oracle.com> <51C7F2FE.3090803@oracle.com> <51C92B8F.6010904@oracle.com> <51C9611802000078000E0336@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51C9611802000078000E0336@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@lists.xensource.com" , Stefano Stabellini , Andrew Cooper , Konrad Rzeszutek Wilk , Feng Jin , Yuval Shaia , Chien Yen , David Vrabel List-Id: xen-devel@lists.xenproject.org Jan Beulich wrote: >>>> On 25.06.13 at 07:33, DuanZhenzhong wrote: >>>> >> Stefano Stabellini wrote: >> >>> On Mon, 24 Jun 2013, Zhenzhong Duan wrote: >>> >>>> Could you have a look if there is something wrong in xen side of clearing >>>> the mapping? >>>> >>> What I am saying is that the error you are getting: >>> >>> pt_msix_disable: Unbind msix with pirq 67, gvec 0 >>> pt_msix_disable: Unmap msix with pirq 67 >>> pt_msix_disable: Error: Unmapping of MSI-X failed. [00:04.0] >>> >>> cannot be caused by domain_pirq_to_emuirq(d, pirq) returning >>> IRQ_UNBOUND. >>> So, why are you getting this error? What is failing? >>> I am ready to believe the problem is in Xen but Without understanding >>> why you are getting the error it's hard to find a solution. >>> >>> >> I found the reason, you are looking at xen-unstable, I was working with >> 4.1.30-OVM, it has patch of CVE-2012-4536 / XSA-21. >> That patch set ret to -EINVAL initially. After remove that line, unmap >> succeed. >> > > Removing that line certainly isn't right. The proper fix is the one > below/attached. > > Jan > > **************************************************** > x86: fix emuirq regression from XSA-21 fix > > The XSA-21 patch broke the assumption of "ret" being zero prior to the > IRQ_UNBOUND check. > > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/physdev.c > +++ b/xen/arch/x86/physdev.c > @@ -243,6 +243,8 @@ static int physdev_unmap_pirq(struct phy > spin_lock(&d->event_lock); > if ( domain_pirq_to_emuirq(d, unmap->pirq) != IRQ_UNBOUND ) > ret = unmap_domain_pirq_emuirq(d, unmap->pirq); > + else > + ret = 0; > spin_unlock(&d->event_lock); > if ( unmap->domid == DOMID_SELF || ret ) > goto free_domain; > > > > Any reason of not right? below patch does work as ret is already zero if it could get to that line. zduan **************************************************** x86: fix emuirq regression from XSA-21 fix The XSA-21 patch broke the assumption of "ret" being zero prior to the IRQ_UNBOUND check. Signed-off-by: Zhenzhong Duan --- a/xen/arch/x86/physdev.c +++ b/xen/arch/x86/physdev.c @@ -240,7 +240,6 @@ static int physdev_unmap_pirq(struct phy if ( ret ) return ret; - ret = -EINVAL; if ( unmap->pirq < 0 || unmap->pirq >= d->nr_pirqs ) goto free_domain;