From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43556) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1braar-0007Dd-4W for qemu-devel@nongnu.org; Tue, 04 Oct 2016 21:01:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1braam-0002hx-3U for qemu-devel@nongnu.org; Tue, 04 Oct 2016 21:01:56 -0400 Received: from ozlabs.org ([103.22.144.67]:50627) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1braal-0002hO-Of for qemu-devel@nongnu.org; Tue, 04 Oct 2016 21:01:52 -0400 Date: Wed, 5 Oct 2016 10:43:52 +1100 From: David Gibson Message-ID: <20161004234352.GE18648@umbus.fritz.box> References: <1475583448-21013-1-git-send-email-clg@kaod.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="5oH/S/bF6lOfqCQb" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: =?iso-8859-1?Q?C=E9dric?= Le Goater , QEMU Developers , Greg Kurz , Laurent Vivier --5oH/S/bF6lOfqCQb Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 04, 2016 at 01:36:09PM +0100, Peter Maydell wrote: > On 4 October 2016 at 13:17, C=E9dric Le Goater wrote: > > Some test scenarios require to access memory regions using a specific > > endianness, such as a device region, but the current qtest memory > > accessors are done in native endian, which means that the values are > > byteswapped in qtest if the endianness of the guest and the host are > > different. > > > > To maintain the endianness of a value, we need a new set of memory > > accessor. This can be done in two ways: > > > > - first, convert the value to the required endianness in libqtest and > > then use the memread/write routines so that qtest accesses the guest > > memory without doing any supplementary byteswapping > > > > - an alternative method would be to handle the byte swapping on the > > qtest side. For that, we would need to extend the read/write > > protocol with an ending word : "native|le|be" and modify the tswap > > calls accordingly under the qtest_process_command() routine. > > > > The result is the same and the first method is simpler. >=20 > The difficulty with this patch is that it's hard to tell whether > it's really required, or if this is just adding an extra layer > of byteswapping that should really be done in some other location Actually, it's neither. It's not essential for anything, but it *removes* an extra layer of byteswapping that really never should have been done in the first place. > in the stack. What's the actual test case here? The current readw, readl, etc. all work in "guest endianness". But guest endianness is not well defined - there are a number of targets which can support either. And it's doubly meaningless since it's a property of the guest cpu, which we're essentially replacing with the qtest stub anyway. Furthermore "guest endianness" isn't useful. With a tiny handful of exceptions, all peripherals have their own endianness which is known independent of the target. It makes more sense for test cases to explicitly do their accesses in the correct endianness for the device, without having to compensate for the fact that it'll be swapped into the essentially arbitrary "guest endianness" along the way. Basically, the existing byteswaps, instead of removing the need for them in the testcase code, instead mean that target-conditional byteswaps will be *required* in nearly every testcase. It's a recipe for endianness-unsafe testcases. --=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 --5oH/S/bF6lOfqCQb Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJX9D61AAoJEGw4ysog2bOS58wP/3t4XW0va09rEd268tNXkNf0 YfuULVyLLA9cSlNXgl4F16U/f4Mstu1c4D45+Pw9jYJzVYtJL2HBK+3ruHHVpu3s 0NzQvzRWiddi1OQGeBEDAhgFyptGUwYUUie+wtGVGNpW1+/nL8sJilBtfygKhbkt vpcYkJ3Z7278h4c8AWD8Xti2vyvAOCnDFlYRSdRRvXlJPR6FqJTFTlWA0R/B0wCe BXHFiFc8ui5/vFAXrU9cqlYoEkYPHQO9k9g4AueNU0UkPhCLVxl2LOaJcizbeo4B Uh3EUl3rtoI1YLZhWWUpFpJojx0djr66k2O3b+XU4g01fDehppvWGfqX78yhYne+ JbEEC+4Ut2V/eXDvUfTV2TYStVo9CSS7aKL2x/Db1DiovdQmgeBzCHxgIgUT2ZqF B0RjRJb0q0P2e6EDAIt0vE6sHV5L4u2rFiEwGJWfy3RUDy4325S4APvn6KnVk4qX arBO78mWXQwxvJXvsPaAhS5m1TCrPgRR5a5mGSjrhfRYZJEeUwr9siR/IUJc+KnA 1XUne6qEkyuNH4hHehzqzscxxaUasAk6NY3ivo0w5S0orC484qAYtHsaL/pB16R4 ooe9U6wza4SyZN2thYF3AVRXM7EqtYE/zX2l5iXYQsKHvQS0W68//jHpHerTzTlL LO9H/xk39vODWIjT0hPd =RTns -----END PGP SIGNATURE----- --5oH/S/bF6lOfqCQb--