* [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating byte swapping
@ 2016-05-20 13:05 Hervé Poussineau
2016-05-20 19:56 ` Aurelien Jarno
0 siblings, 1 reply; 5+ messages in thread
From: Hervé Poussineau @ 2016-05-20 13:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Aurelien Jarno, Leon Alrae, Hervé Poussineau
Incidentally, this fixes YAMON on big endian guest.
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
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) {
+ addr = (addr & ~3) + 4 - size - (addr & 3);
+ }
+
+ 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();
+ }
+}
+
+static void gt64120_pci_io_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned int size)
+{
+ GT64120State *s = opaque;
+ uint8_t buf[4];
+
+ if (s->regs[GT_PCI0_CMD] & GT_CMD_MWORDSWAP) {
+ addr = (addr & ~3) + 4 - size - (addr & 3);
+ }
+
+ if (size == 1) {
+ buf[0] = data;
+ } else if (size == 2) {
+ stw_le_p(buf, data);
+ } else if (size == 4) {
+ stl_le_p(buf, data);
+ } else {
+ g_assert_not_reached();
+ }
+
+ address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+ buf, size);
+}
+
+static const MemoryRegionOps gt64120_pci_io_ops = {
+ .read = gt64120_pci_io_read,
+ .write = gt64120_pci_io_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .impl.max_access_size = 4,
+ .valid.unaligned = true,
+};
+
static void gt64120_pci_mapping(GT64120State *s)
{
/* Update PCI0IO mapping */
@@ -308,8 +365,9 @@ static void gt64120_pci_mapping(GT64120State *s)
s->PCI0IO_length = ((s->regs[GT_PCI0IOHD] + 1) -
(s->regs[GT_PCI0IOLD] & 0x7f)) << 21;
if (s->PCI0IO_length) {
- memory_region_init_alias(&s->PCI0IO_mem, OBJECT(s), "pci0-io",
- get_system_io(), 0, s->PCI0IO_length);
+ memory_region_init_io(&s->PCI0IO_mem, OBJECT(s),
+ >64120_pci_io_ops,
+ s, "pci0-io", s->PCI0IO_length);
memory_region_add_subregion(get_system_memory(), s->PCI0IO_start,
&s->PCI0IO_mem);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating byte swapping
2016-05-20 13:05 [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating byte swapping Hervé Poussineau
@ 2016-05-20 19:56 ` Aurelien Jarno
2016-06-18 20:48 ` Hervé Poussineau
0 siblings, 1 reply; 5+ messages in thread
From: Aurelien Jarno @ 2016-05-20 19:56 UTC (permalink / raw)
To: Hervé Poussineau; +Cc: qemu-devel, Leon Alrae
On 2016-05-20 15:05, Hervé Poussineau wrote:
> Incidentally, this fixes YAMON on big endian guest.
>
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
> 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
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating byte swapping
2016-05-20 19:56 ` Aurelien Jarno
@ 2016-06-18 20:48 ` Hervé Poussineau
2016-07-22 13:55 ` Aurelien Jarno
0 siblings, 1 reply; 5+ messages in thread
From: Hervé Poussineau @ 2016-06-18 20:48 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel, Leon Alrae
Hi Aurélien,
Le 20/05/2016 à 21:56, Aurelien Jarno a écrit :
> On 2016-05-20 15:05, Hervé Poussineau wrote:
>> Incidentally, this fixes YAMON on big endian guest.
>>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>> 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.
The real word problem (ie the one from Yamon) is:
In LE Yamon, there is a read a 0x4d1 (len = 1). MWORDSWAP and MBYTESWAP are disabled
In BE Yamon, the same read is at address 0x4d2. MWORDSWAP is enabled while MBYTESWAP is disabled.
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)"
Do we have to ignore it, as QEMU only handles 4-bytes accesses?
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.
>
>> + }
>> +
>> + 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?
OK for this part, DEVICE_NATIVE_ENDIAN is indeed better and simplifies code.
Hervé
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating byte swapping
2016-06-18 20:48 ` Hervé Poussineau
@ 2016-07-22 13:55 ` Aurelien Jarno
0 siblings, 0 replies; 5+ messages in thread
From: Aurelien Jarno @ 2016-07-22 13:55 UTC (permalink / raw)
To: Hervé Poussineau; +Cc: qemu-devel, Leon Alrae
On 2016-06-18 22:48, Hervé Poussineau wrote:
> Hi Aurélien,
>
> Le 20/05/2016 à 21:56, Aurelien Jarno a écrit :
> > On 2016-05-20 15:05, Hervé Poussineau wrote:
> > > Incidentally, this fixes YAMON on big endian guest.
> > >
> > > Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> > > ---
> > > 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.
>
> The real word problem (ie the one from Yamon) is:
> In LE Yamon, there is a read a 0x4d1 (len = 1). MWORDSWAP and MBYTESWAP are disabled
> In BE Yamon, the same read is at address 0x4d2. MWORDSWAP is enabled while MBYTESWAP is disabled.
>
> 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)"
>
> 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 = 0x4d1 and 0x72 ^ 3 = 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
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating byte swapping
@ 2015-06-10 20:03 Hervé Poussineau
0 siblings, 0 replies; 5+ messages in thread
From: Hervé Poussineau @ 2015-06-10 20:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Leon Alrae, Hervé Poussineau, Aurelien Jarno
Incidentally, this fixes YAMON on big endian guest.
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
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 10fcca3..39dd8ff 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -176,6 +176,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)
@@ -292,6 +293,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) {
+ addr = (addr & ~3) + 4 - size - (addr & 3);
+ }
+
+ 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();
+ }
+}
+
+static void gt64120_pci_io_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned int size)
+{
+ GT64120State *s = opaque;
+ uint8_t buf[4];
+
+ if (s->regs[GT_PCI0_CMD] & GT_CMD_MWORDSWAP) {
+ addr = (addr & ~3) + 4 - size - (addr & 3);
+ }
+
+ if (size == 1) {
+ buf[0] = data;
+ } else if (size == 2) {
+ stw_le_p(buf, data);
+ } else if (size == 4) {
+ stl_le_p(buf, data);
+ } else {
+ g_assert_not_reached();
+ }
+
+ address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+ buf, size);
+}
+
+static const MemoryRegionOps gt64120_pci_io_ops = {
+ .read = gt64120_pci_io_read,
+ .write = gt64120_pci_io_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .impl.max_access_size = 4,
+ .valid.unaligned = true,
+};
+
static void gt64120_pci_mapping(GT64120State *s)
{
/* Update PCI0IO mapping */
@@ -306,8 +363,9 @@ static void gt64120_pci_mapping(GT64120State *s)
s->PCI0IO_length = ((s->regs[GT_PCI0IOHD] + 1) -
(s->regs[GT_PCI0IOLD] & 0x7f)) << 21;
if (s->PCI0IO_length) {
- memory_region_init_alias(&s->PCI0IO_mem, OBJECT(s), "pci0-io",
- get_system_io(), 0, s->PCI0IO_length);
+ memory_region_init_io(&s->PCI0IO_mem, OBJECT(s),
+ >64120_pci_io_ops,
+ s, "pci0-io", s->PCI0IO_length);
memory_region_add_subregion(get_system_memory(), s->PCI0IO_start,
&s->PCI0IO_mem);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-07-22 13:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-20 13:05 [Qemu-devel] [PATCH] gt64xxx: access right I/O port when activating byte swapping Hervé Poussineau
2016-05-20 19:56 ` Aurelien Jarno
2016-06-18 20:48 ` Hervé Poussineau
2016-07-22 13:55 ` Aurelien Jarno
-- strict thread matches above, loose matches on Subject: below --
2015-06-10 20:03 Hervé Poussineau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.