[Qemu-devel] [RFC 00/10] ISA reconfigurability

[Qemu-devel] [RFC 00/10] ISA reconfigurability

[Andreas Färber]

Hello,

The current qdev model seems to assume that a PC is equipped with the controllers
and devices (part of the default config or) explicitly passed as parameters by
the user.

In reality though, a PC is usually equipped with a fixed set of internal devices,
which can be enabled/disabled and configured from the BIOS menu.

Similarly, the PC87312 Super I/O chipset used in IBM PReP machines aggregates
a number of PC-compatible devices and allows to reconfigure them through register
writes. This is actually done by the real IBM firmware we're trying to boot.

The original patch by Hervé unplugged and added new isa-parallel, etc. devices
for each reconfiguration. That failed because the devices are not meant to be
hot-pluggable.

This series proposes a new solution: helpers that reconfigure existing ISADevices.

It also contains a resend of a prerequisite patch with updated summary.

As for the remaining issue of enabling/disabling devices I'd like to hear
opinions on whether we should allow hot-plugging of these devices or
whether we should add an enabled/disabled state to a qdev device.
The latter would help avoid guest-triggerable qdev_init[_nofail]() failures but
would crowd the qtree and thus be a user-visible change.

Further, some qdev devices like isa-parallel and isa-serial seem to depend on a
valid char device. The runtime choice of whether to enable the parallel port is
obviously independent of whether the user supplied us with a valid chr property.
Do we have a nop char driver to use in such a case?

Thanks,

Andreas


Andreas Färber (9):
  isa: Allow to un-assign I/O ports
  isa: Allow to un-associate an IRQ
  parallel: Allow to reconfigure ISA I/O base
  parallel: Allow to reconfigure ISA IRQ
  serial: Allow to reconfigure ISA I/O base
  serial: Allow to reconfigure ISA IRQ
  fdc: Allow to reconfigure ISA I/O base
  ide: Allow to reconfigure ISA I/O base
  prep: Add pc87312 Super I/O emulation

Hervé Poussineau (1):
  fdc: Parametrize ISA base, IRQ and DMA

 Makefile.objs                   |    1 +
 default-configs/ppc-softmmu.mak |    2 +
 hw/fdc.c                        |   56 ++++--
 hw/fdc.h                        |    3 +
 hw/ide.h                        |    1 +
 hw/ide/core.c                   |    8 +
 hw/ide/internal.h               |    1 +
 hw/ide/isa.c                    |   28 +++-
 hw/isa-bus.c                    |   28 +++
 hw/isa.h                        |   10 +
 hw/parallel.c                   |   83 ++++++--
 hw/pc87312.c                    |  435 +++++++++++++++++++++++++++++++++++++++
 hw/serial.c                     |   34 +++-
 13 files changed, 647 insertions(+), 43 deletions(-)
 create mode 100644 hw/pc87312.c

-- 
1.7.5.3

[Qemu-devel] [RFC 01/10] isa: Allow to un-assign I/O ports

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 2765543..46716ad 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -105,6 +105,20 @@ void isa_init_ioport(ISADevice *dev, uint16_t ioport)
     isa_init_ioport_range(dev, ioport, 1);
 }
 
+void isa_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length)
+{
+    int i, j;
+    for (i = 0; i < dev->nioports; i++) {
+        if (dev->ioports[i] == start) {
+            for (j = 0; j < dev->nioports - i; j++) {
+                dev->ioports[i + j] = dev->ioports[i + length + j];
+            }
+            dev->nioports -= length;
+            break;
+        }
+    }
+}
+
 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 d2b6126..1b41a27 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -30,6 +30,7 @@ qemu_irq isa_get_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_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create(const char *name);
 ISADevice *isa_try_create(const char *name);
-- 
1.7.5.3

[Qemu-devel] [RFC 02/10] isa: Allow to un-associate an IRQ

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 46716ad..acccadb 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -80,6 +80,20 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
     dev->nirqs++;
 }
 
+void isa_discard_irq(ISADevice *dev, int isairq)
+{
+    int i, j;
+    for (i = 0; i < dev->nirqs; i++) {
+        if (dev->isairq[i] == isairq) {
+            for (j = i + 1; j < dev->nirqs; j++) {
+                dev->isairq[j - 1] = dev->isairq[j];
+            }
+            dev->nirqs--;
+            break;
+        }
+    }
+}
+
 static void isa_init_ioport_one(ISADevice *dev, uint16_t ioport)
 {
     assert(dev->nioports < ARRAY_SIZE(dev->ioports));
diff --git a/hw/isa.h b/hw/isa.h
index 1b41a27..789d91c 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -28,6 +28,7 @@ ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 qemu_irq isa_get_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
+void isa_discard_irq(ISADevice *dev, 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_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
-- 
1.7.5.3

[Qemu-devel] [RFC 03/10] parallel: Allow to reconfigure ISA I/O base

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa.h      |    3 ++
 hw/parallel.c |   70 +++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/hw/isa.h b/hw/isa.h
index 789d91c..2bd8c82 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -41,6 +41,9 @@ extern target_phys_addr_t isa_mem_base;
 
 void isa_mmio_init(target_phys_addr_t base, target_phys_addr_t size);
 
+/* parallel.c */
+void parallel_isa_reconfigure_iobase(ISADevice *dev, uint32_t base);
+
 /* dma.c */
 int DMA_get_channel_mode (int nchan);
 int DMA_read_memory (int nchan, void *buf, int pos, int size);
diff --git a/hw/parallel.c b/hw/parallel.c
index cc853a5..5cb3856 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -446,6 +446,53 @@ static void parallel_reset(void *opaque)
     s->last_read_offset = ~0U;
 }
 
+static void parallel_isa_init_iobase(ISAParallelState *isa)
+{
+    ISADevice *dev = &isa->dev;
+    ParallelState *s = &isa->state;
+    int base;
+
+    base = isa->iobase;
+    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);
+    }
+}
+
+void parallel_isa_reconfigure_iobase(ISADevice *dev, uint32_t base)
+{
+    ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
+    ParallelState *s = &isa->state;
+
+    if (base != isa->iobase) {
+        isa_discard_ioport_range(dev, base, 8);
+        isa_unassign_ioport(base, 8);
+        if (s->hw_driver) {
+            isa_discard_ioport_range(dev, base + 4, 1);
+            isa_unassign_ioport(base + 4, 1);
+            isa_discard_ioport_range(dev, base + 0x400, 8);
+            isa_unassign_ioport(base + 0x400, 8);
+        }
+
+        isa->iobase = base;
+        parallel_isa_init_iobase(isa);
+    }
+}
+
 static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
 
 static int parallel_isa_initfn(ISADevice *dev)
@@ -453,7 +500,6 @@ static int parallel_isa_initfn(ISADevice *dev)
     static int index;
     ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
     ParallelState *s = &isa->state;
-    int base;
     uint8_t dummy;
 
     if (!s->chr) {
@@ -469,7 +515,6 @@ static int parallel_isa_initfn(ISADevice *dev)
         isa->iobase = isa_parallel_io[isa->index];
     index++;
 
-    base = isa->iobase;
     isa_init_irq(dev, &s->irq, isa->isairq);
     qemu_register_reset(parallel_reset, s);
 
@@ -477,26 +522,7 @@ static int parallel_isa_initfn(ISADevice *dev)
         s->hw_driver = 1;
         s->status = dummy;
     }
-
-    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);
-    }
+    parallel_isa_init_iobase(isa);
     return 0;
 }
 
-- 
1.7.5.3

[Qemu-devel] [RFC 04/10] parallel: Allow to reconfigure ISA IRQ

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa.h      |    1 +
 hw/parallel.c |   13 +++++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/hw/isa.h b/hw/isa.h
index 2bd8c82..54698b5 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -43,6 +43,7 @@ void isa_mmio_init(target_phys_addr_t base, target_phys_addr_t size);
 
 /* parallel.c */
 void parallel_isa_reconfigure_iobase(ISADevice *dev, uint32_t base);
+void parallel_isa_reconfigure_irq(ISADevice *dev, uint32_t isairq);
 
 /* dma.c */
 int DMA_get_channel_mode (int nchan);
diff --git a/hw/parallel.c b/hw/parallel.c
index 5cb3856..a64e7c5 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -493,6 +493,18 @@ void parallel_isa_reconfigure_iobase(ISADevice *dev, uint32_t base)
     }
 }
 
+void parallel_isa_reconfigure_irq(ISADevice *dev, uint32_t isairq)
+{
+    ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
+    ParallelState *s = &isa->state;
+
+    if (isairq != isa->isairq) {
+        isa_discard_irq(dev, isa->isairq);
+        isa->isairq = isairq;
+        isa_init_irq(dev, &s->irq, isa->isairq);
+    }
+}
+
 static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
 
 static int parallel_isa_initfn(ISADevice *dev)
@@ -516,6 +528,7 @@ static int parallel_isa_initfn(ISADevice *dev)
     index++;
 
     isa_init_irq(dev, &s->irq, isa->isairq);
+
     qemu_register_reset(parallel_reset, s);
 
     if (qemu_chr_ioctl(s->chr, CHR_IOCTL_PP_READ_STATUS, &dummy) == 0) {
-- 
1.7.5.3

[Qemu-devel] [RFC 05/10] serial: Allow to reconfigure ISA I/O base

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa.h    |    3 +++
 hw/serial.c |   23 ++++++++++++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/hw/isa.h b/hw/isa.h
index 54698b5..8bd082a 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -45,6 +45,9 @@ void isa_mmio_init(target_phys_addr_t base, target_phys_addr_t size);
 void parallel_isa_reconfigure_iobase(ISADevice *dev, uint32_t base);
 void parallel_isa_reconfigure_irq(ISADevice *dev, uint32_t isairq);
 
+/* serial.c */
+void serial_isa_reconfigure_iobase(ISADevice *dev, uint32_t base);
+
 /* dma.c */
 int DMA_get_channel_mode (int nchan);
 int DMA_read_memory (int nchan, void *buf, int pos, int size);
diff --git a/hw/serial.c b/hw/serial.c
index 0ee61dd..dd91272 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -752,6 +752,25 @@ void serial_set_frequency(SerialState *s, uint32_t frequency)
     serial_update_parameters(s);
 }
 
+static void serial_isa_init_iobase(ISASerialState *isa)
+{
+    SerialState *s = &isa->state;
+
+    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(&isa->dev, isa->iobase, 8);
+}
+
+void serial_isa_reconfigure_iobase(ISADevice *dev, uint32_t iobase)
+{
+    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
+
+    if (iobase != isa->iobase) {
+        isa->iobase = iobase;
+        serial_isa_init_iobase(isa);
+    }
+}
+
 static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
 static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
 
@@ -776,9 +795,7 @@ static int serial_isa_initfn(ISADevice *dev)
     serial_init_core(s);
     qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
 
-    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);
+    serial_isa_init_iobase(isa);
     return 0;
 }
 
-- 
1.7.5.3

[Qemu-devel] [RFC 06/10] serial: Allow to reconfigure ISA IRQ

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa.h    |    1 +
 hw/serial.c |   11 +++++++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/isa.h b/hw/isa.h
index 8bd082a..b577fe3 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -47,6 +47,7 @@ void parallel_isa_reconfigure_irq(ISADevice *dev, uint32_t isairq);
 
 /* serial.c */
 void serial_isa_reconfigure_iobase(ISADevice *dev, uint32_t base);
+void serial_isa_reconfigure_irq(ISADevice *dev, uint32_t isairq);
 
 /* dma.c */
 int DMA_get_channel_mode (int nchan);
diff --git a/hw/serial.c b/hw/serial.c
index dd91272..714e5b2 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -771,6 +771,17 @@ void serial_isa_reconfigure_iobase(ISADevice *dev, uint32_t iobase)
     }
 }
 
+void serial_isa_reconfigure_irq(ISADevice *dev, uint32_t isairq)
+{
+    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
+
+    if (isairq != isa->isairq) {
+        isa_discard_irq(dev, isa->isairq);
+        isa->isairq = isairq;
+        isa_init_irq(dev, &isa->state.irq, isa->isairq);
+    }
+}
+
 static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
 static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
 
-- 
1.7.5.3

[Qemu-devel] [PATCH v2 07/10] fdc: Parametrize ISA base, IRQ and DMA

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

Keep the PC values as defaults but allow to override them for PReP.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/fdc.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index edf0360..f4e3e0d 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -425,6 +425,9 @@ typedef struct FDCtrlSysBus {
 
 typedef struct FDCtrlISABus {
     ISADevice busdev;
+    uint32_t iobase;
+    uint32_t irq;
+    uint32_t dma;
     struct FDCtrl state;
     int32_t bootindexA;
     int32_t bootindexB;
@@ -1895,26 +1898,23 @@ static int isabus_fdc_init1(ISADevice *dev)
 {
     FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
     FDCtrl *fdctrl = &isa->state;
-    int iobase = 0x3f0;
-    int isairq = 6;
-    int dma_chann = 2;
     int ret;
 
-    register_ioport_read(iobase + 0x01, 5, 1,
+    register_ioport_read(isa->iobase + 0x01, 5, 1,
                          &fdctrl_read_port, fdctrl);
-    register_ioport_read(iobase + 0x07, 1, 1,
+    register_ioport_read(isa->iobase + 0x07, 1, 1,
                          &fdctrl_read_port, fdctrl);
-    register_ioport_write(iobase + 0x01, 5, 1,
+    register_ioport_write(isa->iobase + 0x01, 5, 1,
                           &fdctrl_write_port, fdctrl);
-    register_ioport_write(iobase + 0x07, 1, 1,
+    register_ioport_write(isa->iobase + 0x07, 1, 1,
                           &fdctrl_write_port, fdctrl);
-    isa_init_ioport_range(dev, iobase, 6);
-    isa_init_ioport(dev, iobase + 7);
+    isa_init_ioport_range(dev, isa->iobase, 6);
+    isa_init_ioport(dev, isa->iobase + 7);
 
-    isa_init_irq(&isa->busdev, &fdctrl->irq, isairq);
-    fdctrl->dma_chann = dma_chann;
+    isa_init_irq(&isa->busdev, &fdctrl->irq, isa->irq);
+    fdctrl->dma_chann = isa->dma;
 
-    qdev_set_legacy_instance_id(&dev->qdev, iobase, 2);
+    qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 2);
     ret = fdctrl_init_common(fdctrl);
 
     add_boot_device_path(isa->bootindexA, &dev->qdev, "/floppy@0");
@@ -1979,6 +1979,9 @@ static ISADeviceInfo isa_fdc_info = {
     .qdev.vmsd  = &vmstate_isa_fdc,
     .qdev.reset = fdctrl_external_reset_isa,
     .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", FDCtrlISABus, iobase, 0x3f0),
+        DEFINE_PROP_UINT32("irq", FDCtrlISABus, irq, 6),
+        DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
         DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].bs),
         DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].bs),
         DEFINE_PROP_INT32("bootindexA", FDCtrlISABus, bootindexA, -1),
-- 
1.7.5.3

[Qemu-devel] [RFC 08/10] fdc: Allow to reconfigure ISA I/O base

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/fdc.c |   29 ++++++++++++++++++++++++++---
 hw/fdc.h |    3 +++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index f4e3e0d..abdd806 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1894,11 +1894,10 @@ static int fdctrl_init_common(FDCtrl *fdctrl)
     return fdctrl_connect_drives(fdctrl);
 }
 
-static int isabus_fdc_init1(ISADevice *dev)
+static void isabus_fdc_init_iobase(FDCtrlISABus *isa)
 {
-    FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
+    ISADevice *dev = &isa->busdev;
     FDCtrl *fdctrl = &isa->state;
-    int ret;
 
     register_ioport_read(isa->iobase + 0x01, 5, 1,
                          &fdctrl_read_port, fdctrl);
@@ -1910,6 +1909,30 @@ static int isabus_fdc_init1(ISADevice *dev)
                           &fdctrl_write_port, fdctrl);
     isa_init_ioport_range(dev, isa->iobase, 6);
     isa_init_ioport(dev, isa->iobase + 7);
+}
+
+void fdctrl_isa_reconfigure_iobase(ISADevice *dev, uint32_t base)
+{
+    FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
+
+    if (base != isa->iobase) {
+        isa_discard_ioport_range(dev, isa->iobase + 0x07, 1);
+        isa_discard_ioport_range(dev, isa->iobase + 0x01, 5);
+        isa_unassign_ioport(isa->iobase + 7, 1);
+        isa_unassign_ioport(isa->iobase, 6);
+
+        isa->iobase = base;
+        isabus_fdc_init_iobase(isa);
+    }
+}
+
+static int isabus_fdc_init1(ISADevice *dev)
+{
+    FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
+    FDCtrl *fdctrl = &isa->state;
+    int ret;
+
+    isabus_fdc_init_iobase(isa);
 
     isa_init_irq(&isa->busdev, &fdctrl->irq, isa->irq);
     fdctrl->dma_chann = isa->dma;
diff --git a/hw/fdc.h b/hw/fdc.h
index 09f73c6..3e410e9 100644
--- a/hw/fdc.h
+++ b/hw/fdc.h
@@ -28,4 +28,7 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
                         target_phys_addr_t mmio_base, DriveInfo **fds);
 void sun4m_fdctrl_init(qemu_irq irq, target_phys_addr_t io_base,
                        DriveInfo **fds, qemu_irq *fdc_tc);
+
+void fdctrl_isa_reconfigure_iobase(ISADevice *dev, uint32_t base);
+
 #endif
-- 
1.7.5.3

[Qemu-devel] [RFC 09/10] ide: Allow to reconfigure ISA I/O base

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/ide.h          |    1 +
 hw/ide/core.c     |    8 ++++++++
 hw/ide/internal.h |    1 +
 hw/ide/isa.c      |   28 +++++++++++++++++++++++++---
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/hw/ide.h b/hw/ide.h
index 34d9394..25b46a7 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -9,6 +9,7 @@
 /* ide-isa.c */
 ISADevice *isa_ide_init(int iobase, int iobase2, int isairq,
                         DriveInfo *hd0, DriveInfo *hd1);
+void isa_ide_reconfigure_iobase(ISADevice *dev, uint32_t iobase, uint32_t iobase2);
 
 /* ide-pci.c */
 void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 45410e8..c3b82de 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1760,6 +1760,14 @@ void ide_init_ioport(IDEBus *bus, int iobase, int iobase2)
     register_ioport_read(iobase, 4, 4, ide_data_readl, bus);
 }
 
+void ide_discard_ioport(int iobase, int iobase2)
+{
+    isa_unassign_ioport(iobase, 8);
+    if (iobase2 != 0) {
+        isa_unassign_ioport(iobase2, 1);
+    }
+}
+
 static bool is_identify_set(void *opaque, int version_id)
 {
     IDEState *s = opaque;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index c2b35ec..dc0a2c9 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -564,6 +564,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
                                     DriveInfo *hd1, qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
+void ide_discard_ioport(int iobase, int iobase2);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
 void ide_dma_cb(void *opaque, int ret);
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 4ac7453..d1f81d7 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -62,15 +62,37 @@ static const VMStateDescription vmstate_ide_isa = {
     }
 };
 
+static void isa_ide_init_iobase(ISAIDEState *s)
+{
+    ide_init_ioport(&s->bus, s->iobase, s->iobase2);
+
+    isa_init_ioport_range(&s->dev, s->iobase, 8);
+    isa_init_ioport(&s->dev, s->iobase2);
+}
+
+void isa_ide_reconfigure_iobase(ISADevice *dev, uint32_t iobase, uint32_t iobase2)
+{
+    ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
+
+    if (iobase != s->iobase || iobase2 != s->iobase2) {
+        ide_discard_ioport(s->iobase, s->iobase2);
+
+        isa_discard_ioport_range(dev, s->iobase2, 1);
+        isa_discard_ioport_range(dev, s->iobase, 8);
+
+        s->iobase = iobase;
+        s->iobase2 = iobase2;
+        isa_ide_init_iobase(s);
+    }
+}
+
 static int isa_ide_initfn(ISADevice *dev)
 {
     ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
 
     ide_bus_new(&s->bus, &s->dev.qdev, 0);
-    ide_init_ioport(&s->bus, s->iobase, s->iobase2);
+    isa_ide_init_iobase(s);
     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;
-- 
1.7.5.3

[Qemu-devel] [RFC 10/10] prep: Add pc87312 Super I/O emulation

[Andreas Färber]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Avoid unplugging ISA devices (not hotpluggable).

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 Makefile.objs                   |    1 +
 default-configs/ppc-softmmu.mak |    2 +
 hw/pc87312.c                    |  435 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 438 insertions(+), 0 deletions(-)
 create mode 100644 hw/pc87312.c

diff --git a/Makefile.objs b/Makefile.objs
index 90838f6..4729063 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -206,6 +206,7 @@ hw-obj-$(CONFIG_SMARTCARD_NSS) += ccid-card-emulated.o
 # PPC devices
 hw-obj-$(CONFIG_OPENPIC) += openpic.o
 hw-obj-$(CONFIG_PREP_PCI) += prep_pci.o
+hw-obj-$(CONFIG_PC87312) += pc87312.o
 # Mac shared devices
 hw-obj-$(CONFIG_MACIO) += macio.o
 hw-obj-$(CONFIG_CUDA) += cuda.o
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 4563742..4b3ebec 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -7,12 +7,14 @@ CONFIG_ESCC=y
 CONFIG_M48T59=y
 CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
+CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
 CONFIG_FDC=y
 CONFIG_DMA=y
 CONFIG_OPENPIC=y
 CONFIG_PREP_PCI=y
+CONFIG_PC87312=y
 CONFIG_MACIO=y
 CONFIG_CUDA=y
 CONFIG_ADB=y
diff --git a/hw/pc87312.c b/hw/pc87312.c
new file mode 100644
index 0000000..5147619
--- /dev/null
+++ b/hw/pc87312.c
@@ -0,0 +1,435 @@
+/*
+ * QEMU National Semiconductor PC87312 (Super I/O)
+ *
+ * Copyright (c) 2010 Herve Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "isa.h"
+#include "fdc.h"
+#include "ide.h"
+
+//#define DEBUG_PC87312
+
+#ifdef DEBUG_PC87312
+#define DPRINTF(fmt, ...) \
+do { fprintf(stderr, "pc87312: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do {} while (0)
+#endif
+
+#define BADF(fmt, ...) \
+do { fprintf(stderr, "pc87312 ERROR: " fmt , ## __VA_ARGS__); } while (0)
+
+#define REG_FER 0
+#define REG_FAR 1
+#define REG_PTR 2
+
+#define FER regs[REG_FER]
+#define FAR regs[REG_FAR]
+#define PTR regs[REG_PTR]
+
+#define FER_PARALLEL_EN   0x01
+#define FER_UART1_EN      0x02
+#define FER_UART2_EN      0x04
+#define FER_FDC_EN        0x08
+#define FER_FDC_4         0x10
+#define FER_FDC_ADDR      0x20
+#define FER_IDE_EN        0x40
+#define FER_IDE_ADDR      0x80
+
+#define FAR_PARALLEL_ADDR 0x03
+#define FAR_UART1_ADDR    0x0C
+#define FAR_UART2_ADDR    0x30
+#define FAR_UART_3_4      0xC0
+
+#define PTR_POWER_DOWN    0x01
+#define PTR_CLOCK_DOWN    0x02
+#define PTR_PWDN          0x04
+#define PTR_IRQ_5_7       0x08
+#define PTR_UART1_TEST    0x10
+#define PTR_UART2_TEST    0x20
+#define PTR_LOCK_CONF     0x40
+#define PTR_EPP_MODE      0x80
+
+typedef struct PC87312State {
+    uint8_t config; /* initial configuration */
+
+    struct {
+        DeviceState *dev;
+        CharDriverState *chr;
+    } parallel;
+
+    struct {
+        DeviceState *dev;
+        CharDriverState *chr;
+    } uart[2];
+
+    struct {
+        DeviceState *dev;
+        BlockDriverState *drive[2];
+        uint32_t base;
+    } fdc;
+
+    struct {
+        DeviceState *dev;
+        uint32_t base;
+    } ide;
+
+    uint8_t read_id_step;
+    uint8_t selected_index;
+
+    uint8_t regs[3];
+} PC87312State;
+
+static inline bool is_parallel_enabled(PC87312State *s)
+{
+    return s->FER & FER_PARALLEL_EN;
+}
+
+static inline uint32_t get_parallel_iobase(PC87312State *s)
+{
+    static const uint32_t parallel_base[] = { 0x378, 0x3bc, 0x278, 0x00 };
+
+    return parallel_base[s->FAR & FAR_PARALLEL_ADDR];
+}
+
+static inline uint32_t get_parallel_irq(PC87312State *s)
+{
+    static const uint32_t parallel_irq[] = { 5, 7, 5, 0 };
+
+    int idx;
+    idx = (s->FAR & FAR_PARALLEL_ADDR);
+    if (idx == 0) {
+        return (s->PTR & PTR_IRQ_5_7) ? 7 : 5;
+    } else {
+        return parallel_irq[idx];
+    }
+}
+
+static inline bool is_parallel_epp(PC87312State *s)
+{
+    return (s->PTR & PTR_EPP_MODE);
+}
+
+/* Parallel port */
+static void update_parallel(PC87312State *s)
+{
+    ISADevice *isa;
+
+    if (s->parallel.dev == NULL) {
+        if (is_parallel_enabled(s) && s->parallel.chr != NULL) {
+            isa = isa_create("isa-parallel");
+            qdev_prop_set_uint32(&isa->qdev, "index", 0);
+            qdev_prop_set_uint32(&isa->qdev, "iobase", get_parallel_iobase(s));
+            qdev_prop_set_uint32(&isa->qdev, "irq", get_parallel_irq(s));
+            qdev_prop_set_chr(&isa->qdev, "chardev", s->parallel.chr);
+            qdev_init_nofail(&isa->qdev);
+            s->parallel.dev = &isa->qdev;
+        }
+    } else {
+        isa = DO_UPCAST(ISADevice, qdev, s->parallel.dev);
+        parallel_isa_reconfigure_iobase(isa, get_parallel_iobase(s));
+        parallel_isa_reconfigure_irq(isa, get_parallel_irq(s));
+        if (!is_parallel_enabled(s)) {
+            /* TODO can't deactivate qdev */
+        }
+    }
+}
+
+static const uint32_t uart_base[2][4] = {
+    { 0x3e8, 0x338, 0x2e8, 0x220 },
+    { 0x2e8, 0x238, 0x2e0, 0x228 }
+};
+
+static inline uint32_t get_uart_iobase(PC87312State *s, int i)
+{
+    int idx;
+    idx = (s->FAR >> (2 * i + 2)) & 0x3;
+    if (idx == 0) {
+        return 0x3f8;
+    } else if (idx == 1) {
+        return 0x2f8;
+    } else {
+        return uart_base[idx & 1][(s->FAR & FAR_UART_3_4) >> 6];
+    }
+}
+
+static inline uint32_t get_uart_irq(PC87312State *s, int i)
+{
+    int idx;
+    idx = (s->FAR >> (2 * i + 2)) & 0x3;
+    return (idx & 1) ? 3 : 4;
+}
+
+static inline bool is_uart_enabled(PC87312State *s, int i)
+{
+    return s->FER & (FER_UART1_EN << i);
+}
+
+/* UARTs */
+static void update_uarts(PC87312State *s)
+{
+    ISADevice *isa;
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        if (s->uart[i].dev == NULL) {
+            if (is_uart_enabled(s, i) && s->uart[i].chr != NULL) {
+                isa = isa_create("isa-serial");
+                qdev_prop_set_uint32(&isa->qdev, "index", i);
+                qdev_prop_set_uint32(&isa->qdev, "iobase", get_uart_iobase(s, i));
+                qdev_prop_set_uint32(&isa->qdev, "irq", get_uart_irq(s, i));
+                qdev_prop_set_chr(&isa->qdev, "chardev", s->uart[i].chr);
+                qdev_init_nofail(&isa->qdev);
+                s->uart[i].dev = &isa->qdev;
+            }
+        } else {
+            isa = DO_UPCAST(ISADevice, qdev, s->parallel.dev);
+            serial_isa_reconfigure_iobase(isa, get_uart_iobase(s, i));
+            serial_isa_reconfigure_irq(isa, get_uart_irq(s, i));
+            if (!is_uart_enabled(s, i)) {
+                // TODO can't deactivate
+            }
+        }
+    }
+}
+
+
+/* Floppy controller */
+
+static inline bool is_fdc_enabled(PC87312State *s)
+{
+    return (s->FER & FER_FDC_EN);
+}
+
+static inline uint32_t get_fdc_iobase(PC87312State *s)
+{
+    return (s->FER & FER_FDC_ADDR) ? 0x370 : 0x3f0;
+}
+
+static void update_fdc(PC87312State *s)
+{
+    ISADevice *isa;
+
+    if (s->fdc.dev == NULL) {
+        if (is_fdc_enabled(s)) {
+            isa = isa_create("isa-fdc");
+            qdev_prop_set_uint32(&isa->qdev, "iobase", get_fdc_iobase(s));
+            qdev_prop_set_uint32(&isa->qdev, "irq", 6);
+            if (s->fdc.drive[0] != NULL) {
+                qdev_prop_set_drive_nofail(&isa->qdev, "driveA", s->fdc.drive[0]);
+            }
+            if (s->fdc.drive[1] != NULL) {
+                qdev_prop_set_drive_nofail(&isa->qdev, "driveB", s->fdc.drive[1]);
+            }
+            qdev_init_nofail(&isa->qdev);
+            s->fdc.dev = &isa->qdev;
+        }
+    } else {
+        isa = DO_UPCAST(ISADevice, qdev, s->fdc.dev);
+        fdctrl_isa_reconfigure_iobase(isa, get_fdc_iobase(s));
+        if (!is_fdc_enabled(s)) {
+            // TODO can't deactivate
+        }
+    }
+}
+
+
+/* IDE controller */
+
+static inline bool is_ide_enabled(PC87312State *s)
+{
+    return (s->FER & FER_IDE_EN);
+}
+
+static inline uint32_t get_ide_iobase(PC87312State *s)
+{
+    return (s->FER & FER_IDE_ADDR) ? 0x170 : 0x1f0;
+}
+
+static void update_ide(PC87312State *s)
+{
+    ISADevice *isa;
+
+    if (s->ide.dev == NULL) {
+        if (is_ide_enabled(s)) {
+            isa = isa_create("isa-ide");
+            qdev_prop_set_uint32(&isa->qdev, "iobase", get_ide_iobase(s));
+            qdev_prop_set_uint32(&isa->qdev, "iobase2", get_ide_iobase(s) + 0x206);
+            qdev_prop_set_uint32(&isa->qdev, "irq", 14);
+            qdev_init_nofail(&isa->qdev);
+            s->ide.dev = &isa->qdev;
+        }
+    } else {
+        isa = DO_UPCAST(ISADevice, qdev, s->ide.dev);
+        isa_ide_reconfigure_iobase(isa, get_ide_iobase(s),
+                                        get_ide_iobase(s) + 0x206);
+        if (!is_ide_enabled(s)) {
+            // TODO can't deactivate
+        }
+    }
+}
+
+
+static void update_mappings(PC87312State *s)
+{
+    update_parallel(s);
+    update_uarts(s);
+    update_fdc(s);
+    update_ide(s);
+}
+
+static void pc87312_soft_reset(PC87312State *s)
+{
+    static const uint8_t fer_init[] = {
+        0x4f, 0x4f, 0x4f, 0x4f, 0x4f, 0x4f, 0x4b, 0x4b,
+        0x4b, 0x4b, 0x4b, 0x4b, 0x0f, 0x0f, 0x0f, 0x0f,
+        0x49, 0x49, 0x49, 0x49, 0x07, 0x07, 0x07, 0x07,
+        0x47, 0x47, 0x47, 0x47, 0x47, 0x47, 0x08, 0x00,
+    };
+    static const uint8_t far_init[] = {
+        0x10, 0x11, 0x11, 0x39, 0x24, 0x38, 0x00, 0x01,
+        0x01, 0x09, 0x08, 0x08, 0x10, 0x11, 0x39, 0x24,
+        0x00, 0x01, 0x01, 0x00, 0x10, 0x11, 0x39, 0x24,
+        0x10, 0x11, 0x11, 0x39, 0x24, 0x38, 0x10, 0x10,
+    };
+    static const uint8_t ptr_init[] = {
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
+    };
+
+    s->read_id_step = 0;
+    s->selected_index = REG_FER;
+
+    s->FER = fer_init[s->config & 0x1f];
+    s->FAR = far_init[s->config & 0x1f];
+    s->PTR = ptr_init[s->config & 0x1f];
+}
+
+static void pc87312_hard_reset(PC87312State *s)
+{
+    pc87312_soft_reset(s);
+}
+
+static void pc87312_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+{
+    PC87312State *s = opaque;
+
+    DPRINTF("%s: write %x at %x\n", __func__, val, addr);
+
+    if ((addr & 1) == 0) {
+        /* Index register */
+        s->read_id_step = 2;
+        s->selected_index = val;
+    } else {
+        /* Data register */
+        if (s->selected_index < 3) {
+            s->regs[s->selected_index] = val;
+            update_mappings(s);
+        }
+    }
+}
+
+static uint32_t pc87312_ioport_read(void *opaque, uint32_t addr)
+{
+    PC87312State *s = opaque;
+    uint32_t val;
+
+    if ((addr & 1) == 0) {
+        /* Index register */
+        if (s->read_id_step++ == 0) {
+            val = 0x88;
+        } else if (s->read_id_step++ == 1) {
+            val = 0;
+        } else {
+            val = s->selected_index;
+        }
+    } else {
+        /* Data register */
+        if (s->selected_index < 3) {
+            val = s->regs[s->selected_index];
+        } else {
+            /* Invalid selected index */
+            val = 0;
+        }
+    }
+
+    DPRINTF("%s: read %x at %x\n", __func__, val, addr);
+    return val;
+}
+
+static void pc87312_init_core(PC87312State *s)
+{
+    pc87312_hard_reset(s);
+
+    update_mappings(s);
+}
+
+typedef struct ISAPC87312State {
+    ISADevice dev;
+    uint32_t iobase;
+    PC87312State state;
+} ISAPC87312State;
+
+static void isa_pc87312_reset(DeviceState *d)
+{
+    PC87312State *s = &container_of(d, ISAPC87312State, dev.qdev)->state;
+    pc87312_soft_reset(s);
+}
+
+static int isa_pc87312_init(ISADevice *dev)
+{
+    ISAPC87312State *isa = DO_UPCAST(ISAPC87312State, dev, dev);
+    PC87312State *s = &isa->state;
+
+    pc87312_init_core(s);
+
+    register_ioport_write(isa->iobase, 2, 1, pc87312_ioport_write, s);
+    register_ioport_read(isa->iobase, 2, 1, pc87312_ioport_read, s);
+    return 0;
+}
+
+static ISADeviceInfo pc87312_isa_info = {
+    .qdev.name = "isa-pc87312",
+    .qdev.size = sizeof(ISAPC87312State),
+    .qdev.reset = isa_pc87312_reset,
+    .init = isa_pc87312_init,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", ISAPC87312State, iobase, 0x398),
+        DEFINE_PROP_UINT8("config", ISAPC87312State, state.config, 1),
+        DEFINE_PROP_CHR("parallel", ISAPC87312State, state.parallel.chr),
+        DEFINE_PROP_CHR("uart1", ISAPC87312State, state.uart[0].chr),
+        DEFINE_PROP_CHR("uart2", ISAPC87312State, state.uart[1].chr),
+        DEFINE_PROP_DRIVE("floppyA", ISAPC87312State, state.fdc.drive[0]),
+        DEFINE_PROP_DRIVE("floppyB", ISAPC87312State, state.fdc.drive[1]),
+        DEFINE_PROP_END_OF_LIST()
+    },
+};
+
+static void pc87312_register_devices(void)
+{
+    isa_qdev_register(&pc87312_isa_info);
+}
+
+device_init(pc87312_register_devices)
-- 
1.7.5.3

Re: [Qemu-devel] [RFC 05/10] serial: Allow to reconfigure ISA I/O base

[Richard Henderson]

On 06/06/2011 11:20 AM, Andreas Färber wrote:
> +    if (iobase != isa->iobase) {
> +        isa->iobase = iobase;
> +        serial_isa_init_iobase(isa);

Forgot to unregister here?


r~

Re: [Qemu-devel] [RFC 05/10] serial: Allow to reconfigure ISA I/O base

[Andreas Färber]

Am 06.06.2011 um 22:08 schrieb Richard Henderson:

> On 06/06/2011 11:20 AM, Andreas Färber wrote:
>> +    if (iobase != isa->iobase) {
>> +        isa->iobase = iobase;
>> +        serial_isa_init_iobase(isa);
>
> Forgot to unregister here?

Yeah, looks like it. Thanks.

Probably some stylistic issues, too. Just an RFC. :)

Andreas

Re: [Qemu-devel] [RFC 00/10] ISA reconfigurability

[Gerd Hoffmann]

   Hi,

> This series proposes a new solution: helpers that reconfigure existing ISADevices.
>
> It also contains a resend of a prerequisite patch with updated summary.
>
> As for the remaining issue of enabling/disabling devices I'd like to hear
> opinions on whether we should allow hot-plugging of these devices or
> whether we should add an enabled/disabled state to a qdev device.

I'd tend to think having a enable/disable state (and a setstate callback 
in ISADeviceInfo) is easier to handle, especially wrt the host-side 
state of the device (i.e. the chardev).

Reconfiguration can be done this way then:

setstate(0)           ->  device unregisters irq + ports.
qdev_prop_set_*()     ->  updates iobase + irqs
setstate(1)           ->  device registers new irqs + ports.

cheers,
   Gerd

[Qemu-devel] [RFC v2 00/10] ISA reconfigurability v2

[Andreas Färber]

Hi,

As suggested by Gerd, I've introduced a set_state() callback at ISA level
and implemented it as required for pc87312.

This approach simplifies some things but currently has the drawback that the
devices are disabled and potentially re-enabled for each register write.
This is because in the previous patch I had tried to keep knowledge
about irq / iobase to the respective ISA qdev device, whereas now we
always disable it and set via unintelligent qdev property setters.

Seems to call for qdev_prop_get_uint32() to check for changes...

Andreas


Andreas Färber (9):
  isa: Provide set_state callback
  isa: Allow to un-assign I/O ports
  isa: Allow to un-associate an IRQ
  parallel: Implement set_state callback
  serial: Implement ISA set_state() callback
  fdc: Implement ISA set_state() callback
  ide: Allow to discard I/O ports
  ide: Implement ISA set_state() callback
  prep: Add pc87312 Super I/O emulation

Hervé Poussineau (1):
  fdc: Parametrize ISA base, IRQ and DMA

 Makefile.objs                   |    1 +
 default-configs/ppc-softmmu.mak |    2 +
 hw/fdc.c                        |   57 ++++--
 hw/ide/core.c                   |    8 +
 hw/ide/internal.h               |    1 +
 hw/ide/isa.c                    |   27 ++-
 hw/isa-bus.c                    |   37 ++++
 hw/isa.h                        |    5 +
 hw/parallel.c                   |   67 ++++--
 hw/pc87312.c                    |  438 +++++++++++++++++++++++++++++++++++++++
 hw/serial.c                     |   25 ++-
 11 files changed, 620 insertions(+), 48 deletions(-)
 create mode 100644 hw/pc87312.c

-- 
1.7.5.3

[Qemu-devel] [RFC v2 01/10] isa: Provide set_state callback

[Andreas Färber]

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa-bus.c |    9 +++++++++
 hw/isa.h     |    3 +++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 2765543..6f296bd 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -156,6 +156,15 @@ ISADevice *isa_create_simple(const char *name)
     return dev;
 }
 
+void isa_set_state(ISADevice *dev, bool enabled)
+{
+    ISADeviceInfo *info = DO_UPCAST(ISADeviceInfo, qdev, dev->qdev.info);
+
+    if (info->set_state != NULL) {
+        info->set_state(dev, enabled);
+    }
+}
+
 static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     ISADevice *d = DO_UPCAST(ISADevice, qdev, dev);
diff --git a/hw/isa.h b/hw/isa.h
index d2b6126..0e944b9 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -19,9 +19,11 @@ struct ISADevice {
 };
 
 typedef int (*isa_qdev_initfn)(ISADevice *dev);
+typedef void (*isa_qdev_statefn)(ISADevice *dev, bool enabled);
 struct ISADeviceInfo {
     DeviceInfo qdev;
     isa_qdev_initfn init;
+    isa_qdev_statefn set_state;
 };
 
 ISABus *isa_bus_new(DeviceState *dev);
@@ -34,6 +36,7 @@ void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create(const char *name);
 ISADevice *isa_try_create(const char *name);
 ISADevice *isa_create_simple(const char *name);
+void isa_set_state(ISADevice *dev, bool enabled);
 
 extern target_phys_addr_t isa_mem_base;
 
-- 
1.7.5.3

[Qemu-devel] [RFC v2 02/10] isa: Allow to un-assign I/O ports

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 6f296bd..68fd3db 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -105,6 +105,20 @@ void isa_init_ioport(ISADevice *dev, uint16_t ioport)
     isa_init_ioport_range(dev, ioport, 1);
 }
 
+void isa_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length)
+{
+    int i, j;
+    for (i = 0; i < dev->nioports; i++) {
+        if (dev->ioports[i] == start) {
+            for (j = 0; j < dev->nioports - i; j++) {
+                dev->ioports[i + j] = dev->ioports[i + length + j];
+            }
+            dev->nioports -= length;
+            break;
+        }
+    }
+}
+
 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 0e944b9..a524a56 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -32,6 +32,7 @@ qemu_irq isa_get_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_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create(const char *name);
 ISADevice *isa_try_create(const char *name);
-- 
1.7.5.3

[Qemu-devel] [RFC v2 03/10] isa: Allow to un-associate an IRQ

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 68fd3db..e10e9a6 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -80,6 +80,20 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
     dev->nirqs++;
 }
 
+void isa_discard_irq(ISADevice *dev, int isairq)
+{
+    int i, j;
+    for (i = 0; i < dev->nirqs; i++) {
+        if (dev->isairq[i] == isairq) {
+            for (j = i + 1; j < dev->nirqs; j++) {
+                dev->isairq[j - 1] = dev->isairq[j];
+            }
+            dev->nirqs--;
+            break;
+        }
+    }
+}
+
 static void isa_init_ioport_one(ISADevice *dev, uint16_t ioport)
 {
     assert(dev->nioports < ARRAY_SIZE(dev->ioports));
diff --git a/hw/isa.h b/hw/isa.h
index a524a56..6577a92 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -30,6 +30,7 @@ ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 qemu_irq isa_get_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
+void isa_discard_irq(ISADevice *dev, 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_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
-- 
1.7.5.3

[Qemu-devel] [RFC v2 04/10] parallel: Implement ISA set_state callback

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/parallel.c |   67 ++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/hw/parallel.c b/hw/parallel.c
index cc853a5..db5bb57 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -446,6 +446,49 @@ static void parallel_reset(void *opaque)
     s->last_read_offset = ~0U;
 }
 
+static void parallel_isa_statefn(ISADevice *dev, bool enabled)
+{
+    ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
+    ParallelState *s = &isa->state;
+    int base;
+
+    base = isa->iobase;
+    if (enabled) {
+        isa_init_irq(dev, &s->irq, isa->isairq);
+
+        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);
+        }
+    } else {
+        isa_discard_irq(dev, isa->isairq);
+
+        isa_discard_ioport_range(dev, base, 8);
+        isa_unassign_ioport(base, 8);
+        if (s->hw_driver) {
+            isa_discard_ioport_range(dev, base + 4, 1);
+            isa_unassign_ioport(base + 4, 1);
+            isa_discard_ioport_range(dev, base + 0x400, 8);
+            isa_unassign_ioport(base + 0x400, 8);
+        }
+    }
+}
+
 static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
 
 static int parallel_isa_initfn(ISADevice *dev)
@@ -453,7 +496,6 @@ static int parallel_isa_initfn(ISADevice *dev)
     static int index;
     ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
     ParallelState *s = &isa->state;
-    int base;
     uint8_t dummy;
 
     if (!s->chr) {
@@ -469,8 +511,6 @@ static int parallel_isa_initfn(ISADevice *dev)
         isa->iobase = isa_parallel_io[isa->index];
     index++;
 
-    base = isa->iobase;
-    isa_init_irq(dev, &s->irq, isa->isairq);
     qemu_register_reset(parallel_reset, s);
 
     if (qemu_chr_ioctl(s->chr, CHR_IOCTL_PP_READ_STATUS, &dummy) == 0) {
@@ -478,25 +518,7 @@ static int parallel_isa_initfn(ISADevice *dev)
         s->status = dummy;
     }
 
-    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);
-    }
+    parallel_isa_statefn(dev, true);
     return 0;
 }
 
@@ -581,6 +603,7 @@ static ISADeviceInfo parallel_isa_info = {
     .qdev.name  = "isa-parallel",
     .qdev.size  = sizeof(ISAParallelState),
     .init       = parallel_isa_initfn,
+    .set_state  = parallel_isa_statefn,
     .qdev.props = (Property[]) {
         DEFINE_PROP_UINT32("index", ISAParallelState, index,   -1),
         DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  -1),
-- 
1.7.5.3

[Qemu-devel] [RFC v2 05/10] serial: Implement ISA set_state() callback

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/serial.c |   25 +++++++++++++++++++++----
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/hw/serial.c b/hw/serial.c
index 0ee61dd..ec1e0a0 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -752,6 +752,25 @@ void serial_set_frequency(SerialState *s, uint32_t frequency)
     serial_update_parameters(s);
 }
 
+static void serial_isa_statefn(ISADevice *dev, bool enabled)
+{
+    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
+    SerialState *s = &isa->state;
+
+    if (enabled) {
+        isa_init_irq(dev, &s->irq, isa->isairq);
+
+        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);
+    } else {
+        isa_discard_irq(dev, isa->isairq);
+
+        isa_discard_ioport_range(dev, isa->iobase, 8);
+        isa_unassign_ioport(isa->iobase, 8);
+    }
+}
+
 static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
 static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
 
@@ -772,13 +791,10 @@ static int serial_isa_initfn(ISADevice *dev)
     index++;
 
     s->baudbase = 115200;
-    isa_init_irq(dev, &s->irq, isa->isairq);
     serial_init_core(s);
     qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
 
-    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);
+    serial_isa_statefn(dev, true);
     return 0;
 }
 
@@ -962,6 +978,7 @@ static ISADeviceInfo serial_isa_info = {
     .qdev.size  = sizeof(ISASerialState),
     .qdev.vmsd  = &vmstate_isa_serial,
     .init       = serial_isa_initfn,
+    .set_state  = serial_isa_statefn,
     .qdev.props = (Property[]) {
         DEFINE_PROP_UINT32("index", ISASerialState, index,   -1),
         DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
-- 
1.7.5.3

[Qemu-devel] [PATCH v2 06/10] fdc: Parametrize ISA base, IRQ and DMA

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

Keep the PC values as defaults but allow to override them for PReP.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/fdc.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index edf0360..f4e3e0d 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -425,6 +425,9 @@ typedef struct FDCtrlSysBus {
 
 typedef struct FDCtrlISABus {
     ISADevice busdev;
+    uint32_t iobase;
+    uint32_t irq;
+    uint32_t dma;
     struct FDCtrl state;
     int32_t bootindexA;
     int32_t bootindexB;
@@ -1895,26 +1898,23 @@ static int isabus_fdc_init1(ISADevice *dev)
 {
     FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
     FDCtrl *fdctrl = &isa->state;
-    int iobase = 0x3f0;
-    int isairq = 6;
-    int dma_chann = 2;
     int ret;
 
-    register_ioport_read(iobase + 0x01, 5, 1,
+    register_ioport_read(isa->iobase + 0x01, 5, 1,
                          &fdctrl_read_port, fdctrl);
-    register_ioport_read(iobase + 0x07, 1, 1,
+    register_ioport_read(isa->iobase + 0x07, 1, 1,
                          &fdctrl_read_port, fdctrl);
-    register_ioport_write(iobase + 0x01, 5, 1,
+    register_ioport_write(isa->iobase + 0x01, 5, 1,
                           &fdctrl_write_port, fdctrl);
-    register_ioport_write(iobase + 0x07, 1, 1,
+    register_ioport_write(isa->iobase + 0x07, 1, 1,
                           &fdctrl_write_port, fdctrl);
-    isa_init_ioport_range(dev, iobase, 6);
-    isa_init_ioport(dev, iobase + 7);
+    isa_init_ioport_range(dev, isa->iobase, 6);
+    isa_init_ioport(dev, isa->iobase + 7);
 
-    isa_init_irq(&isa->busdev, &fdctrl->irq, isairq);
-    fdctrl->dma_chann = dma_chann;
+    isa_init_irq(&isa->busdev, &fdctrl->irq, isa->irq);
+    fdctrl->dma_chann = isa->dma;
 
-    qdev_set_legacy_instance_id(&dev->qdev, iobase, 2);
+    qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 2);
     ret = fdctrl_init_common(fdctrl);
 
     add_boot_device_path(isa->bootindexA, &dev->qdev, "/floppy@0");
@@ -1979,6 +1979,9 @@ static ISADeviceInfo isa_fdc_info = {
     .qdev.vmsd  = &vmstate_isa_fdc,
     .qdev.reset = fdctrl_external_reset_isa,
     .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", FDCtrlISABus, iobase, 0x3f0),
+        DEFINE_PROP_UINT32("irq", FDCtrlISABus, irq, 6),
+        DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
         DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].bs),
         DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].bs),
         DEFINE_PROP_INT32("bootindexA", FDCtrlISABus, bootindexA, -1),
-- 
1.7.5.3

[Qemu-devel] [RFC v2 07/10] fdc: Implement ISA set_state() callback

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/fdc.c |   42 ++++++++++++++++++++++++++++++------------
 1 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index f4e3e0d..6d8914f 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1894,25 +1894,42 @@ static int fdctrl_init_common(FDCtrl *fdctrl)
     return fdctrl_connect_drives(fdctrl);
 }
 
+static void isabus_fdc_set_state(ISADevice *dev, bool enabled)
+{
+    FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
+    FDCtrl *fdctrl = &isa->state;
+
+    if (enabled) {
+        register_ioport_read(isa->iobase + 0x01, 5, 1,
+                             &fdctrl_read_port, fdctrl);
+        register_ioport_read(isa->iobase + 0x07, 1, 1,
+                             &fdctrl_read_port, fdctrl);
+        register_ioport_write(isa->iobase + 0x01, 5, 1,
+                              &fdctrl_write_port, fdctrl);
+        register_ioport_write(isa->iobase + 0x07, 1, 1,
+                              &fdctrl_write_port, fdctrl);
+        isa_init_ioport_range(dev, isa->iobase, 6);
+        isa_init_ioport(dev, isa->iobase + 7);
+
+        isa_init_irq(&isa->busdev, &fdctrl->irq, isa->irq);
+    } else {
+        isa_discard_ioport_range(dev, isa->iobase + 0x07, 1);
+        isa_discard_ioport_range(dev, isa->iobase + 0x01, 5);
+        isa_unassign_ioport(isa->iobase + 7, 1);
+        isa_unassign_ioport(isa->iobase, 6);
+        
+        isa_discard_irq(&isa->busdev, isa->irq);
+    }
+}
+
 static int isabus_fdc_init1(ISADevice *dev)
 {
     FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
     FDCtrl *fdctrl = &isa->state;
     int ret;
 
-    register_ioport_read(isa->iobase + 0x01, 5, 1,
-                         &fdctrl_read_port, fdctrl);
-    register_ioport_read(isa->iobase + 0x07, 1, 1,
-                         &fdctrl_read_port, fdctrl);
-    register_ioport_write(isa->iobase + 0x01, 5, 1,
-                          &fdctrl_write_port, fdctrl);
-    register_ioport_write(isa->iobase + 0x07, 1, 1,
-                          &fdctrl_write_port, fdctrl);
-    isa_init_ioport_range(dev, isa->iobase, 6);
-    isa_init_ioport(dev, isa->iobase + 7);
-
-    isa_init_irq(&isa->busdev, &fdctrl->irq, isa->irq);
     fdctrl->dma_chann = isa->dma;
+    isabus_fdc_set_state(dev, true);
 
     qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 2);
     ret = fdctrl_init_common(fdctrl);
@@ -1972,6 +1989,7 @@ static const VMStateDescription vmstate_isa_fdc ={
 
 static ISADeviceInfo isa_fdc_info = {
     .init = isabus_fdc_init1,
+    .set_state = isabus_fdc_set_state,
     .qdev.name  = "isa-fdc",
     .qdev.fw_name  = "fdc",
     .qdev.size  = sizeof(FDCtrlISABus),
-- 
1.7.5.3

[Qemu-devel] [RFC v2 08/10] ide: Allow to discard I/O ports

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/ide/core.c     |    8 ++++++++
 hw/ide/internal.h |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 45410e8..c3b82de 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1760,6 +1760,14 @@ void ide_init_ioport(IDEBus *bus, int iobase, int iobase2)
     register_ioport_read(iobase, 4, 4, ide_data_readl, bus);
 }
 
+void ide_discard_ioport(int iobase, int iobase2)
+{
+    isa_unassign_ioport(iobase, 8);
+    if (iobase2 != 0) {
+        isa_unassign_ioport(iobase2, 1);
+    }
+}
+
 static bool is_identify_set(void *opaque, int version_id)
 {
     IDEState *s = opaque;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index c2b35ec..dc0a2c9 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -564,6 +564,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
                                     DriveInfo *hd1, qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
+void ide_discard_ioport(int iobase, int iobase2);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
 void ide_dma_cb(void *opaque, int ret);
-- 
1.7.5.3

[Qemu-devel] [RFC v2 09/10] ide: Implement ISA set_state() callback

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/ide/isa.c |   27 +++++++++++++++++++++++----
 1 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 4ac7453..e6ee263 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -62,15 +62,33 @@ static const VMStateDescription vmstate_ide_isa = {
     }
 };
 
+static void isa_ide_statefn(ISADevice *dev, bool enabled)
+{
+    ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
+
+    if (enabled) {
+        ide_init_ioport(&s->bus, s->iobase, s->iobase2);
+
+        isa_init_ioport_range(dev, s->iobase, 8);
+        isa_init_ioport(dev, s->iobase2);
+
+        isa_init_irq(dev, &s->irq, s->isairq);
+    } else {
+        ide_discard_ioport(s->iobase, s->iobase2);
+
+        isa_discard_ioport_range(dev, s->iobase2, 1);
+        isa_discard_ioport_range(dev, s->iobase, 8);
+
+        isa_discard_irq(dev, s->isairq);
+    }
+}
+
 static int isa_ide_initfn(ISADevice *dev)
 {
     ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
 
     ide_bus_new(&s->bus, &s->dev.qdev, 0);
-    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);
+    isa_ide_statefn(dev, true);
     ide_init2(&s->bus, s->irq);
     vmstate_register(&dev->qdev, 0, &vmstate_ide_isa, s);
     return 0;
@@ -102,6 +120,7 @@ static ISADeviceInfo isa_ide_info = {
     .qdev.fw_name  = "ide",
     .qdev.size  = sizeof(ISAIDEState),
     .init       = isa_ide_initfn,
+    .set_state  = isa_ide_statefn,
     .qdev.reset = isa_ide_reset,
     .qdev.props = (Property[]) {
         DEFINE_PROP_HEX32("iobase",  ISAIDEState, iobase,  0x1f0),
-- 
1.7.5.3

[Qemu-devel] [RFC v2 10/10] prep: Add pc87312 Super I/O emulation

[Andreas Färber]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Create all devices ahead of time and enable/disable instead.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 Makefile.objs                   |    1 +
 default-configs/ppc-softmmu.mak |    2 +
 hw/pc87312.c                    |  438 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 441 insertions(+), 0 deletions(-)
 create mode 100644 hw/pc87312.c

diff --git a/Makefile.objs b/Makefile.objs
index 90838f6..4729063 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -206,6 +206,7 @@ hw-obj-$(CONFIG_SMARTCARD_NSS) += ccid-card-emulated.o
 # PPC devices
 hw-obj-$(CONFIG_OPENPIC) += openpic.o
 hw-obj-$(CONFIG_PREP_PCI) += prep_pci.o
+hw-obj-$(CONFIG_PC87312) += pc87312.o
 # Mac shared devices
 hw-obj-$(CONFIG_MACIO) += macio.o
 hw-obj-$(CONFIG_CUDA) += cuda.o
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 4563742..4b3ebec 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -7,12 +7,14 @@ CONFIG_ESCC=y
 CONFIG_M48T59=y
 CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
+CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
 CONFIG_FDC=y
 CONFIG_DMA=y
 CONFIG_OPENPIC=y
 CONFIG_PREP_PCI=y
+CONFIG_PC87312=y
 CONFIG_MACIO=y
 CONFIG_CUDA=y
 CONFIG_ADB=y
diff --git a/hw/pc87312.c b/hw/pc87312.c
new file mode 100644
index 0000000..17221b8
--- /dev/null
+++ b/hw/pc87312.c
@@ -0,0 +1,438 @@
+/*
+ * QEMU National Semiconductor PC87312 (Super I/O)
+ *
+ * Copyright (c) 2010 Herve Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "isa.h"
+#include "fdc.h"
+#include "ide.h"
+
+//#define DEBUG_PC87312
+
+#ifdef DEBUG_PC87312
+#define DPRINTF(fmt, ...) \
+do { fprintf(stderr, "pc87312: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do {} while (0)
+#endif
+
+#define BADF(fmt, ...) \
+do { fprintf(stderr, "pc87312 ERROR: " fmt , ## __VA_ARGS__); } while (0)
+
+#define REG_FER 0
+#define REG_FAR 1
+#define REG_PTR 2
+
+#define FER regs[REG_FER]
+#define FAR regs[REG_FAR]
+#define PTR regs[REG_PTR]
+
+#define FER_PARALLEL_EN   0x01
+#define FER_UART1_EN      0x02
+#define FER_UART2_EN      0x04
+#define FER_FDC_EN        0x08
+#define FER_FDC_4         0x10
+#define FER_FDC_ADDR      0x20
+#define FER_IDE_EN        0x40
+#define FER_IDE_ADDR      0x80
+
+#define FAR_PARALLEL_ADDR 0x03
+#define FAR_UART1_ADDR    0x0C
+#define FAR_UART2_ADDR    0x30
+#define FAR_UART_3_4      0xC0
+
+#define PTR_POWER_DOWN    0x01
+#define PTR_CLOCK_DOWN    0x02
+#define PTR_PWDN          0x04
+#define PTR_IRQ_5_7       0x08
+#define PTR_UART1_TEST    0x10
+#define PTR_UART2_TEST    0x20
+#define PTR_LOCK_CONF     0x40
+#define PTR_EPP_MODE      0x80
+
+typedef struct PC87312State {
+    uint8_t config; /* initial configuration */
+
+    struct {
+        DeviceState *dev;
+        CharDriverState *chr;
+    } parallel;
+
+    struct {
+        DeviceState *dev;
+        CharDriverState *chr;
+    } uart[2];
+
+    struct {
+        DeviceState *dev;
+        BlockDriverState *drive[2];
+        uint32_t base;
+    } fdc;
+
+    struct {
+        DeviceState *dev;
+        uint32_t base;
+    } ide;
+
+    uint8_t read_id_step;
+    uint8_t selected_index;
+
+    uint8_t regs[3];
+} PC87312State;
+
+
+/* Parallel port */
+
+static inline bool is_parallel_enabled(PC87312State *s)
+{
+    return s->FER & FER_PARALLEL_EN;
+}
+
+static const uint32_t parallel_base[] = { 0x378, 0x3bc, 0x278, 0x00 };
+
+static inline uint32_t get_parallel_iobase(PC87312State *s)
+{
+    return parallel_base[s->FAR & FAR_PARALLEL_ADDR];
+}
+
+static const uint32_t parallel_irq[] = { 5, 7, 5, 0 };
+
+static inline uint32_t get_parallel_irq(PC87312State *s)
+{
+    int idx;
+    idx = (s->FAR & FAR_PARALLEL_ADDR);
+    if (idx == 0) {
+        return (s->PTR & PTR_IRQ_5_7) ? 7 : 5;
+    } else {
+        return parallel_irq[idx];
+    }
+}
+
+static inline bool is_parallel_epp(PC87312State *s)
+{
+    return (s->PTR & PTR_EPP_MODE);
+}
+
+static void update_parallel(PC87312State *s)
+{
+    ISADevice *isa;
+
+    if (s->parallel.dev != NULL) {
+        isa = DO_UPCAST(ISADevice, qdev, s->parallel.dev);
+        isa_set_state(isa, false);
+        qdev_prop_set_uint32(&isa->qdev, "iobase", get_parallel_iobase(s));
+        qdev_prop_set_uint32(&isa->qdev, "irq", get_parallel_irq(s));
+        if (is_parallel_enabled(s)) {
+            isa_set_state(isa, true);
+        }
+    }
+}
+
+
+/* UARTs */
+
+static const uint32_t uart_base[2][4] = {
+    { 0x3e8, 0x338, 0x2e8, 0x220 },
+    { 0x2e8, 0x238, 0x2e0, 0x228 }
+};
+
+static inline uint32_t get_uart_iobase(PC87312State *s, int i)
+{
+    int idx;
+    idx = (s->FAR >> (2 * i + 2)) & 0x3;
+    if (idx == 0) {
+        return 0x3f8;
+    } else if (idx == 1) {
+        return 0x2f8;
+    } else {
+        return uart_base[idx & 1][(s->FAR & FAR_UART_3_4) >> 6];
+    }
+}
+
+static inline uint32_t get_uart_irq(PC87312State *s, int i)
+{
+    int idx;
+    idx = (s->FAR >> (2 * i + 2)) & 0x3;
+    return (idx & 1) ? 3 : 4;
+}
+
+static inline bool is_uart_enabled(PC87312State *s, int i)
+{
+    return s->FER & (FER_UART1_EN << i);
+}
+
+static void update_uarts(PC87312State *s)
+{
+    ISADevice *isa;
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        if (s->uart[i].dev != NULL) {
+            isa = DO_UPCAST(ISADevice, qdev, s->parallel.dev);
+            isa_set_state(isa, false);
+            qdev_prop_set_uint32(&isa->qdev, "iobase", get_uart_iobase(s, i));
+            qdev_prop_set_uint32(&isa->qdev, "irq", get_uart_irq(s, i));
+            if (is_uart_enabled(s, i)) {
+                isa_set_state(isa, true);
+            }
+        }
+    }
+}
+
+
+/* Floppy controller */
+
+static inline bool is_fdc_enabled(PC87312State *s)
+{
+    return (s->FER & FER_FDC_EN);
+}
+
+static inline uint32_t get_fdc_iobase(PC87312State *s)
+{
+    return (s->FER & FER_FDC_ADDR) ? 0x370 : 0x3f0;
+}
+
+static void update_fdc(PC87312State *s)
+{
+    ISADevice *isa = DO_UPCAST(ISADevice, qdev, s->fdc.dev);
+
+    isa_set_state(isa, false);
+    qdev_prop_set_uint32(&isa->qdev, "iobase", get_fdc_iobase(s));
+    if (is_fdc_enabled(s)) {
+        isa_set_state(isa, true);
+    }
+}
+
+
+/* IDE controller */
+
+static inline bool is_ide_enabled(PC87312State *s)
+{
+    return (s->FER & FER_IDE_EN);
+}
+
+static inline uint32_t get_ide_iobase(PC87312State *s)
+{
+    return (s->FER & FER_IDE_ADDR) ? 0x170 : 0x1f0;
+}
+
+static void update_ide(PC87312State *s)
+{
+    ISADevice *isa = DO_UPCAST(ISADevice, qdev, s->ide.dev);
+
+    isa_set_state(isa, false);
+    qdev_prop_set_uint32(&isa->qdev, "iobase", get_ide_iobase(s));
+    qdev_prop_set_uint32(&isa->qdev, "iobase2", get_ide_iobase(s) + 0x206);
+    if (is_ide_enabled(s)) {
+        isa_set_state(isa, true);
+    }
+}
+
+
+static void update_mappings(PC87312State *s)
+{
+    update_parallel(s);
+    update_uarts(s);
+    update_fdc(s);
+    update_ide(s);
+}
+
+static void pc87312_soft_reset(PC87312State *s)
+{
+    static const uint8_t fer_init[] = {
+        0x4f, 0x4f, 0x4f, 0x4f, 0x4f, 0x4f, 0x4b, 0x4b,
+        0x4b, 0x4b, 0x4b, 0x4b, 0x0f, 0x0f, 0x0f, 0x0f,
+        0x49, 0x49, 0x49, 0x49, 0x07, 0x07, 0x07, 0x07,
+        0x47, 0x47, 0x47, 0x47, 0x47, 0x47, 0x08, 0x00,
+    };
+    static const uint8_t far_init[] = {
+        0x10, 0x11, 0x11, 0x39, 0x24, 0x38, 0x00, 0x01,
+        0x01, 0x09, 0x08, 0x08, 0x10, 0x11, 0x39, 0x24,
+        0x00, 0x01, 0x01, 0x00, 0x10, 0x11, 0x39, 0x24,
+        0x10, 0x11, 0x11, 0x39, 0x24, 0x38, 0x10, 0x10,
+    };
+    static const uint8_t ptr_init[] = {
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
+    };
+
+    s->read_id_step = 0;
+    s->selected_index = REG_FER;
+
+    s->FER = fer_init[s->config & 0x1f];
+    s->FAR = far_init[s->config & 0x1f];
+    s->PTR = ptr_init[s->config & 0x1f];
+}
+
+static void pc87312_hard_reset(PC87312State *s)
+{
+    pc87312_soft_reset(s);
+}
+
+static void pc87312_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+{
+    PC87312State *s = opaque;
+
+    DPRINTF("%s: write %x at %x\n", __func__, val, addr);
+
+    if ((addr & 1) == 0) {
+        /* Index register */
+        s->read_id_step = 2;
+        s->selected_index = val;
+    } else {
+        /* Data register */
+        if (s->selected_index < 3) {
+            s->regs[s->selected_index] = val;
+            update_mappings(s);
+        }
+    }
+}
+
+static uint32_t pc87312_ioport_read(void *opaque, uint32_t addr)
+{
+    PC87312State *s = opaque;
+    uint32_t val;
+
+    if ((addr & 1) == 0) {
+        /* Index register */
+        if (s->read_id_step++ == 0) {
+            val = 0x88;
+        } else if (s->read_id_step++ == 1) {
+            val = 0;
+        } else {
+            val = s->selected_index;
+        }
+    } else {
+        /* Data register */
+        if (s->selected_index < 3) {
+            val = s->regs[s->selected_index];
+        } else {
+            /* Invalid selected index */
+            val = 0;
+        }
+    }
+
+    DPRINTF("%s: read %x at %x\n", __func__, val, addr);
+    return val;
+}
+
+static void pc87312_init_core(PC87312State *s)
+{
+    ISADevice *isa;
+    int i;
+
+    pc87312_hard_reset(s);
+
+    if (s->parallel.chr != NULL) {
+        isa = isa_create("isa-parallel");
+        qdev_prop_set_uint32(&isa->qdev, "index", 0);
+        qdev_prop_set_uint32(&isa->qdev, "iobase", get_parallel_iobase(s));
+        qdev_prop_set_uint32(&isa->qdev, "irq", get_parallel_irq(s));
+        qdev_prop_set_chr(&isa->qdev, "chardev", s->parallel.chr);
+        qdev_init_nofail(&isa->qdev);
+        s->parallel.dev = &isa->qdev;
+    }
+
+    for (i = 0; i < 2; i++) {
+        if (s->uart[i].chr != NULL) {
+            isa = isa_create("isa-serial");
+            qdev_prop_set_uint32(&isa->qdev, "index", i);
+            qdev_prop_set_uint32(&isa->qdev, "iobase", get_uart_iobase(s, i));
+            qdev_prop_set_uint32(&isa->qdev, "irq", get_uart_irq(s, i));
+            qdev_prop_set_chr(&isa->qdev, "chardev", s->uart[i].chr);
+            qdev_init_nofail(&isa->qdev);
+            s->uart[i].dev = &isa->qdev;
+        }
+    }
+
+    isa = isa_create("isa-fdc");
+    qdev_prop_set_uint32(&isa->qdev, "iobase", get_fdc_iobase(s));
+    qdev_prop_set_uint32(&isa->qdev, "irq", 6);
+    if (s->fdc.drive[0] != NULL) {
+        qdev_prop_set_drive_nofail(&isa->qdev, "driveA", s->fdc.drive[0]);
+    }
+    if (s->fdc.drive[1] != NULL) {
+        qdev_prop_set_drive_nofail(&isa->qdev, "driveB", s->fdc.drive[1]);
+    }
+    qdev_init_nofail(&isa->qdev);
+    s->fdc.dev = &isa->qdev;
+
+    isa = isa_create("isa-ide");
+    qdev_prop_set_uint32(&isa->qdev, "iobase", get_ide_iobase(s));
+    qdev_prop_set_uint32(&isa->qdev, "iobase2", get_ide_iobase(s) + 0x206);
+    qdev_prop_set_uint32(&isa->qdev, "irq", 14);
+    qdev_init_nofail(&isa->qdev);
+    s->ide.dev = &isa->qdev;
+
+    update_mappings(s);
+}
+
+typedef struct ISAPC87312State {
+    ISADevice dev;
+    uint32_t iobase;
+    PC87312State state;
+} ISAPC87312State;
+
+static void isa_pc87312_reset(DeviceState *d)
+{
+    PC87312State *s = &container_of(d, ISAPC87312State, dev.qdev)->state;
+    pc87312_soft_reset(s);
+}
+
+static int isa_pc87312_init(ISADevice *dev)
+{
+    ISAPC87312State *isa = DO_UPCAST(ISAPC87312State, dev, dev);
+    PC87312State *s = &isa->state;
+
+    pc87312_init_core(s);
+
+    register_ioport_write(isa->iobase, 2, 1, pc87312_ioport_write, s);
+    register_ioport_read(isa->iobase, 2, 1, pc87312_ioport_read, s);
+    return 0;
+}
+
+static ISADeviceInfo pc87312_isa_info = {
+    .qdev.name = "isa-pc87312",
+    .qdev.size = sizeof(ISAPC87312State),
+    .qdev.reset = isa_pc87312_reset,
+    .init = isa_pc87312_init,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", ISAPC87312State, iobase, 0x398),
+        DEFINE_PROP_UINT8("config", ISAPC87312State, state.config, 1),
+        DEFINE_PROP_CHR("parallel", ISAPC87312State, state.parallel.chr),
+        DEFINE_PROP_CHR("uart1", ISAPC87312State, state.uart[0].chr),
+        DEFINE_PROP_CHR("uart2", ISAPC87312State, state.uart[1].chr),
+        DEFINE_PROP_DRIVE("floppyA", ISAPC87312State, state.fdc.drive[0]),
+        DEFINE_PROP_DRIVE("floppyB", ISAPC87312State, state.fdc.drive[1]),
+        DEFINE_PROP_END_OF_LIST()
+    },
+};
+
+static void pc87312_register_devices(void)
+{
+    isa_qdev_register(&pc87312_isa_info);
+}
+
+device_init(pc87312_register_devices)
-- 
1.7.5.3

Re: [Qemu-devel] [RFC v2 00/10] ISA reconfigurability v2

[Gerd Hoffmann]

On 06/07/11 17:02, Andreas Färber wrote:
> Hi,
>
> As suggested by Gerd, I've introduced a set_state() callback at ISA level
> and implemented it as required for pc87312.
>
> This approach simplifies some things but currently has the drawback that the
> devices are disabled and potentially re-enabled for each register write.

Doesn't the pc87312 keep track of the configured iobases and irqs?  So 
it should know whenever a register write changed something or not ...

cheers,
   Gerd

Re: [Qemu-devel] [RFC v2 00/10] ISA reconfigurability v2

[Andreas Färber]

Am 07.06.2011 um 17:16 schrieb Gerd Hoffmann:

> On 06/07/11 17:02, Andreas Färber wrote:
>> As suggested by Gerd, I've introduced a set_state() callback at ISA  
>> level
>> and implemented it as required for pc87312.
>>
>> This approach simplifies some things but currently has the drawback  
>> that the
>> devices are disabled and potentially re-enabled for each register  
>> write.
>
> Doesn't the pc87312 keep track of the configured iobases and irqs?   
> So it should know whenever a register write changed something or  
> not ...

The original patch submission kept copies of the values in the pc87312  
DeviceState.
I consider that redundant with the devices' DeviceState though and  
removed it with my introduction of helpers that had access to the  
devices' DeviceState in v1.
That access we have now lost in v2 (only the set_state callback has,  
but that doesn't know its former state; disabling twice is expected to  
succeed btw), so once set, the pc87312 has no clue about the current  
property values.

Just wondering, if we're allowing to change qdev properties at runtime  
here, don't these fields need to be considered for VMState? (cc'ing  
Juan)
If I understand the theory of live migration correctly, then the  
command line needs to be identical wrt devices on both machines - if  
the migration source has executed firmware code changing the iobase  
and the migration target resumes wherever the source was, they'd end  
up with different DeviceState, right? Or are qdev properties  
transmitted anyway? (not that I care for PReP but these are generic  
ISA devices)
Interestingly, I don't see any VMState in parallel.c.
serial.c, fdc.c and ide/isa.c do have it, but I don't see iobase and  
irq in there. Would having iobase and irq number defined as VMState  
make reading them any easier?

Andreas

[Qemu-devel] [RFC v3 10/11] qdev: Add helpers for reading properties

[Andreas Färber]

This effectively turns qdev properties from write-only to read/write,
allowing to inspect a private DeviceState.

As a start, add qdev_prop_get_uint32().

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/qdev-properties.c |   23 +++++++++++++++++++++++
 hw/qdev.h            |    2 ++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index eff2d24..bca3028 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -639,6 +639,24 @@ void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyT
     qdev_prop_cpy(dev, prop, src);
 }
 
+void *qdev_prop_get(DeviceState *dev, const char *name, enum PropertyType type)
+{
+    Property *prop;
+
+    prop = qdev_prop_find(dev, name);
+    if (!prop) {
+        fprintf(stderr, "%s: property \"%s.%s\" not found\n",
+                __FUNCTION__, dev->info->name, name);
+        abort();
+    }
+    if (prop->info->type != type) {
+        fprintf(stderr, "%s: property \"%s.%s\" type mismatch\n",
+                __FUNCTION__, dev->info->name, name);
+        abort();
+    }
+    return qdev_get_prop_ptr(dev, prop);
+}
+
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_BIT);
@@ -659,6 +677,11 @@ void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value)
     qdev_prop_set(dev, name, &value, PROP_TYPE_UINT32);
 }
 
+uint32_t qdev_prop_get_uint32(DeviceState *dev, const char *name)
+{
+    return *(uint32_t *)qdev_prop_get(dev, name, PROP_TYPE_UINT32);
+}
+
 void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_INT32);
diff --git a/hw/qdev.h b/hw/qdev.h
index 8a13ec9..051d03e 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -297,10 +297,12 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
 int qdev_prop_exists(DeviceState *dev, const char *name);
 int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
 void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type);
+void *qdev_prop_get(DeviceState *dev, const char *name, enum PropertyType type);
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
+uint32_t qdev_prop_get_uint32(DeviceState *dev, const char *name);
 void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value);
 void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
 void qdev_prop_set_string(DeviceState *dev, const char *name, char *value);
-- 
1.7.5.3

[Qemu-devel] [RFC v3 11/11] prep: Add pc87312 Super I/O emulation

[Andreas Färber]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Create all devices ahead of time and enable/disable instead.
Check the qdev properties for whether a change is necessary.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 Makefile.objs                   |    1 +
 default-configs/ppc-softmmu.mak |    2 +
 hw/pc87312.c                    |  462 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 465 insertions(+), 0 deletions(-)
 create mode 100644 hw/pc87312.c

diff --git a/Makefile.objs b/Makefile.objs
index 90838f6..4729063 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -206,6 +206,7 @@ hw-obj-$(CONFIG_SMARTCARD_NSS) += ccid-card-emulated.o
 # PPC devices
 hw-obj-$(CONFIG_OPENPIC) += openpic.o
 hw-obj-$(CONFIG_PREP_PCI) += prep_pci.o
+hw-obj-$(CONFIG_PC87312) += pc87312.o
 # Mac shared devices
 hw-obj-$(CONFIG_MACIO) += macio.o
 hw-obj-$(CONFIG_CUDA) += cuda.o
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 4563742..4b3ebec 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -7,12 +7,14 @@ CONFIG_ESCC=y
 CONFIG_M48T59=y
 CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
+CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
 CONFIG_FDC=y
 CONFIG_DMA=y
 CONFIG_OPENPIC=y
 CONFIG_PREP_PCI=y
+CONFIG_PC87312=y
 CONFIG_MACIO=y
 CONFIG_CUDA=y
 CONFIG_ADB=y
diff --git a/hw/pc87312.c b/hw/pc87312.c
new file mode 100644
index 0000000..7296019
--- /dev/null
+++ b/hw/pc87312.c
@@ -0,0 +1,462 @@
+/*
+ * QEMU National Semiconductor PC87312 (Super I/O)
+ *
+ * Copyright (c) 2010 Herve Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "isa.h"
+#include "fdc.h"
+#include "ide.h"
+
+//#define DEBUG_PC87312
+
+#ifdef DEBUG_PC87312
+#define DPRINTF(fmt, ...) \
+do { fprintf(stderr, "pc87312: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do {} while (0)
+#endif
+
+#define BADF(fmt, ...) \
+do { fprintf(stderr, "pc87312 ERROR: " fmt , ## __VA_ARGS__); } while (0)
+
+#define REG_FER 0
+#define REG_FAR 1
+#define REG_PTR 2
+
+#define FER regs[REG_FER]
+#define FAR regs[REG_FAR]
+#define PTR regs[REG_PTR]
+
+#define FER_PARALLEL_EN   0x01
+#define FER_UART1_EN      0x02
+#define FER_UART2_EN      0x04
+#define FER_FDC_EN        0x08
+#define FER_FDC_4         0x10
+#define FER_FDC_ADDR      0x20
+#define FER_IDE_EN        0x40
+#define FER_IDE_ADDR      0x80
+
+#define FAR_PARALLEL_ADDR 0x03
+#define FAR_UART1_ADDR    0x0C
+#define FAR_UART2_ADDR    0x30
+#define FAR_UART_3_4      0xC0
+
+#define PTR_POWER_DOWN    0x01
+#define PTR_CLOCK_DOWN    0x02
+#define PTR_PWDN          0x04
+#define PTR_IRQ_5_7       0x08
+#define PTR_UART1_TEST    0x10
+#define PTR_UART2_TEST    0x20
+#define PTR_LOCK_CONF     0x40
+#define PTR_EPP_MODE      0x80
+
+typedef struct PC87312State {
+    uint8_t config; /* initial configuration */
+
+    struct {
+        DeviceState *dev;
+        CharDriverState *chr;
+    } parallel;
+
+    struct {
+        DeviceState *dev;
+        CharDriverState *chr;
+    } uart[2];
+
+    struct {
+        DeviceState *dev;
+        BlockDriverState *drive[2];
+        uint32_t base;
+    } fdc;
+
+    struct {
+        DeviceState *dev;
+        uint32_t base;
+    } ide;
+
+    uint8_t read_id_step;
+    uint8_t selected_index;
+
+    uint8_t regs[3];
+} PC87312State;
+
+
+/* Parallel port */
+
+static inline bool is_parallel_enabled(PC87312State *s)
+{
+    return s->FER & FER_PARALLEL_EN;
+}
+
+static const uint32_t parallel_base[] = { 0x378, 0x3bc, 0x278, 0x00 };
+
+static inline uint32_t get_parallel_iobase(PC87312State *s)
+{
+    return parallel_base[s->FAR & FAR_PARALLEL_ADDR];
+}
+
+static const uint32_t parallel_irq[] = { 5, 7, 5, 0 };
+
+static inline uint32_t get_parallel_irq(PC87312State *s)
+{
+    int idx;
+    idx = (s->FAR & FAR_PARALLEL_ADDR);
+    if (idx == 0) {
+        return (s->PTR & PTR_IRQ_5_7) ? 7 : 5;
+    } else {
+        return parallel_irq[idx];
+    }
+}
+
+static inline bool is_parallel_epp(PC87312State *s)
+{
+    return (s->PTR & PTR_EPP_MODE);
+}
+
+static void update_parallel(PC87312State *s)
+{
+    ISADevice *isa;
+    uint32_t base, isairq;
+
+    if (s->parallel.dev != NULL) {
+        isa = DO_UPCAST(ISADevice, qdev, s->parallel.dev);
+        base = get_parallel_iobase(s);
+        isairq = get_parallel_irq(s);
+        // TODO check for state change
+        if (qdev_prop_get_uint32(&isa->qdev, "iobase") != base ||
+            qdev_prop_get_uint32(&isa->qdev, "irq") != isairq) {
+            isa_set_state(isa, false);
+            qdev_prop_set_uint32(&isa->qdev, "iobase", base);
+            qdev_prop_set_uint32(&isa->qdev, "irq", isairq);
+            if (is_parallel_enabled(s)) {
+                isa_set_state(isa, true);
+            }
+        }
+    }
+}
+
+
+/* UARTs */
+
+static const uint32_t uart_base[2][4] = {
+    { 0x3e8, 0x338, 0x2e8, 0x220 },
+    { 0x2e8, 0x238, 0x2e0, 0x228 }
+};
+
+static inline uint32_t get_uart_iobase(PC87312State *s, int i)
+{
+    int idx;
+    idx = (s->FAR >> (2 * i + 2)) & 0x3;
+    if (idx == 0) {
+        return 0x3f8;
+    } else if (idx == 1) {
+        return 0x2f8;
+    } else {
+        return uart_base[idx & 1][(s->FAR & FAR_UART_3_4) >> 6];
+    }
+}
+
+static inline uint32_t get_uart_irq(PC87312State *s, int i)
+{
+    int idx;
+    idx = (s->FAR >> (2 * i + 2)) & 0x3;
+    return (idx & 1) ? 3 : 4;
+}
+
+static inline bool is_uart_enabled(PC87312State *s, int i)
+{
+    return s->FER & (FER_UART1_EN << i);
+}
+
+static void update_uarts(PC87312State *s)
+{
+    ISADevice *isa;
+    uint32_t base, isairq;
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        if (s->uart[i].dev != NULL) {
+            isa = DO_UPCAST(ISADevice, qdev, s->parallel.dev);
+            base = get_uart_iobase(s, i);
+            isairq = get_uart_irq(s, i);
+            // TODO check for state change
+            if (qdev_prop_get_uint32(&isa->qdev, "iobase") != base ||
+                qdev_prop_get_uint32(&isa->qdev, "irq") != isairq) {
+                isa_set_state(isa, false);
+                qdev_prop_set_uint32(&isa->qdev, "iobase", base);
+                qdev_prop_set_uint32(&isa->qdev, "irq", isairq);
+                if (is_uart_enabled(s, i)) {
+                    isa_set_state(isa, true);
+                }
+            }
+        }
+    }
+}
+
+
+/* Floppy controller */
+
+static inline bool is_fdc_enabled(PC87312State *s)
+{
+    return (s->FER & FER_FDC_EN);
+}
+
+static inline uint32_t get_fdc_iobase(PC87312State *s)
+{
+    return (s->FER & FER_FDC_ADDR) ? 0x370 : 0x3f0;
+}
+
+static void update_fdc(PC87312State *s)
+{
+    ISADevice *isa = DO_UPCAST(ISADevice, qdev, s->fdc.dev);
+    uint32_t base;
+
+    base = get_fdc_iobase(s);
+    // TODO check for state change
+    if (qdev_prop_get_uint32(&isa->qdev, "iobase") != base) {
+        isa_set_state(isa, false);
+        qdev_prop_set_uint32(&isa->qdev, "iobase", base);
+        if (is_fdc_enabled(s)) {
+            isa_set_state(isa, true);
+        }
+    }
+}
+
+
+/* IDE controller */
+
+static inline bool is_ide_enabled(PC87312State *s)
+{
+    return (s->FER & FER_IDE_EN);
+}
+
+static inline uint32_t get_ide_iobase(PC87312State *s)
+{
+    return (s->FER & FER_IDE_ADDR) ? 0x170 : 0x1f0;
+}
+
+static void update_ide(PC87312State *s)
+{
+    ISADevice *isa = DO_UPCAST(ISADevice, qdev, s->ide.dev);
+    uint32_t base;
+
+    base = get_ide_iobase(s);
+    // TODO check for state change
+    if (qdev_prop_get_uint32(&isa->qdev, "iobase") != base) {
+        isa_set_state(isa, false);
+        qdev_prop_set_uint32(&isa->qdev, "iobase", base);
+        qdev_prop_set_uint32(&isa->qdev, "iobase2", base + 0x206);
+        if (is_ide_enabled(s)) {
+            isa_set_state(isa, true);
+        }
+    }
+}
+
+
+static void update_mappings(PC87312State *s)
+{
+    update_parallel(s);
+    update_uarts(s);
+    update_fdc(s);
+    update_ide(s);
+}
+
+static void pc87312_soft_reset(PC87312State *s)
+{
+    static const uint8_t fer_init[] = {
+        0x4f, 0x4f, 0x4f, 0x4f, 0x4f, 0x4f, 0x4b, 0x4b,
+        0x4b, 0x4b, 0x4b, 0x4b, 0x0f, 0x0f, 0x0f, 0x0f,
+        0x49, 0x49, 0x49, 0x49, 0x07, 0x07, 0x07, 0x07,
+        0x47, 0x47, 0x47, 0x47, 0x47, 0x47, 0x08, 0x00,
+    };
+    static const uint8_t far_init[] = {
+        0x10, 0x11, 0x11, 0x39, 0x24, 0x38, 0x00, 0x01,
+        0x01, 0x09, 0x08, 0x08, 0x10, 0x11, 0x39, 0x24,
+        0x00, 0x01, 0x01, 0x00, 0x10, 0x11, 0x39, 0x24,
+        0x10, 0x11, 0x11, 0x39, 0x24, 0x38, 0x10, 0x10,
+    };
+    static const uint8_t ptr_init[] = {
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
+    };
+
+    s->read_id_step = 0;
+    s->selected_index = REG_FER;
+
+    s->FER = fer_init[s->config & 0x1f];
+    s->FAR = far_init[s->config & 0x1f];
+    s->PTR = ptr_init[s->config & 0x1f];
+}
+
+static void pc87312_hard_reset(PC87312State *s)
+{
+    pc87312_soft_reset(s);
+}
+
+static void pc87312_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+{
+    PC87312State *s = opaque;
+
+    DPRINTF("%s: write %x at %x\n", __func__, val, addr);
+
+    if ((addr & 1) == 0) {
+        /* Index register */
+        s->read_id_step = 2;
+        s->selected_index = val;
+    } else {
+        /* Data register */
+        if (s->selected_index < 3) {
+            s->regs[s->selected_index] = val;
+            update_mappings(s);
+        }
+    }
+}
+
+static uint32_t pc87312_ioport_read(void *opaque, uint32_t addr)
+{
+    PC87312State *s = opaque;
+    uint32_t val;
+
+    if ((addr & 1) == 0) {
+        /* Index register */
+        if (s->read_id_step++ == 0) {
+            val = 0x88;
+        } else if (s->read_id_step++ == 1) {
+            val = 0;
+        } else {
+            val = s->selected_index;
+        }
+    } else {
+        /* Data register */
+        if (s->selected_index < 3) {
+            val = s->regs[s->selected_index];
+        } else {
+            /* Invalid selected index */
+            val = 0;
+        }
+    }
+
+    DPRINTF("%s: read %x at %x\n", __func__, val, addr);
+    return val;
+}
+
+static void pc87312_init_core(PC87312State *s)
+{
+    ISADevice *isa;
+    int i;
+
+    pc87312_hard_reset(s);
+
+    if (s->parallel.chr != NULL) {
+        isa = isa_create("isa-parallel");
+        qdev_prop_set_uint32(&isa->qdev, "index", 0);
+        qdev_prop_set_uint32(&isa->qdev, "iobase", get_parallel_iobase(s));
+        qdev_prop_set_uint32(&isa->qdev, "irq", get_parallel_irq(s));
+        qdev_prop_set_chr(&isa->qdev, "chardev", s->parallel.chr);
+        qdev_init_nofail(&isa->qdev);
+        s->parallel.dev = &isa->qdev;
+    }
+
+    for (i = 0; i < 2; i++) {
+        if (s->uart[i].chr != NULL) {
+            isa = isa_create("isa-serial");
+            qdev_prop_set_uint32(&isa->qdev, "index", i);
+            qdev_prop_set_uint32(&isa->qdev, "iobase", get_uart_iobase(s, i));
+            qdev_prop_set_uint32(&isa->qdev, "irq", get_uart_irq(s, i));
+            qdev_prop_set_chr(&isa->qdev, "chardev", s->uart[i].chr);
+            qdev_init_nofail(&isa->qdev);
+            s->uart[i].dev = &isa->qdev;
+        }
+    }
+
+    isa = isa_create("isa-fdc");
+    qdev_prop_set_uint32(&isa->qdev, "iobase", get_fdc_iobase(s));
+    qdev_prop_set_uint32(&isa->qdev, "irq", 6);
+    if (s->fdc.drive[0] != NULL) {
+        qdev_prop_set_drive_nofail(&isa->qdev, "driveA", s->fdc.drive[0]);
+    }
+    if (s->fdc.drive[1] != NULL) {
+        qdev_prop_set_drive_nofail(&isa->qdev, "driveB", s->fdc.drive[1]);
+    }
+    qdev_init_nofail(&isa->qdev);
+    s->fdc.dev = &isa->qdev;
+
+    isa = isa_create("isa-ide");
+    qdev_prop_set_uint32(&isa->qdev, "iobase", get_ide_iobase(s));
+    qdev_prop_set_uint32(&isa->qdev, "iobase2", get_ide_iobase(s) + 0x206);
+    qdev_prop_set_uint32(&isa->qdev, "irq", 14);
+    qdev_init_nofail(&isa->qdev);
+    s->ide.dev = &isa->qdev;
+
+    update_mappings(s);
+}
+
+typedef struct ISAPC87312State {
+    ISADevice dev;
+    uint32_t iobase;
+    PC87312State state;
+} ISAPC87312State;
+
+static void isa_pc87312_reset(DeviceState *d)
+{
+    PC87312State *s = &container_of(d, ISAPC87312State, dev.qdev)->state;
+    pc87312_soft_reset(s);
+}
+
+static int isa_pc87312_init(ISADevice *dev)
+{
+    ISAPC87312State *isa = DO_UPCAST(ISAPC87312State, dev, dev);
+    PC87312State *s = &isa->state;
+
+    pc87312_init_core(s);
+
+    register_ioport_write(isa->iobase, 2, 1, pc87312_ioport_write, s);
+    register_ioport_read(isa->iobase, 2, 1, pc87312_ioport_read, s);
+    return 0;
+}
+
+static ISADeviceInfo pc87312_isa_info = {
+    .qdev.name = "isa-pc87312",
+    .qdev.size = sizeof(ISAPC87312State),
+    .qdev.reset = isa_pc87312_reset,
+    .init = isa_pc87312_init,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", ISAPC87312State, iobase, 0x398),
+        DEFINE_PROP_UINT8("config", ISAPC87312State, state.config, 1),
+        DEFINE_PROP_CHR("parallel", ISAPC87312State, state.parallel.chr),
+        DEFINE_PROP_CHR("uart1", ISAPC87312State, state.uart[0].chr),
+        DEFINE_PROP_CHR("uart2", ISAPC87312State, state.uart[1].chr),
+        DEFINE_PROP_DRIVE("floppyA", ISAPC87312State, state.fdc.drive[0]),
+        DEFINE_PROP_DRIVE("floppyB", ISAPC87312State, state.fdc.drive[1]),
+        DEFINE_PROP_END_OF_LIST()
+    },
+};
+
+static void pc87312_register_devices(void)
+{
+    isa_qdev_register(&pc87312_isa_info);
+}
+
+device_init(pc87312_register_devices)
-- 
1.7.5.3

Re: [Qemu-devel] [RFC v2 00/10] ISA reconfigurability v2

[Gerd Hoffmann]

   Hi,

> Just wondering, if we're allowing to change qdev properties at runtime
> here, don't these fields need to be considered for VMState?

Yes.  Option one is to have the isa devices save them themself, option 
two is to save them as part of the pc87312 state.

cheers,
   Gerd

[Qemu-devel] [RFC v4 00/12] ISA reconfigurability v4

[Andreas Färber]

Hey,

I've refined the series to track the state in ISADevice and to expose it as VMState.
Error handling has been improved, and setting the state multiple times is no-op now.

To read the state, I'm introducing support for bool qdev properties.
Some more qdev_prop_get_*() helpers are introduced, too.

Still need to do some runtime testing, but I'd like to hear if this is getting
mergeable now, especially wrt VMState.

Andreas


Andreas Färber (11):
  qdev: Add support for property type bool
  qdev: Add helpers for reading properties
  isa: Provide set_state callback
  isa: Allow to un-assign I/O ports
  isa: Allow to un-associate an IRQ
  parallel: Implement ISA set_state callback
  serial: Implement ISA set_state() callback
  fdc: Implement ISA set_state() callback
  ide: Allow to discard I/O ports
  ide: Implement ISA set_state() callback
  prep: Add pc87312 Super I/O emulation

Hervé Poussineau (1):
  fdc: Parametrize ISA base, IRQ and DMA

 Makefile.objs                   |    1 +
 default-configs/ppc-softmmu.mak |    2 +
 hw/fdc.c                        |   62 ++++--
 hw/hw.h                         |   15 ++
 hw/ide/core.c                   |    8 +
 hw/ide/internal.h               |    1 +
 hw/ide/isa.c                    |   32 +++-
 hw/isa-bus.c                    |   57 +++++
 hw/isa.h                        |    6 +
 hw/parallel.c                   |   69 ++++--
 hw/pc87312.c                    |  470 +++++++++++++++++++++++++++++++++++++++
 hw/qdev-properties.c            |   88 ++++++++
 hw/qdev.h                       |   13 +
 hw/serial.c                     |   30 ++-
 14 files changed, 803 insertions(+), 51 deletions(-)
 create mode 100644 hw/pc87312.c

-- 
1.7.5.3

[Qemu-devel] [PATCH v4 01/12] qdev: Add support for property type bool

[Andreas Färber]

VMState supports the type bool but qdev instead supports bit, backed by
uint32_t. Therefore let's add DEFINE_PROP_BOOL() and qdev_prop_set_bool().

Since, e.g., enabled=on does not look nice, parse/print "yes" and "no".

Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/qdev-properties.c |   35 +++++++++++++++++++++++++++++++++++
 hw/qdev.h            |    5 +++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index eff2d24..e262f9a 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -63,6 +63,36 @@ PropertyInfo qdev_prop_bit = {
     .print = print_bit,
 };
 
+/* --- bool --- */
+
+static int parse_bool(DeviceState *dev, Property *prop, const char *str)
+{
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (strncasecmp(str, "yes", 3) == 0) {
+        *ptr = true;
+    } else if (strncasecmp(str, "no", 2) == 0) {
+        *ptr = false;
+    } else {
+        return -EINVAL;
+    }
+    return 0;
+}
+
+static int print_bool(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+    return snprintf(dest, len, (*ptr) ? "yes" : "no");
+}
+
+PropertyInfo qdev_prop_bool = {
+    .name = "yes/no",
+    .type = PROP_TYPE_BOOL,
+    .size = sizeof(bool),
+    .parse = parse_bool,
+    .print = print_bool,
+};
+
 /* --- 8bit integer --- */
 
 static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
@@ -644,6 +674,11 @@ void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
     qdev_prop_set(dev, name, &value, PROP_TYPE_BIT);
 }
 
+void qdev_prop_set_bool(DeviceState *dev, const char *name, bool value)
+{
+    qdev_prop_set(dev, name, &value, PROP_TYPE_BOOL);
+}
+
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_UINT8);
diff --git a/hw/qdev.h b/hw/qdev.h
index 8a13ec9..f05166d 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -101,6 +101,7 @@ enum PropertyType {
     PROP_TYPE_VLAN,
     PROP_TYPE_PTR,
     PROP_TYPE_BIT,
+    PROP_TYPE_BOOL,
 };
 
 struct PropertyInfo {
@@ -219,6 +220,7 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 /*** qdev-properties.c ***/
 
 extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_bool;
 extern PropertyInfo qdev_prop_uint8;
 extern PropertyInfo qdev_prop_uint16;
 extern PropertyInfo qdev_prop_uint32;
@@ -257,6 +259,8 @@ extern PropertyInfo qdev_prop_pci_devfn;
         .defval    = (bool[]) { (_defval) },                     \
         }
 
+#define DEFINE_PROP_BOOL(_n, _s, _f, _d)                        \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bool, bool)
 #define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
 #define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
@@ -298,6 +302,7 @@ int qdev_prop_exists(DeviceState *dev, const char *name);
 int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
 void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type);
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
+void qdev_prop_set_bool(DeviceState *dev, const char *name, bool value);
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
-- 
1.7.5.3

[Qemu-devel] [PATCH v4 02/12] qdev: Add helpers for reading properties

[Andreas Färber]

Add helpers qdev_prop_get_*() to access all integer qdev properties
as well as string properties.

This effectively turns qdev properties from write-only to read/write,
allowing to inspect a private DeviceState.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/qdev-properties.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h            |    8 +++++++
 2 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index e262f9a..0be4130 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -669,6 +669,24 @@ void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyT
     qdev_prop_cpy(dev, prop, src);
 }
 
+void *qdev_prop_get(DeviceState *dev, const char *name, enum PropertyType type)
+{
+    Property *prop;
+
+    prop = qdev_prop_find(dev, name);
+    if (!prop) {
+        fprintf(stderr, "%s: property \"%s.%s\" not found\n",
+                __FUNCTION__, dev->info->name, name);
+        abort();
+    }
+    if (prop->info->type != type) {
+        fprintf(stderr, "%s: property \"%s.%s\" type mismatch\n",
+                __FUNCTION__, dev->info->name, name);
+        abort();
+    }
+    return qdev_get_prop_ptr(dev, prop);
+}
+
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_BIT);
@@ -679,36 +697,71 @@ void qdev_prop_set_bool(DeviceState *dev, const char *name, bool value)
     qdev_prop_set(dev, name, &value, PROP_TYPE_BOOL);
 }
 
+bool qdev_prop_get_bool(DeviceState *dev, const char *name)
+{
+    return *(bool *)qdev_prop_get(dev, name, PROP_TYPE_BOOL);
+}
+
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_UINT8);
 }
 
+uint8_t qdev_prop_get_uint8(DeviceState *dev, const char *name)
+{
+    return *(uint8_t *)qdev_prop_get(dev, name, PROP_TYPE_UINT8);
+}
+
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_UINT16);
 }
 
+uint16_t qdev_prop_get_uint16(DeviceState *dev, const char *name)
+{
+    return *(uint16_t *)qdev_prop_get(dev, name, PROP_TYPE_UINT16);
+}
+
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_UINT32);
 }
 
+uint32_t qdev_prop_get_uint32(DeviceState *dev, const char *name)
+{
+    return *(uint32_t *)qdev_prop_get(dev, name, PROP_TYPE_UINT32);
+}
+
 void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_INT32);
 }
 
+int32_t qdev_prop_get_int32(DeviceState *dev, const char *name)
+{
+    return *(int32_t *)qdev_prop_get(dev, name, PROP_TYPE_INT32);
+}
+
 void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_UINT64);
 }
 
+uint64_t qdev_prop_get_uint64(DeviceState *dev, const char *name)
+{
+    return *(uint64_t *)qdev_prop_get(dev, name, PROP_TYPE_UINT64);
+}
+
 void qdev_prop_set_string(DeviceState *dev, const char *name, char *value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_STRING);
 }
 
+char *qdev_prop_get_string(DeviceState *dev, const char *name)
+{
+    return (char *)qdev_prop_get(dev, name, PROP_TYPE_STRING);
+}
+
 int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value)
 {
     int res;
diff --git a/hw/qdev.h b/hw/qdev.h
index f05166d..71bd230 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -301,14 +301,22 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
 int qdev_prop_exists(DeviceState *dev, const char *name);
 int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
 void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type);
+void *qdev_prop_get(DeviceState *dev, const char *name, enum PropertyType type);
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
 void qdev_prop_set_bool(DeviceState *dev, const char *name, bool value);
+bool qdev_prop_get_bool(DeviceState *dev, const char *name);
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
+uint8_t qdev_prop_get_uint8(DeviceState *dev, const char *name);
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
+uint16_t qdev_prop_get_uint16(DeviceState *dev, const char *name);
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
+uint32_t qdev_prop_get_uint32(DeviceState *dev, const char *name);
 void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value);
+int32_t qdev_prop_get_int32(DeviceState *dev, const char *name);
 void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
+uint64_t qdev_prop_get_uint64(DeviceState *dev, const char *name);
 void qdev_prop_set_string(DeviceState *dev, const char *name, char *value);
+char *qdev_prop_get_string(DeviceState *dev, const char *name);
 void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value);
 void qdev_prop_set_netdev(DeviceState *dev, const char *name, VLANClientState *value);
 void qdev_prop_set_vlan(DeviceState *dev, const char *name, VLANState *value);
-- 
1.7.5.3

[Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Andreas Färber]

To allow enabling/disabling present ISA devices without hotplug,
keep track of state and add a helper to avoid enabling twice.
Since the properties to be configured are defined at device level,
delegate the actual work to a callback function.

If no callback is supplied, the device can't be disabled.

Prepare VMSTATE_ISA_DEVICE for devices that support disabling.
Legacy devices never change their state and won't need this yet.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/hw.h      |   15 +++++++++++++++
 hw/isa-bus.c |   29 +++++++++++++++++++++++++++++
 hw/isa.h     |    4 ++++
 3 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 56447a7..07b1e2e 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -628,6 +628,21 @@ extern const VMStateInfo vmstate_info_unused_buffer;
     .info         = &vmstate_info_unused_buffer,                     \
     .flags        = VMS_BUFFER,                                      \
 }
+
+extern const VMStateDescription vmstate_isa_device;
+
+#define VMSTATE_ISA_DEVICE_V(_field, _state, _version) {             \
+    .name       = (stringify(_field)),                               \
+    .version_id   = (_version),                                      \
+    .size       = sizeof(ISADevice),                                 \
+    .vmsd       = &vmstate_isa_device,                               \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = vmstate_offset_value(_state, _field, ISADevice),   \
+}
+
+#define VMSTATE_ISA_DEVICE(_field, _state) {                         \
+    VMSTATE_ISA_DEVICE_V(_field, _state, 0)
+
 extern const VMStateDescription vmstate_pci_device;
 
 #define VMSTATE_PCI_DEVICE(_field, _state) {                         \
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 2765543..d258932 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -112,6 +112,7 @@ static int isa_qdev_init(DeviceState *qdev, DeviceInfo *base)
 
     dev->isairq[0] = -1;
     dev->isairq[1] = -1;
+    dev->enabled = true;
 
     return info->init(dev);
 }
@@ -156,6 +157,34 @@ ISADevice *isa_create_simple(const char *name)
     return dev;
 }
 
+const VMStateDescription vmstate_isa_device = {
+    .name = "ISADevice",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_BOOL(enabled, ISADevice),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+int isa_set_state(ISADevice *dev, bool enabled)
+{
+    ISADeviceInfo *info = DO_UPCAST(ISADeviceInfo, qdev, dev->qdev.info);
+    int err;
+
+    if (dev->enabled == enabled) {
+        return 42;
+    } else if (info->set_state == NULL) {
+        return -1;
+    }
+    err = info->set_state(dev, enabled);
+    if (err < 0) {
+        return err;
+    }
+    dev->enabled = enabled;
+    return err;
+}
+
 static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     ISADevice *d = DO_UPCAST(ISADevice, qdev, dev);
diff --git a/hw/isa.h b/hw/isa.h
index d2b6126..5d460ab 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -16,12 +16,15 @@ struct ISADevice {
     int nirqs;
     uint16_t ioports[32];
     int nioports;
+    bool enabled;
 };
 
 typedef int (*isa_qdev_initfn)(ISADevice *dev);
+typedef int (*isa_qdev_statefn)(ISADevice *dev, bool enabled);
 struct ISADeviceInfo {
     DeviceInfo qdev;
     isa_qdev_initfn init;
+    isa_qdev_statefn set_state;
 };
 
 ISABus *isa_bus_new(DeviceState *dev);
@@ -34,6 +37,7 @@ void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create(const char *name);
 ISADevice *isa_try_create(const char *name);
 ISADevice *isa_create_simple(const char *name);
+int isa_set_state(ISADevice *dev, bool enabled);
 
 extern target_phys_addr_t isa_mem_base;
 
-- 
1.7.5.3

[Qemu-devel] [RFC v4 04/12] isa: Allow to un-assign I/O ports

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index d258932..1f64673 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -105,6 +105,20 @@ void isa_init_ioport(ISADevice *dev, uint16_t ioport)
     isa_init_ioport_range(dev, ioport, 1);
 }
 
+void isa_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length)
+{
+    int i, j;
+    for (i = 0; i < dev->nioports; i++) {
+        if (dev->ioports[i] == start) {
+            for (j = 0; j < dev->nioports - i; j++) {
+                dev->ioports[i + j] = dev->ioports[i + length + j];
+            }
+            dev->nioports -= length;
+            break;
+        }
+    }
+}
+
 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 5d460ab..ba7a696 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -33,6 +33,7 @@ qemu_irq isa_get_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_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create(const char *name);
 ISADevice *isa_try_create(const char *name);
-- 
1.7.5.3

[Qemu-devel] [RFC v4 05/12] isa: Allow to un-associate an IRQ

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 1f64673..6ac3e61 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -80,6 +80,20 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
     dev->nirqs++;
 }
 
+void isa_discard_irq(ISADevice *dev, int isairq)
+{
+    int i, j;
+    for (i = 0; i < dev->nirqs; i++) {
+        if (dev->isairq[i] == isairq) {
+            for (j = i + 1; j < dev->nirqs; j++) {
+                dev->isairq[j - 1] = dev->isairq[j];
+            }
+            dev->nirqs--;
+            break;
+        }
+    }
+}
+
 static void isa_init_ioport_one(ISADevice *dev, uint16_t ioport)
 {
     assert(dev->nioports < ARRAY_SIZE(dev->ioports));
diff --git a/hw/isa.h b/hw/isa.h
index ba7a696..46b35f3 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -31,6 +31,7 @@ ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 qemu_irq isa_get_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
+void isa_discard_irq(ISADevice *dev, 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_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
-- 
1.7.5.3

[Qemu-devel] [RFC v4 06/12] parallel: Implement ISA set_state callback

[Andreas Färber]

Add "enabled" property.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/parallel.c |   69 ++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/hw/parallel.c b/hw/parallel.c
index cc853a5..0cfc697 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -446,6 +446,50 @@ static void parallel_reset(void *opaque)
     s->last_read_offset = ~0U;
 }
 
+static int parallel_isa_statefn(ISADevice *dev, bool enabled)
+{
+    ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
+    ParallelState *s = &isa->state;
+    int base;
+
+    base = isa->iobase;
+    if (enabled) {
+        isa_init_irq(dev, &s->irq, isa->isairq);
+
+        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);
+        }
+    } else {
+        isa_discard_irq(dev, isa->isairq);
+
+        isa_discard_ioport_range(dev, base, 8);
+        isa_unassign_ioport(base, 8);
+        if (s->hw_driver) {
+            isa_discard_ioport_range(dev, base + 4, 1);
+            isa_unassign_ioport(base + 4, 1);
+            isa_discard_ioport_range(dev, base + 0x400, 8);
+            isa_unassign_ioport(base + 0x400, 8);
+        }
+    }
+    return 0;
+}
+
 static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
 
 static int parallel_isa_initfn(ISADevice *dev)
@@ -453,7 +497,6 @@ static int parallel_isa_initfn(ISADevice *dev)
     static int index;
     ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
     ParallelState *s = &isa->state;
-    int base;
     uint8_t dummy;
 
     if (!s->chr) {
@@ -469,8 +512,6 @@ static int parallel_isa_initfn(ISADevice *dev)
         isa->iobase = isa_parallel_io[isa->index];
     index++;
 
-    base = isa->iobase;
-    isa_init_irq(dev, &s->irq, isa->isairq);
     qemu_register_reset(parallel_reset, s);
 
     if (qemu_chr_ioctl(s->chr, CHR_IOCTL_PP_READ_STATUS, &dummy) == 0) {
@@ -478,25 +519,7 @@ static int parallel_isa_initfn(ISADevice *dev)
         s->status = dummy;
     }
 
-    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);
-    }
+    parallel_isa_statefn(dev, true);
     return 0;
 }
 
@@ -581,11 +604,13 @@ static ISADeviceInfo parallel_isa_info = {
     .qdev.name  = "isa-parallel",
     .qdev.size  = sizeof(ISAParallelState),
     .init       = parallel_isa_initfn,
+    .set_state  = parallel_isa_statefn,
     .qdev.props = (Property[]) {
         DEFINE_PROP_UINT32("index", ISAParallelState, index,   -1),
         DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  -1),
         DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  7),
         DEFINE_PROP_CHR("chardev",  ISAParallelState, state.chr),
+        DEFINE_PROP_BOOL("enabled", ISAParallelState, dev.enabled, true),
         DEFINE_PROP_END_OF_LIST(),
     },
 };
-- 
1.7.5.3

[Qemu-devel] [RFC v4 07/12] serial: Implement ISA set_state() callback

[Andreas Färber]

Incorporate ISA VMState. Add "enabled" property.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/serial.c |   30 +++++++++++++++++++++++++-----
 1 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/hw/serial.c b/hw/serial.c
index 0ee61dd..a058cb6 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -752,6 +752,26 @@ void serial_set_frequency(SerialState *s, uint32_t frequency)
     serial_update_parameters(s);
 }
 
+static int serial_isa_statefn(ISADevice *dev, bool enabled)
+{
+    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
+    SerialState *s = &isa->state;
+
+    if (enabled) {
+        isa_init_irq(dev, &s->irq, isa->isairq);
+
+        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);
+    } else {
+        isa_discard_irq(dev, isa->isairq);
+
+        isa_discard_ioport_range(dev, isa->iobase, 8);
+        isa_unassign_ioport(isa->iobase, 8);
+    }
+    return 0;
+}
+
 static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
 static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
 
@@ -772,21 +792,19 @@ static int serial_isa_initfn(ISADevice *dev)
     index++;
 
     s->baudbase = 115200;
-    isa_init_irq(dev, &s->irq, isa->isairq);
     serial_init_core(s);
     qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
 
-    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);
+    serial_isa_statefn(dev, true);
     return 0;
 }
 
 static const VMStateDescription vmstate_isa_serial = {
     .name = "serial",
-    .version_id = 3,
+    .version_id = 4,
     .minimum_version_id = 2,
     .fields      = (VMStateField []) {
+        VMSTATE_ISA_DEVICE_V(dev, ISASerialState, 4),
         VMSTATE_STRUCT(state, ISASerialState, 0, vmstate_serial, SerialState),
         VMSTATE_END_OF_LIST()
     }
@@ -962,11 +980,13 @@ static ISADeviceInfo serial_isa_info = {
     .qdev.size  = sizeof(ISASerialState),
     .qdev.vmsd  = &vmstate_isa_serial,
     .init       = serial_isa_initfn,
+    .set_state  = serial_isa_statefn,
     .qdev.props = (Property[]) {
         DEFINE_PROP_UINT32("index", ISASerialState, index,   -1),
         DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
         DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  -1),
         DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),
+        DEFINE_PROP_BOOL("enabled", ISASerialState, dev.enabled, true),
         DEFINE_PROP_END_OF_LIST(),
     },
 };
-- 
1.7.5.3

[Qemu-devel] [PATCH v4 08/12] fdc: Parametrize ISA base, IRQ and DMA

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

Keep the PC values as defaults but allow to override them for PReP.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/fdc.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index edf0360..f4e3e0d 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -425,6 +425,9 @@ typedef struct FDCtrlSysBus {
 
 typedef struct FDCtrlISABus {
     ISADevice busdev;
+    uint32_t iobase;
+    uint32_t irq;
+    uint32_t dma;
     struct FDCtrl state;
     int32_t bootindexA;
     int32_t bootindexB;
@@ -1895,26 +1898,23 @@ static int isabus_fdc_init1(ISADevice *dev)
 {
     FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
     FDCtrl *fdctrl = &isa->state;
-    int iobase = 0x3f0;
-    int isairq = 6;
-    int dma_chann = 2;
     int ret;
 
-    register_ioport_read(iobase + 0x01, 5, 1,
+    register_ioport_read(isa->iobase + 0x01, 5, 1,
                          &fdctrl_read_port, fdctrl);
-    register_ioport_read(iobase + 0x07, 1, 1,
+    register_ioport_read(isa->iobase + 0x07, 1, 1,
                          &fdctrl_read_port, fdctrl);
-    register_ioport_write(iobase + 0x01, 5, 1,
+    register_ioport_write(isa->iobase + 0x01, 5, 1,
                           &fdctrl_write_port, fdctrl);
-    register_ioport_write(iobase + 0x07, 1, 1,
+    register_ioport_write(isa->iobase + 0x07, 1, 1,
                           &fdctrl_write_port, fdctrl);
-    isa_init_ioport_range(dev, iobase, 6);
-    isa_init_ioport(dev, iobase + 7);
+    isa_init_ioport_range(dev, isa->iobase, 6);
+    isa_init_ioport(dev, isa->iobase + 7);
 
-    isa_init_irq(&isa->busdev, &fdctrl->irq, isairq);
-    fdctrl->dma_chann = dma_chann;
+    isa_init_irq(&isa->busdev, &fdctrl->irq, isa->irq);
+    fdctrl->dma_chann = isa->dma;
 
-    qdev_set_legacy_instance_id(&dev->qdev, iobase, 2);
+    qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 2);
     ret = fdctrl_init_common(fdctrl);
 
     add_boot_device_path(isa->bootindexA, &dev->qdev, "/floppy@0");
@@ -1979,6 +1979,9 @@ static ISADeviceInfo isa_fdc_info = {
     .qdev.vmsd  = &vmstate_isa_fdc,
     .qdev.reset = fdctrl_external_reset_isa,
     .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", FDCtrlISABus, iobase, 0x3f0),
+        DEFINE_PROP_UINT32("irq", FDCtrlISABus, irq, 6),
+        DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
         DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].bs),
         DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].bs),
         DEFINE_PROP_INT32("bootindexA", FDCtrlISABus, bootindexA, -1),
-- 
1.7.5.3

[Qemu-devel] [RFC v4 09/12] fdc: Implement ISA set_state() callback

[Andreas Färber]

Incorporate ISA VMState. Add "enabled" property.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/fdc.c |   47 ++++++++++++++++++++++++++++++++++-------------
 1 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index f4e3e0d..e2dcff0 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1894,25 +1894,43 @@ static int fdctrl_init_common(FDCtrl *fdctrl)
     return fdctrl_connect_drives(fdctrl);
 }
 
+static int isabus_fdc_set_state(ISADevice *dev, bool enabled)
+{
+    FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
+    FDCtrl *fdctrl = &isa->state;
+
+    if (enabled) {
+        register_ioport_read(isa->iobase + 0x01, 5, 1,
+                             &fdctrl_read_port, fdctrl);
+        register_ioport_read(isa->iobase + 0x07, 1, 1,
+                             &fdctrl_read_port, fdctrl);
+        register_ioport_write(isa->iobase + 0x01, 5, 1,
+                              &fdctrl_write_port, fdctrl);
+        register_ioport_write(isa->iobase + 0x07, 1, 1,
+                              &fdctrl_write_port, fdctrl);
+        isa_init_ioport_range(dev, isa->iobase, 6);
+        isa_init_ioport(dev, isa->iobase + 7);
+
+        isa_init_irq(&isa->busdev, &fdctrl->irq, isa->irq);
+    } else {
+        isa_discard_ioport_range(dev, isa->iobase + 0x07, 1);
+        isa_discard_ioport_range(dev, isa->iobase + 0x01, 5);
+        isa_unassign_ioport(isa->iobase + 7, 1);
+        isa_unassign_ioport(isa->iobase, 6);
+        
+        isa_discard_irq(&isa->busdev, isa->irq);
+    }
+    return 0;
+}
+
 static int isabus_fdc_init1(ISADevice *dev)
 {
     FDCtrlISABus *isa = DO_UPCAST(FDCtrlISABus, busdev, dev);
     FDCtrl *fdctrl = &isa->state;
     int ret;
 
-    register_ioport_read(isa->iobase + 0x01, 5, 1,
-                         &fdctrl_read_port, fdctrl);
-    register_ioport_read(isa->iobase + 0x07, 1, 1,
-                         &fdctrl_read_port, fdctrl);
-    register_ioport_write(isa->iobase + 0x01, 5, 1,
-                          &fdctrl_write_port, fdctrl);
-    register_ioport_write(isa->iobase + 0x07, 1, 1,
-                          &fdctrl_write_port, fdctrl);
-    isa_init_ioport_range(dev, isa->iobase, 6);
-    isa_init_ioport(dev, isa->iobase + 7);
-
-    isa_init_irq(&isa->busdev, &fdctrl->irq, isa->irq);
     fdctrl->dma_chann = isa->dma;
+    isabus_fdc_set_state(dev, true);
 
     qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 2);
     ret = fdctrl_init_common(fdctrl);
@@ -1962,9 +1980,10 @@ static int sun4m_fdc_init1(SysBusDevice *dev)
 
 static const VMStateDescription vmstate_isa_fdc ={
     .name = "fdc",
-    .version_id = 2,
+    .version_id = 3,
     .minimum_version_id = 2,
     .fields = (VMStateField []) {
+        VMSTATE_ISA_DEVICE_V(busdev, FDCtrlISABus, 3),
         VMSTATE_STRUCT(state, FDCtrlISABus, 0, vmstate_fdc, FDCtrl),
         VMSTATE_END_OF_LIST()
     }
@@ -1972,6 +1991,7 @@ static const VMStateDescription vmstate_isa_fdc ={
 
 static ISADeviceInfo isa_fdc_info = {
     .init = isabus_fdc_init1,
+    .set_state = isabus_fdc_set_state,
     .qdev.name  = "isa-fdc",
     .qdev.fw_name  = "fdc",
     .qdev.size  = sizeof(FDCtrlISABus),
@@ -1986,6 +2006,7 @@ static ISADeviceInfo isa_fdc_info = {
         DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].bs),
         DEFINE_PROP_INT32("bootindexA", FDCtrlISABus, bootindexA, -1),
         DEFINE_PROP_INT32("bootindexB", FDCtrlISABus, bootindexB, -1),
+        DEFINE_PROP_BOOL("enabled", FDCtrlISABus, busdev.enabled, true),
         DEFINE_PROP_END_OF_LIST(),
     },
 };
-- 
1.7.5.3

[Qemu-devel] [RFC v4 10/12] ide: Allow to discard I/O ports

[Andreas Färber]

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/ide/core.c     |    8 ++++++++
 hw/ide/internal.h |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 45410e8..c3b82de 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1760,6 +1760,14 @@ void ide_init_ioport(IDEBus *bus, int iobase, int iobase2)
     register_ioport_read(iobase, 4, 4, ide_data_readl, bus);
 }
 
+void ide_discard_ioport(int iobase, int iobase2)
+{
+    isa_unassign_ioport(iobase, 8);
+    if (iobase2 != 0) {
+        isa_unassign_ioport(iobase2, 1);
+    }
+}
+
 static bool is_identify_set(void *opaque, int version_id)
 {
     IDEState *s = opaque;
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index c2b35ec..dc0a2c9 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -564,6 +564,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
                                     DriveInfo *hd1, qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
+void ide_discard_ioport(int iobase, int iobase2);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
 void ide_dma_cb(void *opaque, int ret);
-- 
1.7.5.3

[Qemu-devel] [RFC v4 11/12] ide: Implement ISA set_state() callback

[Andreas Färber]

Incorporate ISA VMState. Add "enabled" property.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/ide/isa.c |   32 +++++++++++++++++++++++++++-----
 1 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 4ac7453..fd5c55e 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -52,25 +52,45 @@ static void isa_ide_reset(DeviceState *d)
 
 static const VMStateDescription vmstate_ide_isa = {
     .name = "isa-ide",
-    .version_id = 3,
+    .version_id = 4,
     .minimum_version_id = 0,
     .minimum_version_id_old = 0,
     .fields      = (VMStateField []) {
+        VMSTATE_ISA_DEVICE_V(dev, ISAIDEState, 4),
         VMSTATE_IDE_BUS(bus, ISAIDEState),
         VMSTATE_IDE_DRIVES(bus.ifs, ISAIDEState),
         VMSTATE_END_OF_LIST()
     }
 };
 
+static int isa_ide_statefn(ISADevice *dev, bool enabled)
+{
+    ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
+
+    if (enabled) {
+        ide_init_ioport(&s->bus, s->iobase, s->iobase2);
+
+        isa_init_ioport_range(dev, s->iobase, 8);
+        isa_init_ioport(dev, s->iobase2);
+
+        isa_init_irq(dev, &s->irq, s->isairq);
+    } else {
+        ide_discard_ioport(s->iobase, s->iobase2);
+
+        isa_discard_ioport_range(dev, s->iobase2, 1);
+        isa_discard_ioport_range(dev, s->iobase, 8);
+
+        isa_discard_irq(dev, s->isairq);
+    }
+    return 0;
+}
+
 static int isa_ide_initfn(ISADevice *dev)
 {
     ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
 
     ide_bus_new(&s->bus, &s->dev.qdev, 0);
-    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);
+    isa_ide_statefn(dev, true);
     ide_init2(&s->bus, s->irq);
     vmstate_register(&dev->qdev, 0, &vmstate_ide_isa, s);
     return 0;
@@ -102,11 +122,13 @@ static ISADeviceInfo isa_ide_info = {
     .qdev.fw_name  = "ide",
     .qdev.size  = sizeof(ISAIDEState),
     .init       = isa_ide_initfn,
+    .set_state  = isa_ide_statefn,
     .qdev.reset = isa_ide_reset,
     .qdev.props = (Property[]) {
         DEFINE_PROP_HEX32("iobase",  ISAIDEState, iobase,  0x1f0),
         DEFINE_PROP_HEX32("iobase2", ISAIDEState, iobase2, 0x3f6),
         DEFINE_PROP_UINT32("irq",    ISAIDEState, isairq,  14),
+        DEFINE_PROP_BOOL("enabled",  ISAIDEState, dev.enabled, true),
         DEFINE_PROP_END_OF_LIST(),
     },
 };
-- 
1.7.5.3

[Qemu-devel] [RFC v4 12/12] prep: Add pc87312 Super I/O emulation

[Andreas Färber]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Create all devices ahead of time and enable/disable instead.
Check the qdev properties for whether a change is necessary.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 Makefile.objs                   |    1 +
 default-configs/ppc-softmmu.mak |    2 +
 hw/pc87312.c                    |  470 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 473 insertions(+), 0 deletions(-)
 create mode 100644 hw/pc87312.c

diff --git a/Makefile.objs b/Makefile.objs
index 66ffad4..3d9fc7a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -206,6 +206,7 @@ hw-obj-$(CONFIG_SMARTCARD_NSS) += ccid-card-emulated.o
 # PPC devices
 hw-obj-$(CONFIG_OPENPIC) += openpic.o
 hw-obj-$(CONFIG_PREP_PCI) += prep_pci.o
+hw-obj-$(CONFIG_PC87312) += pc87312.o
 # Mac shared devices
 hw-obj-$(CONFIG_MACIO) += macio.o
 hw-obj-$(CONFIG_CUDA) += cuda.o
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 4563742..4b3ebec 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -7,12 +7,14 @@ CONFIG_ESCC=y
 CONFIG_M48T59=y
 CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
+CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
 CONFIG_FDC=y
 CONFIG_DMA=y
 CONFIG_OPENPIC=y
 CONFIG_PREP_PCI=y
+CONFIG_PC87312=y
 CONFIG_MACIO=y
 CONFIG_CUDA=y
 CONFIG_ADB=y
diff --git a/hw/pc87312.c b/hw/pc87312.c
new file mode 100644
index 0000000..71295ec
--- /dev/null
+++ b/hw/pc87312.c
@@ -0,0 +1,470 @@
+/*
+ * QEMU National Semiconductor PC87312 (Super I/O)
+ *
+ * Copyright (c) 2010 Herve Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "isa.h"
+#include "fdc.h"
+#include "ide.h"
+
+//#define DEBUG_PC87312
+
+#ifdef DEBUG_PC87312
+#define DPRINTF(fmt, ...) \
+do { fprintf(stderr, "pc87312: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do {} while (0)
+#endif
+
+#define BADF(fmt, ...) \
+do { fprintf(stderr, "pc87312 ERROR: " fmt , ## __VA_ARGS__); } while (0)
+
+#define REG_FER 0
+#define REG_FAR 1
+#define REG_PTR 2
+
+#define FER regs[REG_FER]
+#define FAR regs[REG_FAR]
+#define PTR regs[REG_PTR]
+
+#define FER_PARALLEL_EN   0x01
+#define FER_UART1_EN      0x02
+#define FER_UART2_EN      0x04
+#define FER_FDC_EN        0x08
+#define FER_FDC_4         0x10
+#define FER_FDC_ADDR      0x20
+#define FER_IDE_EN        0x40
+#define FER_IDE_ADDR      0x80
+
+#define FAR_PARALLEL_ADDR 0x03
+#define FAR_UART1_ADDR    0x0C
+#define FAR_UART2_ADDR    0x30
+#define FAR_UART_3_4      0xC0
+
+#define PTR_POWER_DOWN    0x01
+#define PTR_CLOCK_DOWN    0x02
+#define PTR_PWDN          0x04
+#define PTR_IRQ_5_7       0x08
+#define PTR_UART1_TEST    0x10
+#define PTR_UART2_TEST    0x20
+#define PTR_LOCK_CONF     0x40
+#define PTR_EPP_MODE      0x80
+
+typedef struct PC87312State {
+    uint8_t config; /* initial configuration */
+
+    struct {
+        DeviceState *dev;
+        CharDriverState *chr;
+    } parallel;
+
+    struct {
+        DeviceState *dev;
+        CharDriverState *chr;
+    } uart[2];
+
+    struct {
+        DeviceState *dev;
+        BlockDriverState *drive[2];
+        uint32_t base;
+    } fdc;
+
+    struct {
+        DeviceState *dev;
+        uint32_t base;
+    } ide;
+
+    uint8_t read_id_step;
+    uint8_t selected_index;
+
+    uint8_t regs[3];
+} PC87312State;
+
+
+/* Parallel port */
+
+static inline bool is_parallel_enabled(PC87312State *s)
+{
+    return s->FER & FER_PARALLEL_EN;
+}
+
+static const uint32_t parallel_base[] = { 0x378, 0x3bc, 0x278, 0x00 };
+
+static inline uint32_t get_parallel_iobase(PC87312State *s)
+{
+    return parallel_base[s->FAR & FAR_PARALLEL_ADDR];
+}
+
+static const uint32_t parallel_irq[] = { 5, 7, 5, 0 };
+
+static inline uint32_t get_parallel_irq(PC87312State *s)
+{
+    int idx;
+    idx = (s->FAR & FAR_PARALLEL_ADDR);
+    if (idx == 0) {
+        return (s->PTR & PTR_IRQ_5_7) ? 7 : 5;
+    } else {
+        return parallel_irq[idx];
+    }
+}
+
+static inline bool is_parallel_epp(PC87312State *s)
+{
+    return (s->PTR & PTR_EPP_MODE);
+}
+
+static void update_parallel(PC87312State *s)
+{
+    ISADevice *isa;
+    uint32_t base, isairq;
+    bool enabled;
+
+    if (s->parallel.dev != NULL) {
+        isa = DO_UPCAST(ISADevice, qdev, s->parallel.dev);
+        base = get_parallel_iobase(s);
+        isairq = get_parallel_irq(s);
+        enabled = is_parallel_enabled(s);
+        if (qdev_prop_get_bool(&isa->qdev, "enabled") != enabled ||
+            qdev_prop_get_uint32(&isa->qdev, "iobase") != base ||
+            qdev_prop_get_uint32(&isa->qdev, "irq") != isairq) {
+            isa_set_state(isa, false);
+            qdev_prop_set_uint32(&isa->qdev, "iobase", base);
+            qdev_prop_set_uint32(&isa->qdev, "irq", isairq);
+            if (enabled) {
+                isa_set_state(isa, true);
+            }
+        }
+    }
+}
+
+
+/* UARTs */
+
+static const uint32_t uart_base[2][4] = {
+    { 0x3e8, 0x338, 0x2e8, 0x220 },
+    { 0x2e8, 0x238, 0x2e0, 0x228 }
+};
+
+static inline uint32_t get_uart_iobase(PC87312State *s, int i)
+{
+    int idx;
+    idx = (s->FAR >> (2 * i + 2)) & 0x3;
+    if (idx == 0) {
+        return 0x3f8;
+    } else if (idx == 1) {
+        return 0x2f8;
+    } else {
+        return uart_base[idx & 1][(s->FAR & FAR_UART_3_4) >> 6];
+    }
+}
+
+static inline uint32_t get_uart_irq(PC87312State *s, int i)
+{
+    int idx;
+    idx = (s->FAR >> (2 * i + 2)) & 0x3;
+    return (idx & 1) ? 3 : 4;
+}
+
+static inline bool is_uart_enabled(PC87312State *s, int i)
+{
+    return s->FER & (FER_UART1_EN << i);
+}
+
+static void update_uarts(PC87312State *s)
+{
+    ISADevice *isa;
+    uint32_t base, isairq;
+    bool enabled;
+    int i;
+
+    for (i = 0; i < 2; i++) {
+        if (s->uart[i].dev != NULL) {
+            isa = DO_UPCAST(ISADevice, qdev, s->parallel.dev);
+            base = get_uart_iobase(s, i);
+            isairq = get_uart_irq(s, i);
+            enabled = is_uart_enabled(s, i);
+            if (qdev_prop_get_bool(&isa->qdev, "enabled") != enabled ||
+                qdev_prop_get_uint32(&isa->qdev, "iobase") != base ||
+                qdev_prop_get_uint32(&isa->qdev, "irq") != isairq) {
+                isa_set_state(isa, false);
+                qdev_prop_set_uint32(&isa->qdev, "iobase", base);
+                qdev_prop_set_uint32(&isa->qdev, "irq", isairq);
+                if (enabled) {
+                    isa_set_state(isa, true);
+                }
+            }
+        }
+    }
+}
+
+
+/* Floppy controller */
+
+static inline bool is_fdc_enabled(PC87312State *s)
+{
+    return (s->FER & FER_FDC_EN);
+}
+
+static inline uint32_t get_fdc_iobase(PC87312State *s)
+{
+    return (s->FER & FER_FDC_ADDR) ? 0x370 : 0x3f0;
+}
+
+static void update_fdc(PC87312State *s)
+{
+    ISADevice *isa = DO_UPCAST(ISADevice, qdev, s->fdc.dev);
+    uint32_t base;
+    bool enabled;
+
+    base = get_fdc_iobase(s);
+    enabled = is_fdc_enabled(s);
+    if (qdev_prop_get_bool(&isa->qdev, "enabled") != enabled ||
+        qdev_prop_get_uint32(&isa->qdev, "iobase") != base) {
+        isa_set_state(isa, false);
+        qdev_prop_set_uint32(&isa->qdev, "iobase", base);
+        if (enabled) {
+            isa_set_state(isa, true);
+        }
+    }
+}
+
+
+/* IDE controller */
+
+static inline bool is_ide_enabled(PC87312State *s)
+{
+    return (s->FER & FER_IDE_EN);
+}
+
+static inline uint32_t get_ide_iobase(PC87312State *s)
+{
+    return (s->FER & FER_IDE_ADDR) ? 0x170 : 0x1f0;
+}
+
+static void update_ide(PC87312State *s)
+{
+    ISADevice *isa = DO_UPCAST(ISADevice, qdev, s->ide.dev);
+    uint32_t base;
+    bool enabled;
+
+    base = get_ide_iobase(s);
+    enabled = is_ide_enabled(s);
+    if (qdev_prop_get_bool(&isa->qdev, "enabled") != enabled ||
+        qdev_prop_get_uint32(&isa->qdev, "iobase") != base) {
+        isa_set_state(isa, false);
+        qdev_prop_set_uint32(&isa->qdev, "iobase", base);
+        qdev_prop_set_uint32(&isa->qdev, "iobase2", base + 0x206);
+        if (enabled) {
+            isa_set_state(isa, true);
+        }
+    }
+}
+
+
+static void update_mappings(PC87312State *s)
+{
+    update_parallel(s);
+    update_uarts(s);
+    update_fdc(s);
+    update_ide(s);
+}
+
+static void pc87312_soft_reset(PC87312State *s)
+{
+    static const uint8_t fer_init[] = {
+        0x4f, 0x4f, 0x4f, 0x4f, 0x4f, 0x4f, 0x4b, 0x4b,
+        0x4b, 0x4b, 0x4b, 0x4b, 0x0f, 0x0f, 0x0f, 0x0f,
+        0x49, 0x49, 0x49, 0x49, 0x07, 0x07, 0x07, 0x07,
+        0x47, 0x47, 0x47, 0x47, 0x47, 0x47, 0x08, 0x00,
+    };
+    static const uint8_t far_init[] = {
+        0x10, 0x11, 0x11, 0x39, 0x24, 0x38, 0x00, 0x01,
+        0x01, 0x09, 0x08, 0x08, 0x10, 0x11, 0x39, 0x24,
+        0x00, 0x01, 0x01, 0x00, 0x10, 0x11, 0x39, 0x24,
+        0x10, 0x11, 0x11, 0x39, 0x24, 0x38, 0x10, 0x10,
+    };
+    static const uint8_t ptr_init[] = {
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
+    };
+
+    s->read_id_step = 0;
+    s->selected_index = REG_FER;
+
+    s->FER = fer_init[s->config & 0x1f];
+    s->FAR = far_init[s->config & 0x1f];
+    s->PTR = ptr_init[s->config & 0x1f];
+}
+
+static void pc87312_hard_reset(PC87312State *s)
+{
+    pc87312_soft_reset(s);
+}
+
+static void pc87312_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+{
+    PC87312State *s = opaque;
+
+    DPRINTF("%s: write %x at %x\n", __func__, val, addr);
+
+    if ((addr & 1) == 0) {
+        /* Index register */
+        s->read_id_step = 2;
+        s->selected_index = val;
+    } else {
+        /* Data register */
+        if (s->selected_index < 3) {
+            s->regs[s->selected_index] = val;
+            update_mappings(s);
+        }
+    }
+}
+
+static uint32_t pc87312_ioport_read(void *opaque, uint32_t addr)
+{
+    PC87312State *s = opaque;
+    uint32_t val;
+
+    if ((addr & 1) == 0) {
+        /* Index register */
+        if (s->read_id_step++ == 0) {
+            val = 0x88;
+        } else if (s->read_id_step++ == 1) {
+            val = 0;
+        } else {
+            val = s->selected_index;
+        }
+    } else {
+        /* Data register */
+        if (s->selected_index < 3) {
+            val = s->regs[s->selected_index];
+        } else {
+            /* Invalid selected index */
+            val = 0;
+        }
+    }
+
+    DPRINTF("%s: read %x at %x\n", __func__, val, addr);
+    return val;
+}
+
+static void pc87312_init_core(PC87312State *s)
+{
+    ISADevice *isa;
+    int i;
+
+    pc87312_hard_reset(s);
+
+    if (s->parallel.chr != NULL) {
+        isa = isa_create("isa-parallel");
+        qdev_prop_set_uint32(&isa->qdev, "index", 0);
+        qdev_prop_set_uint32(&isa->qdev, "iobase", get_parallel_iobase(s));
+        qdev_prop_set_uint32(&isa->qdev, "irq", get_parallel_irq(s));
+        qdev_prop_set_chr(&isa->qdev, "chardev", s->parallel.chr);
+        qdev_init_nofail(&isa->qdev);
+        s->parallel.dev = &isa->qdev;
+    }
+
+    for (i = 0; i < 2; i++) {
+        if (s->uart[i].chr != NULL) {
+            isa = isa_create("isa-serial");
+            qdev_prop_set_uint32(&isa->qdev, "index", i);
+            qdev_prop_set_uint32(&isa->qdev, "iobase", get_uart_iobase(s, i));
+            qdev_prop_set_uint32(&isa->qdev, "irq", get_uart_irq(s, i));
+            qdev_prop_set_chr(&isa->qdev, "chardev", s->uart[i].chr);
+            qdev_init_nofail(&isa->qdev);
+            s->uart[i].dev = &isa->qdev;
+        }
+    }
+
+    isa = isa_create("isa-fdc");
+    qdev_prop_set_uint32(&isa->qdev, "iobase", get_fdc_iobase(s));
+    qdev_prop_set_uint32(&isa->qdev, "irq", 6);
+    if (s->fdc.drive[0] != NULL) {
+        qdev_prop_set_drive_nofail(&isa->qdev, "driveA", s->fdc.drive[0]);
+    }
+    if (s->fdc.drive[1] != NULL) {
+        qdev_prop_set_drive_nofail(&isa->qdev, "driveB", s->fdc.drive[1]);
+    }
+    qdev_init_nofail(&isa->qdev);
+    s->fdc.dev = &isa->qdev;
+
+    isa = isa_create("isa-ide");
+    qdev_prop_set_uint32(&isa->qdev, "iobase", get_ide_iobase(s));
+    qdev_prop_set_uint32(&isa->qdev, "iobase2", get_ide_iobase(s) + 0x206);
+    qdev_prop_set_uint32(&isa->qdev, "irq", 14);
+    qdev_init_nofail(&isa->qdev);
+    s->ide.dev = &isa->qdev;
+
+    update_mappings(s);
+}
+
+typedef struct ISAPC87312State {
+    ISADevice dev;
+    uint32_t iobase;
+    PC87312State state;
+} ISAPC87312State;
+
+static void isa_pc87312_reset(DeviceState *d)
+{
+    PC87312State *s = &container_of(d, ISAPC87312State, dev.qdev)->state;
+    pc87312_soft_reset(s);
+}
+
+static int isa_pc87312_init(ISADevice *dev)
+{
+    ISAPC87312State *isa = DO_UPCAST(ISAPC87312State, dev, dev);
+    PC87312State *s = &isa->state;
+
+    pc87312_init_core(s);
+
+    register_ioport_write(isa->iobase, 2, 1, pc87312_ioport_write, s);
+    register_ioport_read(isa->iobase, 2, 1, pc87312_ioport_read, s);
+    return 0;
+}
+
+static ISADeviceInfo pc87312_isa_info = {
+    .qdev.name = "isa-pc87312",
+    .qdev.size = sizeof(ISAPC87312State),
+    .qdev.reset = isa_pc87312_reset,
+    .init = isa_pc87312_init,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", ISAPC87312State, iobase, 0x398),
+        DEFINE_PROP_UINT8("config", ISAPC87312State, state.config, 1),
+        DEFINE_PROP_CHR("parallel", ISAPC87312State, state.parallel.chr),
+        DEFINE_PROP_CHR("uart1", ISAPC87312State, state.uart[0].chr),
+        DEFINE_PROP_CHR("uart2", ISAPC87312State, state.uart[1].chr),
+        DEFINE_PROP_DRIVE("floppyA", ISAPC87312State, state.fdc.drive[0]),
+        DEFINE_PROP_DRIVE("floppyB", ISAPC87312State, state.fdc.drive[1]),
+        DEFINE_PROP_END_OF_LIST()
+    },
+};
+
+static void pc87312_register_devices(void)
+{
+    isa_qdev_register(&pc87312_isa_info);
+}
+
+device_init(pc87312_register_devices)
-- 
1.7.5.3

Re: [Qemu-devel] [RFC v4 09/12] fdc: Implement ISA set_state() callback

[Gerd Hoffmann]

   Hi,

> +        VMSTATE_ISA_DEVICE_V(busdev, FDCtrlISABus, 3),

Where is the patch adding this?

cheers,
   Gerd

Re: [Qemu-devel] [RFC v4 09/12] fdc: Implement ISA set_state() callback

[Andreas Färber]

Am 09.06.2011 um 09:56 schrieb Gerd Hoffmann:

>> +        VMSTATE_ISA_DEVICE_V(busdev, FDCtrlISABus, 3),
>
> Where is the patch adding this?

v4 03/12: http://patchwork.ozlabs.org/patch/99537/

Note that there's no VMSTATE_PCI_DEVICE_V(), but I found the _V very  
convenient here.

Andreas

Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Gerd Hoffmann]

   Hi,

> +const VMStateDescription vmstate_isa_device = {
> +    .name = "ISADevice",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField []) {
> +        VMSTATE_BOOL(enabled, ISADevice),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

That alone isn't enougth.  You also need to save iobase and irq.  And 
have a pre-load callback which disables the device + a post-load 
callback which re-enables them with the new settings.

I get the feeling that doing all this in the pc87312 emulation is easier 
as it needs to have this logic anyway for config register writes and you 
can probably reuse the code for loadvm pre- and postprocessing.

cheers,
   Gerd

Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Andreas Färber]

Hi,

Am 09.06.2011 um 12:39 schrieb Gerd Hoffmann:

>> +const VMStateDescription vmstate_isa_device = {
>> +    .name = "ISADevice",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField []) {
>> +        VMSTATE_BOOL(enabled, ISADevice),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>
> That alone isn't enougth.  You also need to save iobase and irq.

Here it becomes tricky: the enabling was done at ISA level on your  
suggestion, but the iobase and isairq are kept in the ISADevices'  
state. Duplicates are however kept at ISA level, it's fairly ugly.

I would prefer to save the state where it is stored. In this case  
enabled is store at ISADevice level, so I added the VMState for it here.

Btw is 1 correct here or should an initial version be 0?

> And have a pre-load callback which disables the device + a post-load  
> callback which re-enables them with the new settings.

Right.

> I get the feeling that doing all this in the pc87312 emulation is  
> easier as it needs to have this logic anyway for config register  
> writes and you can probably reuse the code for loadvm pre- and  
> postprocessing.


Well, I wasn't looking for the easiest way but for the proper way. I  
don't want to let pc87312-internal state get out-of-sync with that of  
the aggregated devices. So we still need the qdev getters, and we  
still need each device to handle enabling/disabling itself. Are you  
okay with those parts if we move just the VMState to pc87312? That  
feels wrong.

Do we have any ordering guarantee that the isa devices were loaded  
prior to the pc87312 post hook?

How do BIOS config changes work on a PC? Which qdev would be  
responsible for saving their state?

Andreas

Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Gerd Hoffmann]

   Hi,

> Btw is 1 correct here or should an initial version be 0?

I think for sub-structs it doesn't matter at all, only for top-level 
vmstates.

>> I get the feeling that doing all this in the pc87312 emulation is
>> easier as it needs to have this logic anyway for config register
>> writes and you can probably reuse the code for loadvm pre- and
>> postprocessing.
>
> Well, I wasn't looking for the easiest way but for the proper way. I
> don't want to let pc87312-internal state get out-of-sync with that of
> the aggregated devices. So we still need the qdev getters, and we still
> need each device to handle enabling/disabling itself.

Do we?  The pc87312 is the only instance which changes those settings at 
runtime, so they should not get out-of-sync even if they are write-only 
for the pc87312.

> Are you okay with
> those parts if we move just the VMState to pc87312? That feels wrong.

I'd keep everything (iobase, irq, enabled state) in pc87312, so the isa 
vmstate doesn't need any updates.  The pc87312 actually applies the 
configuration, so this doesn't feel wrong to me ...

> Do we have any ordering guarantee that the isa devices were loaded prior
> to the pc87312 post hook?

I don't think so.  Juan?

> How do BIOS config changes work on a PC? Which qdev would be responsible
> for saving their state?

They are not re-configurable at runtime.  I think even on real hardware 
you usually can only enable/disable devices, not change the configuration.

cheers,
   Gerd

Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Andreas Färber]

Hi,

Am 09.06.2011 um 14:23 schrieb Gerd Hoffmann:

>>> I get the feeling that doing all this in the pc87312 emulation is
>>> easier as it needs to have this logic anyway for config register
>>> writes and you can probably reuse the code for loadvm pre- and
>>> postprocessing.
>>
>> Well, I wasn't looking for the easiest way but for the proper way. I
>> don't want to let pc87312-internal state get out-of-sync with that of
>> the aggregated devices. So we still need the qdev getters, and we  
>> still
>> need each device to handle enabling/disabling itself.
>
> Do we?  The pc87312 is the only instance which changes those  
> settings at runtime, so they should not get out-of-sync even if they  
> are write-only for the pc87312.

The devices need to register the I/O ports for sure. No one else knows  
what functions to use.
So we either need your generic set_state callback or my explicit  
helper functions to invoke that.

>> Are you okay with
>> those parts if we move just the VMState to pc87312? That feels wrong.
>
> I'd keep everything (iobase, irq, enabled state) in pc87312, so the  
> isa vmstate doesn't need any updates.  The pc87312 actually applies  
> the configuration, so this doesn't feel wrong to me ...
>
>> Do we have any ordering guarantee that the isa devices were loaded  
>> prior
>> to the pc87312 post hook?
>
> I don't think so.  Juan?

In that case it won't work (out-of-sync) and we shouldn't introduce  
VMState for pc87312 at all imo. In theory we'd probably need a pc87312- 
owned bus to put the devices on but then I don't see how to reuse the  
existing isa devices.

>> How do BIOS config changes work on a PC? Which qdev would be  
>> responsible
>> for saving their state?
>
> They are not re-configurable at runtime.  I think even on real  
> hardware you usually can only enable/disable devices, not change the  
> configuration.

I'm positive they are configurable by the BIOS, that's why I called it  
"ISA reconfigurability" (not "Weird workarounds for PReP") and thought  
about VMState in the first place.

SystemSoft MobilePRO BIOS, PhoenixBIOS, AMI BIOS, Award BIOS all allow  
to reconfigure I/O base (same also IRQ) of both serial ports and  
parallel port. fdc and ide however I've seen nowhere before.
Some newer ones have an "Auto" setting that hides this, and when  
choosing "Enabled" a new field shows up.
Configuration options are a set of fixed options, similar to pc87312.

DOS, Windows and Linux were able to apply such driver configuration  
changes after a reboot iirc. For example, ISA SoundBlaster or NE2000  
cards. I also remember 3Com having special DOS boot discs with a tool  
to change on-card config for some ISA network cards.

Andreas

Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Gerd Hoffmann]

   Hi,

> In that case it won't work (out-of-sync) and we shouldn't introduce
> VMState for pc87312 at all imo. In theory we'd probably need a
> pc87312-owned bus to put the devices on but then I don't see how to
> reuse the existing isa devices.

Oh, that should actually work just fine and is maybe the best idea.
The bus interface and the bus implementation are independant from each 
other.  USB for example has UHCI and OHCI, both implement a usb bus and 
you can hook the USB devices to both of them just fine.  Might be the 
ISA bus is a bit sloppy because a single implementation exists and needs 
some cleanups to make this actually work.  But worth investigating I think.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH v4 01/12] qdev: Add support for property type bool

[Markus Armbruster]

Andreas Färber <andreas.faerber@web.de> writes:

> VMState supports the type bool but qdev instead supports bit, backed by
> uint32_t. Therefore let's add DEFINE_PROP_BOOL() and qdev_prop_set_bool().
>
> Since, e.g., enabled=on does not look nice, parse/print "yes" and "no".

The difference between bool and bit properties is implementation detail.
Using "on/off" for one, and "yes/no" for the other is a gratuitously
inconsistent user interface.

For what it's worth, "on" doesn't look nice for many bit properties
either.

Options:

1. Live with the inconsistent user interface

1. Stick to on/off ugly or not

3. Switch to yes/no and deprecate on/off

Opinions?

Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Markus Armbruster]

Andreas Färber <andreas.faerber@web.de> writes:

> To allow enabling/disabling present ISA devices without hotplug,
> keep track of state and add a helper to avoid enabling twice.
> Since the properties to be configured are defined at device level,
> delegate the actual work to a callback function.
>
> If no callback is supplied, the device can't be disabled.
>
> Prepare VMSTATE_ISA_DEVICE for devices that support disabling.
> Legacy devices never change their state and won't need this yet.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  hw/hw.h      |   15 +++++++++++++++
>  hw/isa-bus.c |   29 +++++++++++++++++++++++++++++
>  hw/isa.h     |    4 ++++
>  3 files changed, 48 insertions(+), 0 deletions(-)
>
> diff --git a/hw/hw.h b/hw/hw.h
> index 56447a7..07b1e2e 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -628,6 +628,21 @@ extern const VMStateInfo vmstate_info_unused_buffer;
>      .info         = &vmstate_info_unused_buffer,                     \
>      .flags        = VMS_BUFFER,                                      \
>  }
> +
> +extern const VMStateDescription vmstate_isa_device;
> +
> +#define VMSTATE_ISA_DEVICE_V(_field, _state, _version) {             \
> +    .name       = (stringify(_field)),                               \
> +    .version_id   = (_version),                                      \
> +    .size       = sizeof(ISADevice),                                 \
> +    .vmsd       = &vmstate_isa_device,                               \
> +    .flags      = VMS_STRUCT,                                        \
> +    .offset     = vmstate_offset_value(_state, _field, ISADevice),   \
> +}
> +
> +#define VMSTATE_ISA_DEVICE(_field, _state) {                         \
> +    VMSTATE_ISA_DEVICE_V(_field, _state, 0)
> +
>  extern const VMStateDescription vmstate_pci_device;
>  
>  #define VMSTATE_PCI_DEVICE(_field, _state) {                         \
> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> index 2765543..d258932 100644
> --- a/hw/isa-bus.c
> +++ b/hw/isa-bus.c
> @@ -112,6 +112,7 @@ static int isa_qdev_init(DeviceState *qdev, DeviceInfo *base)
>  
>      dev->isairq[0] = -1;
>      dev->isairq[1] = -1;
> +    dev->enabled = true;
>  
>      return info->init(dev);
>  }
> @@ -156,6 +157,34 @@ ISADevice *isa_create_simple(const char *name)
>      return dev;
>  }
>  
> +const VMStateDescription vmstate_isa_device = {
> +    .name = "ISADevice",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField []) {
> +        VMSTATE_BOOL(enabled, ISADevice),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +int isa_set_state(ISADevice *dev, bool enabled)
> +{
> +    ISADeviceInfo *info = DO_UPCAST(ISADeviceInfo, qdev, dev->qdev.info);
> +    int err;
> +
> +    if (dev->enabled == enabled) {
> +        return 42;

Sorry, too clever.  Make it return 0 for the benefit of future
maintenance programmers.

> +    } else if (info->set_state == NULL) {
> +        return -1;
> +    }
> +    err = info->set_state(dev, enabled);
> +    if (err < 0) {
> +        return err;
> +    }
> +    dev->enabled = enabled;
> +    return err;
> +}
> +
[...]

Re: [Qemu-devel] [RFC v4 04/12] isa: Allow to un-assign I/O ports

[Markus Armbruster]

Andreas Färber <andreas.faerber@web.de> writes:

> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  hw/isa-bus.c |   14 ++++++++++++++
>  hw/isa.h     |    1 +
>  2 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> index d258932..1f64673 100644
> --- a/hw/isa-bus.c
> +++ b/hw/isa-bus.c
> @@ -105,6 +105,20 @@ void isa_init_ioport(ISADevice *dev, uint16_t ioport)
>      isa_init_ioport_range(dev, ioport, 1);
>  }
>  
> +void isa_discard_ioport_range(ISADevice *dev, uint16_t start, uint16_t length)
> +{
> +    int i, j;
> +    for (i = 0; i < dev->nioports; i++) {
> +        if (dev->ioports[i] == start) {
> +            for (j = 0; j < dev->nioports - i; j++) {
> +                dev->ioports[i + j] = dev->ioports[i + length + j];
> +            }
> +            dev->nioports -= length;
> +            break;
> +        }
> +    }
> +}
> +

"discard"?  It's the opposite of isa_init_ioport_range(), name should
make that obvious.  "uninit"?

Note: this only affects the I/O-port bookkeeping within ISADevice, not
the actual I/O port handler registration.  Any use must be accompanied
by a matching handler de-registration.  Just like any use of
isa_init_ioport_range() must be accompanied by matching
register_ioport_FOO()s.  You can thank Gleb for this mess.

[...]

Re: [Qemu-devel] [RFC v4 05/12] isa: Allow to un-associate an IRQ

[Markus Armbruster]

Andreas Färber <andreas.faerber@web.de> writes:

> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  hw/isa-bus.c |   14 ++++++++++++++
>  hw/isa.h     |    1 +
>  2 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> index 1f64673..6ac3e61 100644
> --- a/hw/isa-bus.c
> +++ b/hw/isa-bus.c
> @@ -80,6 +80,20 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
>      dev->nirqs++;
>  }
>  
> +void isa_discard_irq(ISADevice *dev, int isairq)
> +{
> +    int i, j;
> +    for (i = 0; i < dev->nirqs; i++) {
> +        if (dev->isairq[i] == isairq) {
> +            for (j = i + 1; j < dev->nirqs; j++) {
> +                dev->isairq[j - 1] = dev->isairq[j];
> +            }
> +            dev->nirqs--;
> +            break;
> +        }
> +    }
> +}

Comment to 04/12 applies.

[...]

Re: [Qemu-devel] [RFC v4 05/12] isa: Allow to un-associate an IRQ

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Andreas Färber <andreas.faerber@web.de> writes:
>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> ---
>>  hw/isa-bus.c |   14 ++++++++++++++
>>  hw/isa.h     |    1 +
>>  2 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
>> index 1f64673..6ac3e61 100644
>> --- a/hw/isa-bus.c
>> +++ b/hw/isa-bus.c
>> @@ -80,6 +80,20 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
>>      dev->nirqs++;
>>  }
>>  
>> +void isa_discard_irq(ISADevice *dev, int isairq)
>> +{
>> +    int i, j;
>> +    for (i = 0; i < dev->nirqs; i++) {
>> +        if (dev->isairq[i] == isairq) {
>> +            for (j = i + 1; j < dev->nirqs; j++) {
>> +                dev->isairq[j - 1] = dev->isairq[j];
>> +            }
>> +            dev->nirqs--;
>> +            break;
>> +        }
>> +    }
>> +}
>
> Comment to 04/12 applies.

Sorry, misleading.

The comment about the naming applies.

The comment about the use of the function doesn't apply: isa_init_irq()
does the complete job, unlike isa_init_ioport_range().

Your isa_discard_irq() keeps the qemu_irq that was set by isa_init_irq()
around, which is perhaps not perfectly clean, but should work.

> [...]

Re: [Qemu-devel] [RFC v4 07/12] serial: Implement ISA set_state() callback

[Markus Armbruster]

Andreas Färber <andreas.faerber@web.de> writes:

> Incorporate ISA VMState. Add "enabled" property.

Could you explain why you need to stick VMSTATE_ISA_DEVICE_V() into
vmstate_isa_serial, but not for the other devices?

>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  hw/serial.c |   30 +++++++++++++++++++++++++-----
>  1 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/hw/serial.c b/hw/serial.c
> index 0ee61dd..a058cb6 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -752,6 +752,26 @@ void serial_set_frequency(SerialState *s, uint32_t frequency)
>      serial_update_parameters(s);
>  }
>  
> +static int serial_isa_statefn(ISADevice *dev, bool enabled)
> +{
> +    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
> +    SerialState *s = &isa->state;
> +
> +    if (enabled) {
> +        isa_init_irq(dev, &s->irq, isa->isairq);
> +
> +        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);
> +    } else {
> +        isa_discard_irq(dev, isa->isairq);
> +
> +        isa_discard_ioport_range(dev, isa->iobase, 8);
> +        isa_unassign_ioport(isa->iobase, 8);
> +    }
> +    return 0;
> +}
> +
>  static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
>  static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
>  
> @@ -772,21 +792,19 @@ static int serial_isa_initfn(ISADevice *dev)
>      index++;
>  
>      s->baudbase = 115200;
> -    isa_init_irq(dev, &s->irq, isa->isairq);
>      serial_init_core(s);
>      qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
>  
> -    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);
> +    serial_isa_statefn(dev, true);
>      return 0;
>  }
>  
>  static const VMStateDescription vmstate_isa_serial = {
>      .name = "serial",
> -    .version_id = 3,
> +    .version_id = 4,
>      .minimum_version_id = 2,
>      .fields      = (VMStateField []) {
> +        VMSTATE_ISA_DEVICE_V(dev, ISASerialState, 4),
>          VMSTATE_STRUCT(state, ISASerialState, 0, vmstate_serial, SerialState),
>          VMSTATE_END_OF_LIST()
>      }

If I understand vmstate correctly, this breaks migration new -> old.  Do
we care?

> @@ -962,11 +980,13 @@ static ISADeviceInfo serial_isa_info = {
>      .qdev.size  = sizeof(ISASerialState),
>      .qdev.vmsd  = &vmstate_isa_serial,
>      .init       = serial_isa_initfn,
> +    .set_state  = serial_isa_statefn,
>      .qdev.props = (Property[]) {
>          DEFINE_PROP_UINT32("index", ISASerialState, index,   -1),
>          DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
>          DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  -1),
>          DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),
> +        DEFINE_PROP_BOOL("enabled", ISASerialState, dev.enabled, true),
>          DEFINE_PROP_END_OF_LIST(),
>      },
>  };

Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Markus Armbruster]

Andreas Färber <andreas.faerber@web.de> writes:

> Hi,
>
> Am 09.06.2011 um 14:23 schrieb Gerd Hoffmann:
>
>>>> I get the feeling that doing all this in the pc87312 emulation is
>>>> easier as it needs to have this logic anyway for config register
>>>> writes and you can probably reuse the code for loadvm pre- and
>>>> postprocessing.
>>>
>>> Well, I wasn't looking for the easiest way but for the proper way. I
>>> don't want to let pc87312-internal state get out-of-sync with that of
>>> the aggregated devices. So we still need the qdev getters, and we
>>> still
>>> need each device to handle enabling/disabling itself.
>>
>> Do we?  The pc87312 is the only instance which changes those
>> settings at runtime, so they should not get out-of-sync even if they
>> are write-only for the pc87312.
>
> The devices need to register the I/O ports for sure. No one else knows
> what functions to use.
> So we either need your generic set_state callback or my explicit
> helper functions to invoke that.
>
>>> Are you okay with
>>> those parts if we move just the VMState to pc87312? That feels wrong.
>>
>> I'd keep everything (iobase, irq, enabled state) in pc87312, so the
>> isa vmstate doesn't need any updates.  The pc87312 actually applies
>> the configuration, so this doesn't feel wrong to me ...

Feels weird to me.

A device on a real ISA bus decodes whatever addresses it likes, and
raises whatever interrupt lines it likes.

Of course, the thing within the Super I/O isn't a real ISA bus, it just
looks like one to software.  Not that there's much to look at with ISA.

>>> Do we have any ordering guarantee that the isa devices were loaded
>>> prior
>>> to the pc87312 post hook?
>>
>> I don't think so.  Juan?
>
> In that case it won't work (out-of-sync) and we shouldn't introduce
> VMState for pc87312 at all imo. In theory we'd probably need a
> pc87312- 
> owned bus to put the devices on but then I don't see how to reuse the
> existing isa devices.

Err, which device provides your ISA bus if not pc87312?

>>> How do BIOS config changes work on a PC? Which qdev would be
>>> responsible
>>> for saving their state?
>>
>> They are not re-configurable at runtime.  I think even on real
>> hardware you usually can only enable/disable devices, not change the
>> configuration.
>
> I'm positive they are configurable by the BIOS, that's why I called it
> "ISA reconfigurability" (not "Weird workarounds for PReP") and thought
> about VMState in the first place.

Yes, software-configurable ISA devices are fairly common.

[...]

Re: [Qemu-devel] [RFC v4 07/12] serial: Implement ISA set_state() callback

[Andreas Färber]

Am 09.06.2011 um 17:35 schrieb Markus Armbruster:

> Andreas Färber <andreas.faerber@web.de> writes:
>
>> Incorporate ISA VMState. Add "enabled" property.
>
> Could you explain why you need to stick VMSTATE_ISA_DEVICE_V() into
> vmstate_isa_serial, but not for the other devices?

I stuck it in all affected devices that have VMState. isa-parallel, as  
pointed out elsewhere, currently does not.
What are you referring to?

>> static const VMStateDescription vmstate_isa_serial = {
>>     .name = "serial",
>> -    .version_id = 3,
>> +    .version_id = 4,
>>     .minimum_version_id = 2,
>>     .fields      = (VMStateField []) {
>> +        VMSTATE_ISA_DEVICE_V(dev, ISASerialState, 4),
>>         VMSTATE_STRUCT(state, ISASerialState, 0, vmstate_serial,  
>> SerialState),
>>         VMSTATE_END_OF_LIST()
>>     }
>
> If I understand vmstate correctly, this breaks migration new ->  
> old.  Do
> we care?

Are you referring to touching vmstate_isa_serial at all, or does the  
placement of the new field make the difference here?

Andreas

Re: [Qemu-devel] [PATCH v4 01/12] qdev: Add support for property type bool

[Andreas Färber]

Am 09.06.2011 um 16:45 schrieb Markus Armbruster:

> Andreas Färber <andreas.faerber@web.de> writes:
>
>> VMState supports the type bool but qdev instead supports bit,  
>> backed by
>> uint32_t. Therefore let's add DEFINE_PROP_BOOL() and  
>> qdev_prop_set_bool().
>>
>> Since, e.g., enabled=on does not look nice, parse/print "yes" and  
>> "no".
>
> The difference between bool and bit properties is implementation  
> detail.
> Using "on/off" for one, and "yes/no" for the other is a gratuitously
> inconsistent user interface.
>
> For what it's worth, "on" doesn't look nice for many bit properties
> either.
>
> Options:
>
> 1. Live with the inconsistent user interface
>
> 1. Stick to on/off ugly or not
>
> 3. Switch to yes/no and deprecate on/off
>
> Opinions?

It would certainly be possible to support all of on/off, yes/no, true/ 
false, 1/0 in the two parsing methods. Independent of any  
implementation detail.

My reasoning was that grammatically, using on/off with a noun in just  
fine, e.g., cache=off.
With an adjective however, it's wrong. Therefore my choice of yes/no  
here.

Juan, what did you think of?

Andreas

Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Andreas Färber]

Am 09.06.2011 um 16:53 schrieb Markus Armbruster:

> Andreas Färber <andreas.faerber@web.de> writes:
>
>> To allow enabling/disabling present ISA devices without hotplug,
>> keep track of state and add a helper to avoid enabling twice.
>> Since the properties to be configured are defined at device level,
>> delegate the actual work to a callback function.
>>
>> If no callback is supplied, the device can't be disabled.
>>
>> Prepare VMSTATE_ISA_DEVICE for devices that support disabling.
>> Legacy devices never change their state and won't need this yet.
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> ---
>> hw/hw.h      |   15 +++++++++++++++
>> hw/isa-bus.c |   29 +++++++++++++++++++++++++++++
>> hw/isa.h     |    4 ++++
>> 3 files changed, 48 insertions(+), 0 deletions(-)

>> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
>> index 2765543..d258932 100644
>> --- a/hw/isa-bus.c
>> +++ b/hw/isa-bus.c

>> +int isa_set_state(ISADevice *dev, bool enabled)
>> +{
>> +    ISADeviceInfo *info = DO_UPCAST(ISADeviceInfo, qdev, dev- 
>> >qdev.info);
>> +    int err;
>> +
>> +    if (dev->enabled == enabled) {
>> +        return 42;
>
> Sorry, too clever.  Make it return 0 for the benefit of future
> maintenance programmers.

Done.

Re: [Qemu-devel] [RFC v4 04/12] isa: Allow to un-assign I/O ports

[Andreas Färber]

Am 09.06.2011 um 17:03 schrieb Markus Armbruster:

> Andreas Färber <andreas.faerber@web.de> writes:
>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> ---
>> hw/isa-bus.c |   14 ++++++++++++++
>> hw/isa.h     |    1 +
>> 2 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
>> index d258932..1f64673 100644
>> --- a/hw/isa-bus.c
>> +++ b/hw/isa-bus.c
>> @@ -105,6 +105,20 @@ void isa_init_ioport(ISADevice *dev, uint16_t  
>> ioport)
>>     isa_init_ioport_range(dev, ioport, 1);
>> }
>>
>> +void isa_discard_ioport_range(ISADevice *dev, uint16_t start,  
>> uint16_t length)
>> +{
>> +    int i, j;
>> +    for (i = 0; i < dev->nioports; i++) {
>> +        if (dev->ioports[i] == start) {
>> +            for (j = 0; j < dev->nioports - i; j++) {
>> +                dev->ioports[i + j] = dev->ioports[i + length + j];
>> +            }
>> +            dev->nioports -= length;
>> +            break;
>> +        }
>> +    }
>> +}
>> +
>
> "discard"?  It's the opposite of isa_init_ioport_range(), name should
> make that obvious.  "uninit"?

"uninit" felt wrong.

> Note: this only affects the I/O-port bookkeeping within ISADevice, not
> the actual I/O port handler registration.  Any use must be accompanied
> by a matching handler de-registration.  Just like any use of
> isa_init_ioport_range() must be accompanied by matching
> register_ioport_FOO()s.  You can thank Gleb for this mess.

Right, I didn't spot any wrong usage though.

So what about cleaning up the mess and doing  
isa_[un]assign_ioport_range(), wrapping the ioport.c functions?
The overhead of calling it up to six times for the different widths  
and read/write would seem negligible as a startup cost. And for  
pc87312 we don't seriously have to care about performance imo.

Andreas

Re: [Qemu-devel] [RFC v4 05/12] isa: Allow to un-associate an IRQ

[Andreas Färber]

Am 09.06.2011 um 17:12 schrieb Markus Armbruster:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Andreas Färber <andreas.faerber@web.de> writes:
>>
>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>> ---
>>> hw/isa-bus.c |   14 ++++++++++++++
>>> hw/isa.h     |    1 +
>>> 2 files changed, 15 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
>>> index 1f64673..6ac3e61 100644
>>> --- a/hw/isa-bus.c
>>> +++ b/hw/isa-bus.c
>>> @@ -80,6 +80,20 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p,  
>>> int isairq)
>>>     dev->nirqs++;
>>> }
>>>
>>> +void isa_discard_irq(ISADevice *dev, int isairq)
>>> +{
>>> +    int i, j;
>>> +    for (i = 0; i < dev->nirqs; i++) {
>>> +        if (dev->isairq[i] == isairq) {
>>> +            for (j = i + 1; j < dev->nirqs; j++) {
>>> +                dev->isairq[j - 1] = dev->isairq[j];
>>> +            }
>>> +            dev->nirqs--;
>>> +            break;
>>> +        }
>>> +    }
>>> +}

> Your isa_discard_irq() keeps the qemu_irq that was set by  
> isa_init_irq()
> around, which is perhaps not perfectly clean, but should work.

We could NULL it and hope that no one uses it unchecked from some  
bottom-half or I/O thread.
Or do we have some global no-op qemu_irq?

Andreas

Re: [Qemu-devel] [RFC v4 03/12] isa: Provide set_state callback

[Andreas Färber]

Am 09.06.2011 um 18:04 schrieb Markus Armbruster:

> Andreas Färber <andreas.faerber@web.de> writes:
>
>> Am 09.06.2011 um 14:23 schrieb Gerd Hoffmann:
>>
>>>>> I get the feeling that doing all this in the pc87312 emulation is
>>>>> easier as it needs to have this logic anyway for config register
>>>>> writes and you can probably reuse the code for loadvm pre- and
>>>>> postprocessing.
>>>>
>>>> Well, I wasn't looking for the easiest way but for the proper  
>>>> way. I
>>>> don't want to let pc87312-internal state get out-of-sync with  
>>>> that of
>>>> the aggregated devices. So we still need the qdev getters, and we
>>>> still
>>>> need each device to handle enabling/disabling itself.
>>>
>>> Do we?  The pc87312 is the only instance which changes those
>>> settings at runtime, so they should not get out-of-sync even if they
>>> are write-only for the pc87312.
>>
>> The devices need to register the I/O ports for sure. No one else  
>> knows
>> what functions to use.
>> So we either need your generic set_state callback or my explicit
>> helper functions to invoke that.
>>
>>>> Are you okay with
>>>> those parts if we move just the VMState to pc87312? That feels  
>>>> wrong.
>>>
>>> I'd keep everything (iobase, irq, enabled state) in pc87312, so the
>>> isa vmstate doesn't need any updates.  The pc87312 actually applies
>>> the configuration, so this doesn't feel wrong to me ...
>
> Feels weird to me.
>
> A device on a real ISA bus decodes whatever addresses it likes, and
> raises whatever interrupt lines it likes.
>
> Of course, the thing within the Super I/O isn't a real ISA bus, it  
> just
> looks like one to software.  Not that there's much to look at with  
> ISA.
>
>>>> Do we have any ordering guarantee that the isa devices were loaded
>>>> prior
>>>> to the pc87312 post hook?
>>>
>>> I don't think so.  Juan?
>>
>> In that case it won't work (out-of-sync) and we shouldn't introduce
>> VMState for pc87312 at all imo. In theory we'd probably need a
>> pc87312-
>> owned bus to put the devices on but then I don't see how to reuse the
>> existing isa devices.
>
> Err, which device provides your ISA bus if not pc87312?

As discussed on IRC, I'm now creating the ISA bus on the upcoming  
i82378 PCI-ISA bridge, as done for PIIX3. Makes no change for the  
pc87312 Super I/O situation.

Since the whole PReP series goes to ~25 patches, I'd rather postpone  
the pc87312 VMState issues.

Andreas

Re: [Qemu-devel] [RFC v4 04/12] isa: Allow to un-assign I/O ports

[Gleb Natapov]

On Sun, Jun 12, 2011 at 01:59:51PM +0200, Andreas Färber wrote:
> Am 09.06.2011 um 17:03 schrieb Markus Armbruster:
> 
> >Andreas Färber <andreas.faerber@web.de> writes:
> >
> >>Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> >>---
> >>hw/isa-bus.c |   14 ++++++++++++++
> >>hw/isa.h     |    1 +
> >>2 files changed, 15 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> >>index d258932..1f64673 100644
> >>--- a/hw/isa-bus.c
> >>+++ b/hw/isa-bus.c
> >>@@ -105,6 +105,20 @@ void isa_init_ioport(ISADevice *dev,
> >>uint16_t ioport)
> >>    isa_init_ioport_range(dev, ioport, 1);
> >>}
> >>
> >>+void isa_discard_ioport_range(ISADevice *dev, uint16_t start,
> >>uint16_t length)
> >>+{
> >>+    int i, j;
> >>+    for (i = 0; i < dev->nioports; i++) {
> >>+        if (dev->ioports[i] == start) {
> >>+            for (j = 0; j < dev->nioports - i; j++) {
> >>+                dev->ioports[i + j] = dev->ioports[i + length + j];
> >>+            }
> >>+            dev->nioports -= length;
> >>+            break;
> >>+        }
> >>+    }
> >>+}
> >>+
> >
> >"discard"?  It's the opposite of isa_init_ioport_range(), name should
> >make that obvious.  "uninit"?
> 
> "uninit" felt wrong.
> 
> >Note: this only affects the I/O-port bookkeeping within ISADevice, not
> >the actual I/O port handler registration.  Any use must be accompanied
> >by a matching handler de-registration.  Just like any use of
> >isa_init_ioport_range() must be accompanied by matching
> >register_ioport_FOO()s.  You can thank Gleb for this mess.
> 
> Right, I didn't spot any wrong usage though.
> 
> So what about cleaning up the mess and doing
> isa_[un]assign_ioport_range(), wrapping the ioport.c functions?
> The overhead of calling it up to six times for the different widths
> and read/write would seem negligible as a startup cost. And for
> pc87312 we don't seriously have to care about performance imo.
> 
Ideally every ioport registration should be moved to use IORange. I
tried to move all isa devices to it, but it is complicated for two
reasons. First not every device is qdevified and second some devises 
can be instantiated as isa device and non-isa device and they do ioport
registration in a common code. With your approach you will have to
duplicate ioport registration code for isa and non-isa case for such
devices and code duplication is not good.

It is always easier to call something a mess instead of actually fixing
it.

--
			Gleb.

Re: [Qemu-devel] [RFC v4 04/12] isa: Allow to un-assign I/O ports

[Andreas Färber]

Am 12.06.2011 um 15:48 schrieb Gleb Natapov:

> On Sun, Jun 12, 2011 at 01:59:51PM +0200, Andreas Färber wrote:
>> Am 09.06.2011 um 17:03 schrieb Markus Armbruster:
>>
>>> Andreas Färber <andreas.faerber@web.de> writes:
>>>
>>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>>> ---
>>>> hw/isa-bus.c |   14 ++++++++++++++
>>>> hw/isa.h     |    1 +
>>>> 2 files changed, 15 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/hw/isa-bus.c b/hw/isa-bus.c
>>>> index d258932..1f64673 100644
>>>> --- a/hw/isa-bus.c
>>>> +++ b/hw/isa-bus.c
>>>> @@ -105,6 +105,20 @@ void isa_init_ioport(ISADevice *dev,
>>>> uint16_t ioport)
>>>>   isa_init_ioport_range(dev, ioport, 1);
>>>> }
>>>>
>>>> +void isa_discard_ioport_range(ISADevice *dev, uint16_t start,
>>>> uint16_t length)
>>>> +{
>>>> +    int i, j;
>>>> +    for (i = 0; i < dev->nioports; i++) {
>>>> +        if (dev->ioports[i] == start) {
>>>> +            for (j = 0; j < dev->nioports - i; j++) {
>>>> +                dev->ioports[i + j] = dev->ioports[i + length +  
>>>> j];
>>>> +            }
>>>> +            dev->nioports -= length;
>>>> +            break;
>>>> +        }
>>>> +    }
>>>> +}
>>>> +
>>>
>>> "discard"?  It's the opposite of isa_init_ioport_range(), name  
>>> should
>>> make that obvious.  "uninit"?
>>
>> "uninit" felt wrong.
>>
>>> Note: this only affects the I/O-port bookkeeping within ISADevice,  
>>> not
>>> the actual I/O port handler registration.  Any use must be  
>>> accompanied
>>> by a matching handler de-registration.  Just like any use of
>>> isa_init_ioport_range() must be accompanied by matching
>>> register_ioport_FOO()s.  You can thank Gleb for this mess.
>>
>> Right, I didn't spot any wrong usage though.
>>
>> So what about cleaning up the mess and doing
>> isa_[un]assign_ioport_range(), wrapping the ioport.c functions?
>> The overhead of calling it up to six times for the different widths
>> and read/write would seem negligible as a startup cost. And for
>> pc87312 we don't seriously have to care about performance imo.
>>
> Ideally every ioport registration should be moved to use IORange. I
> tried to move all isa devices to it, but it is complicated for two
> reasons. First not every device is qdevified and second some devises
> can be instantiated as isa device and non-isa device and they do  
> ioport
> registration in a common code. With your approach you will have to
> duplicate ioport registration code for isa and non-isa case for such
> devices and code duplication is not good.

I'm not trying to duplicate anything! What I've been seeing is that  
many of the ISA devices I've touched in this series first register the  
I/O ports and then associate them with the ISADevice, in ISA-only  
code. So the numbers *are* duplicated and I'm proposing to consolidate  
this into one call.

The existing "init" would stay, so that code with disseparate ISA and  
common parts (like IDE) remains unaffected.

Andreas

Re: [Qemu-devel] [RFC v4 04/12] isa: Allow to un-assign I/O ports

[Gleb Natapov]

On Sun, Jun 12, 2011 at 05:32:57PM +0200, Andreas Färber wrote:
> Am 12.06.2011 um 15:48 schrieb Gleb Natapov:
> 
> >On Sun, Jun 12, 2011 at 01:59:51PM +0200, Andreas Färber wrote:
> >>Am 09.06.2011 um 17:03 schrieb Markus Armbruster:
> >>
> >>>Andreas Färber <andreas.faerber@web.de> writes:
> >>>
> >>>>Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> >>>>---
> >>>>hw/isa-bus.c |   14 ++++++++++++++
> >>>>hw/isa.h     |    1 +
> >>>>2 files changed, 15 insertions(+), 0 deletions(-)
> >>>>
> >>>>diff --git a/hw/isa-bus.c b/hw/isa-bus.c
> >>>>index d258932..1f64673 100644
> >>>>--- a/hw/isa-bus.c
> >>>>+++ b/hw/isa-bus.c
> >>>>@@ -105,6 +105,20 @@ void isa_init_ioport(ISADevice *dev,
> >>>>uint16_t ioport)
> >>>>  isa_init_ioport_range(dev, ioport, 1);
> >>>>}
> >>>>
> >>>>+void isa_discard_ioport_range(ISADevice *dev, uint16_t start,
> >>>>uint16_t length)
> >>>>+{
> >>>>+    int i, j;
> >>>>+    for (i = 0; i < dev->nioports; i++) {
> >>>>+        if (dev->ioports[i] == start) {
> >>>>+            for (j = 0; j < dev->nioports - i; j++) {
> >>>>+                dev->ioports[i + j] = dev->ioports[i +
> >>>>length + j];
> >>>>+            }
> >>>>+            dev->nioports -= length;
> >>>>+            break;
> >>>>+        }
> >>>>+    }
> >>>>+}
> >>>>+
> >>>
> >>>"discard"?  It's the opposite of isa_init_ioport_range(), name
> >>>should
> >>>make that obvious.  "uninit"?
> >>
> >>"uninit" felt wrong.
> >>
> >>>Note: this only affects the I/O-port bookkeeping within
> >>>ISADevice, not
> >>>the actual I/O port handler registration.  Any use must be
> >>>accompanied
> >>>by a matching handler de-registration.  Just like any use of
> >>>isa_init_ioport_range() must be accompanied by matching
> >>>register_ioport_FOO()s.  You can thank Gleb for this mess.
> >>
> >>Right, I didn't spot any wrong usage though.
> >>
> >>So what about cleaning up the mess and doing
> >>isa_[un]assign_ioport_range(), wrapping the ioport.c functions?
> >>The overhead of calling it up to six times for the different widths
> >>and read/write would seem negligible as a startup cost. And for
> >>pc87312 we don't seriously have to care about performance imo.
> >>
> >Ideally every ioport registration should be moved to use IORange. I
> >tried to move all isa devices to it, but it is complicated for two
> >reasons. First not every device is qdevified and second some devises
> >can be instantiated as isa device and non-isa device and they do
> >ioport
> >registration in a common code. With your approach you will have to
> >duplicate ioport registration code for isa and non-isa case for such
> >devices and code duplication is not good.
> 
> I'm not trying to duplicate anything! What I've been seeing is that
> many of the ISA devices I've touched in this series first register
> the I/O ports and then associate them with the ISADevice, in
> ISA-only code. So the numbers *are* duplicated and I'm proposing to
> consolidate this into one call.
> 
> The existing "init" would stay, so that code with disseparate ISA
> and common parts (like IDE) remains unaffected.
> 
OK, if you think that it is less of a "mess" this way. In a perfect
world you would have something like:

void init_ioport(DeviceState *dev, IORange *r, const IORangeOps *ops,
                     uint64_t base, uint64_t len)

And it will do correct thing for each device type.

--
			Gleb.

Re: [Qemu-devel] [RFC v4 00/12] ISA reconfigurability v4

[Blue Swirl]

On Wed, Jun 8, 2011 at 9:55 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> Hey,
>
> I've refined the series to track the state in ISADevice and to expose it as VMState.
> Error handling has been improved, and setting the state multiple times is no-op now.
>
> To read the state, I'm introducing support for bool qdev properties.
> Some more qdev_prop_get_*() helpers are introduced, too.
>
> Still need to do some runtime testing, but I'd like to hear if this is getting
> mergeable now, especially wrt VMState.
>
> Andreas
>
>
> Andreas Färber (11):
>  qdev: Add support for property type bool
>  qdev: Add helpers for reading properties
>  isa: Provide set_state callback
>  isa: Allow to un-assign I/O ports
>  isa: Allow to un-associate an IRQ

I like the patches above.

But I think the set_state() interface could be improved. For example,
cpu_register_io_memory() gives an index which is passed to
sysbus_register_mmio(). Then the board can instantiate the device at
desired location without caring about the device internals. With
set_state(), the device does everything.

>  parallel: Implement ISA set_state callback
>  serial: Implement ISA set_state() callback
>  fdc: Implement ISA set_state() callback
>  ide: Allow to discard I/O ports
>  ide: Implement ISA set_state() callback
>  prep: Add pc87312 Super I/O emulation
>
> Hervé Poussineau (1):
>  fdc: Parametrize ISA base, IRQ and DMA
>
>  Makefile.objs                   |    1 +
>  default-configs/ppc-softmmu.mak |    2 +
>  hw/fdc.c                        |   62 ++++--
>  hw/hw.h                         |   15 ++
>  hw/ide/core.c                   |    8 +
>  hw/ide/internal.h               |    1 +
>  hw/ide/isa.c                    |   32 +++-
>  hw/isa-bus.c                    |   57 +++++
>  hw/isa.h                        |    6 +
>  hw/parallel.c                   |   69 ++++--
>  hw/pc87312.c                    |  470 +++++++++++++++++++++++++++++++++++++++
>  hw/qdev-properties.c            |   88 ++++++++
>  hw/qdev.h                       |   13 +
>  hw/serial.c                     |   30 ++-
>  14 files changed, 803 insertions(+), 51 deletions(-)
>  create mode 100644 hw/pc87312.c
>
> --
> 1.7.5.3
>
>
>

Re: [Qemu-devel] [RFC v4 00/12] ISA reconfigurability v4

[Andreas Färber]

Am 13.06.2011 um 22:08 schrieb Blue Swirl:

> On Wed, Jun 8, 2011 at 9:55 PM, Andreas Färber  
> <andreas.faerber@web.de> wrote:
>> I've refined the series to track the state in ISADevice and to  
>> expose it as VMState.
>> Error handling has been improved, and setting the state multiple  
>> times is no-op now.
>>
>> To read the state, I'm introducing support for bool qdev properties.
>> Some more qdev_prop_get_*() helpers are introduced, too.
>>
>> Still need to do some runtime testing, but I'd like to hear if this  
>> is getting
>> mergeable now, especially wrt VMState.
>>
>> Andreas
>>
>>
>> Andreas Färber (11):
>>  qdev: Add support for property type bool
>>  qdev: Add helpers for reading properties
>>  isa: Provide set_state callback
>>  isa: Allow to un-assign I/O ports
>>  isa: Allow to un-associate an IRQ
>
> I like the patches above.
>
> But I think the set_state() interface could be improved. For example,
> cpu_register_io_memory() gives an index which is passed to
> sysbus_register_mmio(). Then the board can instantiate the device at
> desired location without caring about the device internals. With
> set_state(), the device does everything.

Thanks. On IRC, Juan proposed to replace set_state with enable and  
disable, that's what I hope to post together with VMStateSubsections  
tonight, as part of the large PReP series.

Your proposal I don't understand yet. The ioport handlers are device- 
specific, so must be registered from within the device. We discussed  
consolidating that into helpers at ISA level to avoid one call, Gleb  
further suggested converting to IORange. We can't do that  
declaratively since some are conditional.

Andreas

Re: [Qemu-devel] [RFC v4 00/12] ISA reconfigurability v4

[Blue Swirl]

On Tue, Jun 14, 2011 at 12:09 AM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 13.06.2011 um 22:08 schrieb Blue Swirl:
>
>> On Wed, Jun 8, 2011 at 9:55 PM, Andreas Färber <andreas.faerber@web.de>
>> wrote:
>>>
>>> I've refined the series to track the state in ISADevice and to expose it
>>> as VMState.
>>> Error handling has been improved, and setting the state multiple times is
>>> no-op now.
>>>
>>> To read the state, I'm introducing support for bool qdev properties.
>>> Some more qdev_prop_get_*() helpers are introduced, too.
>>>
>>> Still need to do some runtime testing, but I'd like to hear if this is
>>> getting
>>> mergeable now, especially wrt VMState.
>>>
>>> Andreas
>>>
>>>
>>> Andreas Färber (11):
>>>  qdev: Add support for property type bool
>>>  qdev: Add helpers for reading properties
>>>  isa: Provide set_state callback
>>>  isa: Allow to un-assign I/O ports
>>>  isa: Allow to un-associate an IRQ
>>
>> I like the patches above.
>>
>> But I think the set_state() interface could be improved. For example,
>> cpu_register_io_memory() gives an index which is passed to
>> sysbus_register_mmio(). Then the board can instantiate the device at
>> desired location without caring about the device internals. With
>> set_state(), the device does everything.
>
> Thanks. On IRC, Juan proposed to replace set_state with enable and disable,
> that's what I hope to post together with VMStateSubsections tonight, as part
> of the large PReP series.
>
> Your proposal I don't understand yet. The ioport handlers are
> device-specific, so must be registered from within the device. We discussed
> consolidating that into helpers at ISA level to avoid one call, Gleb further
> suggested converting to IORange. We can't do that declaratively since some
> are conditional.

Yes, I would have wanted something declarative. But maybe ISA is special then.