From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH 1/3] x86/HVM: properly propagate errors from HVMOP_inject_msi Date: Tue, 03 Jun 2014 14:58:55 +0100 Message-ID: <538DF0BF0200007800017689@mail.emea.novell.com> References: <538DEF42020000780001767A@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__PartEDDF3E8F.1__=" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WrpEw-0001CE-Nh for xen-devel@lists.xenproject.org; Tue, 03 Jun 2014 13:58:59 +0000 In-Reply-To: <538DEF42020000780001767A@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: xen-devel Cc: Keir Fraser , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__PartEDDF3E8F.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline There are a number of ways this operation can go wrong, all of which got ignored so far. In the context of this I wonder whether map_domain_emuirq_pirq() returning 0 in the "already mapped" case is really intended to be that way (this is why the subsequent NULL check here can't be an ASSERT()). Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -5096,7 +5096,7 @@ static int hvmop_inject_msi( if ( rc ) goto out; =20 - hvm_inject_msi(d, op.addr, op.data); + rc =3D hvm_inject_msi(d, op.addr, op.data); =20 out: rcu_unlock_domain(d); --- a/xen/arch/x86/hvm/irq.c +++ b/xen/arch/x86/hvm/irq.c @@ -270,7 +270,7 @@ void hvm_set_pci_link_route(struct domai d->domain_id, link, old_isa_irq, isa_irq); } =20 -void hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data) +int hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data) { uint32_t tmp =3D (uint32_t) addr; uint8_t dest =3D (tmp & MSI_ADDR_DEST_ID_MASK) >> MSI_ADDR_DEST_ID_SH= IFT; @@ -292,20 +292,28 @@ void hvm_inject_msi(struct domain *d, ui /* if it is the first time, allocate the pirq */ if ( !info || info->arch.hvm.emuirq =3D=3D IRQ_UNBOUND ) { + int rc; + spin_lock(&d->event_lock); - map_domain_emuirq_pirq(d, pirq, IRQ_MSI_EMU); + rc =3D map_domain_emuirq_pirq(d, pirq, IRQ_MSI_EMU); spin_unlock(&d->event_lock); + if ( rc ) + return rc; info =3D pirq_info(d, pirq); if ( !info ) - return; - } else if (info->arch.hvm.emuirq !=3D IRQ_MSI_EMU) - return; + return -EBUSY; + } + else if ( info->arch.hvm.emuirq !=3D IRQ_MSI_EMU ) + return -EINVAL; send_guest_pirq(d, info); - return; + return 0; } + return -ERANGE; } =20 vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode); + + return 0; } =20 void hvm_set_callback_via(struct domain *d, uint64_t via) --- a/xen/include/xen/hvm/irq.h +++ b/xen/include/xen/hvm/irq.h @@ -123,7 +123,7 @@ void hvm_isa_irq_deassert( =20 void hvm_set_pci_link_route(struct domain *d, u8 link, u8 isa_irq); =20 -void hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data); +int hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data); =20 void hvm_maybe_deassert_evtchn_irq(void); void hvm_assert_evtchn_irq(struct vcpu *v); --=__PartEDDF3E8F.1__= Content-Type: text/plain; name="x86-HVM-inject-MSI-retval.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="x86-HVM-inject-MSI-retval.patch" x86/HVM: properly propagate errors from HVMOP_inject_msi=0A=0AThere are a = number of ways this operation can go wrong, all of which=0Agot ignored so = far.=0A=0AIn the context of this I wonder whether map_domain_emuirq_pirq()= =0Areturning 0 in the "already mapped" case is really intended to be = that=0Away (this is why the subsequent NULL check here can't be an = ASSERT()).=0A=0ASigned-off-by: Jan Beulich =0A=0A--- = a/xen/arch/x86/hvm/hvm.c=0A+++ b/xen/arch/x86/hvm/hvm.c=0A@@ -5096,7 = +5096,7 @@ static int hvmop_inject_msi(=0A if ( rc )=0A goto = out;=0A =0A- hvm_inject_msi(d, op.addr, op.data);=0A+ rc =3D = hvm_inject_msi(d, op.addr, op.data);=0A =0A out:=0A rcu_unlock_domain(= d);=0A--- a/xen/arch/x86/hvm/irq.c=0A+++ b/xen/arch/x86/hvm/irq.c=0A@@ = -270,7 +270,7 @@ void hvm_set_pci_link_route(struct domai=0A = d->domain_id, link, old_isa_irq, isa_irq);=0A }=0A =0A-void hvm_inject_msi(= struct domain *d, uint64_t addr, uint32_t data)=0A+int hvm_inject_msi(struc= t domain *d, uint64_t addr, uint32_t data)=0A {=0A uint32_t tmp =3D = (uint32_t) addr;=0A uint8_t dest =3D (tmp & MSI_ADDR_DEST_ID_MASK) >> = MSI_ADDR_DEST_ID_SHIFT;=0A@@ -292,20 +292,28 @@ void hvm_inject_msi(struct = domain *d, ui=0A /* if it is the first time, allocate the pirq = */=0A if ( !info || info->arch.hvm.emuirq =3D=3D IRQ_UNBOUND = )=0A {=0A+ int rc;=0A+=0A = spin_lock(&d->event_lock);=0A- map_domain_emuirq_pirq(d, = pirq, IRQ_MSI_EMU);=0A+ rc =3D map_domain_emuirq_pirq(d, = pirq, IRQ_MSI_EMU);=0A spin_unlock(&d->event_lock);=0A+ = if ( rc )=0A+ return rc;=0A = info =3D pirq_info(d, pirq);=0A if ( !info )=0A- = return;=0A- } else if (info->arch.hvm.emuirq !=3D = IRQ_MSI_EMU)=0A- return;=0A+ return = -EBUSY;=0A+ }=0A+ else if ( info->arch.hvm.emuirq = !=3D IRQ_MSI_EMU )=0A+ return -EINVAL;=0A = send_guest_pirq(d, info);=0A- return;=0A+ return = 0;=0A }=0A+ return -ERANGE;=0A }=0A =0A vmsi_deliver= (d, vector, dest, dest_mode, delivery_mode, trig_mode);=0A+=0A+ return = 0;=0A }=0A =0A void hvm_set_callback_via(struct domain *d, uint64_t = via)=0A--- a/xen/include/xen/hvm/irq.h=0A+++ b/xen/include/xen/hvm/irq.h=0A= @@ -123,7 +123,7 @@ void hvm_isa_irq_deassert(=0A =0A void hvm_set_pci_link= _route(struct domain *d, u8 link, u8 isa_irq);=0A =0A-void hvm_inject_msi(s= truct domain *d, uint64_t addr, uint32_t data);=0A+int hvm_inject_msi(struc= t domain *d, uint64_t addr, uint32_t data);=0A =0A void hvm_maybe_deassert_= evtchn_irq(void);=0A void hvm_assert_evtchn_irq(struct vcpu *v);=0A --=__PartEDDF3E8F.1__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --=__PartEDDF3E8F.1__=--