[PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[Philippe Mathieu-Daudé]

The motivation behind this series is to remove the
isa_get_irq(NULL) call to simplify the ISA generic model.

Since v1:
- rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504

Philippe Mathieu-Daudé (6):
  hw/isa/vt82c686: Name output IRQ as 'intr'
  hw/isa/vt82c686: Simplify removing unuseful qemu_allocate_irqs() call
  hw/isa/vt82c686: Let ISA function expose ISA IRQs
  hw/ide/via: Replace magic 2 value by ARRAY_SIZE / MAX_IDE_DEVS
  hw/ide/via: Connect IDE function output IRQs to the ISA function input
  hw/southbridge/vt82c686: Introduce VT82C686B_SOUTHBRIDGE

 hw/ide/via.c               |  31 ++++++++---
 hw/isa/vt82c686.c          |  27 +++++-----
 hw/mips/fuloong2e.c        |  35 +++---------
 hw/southbridge/vt82c686.c  | 107 +++++++++++++++++++++++++++++++++++++
 MAINTAINERS                |   1 +
 hw/Kconfig                 |   1 +
 hw/isa/Kconfig             |   9 ----
 hw/meson.build             |   1 +
 hw/southbridge/Kconfig     |   8 +++
 hw/southbridge/meson.build |   1 +
 10 files changed, 164 insertions(+), 57 deletions(-)
 create mode 100644 hw/southbridge/vt82c686.c
 create mode 100644 hw/southbridge/Kconfig
 create mode 100644 hw/southbridge/meson.build

-- 
2.26.3

[PATCH v2 1/6] hw/isa/vt82c686: Name output IRQ as 'intr'

[Philippe Mathieu-Daudé]

Named IRQs are easier to understand in the monitor.
Name the single output interrupt as 'intr'.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/isa/vt82c686.c   | 2 +-
 hw/mips/fuloong2e.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index f57f3e70679..b177016125d 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -615,7 +615,7 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
     qemu_irq *isa_irq;
     int i;
 
-    qdev_init_gpio_out(dev, &s->cpu_intr, 1);
+    qdev_init_gpio_out_named(dev, &s->cpu_intr, "intr", 1);
     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     isa_bus = isa_bus_new(dev, get_system_memory(), pci_address_space_io(d),
                           &error_fatal);
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index c1b8066a13b..3e7a7e4389d 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -204,7 +204,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
 
     dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
                                           TYPE_VT82C686B_ISA);
-    qdev_connect_gpio_out(DEVICE(dev), 0, intc);
+    qdev_connect_gpio_out_named(DEVICE(dev), "intr", 0, intc);
 
     dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
     pci_ide_create_devs(dev);
-- 
2.26.3

[PATCH v2 2/6] hw/isa/vt82c686: Simplify removing unuseful qemu_allocate_irqs() call

[Philippe Mathieu-Daudé]

Instead of creating an input IRQ with qemu_allocate_irqs()
to pass it as output IRQ of the PIC, with its handler simply
dispatching into the "intr" output IRQ, simplify by directly
connecting the PIC to the "intr" named output.

Fixes: 3dc31cb8490 ("vt82c686: Move creation of ISA devices to the ISA bridge")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/isa/vt82c686.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index b177016125d..2ce5c6cbb00 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -566,12 +566,6 @@ static const TypeInfo via_isa_info = {
     },
 };
 
-static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
-{
-    ViaISAState *s = opaque;
-    qemu_set_irq(s->cpu_intr, level);
-}
-
 /* TYPE_VT82C686B_ISA */
 
 static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
@@ -612,14 +606,12 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
     ViaISAState *s = VIA_ISA(d);
     DeviceState *dev = DEVICE(d);
     ISABus *isa_bus;
-    qemu_irq *isa_irq;
     int i;
 
     qdev_init_gpio_out_named(dev, &s->cpu_intr, "intr", 1);
-    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     isa_bus = isa_bus_new(dev, get_system_memory(), pci_address_space_io(d),
                           &error_fatal);
-    isa_bus_irqs(isa_bus, i8259_init(isa_bus, *isa_irq));
+    isa_bus_irqs(isa_bus, i8259_init(isa_bus, s->cpu_intr));
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
     i8257_dma_init(isa_bus, 0);
     s->via_sio = VIA_SUPERIO(isa_create_simple(isa_bus,
@@ -693,14 +685,12 @@ static void vt8231_realize(PCIDevice *d, Error **errp)
     ViaISAState *s = VIA_ISA(d);
     DeviceState *dev = DEVICE(d);
     ISABus *isa_bus;
-    qemu_irq *isa_irq;
     int i;
 
     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
-    isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     isa_bus = isa_bus_new(dev, get_system_memory(), pci_address_space_io(d),
                           &error_fatal);
-    isa_bus_irqs(isa_bus, i8259_init(isa_bus, *isa_irq));
+    isa_bus_irqs(isa_bus, i8259_init(isa_bus, s->cpu_intr));
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
     i8257_dma_init(isa_bus, 0);
     s->via_sio = VIA_SUPERIO(isa_create_simple(isa_bus, TYPE_VT8231_SUPERIO));
-- 
2.26.3

[PATCH v2 3/6] hw/isa/vt82c686: Let ISA function expose ISA IRQs

[Philippe Mathieu-Daudé]

The 2 cascaded 8259 PIC are managed by the PCI function #0
(ISA bridge). Expose the 16 IRQs on this function, so other
functions from the same chipset can access them.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/isa/vt82c686.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 2ce5c6cbb00..388833f4eb1 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -542,6 +542,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(ViaISAState, VIA_ISA)
 struct ViaISAState {
     PCIDevice dev;
     qemu_irq cpu_intr;
+    qemu_irq *pic_irq;
     ViaSuperIOState *via_sio;
 };
 
@@ -601,6 +602,13 @@ static void vt82c686b_isa_reset(DeviceState *dev)
     pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
 }
 
+static void vt82c686b_isa_irq(void *opaque, int irq, int level)
+{
+    ViaISAState *s = opaque;
+
+    qemu_set_irq(s->pic_irq[irq], level);
+}
+
 static void vt82c686b_realize(PCIDevice *d, Error **errp)
 {
     ViaISAState *s = VIA_ISA(d);
@@ -611,7 +619,10 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
     qdev_init_gpio_out_named(dev, &s->cpu_intr, "intr", 1);
     isa_bus = isa_bus_new(dev, get_system_memory(), pci_address_space_io(d),
                           &error_fatal);
-    isa_bus_irqs(isa_bus, i8259_init(isa_bus, s->cpu_intr));
+    s->pic_irq = i8259_init(isa_bus, s->cpu_intr);
+    isa_bus_irqs(isa_bus, s->pic_irq);
+    qdev_init_gpio_in_named(dev, vt82c686b_isa_irq, "isa-irq", ISA_NUM_IRQS);
+
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
     i8257_dma_init(isa_bus, 0);
     s->via_sio = VIA_SUPERIO(isa_create_simple(isa_bus,
-- 
2.26.3

[PATCH v2 4/6] hw/ide/via: Replace magic 2 value by ARRAY_SIZE / MAX_IDE_DEVS

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/ide/via.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index be09912b334..6c667a92130 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -90,7 +90,7 @@ static void bmdma_setup_bar(PCIIDEState *d)
     int i;
 
     memory_region_init(&d->bmdma_bar, OBJECT(d), "via-bmdma-container", 16);
-    for(i = 0;i < 2; i++) {
+    for (i = 0; i < ARRAY_SIZE(d->bmdma); i++) {
         BMDMAState *bm = &d->bmdma[i];
 
         memory_region_init_io(&bm->extra_io, OBJECT(d), &via_bmdma_ops, bm,
@@ -122,7 +122,7 @@ static void via_ide_reset(DeviceState *dev)
     uint8_t *pci_conf = pd->config;
     int i;
 
-    for (i = 0; i < 2; i++) {
+    for (i = 0; i < ARRAY_SIZE(d->bus); i++) {
         ide_bus_reset(&d->bus[i]);
     }
 
@@ -188,9 +188,9 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
     bmdma_setup_bar(d);
     pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
 
-    qdev_init_gpio_in(ds, via_ide_set_irq, 2);
-    for (i = 0; i < 2; i++) {
-        ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
+    qdev_init_gpio_in(ds, via_ide_set_irq, ARRAY_SIZE(d->bus));
+    for (i = 0; i < ARRAY_SIZE(d->bus); i++) {
+        ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, MAX_IDE_DEVS);
         ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
@@ -204,7 +204,7 @@ static void via_ide_exitfn(PCIDevice *dev)
     PCIIDEState *d = PCI_IDE(dev);
     unsigned i;
 
-    for (i = 0; i < 2; ++i) {
+    for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) {
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
         memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
     }
-- 
2.26.3

Re: [PATCH v2 4/6] hw/ide/via: Replace magic 2 value by ARRAY_SIZE / MAX_IDE_DEVS

[John Snow]

On 5/11/21 12:18 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Acked-by: John Snow <jsnow@redhat.com>

> ---
>   hw/ide/via.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index be09912b334..6c667a92130 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -90,7 +90,7 @@ static void bmdma_setup_bar(PCIIDEState *d)
>       int i;
>   
>       memory_region_init(&d->bmdma_bar, OBJECT(d), "via-bmdma-container", 16);
> -    for(i = 0;i < 2; i++) {
> +    for (i = 0; i < ARRAY_SIZE(d->bmdma); i++) {
>           BMDMAState *bm = &d->bmdma[i];
>   
>           memory_region_init_io(&bm->extra_io, OBJECT(d), &via_bmdma_ops, bm,
> @@ -122,7 +122,7 @@ static void via_ide_reset(DeviceState *dev)
>       uint8_t *pci_conf = pd->config;
>       int i;
>   
> -    for (i = 0; i < 2; i++) {
> +    for (i = 0; i < ARRAY_SIZE(d->bus); i++) {
>           ide_bus_reset(&d->bus[i]);
>       }
>   
> @@ -188,9 +188,9 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>       bmdma_setup_bar(d);
>       pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
>   
> -    qdev_init_gpio_in(ds, via_ide_set_irq, 2);
> -    for (i = 0; i < 2; i++) {
> -        ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
> +    qdev_init_gpio_in(ds, via_ide_set_irq, ARRAY_SIZE(d->bus));
> +    for (i = 0; i < ARRAY_SIZE(d->bus); i++) {
> +        ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, MAX_IDE_DEVS);
>           ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
>   
>           bmdma_init(&d->bus[i], &d->bmdma[i], d);
> @@ -204,7 +204,7 @@ static void via_ide_exitfn(PCIDevice *dev)
>       PCIIDEState *d = PCI_IDE(dev);
>       unsigned i;
>   
> -    for (i = 0; i < 2; ++i) {
> +    for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) {
>           memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
>           memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
>       }
> 

[PATCH v2 5/6] hw/ide/via: Connect IDE function output IRQs to the ISA function input

[Philippe Mathieu-Daudé]

To avoid abusing isa_get_irq(NULL) using a hidden ISA bridge
under the hood, let the IDE function expose 2 output IRQs,
and connect them to the ISA function inputs when creating
the south bridge chipset model in vt82c686b_southbridge_init.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/ide/via.c        | 19 +++++++++++++++++--
 hw/mips/fuloong2e.c |  9 ++++++++-
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index 6c667a92130..7887bf181e6 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -33,6 +33,17 @@
 #include "hw/ide/pci.h"
 #include "trace.h"
 
+#define TYPE_VIA_IDE "via-ide"
+OBJECT_DECLARE_SIMPLE_TYPE(VIAIDEState, VIA_IDE)
+
+struct VIAIDEState {
+    /* <private> */
+    PCIIDEState parent_obj;
+    /* <public> */
+
+    qemu_irq irq[2];
+};
+
 static uint64_t bmdma_read(void *opaque, hwaddr addr,
                            unsigned size)
 {
@@ -105,6 +116,7 @@ static void bmdma_setup_bar(PCIIDEState *d)
 static void via_ide_set_irq(void *opaque, int n, int level)
 {
     PCIDevice *d = PCI_DEVICE(opaque);
+    VIAIDEState *s = VIA_IDE(d);
 
     if (level) {
         d->config[0x70 + n * 8] |= 0x80;
@@ -112,7 +124,7 @@ static void via_ide_set_irq(void *opaque, int n, int level)
         d->config[0x70 + n * 8] &= ~0x80;
     }
 
-    qemu_set_irq(isa_get_irq(NULL, 14 + n), level);
+    qemu_set_irq(s->irq[n], level);
 }
 
 static void via_ide_reset(DeviceState *dev)
@@ -159,6 +171,7 @@ static void via_ide_reset(DeviceState *dev)
 
 static void via_ide_realize(PCIDevice *dev, Error **errp)
 {
+    VIAIDEState *s = VIA_IDE(dev);
     PCIIDEState *d = PCI_IDE(dev);
     DeviceState *ds = DEVICE(dev);
     uint8_t *pci_conf = dev->config;
@@ -188,6 +201,7 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
     bmdma_setup_bar(d);
     pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
 
+    qdev_init_gpio_out_named(ds, s->irq, "ide-irq", ARRAY_SIZE(s->irq));
     qdev_init_gpio_in(ds, via_ide_set_irq, ARRAY_SIZE(d->bus));
     for (i = 0; i < ARRAY_SIZE(d->bus); i++) {
         ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, MAX_IDE_DEVS);
@@ -227,8 +241,9 @@ static void via_ide_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo via_ide_info = {
-    .name          = "via-ide",
+    .name          = TYPE_VIA_IDE,
     .parent        = TYPE_PCI_IDE,
+    .instance_size = sizeof(VIAIDEState),
     .class_init    = via_ide_class_init,
 };
 
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 3e7a7e4389d..17b5e41cc7d 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -201,12 +201,19 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
                                        I2CBus **i2c_bus)
 {
     PCIDevice *dev;
+    DeviceState *isa;
 
     dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
                                           TYPE_VT82C686B_ISA);
-    qdev_connect_gpio_out_named(DEVICE(dev), "intr", 0, intc);
+    isa = DEVICE(dev);
+    qdev_connect_gpio_out_named(isa, "intr", 0, intc);
 
     dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
+    for (unsigned i = 0; i < 2; i++) {
+        qdev_connect_gpio_out_named(DEVICE(dev), "ide-irq", i,
+                                    qdev_get_gpio_in_named(isa,
+                                                           "isa-irq", 14 + i));
+    }
     pci_ide_create_devs(dev);
 
     pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
-- 
2.26.3

Re: [PATCH v2 5/6] hw/ide/via: Connect IDE function output IRQs to the ISA function input

[John Snow]

On 5/11/21 12:18 AM, Philippe Mathieu-Daudé wrote:
> To avoid abusing isa_get_irq(NULL) using a hidden ISA bridge
> under the hood, let the IDE function expose 2 output IRQs,
> and connect them to the ISA function inputs when creating
> the south bridge chipset model in vt82c686b_southbridge_init.
> 

Seems reasonable on the surface. How do I test this? How did you test it?

--js

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/ide/via.c        | 19 +++++++++++++++++--
>   hw/mips/fuloong2e.c |  9 ++++++++-
>   2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index 6c667a92130..7887bf181e6 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -33,6 +33,17 @@
>   #include "hw/ide/pci.h"
>   #include "trace.h"
>   
> +#define TYPE_VIA_IDE "via-ide"
> +OBJECT_DECLARE_SIMPLE_TYPE(VIAIDEState, VIA_IDE)
> +
> +struct VIAIDEState {
> +    /* <private> */
> +    PCIIDEState parent_obj;
> +    /* <public> */
> +
> +    qemu_irq irq[2];
> +};
> +
>   static uint64_t bmdma_read(void *opaque, hwaddr addr,
>                              unsigned size)
>   {
> @@ -105,6 +116,7 @@ static void bmdma_setup_bar(PCIIDEState *d)
>   static void via_ide_set_irq(void *opaque, int n, int level)
>   {
>       PCIDevice *d = PCI_DEVICE(opaque);
> +    VIAIDEState *s = VIA_IDE(d);
>   
>       if (level) {
>           d->config[0x70 + n * 8] |= 0x80;
> @@ -112,7 +124,7 @@ static void via_ide_set_irq(void *opaque, int n, int level)
>           d->config[0x70 + n * 8] &= ~0x80;
>       }
>   
> -    qemu_set_irq(isa_get_irq(NULL, 14 + n), level);
> +    qemu_set_irq(s->irq[n], level);
>   }
>   
>   static void via_ide_reset(DeviceState *dev)
> @@ -159,6 +171,7 @@ static void via_ide_reset(DeviceState *dev)
>   
>   static void via_ide_realize(PCIDevice *dev, Error **errp)
>   {
> +    VIAIDEState *s = VIA_IDE(dev);
>       PCIIDEState *d = PCI_IDE(dev);
>       DeviceState *ds = DEVICE(dev);
>       uint8_t *pci_conf = dev->config;
> @@ -188,6 +201,7 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>       bmdma_setup_bar(d);
>       pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
>   
> +    qdev_init_gpio_out_named(ds, s->irq, "ide-irq", ARRAY_SIZE(s->irq));
>       qdev_init_gpio_in(ds, via_ide_set_irq, ARRAY_SIZE(d->bus));
>       for (i = 0; i < ARRAY_SIZE(d->bus); i++) {
>           ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, MAX_IDE_DEVS);
> @@ -227,8 +241,9 @@ static void via_ide_class_init(ObjectClass *klass, void *data)
>   }
>   
>   static const TypeInfo via_ide_info = {
> -    .name          = "via-ide",
> +    .name          = TYPE_VIA_IDE,
>       .parent        = TYPE_PCI_IDE,
> +    .instance_size = sizeof(VIAIDEState),
>       .class_init    = via_ide_class_init,
>   };
>   
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 3e7a7e4389d..17b5e41cc7d 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -201,12 +201,19 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>                                          I2CBus **i2c_bus)
>   {
>       PCIDevice *dev;
> +    DeviceState *isa;
>   
>       dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
>                                             TYPE_VT82C686B_ISA);
> -    qdev_connect_gpio_out_named(DEVICE(dev), "intr", 0, intc);
> +    isa = DEVICE(dev);
> +    qdev_connect_gpio_out_named(isa, "intr", 0, intc);
>   
>       dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
> +    for (unsigned i = 0; i < 2; i++) {
> +        qdev_connect_gpio_out_named(DEVICE(dev), "ide-irq", i,
> +                                    qdev_get_gpio_in_named(isa,
> +                                                           "isa-irq", 14 + i));
> +    }
>       pci_ide_create_devs(dev);
>   
>       pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
> 

[PATCH v2 6/6] hw/southbridge/vt82c686: Introduce VT82C686B_SOUTHBRIDGE

[Philippe Mathieu-Daudé]

The VT82C686B south bridge is a single chipset. Model
it as a single sysbus device.
Move the vt82c686b_southbridge_init as via_southbridge_realize,
add the QOM state: ViaSouthBridgeState. This device needs 2
properties to be realized: the PCI bus and its slot number.
2 aliases are exposed: the ISA PIC output IRQ and the I2C bus.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/fuloong2e.c        |  42 +++------------
 hw/southbridge/vt82c686.c  | 107 +++++++++++++++++++++++++++++++++++++
 MAINTAINERS                |   1 +
 hw/Kconfig                 |   1 +
 hw/isa/Kconfig             |   9 ----
 hw/meson.build             |   1 +
 hw/southbridge/Kconfig     |   8 +++
 hw/southbridge/meson.build |   1 +
 8 files changed, 127 insertions(+), 43 deletions(-)
 create mode 100644 hw/southbridge/vt82c686.c
 create mode 100644 hw/southbridge/Kconfig
 create mode 100644 hw/southbridge/meson.build

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 17b5e41cc7d..00aa085b982 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -34,10 +34,8 @@
 #include "hw/mips/cpudevs.h"
 #include "hw/pci/pci.h"
 #include "hw/loader.h"
-#include "hw/ide/pci.h"
 #include "hw/qdev-properties.h"
 #include "elf.h"
-#include "hw/isa/vt82c686.h"
 #include "sysemu/qtest.h"
 #include "sysemu/reset.h"
 #include "sysemu/sysemu.h"
@@ -197,36 +195,6 @@ static void main_cpu_reset(void *opaque)
     }
 }
 
-static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
-                                       I2CBus **i2c_bus)
-{
-    PCIDevice *dev;
-    DeviceState *isa;
-
-    dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
-                                          TYPE_VT82C686B_ISA);
-    isa = DEVICE(dev);
-    qdev_connect_gpio_out_named(isa, "intr", 0, intc);
-
-    dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
-    for (unsigned i = 0; i < 2; i++) {
-        qdev_connect_gpio_out_named(DEVICE(dev), "ide-irq", i,
-                                    qdev_get_gpio_in_named(isa,
-                                                           "isa-irq", 14 + i));
-    }
-    pci_ide_create_devs(dev);
-
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
-
-    dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
-    *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
-
-    /* Audio support */
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 5), TYPE_VIA_AC97);
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 6), TYPE_VIA_MC97);
-}
-
 /* Network support */
 static void network_init(PCIBus *pci_bus)
 {
@@ -323,8 +291,14 @@ static void mips_fuloong2e_init(MachineState *machine)
     pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
 
     /* South bridge -> IP5 */
-    vt82c686b_southbridge_init(pci_bus, FULOONG2E_VIA_SLOT, env->irq[5],
-                               &smbus);
+    dev = qdev_new("vt82c686b-southbridge");
+    object_property_set_uint(OBJECT(dev), "pci-slot",
+                             FULOONG2E_VIA_SLOT, &error_fatal);
+    object_property_set_link(OBJECT(dev), "pci-bus",
+                             OBJECT(pci_bus), &error_fatal);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    qdev_connect_gpio_out_named(dev, "intr", 0, env->irq[5]);
+    smbus = I2C_BUS(qdev_get_child_bus(dev, "i2c"));
 
     /* GPU */
     if (vga_interface_type != VGA_NONE) {
diff --git a/hw/southbridge/vt82c686.c b/hw/southbridge/vt82c686.c
new file mode 100644
index 00000000000..61c3e6ae306
--- /dev/null
+++ b/hw/southbridge/vt82c686.c
@@ -0,0 +1,107 @@
+/*
+ * VT82C686B south bridge emulation
+ *
+ * Copyright (c) 2008 yajin (yajin@vm-kernel.org)
+ * Copyright (c) 2009 chenming (chenming@rdc.faw.com.cn)
+ * Copyright (c) 2010 Huacai Chen (zltjiangshi@gmail.com)
+ * Copyright (c) 2021 Philippe Mathieu-Daudé <f4bug@amsat.org>
+ * This code is licensed under the GNU GPL v2.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "hw/pci/pci.h"
+#include "hw/ide/pci.h"
+#include "hw/isa/vt82c686.h"
+
+#define TYPE_VT82C686B_SOUTHBRIDGE "vt82c686b-southbridge"
+OBJECT_DECLARE_SIMPLE_TYPE(ViaSouthBridgeState, VT82C686B_SOUTHBRIDGE)
+
+struct ViaSouthBridgeState {
+    /* <private> */
+    SysBusDevice parent_obj;
+    /* <public> */
+
+    uint8_t pci_slot;
+    PCIBus *pci_bus;
+    PCIDevice *isa;
+    PCIDevice *ide;
+    PCIDevice *usb[2];
+    PCIDevice *apm;
+    PCIDevice *audio;
+    PCIDevice *modem;
+};
+
+static void via_southbridge_realize(DeviceState *dev, Error **errp)
+{
+    ViaSouthBridgeState *s = VT82C686B_SOUTHBRIDGE(dev);
+
+    if (!s->pci_bus) {
+        error_setg(errp, "SMMU is not attached to any PCI bus!");
+        return;
+    }
+
+    s->isa = pci_create_simple_multifunction(s->pci_bus,
+                                             PCI_DEVFN(s->pci_slot, 0),
+                                             true, TYPE_VT82C686B_ISA);
+    qdev_pass_gpios(DEVICE(s->isa), dev, "intr");
+
+    s->ide = pci_create_simple(s->pci_bus,
+                               PCI_DEVFN(s->pci_slot, 1), "via-ide");
+    for (unsigned i = 0; i < 2; i++) {
+        qdev_connect_gpio_out_named(DEVICE(s->ide), "ide-irq", i,
+                            qdev_get_gpio_in_named(DEVICE(s->isa),
+                                                   "isa-irq", 14 + i));
+    }
+    pci_ide_create_devs(s->ide);
+
+    s->usb[0] = pci_create_simple(s->pci_bus,
+                                  PCI_DEVFN(s->pci_slot, 2),
+                                  "vt82c686b-usb-uhci");
+    s->usb[1] = pci_create_simple(s->pci_bus,
+                                  PCI_DEVFN(s->pci_slot, 3),
+                                  "vt82c686b-usb-uhci");
+
+    s->apm = pci_create_simple(s->pci_bus,
+                               PCI_DEVFN(s->pci_slot, 4),
+                               TYPE_VT82C686B_PM);
+    object_property_add_alias(OBJECT(s), "i2c",
+                              OBJECT(s->apm), "i2c");
+
+    s->audio = pci_create_simple(s->pci_bus,
+                                 PCI_DEVFN(s->pci_slot, 5),
+                                 TYPE_VIA_AC97);
+    s->modem = pci_create_simple(s->pci_bus,
+                                 PCI_DEVFN(s->pci_slot, 6),
+                                 TYPE_VIA_MC97);
+}
+
+static Property via_southbridge_properties[] = {
+    DEFINE_PROP_UINT8("pci-slot", ViaSouthBridgeState, pci_slot, 0),
+    DEFINE_PROP_LINK("pci-bus", ViaSouthBridgeState, pci_bus, "PCI", PCIBus *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void via_southbridge_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = via_southbridge_realize;
+    device_class_set_props(dc, via_southbridge_properties);
+}
+
+static const TypeInfo via_southbridge_info = {
+    .name          = TYPE_VT82C686B_SOUTHBRIDGE,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(ViaSouthBridgeState),
+    .class_init    = via_southbridge_class_init,
+};
+
+static void via_southbridge_register_types(void)
+{
+    type_register_static(&via_southbridge_info);
+}
+
+type_init(via_southbridge_register_types)
diff --git a/MAINTAINERS b/MAINTAINERS
index b692c8fbee2..3275479672a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1204,6 +1204,7 @@ S: Odd Fixes
 F: hw/mips/fuloong2e.c
 F: hw/isa/vt82c686.c
 F: hw/pci-host/bonito.c
+F: hw/southbridge/vt82c686.c
 F: hw/usb/vt82c686-uhci-pci.c
 F: include/hw/isa/vt82c686.h
 
diff --git a/hw/Kconfig b/hw/Kconfig
index ff40bd3f7bb..76e35ad189f 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -32,6 +32,7 @@ source rtc/Kconfig
 source scsi/Kconfig
 source sd/Kconfig
 source smbios/Kconfig
+source southbridge/Kconfig
 source ssi/Kconfig
 source timer/Kconfig
 source tpm/Kconfig
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 55e0003ce40..34adc411fa6 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -41,15 +41,6 @@ config PIIX4
     select ISA_BUS
     select USB_UHCI
 
-config VT82C686
-    bool
-    select ISA_SUPERIO
-    select ACPI_SMBUS
-    select SERIAL_ISA
-    select FDC
-    select USB_UHCI
-    select APM
-
 config SMC37C669
     bool
     select ISA_SUPERIO
diff --git a/hw/meson.build b/hw/meson.build
index 8ba79b1a528..4bdd254e041 100644
--- a/hw/meson.build
+++ b/hw/meson.build
@@ -31,6 +31,7 @@
 subdir('scsi')
 subdir('sd')
 subdir('smbios')
+subdir('southbridge')
 subdir('ssi')
 subdir('timer')
 subdir('tpm')
diff --git a/hw/southbridge/Kconfig b/hw/southbridge/Kconfig
new file mode 100644
index 00000000000..9cd9fe5ee64
--- /dev/null
+++ b/hw/southbridge/Kconfig
@@ -0,0 +1,8 @@
+config VT82C686
+    bool
+    select ISA_SUPERIO
+    select ACPI_SMBUS
+    select SERIAL_ISA
+    select FDC
+    select USB_UHCI
+    select APM
diff --git a/hw/southbridge/meson.build b/hw/southbridge/meson.build
new file mode 100644
index 00000000000..53b02e9563c
--- /dev/null
+++ b/hw/southbridge/meson.build
@@ -0,0 +1 @@
+softmmu_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
-- 
2.26.3

Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
> The motivation behind this series is to remove the
> isa_get_irq(NULL) call to simplify the ISA generic model.
>
> Since v1:
> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504

I'll try to have a look at these later but some notes: The pegasos2 
changes are now in master so if this was before that maybe rebasing on 
master is now enough. However I wonder if any changes to pegasos2.c is 
needed due to changed init of the chip model or is that only affecting 
82c686b? Please also note that pegasos2 is not enabled by default due to 
needing undistributable firmware ROM so to test it you need to enable it 
in default-configs/devices/ppc-softmmu.mak

Regards,
BALATON Zoltan

> Philippe Mathieu-Daudé (6):
>  hw/isa/vt82c686: Name output IRQ as 'intr'
>  hw/isa/vt82c686: Simplify removing unuseful qemu_allocate_irqs() call
>  hw/isa/vt82c686: Let ISA function expose ISA IRQs
>  hw/ide/via: Replace magic 2 value by ARRAY_SIZE / MAX_IDE_DEVS
>  hw/ide/via: Connect IDE function output IRQs to the ISA function input
>  hw/southbridge/vt82c686: Introduce VT82C686B_SOUTHBRIDGE
>
> hw/ide/via.c               |  31 ++++++++---
> hw/isa/vt82c686.c          |  27 +++++-----
> hw/mips/fuloong2e.c        |  35 +++---------
> hw/southbridge/vt82c686.c  | 107 +++++++++++++++++++++++++++++++++++++
> MAINTAINERS                |   1 +
> hw/Kconfig                 |   1 +
> hw/isa/Kconfig             |   9 ----
> hw/meson.build             |   1 +
> hw/southbridge/Kconfig     |   8 +++
> hw/southbridge/meson.build |   1 +
> 10 files changed, 164 insertions(+), 57 deletions(-)
> create mode 100644 hw/southbridge/vt82c686.c
> create mode 100644 hw/southbridge/Kconfig
> create mode 100644 hw/southbridge/meson.build
>
> --
> 2.26.3
>
>
>

Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[Philippe Mathieu-Daudé]

Hi Zoltan,

On 5/11/21 1:28 PM, BALATON Zoltan wrote:
> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>> The motivation behind this series is to remove the
>> isa_get_irq(NULL) call to simplify the ISA generic model.
>>
>> Since v1:
>> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504
> 
> I'll try to have a look at these later but some notes: The pegasos2
> changes are now in master so if this was before that maybe rebasing on
> master is now enough.

This is what this series does, simply rebase on top of your merged
patches.

> However I wonder if any changes to pegasos2.c is
> needed due to changed init of the chip model or is that only affecting
> 82c686b?

There is no change in 'init' in this series, it is only QOM boilerplate
code churn, no logical change intended.

> Please also note that pegasos2 is not enabled by default due to
> needing undistributable firmware ROM so to test it you need to enable it
> in default-configs/devices/ppc-softmmu.mak

I remember you said you were mostly interested in the VT8231, not
the VT82C686. This series only QOM'ify the latter.

What is your idea? Send the firmware off-list and explain how
the OS works and how (what) to test?

Regards,

Phil.

>> Philippe Mathieu-Daudé (6):
>>  hw/isa/vt82c686: Name output IRQ as 'intr'
>>  hw/isa/vt82c686: Simplify removing unuseful qemu_allocate_irqs() call
>>  hw/isa/vt82c686: Let ISA function expose ISA IRQs
>>  hw/ide/via: Replace magic 2 value by ARRAY_SIZE / MAX_IDE_DEVS
>>  hw/ide/via: Connect IDE function output IRQs to the ISA function input
>>  hw/southbridge/vt82c686: Introduce VT82C686B_SOUTHBRIDGE

Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]

On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
> Hi Zoltan,
>
> On 5/11/21 1:28 PM, BALATON Zoltan wrote:
>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>> The motivation behind this series is to remove the
>>> isa_get_irq(NULL) call to simplify the ISA generic model.
>>>
>>> Since v1:
>>> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504
>>
>> I'll try to have a look at these later but some notes: The pegasos2
>> changes are now in master so if this was before that maybe rebasing on
>> master is now enough.
>
> This is what this series does, simply rebase on top of your merged
> patches.
>
>> However I wonder if any changes to pegasos2.c is
>> needed due to changed init of the chip model or is that only affecting
>> 82c686b?
>
> There is no change in 'init' in this series, it is only QOM boilerplate
> code churn, no logical change intended.
>
>> Please also note that pegasos2 is not enabled by default due to
>> needing undistributable firmware ROM so to test it you need to enable it
>> in default-configs/devices/ppc-softmmu.mak
>
> I remember you said you were mostly interested in the VT8231, not
> the VT82C686. This series only QOM'ify the latter.

OK as I said I haven't looked at it in detail.

> What is your idea? Send the firmware off-list and explain how
> the OS works and how (what) to test?

I've already sent you this info:

https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01553.html

but I can't write a test case so if you want to automate this and make it 
part of QEMU tests then some help with that would be appreciated.

As for the firmware, once Alexey's VOF (Virtual Open Firmware, minimial OF 
emulation in QEMU) is merged I plan to try to use that to make it possible 
to boot some guests with that so no firmware image would be needed and 
pegasos2 could be enabled by default. But for now a firmware image is 
needed as guests expect an OF environment to boot.

Regards,
BALATON Zoltan

Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[Philippe Mathieu-Daudé]

On 5/11/21 3:09 PM, BALATON Zoltan wrote:
> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>> Hi Zoltan,
>>
>> On 5/11/21 1:28 PM, BALATON Zoltan wrote:
>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>> The motivation behind this series is to remove the
>>>> isa_get_irq(NULL) call to simplify the ISA generic model.
>>>>
>>>> Since v1:
>>>> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504
>>>
>>> I'll try to have a look at these later but some notes: The pegasos2
>>> changes are now in master so if this was before that maybe rebasing on
>>> master is now enough.
>>
>> This is what this series does, simply rebase on top of your merged
>> patches.
>>
>>> However I wonder if any changes to pegasos2.c is
>>> needed due to changed init of the chip model or is that only affecting
>>> 82c686b?
>>
>> There is no change in 'init' in this series, it is only QOM boilerplate
>> code churn, no logical change intended.
>>
>>> Please also note that pegasos2 is not enabled by default due to
>>> needing undistributable firmware ROM so to test it you need to enable it
>>> in default-configs/devices/ppc-softmmu.mak
>>
>> I remember you said you were mostly interested in the VT8231, not
>> the VT82C686. This series only QOM'ify the latter.
> 
> OK as I said I haven't looked at it in detail.
> 
>> What is your idea? Send the firmware off-list and explain how
>> the OS works and how (what) to test?
> 
> I've already sent you this info:
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01553.html

Well, if you have everything setup, it is easier to test and send
a Tested-by tag.

> but I can't write a test case so if you want to automate this and make
> it part of QEMU tests then some help with that would be appreciated.

You are not the only want wanting that. I'm working on a solution to run
such tests (depending on binary blobs) in your own namespace, but it
will take me time (doing it in my free time, without help).

> As for the firmware, once Alexey's VOF (Virtual Open Firmware, minimial
> OF emulation in QEMU) is merged I plan to try to use that to make it
> possible to boot some guests with that so no firmware image would be
> needed and pegasos2 could be enabled by default. But for now a firmware
> image is needed as guests expect an OF environment to boot.
> 
> Regards,
> BALATON Zoltan

Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2351 bytes --]

On Thu, 13 May 2021, Philippe Mathieu-Daudé wrote:
> On 5/11/21 3:09 PM, BALATON Zoltan wrote:
>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>> Hi Zoltan,
>>>
>>> On 5/11/21 1:28 PM, BALATON Zoltan wrote:
>>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>>> The motivation behind this series is to remove the
>>>>> isa_get_irq(NULL) call to simplify the ISA generic model.
>>>>>
>>>>> Since v1:
>>>>> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504
>>>>
>>>> I'll try to have a look at these later but some notes: The pegasos2
>>>> changes are now in master so if this was before that maybe rebasing on
>>>> master is now enough.
>>>
>>> This is what this series does, simply rebase on top of your merged
>>> patches.
>>>
>>>> However I wonder if any changes to pegasos2.c is
>>>> needed due to changed init of the chip model or is that only affecting
>>>> 82c686b?
>>>
>>> There is no change in 'init' in this series, it is only QOM boilerplate
>>> code churn, no logical change intended.
>>>
>>>> Please also note that pegasos2 is not enabled by default due to
>>>> needing undistributable firmware ROM so to test it you need to enable it
>>>> in default-configs/devices/ppc-softmmu.mak
>>>
>>> I remember you said you were mostly interested in the VT8231, not
>>> the VT82C686. This series only QOM'ify the latter.
>>
>> OK as I said I haven't looked at it in detail.
>>
>>> What is your idea? Send the firmware off-list and explain how
>>> the OS works and how (what) to test?
>>
>> I've already sent you this info:
>>
>> https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01553.html
>
> Well, if you have everything setup, it is easier to test and send
> a Tested-by tag.

I indend to test it when I'll have some time but I could not get to it 
yet.

>> but I can't write a test case so if you want to automate this and make
>> it part of QEMU tests then some help with that would be appreciated.
>
> You are not the only want wanting that. I'm working on a solution to run
> such tests (depending on binary blobs) in your own namespace, but it
> will take me time (doing it in my free time, without help).

I did not mean to say you should do this urgently, just sent this as a 
reminder about how this could be tested in case you forgot because you've 
asked about testing.

Regards,
BALATON Zoltan

Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[Philippe Mathieu-Daudé]

On 5/13/21 1:54 PM, BALATON Zoltan wrote:
> On Thu, 13 May 2021, Philippe Mathieu-Daudé wrote:
>> On 5/11/21 3:09 PM, BALATON Zoltan wrote:
>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>> Hi Zoltan,
>>>>
>>>> On 5/11/21 1:28 PM, BALATON Zoltan wrote:
>>>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>>>> The motivation behind this series is to remove the
>>>>>> isa_get_irq(NULL) call to simplify the ISA generic model.
>>>>>>
>>>>>> Since v1:
>>>>>> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504
>>>>>
>>>>> I'll try to have a look at these later but some notes: The pegasos2
>>>>> changes are now in master so if this was before that maybe rebasing on
>>>>> master is now enough.
>>>>
>>>> This is what this series does, simply rebase on top of your merged
>>>> patches.
>>>>
>>>>> However I wonder if any changes to pegasos2.c is
>>>>> needed due to changed init of the chip model or is that only affecting
>>>>> 82c686b?
>>>>
>>>> There is no change in 'init' in this series, it is only QOM boilerplate
>>>> code churn, no logical change intended.
>>>>
>>>>> Please also note that pegasos2 is not enabled by default due to
>>>>> needing undistributable firmware ROM so to test it you need to
>>>>> enable it
>>>>> in default-configs/devices/ppc-softmmu.mak
>>>>
>>>> I remember you said you were mostly interested in the VT8231, not
>>>> the VT82C686. This series only QOM'ify the latter.
>>>
>>> OK as I said I haven't looked at it in detail.
>>>
>>>> What is your idea? Send the firmware off-list and explain how
>>>> the OS works and how (what) to test?
>>>
>>> I've already sent you this info:
>>>
>>> https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01553.html
>>
>> Well, if you have everything setup, it is easier to test and send
>> a Tested-by tag.
> 
> I indend to test it when I'll have some time but I could not get to it yet.
> 
>>> but I can't write a test case so if you want to automate this and make
>>> it part of QEMU tests then some help with that would be appreciated.
>>
>> You are not the only want wanting that. I'm working on a solution to run
>> such tests (depending on binary blobs) in your own namespace, but it
>> will take me time (doing it in my free time, without help).
> 
> I did not mean to say you should do this urgently, just sent this as a
> reminder about how this could be tested in case you forgot because
> you've asked about testing.

Unrelated to this series, with master (dab59ce0312) I sometime get:

Initializing KBD...00000012                               FAILED.

and then the mouse isn't working.

Sometimes:

Initializing KBD...                                       Done.

and the mouse is crazy (similar to my host mouse).

Anyway, there is smth wrong with patch #2 of this series:
"Simplify removing unuseful qemu_allocate_irqs() call".

Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 3315 bytes --]

On Thu, 13 May 2021, Philippe Mathieu-Daudé wrote:
> On 5/13/21 1:54 PM, BALATON Zoltan wrote:
>> On Thu, 13 May 2021, Philippe Mathieu-Daudé wrote:
>>> On 5/11/21 3:09 PM, BALATON Zoltan wrote:
>>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>>> Hi Zoltan,
>>>>>
>>>>> On 5/11/21 1:28 PM, BALATON Zoltan wrote:
>>>>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>>>>> The motivation behind this series is to remove the
>>>>>>> isa_get_irq(NULL) call to simplify the ISA generic model.
>>>>>>>
>>>>>>> Since v1:
>>>>>>> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504
>>>>>>
>>>>>> I'll try to have a look at these later but some notes: The pegasos2
>>>>>> changes are now in master so if this was before that maybe rebasing on
>>>>>> master is now enough.
>>>>>
>>>>> This is what this series does, simply rebase on top of your merged
>>>>> patches.
>>>>>
>>>>>> However I wonder if any changes to pegasos2.c is
>>>>>> needed due to changed init of the chip model or is that only affecting
>>>>>> 82c686b?
>>>>>
>>>>> There is no change in 'init' in this series, it is only QOM boilerplate
>>>>> code churn, no logical change intended.
>>>>>
>>>>>> Please also note that pegasos2 is not enabled by default due to
>>>>>> needing undistributable firmware ROM so to test it you need to
>>>>>> enable it
>>>>>> in default-configs/devices/ppc-softmmu.mak
>>>>>
>>>>> I remember you said you were mostly interested in the VT8231, not
>>>>> the VT82C686. This series only QOM'ify the latter.
>>>>
>>>> OK as I said I haven't looked at it in detail.
>>>>
>>>>> What is your idea? Send the firmware off-list and explain how
>>>>> the OS works and how (what) to test?
>>>>
>>>> I've already sent you this info:
>>>>
>>>> https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01553.html
>>>
>>> Well, if you have everything setup, it is easier to test and send
>>> a Tested-by tag.
>>
>> I indend to test it when I'll have some time but I could not get to it yet.
>>
>>>> but I can't write a test case so if you want to automate this and make
>>>> it part of QEMU tests then some help with that would be appreciated.
>>>
>>> You are not the only want wanting that. I'm working on a solution to run
>>> such tests (depending on binary blobs) in your own namespace, but it
>>> will take me time (doing it in my free time, without help).
>>
>> I did not mean to say you should do this urgently, just sent this as a
>> reminder about how this could be tested in case you forgot because
>> you've asked about testing.
>
> Unrelated to this series, with master (dab59ce0312) I sometime get:
>
> Initializing KBD...00000012                               FAILED.
>
> and then the mouse isn't working.
>
> Sometimes:
>
> Initializing KBD...                                       Done.
>
> and the mouse is crazy (similar to my host mouse).
>
> Anyway, there is smth wrong with patch #2 of this series:
> "Simplify removing unuseful qemu_allocate_irqs() call".

As I said before, when I've tried to do it that way first it did not work 
for me so I introduced the indirection which fixed it but I did not 
understand why it was needed or I forgot by now so all I remember is that 
I could not directly connect the irq and needed the local function for 
some reason.

Regards,
BALATON Zoltan

Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[Philippe Mathieu-Daudé]

On 5/15/21 4:37 PM, BALATON Zoltan wrote:
> On Thu, 13 May 2021, Philippe Mathieu-Daudé wrote:
>> On 5/13/21 1:54 PM, BALATON Zoltan wrote:
>>> On Thu, 13 May 2021, Philippe Mathieu-Daudé wrote:
>>>> On 5/11/21 3:09 PM, BALATON Zoltan wrote:
>>>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>>>> Hi Zoltan,
>>>>>>
>>>>>> On 5/11/21 1:28 PM, BALATON Zoltan wrote:
>>>>>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>>>>>> The motivation behind this series is to remove the
>>>>>>>> isa_get_irq(NULL) call to simplify the ISA generic model.
>>>>>>>>
>>>>>>>> Since v1:
>>>>>>>> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504
>>>>>>>
>>>>>>> I'll try to have a look at these later but some notes: The pegasos2
>>>>>>> changes are now in master so if this was before that maybe
>>>>>>> rebasing on
>>>>>>> master is now enough.
>>>>>>
>>>>>> This is what this series does, simply rebase on top of your merged
>>>>>> patches.
>>>>>>
>>>>>>> However I wonder if any changes to pegasos2.c is
>>>>>>> needed due to changed init of the chip model or is that only
>>>>>>> affecting
>>>>>>> 82c686b?
>>>>>>
>>>>>> There is no change in 'init' in this series, it is only QOM
>>>>>> boilerplate
>>>>>> code churn, no logical change intended.
>>>>>>
>>>>>>> Please also note that pegasos2 is not enabled by default due to
>>>>>>> needing undistributable firmware ROM so to test it you need to
>>>>>>> enable it
>>>>>>> in default-configs/devices/ppc-softmmu.mak
>>>>>>
>>>>>> I remember you said you were mostly interested in the VT8231, not
>>>>>> the VT82C686. This series only QOM'ify the latter.
>>>>>
>>>>> OK as I said I haven't looked at it in detail.
>>>>>
>>>>>> What is your idea? Send the firmware off-list and explain how
>>>>>> the OS works and how (what) to test?
>>>>>
>>>>> I've already sent you this info:
>>>>>
>>>>> https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01553.html
>>>>
>>>> Well, if you have everything setup, it is easier to test and send
>>>> a Tested-by tag.
>>>
>>> I indend to test it when I'll have some time but I could not get to
>>> it yet.
>>>
>>>>> but I can't write a test case so if you want to automate this and make
>>>>> it part of QEMU tests then some help with that would be appreciated.
>>>>
>>>> You are not the only want wanting that. I'm working on a solution to
>>>> run
>>>> such tests (depending on binary blobs) in your own namespace, but it
>>>> will take me time (doing it in my free time, without help).
>>>
>>> I did not mean to say you should do this urgently, just sent this as a
>>> reminder about how this could be tested in case you forgot because
>>> you've asked about testing.
>>
>> Unrelated to this series, with master (dab59ce0312) I sometime get:
>>
>> Initializing KBD...00000012                               FAILED.
>>
>> and then the mouse isn't working.
>>
>> Sometimes:
>>
>> Initializing KBD...                                       Done.
>>
>> and the mouse is crazy (similar to my host mouse).
>>
>> Anyway, there is smth wrong with patch #2 of this series:
>> "Simplify removing unuseful qemu_allocate_irqs() call".
> 
> As I said before, when I've tried to do it that way first it did not
> work for me so I introduced the indirection which fixed it but I did not
> understand why it was needed or I forgot by now so all I remember is
> that I could not directly connect the irq and needed the local function
> for some reason.

OK, I'll try to figure out what the problem is and come back to you.

Regards,

Phil.

Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 3984 bytes --]

On Sat, 15 May 2021, Philippe Mathieu-Daudé wrote:
> On 5/15/21 4:37 PM, BALATON Zoltan wrote:
>> On Thu, 13 May 2021, Philippe Mathieu-Daudé wrote:
>>> On 5/13/21 1:54 PM, BALATON Zoltan wrote:
>>>> On Thu, 13 May 2021, Philippe Mathieu-Daudé wrote:
>>>>> On 5/11/21 3:09 PM, BALATON Zoltan wrote:
>>>>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>>>>> Hi Zoltan,
>>>>>>>
>>>>>>> On 5/11/21 1:28 PM, BALATON Zoltan wrote:
>>>>>>>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote:
>>>>>>>>> The motivation behind this series is to remove the
>>>>>>>>> isa_get_irq(NULL) call to simplify the ISA generic model.
>>>>>>>>>
>>>>>>>>> Since v1:
>>>>>>>>> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504
>>>>>>>>
>>>>>>>> I'll try to have a look at these later but some notes: The pegasos2
>>>>>>>> changes are now in master so if this was before that maybe
>>>>>>>> rebasing on
>>>>>>>> master is now enough.
>>>>>>>
>>>>>>> This is what this series does, simply rebase on top of your merged
>>>>>>> patches.
>>>>>>>
>>>>>>>> However I wonder if any changes to pegasos2.c is
>>>>>>>> needed due to changed init of the chip model or is that only
>>>>>>>> affecting
>>>>>>>> 82c686b?
>>>>>>>
>>>>>>> There is no change in 'init' in this series, it is only QOM
>>>>>>> boilerplate
>>>>>>> code churn, no logical change intended.
>>>>>>>
>>>>>>>> Please also note that pegasos2 is not enabled by default due to
>>>>>>>> needing undistributable firmware ROM so to test it you need to
>>>>>>>> enable it
>>>>>>>> in default-configs/devices/ppc-softmmu.mak
>>>>>>>
>>>>>>> I remember you said you were mostly interested in the VT8231, not
>>>>>>> the VT82C686. This series only QOM'ify the latter.
>>>>>>
>>>>>> OK as I said I haven't looked at it in detail.
>>>>>>
>>>>>>> What is your idea? Send the firmware off-list and explain how
>>>>>>> the OS works and how (what) to test?
>>>>>>
>>>>>> I've already sent you this info:
>>>>>>
>>>>>> https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01553.html
>>>>>
>>>>> Well, if you have everything setup, it is easier to test and send
>>>>> a Tested-by tag.
>>>>
>>>> I indend to test it when I'll have some time but I could not get to
>>>> it yet.
>>>>
>>>>>> but I can't write a test case so if you want to automate this and make
>>>>>> it part of QEMU tests then some help with that would be appreciated.
>>>>>
>>>>> You are not the only want wanting that. I'm working on a solution to
>>>>> run
>>>>> such tests (depending on binary blobs) in your own namespace, but it
>>>>> will take me time (doing it in my free time, without help).
>>>>
>>>> I did not mean to say you should do this urgently, just sent this as a
>>>> reminder about how this could be tested in case you forgot because
>>>> you've asked about testing.
>>>
>>> Unrelated to this series, with master (dab59ce0312) I sometime get:
>>>
>>> Initializing KBD...00000012                               FAILED.
>>>
>>> and then the mouse isn't working.
>>>
>>> Sometimes:
>>>
>>> Initializing KBD...                                       Done.
>>>
>>> and the mouse is crazy (similar to my host mouse).
>>>
>>> Anyway, there is smth wrong with patch #2 of this series:
>>> "Simplify removing unuseful qemu_allocate_irqs() call".
>>
>> As I said before, when I've tried to do it that way first it did not
>> work for me so I introduced the indirection which fixed it but I did not
>> understand why it was needed or I forgot by now so all I remember is
>> that I could not directly connect the irq and needed the local function
>> for some reason.
>
> OK, I'll try to figure out what the problem is and come back to you.

I'm not sure, but could this be related to a note Peter made in this 
series:

https://lists.nongnu.org/archive/html/qemu-ppc/2020-12/msg00171.html

about converting PPC interrupts to qdev qpios? Could it be that the irq 
cannot be connected directly as it has some unexpected behaviour due to 
this?

Regards,
BALATON Zoltan