From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 1/2] VMX: fix VMCS race on context-switch paths Date: Thu, 09 Nov 2017 10:54:51 +0100 Message-ID: <1510221291.4517.170.camel@linux.it> References: <58A596C0020000780013AA84@prv-mh.provo.novell.com> <58A597D8020000780013AAAF@prv-mh.provo.novell.com> <5ca9f140-a574-a8d0-1231-4ce0aec0e124@citrix.com> <5A0177B8020000780018CCC9@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3701647120533581806==" Return-path: Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eCjYJ-0005Pi-0o for xen-devel@lists.xenproject.org; Thu, 09 Nov 2017 09:55:15 +0000 Received: by mail-wm0-f53.google.com with SMTP id s66so529884wmf.2 for ; Thu, 09 Nov 2017 01:55:11 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Igor Druzhinin , Jan Beulich Cc: Sergey Dyasli , Kevin Tian , Andrew Cooper , Anshul Makkar , Jun Nakajima , xen-devel List-Id: xen-devel@lists.xenproject.org --===============3701647120533581806== Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-uJK1h5RnxtrXM+EcMtrH" --=-uJK1h5RnxtrXM+EcMtrH Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2017-11-07 at 14:24 +0000, Igor Druzhinin wrote: > Perhaps I should improve my diagram: >=20 > pCPU1: vCPUx of domain X -> migrate to pCPU2 -> switch to idle > context > -> RCU callbacks -> vcpu_destroy(vCPUy of domain Y) -> > vmx_vcpu_disable_pml() -> vmx_vmcs_clear() (VMCS is trashed at this > point on pCPU1) >=20 > pCPU2: context switch into vCPUx -> vCPUx.is_running =3D 1 -> TLB flush > from context switch to clean TLB on pCPU1 >=20 Sorry, there must be something I'm missing (or misunderstanding). What is this code that checks is_running and triggers the TLB flush? But, more important, how come you are context switching to something that has is_running =3D=3D 1 ? That should not be possible. In fact, from a scheduling code perspective, since you're mentioning vCPU migration between pCPUs: pCPU1 . . //vCPUx->is_running is 1 vCPUx->pause_flags |=3D _VPF_migrating schedule() idle->is_running =3D 1 //vCPUx->pause_flags !=3D 0 =3D=3D> it's blocked and can't be scheduled! context_switch( prev=3DvCPUx, next=3Didle ) set_current( idle ) //let's be lazy! don't call __context_switch() context_saved( vCPUx ) vCPUx->is_running =3D 0 SCHED_OP( context_saved ) //NULL for Credit1 vcpu_migrate( vCPUx ) if ( vCPUx->is_running || !test_and_clear(_VPF_migrating) ) return; vcpu_wake( vCPUx ) . . . So, basically, the scheduler on pCPU2 can decide to pick vCPUx from the runqueue and switch to it _only_ if it has gone through vcpu_wake(), which must actually have woken up it, which happens if _VPF_migrating has been cleared, which means is_running was 0 already. Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli --=-uJK1h5RnxtrXM+EcMtrH Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEES5ssOj3Vhr0WPnOLFkJ4iaW4c+4FAloEJewACgkQFkJ4iaW4 c+4xGxAA1LBLLnJJVHzWWMblCFiNRG0C9Gb58zRTia5G6E/g7lKjOJ180Ea9d1ty YPbBsPOKtkcClf+vJPwqo37c4nH+QuhN5RjegOasMz/HEkiOxKhlNl7OvCbIF27L 0XO710RbSb6lShyNusX2ypgzPC4JVlvdkOS+3nCivXLxcUPYwDAR6firxaFKElcA zegJsbunZ6LbVqNDbIJdab7/g7syEN158ZZ7uo8leA/3B4JNzK4BU/cfUFHopy1o VRPQ8rlBbikL/FNnn89V+873BRHOX9hinSRywVmvsGswnWsvQN7D/ttAEUwsbDyq H0lRWBpdiKFQf0Ywkz845zYm7wLn4+cCc9YJQCI20aBBd85GxXQC/blrXn1CHe78 OztwnU68tAESsj8UdUMDFDSJscVIsYHtkysFzIYrnAp1brrwIpnW9tcbd95yuFYs owWhV/3k85lSjxVG7Tyhdd6Ne1252JFu80eVNB+WXW39KynZ39hcTmtiWEK5eLhR tSxwMmFn7gcXgv0+Pn40K8TCHPhvE2FCKMVdIN/2OBUReQQAl2dZkuKebpk95zAm R+stF8i7YSIX8gWLwHnn8nJ1wQT35JQNGGGTIeuS9cdDFsUQnU+ZY+Voz/1C4ZWU mIDEycMr1g+2Fl66Ch1KsVBT5QPG/1o+RQFSPzI9MM2eVBJqz/o= =jSi0 -----END PGP SIGNATURE----- --=-uJK1h5RnxtrXM+EcMtrH-- --===============3701647120533581806== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============3701647120533581806==--