Yes, both the comment and enum should be updated. -----Original Message----- From: Aleksandar Markovic <amarko...@wavecomp.com> Sent: Thursday, December 27, 2018 2:23 PM To: Janeczek, Craig <jancr...@amazon.com>; Stefan Markovic <smarko...@wavecomp.com>; Aleksandar Markovic <aleksandar.marko...@rt-rk.com>; qemu-devel@nongnu.org Subject: Re: [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for LX* instructions
> From: Janeczek, Craig <jancr...@amazon.com> > Sent: Thursday, December 27, 2018 7:50 PM > To: Aleksandar Markovic; Stefan Markovic; Aleksandar Markovic; > qemu-devel@nongnu.org > Subject: RE: [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for > LX* instructions > > Yes built a binary testing these instructions and ran it against HW along > with a qemu binary with your > patches. The opcodes align with what I > mentioned in the email. > I truly appreciate your help. Just to clarify details, beside correcting comments, does this mean that this code segment: /* * MXU pool 16 */ enum { OPC_MXU_D32SARW = 0x00, OPC_MXU_S32ALN = 0x01, OPC_MXU_S32ALNI = 0x02, OPC_MXU_S32NOR = 0x03, OPC_MXU_S32AND = 0x04, OPC_MXU_S32OR = 0x05, OPC_MXU_S32XOR = 0x06, OPC_MXU_S32LUI = 0x07, }; should be corrected to be: /* * MXU pool 16 */ enum { OPC_MXU_D32SARW = 0x00, OPC_MXU_S32ALN = 0x01, OPC_MXU_S32ALNI = 0x02, OPC_MXU_S32LUI = 0x03, OPC_MXU_S32NOR = 0x04, OPC_MXU_S32AND = 0x05, OPC_MXU_S32OR = 0x06, OPC_MXU_S32XOR = 0x07, }; and all things will line up for "pool 16"? Sincerely, Aleksandar > -----Original Message----- > From: Aleksandar Markovic <amarko...@wavecomp.com> > Subject: Re: [PATCH 1/6] target/mips: MXU: Add missing opcodes/decoding for > LX* instructions > > > > @@ -1663,12 +1663,21 @@ enum { > > > * │ 20..18 > > > * ├─ 100111 ─ OPC_MXU__POOL16 ─┬─ 000 ─ OPC_MXU_D32SARW > > > * │ ├─ 001 ─ OPC_MXU_S32ALN > > > - * ├─ 101000 ─ OPC_MXU_LXB ├─ 010 ─ OPC_MXU_S32ALNI > > > - * ├─ 101001 ─ <not assigned> ├─ 011 ─ OPC_MXU_S32NOR > > > - * ├─ 101010 ─ OPC_MXU_S16LDD ├─ 100 ─ OPC_MXU_S32AND > > > - * ├─ 101011 ─ OPC_MXU_S16STD ├─ 101 ─ OPC_MXU_S32OR > > > - * ├─ 101100 ─ OPC_MXU_S16LDI ├─ 110 ─ OPC_MXU_S32XOR > > > - * ├─ 101101 ─ OPC_MXU_S16SDI └─ 111 ─ OPC_MXU_S32LUI > > > + * │ ├─ 010 ─ OPC_MXU_S32ALNI > > > + * │ ├─ 011 ─ OPC_MXU_S32NOR > > > + * │ ├─ 100 ─ OPC_MXU_S32AND > > > + * │ ├─ 101 ─ OPC_MXU_S32OR > > > + * │ ├─ 110 ─ OPC_MXU_S32XOR > > > + * │ └─ 111 ─ OPC_MXU_S32LUI > > The opcodes for pool 16 do not look correct. I think the minor bits should > > look like the following. > > > > ┬─ 000 ─ OPC_MXU_D32SARW > > ├─ 001 ─ OPC_MXU_S32ALN > > ├─ 010 ─ OPC_MXU_S32ALNI > > ├─ 011 ─ OPC_MXU_S32LUI > > ├─ 100 ─ OPC_MXU_S32NOR > > ├─ 101 ─ OPC_MXU_S32AND > > ├─ 110 ─ OPC_MXU_S32OR > > └─ 111 ─ OPC_MXU_S32XOR > > > > Hi, Craig! > > My primary source for opcodes was the latest Ingenic documentation, from > Ingenic's site: (dated June > 2017) > > ftp://ftp.ingenic.com/SOC/M200/X1000_M200_XBurst_ISA_MXU_PM.pdf > > and on page 109 there is a table in the middle of the page that contains > codes that are in agreement > with what is currently in QEMU. > > However, I searched more, and in a repository that seems to be derived from > Android NDK r9d binutils > tree, in file > > https://github.com/apportable/binutils/blob/master/opcodes/mxu-opc.c > > opcodes are as you said. > > I guess the definitive answer would be to involve tests on hardware, no? > > Thanks for bringing this up! > > Aleksandar > > > > > + * │ > > > + * │ 7..5 > > > + * ├─ 101000 ─ OPC_MXU__POOL17 ─┬─ 000 ─ OPC_MXU_LXB > > > + * │ ├─ 001 ─ OPC_MXU_LXH > > > + * ├─ 101001 ─ <not assigned> ├─ 011 ─ OPC_MXU_LXW > > > + * ├─ 101010 ─ OPC_MXU_S16LDD ├─ 100 ─ OPC_MXU_LXBU > > > + * ├─ 101011 ─ OPC_MXU_S16STD └─ 101 ─ OPC_MXU_LXHU > > > + * ├─ 101100 ─ OPC_MXU_S16LDI > > > + * ├─ 101101 ─ OPC_MXU_S16SDI > > > * ├─ 101110 ─ OPC_MXU_S32M2I > > > * ├─ 101111 ─ OPC_MXU_S32I2M > > > * ├─ 110000 ─ OPC_MXU_D32SLL