From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bx4GR-0003ni-OL for qemu-devel@nongnu.org; Wed, 19 Oct 2016 23:43:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bx4GN-0006Wv-KM for qemu-devel@nongnu.org; Wed, 19 Oct 2016 23:43:31 -0400 Date: Thu, 20 Oct 2016 14:24:11 +1100 From: David Gibson Message-ID: <20161020032411.GN11140@umbus.fritz.box> References: <1476879941-14360-1-git-send-email-david@gibson.dropbear.id.au> <1476879941-14360-9-git-send-email-david@gibson.dropbear.id.au> <2926dace-6ab1-dc32-bcd5-4e2db4ec0100@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="EqVOK5mkaJAMmtSx" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCHv2 08/11] tests: Clean up IO handling in ide-test List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, agraf@suse.de, stefanha@redhat.com, mst@redhat.com, aik@ozlabs.ru, mdroth@linux.vnet.ibm.com, groug@kaod.org, thuth@redhat.com --EqVOK5mkaJAMmtSx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 19, 2016 at 04:51:41PM +0200, Laurent Vivier wrote: >=20 >=20 > On 19/10/2016 16:43, Laurent Vivier wrote: > >=20 > >=20 > > On 19/10/2016 14:25, David Gibson wrote: > >> ide-test uses many explicit inb() / outb() operations for its IO, which > >> means it's not portable to non-x86 platforms. This cleans it up to use > >> the libqos PCI accessors instead. > >> > >> Signed-off-by: David Gibson > >> --- > >> tests/ide-test.c | 179 ++++++++++++++++++++++++++++++++++++----------= --------- > >> 1 file changed, 118 insertions(+), 61 deletions(-) > >=20 > > Could explain why you have swapped the le16_to_cpu() and cpu_to_le16()? > >=20 > > For me, they were correct. >=20 > And I have just finished testing your series on a BE host, and ide-test > is broken: >=20 > TEST: tests/ide-test... (pid=3D12472) > /i386/ide/identify: ** > ERROR:/home/laurent/Projects/qemu/tests/ide-test.c:518:test_identify: > assertion failed: (ret =3D=3D 0) > FAIL Ah, thanks for testing this. > You should not add the cpu_to_le16(): >=20 > for (i =3D 0; i < 256; i++) { > - data =3D inb(IDE_BASE + reg_status); > + data =3D qpci_io_readb(dev, ide_base + reg_status); > assert_bit_set(data, DRDY | DRQ); > assert_bit_clear(data, BSY | DF | ERR); >=20 > - ((uint16_t*) buf)[i] =3D inw(IDE_BASE + reg_data); > + buf[i] =3D cpu_to_le16(qpci_io_readw(dev, ide_base + reg_data)); > } Urgh, the endianness here is doing my head in. So, the cpu_to_le16() was supposed to counteract the implicit conversion from LE inside qpci_io_readw. We're reading from the data register here, and those are usually "streaming" style, meaning that we want byte-order preserving rather than byte-significance preserving. But.. the IDENTIFY command describes most of the output in terms of (16-bit) words, meaning I guess we do want to swap from LE in order to interpret those. Except that the string portions seem to be encoded strangely, which the later string_cpu_to_be16() calls are about. In summary, I think you're right and the cpu_to_le16() shouldn't be there. Seems to work on a BE host, anyway. --=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 --EqVOK5mkaJAMmtSx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYCDjZAAoJEGw4ysog2bOSEXsQAN3RaZCtsc1+Xcb6hDBaWKg3 r4UW91ieJknnLPWvKbH+bh7yWtERo/bg9JbFBIQCO7kR0MhnhjAnDK+UfuZn2eqi iutN1cVojO06yvo2X6DkfTVjum+XbN8qKxi5RgB1dHqHe2YEZv//rfghDr+Vpzr0 9m+H6ADdjuIpYs4CM9Z9cNSfM5UZ5+fVJBmxPrM6KXIo6haOHj+NEucIifjVsAEB sLxrfPw3R4FNoJl0rmzzPNt6pHZoB/jj+8Ae7MlV+qQPYJsnygL2be/9IeE+XjnN FD+SqhkGVKqo2omwdBA6Vv7xp5miA+ZWb7AOdas0wLRpVu78BtSMV9wKVGI0825Q dLO1uqDcQrMWPzWopxhsCXfsfgW3CxgS2n3m9nJo3aoq/4qTDmeSEa6rXKBRq80L Xw/Keubsi8cPgee79/cavXwkKhoR9pNJgwzA+SaHyDVpzlI3pWBTq7qV92erovR3 lMhQMk0rzFF23ln2VSlhOPxQzvVvOAIy+ImxVjuiZFHnNBSG52EozFBWcSmwLIJ3 BKn75JzhXaPW4SPhYJaS7SuDGJQWeGLOw5u/KkWcf7Ql5gB8dTn3PJGFqnhh9Eh0 wxqBuZyucCNpPyPrcO3Vr/Ef5EKTu7ARylr6hdhiD4gSHDQOIe2LrFNtEEJ1IxRC k1g2ZRc/tjkHD27MMhUq =laLO -----END PGP SIGNATURE----- --EqVOK5mkaJAMmtSx--