[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating byte swapping |
Date: |
Fri, 20 May 2016 21:56:04 +0200 |
User-agent: |
Mutt/1.6.0 (2016-04-01) |
On 2016-05-20 15:05, Hervé Poussineau wrote:
> Incidentally, this fixes YAMON on big endian guest.
>
> Signed-off-by: Hervé Poussineau <address@hidden>
> ---
> hw/mips/gt64xxx_pci.c | 62
> +++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 60 insertions(+), 2 deletions(-)
>
> 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 @@
>
> /* 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);
> }
>
> +static uint64_t gt64120_pci_io_read(void *opaque, hwaddr addr,
> + unsigned int size)
> +{
> + GT64120State *s = 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 = (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 <= 4, swapping the 2 words should be done with addr ^= 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 == 1) {
> + return buf[0];
> + } else if (size == 2) {
> + return lduw_le_p(buf);
> + } else if (size == 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
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
address@hidden http://www.aurel32.net