From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v3 2/6] VMX: Properly handle pi when all the assigned devices are removed Date: Tue, 6 Sep 2016 10:58:55 +0200 Message-ID: <1473152335.19612.89.camel@citrix.com> References: <1472615791-8664-1-git-send-email-feng.wu@intel.com> <1472615791-8664-3-git-send-email-feng.wu@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5086035933755438342==" Return-path: In-Reply-To: <1472615791-8664-3-git-send-email-feng.wu@intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Feng Wu , xen-devel@lists.xen.org Cc: george.dunlap@eu.citrix.com, andrew.cooper3@citrix.com, kevin.tian@intel.com, jbeulich@suse.com List-Id: xen-devel@lists.xenproject.org --===============5086035933755438342== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-0Auk/v0PQfOm64Qp5m50" --=-0Auk/v0PQfOm64Qp5m50 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2016-08-31 at 11:56 +0800, Feng Wu wrote: > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > @@ -158,14 +158,12 @@ static void vmx_pi_switch_to(struct vcpu *v) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0pi_clear_sn(pi_desc); > =C2=A0} > =C2=A0 Not terribly important, but what about calling this: > -static void vmx_pi_do_resume(struct vcpu *v) > +static void vmx_pi_remove_vcpu_from_blocking_list(struct vcpu *v) vmx_pi_list_del() or vmx_pi_list_remove() > @@ -198,6 +196,21 @@ static void vmx_pi_do_resume(struct vcpu *v) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0spin_unlock_irqrestore(pi_blocking_list_loc= k, flags); > =C2=A0} > =C2=A0 > +static void vmx_pi_do_resume(struct vcpu *v) > +{ > +=C2=A0=C2=A0=C2=A0=C2=A0ASSERT(!test_bit(_VPF_blocked, &v->pause_flags))= ; > + > +=C2=A0=C2=A0=C2=A0=C2=A0vmx_pi_remove_vcpu_from_blocking_list(v); > +} > + And this: > +static void vmx_pi_blocking_cleanup(struct vcpu *v) > vmx_pi_list_cleanup() etc.? I.e., using shorter and more consistent names. > +{ > +=C2=A0=C2=A0=C2=A0=C2=A0if ( !iommu_intpost ) > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > + At least as far as this patch is concerned, you are only calling this function from vmx_pi_hooks_deassing() which already checks at the very beginning iommu_intpost to be true, or it returns, and you won't get here. So you don't need to re-check the same thing here. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-0Auk/v0PQfOm64Qp5m50 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJXzoVQAAoJEBZCeImluHPuxDkQAJo9p/3RlW1i8T/lM03TUhbJ jN2UXiVAGCzdcQ2ml/4P2j7F3wB8cwX9oT+XUJyid9Dm35eow3fNPLsVySQzM8qt j1OzwD4ssG+cfybbNwAh0wb5K5RZs4xh5YaVtpmP0lnow/5bDPqvpf8T3KwG75a4 X9HxoxYSNvUCVn8aVmbPHVY++QPvXFN9Ll/3iUH6xX3+JERzXUWqHtiNuV0PLaHG tj+REys/FDKgy9Wvt0SzBdO/uotJAtJzpsrHTA8cn8/mHfpW8frMngpHv3A4kcqp 2rrpilfMrvbDwW+cgtvLXncPNw19RhkL7Gv8Zfvy2iKQ7XXeTlvP8S4EaJqG/MJ/ GKmbDkfjlGPhIwBsy60qosi78gX+EqYUl/o3GCy3m3pfseUkTvQ1FBRC1SS9tDin UAGyrxZBMZ3dN6KerwflxnOm3wjYMdSkuNB1WnYCfpMgG3c+3Xfaqdf8ZGvTrNZA 1rIxPpkbdJoTLa+vKMEB+T4mH7ru9ptTUHpW9jFkAPfn4SyY6MiSB9OzLHiHeN5u 4nsg7tgdmHUlMRnmJcl7S61aLUqQZcYo2jLOnwOG5sAflnO366M2qlEjP4pCpyMh t1UvuOZchrJQW9hZ55LFDo7HUqrlgK19HZ+fOJahDOo3mlX5AGNoswllO7ve3YVo 8wIOwsQNHG4lxWTOyl2C =15Co -----END PGP SIGNATURE----- --=-0Auk/v0PQfOm64Qp5m50-- --===============5086035933755438342== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============5086035933755438342==--