From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34784) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3qWx-0003gh-7h for qemu-devel@nongnu.org; Fri, 20 May 2016 15:56:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b3qWu-0005XT-Vg for qemu-devel@nongnu.org; Fri, 20 May 2016 15:56:18 -0400 Received: from hall.aurel32.net ([2001:bc8:30d7:100::1]:42524) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3qWu-0005Uc-PO for qemu-devel@nongnu.org; Fri, 20 May 2016 15:56:16 -0400 Date: Fri, 20 May 2016 21:56:04 +0200 From: Aurelien Jarno Message-ID: <20160520195604.GA19765@aurel32.net> References: <1463749503-27374-1-git-send-email-hpoussin@reactos.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <1463749503-27374-1-git-send-email-hpoussin@reactos.org> Subject: Re: [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating byte swapping List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-15?Q?Herv=E9?= Poussineau Cc: qemu-devel@nongnu.org, Leon Alrae On 2016-05-20 15:05, Herv=E9 Poussineau wrote: > Incidentally, this fixes YAMON on big endian guest. >=20 > Signed-off-by: Herv=E9 Poussineau > --- > hw/mips/gt64xxx_pci.c | 62 +++++++++++++++++++++++++++++++++++++++++++++= ++++-- > 1 file changed, 60 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c > index 3f4523d..c76ee88 100644 > --- a/hw/mips/gt64xxx_pci.c > +++ b/hw/mips/gt64xxx_pci.c > @@ -177,6 +177,7 @@ > =20 > /* PCI Internal */ > #define GT_PCI0_CMD (0xc00 >> 2) > +#define GT_CMD_MWORDSWAP (1 << 10) > #define GT_PCI0_TOR (0xc04 >> 2) > #define GT_PCI0_BS_SCS10 (0xc08 >> 2) > #define GT_PCI0_BS_SCS32 (0xc0c >> 2) > @@ -294,6 +295,62 @@ static void gt64120_isd_mapping(GT64120State *s) > memory_region_add_subregion(get_system_memory(), s->ISD_start, &s->I= SD_mem); > } > =20 > +static uint64_t gt64120_pci_io_read(void *opaque, hwaddr addr, > + unsigned int size) > +{ > + GT64120State *s =3D opaque; > + uint8_t buf[4]; > + > + if (s->regs[GT_PCI0_CMD] & GT_CMD_MWORDSWAP) { First of all, it should be noted that this bit doesn't control byte swapping, but swaps the 2 4-byte words in a 8-byte word. > + addr =3D (addr & ~3) + 4 - size - (addr & 3); This looks complicated, and I don't think it is correct. In addition this doesn't behave correctly at the edges of the address space. For example a 2 byte access at address 0x3 would access address 0xffffffffffffffff. For sizes <=3D 4, swapping the 2 words should be done with addr ^=3D 4. Maybe you should also check for MBYTESWAP which also swaps the bytes within a 8-byte word. > + } > + > + address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, > + buf, size); > + > + if (size =3D=3D 1) { > + return buf[0]; > + } else if (size =3D=3D 2) { > + return lduw_le_p(buf); > + } else if (size =3D=3D 4) { > + return ldl_le_p(buf); > + } else { > + g_assert_not_reached(); > + } The device is configured is little endian, and then the little endian value converted into native endianness. Wouldn't it be simple to declare it as DEVICE_NATIVE_ENDIAN? Aurelien --=20 Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net