[PULL,for-2.0,2/7] raven: Implement non-contiguous I/O region
diff mbox

Message ID 1395272166-687-3-git-send-email-andreas.faerber@web.de
State New
Headers show

Commit Message

Andreas Färber March 19, 2014, 11:36 p.m. UTC
From: Hervé Poussineau <hpoussin@reactos.org>

Remove now duplicated code from prep board.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/prep.c      | 94 ++----------------------------------------------------
 2 files changed, 88 insertions(+), 91 deletions(-)

Comments

Andreas Färber April 5, 2014, 3:41 p.m. UTC | #1
Hi Hervé,

Am 20.03.2014 00:36, schrieb Andreas Färber:
> From: Hervé Poussineau <hpoussin@reactos.org>
> 
> Remove now duplicated code from prep board.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/ppc/prep.c      | 94 ++----------------------------------------------------
>  2 files changed, 88 insertions(+), 91 deletions(-)

I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.

$ make check-qtest-ppc V=1
[...]
  /ppc/endianness/prep:                                              **
ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
FAIL
[...]
  /ppc/endianness/split/prep:                                        **
ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
FAIL
[...]
  /ppc/endianness/combine/prep:                                      **
ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 == 0x00008765)
FAIL
[...]
FAIL: tests/endianness-test

On x86 everything is fine. git-bisect points to this commit.

There is one "FIXME: handle endianness switch" in here, but I don't spot
such code where it's being moved from either.

My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
the code or the test is wrong...

Any thoughts?

Thanks,
Andreas
Hervé Poussineau April 5, 2014, 8:26 p.m. UTC | #2
Hi Andreas,

Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
> Hi Hervé,
>
> Am 20.03.2014 00:36, schrieb Andreas Färber:
>> From: Hervé Poussineau <hpoussin@reactos.org>
>>
>> Remove now duplicated code from prep board.
>>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> ---
>>   hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   hw/ppc/prep.c      | 94 ++----------------------------------------------------
>>   2 files changed, 88 insertions(+), 91 deletions(-)
>
> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>
> $ make check-qtest-ppc V=1
> [...]
>    /ppc/endianness/prep:                                              **
> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
> FAIL
> [...]
>    /ppc/endianness/split/prep:                                        **
> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
> FAIL
> [...]
>    /ppc/endianness/combine/prep:                                      **
> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 == 0x00008765)
> FAIL
> [...]
> FAIL: tests/endianness-test
>
> On x86 everything is fine. git-bisect points to this commit.
>
> There is one "FIXME: handle endianness switch" in here, but I don't spot
> such code where it's being moved from either.
>
> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
> the code or the test is wrong...

Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then 
using cpu_inl, which does a ldl_p.
Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is 
using ldl_p.
So, yes, it seems that endianness of memory region does change things. 
Native endian means native endian of the guest of of the host?

I also checked tests/endianness-test.c.
The failing test is:
    isa_outl(test, 0xe0, 0x87654321);
    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654321);
    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4321);
which seems perfectly valid as ISA is little-endian.

However, PReP is marked as bswap= true in this file, which means that 
values are inverted before writing and after reading them.
Paolo, what does it mean? It is supposed to be true for big endian 
machines, and false for little endian machines ?

Hervé
Alexander Graf April 5, 2014, 8:34 p.m. UTC | #3
On 05.04.2014, at 22:26, Hervé Poussineau <hpoussin@reactos.org> wrote:

> Hi Andreas,
> 
> Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
>> Hi Hervé,
>> 
>> Am 20.03.2014 00:36, schrieb Andreas Färber:
>>> From: Hervé Poussineau <hpoussin@reactos.org>
>>> 
>>> Remove now duplicated code from prep board.
>>> 
>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>> ---
>>>  hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>  hw/ppc/prep.c      | 94 ++----------------------------------------------------
>>>  2 files changed, 88 insertions(+), 91 deletions(-)
>> 
>> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
>> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>> 
>> $ make check-qtest-ppc V=1
>> [...]
>>   /ppc/endianness/prep:                                              **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
>> FAIL
>> [...]
>>   /ppc/endianness/split/prep:                                        **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
>> FAIL
>> [...]
>>   /ppc/endianness/combine/prep:                                      **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
>> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 == 0x00008765)
>> FAIL
>> [...]
>> FAIL: tests/endianness-test
>> 
>> On x86 everything is fine. git-bisect points to this commit.
>> 
>> There is one "FIXME: handle endianness switch" in here, but I don't spot
>> such code where it's being moved from either.
>> 
>> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
>> the code or the test is wrong...
> 
> Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then using cpu_inl, which does a ldl_p.
> Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is using ldl_p.
> So, yes, it seems that endianness of memory region does change things. Native endian means native endian of the guest of of the host?

It means "default endianness of the guest cpu" so that accesses coming from the CPU get passed in the value they had in the CPU registers :).


Alex
Hervé Poussineau April 5, 2014, 8:50 p.m. UTC | #4
Le sam. 05 avril 2014 22:34:38 CEST, Alexander Graf a écrit :
>
> On 05.04.2014, at 22:26, Hervé Poussineau <hpoussin@reactos.org> wrote:
>
>> Hi Andreas,
>>
>> Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
>>> Hi Hervé,
>>>
>>> Am 20.03.2014 00:36, schrieb Andreas Färber:
>>>> From: Hervé Poussineau <hpoussin@reactos.org>
>>>>
>>>> Remove now duplicated code from prep board.
>>>>
>>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>>> ---
>>>>   hw/pci-host/prep.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   hw/ppc/prep.c      | 94 ++----------------------------------------------------
>>>>   2 files changed, 88 insertions(+), 91 deletions(-)
>>>
>>> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
>>> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>>>
>>> $ make check-qtest-ppc V=1
>>> [...]
>>>    /ppc/endianness/prep:                                              **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
>>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
>>> FAIL
>>> [...]
>>>    /ppc/endianness/split/prep:                                        **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
>>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 == 0x00008765)
>>> FAIL
>>> [...]
>>>    /ppc/endianness/combine/prep:                                      **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
>>> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 == 0x00008765)
>>> FAIL
>>> [...]
>>> FAIL: tests/endianness-test
>>>
>>> On x86 everything is fine. git-bisect points to this commit.
>>>
>>> There is one "FIXME: handle endianness switch" in here, but I don't spot
>>> such code where it's being moved from either.

The FIXME here is only when we'll handle dynamic endianess switching of 
the system (not of the CPU). It was not present in code moved in this 
commit, and I only added the comment to tell where to place it.

>>>
>>> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
>>> the code or the test is wrong...
>>
>> Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then using cpu_inl, which does a ldl_p.
>> Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is using ldl_p.
>> So, yes, it seems that endianness of memory region does change things. Native endian means native endian of the guest of of the host?
>
> It means "default endianness of the guest cpu" so that accesses coming from the CPU get passed in the value they had in the CPU registers :).

In that case, behaviour should be the same between x86 host and 
openSUSE ppc/ppc64 and OSX ppc64 hosts.
The culprit is elsewhere...

Hervé
Peter Maydell April 5, 2014, 11:20 p.m. UTC | #5
On 5 April 2014 21:50, Hervé Poussineau <hpoussin@reactos.org> wrote:
> Le sam. 05 avril 2014 22:34:38 CEST, Alexander Graf a écrit :
>> It means "default endianness of the guest cpu" so that accesses coming
>> from the CPU get passed in the value they had in the CPU registers :).
>
>
> In that case, behaviour should be the same between x86 host and openSUSE
> ppc/ppc64 and OSX ppc64 hosts.
> The culprit is elsewhere...

Yes, if we're behaving differently between different host
endiannesses then something is definitely wrong and my
suspicion would be that it's the test harness code.

thanks
-- PMM
Paolo Bonzini April 7, 2014, 7:31 p.m. UTC | #6
Il 05/04/2014 19:20, Peter Maydell ha scritto:
>>> >> It means "default endianness of the guest cpu" so that accesses coming
>>> >> from the CPU get passed in the value they had in the CPU registers :).
>> >
>> >
>> > In that case, behaviour should be the same between x86 host and openSUSE
>> > ppc/ppc64 and OSX ppc64 hosts.
>> > The culprit is elsewhere...
> Yes, if we're behaving differently between different host
> endiannesses then something is definitely wrong and my
> suspicion would be that it's the test harness code.

No, that's very unlikely.  I think if you are using 
address_space_read/write then you must use DEVICE_NATIVE_ENDIAN, but I 
cannot check as I'm on vacation now, about 6 time zones away from 
anything big-endian.

Paolo
Andreas Färber April 7, 2014, 7:32 p.m. UTC | #7
Am 05.04.2014 22:26, schrieb Hervé Poussineau:
> Hi Andreas,
> 
> Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
>> Hi Hervé,
>>
>> Am 20.03.2014 00:36, schrieb Andreas Färber:
>>> From: Hervé Poussineau <hpoussin@reactos.org>
>>>
>>> Remove now duplicated code from prep board.
>>>
>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>> ---
>>>   hw/pci-host/prep.c | 85
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>   hw/ppc/prep.c      | 94
>>> ++----------------------------------------------------
>>>   2 files changed, 88 insertions(+), 91 deletions(-)
>>
>> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
>> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>>
>> $ make check-qtest-ppc V=1
>> [...]
>>    /ppc/endianness/prep:                                              **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 ==
>> 0x00008765)
>> FAIL
>> [...]
>>    /ppc/endianness/split/prep:                                        **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
>>
>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 ==
>> 0x00008765)
>> FAIL
>> [...]
>>    /ppc/endianness/combine/prep:                                      **
>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
>>
>> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 ==
>> 0x00008765)
>> FAIL
>> [...]
>> FAIL: tests/endianness-test
>>
>> On x86 everything is fine. git-bisect points to this commit.
>>
>> There is one "FIXME: handle endianness switch" in here, but I don't spot
>> such code where it's being moved from either.
>>
>> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
>> the code or the test is wrong...
> 
> Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then
> using cpu_inl, which does a ldl_p.
> Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is
> using ldl_p.
> So, yes, it seems that endianness of memory region does change things.
> Native endian means native endian of the guest of of the host?
> 
> I also checked tests/endianness-test.c.
> The failing test is:
>    isa_outl(test, 0xe0, 0x87654321);
>    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654321);
>    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
>    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4321);
> which seems perfectly valid as ISA is little-endian.
> 
> However, PReP is marked as bswap= true in this file, which means that
> values are inverted before writing and after reading them.

I tested .bswap = false - that fixes ppc64 host but breaks x86_64 host.

Any further suggestions anyone?

Regards,
Andreas


> Paolo, what does it mean? It is supposed to be true for big endian
> machines, and false for little endian machines ?
> 
> Hervé
Peter Maydell April 8, 2014, 2:37 p.m. UTC | #8
On 7 April 2014 20:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I think if you are using address_space_read/write
> then you must use DEVICE_NATIVE_ENDIAN

I think it's actually worse than that. address_space_read/write
have an API which requires you to pass them a buffer which is
in guest CPU endianness. This means they cannot be used from
target-independent source files (like hw/pci-host/prep.c)
because there's no way to say "write this 32 bit value to
the buffer in target endianness". ioport.c which has pretty
much identical code works OK because it is built per target.

Worse, we have two versions of the ldl_p()/stl_p() &c
functions with conflicting semantics!
cpu-all.h defines these to be "target CPU endianness".
bswap.h defines these to be "host CPU endianness".
So which version any particular source file gets depends
on which of these two headers it ends up with. prep.c gets
the bswap.h versions, and exec.c gets the cpu-all.h versions,
which means on x86 we get the bizarre effect of doing an
stl_p() into a buffer in raven_io_write() and then having
address_space_rw() do a ldl_p() from the buffer and getting
a different value...

thanks
-- PMM
Paolo Bonzini April 8, 2014, 6:39 p.m. UTC | #9
Il 08/04/2014 10:37, Peter Maydell ha scritto:
> I think it's actually worse than that. address_space_read/write
> have an API which requires you to pass them a buffer which is
> in guest CPU endianness. This means they cannot be used from
> target-independent source files (like hw/pci-host/prep.c)
> because there's no way to say "write this 32 bit value to
> the buffer in target endianness". ioport.c which has pretty
> much identical code works OK because it is built per target.

So the fix could be to compile prep.c per-target (and change to 
DEVICE_NATIVE_ENDIAN too).

> Worse, we have two versions of the ldl_p()/stl_p() &c
> functions with conflicting semantics!
> cpu-all.h defines these to be "target CPU endianness".
> bswap.h defines these to be "host CPU endianness".

Ouch!  I have some cleanups for CPU ld/st ready for 2.1, I'll add a 
patch to rename bswap.h's definition to ldl_host_p/stl_host_p.

Paolo
Peter Maydell April 8, 2014, 6:55 p.m. UTC | #10
On 8 April 2014 19:39, Paolo Bonzini <pbonzini@redhat.com> wrote:
> So the fix could be to compile prep.c per-target (and change to
> DEVICE_NATIVE_ENDIAN too).

That seems like the wrong direction -- we should be
making fewer files per-target, not more.

>> Worse, we have two versions of the ldl_p()/stl_p() &c
>> functions with conflicting semantics!
>> cpu-all.h defines these to be "target CPU endianness".
>> bswap.h defines these to be "host CPU endianness".

> Ouch!  I have some cleanups for CPU ld/st ready for 2.1, I'll add a patch to
> rename bswap.h's definition to ldl_host_p/stl_host_p.

Richard's suggestion was to make the cpu-all.h ones
be ldl_te_p/stl_te_p, which I think I like better.
We could do both in order to enforce that we audited
everything to see which it thought it was :-)

thanks
-- PMM
Paolo Bonzini April 8, 2014, 8:27 p.m. UTC | #11
Il 08/04/2014 14:55, Peter Maydell ha scritto:
> On 8 April 2014 19:39, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> So the fix could be to compile prep.c per-target (and change to
>> DEVICE_NATIVE_ENDIAN too).
>
> That seems like the wrong direction -- we should be
> making fewer files per-target, not more.

I agree, and in fact we should also use DEVICE_NATIVE_ENDIAN less, not 
more.  Unfortunately, forwarding accesses from one address space to 
another via MMIO accessors requires DEVICE_NATIVE_ENDIAN, and that in 
turn requires target-endianness ldl_p/stl_p.

We could to try and make the non-contiguous I/O region an IOMMU region, 
if that can work.  But that would be post-2.0, I think.

I'm of course assuming that we cannot just revert patch 2 in this series...

>>> Worse, we have two versions of the ldl_p()/stl_p() &c
>>> functions with conflicting semantics!
>>> cpu-all.h defines these to be "target CPU endianness".
>>> bswap.h defines these to be "host CPU endianness".
>
>> Ouch!  I have some cleanups for CPU ld/st ready for 2.1, I'll add a patch to
>> rename bswap.h's definition to ldl_host_p/stl_host_p.
>
> Richard's suggestion was to make the cpu-all.h ones
> be ldl_te_p/stl_te_p, which I think I like better.
> We could do both in order to enforce that we audited
> everything to see which it thought it was :-)

Yes, that's the next obvious step.

Paolo
Peter Maydell April 8, 2014, 8:56 p.m. UTC | #12
On 8 April 2014 21:27, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I agree, and in fact we should also use DEVICE_NATIVE_ENDIAN less, not more.
> Unfortunately, forwarding accesses from one address space to another via
> MMIO accessors requires DEVICE_NATIVE_ENDIAN, and that in turn requires
> target-endianness ldl_p/stl_p.

I don't think this is correct. If we're purely forwarding
then you can just use DEVICE_LITTLE_ENDIAN and ldl_le_p.
There are three places in the sequence of calls from CPU to
outer MMIO accessor to stl_le_p to memory_space_write
that might insert bswaps:
 1 before the call to the outer MMIO accessor we will bswap
   if the guest endianness is not LE (because we're calling an
   MMIO accessor that was marked DEVICE_LITTLE_ENDIAN)
 2 in the stl_le_p we will bswap if the host endianness is not LE
   (because that's what stl_le_p means)
 3 in memory_space_write when we read out of the buffer we
   do a ldl_q, which will bswap if the host endianness is not
   the target endianness (because we get the cpu-all.h version
   of ldl_q, which does a target-endian load)

So:
host = guest = LE: no swapping
host = LE, guest = BE: swap 1 and 3 active
host = BE, guest = LE: swap 2 and 3 active
host = guest = BE: swap 1 and 2 active

In all cases we do an even number of swaps and the whole thing
cancels out :-)

You can use DEVICE_BIG_ENDIAN and stl_be_p if you like,
for equivalent effect.

I'm pretty sure this wasn't an intentional design property, though...

thanks
-- PMM

Patch
diff mbox

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 84d50ca..629735e 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -54,8 +54,12 @@  typedef struct PRePPCIState {
 
     qemu_irq irq[PCI_NUM_PINS];
     PCIBus pci_bus;
+    AddressSpace pci_io_as;
+    MemoryRegion pci_io_non_contiguous;
     MemoryRegion pci_intack;
     RavenPCIState pci_dev;
+
+    int contiguous_map;
 } PREPPCIState;
 
 #define BIOS_SIZE (1024 * 1024)
@@ -107,6 +111,71 @@  static const MemoryRegionOps PPC_intack_ops = {
     },
 };
 
+static inline hwaddr raven_io_address(PREPPCIState *s,
+                                      hwaddr addr)
+{
+    if (s->contiguous_map == 0) {
+        /* 64 KB contiguous space for IOs */
+        addr &= 0xFFFF;
+    } else {
+        /* 8 MB non-contiguous space for IOs */
+        addr = (addr & 0x1F) | ((addr & 0x007FFF000) >> 7);
+    }
+
+    /* FIXME: handle endianness switch */
+
+    return addr;
+}
+
+static uint64_t raven_io_read(void *opaque, hwaddr addr,
+                              unsigned int size)
+{
+    PREPPCIState *s = opaque;
+    uint8_t buf[4];
+
+    addr = raven_io_address(s, addr);
+    address_space_read(&s->pci_io_as, addr, buf, size);
+
+    if (size == 1) {
+        return buf[0];
+    } else if (size == 2) {
+        return lduw_p(buf);
+    } else if (size == 4) {
+        return ldl_p(buf);
+    } else {
+        g_assert_not_reached();
+    }
+}
+
+static void raven_io_write(void *opaque, hwaddr addr,
+                           uint64_t val, unsigned int size)
+{
+    PREPPCIState *s = opaque;
+    uint8_t buf[4];
+
+    addr = raven_io_address(s, addr);
+
+    if (size == 1) {
+        buf[0] = val;
+    } else if (size == 2) {
+        stw_p(buf, val);
+    } else if (size == 4) {
+        stl_p(buf, val);
+    } else {
+        g_assert_not_reached();
+    }
+
+    address_space_write(&s->pci_io_as, addr, buf, size);
+}
+
+static const MemoryRegionOps raven_io_ops = {
+    .read = raven_io_read,
+    .write = raven_io_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl.max_access_size = 4,
+    .valid.unaligned = true,
+};
+
 static int prep_map_irq(PCIDevice *pci_dev, int irq_num)
 {
     return (irq_num + (pci_dev->devfn >> 3)) & 1;
@@ -119,6 +188,13 @@  static void prep_set_irq(void *opaque, int irq_num, int level)
     qemu_set_irq(pic[irq_num] , level);
 }
 
+static void raven_change_gpio(void *opaque, int n, int level)
+{
+    PREPPCIState *s = opaque;
+
+    s->contiguous_map = level;
+}
+
 static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
 {
     SysBusDevice *dev = SYS_BUS_DEVICE(d);
@@ -133,6 +209,8 @@  static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
         sysbus_init_irq(dev, &s->irq[i]);
     }
 
+    qdev_init_gpio_in(d, raven_change_gpio, 1);
+
     pci_bus_irqs(&s->pci_bus, prep_set_irq, prep_map_irq, s->irq, PCI_NUM_PINS);
 
     memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_be_ops, s,
@@ -164,6 +242,13 @@  static void raven_pcihost_initfn(Object *obj)
     MemoryRegion *address_space_io = get_system_io();
     DeviceState *pci_dev;
 
+    memory_region_init_io(&s->pci_io_non_contiguous, obj, &raven_io_ops, s,
+                          "pci-io-non-contiguous", 0x00800000);
+    address_space_init(&s->pci_io_as, get_system_io(), "raven-io");
+
+    /* CPU address space */
+    memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
+                                        &s->pci_io_non_contiguous, 1);
     pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
                         address_space_mem, address_space_io, 0, TYPE_PCI_BUS);
     h->bus = &s->pci_bus;
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 81e13cb..035b5b2 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -185,6 +185,7 @@  typedef struct sysctrl_t {
     uint8_t state;
     uint8_t syscontrol;
     int contiguous_map;
+    qemu_irq contiguous_map_irq;
     int endian;
 } sysctrl_t;
 
@@ -253,6 +254,7 @@  static void PREP_io_800_writeb (void *opaque, uint32_t addr, uint32_t val)
     case 0x0850:
         /* I/O map type register */
         sysctrl->contiguous_map = val & 0x01;
+        qemu_set_irq(sysctrl->contiguous_map_irq, sysctrl->contiguous_map);
         break;
     default:
         printf("ERROR: unaffected IO port write: %04" PRIx32
@@ -327,91 +329,6 @@  static uint32_t PREP_io_800_readb (void *opaque, uint32_t addr)
     return retval;
 }
 
-static inline hwaddr prep_IO_address(sysctrl_t *sysctrl,
-                                                 hwaddr addr)
-{
-    if (sysctrl->contiguous_map == 0) {
-        /* 64 KB contiguous space for IOs */
-        addr &= 0xFFFF;
-    } else {
-        /* 8 MB non-contiguous space for IOs */
-        addr = (addr & 0x1F) | ((addr & 0x007FFF000) >> 7);
-    }
-
-    return addr;
-}
-
-static void PPC_prep_io_writeb (void *opaque, hwaddr addr,
-                                uint32_t value)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    addr = prep_IO_address(sysctrl, addr);
-    cpu_outb(addr, value);
-}
-
-static uint32_t PPC_prep_io_readb (void *opaque, hwaddr addr)
-{
-    sysctrl_t *sysctrl = opaque;
-    uint32_t ret;
-
-    addr = prep_IO_address(sysctrl, addr);
-    ret = cpu_inb(addr);
-
-    return ret;
-}
-
-static void PPC_prep_io_writew (void *opaque, hwaddr addr,
-                                uint32_t value)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    addr = prep_IO_address(sysctrl, addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " => 0x%08" PRIx32 "\n", addr, value);
-    cpu_outw(addr, value);
-}
-
-static uint32_t PPC_prep_io_readw (void *opaque, hwaddr addr)
-{
-    sysctrl_t *sysctrl = opaque;
-    uint32_t ret;
-
-    addr = prep_IO_address(sysctrl, addr);
-    ret = cpu_inw(addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " <= 0x%08" PRIx32 "\n", addr, ret);
-
-    return ret;
-}
-
-static void PPC_prep_io_writel (void *opaque, hwaddr addr,
-                                uint32_t value)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    addr = prep_IO_address(sysctrl, addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " => 0x%08" PRIx32 "\n", addr, value);
-    cpu_outl(addr, value);
-}
-
-static uint32_t PPC_prep_io_readl (void *opaque, hwaddr addr)
-{
-    sysctrl_t *sysctrl = opaque;
-    uint32_t ret;
-
-    addr = prep_IO_address(sysctrl, addr);
-    ret = cpu_inl(addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " <= 0x%08" PRIx32 "\n", addr, ret);
-
-    return ret;
-}
-
-static const MemoryRegionOps PPC_prep_io_ops = {
-    .old_mmio = {
-        .read = { PPC_prep_io_readb, PPC_prep_io_readw, PPC_prep_io_readl },
-        .write = { PPC_prep_io_writeb, PPC_prep_io_writew, PPC_prep_io_writel },
-    },
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
 
 #define NVRAM_SIZE        0x2000
 
@@ -458,7 +375,6 @@  static void ppc_prep_init(QEMUMachineInitArgs *args)
     CPUPPCState *env = NULL;
     nvram_t nvram;
     M48t59State *m48t59;
-    MemoryRegion *PPC_io_memory = g_new(MemoryRegion, 1);
     PortioList *port_list = g_new(PortioList, 1);
 #if 0
     MemoryRegion *xcsr = g_new(MemoryRegion, 1);
@@ -567,6 +483,7 @@  static void ppc_prep_init(QEMUMachineInitArgs *args)
         fprintf(stderr, "Couldn't create PCI host controller.\n");
         exit(1);
     }
+    sysctrl->contiguous_map_irq = qdev_get_gpio_in(dev, 0);
 
     /* PCI -> ISA bridge */
     pci = pci_create_simple(pci_bus, PCI_DEVFN(1, 0), "i82378");
@@ -587,11 +504,6 @@  static void ppc_prep_init(QEMUMachineInitArgs *args)
     qdev_prop_set_uint8(dev, "config", 13); /* fdc, ser0, ser1, par0 */
     qdev_init_nofail(dev);
 
-    /* Register 8 MB of ISA IO space (needed for non-contiguous map) */
-    memory_region_init_io(PPC_io_memory, NULL, &PPC_prep_io_ops, sysctrl,
-                          "ppc-io", 0x00800000);
-    memory_region_add_subregion(sysmem, 0x80000000, PPC_io_memory);
-
     /* init basic PC hardware */
     pci_vga_init(pci_bus);