[Qemu-devel] [PATCH 0/6] prep: some fixes and Super I/O emulation

[Qemu-devel] [PATCH 0/6] prep: some fixes and Super I/O emulation

[Hervé Poussineau]

Hi,

First two patches repair some functionality broken since 2009!
Debian install root floppy can now be started again.

Patches 4 to 6 implement the pc87312 Super I/O chip.
Some versions by me or by Andreas Färber have already been sent on
mailing list. This patch has been tested on PReP emulation and on
IBM 40p (not yet committed).

Hervé Poussineau (6):
  i82378/i82374: do not create DMA controller twice
  prep: change default cpu to '7448'
  isa: add isa_bus_from_device() method
  fdc: Parametrize ISA base, IRQ and DMA
  prep: add pc87312 Super I/O emulation
  prep: use pc87312 Super I/O chip instead of collection of random ISA
    devices

 Makefile.objs                   |    1 +
 default-configs/ppc-softmmu.mak |    2 +
 hw/fdc.c                        |   17 +-
 hw/i82374.c                     |    5 +-
 hw/i82378.c                     |    5 +-
 hw/isa.h                        |    5 +
 hw/pc87312.c                    |  425 +++++++++++++++++++++++++++++++++++++++
 hw/ppc_prep.c                   |   52 ++---
 8 files changed, 469 insertions(+), 43 deletions(-)
 create mode 100644 hw/pc87312.c

-- 
1.7.9.1

[Qemu-devel] [PATCH 1/6] i82378/i82374: do not create DMA controller twice

[Hervé Poussineau]

This fixes a crash in PReP emulation when using DMA controller to access floppy drive.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/i82374.c |    5 ++++-
 hw/i82378.c |    5 +++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/i82374.c b/hw/i82374.c
index 67298a3..4a922c3 100644
--- a/hw/i82374.c
+++ b/hw/i82374.c
@@ -38,6 +38,7 @@ do { fprintf(stderr, "i82374 ERROR: " fmt , ## __VA_ARGS__); } while (0)
 
 typedef struct I82374State {
     uint8_t commands[8];
+    qemu_irq out;
 } I82374State;
 
 static const VMStateDescription vmstate_i82374 = {
@@ -99,7 +100,7 @@ static uint32_t i82374_read_descriptor(void *opaque, uint32_t nport)
 
 static void i82374_init(I82374State *s)
 {
-    DMA_init(1, NULL);
+    DMA_init(1, &s->out);
     memset(s->commands, 0, sizeof(s->commands));
 }
 
@@ -132,6 +133,8 @@ static int i82374_isa_init(ISADevice *dev)
 
     i82374_init(s);
 
+    qdev_init_gpio_out(&dev->qdev, &s->out, 1);
+
     return 0;
 }
 
diff --git a/hw/i82378.c b/hw/i82378.c
index faad1a3..9b11d90 100644
--- a/hw/i82378.c
+++ b/hw/i82378.c
@@ -170,6 +170,7 @@ static void i82378_init(DeviceState *dev, I82378State *s)
 {
     ISABus *isabus = DO_UPCAST(ISABus, qbus, qdev_get_child_bus(dev, "isa.0"));
     ISADevice *pit;
+    ISADevice *isa;
     qemu_irq *out0_irq;
 
     /* This device has:
@@ -199,8 +200,8 @@ static void i82378_init(DeviceState *dev, I82378State *s)
     pcspk_init(isabus, pit);
 
     /* 2 82C37 (dma) */
-    DMA_init(1, &s->out[1]);
-    isa_create_simple(isabus, "i82374");
+    isa = isa_create_simple(isabus, "i82374");
+    qdev_connect_gpio_out(&isa->qdev, 0, s->out[1]);
 
     /* timer */
     isa_create_simple(isabus, "mc146818rtc");
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 1/6] i82378/i82374: do not create DMA controller twice

[Andreas Färber]

Am 17.03.2012 15:39, schrieb Hervé Poussineau:
> This fixes a crash in PReP emulation when using DMA controller to access floppy drive.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Reviewed-by: Andreas Färber <afaerber@suse.de>

Andreas

> ---
>  hw/i82374.c |    5 ++++-
>  hw/i82378.c |    5 +++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i82374.c b/hw/i82374.c
> index 67298a3..4a922c3 100644
> --- a/hw/i82374.c
> +++ b/hw/i82374.c
> @@ -38,6 +38,7 @@ do { fprintf(stderr, "i82374 ERROR: " fmt , ## __VA_ARGS__); } while (0)
>  
>  typedef struct I82374State {
>      uint8_t commands[8];
> +    qemu_irq out;
>  } I82374State;
>  
>  static const VMStateDescription vmstate_i82374 = {
> @@ -99,7 +100,7 @@ static uint32_t i82374_read_descriptor(void *opaque, uint32_t nport)
>  
>  static void i82374_init(I82374State *s)
>  {
> -    DMA_init(1, NULL);
> +    DMA_init(1, &s->out);
>      memset(s->commands, 0, sizeof(s->commands));
>  }
>  
> @@ -132,6 +133,8 @@ static int i82374_isa_init(ISADevice *dev)
>  
>      i82374_init(s);
>  
> +    qdev_init_gpio_out(&dev->qdev, &s->out, 1);
> +
>      return 0;
>  }
>  
> diff --git a/hw/i82378.c b/hw/i82378.c
> index faad1a3..9b11d90 100644
> --- a/hw/i82378.c
> +++ b/hw/i82378.c
> @@ -170,6 +170,7 @@ static void i82378_init(DeviceState *dev, I82378State *s)
>  {
>      ISABus *isabus = DO_UPCAST(ISABus, qbus, qdev_get_child_bus(dev, "isa.0"));
>      ISADevice *pit;
> +    ISADevice *isa;
>      qemu_irq *out0_irq;
>  
>      /* This device has:
> @@ -199,8 +200,8 @@ static void i82378_init(DeviceState *dev, I82378State *s)
>      pcspk_init(isabus, pit);
>  
>      /* 2 82C37 (dma) */
> -    DMA_init(1, &s->out[1]);
> -    isa_create_simple(isabus, "i82374");
> +    isa = isa_create_simple(isabus, "i82374");
> +    qdev_connect_gpio_out(&isa->qdev, 0, s->out[1]);
>  
>      /* timer */
>      isa_create_simple(isabus, "mc146818rtc");

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/6] i82378/i82374: do not create DMA controller twice

[Alexander Graf]

On 19.03.2012, at 12:03, Andreas Färber wrote:

> Am 17.03.2012 15:39, schrieb Hervé Poussineau:
>> This fixes a crash in PReP emulation when using DMA controller to access floppy drive.
>> 
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>

Why only reviewed? You're the PREP maintainer and this bug shows with PREP, so I don't see why this shouldn't go through your tree :).


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/6] i82378/i82374: do not create DMA controller twice

[Andreas Färber]

Am 19.03.2012 12:23, schrieb Alexander Graf:
> 
> On 19.03.2012, at 12:03, Andreas Färber wrote:
> 
>> Am 17.03.2012 15:39, schrieb Hervé Poussineau:
>>> This fixes a crash in PReP emulation when using DMA controller to access floppy drive.
>>>
>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>
>> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> Why only reviewed?

Because I didn't test and apply it yet :) before reading through the
whole series and rebasing prep-up branch. I like feedback to my patches,
so I provide some myself (and get journaling in my outbox as a bonus).

> You're the PREP maintainer and this bug shows with PREP, so I don't see why this shouldn't go through your tree :).

Correct apart from the spelling, and no one said it should go through
you or the ppc tree. Relax and have a nice vacation! I never noticed you
flaming Anthony for reviewing patches he is going to apply later on... ;)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/6] i82378/i82374: do not create DMA controller twice

[Alexander Graf]

On 19.03.2012, at 13:19, Andreas Färber wrote:

> Am 19.03.2012 12:23, schrieb Alexander Graf:
>> 
>> On 19.03.2012, at 12:03, Andreas Färber wrote:
>> 
>>> Am 17.03.2012 15:39, schrieb Hervé Poussineau:
>>>> This fixes a crash in PReP emulation when using DMA controller to access floppy drive.
>>>> 
>>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>> 
>>> Reviewed-by: Andreas Färber <afaerber@suse.de>
>> 
>> Why only reviewed?
> 
> Because I didn't test and apply it yet :) before reading through the
> whole series and rebasing prep-up branch. I like feedback to my patches,
> so I provide some myself (and get journaling in my outbox as a bonus).
> 
>> You're the PREP maintainer and this bug shows with PREP, so I don't see why this shouldn't go through your tree :).
> 
> Correct apart from the spelling, and no one said it should go through
> you or the ppc tree. Relax and have a nice vacation! I never noticed you
> flaming Anthony for reviewing patches he is going to apply later on... ;)

Oh, sorry, it sounded like "Reviewed, someone else please apply it". I didn't think of the case where you'd pick it up yourself later :).


Alex

[Qemu-devel] [PATCH 2/6] prep: change default cpu to '7448'

[Hervé Poussineau]

In b37fc148bb81b5b022846156a6cba266e6c23962, PReP default cpu was changed from
'default' to '602'. However, '7448' is closer of 'default' than '602'.

This repairs following command line, which is available in some tutorials:
qemu-system-ppc -kernel zImage.prep -fda debian_install_root.bin -M prep

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/ppc_prep.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 06d589d..19a674f 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -517,7 +517,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
 
     /* init CPUs */
     if (cpu_model == NULL)
-        cpu_model = "602";
+        cpu_model = "7448";
     for (i = 0; i < smp_cpus; i++) {
         env = cpu_init(cpu_model);
         if (!env) {
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 2/6] prep: change default cpu to '7448'

[Andreas Färber]

Am 17.03.2012 15:39, schrieb Hervé Poussineau:
> In b37fc148bb81b5b022846156a6cba266e6c23962, PReP default cpu was changed from
> 'default' to '602'. However, '7448' is closer of 'default' than '602'.
> 
> This repairs following command line, which is available in some tutorials:
> qemu-system-ppc -kernel zImage.prep -fda debian_install_root.bin -M prep
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Weak nack. I guess this depends more on the image than on the command
line. If at this point we change the default CPU for -M prep, we would
need to start creating prep-1.0 etc. which I would like to avoid as long
as possible...

Did you happen to found out which physical machine 'prep' corresponds
to? If so, what CPU did it have?
The Sandalfoot supposedly had a 601, the BeBox a dual 603.

Andreas

> ---
>  hw/ppc_prep.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 06d589d..19a674f 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -517,7 +517,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
>  
>      /* init CPUs */
>      if (cpu_model == NULL)
> -        cpu_model = "602";
> +        cpu_model = "7448";
>      for (i = 0; i < smp_cpus; i++) {
>          env = cpu_init(cpu_model);
>          if (!env) {

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 2/6] prep: change default cpu to '7448'

[Hervé Poussineau]

Andreas Färber a écrit :
> Am 17.03.2012 15:39, schrieb Hervé Poussineau:
>> In b37fc148bb81b5b022846156a6cba266e6c23962, PReP default cpu was changed from
>> 'default' to '602'. However, '7448' is closer of 'default' than '602'.
>>
>> This repairs following command line, which is available in some tutorials:
>> qemu-system-ppc -kernel zImage.prep -fda debian_install_root.bin -M prep
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> 
> Weak nack. I guess this depends more on the image than on the command
> line. If at this point we change the default CPU for -M prep, we would
> need to start creating prep-1.0 etc. which I would like to avoid as long
> as possible...

I'm using floppy images available at
http://archive.debian.org/debian/dists/woody/main/disks-powerpc/current/prep/images-1.44/
more specifically root.bin
The default cpu has already been changed once between 0.11 and 0.12, so 
I think it's not impossible to change it once more.
Moreover, I don't think prep emulation is used in any production 
environment...

> 
> Did you happen to found out which physical machine 'prep' corresponds
> to? If so, what CPU did it have?
It seems that 'prep' emulates a Motorola, maybe the 
PowerStack/BlackHack, but I'm not sure...

> The Sandalfoot supposedly had a 601, the BeBox a dual 603.

With 601, Open Hack'Ware doesn't work.
With 602, the Debian installer doesn't start.
With 603, the Debian installer starts.
With 604, Open Hack'Ware doesn't work.

Will you accept a patch to change '602' to '603'?

Hervé

Re: [Qemu-devel] [PATCH 2/6] prep: change default cpu to '7448'

[Hervé Poussineau]

Andreas Färber a écrit :
> Am 17.03.2012 15:39, schrieb Hervé Poussineau:
>> In b37fc148bb81b5b022846156a6cba266e6c23962, PReP default cpu was changed from
>> 'default' to '602'. However, '7448' is closer of 'default' than '602'.
>>
>> This repairs following command line, which is available in some tutorials:
>> qemu-system-ppc -kernel zImage.prep -fda debian_install_root.bin -M prep
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> 
> Weak nack. I guess this depends more on the image than on the command
> line. If at this point we change the default CPU for -M prep, we would
> need to start creating prep-1.0 etc. which I would like to avoid as long
> as possible...

I'm using floppy images available at
http://archive.debian.org/debian/dists/woody/main/disks-powerpc/current/prep/images-1.44/
more specifically root.bin
The default cpu has already been changed once between 0.11 and 0.12, so 
I think it's not impossible to change it once more.
Moreover, I'm not sure PReP emulation is used in any real environment...

> 
> Did you happen to found out which physical machine 'prep' corresponds
> to? If so, what CPU did it have?
It seems that 'prep' emulates a Motorola, maybe the 
PowerStack/BlackHack, but I'm not sure...

> The Sandalfoot supposedly had a 601, the BeBox a dual 603.

With 601, Open Hack'Ware doesn't work.
With 602, the Debian installer doesn't start.
With 603, the Debian installer starts.
With 604, Open Hack'Ware doesn't work.

Will you accept a patch to change '602' to '603'?

Hervé

[Qemu-devel] [PATCH 3/6] isa: add isa_bus_from_device() method

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/isa.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/isa.h b/hw/isa.h
index 40373fb..f7bc4b5 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -76,6 +76,11 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start,
                               const MemoryRegionPortio *portio,
                               void *opaque, const char *name);
 
+static inline ISABus *isa_bus_from_device(ISADevice *d)
+{
+    return DO_UPCAST(ISABus, qbus, d->qdev.parent_bus);
+}
+
 extern target_phys_addr_t isa_mem_base;
 
 void isa_mmio_setup(MemoryRegion *mr, target_phys_addr_t size);
-- 
1.7.9.1

[Qemu-devel] [PATCH 4/6] fdc: Parametrize ISA base, IRQ and DMA

[Hervé Poussineau]

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 |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..756d4ce 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -438,6 +438,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;
@@ -1971,17 +1974,14 @@ 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;
 
-    isa_register_portio_list(dev, iobase, fdc_portio_list, fdctrl, "fdc");
+    isa_register_portio_list(dev, isa->iobase, fdc_portio_list, fdctrl, "fdc");
 
-    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");
@@ -2046,6 +2046,9 @@ static const VMStateDescription vmstate_isa_fdc ={
 };
 
 static Property isa_fdc_properties[] = {
+    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.9.1

Re: [Qemu-devel] [PATCH 4/6] fdc: Parametrize ISA base, IRQ and DMA

[Markus Armbruster]

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

> 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>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

[Qemu-devel] [PATCH 5/6] prep: add pc87312 Super I/O emulation

[Hervé Poussineau]

This provides floppy and IDE controllers as well as serial and parallel ports.
However, dynamic configuration of devices is not yet supported.

Cc: Andreas Färber <andreas.faerber@web.de>
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 Makefile.objs |    1 +
 hw/pc87312.c  |  425 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 426 insertions(+), 0 deletions(-)
 create mode 100644 hw/pc87312.c

diff --git a/Makefile.objs b/Makefile.objs
index 226b01d..232eed0 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -252,6 +252,7 @@ hw-obj-$(CONFIG_I8259) += i8259_common.o i8259.o
 # PPC devices
 hw-obj-$(CONFIG_PREP_PCI) += prep_pci.o
 hw-obj-$(CONFIG_I82378) += i82378.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/hw/pc87312.c b/hw/pc87312.c
new file mode 100644
index 0000000..1e28dbd
--- /dev/null
+++ b/hw/pc87312.c
@@ -0,0 +1,425 @@
+/*
+ * QEMU National Semiconductor PC87312 (Super I/O)
+ *
+ * Copyright (c) 2010-2012 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 {
+    ISADevice dev;
+    uint32_t iobase;
+
+    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;
+}
+
+
+/* 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);
+}
+
+
+/* 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;
+}
+
+
+/* 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 reconfigure_devices(PC87312State *s)
+{
+    error_report("pc87312: unsupported device reconfiguration (%02x %02x %02x)",
+                 s->FER, s->FAR, s->PTR);
+}
+
+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;
+            reconfigure_devices(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 int pc87312_post_load(void *opaque, int version_id)
+{
+    PC87312State *s = opaque;
+    reconfigure_devices(s);
+    return 0;
+}
+
+static void pc87312_reset(DeviceState *d)
+{
+    PC87312State *s = container_of(d, PC87312State, dev.qdev);
+    pc87312_soft_reset(s);
+}
+
+static int pc87312_init(ISADevice *dev)
+{
+    PC87312State *s;
+    ISADevice *isa;
+    ISABus *bus;
+    CharDriverState *chr;
+    BlockDriverState *bs;
+    int i;
+
+    s = DO_UPCAST(PC87312State, dev, dev);
+    bus = isa_bus_from_device(dev);
+    pc87312_hard_reset(s);
+
+    chr = s->parallel.chr;
+    if (s->parallel.chr != NULL && is_parallel_enabled(s)) {
+        qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL); /* HACK */
+        isa = isa_create(bus, "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", chr);
+        qdev_init_nofail(&isa->qdev);
+        s->parallel.dev = &isa->qdev;
+        DPRINTF("parallel: base 0x%x, irq %u\n",
+                get_parallel_iobase(s), get_parallel_irq(s));
+    }
+
+    for (i = 0; i < 2; i++) {
+        chr = s->uart[i].chr;
+        if (chr != NULL && is_uart_enabled(s, i)) {
+            qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL); /* HACK */
+            isa = isa_create(bus, "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", chr);
+            qdev_init_nofail(&isa->qdev);
+            s->uart[i].dev = &isa->qdev;
+            DPRINTF("uart%d: base 0x%x, irq %u\n", i,
+                    get_uart_iobase(s, i),
+                    get_uart_irq(s, i));
+        }
+    }
+
+    if (is_fdc_enabled(s)) {
+        isa = isa_create(bus, "isa-fdc");
+        qdev_prop_set_uint32(&isa->qdev, "iobase", get_fdc_iobase(s));
+        qdev_prop_set_uint32(&isa->qdev, "irq", 6);
+        bs = s->fdc.drive[0];
+        if (bs != NULL) {
+            bdrv_detach_dev(bs, bdrv_get_attached_dev(bs)); /* HACK */
+            qdev_prop_set_drive_nofail(&isa->qdev, "driveA", bs);
+        }
+        bs = s->fdc.drive[1];
+        if (bs != NULL) {
+            bdrv_detach_dev(bs, bdrv_get_attached_dev(bs)); /* HACK */
+            qdev_prop_set_drive_nofail(&isa->qdev, "driveB", bs);
+        }
+        qdev_init_nofail(&isa->qdev);
+        s->fdc.dev = &isa->qdev;
+        DPRINTF("fdc: base 0x%x\n", get_fdc_iobase(s));
+    }
+
+    if (is_ide_enabled(s)) {
+        isa = isa_create(bus, "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;
+        DPRINTF("ide: base 0x%x\n", get_ide_iobase(s));
+    }
+
+    register_ioport_write(s->iobase, 2, 1, pc87312_ioport_write, s);
+    register_ioport_read(s->iobase, 2, 1, pc87312_ioport_read, s);
+    return 0;
+}
+
+static const VMStateDescription vmstate_pc87312 = {
+    .name = "pc87312",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = pc87312_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(read_id_step, PC87312State),
+        VMSTATE_UINT8(selected_index, PC87312State),
+        VMSTATE_UINT8_ARRAY(regs, PC87312State, 3),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property pc87312_properties[] = {
+    DEFINE_PROP_HEX32("iobase", PC87312State, iobase, 0x398),
+    DEFINE_PROP_UINT8("config", PC87312State, config, 1),
+    DEFINE_PROP_CHR("parallel", PC87312State, parallel.chr),
+    DEFINE_PROP_CHR("uart1", PC87312State, uart[0].chr),
+    DEFINE_PROP_CHR("uart2", PC87312State, uart[1].chr),
+    DEFINE_PROP_DRIVE("floppyA", PC87312State, fdc.drive[0]),
+    DEFINE_PROP_DRIVE("floppyB", PC87312State, fdc.drive[1]),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void pc87312_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
+
+    ic->init = pc87312_init;
+    dc->reset = pc87312_reset;
+    dc->vmsd = &vmstate_pc87312;
+    dc->props = pc87312_properties;
+}
+
+static TypeInfo pc87312_info = {
+    .name          = "pc87312",
+    .parent        = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(PC87312State),
+    .class_init    = pc87312_class_initfn,
+};
+
+static void pc87312_register_types(void)
+{
+    type_register_static(&pc87312_info);
+}
+
+type_init(pc87312_register_types)
+
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 5/6] prep: add pc87312 Super I/O emulation

[Andreas Färber]

Am 17.03.2012 15:39, schrieb Hervé Poussineau:
> This provides floppy and IDE controllers as well as serial and parallel ports.
> However, dynamic configuration of devices is not yet supported.
> 
> Cc: Andreas Färber <andreas.faerber@web.de>
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  Makefile.objs |    1 +
>  hw/pc87312.c  |  425 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 426 insertions(+), 0 deletions(-)
>  create mode 100644 hw/pc87312.c

> diff --git a/hw/pc87312.c b/hw/pc87312.c
> new file mode 100644
> index 0000000..1e28dbd
> --- /dev/null
> +++ b/hw/pc87312.c
> @@ -0,0 +1,425 @@
> +/*
> + * QEMU National Semiconductor PC87312 (Super I/O)
> + *
> + * Copyright (c) 2010-2012 Herve Poussineau

FWIW mind to add

Copyright (c) 2011 Andreas Färber

?

> + *
> + * 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.
> + */

> +//#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

Mid-term we should replace this through proper tracing.

> +static int pc87312_init(ISADevice *dev)
> +{
> +    PC87312State *s;
> +    ISADevice *isa;
> +    ISABus *bus;
> +    CharDriverState *chr;
> +    BlockDriverState *bs;
> +    int i;
> +
> +    s = DO_UPCAST(PC87312State, dev, dev);
> +    bus = isa_bus_from_device(dev);
> +    pc87312_hard_reset(s);
> +
> +    chr = s->parallel.chr;
> +    if (s->parallel.chr != NULL && is_parallel_enabled(s)) {

This logic still seems to be flawed: it should not depend on
s->parallel.chr. If that is NULL we need to create a null char device to
match the device that's present in hardware according to
is_parallel_enabled(s).

> +        qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL); /* HACK */

HACK alarm in [PATCH]: What for?

> +        isa = isa_create(bus, "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", chr);
> +        qdev_init_nofail(&isa->qdev);
> +        s->parallel.dev = &isa->qdev;
> +        DPRINTF("parallel: base 0x%x, irq %u\n",
> +                get_parallel_iobase(s), get_parallel_irq(s));
> +    }
> +
> +    for (i = 0; i < 2; i++) {
> +        chr = s->uart[i].chr;

> +        if (chr != NULL && is_uart_enabled(s, i)) {
> +            qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL); /* HACK */

2x ditto.

> +            isa = isa_create(bus, "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", chr);
> +            qdev_init_nofail(&isa->qdev);
> +            s->uart[i].dev = &isa->qdev;
> +            DPRINTF("uart%d: base 0x%x, irq %u\n", i,
> +                    get_uart_iobase(s, i),
> +                    get_uart_irq(s, i));
> +        }
> +    }
> +
> +    if (is_fdc_enabled(s)) {
> +        isa = isa_create(bus, "isa-fdc");
> +        qdev_prop_set_uint32(&isa->qdev, "iobase", get_fdc_iobase(s));
> +        qdev_prop_set_uint32(&isa->qdev, "irq", 6);
> +        bs = s->fdc.drive[0];
> +        if (bs != NULL) {
> +            bdrv_detach_dev(bs, bdrv_get_attached_dev(bs)); /* HACK */
> +            qdev_prop_set_drive_nofail(&isa->qdev, "driveA", bs);
> +        }
> +        bs = s->fdc.drive[1];
> +        if (bs != NULL) {
> +            bdrv_detach_dev(bs, bdrv_get_attached_dev(bs)); /* HACK */
> +            qdev_prop_set_drive_nofail(&isa->qdev, "driveB", bs);
> +        }
> +        qdev_init_nofail(&isa->qdev);
> +        s->fdc.dev = &isa->qdev;
> +        DPRINTF("fdc: base 0x%x\n", get_fdc_iobase(s));
> +    }
> +
> +    if (is_ide_enabled(s)) {
> +        isa = isa_create(bus, "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;
> +        DPRINTF("ide: base 0x%x\n", get_ide_iobase(s));
> +    }
> +
> +    register_ioport_write(s->iobase, 2, 1, pc87312_ioport_write, s);
> +    register_ioport_read(s->iobase, 2, 1, pc87312_ioport_read, s);
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_pc87312 = {
> +    .name = "pc87312",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .post_load = pc87312_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(read_id_step, PC87312State),
> +        VMSTATE_UINT8(selected_index, PC87312State),
> +        VMSTATE_UINT8_ARRAY(regs, PC87312State, 3),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static Property pc87312_properties[] = {
> +    DEFINE_PROP_HEX32("iobase", PC87312State, iobase, 0x398),
> +    DEFINE_PROP_UINT8("config", PC87312State, config, 1),
> +    DEFINE_PROP_CHR("parallel", PC87312State, parallel.chr),
> +    DEFINE_PROP_CHR("uart1", PC87312State, uart[0].chr),
> +    DEFINE_PROP_CHR("uart2", PC87312State, uart[1].chr),
> +    DEFINE_PROP_DRIVE("floppyA", PC87312State, fdc.drive[0]),
> +    DEFINE_PROP_DRIVE("floppyB", PC87312State, fdc.drive[1]),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void pc87312_class_initfn(ObjectClass *klass, void *data)

I always thought initfn was used for instances...

> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
> +
> +    ic->init = pc87312_init;
> +    dc->reset = pc87312_reset;
> +    dc->vmsd = &vmstate_pc87312;
> +    dc->props = pc87312_properties;
> +}
> +
> +static TypeInfo pc87312_info = {
> +    .name          = "pc87312",
> +    .parent        = TYPE_ISA_DEVICE,
> +    .instance_size = sizeof(PC87312State),
> +    .class_init    = pc87312_class_initfn,
> +};
> +
> +static void pc87312_register_types(void)
> +{
> +    type_register_static(&pc87312_info);
> +}
> +
> +type_init(pc87312_register_types)
> +

Trailing empty line.

So what about the ugly ISA hot-plug issue that originally stalled our
two series? I'm missing a Change Log about that. You changed the initial
configuration to the one used by 40P firmware IIRC and stopped
supporting the chipset's reconfiguration? Either way any limitation of
this implementation should be prominently documented please.

Thanks for your work on this,

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 5/6] prep: add pc87312 Super I/O emulation

[Hervé Poussineau]

Andreas Färber a écrit :
> Am 17.03.2012 15:39, schrieb Hervé Poussineau:
>> This provides floppy and IDE controllers as well as serial and parallel ports.
>> However, dynamic configuration of devices is not yet supported.
>>
>> Cc: Andreas Färber <andreas.faerber@web.de>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>  Makefile.objs |    1 +
>>  hw/pc87312.c  |  425 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 426 insertions(+), 0 deletions(-)
>>  create mode 100644 hw/pc87312.c
> 
>> diff --git a/hw/pc87312.c b/hw/pc87312.c
>> new file mode 100644
>> index 0000000..1e28dbd
>> --- /dev/null
>> +++ b/hw/pc87312.c
>> @@ -0,0 +1,425 @@
>> +/*
>> + * QEMU National Semiconductor PC87312 (Super I/O)
>> + *
>> + * Copyright (c) 2010-2012 Herve Poussineau
> 
> FWIW mind to add
> 
> Copyright (c) 2011 Andreas Färber
> 
> ?

Yes, of course. Sorry about that.

>> +
>> +    chr = s->parallel.chr;
>> +    if (s->parallel.chr != NULL && is_parallel_enabled(s)) {
> 
> This logic still seems to be flawed: it should not depend on
> s->parallel.chr. If that is NULL we need to create a null char device to
> match the device that's present in hardware according to
> is_parallel_enabled(s).

Ok, will remove the 'chr != NULL' check.

> 
>> +        qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL); /* HACK */
> 
> HACK alarm in [PATCH]: What for?

The problem is composition. Main board contains a pc87312, which 
contains a isa-parallel. Main board doesn't know the isa-parallel device.

isa-parallel device must have a chardev (set by a property). pc87312 
creates the isa-parallel device, so it must know the chardev. I did it 
also with a chardev property.
Unfortunately, a chardev can only be used once, and I have not found a 
better way to give chardev from main board to superI/O to parallel.
Do you have any better idea?


>> +    for (i = 0; i < 2; i++) {
>> +        chr = s->uart[i].chr;
> 
>> +        if (chr != NULL && is_uart_enabled(s, i)) {
>> +            qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL); /* HACK */
> 
> 2x ditto.

Same answer :)

>> +
>> +static void pc87312_class_initfn(ObjectClass *klass, void *data)
> 
> I always thought initfn was used for instances...

Ok, will change to pc87312_class_init

>> +
>> +type_init(pc87312_register_types)
>> +
> 
> Trailing empty line.

Ok, will remove.

> 
> So what about the ugly ISA hot-plug issue that originally stalled our
> two series? I'm missing a Change Log about that. You changed the initial
> configuration to the one used by 40P firmware IIRC and stopped
> supporting the chipset's reconfiguration? Either way any limitation of
> this implementation should be prominently documented please.

Yes, chipset reconfiguration required ISA hot-plug issue, so I didn't 
want to wait for this serie. In commit message, I wrote "However, 
dynamic configuration of devices is not yet supported.", and you get an 
error message at each reconfiguration:

+static void reconfigure_devices(PC87312State *s)
+{
+    error_report("pc87312: unsupported device reconfiguration (%02x 
%02x %02x)",
+                 s->FER, s->FAR, s->PTR);
+}

What else should I add?
Anyway, if/when dynamic reconfiguration will be implemented, only this 
method will be changed.


About the initial configuration, it is controlled by "config" property.
In patch 6/6, when I use the Super I/O chip in 'prep' machine, I set 
this property to 13, which means fdc + serial0 + serial1 + parallel0.
For IBM 40p, the initial configuration should be 15 (fdc + ide + serial0 
+ parallel0).
So I don't think the Super I/O is exclusively tied to IBM 40p.


> 
> Thanks for your work on this,

Thanks

Hervé

[Qemu-devel] [PATCH 6/6] prep: use pc87312 Super I/O chip instead of collection of random ISA devices

[Hervé Poussineau]

We can't however replace the built-in IDE controller, as the one in pc87312
is only single-channel and can use only IRQ 14.

Cc: Andreas Färber <andreas.faerber@web.de>
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 default-configs/ppc-softmmu.mak |    2 +
 hw/ppc_prep.c                   |   50 ++++++++++++++-------------------------
 2 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 1a768fc..14568b3 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -8,6 +8,7 @@ CONFIG_M48T59=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
 CONFIG_SERIAL=y
+CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
 CONFIG_FDC=y
@@ -16,6 +17,7 @@ CONFIG_I82374=y
 CONFIG_OPENPIC=y
 CONFIG_PREP_PCI=y
 CONFIG_I82378=y
+CONFIG_PC87312=y
 CONFIG_MACIO=y
 CONFIG_PCSPK=y
 CONFIG_CUDA=y
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 19a674f..acb7cc5 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -211,7 +211,6 @@ typedef struct sysctrl_t {
     M48t59State *nvram;
     uint8_t state;
     uint8_t syscontrol;
-    uint8_t fake_io[2];
     int contiguous_map;
     int endian;
 } sysctrl_t;
@@ -222,24 +221,6 @@ enum {
 
 static sysctrl_t *sysctrl;
 
-static void PREP_io_write (void *opaque, uint32_t addr, uint32_t val)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    PPC_IO_DPRINTF("0x%08" PRIx32 " => 0x%02" PRIx32 "\n", addr - PPC_IO_BASE,
-                   val);
-    sysctrl->fake_io[addr - 0x0398] = val;
-}
-
-static uint32_t PREP_io_read (void *opaque, uint32_t addr)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    PPC_IO_DPRINTF("0x%08" PRIx32 " <= 0x%02" PRIx32 "\n", addr - PPC_IO_BASE,
-                   sysctrl->fake_io[addr - 0x0398]);
-    return sysctrl->fake_io[addr - 0x0398];
-}
-
 static void PREP_io_800_writeb (void *opaque, uint32_t addr, uint32_t val)
 {
     sysctrl_t *sysctrl = opaque;
@@ -506,10 +487,11 @@ static void ppc_prep_init (ram_addr_t ram_size,
     PCIBus *pci_bus;
     PCIDevice *pci;
     ISABus *isa_bus;
+    ISADevice *isa;
     qemu_irq *cpu_exit_irq;
     int ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    DriveInfo *fd[MAX_FD];
+    DriveInfo *drive;
 
     sysctrl = g_malloc0(sizeof(sysctrl_t));
 
@@ -635,6 +617,22 @@ static void ppc_prep_init (ram_addr_t ram_size,
     sysbus_connect_irq(&pcihost->busdev, 3, qdev_get_gpio_in(&pci->qdev, 11));
     isa_bus = DO_UPCAST(ISABus, qbus, qdev_get_child_bus(&pci->qdev, "isa.0"));
 
+    /* Super I/O (parallel + serial ports) */
+    isa = isa_create(isa_bus, "pc87312");
+    qdev_prop_set_uint8(&isa->qdev, "config", 13); /* fdc, ser0, ser1, par0 */
+    qdev_prop_set_chr(&isa->qdev, "parallel", parallel_hds[0]);
+    qdev_prop_set_chr(&isa->qdev, "uart1", serial_hds[0]);
+    qdev_prop_set_chr(&isa->qdev, "uart2", serial_hds[1]);
+    drive = drive_get(IF_FLOPPY, 0, 0);
+    if (drive) {
+        qdev_prop_set_drive_nofail(&isa->qdev, "floppyA", drive->bdrv);
+    }
+    drive = drive_get(IF_FLOPPY, 0, 1);
+    if (drive) {
+        qdev_prop_set_drive_nofail(&isa->qdev, "floppyB", drive->bdrv);
+    }
+    qdev_init_nofail(&isa->qdev);
+
     /* Register 8 MB of ISA IO space (needed for non-contiguous map) */
     memory_region_init_io(PPC_io_memory, &PPC_prep_io_ops, sysctrl,
                           "ppc-io", 0x00800000);
@@ -643,8 +641,6 @@ static void ppc_prep_init (ram_addr_t ram_size,
     /* init basic PC hardware */
     pci_vga_init(pci_bus);
 
-    if (serial_hds[0])
-        serial_isa_init(isa_bus, 0, serial_hds[0]);
     nb_nics1 = nb_nics;
     if (nb_nics1 > NE2000_NB_MAX)
         nb_nics1 = NE2000_NB_MAX;
@@ -668,17 +664,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
     }
     isa_create_simple(isa_bus, "i8042");
 
-    //    SB16_init();
-
-    for(i = 0; i < MAX_FD; i++) {
-        fd[i] = drive_get(IF_FLOPPY, 0, i);
-    }
-    fdctrl_init_isa(isa_bus, fd);
-
-    /* Register fake IO ports for PREP */
     sysctrl->reset_irq = first_cpu->irq_inputs[PPC6xx_INPUT_HRESET];
-    register_ioport_read(0x398, 2, 1, &PREP_io_read, sysctrl);
-    register_ioport_write(0x398, 2, 1, &PREP_io_write, sysctrl);
     /* System control ports */
     register_ioport_read(0x0092, 0x01, 1, &PREP_io_800_readb, sysctrl);
     register_ioport_write(0x0092, 0x01, 1, &PREP_io_800_writeb, sysctrl);
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 6/6] prep: use pc87312 Super I/O chip instead of collection of random ISA devices

[Paolo Bonzini]

Il 17/03/2012 15:39, Hervé Poussineau ha scritto:
> +    qdev_prop_set_chr(&isa->qdev, "parallel", parallel_hds[0]);
> +    qdev_prop_set_chr(&isa->qdev, "uart1", serial_hds[0]);
> +    qdev_prop_set_chr(&isa->qdev, "uart2", serial_hds[1]);

Set these conditionally on *_hds[] being non-NULL, so that you can use
-chardev/-global on the command-line as an alternative to -serial/-parallel.

Paolo

Re: [Qemu-devel] [PATCH 0/6] prep: some fixes and Super I/O emulation

[Artyom Tarasenko]

On 3/17/12, Hervé Poussineau <hpoussin@reactos.org> wrote:
> First two patches repair some functionality broken since 2009!
> Debian install root floppy can now be started again.

Can you give the qemu command line for this?

> Patches 4 to 6 implement the pc87312 Super I/O chip.
> Some versions by me or by Andreas Färber have already been sent on
> mailing list. This patch has been tested on PReP emulation and on
> IBM 40p (not yet committed).
>
> Hervé Poussineau (6):
>   i82378/i82374: do not create DMA controller twice
>   prep: change default cpu to '7448'
>   isa: add isa_bus_from_device() method
>   fdc: Parametrize ISA base, IRQ and DMA
>   prep: add pc87312 Super I/O emulation
>   prep: use pc87312 Super I/O chip instead of collection of random ISA
>     devices


-- 
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 0/6] prep: some fixes and Super I/O emulation

[Hervé Poussineau]

Hi,

Artyom Tarasenko a écrit :
> On 3/17/12, Hervé Poussineau <hpoussin@reactos.org> wrote:
>> First two patches repair some functionality broken since 2009!
>> Debian install root floppy can now be started again.
> 
> Can you give the qemu command line for this?

The command line is
qemu-system-ppc -kernel zImage.prep -fda debian_install_root.bin -M prep

You can find kernel and floppy image in archive 
http://www.h6.dion.ne.jp/~kazuw/qemu-win/qemu-0.9.0-ppc.zip

You can find other floppy images required for install at
http://archive.debian.org/debian/dists/woody/main/disks-powerpc/current/prep/images-1.44/

> 
>> Patches 4 to 6 implement the pc87312 Super I/O chip.
>> Some versions by me or by Andreas Färber have already been sent on
>> mailing list. This patch has been tested on PReP emulation and on
>> IBM 40p (not yet committed).
>>
>> Hervé Poussineau (6):
>>   i82378/i82374: do not create DMA controller twice
>>   prep: change default cpu to '7448'
>>   isa: add isa_bus_from_device() method
>>   fdc: Parametrize ISA base, IRQ and DMA
>>   prep: add pc87312 Super I/O emulation
>>   prep: use pc87312 Super I/O chip instead of collection of random ISA
>>     devices
> 
> 

Hervé