Patchwork [1/2] Keep track of ISA ports ISA device is using in qdev.

login
register
mail settings
Submitter Gleb Natapov
Date Oct. 24, 2010, 2:02 p.m.
Message ID <1287928933-11423-2-git-send-email-gleb@redhat.com>
Download mbox | patch
Permalink /patch/69037/
State New
Headers show

Comments

Gleb Natapov - Oct. 24, 2010, 2:02 p.m.
Prevent two devices from claiming the same io port.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 hw/cs4231a.c     |    1 +
 hw/fdc.c         |    3 +++
 hw/gus.c         |    4 ++++
 hw/ide/isa.c     |    2 ++
 hw/isa-bus.c     |   31 +++++++++++++++++++++++++++++++
 hw/isa.h         |    4 ++++
 hw/m48t59.c      |    1 +
 hw/mc146818rtc.c |    1 +
 hw/ne2000-isa.c  |    3 +++
 hw/parallel.c    |    5 +++++
 hw/pckbd.c       |    3 +++
 hw/sb16.c        |    4 ++++
 hw/serial.c      |    1 +
 13 files changed, 63 insertions(+), 0 deletions(-)
Markus Armbruster - Oct. 25, 2010, 3:06 p.m.
Gleb Natapov <gleb@redhat.com> writes:

> Prevent two devices from claiming the same io port.

Really?

Your new check for double-claim is in the new isa_init_ioport(), which
is for ISADevice only.  Thus, only qdevified ISA devices can opt for
this protection, by calling isa_init_ioport().  It doesn't protect from
devices who don't or can't opt in, such as PCI devices.

Anyway, we already check for double-claim in
register_ioport_{read,write}().  The check has issues --- hw_error() is
wrong there for hot plug.  But it's where the check should be, isn't it?
Gleb Natapov - Oct. 25, 2010, 4:58 p.m.
On Mon, Oct 25, 2010 at 05:06:51PM +0200, Markus Armbruster wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > Prevent two devices from claiming the same io port.
> 
> Really?
> 
> Your new check for double-claim is in the new isa_init_ioport(), which
> is for ISADevice only.  Thus, only qdevified ISA devices can opt for
> this protection, by calling isa_init_ioport().  It doesn't protect from
> devices who don't or can't opt in, such as PCI devices.
> 
I didn't claim different. This obviously works only for ISA qdev
devices.

> Anyway, we already check for double-claim in
> register_ioport_{read,write}().  The check has issues --- hw_error() is
> wrong there for hot plug.  But it's where the check should be, isn't it?
You don't like double-claim checking? Forget about it (all 3 lines
of code). The real point of the patch is to have ISA resources used
by devices to be stored in common place (ISADevice) which allows
get_dev_path() to be implemented.

--
			Gleb.
Markus Armbruster - Oct. 25, 2010, 6:01 p.m.
Gleb Natapov <gleb@redhat.com> writes:

> On Mon, Oct 25, 2010 at 05:06:51PM +0200, Markus Armbruster wrote:
>> Gleb Natapov <gleb@redhat.com> writes:
>> 
>> > Prevent two devices from claiming the same io port.
>> 
>> Really?
>> 
>> Your new check for double-claim is in the new isa_init_ioport(), which
>> is for ISADevice only.  Thus, only qdevified ISA devices can opt for
>> this protection, by calling isa_init_ioport().  It doesn't protect from
>> devices who don't or can't opt in, such as PCI devices.
>> 
> I didn't claim different. This obviously works only for ISA qdev
> devices.

I read "Prevent two devices from claiming the same io port" as such.

>> Anyway, we already check for double-claim in
>> register_ioport_{read,write}().  The check has issues --- hw_error() is
>> wrong there for hot plug.  But it's where the check should be, isn't it?
> You don't like double-claim checking? Forget about it (all 3 lines
> of code). The real point of the patch is to have ISA resources used
> by devices to be stored in common place (ISADevice) which allows
> get_dev_path() to be implemented.

We already track I/O port use, in ioport.c.  I don't like that
duplicated.  Even less so if the dupe catches fewer double-claims than
the original.

Perhaps your new function should wrap around register_ioport_*() instead
of supplementing it.
Gleb Natapov - Oct. 25, 2010, 6:15 p.m.
On Mon, Oct 25, 2010 at 08:01:13PM +0200, Markus Armbruster wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Mon, Oct 25, 2010 at 05:06:51PM +0200, Markus Armbruster wrote:
> >> Gleb Natapov <gleb@redhat.com> writes:
> >> 
> >> > Prevent two devices from claiming the same io port.
> >> 
> >> Really?
> >> 
> >> Your new check for double-claim is in the new isa_init_ioport(), which
> >> is for ISADevice only.  Thus, only qdevified ISA devices can opt for
> >> this protection, by calling isa_init_ioport().  It doesn't protect from
> >> devices who don't or can't opt in, such as PCI devices.
> >> 
> > I didn't claim different. This obviously works only for ISA qdev
> > devices.
> 
> I read "Prevent two devices from claiming the same io port" as such.
> 
And have you read subject with that?

> >> Anyway, we already check for double-claim in
> >> register_ioport_{read,write}().  The check has issues --- hw_error() is
> >> wrong there for hot plug.  But it's where the check should be, isn't it?
> > You don't like double-claim checking? Forget about it (all 3 lines
> > of code). The real point of the patch is to have ISA resources used
> > by devices to be stored in common place (ISADevice) which allows
> > get_dev_path() to be implemented.
> 
> We already track I/O port use, in ioport.c.  I don't like that
> duplicated.  Even less so if the dupe catches fewer double-claims than
> the original.
Consider it removed although we do keep track of irqs there and this
tracking is also incomplete. Why is it there?

> 
> Perhaps your new function should wrap around register_ioport_*() instead
> of supplementing it.
register_ioport_*() is disconnected from qdev in general and from ISADEvice
in particular, so how "wrap around register_ioport_*()" will help me to
have get_dev_path() for ISABus is beyond my understanding.

--
			Gleb.
Markus Armbruster - Oct. 26, 2010, 9:49 a.m.
Gleb Natapov <gleb@redhat.com> writes:

> On Mon, Oct 25, 2010 at 08:01:13PM +0200, Markus Armbruster wrote:
>> Gleb Natapov <gleb@redhat.com> writes:
>> 
>> > On Mon, Oct 25, 2010 at 05:06:51PM +0200, Markus Armbruster wrote:
>> >> Gleb Natapov <gleb@redhat.com> writes:
>> >> 
>> >> > Prevent two devices from claiming the same io port.
>> >> 
>> >> Really?
>> >> 
>> >> Your new check for double-claim is in the new isa_init_ioport(), which
>> >> is for ISADevice only.  Thus, only qdevified ISA devices can opt for
>> >> this protection, by calling isa_init_ioport().  It doesn't protect from
>> >> devices who don't or can't opt in, such as PCI devices.
>> >> 
>> > I didn't claim different. This obviously works only for ISA qdev
>> > devices.
>> 
>> I read "Prevent two devices from claiming the same io port" as such.
>> 
> And have you read subject with that?

Yes.  Nevertheless, I found it a bit misleading, so I commented.

>> >> Anyway, we already check for double-claim in
>> >> register_ioport_{read,write}().  The check has issues --- hw_error() is
>> >> wrong there for hot plug.  But it's where the check should be, isn't it?
>> > You don't like double-claim checking? Forget about it (all 3 lines
>> > of code). The real point of the patch is to have ISA resources used
>> > by devices to be stored in common place (ISADevice) which allows
>> > get_dev_path() to be implemented.
>> 
>> We already track I/O port use, in ioport.c.  I don't like that
>> duplicated.  Even less so if the dupe catches fewer double-claims than
>> the original.
> Consider it removed although we do keep track of irqs there and this
> tracking is also incomplete. Why is it there?

Where's "there"?

Let's not add to the code duplication if we can help it.  It's bad
enough as it is.

>> Perhaps your new function should wrap around register_ioport_*() instead
>> of supplementing it.
> register_ioport_*() is disconnected from qdev in general and from ISADEvice
> in particular, so how "wrap around register_ioport_*()" will help me to
> have get_dev_path() for ISABus is beyond my understanding.

I was too terse, sorry.

Maybe we should have functions for ISADevices to register ISA resources.
Functions that are *not* disconnected from qdev.  Instead of code like

        register_ioport_write(pm_io_base, 64, 2, pm_ioport_writew, s);
        register_ioport_read(pm_io_base, 64, 2, pm_ioport_readw, s);
        register_ioport_write(pm_io_base, 64, 4, pm_ioport_writel, s);
        register_ioport_read(pm_io_base, 64, 4, pm_ioport_readl, s);
        isa_init_ioport_range(dev, pm_ioport_base, 64);

or, with Avi's proposed interface

        ioport_register(&s->ioport, pm_io_base, 64);
        isa_init_ioport_range(dev, pm_ioport_base, 64);

we'd get something like

        isa_ioport_register(dev, &s->ioport, pm_io_base, 64);

Isn't that neater?

Since some PCI devices also register ISA resources, we'd have to 
offer functions for them as well, properly integrated.

Now, I didn't mean to ask you to do all that as a precondition for
getting your patch in.  I was just observing.
Gleb Natapov - Oct. 26, 2010, 10:29 a.m.
On Tue, Oct 26, 2010 at 11:49:26AM +0200, Markus Armbruster wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Mon, Oct 25, 2010 at 08:01:13PM +0200, Markus Armbruster wrote:
> >> Gleb Natapov <gleb@redhat.com> writes:
> >> 
> >> > On Mon, Oct 25, 2010 at 05:06:51PM +0200, Markus Armbruster wrote:
> >> >> Gleb Natapov <gleb@redhat.com> writes:
> >> >> 
> >> >> > Prevent two devices from claiming the same io port.
> >> >> 
> >> >> Really?
> >> >> 
> >> >> Your new check for double-claim is in the new isa_init_ioport(), which
> >> >> is for ISADevice only.  Thus, only qdevified ISA devices can opt for
> >> >> this protection, by calling isa_init_ioport().  It doesn't protect from
> >> >> devices who don't or can't opt in, such as PCI devices.
> >> >> 
> >> > I didn't claim different. This obviously works only for ISA qdev
> >> > devices.
> >> 
> >> I read "Prevent two devices from claiming the same io port" as such.
> >> 
> > And have you read subject with that?
> 
> Yes.  Nevertheless, I found it a bit misleading, so I commented.
> 
Well, I hope it is clear now.

> >> >> Anyway, we already check for double-claim in
> >> >> register_ioport_{read,write}().  The check has issues --- hw_error() is
> >> >> wrong there for hot plug.  But it's where the check should be, isn't it?
> >> > You don't like double-claim checking? Forget about it (all 3 lines
> >> > of code). The real point of the patch is to have ISA resources used
> >> > by devices to be stored in common place (ISADevice) which allows
> >> > get_dev_path() to be implemented.
> >> 
> >> We already track I/O port use, in ioport.c.  I don't like that
> >> duplicated.  Even less so if the dupe catches fewer double-claims than
> >> the original.
> > Consider it removed although we do keep track of irqs there and this
> > tracking is also incomplete. Why is it there?
> 
> Where's "there"?
> 
In ISADevice. Look for "isabus->assigned" in isa-bus.c.

> Let's not add to the code duplication if we can help it.  It's bad
> enough as it is.
> 
> >> Perhaps your new function should wrap around register_ioport_*() instead
> >> of supplementing it.
> > register_ioport_*() is disconnected from qdev in general and from ISADEvice
> > in particular, so how "wrap around register_ioport_*()" will help me to
> > have get_dev_path() for ISABus is beyond my understanding.
> 
> I was too terse, sorry.
> 
> Maybe we should have functions for ISADevices to register ISA resources.
> Functions that are *not* disconnected from qdev.  Instead of code like
> 
>         register_ioport_write(pm_io_base, 64, 2, pm_ioport_writew, s);
>         register_ioport_read(pm_io_base, 64, 2, pm_ioport_readw, s);
>         register_ioport_write(pm_io_base, 64, 4, pm_ioport_writel, s);
>         register_ioport_read(pm_io_base, 64, 4, pm_ioport_readl, s);
>         isa_init_ioport_range(dev, pm_ioport_base, 64);
> 
> or, with Avi's proposed interface
> 
>         ioport_register(&s->ioport, pm_io_base, 64);
>         isa_init_ioport_range(dev, pm_ioport_base, 64);
> 
> we'd get something like
> 
>         isa_ioport_register(dev, &s->ioport, pm_io_base, 64);
> 
> Isn't that neater?
> 
Neater, but out of scope for my work. Obviously all resource
management should go via qdev at the end.

> Since some PCI devices also register ISA resources, we'd have to 
> offer functions for them as well, properly integrated.
> 
> Now, I didn't mean to ask you to do all that as a precondition for
> getting your patch in.  I was just observing.
OK.


--
			Gleb.

Patch

diff --git a/hw/cs4231a.c b/hw/cs4231a.c
index 4d5ce5c..598f032 100644
--- a/hw/cs4231a.c
+++ b/hw/cs4231a.c
@@ -645,6 +645,7 @@  static int cs4231a_initfn (ISADevice *dev)
     isa_init_irq (dev, &s->pic, s->irq);
 
     for (i = 0; i < 4; i++) {
+        isa_init_ioport(dev, i);
         register_ioport_write (s->port + i, 1, 1, cs_write, s);
         register_ioport_read (s->port + i, 1, 1, cs_read, s);
     }
diff --git a/hw/fdc.c b/hw/fdc.c
index c159dcb..1f38d0d 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1983,6 +1983,9 @@  static int isabus_fdc_init1(ISADevice *dev)
                           &fdctrl_write_port, fdctrl);
     register_ioport_write(iobase + 0x07, 1, 1,
                           &fdctrl_write_port, fdctrl);
+    isa_init_ioport_range(dev, iobase + 1, 5);
+    isa_init_ioport(dev, iobase + 7);
+
     isa_init_irq(&isa->busdev, &fdctrl->irq, isairq);
     fdctrl->dma_chann = dma_chann;
 
diff --git a/hw/gus.c b/hw/gus.c
index e9016d8..ff9e7c7 100644
--- a/hw/gus.c
+++ b/hw/gus.c
@@ -264,20 +264,24 @@  static int gus_initfn (ISADevice *dev)
 
     register_ioport_write (s->port, 1, 1, gus_writeb, s);
     register_ioport_write (s->port, 1, 2, gus_writew, s);
+    isa_init_ioport_range(dev, s->port, 2);
 
     register_ioport_read ((s->port + 0x100) & 0xf00, 1, 1, gus_readb, s);
     register_ioport_read ((s->port + 0x100) & 0xf00, 1, 2, gus_readw, s);
+    isa_init_ioport_range(dev, (s->port + 0x100) & 0xf00, 2);
 
     register_ioport_write (s->port + 6, 10, 1, gus_writeb, s);
     register_ioport_write (s->port + 6, 10, 2, gus_writew, s);
     register_ioport_read (s->port + 6, 10, 1, gus_readb, s);
     register_ioport_read (s->port + 6, 10, 2, gus_readw, s);
+    isa_init_ioport_range(dev, s->port + 6, 10);
 
 
     register_ioport_write (s->port + 0x100, 8, 1, gus_writeb, s);
     register_ioport_write (s->port + 0x100, 8, 2, gus_writew, s);
     register_ioport_read (s->port + 0x100, 8, 1, gus_readb, s);
     register_ioport_read (s->port + 0x100, 8, 2, gus_readw, s);
+    isa_init_ioport_range(dev, s->port + 0x100, 8);
 
     DMA_register_channel (s->emu.gusdma, GUS_read_DMA, s);
     s->emu.himemaddr = s->himem;
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 6b57e0d..163ffba 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -70,6 +70,8 @@  static int isa_ide_initfn(ISADevice *dev)
     ide_bus_new(&s->bus, &s->dev.qdev);
     ide_init_ioport(&s->bus, s->iobase, s->iobase2);
     isa_init_irq(dev, &s->irq, s->isairq);
+    isa_init_ioport_range(dev, s->iobase, 8);
+    isa_init_ioport(dev, s->iobase2);
     ide_init2(&s->bus, s->irq);
     vmstate_register(&dev->qdev, 0, &vmstate_ide_isa, s);
     return 0;
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 4e306de..c509d56 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -26,6 +26,7 @@  struct ISABus {
     BusState qbus;
     qemu_irq *irqs;
     uint32_t assigned;
+    uint32_t assigned_iop[65536/32];
 };
 static ISABus *isabus;
 target_phys_addr_t isa_mem_base = 0;
@@ -92,6 +93,36 @@  void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
     dev->nirqs++;
 }
 
+static void isa_init_ioport_one(ISADevice *dev, uint16_t ioport)
+{
+    assert(dev->nioports < ARRAY_SIZE(dev->ioports));
+    if (isabus->assigned_iop[ioport >> 5] & (1 << (ioport & 0x1f))) {
+        fprintf(stderr, "isa ioport 0x%x already assigned\n", ioport);
+        exit(1);
+    }
+    isabus->assigned_iop[ioport >> 5] |= (1 << (ioport & 0x1f));
+    dev->ioports[dev->nioports++] = ioport;
+}
+
+static int isa_cmp_ports(const void *p1, const void *p2)
+{
+    return *(uint16_t*)p1 - *(uint16_t*)p2;
+}
+
+void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t length)
+{
+    int i;
+    for (i = start; i < start + length; i++) {
+        isa_init_ioport_one(dev, i);
+    }
+    qsort(dev->ioports, dev->nioports, sizeof(dev->ioports[0]), isa_cmp_ports);
+}
+
+void isa_init_ioport(ISADevice *dev, uint16_t ioport)
+{
+    isa_init_ioport_range(dev, ioport, 1);
+}
+
 static int isa_qdev_init(DeviceState *qdev, DeviceInfo *base)
 {
     ISADevice *dev = DO_UPCAST(ISADevice, qdev, qdev);
diff --git a/hw/isa.h b/hw/isa.h
index aaf0272..4794b76 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -14,6 +14,8 @@  struct ISADevice {
     DeviceState qdev;
     uint32_t isairq[2];
     int nirqs;
+    uint16_t ioports[32];
+    int nioports;
 };
 
 typedef int (*isa_qdev_initfn)(ISADevice *dev);
@@ -26,6 +28,8 @@  ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 qemu_irq isa_reserve_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
+void isa_init_ioport(ISADevice *dev, uint16_t ioport);
+void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create(const char *name);
 ISADevice *isa_create_simple(const char *name);
diff --git a/hw/m48t59.c b/hw/m48t59.c
index c7492a6..75a94e1 100644
--- a/hw/m48t59.c
+++ b/hw/m48t59.c
@@ -680,6 +680,7 @@  M48t59State *m48t59_init_isa(uint32_t io_base, uint16_t size, int type)
     if (io_base != 0) {
         register_ioport_read(io_base, 0x04, 1, NVRAM_readb, s);
         register_ioport_write(io_base, 0x04, 1, NVRAM_writeb, s);
+        isa_init_ioport_range(dev, io_base, 4);
     }
 
     return s;
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 2b91fa8..6466aff 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -605,6 +605,7 @@  static int rtc_initfn(ISADevice *dev)
 
     register_ioport_write(base, 2, 1, cmos_ioport_write, s);
     register_ioport_read(base, 2, 1, cmos_ioport_read, s);
+    isa_init_ioport_range(dev, base, 2);
 
     qdev_set_legacy_instance_id(&dev->qdev, base, 2);
     qemu_register_reset(rtc_reset, s);
diff --git a/hw/ne2000-isa.c b/hw/ne2000-isa.c
index 03a5a1f..3ff0d89 100644
--- a/hw/ne2000-isa.c
+++ b/hw/ne2000-isa.c
@@ -68,14 +68,17 @@  static int isa_ne2000_initfn(ISADevice *dev)
 
     register_ioport_write(isa->iobase, 16, 1, ne2000_ioport_write, s);
     register_ioport_read(isa->iobase, 16, 1, ne2000_ioport_read, s);
+    isa_init_ioport_range(dev, isa->iobase, 16);
 
     register_ioport_write(isa->iobase + 0x10, 1, 1, ne2000_asic_ioport_write, s);
     register_ioport_read(isa->iobase + 0x10, 1, 1, ne2000_asic_ioport_read, s);
     register_ioport_write(isa->iobase + 0x10, 2, 2, ne2000_asic_ioport_write, s);
     register_ioport_read(isa->iobase + 0x10, 2, 2, ne2000_asic_ioport_read, s);
+    isa_init_ioport_range(dev, isa->iobase + 0x10, 2);
 
     register_ioport_write(isa->iobase + 0x1f, 1, 1, ne2000_reset_ioport_write, s);
     register_ioport_read(isa->iobase + 0x1f, 1, 1, ne2000_reset_ioport_read, s);
+    isa_init_ioport(dev, isa->iobase + 0x1f);
 
     isa_init_irq(dev, &s->irq, isa->isairq);
 
diff --git a/hw/parallel.c b/hw/parallel.c
index 6b11672..6270b53 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -481,16 +481,21 @@  static int parallel_isa_initfn(ISADevice *dev)
     if (s->hw_driver) {
         register_ioport_write(base, 8, 1, parallel_ioport_write_hw, s);
         register_ioport_read(base, 8, 1, parallel_ioport_read_hw, s);
+        isa_init_ioport_range(dev, base, 8);
+
         register_ioport_write(base+4, 1, 2, parallel_ioport_eppdata_write_hw2, s);
         register_ioport_read(base+4, 1, 2, parallel_ioport_eppdata_read_hw2, s);
         register_ioport_write(base+4, 1, 4, parallel_ioport_eppdata_write_hw4, s);
         register_ioport_read(base+4, 1, 4, parallel_ioport_eppdata_read_hw4, s);
+        isa_init_ioport(dev, base+4);
         register_ioport_write(base+0x400, 8, 1, parallel_ioport_ecp_write, s);
         register_ioport_read(base+0x400, 8, 1, parallel_ioport_ecp_read, s);
+        isa_init_ioport_range(dev, base+0x400, 8);
     }
     else {
         register_ioport_write(base, 8, 1, parallel_ioport_write_sw, s);
         register_ioport_read(base, 8, 1, parallel_ioport_read_sw, s);
+        isa_init_ioport_range(dev, base, 8);
     }
     return 0;
 }
diff --git a/hw/pckbd.c b/hw/pckbd.c
index 6e4e406..e736505 100644
--- a/hw/pckbd.c
+++ b/hw/pckbd.c
@@ -484,10 +484,13 @@  static int i8042_initfn(ISADevice *dev)
 
     register_ioport_read(0x60, 1, 1, kbd_read_data, s);
     register_ioport_write(0x60, 1, 1, kbd_write_data, s);
+    isa_init_ioport(dev, 0x60);
     register_ioport_read(0x64, 1, 1, kbd_read_status, s);
     register_ioport_write(0x64, 1, 1, kbd_write_command, s);
+    isa_init_ioport(dev, 0x64);
     register_ioport_read(0x92, 1, 1, ioport92_read, s);
     register_ioport_write(0x92, 1, 1, ioport92_write, s);
+    isa_init_ioport(dev, 0x92);
 
     s->kbd = ps2_kbd_init(kbd_update_kbd_irq, s);
     s->mouse = ps2_mouse_init(kbd_update_aux_irq, s);
diff --git a/hw/sb16.c b/hw/sb16.c
index 78590a7..c9d37ad 100644
--- a/hw/sb16.c
+++ b/hw/sb16.c
@@ -1368,16 +1368,20 @@  static int sb16_initfn (ISADevice *dev)
 
     for (i = 0; i < ARRAY_SIZE (dsp_write_ports); i++) {
         register_ioport_write (s->port + dsp_write_ports[i], 1, 1, dsp_write, s);
+        isa_init_ioport(dev, s->port + dsp_write_ports[i]);
     }
 
     for (i = 0; i < ARRAY_SIZE (dsp_read_ports); i++) {
         register_ioport_read (s->port + dsp_read_ports[i], 1, 1, dsp_read, s);
+        isa_init_ioport(dev, s->port + dsp_read_ports[i]);
     }
 
     register_ioport_write (s->port + 0x4, 1, 1, mixer_write_indexb, s);
     register_ioport_write (s->port + 0x4, 1, 2, mixer_write_indexw, s);
+    isa_init_ioport(dev, s->port + 0x4);
     register_ioport_read (s->port + 0x5, 1, 1, mixer_read, s);
     register_ioport_write (s->port + 0x5, 1, 1, mixer_write_datab, s);
+    isa_init_ioport(dev, s->port + 0x5);
 
     DMA_register_channel (s->hdma, SB_read_DMA, s);
     DMA_register_channel (s->dma, SB_read_DMA, s);
diff --git a/hw/serial.c b/hw/serial.c
index 9ebc452..20902ae 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -778,6 +778,7 @@  static int serial_isa_initfn(ISADevice *dev)
 
     register_ioport_write(isa->iobase, 8, 1, serial_ioport_write, s);
     register_ioport_read(isa->iobase, 8, 1, serial_ioport_read, s);
+    isa_init_ioport_range(dev, isa->iobase, 8);
     return 0;
 }