From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50882) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fl7f0-00050m-AM for qemu-devel@nongnu.org; Thu, 02 Aug 2018 03:04:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fl7ey-0001I0-UQ for qemu-devel@nongnu.org; Thu, 02 Aug 2018 03:04:34 -0400 Received: from ozlabs.org ([2401:3900:2:1::2]:47653) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fl7ey-00011C-3h for qemu-devel@nongnu.org; Thu, 02 Aug 2018 03:04:32 -0400 Date: Thu, 2 Aug 2018 14:08:30 +1000 From: David Gibson Message-ID: <20180802040830.GB11211@umbus.fritz.box> References: <153252992640.319494.8451297710133862507.stgit@bahia.lan> <20180727052724.GJ3694@umbus.fritz.box> <20180727095452.45edee8b@bahia> <20180730055715.GD2708@umbus.fritz.box> <20180730104145.109ef196@bahia.lan> <20180731032559.GH2708@umbus.fritz.box> <20180801152430.3f1c5cee@bahia.lan> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="GID0FwUMdk1T2AWN" Content-Disposition: inline In-Reply-To: <20180801152430.3f1c5cee@bahia.lan> Subject: Re: [Qemu-devel] [PATCH for-3.1] tests/cpu-plug-test: check CPU hotplug on ppc64 with KVM List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: qemu-devel@nongnu.org, Thomas Huth , =?iso-8859-1?Q?C=E9dric?= Le Goater --GID0FwUMdk1T2AWN Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 01, 2018 at 03:24:30PM +0200, Greg Kurz wrote: > On Tue, 31 Jul 2018 13:25:59 +1000 > David Gibson wrote: > [...] > > >=20 > > > The CPU hotplug test doesn't seem to do anything on the guest side: it > > > just checks that 'device_add' returns a response that isn't an > > > error. =20 > >=20 > > Right, which makes this ill suited to being a qtest test. The whole > > point of qtest is making it easier to test qemu peripherals without > > having to have specific test code within the guest, by essentially > > replacing the guest's cpu with a stub controlled by the test harness. > > That's what the qtest accel is all about. > >=20 >=20 > I agree with what a qtest test should be, but cpu-plug-test doesn't > do anything like that obviously, ie, the guest CPU does nothing at > all. Only the hotplug path of the QEMU device model that don't need > the guest to run is tested here. Right, so I'm suggesting this be changed to a non-qtest test. Those are fine to have -machine accel=3Dkvm:tcg. Of course that doesn't prevent later adding a more detailed cpu hotplug qtest that *does* do actions on the guest side. > The more general issue is that paths guarded with kvm_enabled() cannot > be tested with a genuine qtest test. That's really unfortunate since > these paths are sometimes the one that are mostly used on the field, > eg, in-kernel XICS versus emulated XICS. It makes sense in the context of what qtests are for. They're designed to test the emulated peripherals in isolation from the core emulation (whether that's via tcg or kvm). > > I think it's confusing to have a test which tries things with both > > qtest and kvm accelerators. Looking like a qtest test, people might > > reasonably think they can extend/refine the test to check behaviour > > when the guest does respond to the hotplug events. But such an > > extension won't work with the kvm accel, because the qtest code used > > to simulate that guest response won't have any effect with kvm. > >=20 >=20 > If the motivation is to let the test be a true qtest in case someone > wants to emulate some guest behavior, I agree the kvm change is wrong. >=20 > > > I'm not aware that the guest is expected to have a specific behavior > > > during 'device_add', apart from not crashing or hanging. That was the > > > initial idea behind passing '-S' to ensure the guest doesn't run. =20 > >=20 > > Note that with qtest (or -S) we don't even test that minimal > > condition. We only test that *qemu* doesn't crash - it could fatally > > compromise the guest and the test would never know. > >=20 >=20 > True. >=20 > > > Your remark seems to be more general though... are you meaning that > > > doing something like qtest_start("-machine accel=3Dkvm:tcg") is just > > > wrong ? =20 > >=20 > > Pretty much, yes. A non-qtest test which does that is reasonable, but > > not a qtest test. > >=20 >=20 > So, instead of hijacking the current qtest, we may add a non-qtest test > that would start QEMU with "-machine accel=3Dkvm:tcg -S". This would allow > at least to test that QEMU doesn't crash right away. And, as suggested > by Thomas, the coverage could include SLOF as well if we don't pass -S. > But I would need to understand why SLOF sometimes hits a 0x700 when > running cpu-plug-test with this patch applied... Yup. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --GID0FwUMdk1T2AWN Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEdfRlhq5hpmzETofcbDjKyiDZs5IFAltig74ACgkQbDjKyiDZ s5I15w//WRkV9UAyliXvKZ7todRYPh94yHGFKf0ZQrDhV4N2Tk1KsQM/AuP/1dDV q6uEABCimj5VZ1ETgHk/69yF8JqP0FIr7VilWiZHybyI1bNMQSg14Yglzvdhk75v shWXNtNaSz1jzDZdONSqRH5K7PgVsq0ntq2R/b4iuBf+ywm99IrOmvxwkM+VwBwy mBPC2+JeSDah3Zad8OQMdqJhDnJdh5J4EB+4Fv1MEevxyPfKx0pLhEczMWeFepwH cJ45/Tc/Uqdp03l/eFYNbuG1UJJ20WBLxtlHBIooNDdIp5dGPHMX+3OHQY1tzhIE TtVOAJb13HinSVvERwtjPbXPNUTcOO1RRpORcjZHx4xJhwOKb0Du2cSy7mf3v8U9 0HdA3lQ1av3LeiGGF4OmFzFGyWLa3SQAzBPSJcxYV5n4gmP8CI9V5E6vuiIjGFiH VFrVhymOOTV/YCmjzIbL5xrZ1Mz8Bjuk1TxiXOnBiiJCFHhsPgi/zVcRdX8/pEaU xRF1H4IvwlKxaCszbVCzbSP0KOXo1u7B3JEupChCxqlX+gQJyYslwCBjh2d1Kwz6 7zU4lXCvybWUUTcAmxTmqr9e5Lo6P/DCe6jHnKfAO6Q7WgTJZ1ik3NXSN5vyswk6 x6LG/sotZtsyMn3fpdIH8tF2cv4UNho46/Uso/zz2ktC7AoN+Gc= =s0M7 -----END PGP SIGNATURE----- --GID0FwUMdk1T2AWN--