From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38765) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQava-0004Z7-PF for qemu-devel@nongnu.org; Fri, 22 Jul 2016 09:55:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQavY-0005Nq-De for qemu-devel@nongnu.org; Fri, 22 Jul 2016 09:55:45 -0400 Received: from hall.aurel32.net ([2001:bc8:30d7:100::1]:50276) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQavY-0005Nl-2C for qemu-devel@nongnu.org; Fri, 22 Jul 2016 09:55:44 -0400 Date: Fri, 22 Jul 2016 15:55:41 +0200 From: Aurelien Jarno Message-ID: <20160722135541.GB7630@aurel32.net> References: <1463749503-27374-1-git-send-email-hpoussin@reactos.org> <20160520195604.GA19765@aurel32.net> <12a1396e-6454-cda7-675d-163748e5a3d0@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: <12a1396e-6454-cda7-675d-163748e5a3d0@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-06-18 22:48, Herv=E9 Poussineau wrote: > Hi Aur=E9lien, >=20 > Le 20/05/2016 =E0 21:56, Aurelien Jarno a =E9crit : > > 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->ISD_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) { > >=20 > > 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. > >=20 > > > + addr =3D (addr & ~3) + 4 - size - (addr & 3); > >=20 > > 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. > >=20 > > 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. >=20 > The real word problem (ie the one from Yamon) is: > In LE Yamon, there is a read a 0x4d1 (len =3D 1). MWORDSWAP and MBYTESWAP= are disabled > In BE Yamon, the same read is at address 0x4d2. MWORDSWAP is enabled whil= e MBYTESWAP is disabled. >=20 > MWORDSWAP documentation is: > "The GT-64120 PCI master swaps the words of the incoming and outgoing PCI= data (swap the 2 words of a long word)" >=20 > Do we have to ignore it, as QEMU only handles 4-bytes accesses? I think indeed that it should be ignored. > Then, how to change this address 0x4d2 to 0x4d1, address where is located= the i8259 ELCR register? > Next accesses are for the RTC, at address 0x72 in BE and address 0x71 in = LE. > I think I'm missing something. If you talk about byte accesses, it really looks like a simple byteswap. 0x4d2 ^ 3 =3D 0x4d1 and 0x72 ^ 3 =3D 0x71. This could be there is a byteswap that is missing somewhere. It would be interesting to see how 16-bit and 32-bit accesses are changed between big and little endian. Aurelien --=20 Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net