From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [xen-unstable-smoke test] 112957: regressions - trouble: broken/fail/pass Date: Tue, 5 Sep 2017 17:50:53 +0200 Message-ID: <1504626653.338.4.camel@citrix.com> References: <2017e7bc-aec8-ec04-89b7-46e59020ce16@citrix.com> <7e3c4fda-eb04-2595-bed7-9a459ba1d6e7@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6416700514225300836==" Return-path: In-Reply-To: <7e3c4fda-eb04-2595-bed7-9a459ba1d6e7@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: George Dunlap , Julien Grall , Andrew Cooper , osstest service owner , xen-devel@lists.xensource.com, andre.przywara@arm.com Cc: George Dunlap , Julien Grall , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org --===============6416700514225300836== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-Cw5ADrIXzKmq2EoTupiz" --=-Cw5ADrIXzKmq2EoTupiz Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2017-09-04 at 09:46 +0100, George Dunlap wrote: > On 09/02/2017 04:39 PM, Julien Grall wrote: > >=20 > > If I am not mistaken the hypervisor stack is per-vCPU. So when you > > move the > > vCPU to another pCPU, the stack will be moved. > > This means the smp_processor_id() will return a different value. > > Isn't it > > the same on x86? >=20 > No, the hypervisor stack on x86 has always been per-pcpu.=C2=A0=C2=A0Appa= rently > the powerpc port was per-vcpu, which is why the smp_processor_id() > was > there.=C2=A0=C2=A0I (and apparently Dario) assumed the ARM implementation= was > the > same as x86, which is why I checked in this change. >=20 So, AFAIUI, the reason why the re-sampling at all iterations was introduced (in ae9bfcdc, "[XEN] Various softirq cleanups") was that, on IA64 (not powerpc :-D), actual context_switch() returns. Basically, we are in do_softirq(), with SCHEDULE_SOFTIRQ set, so we call the handler, which is schedule() (__enter_scheduler(), back at the time), which calls context_switch(), which switch the stack. On x86, context_switch() does not 'return', it jumps (via schedule_tail()) to trying to resume guest context (of the to be scheduled vCPU, which may be a different one). During that path, we do check softirqs again, and we may go back to do_softirq(), but if we do, we execute the function from its entry point, and hence we re- initialize cpu, outside of the loop. OTOH, on IA64, context_switch(), and hence schedule() (__enter_scheduler()), does a regular 'return'. So, we go back to the for(;;) loop in do_softirq(), with (I think, but I don't speak any IA64 :-/) the stack changed. And that's why we need to refresh the content of the 'cpu' local variable. So, I now think that what I did not understand, when looking at ARM code, was that context_switch() does indeed return, and hence we do at least another step inside the loop, and hit the ASSERT(), which I guess may trigger if what's in spite of the local variable 'cpu', in the new stack, is different than smp_processor_id(). Re-checking things now, I actually do see that context_switch() on ARM is not 'terminal'. It call schedule_tail(), which on x86 does not return, while in ARM, it does. I must have confused these two... Sorry. Is this analysis correct? Also, mostly out of curiosity, still looking at ARM code, I'm not getting at all how continue_new_vcpu() works (e.g., when/how is it invoked?). Thanks and Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-Cw5ADrIXzKmq2EoTupiz 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 iQIcBAABCAAGBQJZrsffAAoJEBZCeImluHPuNCcQAJnI2A3r8OBOP5+v6wwhODQc 7nEuhK1pnYiuoqcvXPD5e24P6Q7Pff0OEb8c8t+CQkU+dwQUfIO2zKnRlX9H1FKL TSpevi8xVUaJj6TixyUFLfNb4DCaXAg1Yp+9AJ1oZV33zfA+CkHumPK+s1euIYi3 cg7s0YOn2yDKbrM9ZZmmiezglyQpFErmCO5/BM2xjxl9oy1/DIv8a4wO549RpEuJ e8Fi3ZnROsavTf90Gz5ADV2W6Qdeji9fu5mQOIz5utnhk0xOR1m2t8JUUtkR/TYh OaaaUmoINJqGrO2T6WGUW6kE9gjgHo9zlUaGp3P0dXoJj7ipkcmuInKlsBI7J+9n E04PaLO49f4t5M7omCrLsfU9OGGXSSg/RouYToIR8CFpnpNoC6UKHbg9bjZt+dub t/OPSKhtvrdeslGOQLRgvGaINQV9++X3+FzRL78RAKGF5t1fyg6Jpnzra/Lt816k Rj1fYdwejwNswnjFDqt+h+RSZOv+bevB0DKf53MWYeUckQokgLdHGhkNBLw8d8Mv yIxu6XPRE4KGvWRQhquepVfLltCpZijFXemnI/DZL1QEjsGdRH161hKgWnU9cwcY JXeuy72OKV8KakFcZvrt4j0sDR5DKZBRuFuXnY0/4xE2evCwgGTEihkQpzp/gsUw 5qWsgw/owtXCfkL3BKTw =hW6s -----END PGP SIGNATURE----- --=-Cw5ADrIXzKmq2EoTupiz-- --===============6416700514225300836== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============6416700514225300836==--