From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH 2/4] x86/vMSI-X: drop list lock Date: Wed, 08 Jun 2016 06:53:35 -0600 Message-ID: <5758316F02000078000F30A6@prv-mh.provo.novell.com> References: <5758302D02000078000F3087@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part2D1B2F5F.1__=" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bAczN-0007YJ-Iy for xen-devel@lists.xenproject.org; Wed, 08 Jun 2016 12:53:41 +0000 In-Reply-To: <5758302D02000078000F3087@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: xen-devel Cc: Andrew Cooper 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. --=__Part2D1B2F5F.1__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline msixtbl_pt_{,un}register() already run with both the PCI devices lock and the domain event lock held, so there's no need for another lock. Just to be on the safe side, acquire the domain event lock in the cleanup function (albeit I don't think this is strictly necessary). Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/vmsi.c +++ b/xen/arch/x86/hvm/vmsi.c @@ -468,8 +468,6 @@ int msixtbl_pt_register(struct domain *d =20 pdev =3D msi_desc->dev; =20 - spin_lock(&d->arch.hvm_domain.msixtbl_list_lock); - list_for_each_entry( entry, &d->arch.hvm_domain.msixtbl_list, list ) if ( pdev =3D=3D entry->pdev ) goto found; @@ -480,7 +478,6 @@ int msixtbl_pt_register(struct domain *d =20 found: atomic_inc(&entry->refcnt); - spin_unlock(&d->arch.hvm_domain.msixtbl_list_lock); r =3D 0; =20 out: @@ -530,15 +527,10 @@ void msixtbl_pt_unregister(struct domain =20 pdev =3D msi_desc->dev; =20 - spin_lock(&d->arch.hvm_domain.msixtbl_list_lock); - list_for_each_entry( entry, &d->arch.hvm_domain.msixtbl_list, list ) if ( pdev =3D=3D entry->pdev ) goto found; =20 - spin_unlock(&d->arch.hvm_domain.msixtbl_list_lock); - - out: spin_unlock_irq(&irq_desc->lock); return; @@ -547,7 +539,6 @@ found: if ( !atomic_dec_and_test(&entry->refcnt) ) del_msixtbl_entry(entry); =20 - spin_unlock(&d->arch.hvm_domain.msixtbl_list_lock); spin_unlock_irq(&irq_desc->lock); } =20 @@ -558,7 +549,6 @@ void msixtbl_init(struct domain *d) return; =20 INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list); - spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock); =20 register_mmio_handler(d, &msixtbl_mmio_ops); } @@ -566,21 +556,17 @@ void msixtbl_init(struct domain *d) void msixtbl_pt_cleanup(struct domain *d) { struct msixtbl_entry *entry, *temp; - unsigned long flags; =20 if ( !d->arch.hvm_domain.msixtbl_list.next ) return; =20 - /* msixtbl_list_lock must be acquired with irq_disabled for check_lock= () */ - local_irq_save(flags);=20 - spin_lock(&d->arch.hvm_domain.msixtbl_list_lock); + spin_lock(&d->event_lock); =20 list_for_each_entry_safe( entry, temp, &d->arch.hvm_domain.msixtbl_list, list ) del_msixtbl_entry(entry); =20 - spin_unlock(&d->arch.hvm_domain.msixtbl_list_lock); - local_irq_restore(flags); + spin_unlock(&d->event_lock); } =20 void msix_write_completion(struct vcpu *v) --- a/xen/include/asm-x86/hvm/domain.h +++ b/xen/include/asm-x86/hvm/domain.h @@ -124,7 +124,6 @@ struct hvm_domain { =20 /* hypervisor intercepted msix table */ struct list_head msixtbl_list; - spinlock_t msixtbl_list_lock; =20 struct viridian_domain viridian; =20 --=__Part2D1B2F5F.1__= Content-Type: text/plain; name="x86-vMSI-X-drop-list-lock.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="x86-vMSI-X-drop-list-lock.patch" x86/vMSI-X: drop list lock=0A=0Amsixtbl_pt_{,un}register() already run = with both the PCI devices lock=0Aand the domain event lock held, so = there's no need for another lock.=0AJust to be on the safe side, acquire = the domain event lock in the=0Acleanup function (albeit I don't think this = is strictly necessary).=0A=0ASigned-off-by: Jan Beulich = =0A=0A--- a/xen/arch/x86/hvm/vmsi.c=0A+++ b/xen/arch/x86/hvm/vmsi.c=0A@@ = -468,8 +468,6 @@ int msixtbl_pt_register(struct domain *d=0A =0A pdev = =3D msi_desc->dev;=0A =0A- spin_lock(&d->arch.hvm_domain.msixtbl_list_lo= ck);=0A-=0A list_for_each_entry( entry, &d->arch.hvm_domain.msixtbl_lis= t, list )=0A if ( pdev =3D=3D entry->pdev )=0A goto = found;=0A@@ -480,7 +478,6 @@ int msixtbl_pt_register(struct domain *d=0A = =0A found:=0A atomic_inc(&entry->refcnt);=0A- spin_unlock(&d->arch.h= vm_domain.msixtbl_list_lock);=0A r =3D 0;=0A =0A out:=0A@@ -530,15 = +527,10 @@ void msixtbl_pt_unregister(struct domain=0A =0A pdev =3D = msi_desc->dev;=0A =0A- spin_lock(&d->arch.hvm_domain.msixtbl_list_lock);= =0A-=0A list_for_each_entry( entry, &d->arch.hvm_domain.msixtbl_list, = list )=0A if ( pdev =3D=3D entry->pdev )=0A goto = found;=0A =0A- spin_unlock(&d->arch.hvm_domain.msixtbl_list_lock);=0A-= =0A-=0A out:=0A spin_unlock_irq(&irq_desc->lock);=0A return;=0A@@ = -547,7 +539,6 @@ found:=0A if ( !atomic_dec_and_test(&entry->refcnt) = )=0A del_msixtbl_entry(entry);=0A =0A- spin_unlock(&d->arch.hvm_= domain.msixtbl_list_lock);=0A spin_unlock_irq(&irq_desc->lock);=0A = }=0A =0A@@ -558,7 +549,6 @@ void msixtbl_init(struct domain *d)=0A = return;=0A =0A INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list);=0A- = spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock);=0A =0A = register_mmio_handler(d, &msixtbl_mmio_ops);=0A }=0A@@ -566,21 +556,17 @@ = void msixtbl_init(struct domain *d)=0A void msixtbl_pt_cleanup(struct = domain *d)=0A {=0A struct msixtbl_entry *entry, *temp;=0A- unsigned = long flags;=0A =0A if ( !d->arch.hvm_domain.msixtbl_list.next )=0A = return;=0A =0A- /* msixtbl_list_lock must be acquired with = irq_disabled for check_lock() */=0A- local_irq_save(flags); =0A- = spin_lock(&d->arch.hvm_domain.msixtbl_list_lock);=0A+ spin_lock(&d->even= t_lock);=0A =0A list_for_each_entry_safe( entry, temp,=0A = &d->arch.hvm_domain.msixtbl_list, list )=0A = del_msixtbl_entry(entry);=0A =0A- spin_unlock(&d->arch.hvm_domain.msixtb= l_list_lock);=0A- local_irq_restore(flags);=0A+ spin_unlock(&d->event= _lock);=0A }=0A =0A void msix_write_completion(struct vcpu *v)=0A--- = a/xen/include/asm-x86/hvm/domain.h=0A+++ b/xen/include/asm-x86/hvm/domain.h= =0A@@ -124,7 +124,6 @@ struct hvm_domain {=0A =0A /* hypervisor = intercepted msix table */=0A struct list_head msixtbl_list;=0A- = spinlock_t msixtbl_list_lock;=0A =0A struct viridian_doma= in viridian;=0A =0A --=__Part2D1B2F5F.1__= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --=__Part2D1B2F5F.1__=--