[PATCH v3 8/8] hw/audio/via-ac97: Basic implementation of audio playback

[PATCH v3 8/8] hw/audio/via-ac97: Basic implementation of audio playback

[BALATON Zoltan]

Add basic implementation of the AC'97 sound part used in VIA south
bridge chips. Not all features of the device is emulated, only one
playback channel is supported for now but this is enough to get sound
output from some guests using this device on pegasos2.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Rene Engel <ReneEngel80@emailn.de>
---
v3: Fixed CLEN_LEN mask, add check to avoid runaway DMA and some
tweaks to PCI config regs which now make it work with AmigaOS too.
This is probably as good as it gets for QEMU 8.0

 hw/audio/trace-events     |   6 +
 hw/audio/via-ac97.c       | 455 +++++++++++++++++++++++++++++++++++++-
 hw/isa/trace-events       |   1 +
 hw/isa/vt82c686.c         |   2 +-
 include/hw/isa/vt82c686.h |  25 +++
 5 files changed, 482 insertions(+), 7 deletions(-)

diff --git a/hw/audio/trace-events b/hw/audio/trace-events
index e0e71cd9b1..4dec48a4fd 100644
--- a/hw/audio/trace-events
+++ b/hw/audio/trace-events
@@ -11,3 +11,9 @@ hda_audio_running(const char *stream, int nr, bool running) "st %s, nr %d, run %
 hda_audio_format(const char *stream, int chan, const char *fmt, int freq) "st %s, %d x %s @ %d Hz"
 hda_audio_adjust(const char *stream, int pos) "st %s, pos %d"
 hda_audio_overrun(const char *stream) "st %s"
+
+#via-ac97.c
+via_ac97_codec_write(uint8_t addr, uint16_t val) "0x%x <- 0x%x"
+via_ac97_sgd_fetch(uint32_t curr, uint32_t addr, char stop, char eol, char flag, uint32_t len) "curr=0x%x addr=0x%x %c%c%c len=%d"
+via_ac97_sgd_read(uint64_t addr, unsigned size, uint64_t val) "0x%"PRIx64" %d -> 0x%"PRIx64
+via_ac97_sgd_write(uint64_t addr, unsigned size, uint64_t val) "0x%"PRIx64" %d <- 0x%"PRIx64
diff --git a/hw/audio/via-ac97.c b/hw/audio/via-ac97.c
index d1a856f63d..676254b7a4 100644
--- a/hw/audio/via-ac97.c
+++ b/hw/audio/via-ac97.c
@@ -1,39 +1,482 @@
 /*
  * VIA south bridges sound support
  *
+ * Copyright (c) 2022-2023 BALATON Zoltan
+ *
  * This work is licensed under the GNU GPL license version 2 or later.
  */
 
 /*
- * TODO: This is entirely boiler plate just registering empty PCI devices
- * with the right ID guests expect, functionality should be added here.
+ * TODO: This is only a basic implementation of one audio playback channel
+ *       more functionality should be added here.
  */
 
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "hw/isa/vt82c686.h"
-#include "hw/pci/pci_device.h"
+#include "ac97.h"
+#include "trace.h"
+
+#define CLEN_IS_EOL(x)  ((x)->clen & BIT(31))
+#define CLEN_IS_FLAG(x) ((x)->clen & BIT(30))
+#define CLEN_IS_STOP(x) ((x)->clen & BIT(29))
+#define CLEN_LEN(x)     ((x)->clen & 0xffffff)
+
+#define STAT_ACTIVE BIT(7)
+#define STAT_PAUSED BIT(6)
+#define STAT_TRIG   BIT(3)
+#define STAT_STOP   BIT(2)
+#define STAT_EOL    BIT(1)
+#define STAT_FLAG   BIT(0)
+
+#define CNTL_START  BIT(7)
+#define CNTL_TERM   BIT(6)
+#define CNTL_PAUSE  BIT(3)
+
+static void open_voice_out(ViaAC97State *s);
+
+static uint16_t codec_rates[] = { 8000, 11025, 16000, 22050, 32000, 44100,
+                                  48000 };
+
+#define CODEC_REG(s, o)  ((s)->codec_regs[(o) / 2])
+#define CODEC_VOL(vol, mask)  ((255 * ((vol) & mask)) / mask)
+
+static void codec_volume_set_out(ViaAC97State *s)
+{
+    int lvol, rvol, mute;
+
+    lvol = 255 - CODEC_VOL(CODEC_REG(s, AC97_Master_Volume_Mute) >> 8, 0x1f);
+    lvol *= 255 - CODEC_VOL(CODEC_REG(s, AC97_PCM_Out_Volume_Mute) >> 8, 0x1f);
+    lvol /= 255;
+    rvol = 255 - CODEC_VOL(CODEC_REG(s, AC97_Master_Volume_Mute), 0x1f);
+    rvol *= 255 - CODEC_VOL(CODEC_REG(s, AC97_PCM_Out_Volume_Mute), 0x1f);
+    rvol /= 255;
+    mute = CODEC_REG(s, AC97_Master_Volume_Mute) >> MUTE_SHIFT;
+    mute |= CODEC_REG(s, AC97_PCM_Out_Volume_Mute) >> MUTE_SHIFT;
+    AUD_set_volume_out(s->vo, mute, lvol, rvol);
+}
+
+static void codec_reset(ViaAC97State *s)
+{
+    memset(s->codec_regs, 0, sizeof(s->codec_regs));
+    CODEC_REG(s, AC97_Reset) = 0x6a90;
+    CODEC_REG(s, AC97_Master_Volume_Mute) = 0x8000;
+    CODEC_REG(s, AC97_Headphone_Volume_Mute) = 0x8000;
+    CODEC_REG(s, AC97_Master_Volume_Mono_Mute) = 0x8000;
+    CODEC_REG(s, AC97_Phone_Volume_Mute) = 0x8008;
+    CODEC_REG(s, AC97_Mic_Volume_Mute) = 0x8008;
+    CODEC_REG(s, AC97_Line_In_Volume_Mute) = 0x8808;
+    CODEC_REG(s, AC97_CD_Volume_Mute) = 0x8808;
+    CODEC_REG(s, AC97_Video_Volume_Mute) = 0x8808;
+    CODEC_REG(s, AC97_Aux_Volume_Mute) = 0x8808;
+    CODEC_REG(s, AC97_PCM_Out_Volume_Mute) = 0x8808;
+    CODEC_REG(s, AC97_Record_Gain_Mute) = 0x8000;
+    CODEC_REG(s, AC97_Powerdown_Ctrl_Stat) = 0x000f;
+    CODEC_REG(s, AC97_Extended_Audio_ID) = 0x0a05;
+    CODEC_REG(s, AC97_Extended_Audio_Ctrl_Stat) = 0x0400;
+    CODEC_REG(s, AC97_PCM_Front_DAC_Rate) = 48000;
+    CODEC_REG(s, AC97_PCM_LR_ADC_Rate) = 48000;
+    /* Sigmatel 9766 (STAC9766) */
+    CODEC_REG(s, AC97_Vendor_ID1) = 0x8384;
+    CODEC_REG(s, AC97_Vendor_ID2) = 0x7666;
+}
+
+static uint16_t codec_read(ViaAC97State *s, uint8_t addr)
+{
+    return CODEC_REG(s, addr);
+}
+
+static void codec_write(ViaAC97State *s, uint8_t addr, uint16_t val)
+{
+    trace_via_ac97_codec_write(addr, val);
+    switch (addr) {
+    case AC97_Reset:
+        codec_reset(s);
+        return;
+    case AC97_Master_Volume_Mute:
+    case AC97_PCM_Out_Volume_Mute:
+        if (addr == AC97_Master_Volume_Mute) {
+            if (val & BIT(13)) {
+                val |= 0x1f00;
+            }
+            if (val & BIT(5)) {
+                val |= 0x1f;
+            }
+        }
+        CODEC_REG(s, addr) = val & 0x9f1f;
+        codec_volume_set_out(s);
+        return;
+    case AC97_Extended_Audio_Ctrl_Stat:
+        CODEC_REG(s, addr) &= ~EACS_VRA;
+        CODEC_REG(s, addr) |= val & EACS_VRA;
+        if (!(val & EACS_VRA)) {
+            CODEC_REG(s, AC97_PCM_Front_DAC_Rate) = 48000;
+            CODEC_REG(s, AC97_PCM_LR_ADC_Rate) = 48000;
+            open_voice_out(s);
+        }
+        return;
+    case AC97_PCM_Front_DAC_Rate:
+    case AC97_PCM_LR_ADC_Rate:
+        if (CODEC_REG(s, AC97_Extended_Audio_Ctrl_Stat) & EACS_VRA) {
+            int i;
+            uint16_t rate = val;
+
+            for (i = 0; i < ARRAY_SIZE(codec_rates) - 1; i++) {
+                if (rate < codec_rates[i] +
+                    (codec_rates[i + 1] - codec_rates[i]) / 2) {
+                    rate = codec_rates[i];
+                    break;
+                }
+            }
+            if (rate > 48000) {
+                rate = 48000;
+            }
+            CODEC_REG(s, addr) = rate;
+            open_voice_out(s);
+        }
+        return;
+    case AC97_Powerdown_Ctrl_Stat:
+        CODEC_REG(s, addr) = (val & 0xff00) | (CODEC_REG(s, addr) & 0xff);
+        return;
+    case AC97_Extended_Audio_ID:
+    case AC97_Vendor_ID1:
+    case AC97_Vendor_ID2:
+        /* Read only registers */
+        return;
+    default:
+        qemu_log_mask(LOG_UNIMP,
+                      "via-ac97: Unimplemented codec register 0x%x\n", addr);
+        CODEC_REG(s, addr) = val;
+    }
+}
+
+static void fetch_sgd(ViaAC97SGDChannel *c, PCIDevice *d)
+{
+    uint32_t b[2];
+
+    if (c->curr < c->base) {
+        c->curr = c->base;
+    }
+    if (unlikely(pci_dma_read(d, c->curr, b, sizeof(b)) != MEMTX_OK)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "via-ac97: DMA error reading SGD table\n");
+        return;
+    }
+    c->addr = le32_to_cpu(b[0]);
+    c->clen = le32_to_cpu(b[1]);
+    trace_via_ac97_sgd_fetch(c->curr, c->addr, CLEN_IS_STOP(c) ? 'S' : '-',
+                             CLEN_IS_EOL(c) ? 'E' : '-',
+                             CLEN_IS_FLAG(c) ? 'F' : '-', CLEN_LEN(c));
+}
+
+static void out_cb(void *opaque, int avail)
+{
+    ViaAC97State *s = opaque;
+    ViaAC97SGDChannel *c = &s->aur;
+    int temp, to_copy, copied;
+    bool stop = false;
+    uint8_t tmpbuf[4096];
+
+    if (c->stat & STAT_PAUSED) {
+        return;
+    }
+    c->stat |= STAT_ACTIVE;
+    while (avail && !stop) {
+        if (!c->clen) {
+            fetch_sgd(c, &s->dev);
+        }
+        temp = MIN(CLEN_LEN(c), avail);
+        while (temp) {
+            to_copy = MIN(temp, sizeof(tmpbuf));
+            pci_dma_read(&s->dev, c->addr, tmpbuf, to_copy);
+            copied = AUD_write(s->vo, tmpbuf, to_copy);
+            if (!copied) {
+                stop = true;
+                break;
+            }
+            temp -= copied;
+            avail -= copied;
+            c->addr += copied;
+            c->clen -= copied;
+        }
+        if (CLEN_LEN(c) == 0) {
+            c->curr += 8;
+            if (CLEN_IS_EOL(c)) {
+                c->stat |= STAT_EOL;
+                if (c->type & CNTL_START) {
+                    c->curr = c->base;
+                    c->stat |= STAT_PAUSED;
+                } else {
+                    c->stat &= ~STAT_ACTIVE;
+                    AUD_set_active_out(s->vo, 0);
+                }
+                if (c->type & STAT_EOL) {
+                    pci_set_irq(&s->dev, 1);
+                }
+            }
+            if (CLEN_IS_FLAG(c)) {
+                c->stat |= STAT_FLAG;
+                c->stat |= STAT_PAUSED;
+                if (c->type & STAT_FLAG) {
+                    pci_set_irq(&s->dev, 1);
+                }
+            }
+            if (CLEN_IS_STOP(c)) {
+                c->stat |= STAT_STOP;
+                c->stat |= STAT_PAUSED;
+            }
+            c->clen = 0;
+            stop = true;
+        }
+    }
+}
+
+static void open_voice_out(ViaAC97State *s)
+{
+    struct audsettings as = {
+        .freq = CODEC_REG(s, AC97_PCM_Front_DAC_Rate),
+        .nchannels = s->aur.type & BIT(4) ? 2 : 1,
+        .fmt = s->aur.type & BIT(5) ? AUDIO_FORMAT_S16 : AUDIO_FORMAT_S8,
+        .endianness = 0,
+    };
+    s->vo = AUD_open_out(&s->card, s->vo, "via-ac97.out", s, out_cb, &as);
+}
+
+static uint64_t sgd_read(void *opaque, hwaddr addr, unsigned size)
+{
+    ViaAC97State *s = opaque;
+    uint64_t val = 0;
+
+    switch (addr) {
+    case 0:
+        val = s->aur.stat;
+        if (s->aur.type & CNTL_START) {
+            val |= STAT_TRIG;
+        }
+        break;
+    case 1:
+        val = s->aur.stat & STAT_PAUSED ? BIT(3) : 0;
+        break;
+    case 2:
+        val = s->aur.type;
+        break;
+    case 4:
+        val = s->aur.curr;
+        break;
+    case 0xc:
+        val = CLEN_LEN(&s->aur);
+        break;
+    case 0x10:
+        /* silence unimplemented log message that happens at every IRQ */
+        break;
+    case 0x80:
+        val = s->ac97_cmd;
+        break;
+    case 0x84:
+        val = s->aur.stat & STAT_FLAG;
+        if (s->aur.stat & STAT_EOL) {
+            val |= BIT(4);
+        }
+        if (s->aur.stat & STAT_STOP) {
+            val |= BIT(8);
+        }
+        if (s->aur.stat & STAT_ACTIVE) {
+            val |= BIT(12);
+        }
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP, "via-ac97: Unimplemented register read 0x%"
+                      HWADDR_PRIx"\n", addr);
+    }
+    trace_via_ac97_sgd_read(addr, size, val);
+    return val;
+}
+
+static void sgd_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+    ViaAC97State *s = opaque;
+
+    trace_via_ac97_sgd_write(addr, size, val);
+    switch (addr) {
+    case 0:
+        if (val & STAT_STOP) {
+            s->aur.stat &= ~STAT_PAUSED;
+        }
+        if (val & STAT_EOL) {
+            s->aur.stat &= ~(STAT_EOL | STAT_PAUSED);
+            if (s->aur.type & STAT_EOL) {
+                pci_set_irq(&s->dev, 0);
+            }
+        }
+        if (val & STAT_FLAG) {
+            s->aur.stat &= ~(STAT_FLAG | STAT_PAUSED);
+            if (s->aur.type & STAT_FLAG) {
+                pci_set_irq(&s->dev, 0);
+            }
+        }
+        break;
+    case 1:
+        if (val & CNTL_START) {
+            AUD_set_active_out(s->vo, 1);
+            s->aur.stat = STAT_ACTIVE;
+        }
+        if (val & CNTL_TERM) {
+            AUD_set_active_out(s->vo, 0);
+            s->aur.stat &= ~(STAT_ACTIVE | STAT_PAUSED);
+            s->aur.clen = 0;
+        }
+        if (val & CNTL_PAUSE) {
+            AUD_set_active_out(s->vo, 0);
+            s->aur.stat &= ~STAT_ACTIVE;
+            s->aur.stat |= STAT_PAUSED;
+        } else if (!(val & CNTL_PAUSE) && (s->aur.stat & STAT_PAUSED)) {
+            AUD_set_active_out(s->vo, 1);
+            s->aur.stat |= STAT_ACTIVE;
+            s->aur.stat &= ~STAT_PAUSED;
+        }
+        break;
+    case 2:
+    {
+        uint32_t oldval = s->aur.type;
+        s->aur.type = val;
+        if ((oldval & 0x30) != (val & 0x30)) {
+            open_voice_out(s);
+        }
+        break;
+    }
+    case 4:
+        s->aur.base = val & ~1ULL;
+        s->aur.curr = s->aur.base;
+        break;
+    case 0x80:
+        if (val >> 30) {
+            /* we only have primary codec */
+            break;
+        }
+        if (val & BIT(23)) { /* read reg */
+            s->ac97_cmd = val & 0xc0ff0000ULL;
+            s->ac97_cmd |= codec_read(s, (val >> 16) & 0x7f);
+            s->ac97_cmd |= BIT(25); /* data valid */
+        } else {
+            s->ac97_cmd = val & 0xc0ffffffULL;
+            codec_write(s, (val >> 16) & 0x7f, val);
+        }
+        break;
+    case 0xc:
+    case 0x84:
+        /* Read only */
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP, "via-ac97: Unimplemented register write 0x%"
+                      HWADDR_PRIx"\n", addr);
+    }
+}
+
+static const MemoryRegionOps sgd_ops = {
+    .read = sgd_read,
+    .write = sgd_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static uint64_t fm_read(void *opaque, hwaddr addr, unsigned size)
+{
+    qemu_log_mask(LOG_UNIMP, "%s: 0x%"HWADDR_PRIx" %d\n", __func__, addr, size);
+    return 0;
+}
+
+static void fm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+    qemu_log_mask(LOG_UNIMP, "%s: 0x%"HWADDR_PRIx" %d <= 0x%"PRIX64"\n",
+                  __func__, addr, size, val);
+}
+
+static const MemoryRegionOps fm_ops = {
+    .read = fm_read,
+    .write = fm_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static uint64_t midi_read(void *opaque, hwaddr addr, unsigned size)
+{
+    qemu_log_mask(LOG_UNIMP, "%s: 0x%"HWADDR_PRIx" %d\n", __func__, addr, size);
+    return 0;
+}
+
+static void midi_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+    qemu_log_mask(LOG_UNIMP, "%s: 0x%"HWADDR_PRIx" %d <= 0x%"PRIX64"\n",
+                  __func__, addr, size, val);
+}
+
+static const MemoryRegionOps midi_ops = {
+    .read = midi_read,
+    .write = midi_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void via_ac97_reset(DeviceState *dev)
+{
+    ViaAC97State *s = VIA_AC97(dev);
+
+    codec_reset(s);
+}
 
 static void via_ac97_realize(PCIDevice *pci_dev, Error **errp)
 {
-    pci_set_word(pci_dev->config + PCI_COMMAND,
-                 PCI_COMMAND_INVALIDATE | PCI_COMMAND_PARITY);
+    ViaAC97State *s = VIA_AC97(pci_dev);
+    Object *o = OBJECT(s);
+
+    /*
+     * Command register Bus Master bit is documented to be fixed at 0 but it's
+     * needed for PCI DMA to work in QEMU. The pegasos2 firmware writes 0 here
+     * and the AmigaOS driver writes 1 only enabling IO bit which works on
+     * real hardware. So set it here and fix it to 1 to allow DMA.
+     */
+    pci_set_word(pci_dev->config + PCI_COMMAND, PCI_COMMAND_MASTER);
+    pci_set_word(pci_dev->wmask + PCI_COMMAND, PCI_COMMAND_IO);
     pci_set_word(pci_dev->config + PCI_STATUS,
                  PCI_STATUS_CAP_LIST | PCI_STATUS_DEVSEL_MEDIUM);
     pci_set_long(pci_dev->config + PCI_INTERRUPT_PIN, 0x03);
+    pci_set_byte(pci_dev->config + 0x40, 1); /* codec ready */
+
+    memory_region_init_io(&s->sgd, o, &sgd_ops, s, "via-ac97.sgd", 256);
+    pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->sgd);
+    memory_region_init_io(&s->fm, o, &fm_ops, s, "via-ac97.fm", 4);
+    pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->fm);
+    memory_region_init_io(&s->midi, o, &midi_ops, s, "via-ac97.midi", 4);
+    pci_register_bar(pci_dev, 2, PCI_BASE_ADDRESS_SPACE_IO, &s->midi);
+
+    AUD_register_card ("via-ac97", &s->card);
 }
 
+static void via_ac97_exit(PCIDevice *dev)
+{
+    ViaAC97State *s = VIA_AC97(dev);
+
+    AUD_close_out(&s->card, s->vo);
+    AUD_remove_card(&s->card);
+}
+
+static Property via_ac97_properties[] = {
+    DEFINE_AUDIO_PROPERTIES(ViaAC97State, card),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void via_ac97_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->realize = via_ac97_realize;
+    k->exit = via_ac97_exit;
     k->vendor_id = PCI_VENDOR_ID_VIA;
     k->device_id = PCI_DEVICE_ID_VIA_AC97;
     k->revision = 0x50;
     k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
+    device_class_set_props(dc, via_ac97_properties);
     set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
     dc->desc = "VIA AC97";
+    dc->reset = via_ac97_reset;
     /* Reason: Part of a south bridge chip */
     dc->user_creatable = false;
 }
@@ -41,7 +484,7 @@ static void via_ac97_class_init(ObjectClass *klass, void *data)
 static const TypeInfo via_ac97_info = {
     .name          = TYPE_VIA_AC97,
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PCIDevice),
+    .instance_size = sizeof(ViaAC97State),
     .class_init    = via_ac97_class_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
diff --git a/hw/isa/trace-events b/hw/isa/trace-events
index c4567a9b47..1816e8307a 100644
--- a/hw/isa/trace-events
+++ b/hw/isa/trace-events
@@ -16,6 +16,7 @@ apm_io_write(uint8_t addr, uint8_t val) "write addr=0x%x val=0x%02x"
 
 # vt82c686.c
 via_isa_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
+via_pm_read(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
 via_pm_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
 via_pm_io_read(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
 via_pm_io_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 4025f9bcdc..429677d213 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -554,7 +554,7 @@ struct ViaISAState {
     PCIIDEState ide;
     UHCIState uhci[2];
     ViaPMState pm;
-    PCIDevice ac97;
+    ViaAC97State ac97;
     PCIDevice mc97;
 };
 
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index e273cd38dc..da1722daf2 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -1,6 +1,8 @@
 #ifndef HW_VT82C686_H
 #define HW_VT82C686_H
 
+#include "hw/pci/pci_device.h"
+#include "audio/audio.h"
 
 #define TYPE_VT82C686B_ISA "vt82c686b-isa"
 #define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
@@ -9,6 +11,29 @@
 #define TYPE_VIA_IDE "via-ide"
 #define TYPE_VIA_MC97 "via-mc97"
 
+typedef struct {
+    uint8_t stat;
+    uint8_t type;
+    uint32_t base;
+    uint32_t curr;
+    uint32_t addr;
+    uint32_t clen;
+} ViaAC97SGDChannel;
+
+OBJECT_DECLARE_SIMPLE_TYPE(ViaAC97State, VIA_AC97);
+
+struct ViaAC97State {
+    PCIDevice dev;
+    QEMUSoundCard card;
+    MemoryRegion sgd;
+    MemoryRegion fm;
+    MemoryRegion midi;
+    SWVoiceOut *vo;
+    ViaAC97SGDChannel aur;
+    uint16_t codec_regs[128];
+    uint32_t ac97_cmd;
+};
+
 void via_isa_set_irq(PCIDevice *d, int n, int level);
 
 #endif
-- 
2.30.7

[PATCH v3 7/8] hw/audio/ac97: Split off some definitions to a header

[BALATON Zoltan]

These can be shared with other AC97 implementations.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/audio/ac97.c | 43 +-------------------------------
 hw/audio/ac97.h | 65 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 42 deletions(-)
 create mode 100644 hw/audio/ac97.h

diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 364cdfa733..b3fb10284c 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -26,43 +26,7 @@
 #include "qemu/module.h"
 #include "sysemu/dma.h"
 #include "qom/object.h"
-
-enum {
-    AC97_Reset                     = 0x00,
-    AC97_Master_Volume_Mute        = 0x02,
-    AC97_Headphone_Volume_Mute     = 0x04,
-    AC97_Master_Volume_Mono_Mute   = 0x06,
-    AC97_Master_Tone_RL            = 0x08,
-    AC97_PC_BEEP_Volume_Mute       = 0x0A,
-    AC97_Phone_Volume_Mute         = 0x0C,
-    AC97_Mic_Volume_Mute           = 0x0E,
-    AC97_Line_In_Volume_Mute       = 0x10,
-    AC97_CD_Volume_Mute            = 0x12,
-    AC97_Video_Volume_Mute         = 0x14,
-    AC97_Aux_Volume_Mute           = 0x16,
-    AC97_PCM_Out_Volume_Mute       = 0x18,
-    AC97_Record_Select             = 0x1A,
-    AC97_Record_Gain_Mute          = 0x1C,
-    AC97_Record_Gain_Mic_Mute      = 0x1E,
-    AC97_General_Purpose           = 0x20,
-    AC97_3D_Control                = 0x22,
-    AC97_AC_97_RESERVED            = 0x24,
-    AC97_Powerdown_Ctrl_Stat       = 0x26,
-    AC97_Extended_Audio_ID         = 0x28,
-    AC97_Extended_Audio_Ctrl_Stat  = 0x2A,
-    AC97_PCM_Front_DAC_Rate        = 0x2C,
-    AC97_PCM_Surround_DAC_Rate     = 0x2E,
-    AC97_PCM_LFE_DAC_Rate          = 0x30,
-    AC97_PCM_LR_ADC_Rate           = 0x32,
-    AC97_MIC_ADC_Rate              = 0x34,
-    AC97_6Ch_Vol_C_LFE_Mute        = 0x36,
-    AC97_6Ch_Vol_L_R_Surround_Mute = 0x38,
-    AC97_Vendor_Reserved           = 0x58,
-    AC97_Sigmatel_Analog           = 0x6c, /* We emulate a Sigmatel codec */
-    AC97_Sigmatel_Dac2Invert       = 0x6e, /* We emulate a Sigmatel codec */
-    AC97_Vendor_ID1                = 0x7c,
-    AC97_Vendor_ID2                = 0x7e
-};
+#include "ac97.h"
 
 #define SOFT_VOLUME
 #define SR_FIFOE 16             /* rwc */
@@ -121,11 +85,6 @@ enum {
 #define BD_IOC (1 << 31)
 #define BD_BUP (1 << 30)
 
-#define EACS_VRA 1
-#define EACS_VRM 8
-
-#define MUTE_SHIFT 15
-
 #define TYPE_AC97 "AC97"
 OBJECT_DECLARE_SIMPLE_TYPE(AC97LinkState, AC97)
 
diff --git a/hw/audio/ac97.h b/hw/audio/ac97.h
new file mode 100644
index 0000000000..0358b56ff4
--- /dev/null
+++ b/hw/audio/ac97.h
@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2006 InnoTek Systemberatung GmbH
+ *
+ * This file is part of VirtualBox Open Source Edition (OSE), as
+ * available from http://www.virtualbox.org. This file is free software;
+ * you can redistribute it and/or modify it under the terms of the GNU
+ * General Public License as published by the Free Software Foundation,
+ * in version 2 as it comes in the "COPYING" file of the VirtualBox OSE
+ * distribution. VirtualBox OSE is distributed in the hope that it will
+ * be useful, but WITHOUT ANY WARRANTY of any kind.
+ *
+ * If you received this file as part of a commercial VirtualBox
+ * distribution, then only the terms of your commercial VirtualBox
+ * license agreement apply instead of the previous paragraph.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#ifndef AC97_H
+#define AC97_H
+
+enum {
+    AC97_Reset                     = 0x00,
+    AC97_Master_Volume_Mute        = 0x02,
+    AC97_Headphone_Volume_Mute     = 0x04,
+    AC97_Master_Volume_Mono_Mute   = 0x06,
+    AC97_Master_Tone_RL            = 0x08,
+    AC97_PC_BEEP_Volume_Mute       = 0x0A,
+    AC97_Phone_Volume_Mute         = 0x0C,
+    AC97_Mic_Volume_Mute           = 0x0E,
+    AC97_Line_In_Volume_Mute       = 0x10,
+    AC97_CD_Volume_Mute            = 0x12,
+    AC97_Video_Volume_Mute         = 0x14,
+    AC97_Aux_Volume_Mute           = 0x16,
+    AC97_PCM_Out_Volume_Mute       = 0x18,
+    AC97_Record_Select             = 0x1A,
+    AC97_Record_Gain_Mute          = 0x1C,
+    AC97_Record_Gain_Mic_Mute      = 0x1E,
+    AC97_General_Purpose           = 0x20,
+    AC97_3D_Control                = 0x22,
+    AC97_AC_97_RESERVED            = 0x24,
+    AC97_Powerdown_Ctrl_Stat       = 0x26,
+    AC97_Extended_Audio_ID         = 0x28,
+    AC97_Extended_Audio_Ctrl_Stat  = 0x2A,
+    AC97_PCM_Front_DAC_Rate        = 0x2C,
+    AC97_PCM_Surround_DAC_Rate     = 0x2E,
+    AC97_PCM_LFE_DAC_Rate          = 0x30,
+    AC97_PCM_LR_ADC_Rate           = 0x32,
+    AC97_MIC_ADC_Rate              = 0x34,
+    AC97_6Ch_Vol_C_LFE_Mute        = 0x36,
+    AC97_6Ch_Vol_L_R_Surround_Mute = 0x38,
+    AC97_Vendor_Reserved           = 0x58,
+    AC97_Sigmatel_Analog           = 0x6c, /* We emulate a Sigmatel codec */
+    AC97_Sigmatel_Dac2Invert       = 0x6e, /* We emulate a Sigmatel codec */
+    AC97_Vendor_ID1                = 0x7c,
+    AC97_Vendor_ID2                = 0x7e
+};
+
+#define EACS_VRA 1
+#define EACS_VRM 8
+
+#define MUTE_SHIFT 15
+
+#endif /* AC97_H */
-- 
2.30.7

[PATCH v3 1/8] hw/display/sm501: Implement more 2D raster operations

[BALATON Zoltan]

Add simple implementation for two raster operations that are used by
AmigaOS which fixes graphics problems in some programs using these.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reported-by: Rene Engel <ReneEngel80@emailn.de>
Tested-by: Rene Engel <ReneEngel80@emailn.de>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
These are documented for example at:
https://learn.microsoft.com/en-us/windows/win32/gdi/ternary-raster-operations

 hw/display/sm501.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index e1d0591d36..58bc9701ee 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -753,7 +753,7 @@ static void sm501_2d_operation(SM501State *s)
         }
 
         if ((rop_mode && rop == 0x5) || (!rop_mode && rop == 0x55)) {
-            /* Invert dest, is there a way to do this with pixman? */
+            /* DSTINVERT, is there a way to do this with pixman? */
             unsigned int x, y, i;
             uint8_t *d = s->local_mem + dst_base;
 
@@ -763,6 +763,34 @@ static void sm501_2d_operation(SM501State *s)
                     stn_he_p(&d[i], bypp, ~ldn_he_p(&d[i], bypp));
                 }
             }
+        } else if (!rop_mode && rop == 0x99) {
+            /* DSxn, is there a way to do this with pixman? */
+            unsigned int x, y, i, j;
+            uint8_t *sp = s->local_mem + src_base;
+            uint8_t *d = s->local_mem + dst_base;
+
+            for (y = 0; y < height; y++) {
+                i = (dst_x + (dst_y + y) * dst_pitch) * bypp;
+                j = (src_x + (src_y + y) * src_pitch) * bypp;
+                for (x = 0; x < width; x++, i += bypp, j += bypp) {
+                    stn_he_p(&d[i], bypp,
+                             ~(ldn_he_p(&sp[j], bypp) ^ ldn_he_p(&d[i], bypp)));
+                }
+            }
+        } else if (!rop_mode && rop == 0xee) {
+            /* SRCPAINT, is there a way to do this with pixman? */
+            unsigned int x, y, i, j;
+            uint8_t *sp = s->local_mem + src_base;
+            uint8_t *d = s->local_mem + dst_base;
+
+            for (y = 0; y < height; y++) {
+                i = (dst_x + (dst_y + y) * dst_pitch) * bypp;
+                j = (src_x + (src_y + y) * src_pitch) * bypp;
+                for (x = 0; x < width; x++, i += bypp, j += bypp) {
+                    stn_he_p(&d[i], bypp,
+                             ldn_he_p(&sp[j], bypp) | ldn_he_p(&d[i], bypp));
+                }
+            }
         } else {
             /* Do copy src for unimplemented ops, better than unpainted area */
             if ((rop_mode && (rop != 0xc || rop2_source_is_pattern)) ||
-- 
2.30.7

[PATCH v3 5/8] hw/ppc/pegasos2: Fix PCI interrupt routing

[BALATON Zoltan]

According to the PegasosII schematics the PCI interrupt lines are
connected to both the gpp pins of the Mv64361 north bridge and the
PINT pins of the VT8231 south bridge so guests can get interrupts from
either of these. So far we only had the MV64361 connections which
worked for on board devices but for additional PCI devices (such as
network or sound card added with -device) guest OSes expect interrupt
from the ISA IRQ 9 where the firmware routes these PCI interrupts in
VT8231 ISA bridge. After the previous patches we can now model this
and also remove the board specific connection from mv64361. Also
configure routing of these lines when using Virtual Open Firmware to
match board firmware for guests that expect this.

This fixes PCI interrupts on pegasos2 under Linux, MorphOS and AmigaOS.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Rene Engel <ReneEngel80@emailn.de>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/pci-host/mv64361.c |  4 ----
 hw/ppc/pegasos2.c     | 26 +++++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/hw/pci-host/mv64361.c b/hw/pci-host/mv64361.c
index f43f33fbd9..3d9132f989 100644
--- a/hw/pci-host/mv64361.c
+++ b/hw/pci-host/mv64361.c
@@ -874,10 +874,6 @@ static void mv64361_realize(DeviceState *dev, Error **errp)
     }
     sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->cpu_irq);
     qdev_init_gpio_in_named(dev, mv64361_gpp_irq, "gpp", 32);
-    /* FIXME: PCI IRQ connections may be board specific */
-    for (i = 0; i < PCI_NUM_PINS; i++) {
-        s->pci[1].irq[i] = qdev_get_gpio_in_named(dev, "gpp", 12 + i);
-    }
 }
 
 static void mv64361_reset(DeviceState *dev)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index a9563f4fb2..4e1476673b 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -74,6 +74,8 @@ struct Pegasos2MachineState {
     MachineState parent_obj;
     PowerPCCPU *cpu;
     DeviceState *mv;
+    qemu_irq mv_pirq[PCI_NUM_PINS];
+    qemu_irq via_pirq[PCI_NUM_PINS];
     Vof *vof;
     void *fdt_blob;
     uint64_t kernel_addr;
@@ -96,6 +98,15 @@ static void pegasos2_cpu_reset(void *opaque)
     }
 }
 
+static void pegasos2_pci_irq(void *opaque, int n, int level)
+{
+    Pegasos2MachineState *pm = opaque;
+
+    /* PCI interrupt lines are connected to both MV64361 and VT8231 */
+    qemu_set_irq(pm->mv_pirq[n], level);
+    qemu_set_irq(pm->via_pirq[n], level);
+}
+
 static void pegasos2_init(MachineState *machine)
 {
     Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine);
@@ -107,7 +118,7 @@ static void pegasos2_init(MachineState *machine)
     I2CBus *i2c_bus;
     const char *fwname = machine->firmware ?: PROM_FILENAME;
     char *filename;
-    int sz;
+    int i, sz;
     uint8_t *spd_data;
 
     /* init CPU */
@@ -157,11 +168,18 @@ static void pegasos2_init(MachineState *machine)
     /* Marvell Discovery II system controller */
     pm->mv = DEVICE(sysbus_create_simple(TYPE_MV64361, -1,
                           qdev_get_gpio_in(DEVICE(pm->cpu), PPC6xx_INPUT_INT)));
+    for (i = 0; i < PCI_NUM_PINS; i++) {
+        pm->mv_pirq[i] = qdev_get_gpio_in_named(pm->mv, "gpp", 12 + i);
+    }
     pci_bus = mv64361_get_pci_bus(pm->mv, 1);
+    pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
 
     /* VIA VT8231 South Bridge (multifunction PCI device) */
     via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
                                                  true, TYPE_VT8231_ISA));
+    for (i = 0; i < PCI_NUM_PINS; i++) {
+        pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
+    }
     object_property_add_alias(OBJECT(machine), "rtc-time",
                               object_resolve_path_component(via, "rtc"),
                               "date");
@@ -268,6 +286,12 @@ static void pegasos2_machine_reset(MachineState *machine, ShutdownCause reason)
                               PCI_INTERRUPT_LINE, 2, 0x9);
     pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
                               0x50, 1, 0x2);
+    pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
+                              0x55, 1, 0x90);
+    pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
+                              0x56, 1, 0x99);
+    pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
+                              0x57, 1, 0x90);
 
     pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 1) << 8) |
                               PCI_INTERRUPT_LINE, 2, 0x109);
-- 
2.30.7

[PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

From: Bernhard Beschow <shentey@gmail.com>

The real VIA south bridges implement a PCI IRQ router which is configured
by the BIOS or the OS. In order to respect these configurations, QEMU
needs to implement it as well.

Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
[balaton: declare gpio inputs instead of changing pci bus irqs so it can
 be connected in board code; remove some empty lines]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Rene Engel <ReneEngel80@emailn.de>
---
 hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 3f9bd0c04d..4025f9bcdc 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
     qemu_set_irq(s->cpu_intr, level);
 }
 
+static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
+{
+    switch (irq_num) {
+    case 0:
+        return s->dev.config[0x55] >> 4;
+    case 1:
+        return s->dev.config[0x56] & 0xf;
+    case 2:
+        return s->dev.config[0x56] >> 4;
+    case 3:
+        return s->dev.config[0x57] >> 4;
+    }
+    return 0;
+}
+
+static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
+{
+    ViaISAState *s = opaque;
+    PCIBus *bus = pci_get_bus(&s->dev);
+    int pic_irq;
+
+    /* now we change the pic irq level according to the via irq mappings */
+    /* XXX: optimize */
+    pic_irq = via_isa_get_pci_irq(s, irq_num);
+    if (pic_irq < ISA_NUM_IRQS) {
+        int i, pic_level;
+
+        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
+        pic_level = 0;
+        for (i = 0; i < PCI_NUM_PINS; i++) {
+            if (pic_irq == via_isa_get_pci_irq(s, i)) {
+                pic_level |= pci_bus_get_irq_level(bus, i);
+            }
+        }
+        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
+    }
+}
+
 static void via_isa_realize(PCIDevice *d, Error **errp)
 {
     ViaISAState *s = VIA_ISA(d);
@@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     int i;
 
     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
+    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", PCI_NUM_PINS);
     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
                           errp);
-- 
2.30.7

[PATCH v3 6/8] hw/usb/vt82c686-uhci-pci: Use PCI IRQ routing

[BALATON Zoltan]

From: Bernhard Beschow <shentey@gmail.com>

According to the PCI specification, PCI_INTERRUPT_LINE shall have no
effect on hardware operations. Now that the VIA south bridges implement
the internal PCI interrupt router let's be more conformant to the PCI
specification.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Rene Engel <ReneEngel80@emailn.de>
---
 hw/usb/vt82c686-uhci-pci.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/hw/usb/vt82c686-uhci-pci.c b/hw/usb/vt82c686-uhci-pci.c
index 46a901f56f..b4884c9011 100644
--- a/hw/usb/vt82c686-uhci-pci.c
+++ b/hw/usb/vt82c686-uhci-pci.c
@@ -1,17 +1,7 @@
 #include "qemu/osdep.h"
-#include "hw/irq.h"
 #include "hw/isa/vt82c686.h"
 #include "hcd-uhci.h"
 
-static void uhci_isa_set_irq(void *opaque, int irq_num, int level)
-{
-    UHCIState *s = opaque;
-    uint8_t irq = pci_get_byte(s->dev.config + PCI_INTERRUPT_LINE);
-    if (irq > 0 && irq < 15) {
-        via_isa_set_irq(pci_get_function_0(&s->dev), irq, level);
-    }
-}
-
 static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
 {
     UHCIState *s = UHCI(dev);
@@ -25,8 +15,6 @@ static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
     pci_set_long(pci_conf + 0xc0, 0x00002000);
 
     usb_uhci_common_realize(dev, errp);
-    object_unref(s->irq);
-    s->irq = qemu_allocate_irq(uhci_isa_set_irq, s, 0);
 }
 
 static UHCIInfo uhci_info[] = {
-- 
2.30.7

[PATCH v3 2/8] hw/display/sm501: Add fallbacks to pixman routines

[BALATON Zoltan]

Pixman may return false if it does not have a suitable implementation.
Add fallbacks to handle such cases.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reported-by: Rene Engel <ReneEngel80@emailn.de>
Tested-by: Rene Engel <ReneEngel80@emailn.de>
---
 hw/display/sm501.c | 75 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 52 insertions(+), 23 deletions(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 58bc9701ee..23c4418e19 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -691,7 +691,7 @@ static void sm501_2d_operation(SM501State *s)
     unsigned int dst_pitch = (s->twoD_pitch >> 16) & 0x1FFF;
     int crt = (s->dc_crt_control & SM501_DC_CRT_CONTROL_SEL) ? 1 : 0;
     int fb_len = get_width(s, crt) * get_height(s, crt) * get_bpp(s, crt);
-    bool overlap = false;
+    bool overlap = false, fallback = false;
 
     if ((s->twoD_stretch >> 16) & 0xF) {
         qemu_log_mask(LOG_UNIMP, "sm501: only XY addressing is supported.\n");
@@ -834,25 +834,48 @@ static void sm501_2d_operation(SM501State *s)
                 if (tmp_stride * sizeof(uint32_t) * height > sizeof(tmp_buf)) {
                     tmp = g_malloc(tmp_stride * sizeof(uint32_t) * height);
                 }
-                pixman_blt((uint32_t *)&s->local_mem[src_base], tmp,
-                           src_pitch * bypp / sizeof(uint32_t),
-                           tmp_stride, 8 * bypp, 8 * bypp,
-                           src_x, src_y, 0, 0, width, height);
-                pixman_blt(tmp, (uint32_t *)&s->local_mem[dst_base],
-                           tmp_stride,
-                           dst_pitch * bypp / sizeof(uint32_t),
-                           8 * bypp, 8 * bypp,
-                           0, 0, dst_x, dst_y, width, height);
+                fallback = !pixman_blt((uint32_t *)&s->local_mem[src_base],
+                                       tmp,
+                                       src_pitch * bypp / sizeof(uint32_t),
+                                       tmp_stride,
+                                       8 * bypp, 8 * bypp,
+                                       src_x, src_y, 0, 0, width, height);
+                if (!fallback) {
+                    fallback = !pixman_blt(tmp,
+                                       (uint32_t *)&s->local_mem[dst_base],
+                                       tmp_stride,
+                                       dst_pitch * bypp / sizeof(uint32_t),
+                                       8 * bypp, 8 * bypp,
+                                       0, 0, dst_x, dst_y, width, height);
+                }
                 if (tmp != tmp_buf) {
                     g_free(tmp);
                 }
             } else {
-                pixman_blt((uint32_t *)&s->local_mem[src_base],
-                           (uint32_t *)&s->local_mem[dst_base],
-                           src_pitch * bypp / sizeof(uint32_t),
-                           dst_pitch * bypp / sizeof(uint32_t),
-                           8 * bypp, 8 * bypp,
-                           src_x, src_y, dst_x, dst_y, width, height);
+                fallback = !pixman_blt((uint32_t *)&s->local_mem[src_base],
+                                       (uint32_t *)&s->local_mem[dst_base],
+                                       src_pitch * bypp / sizeof(uint32_t),
+                                       dst_pitch * bypp / sizeof(uint32_t),
+                                       8 * bypp, 8 * bypp, src_x, src_y,
+                                       dst_x, dst_y, width, height);
+            }
+            if (fallback) {
+                uint8_t *sp = s->local_mem + src_base;
+                uint8_t *d = s->local_mem + dst_base;
+                unsigned int y, i, j;
+                for (y = 0; y < height; y++) {
+                    if (overlap) { /* overlap also means rtl */
+                        i = (dst_y + height - 1 - y) * dst_pitch;
+                        i = (dst_x + i) * bypp;
+                        j = (src_y + height - 1 - y) * src_pitch;
+                        j = (src_x + j) * bypp;
+                        memmove(&d[i], &sp[j], width * bypp);
+                    } else {
+                        i = (dst_x + (dst_y + y) * dst_pitch) * bypp;
+                        j = (src_x + (src_y + y) * src_pitch) * bypp;
+                        memcpy(&d[i], &sp[j], width * bypp);
+                    }
+                }
             }
         }
         break;
@@ -867,13 +890,19 @@ static void sm501_2d_operation(SM501State *s)
             color = cpu_to_le16(color);
         }
 
-        if (width == 1 && height == 1) {
-            unsigned int i = (dst_x + dst_y * dst_pitch) * bypp;
-            stn_he_p(&s->local_mem[dst_base + i], bypp, color);
-        } else {
-            pixman_fill((uint32_t *)&s->local_mem[dst_base],
-                        dst_pitch * bypp / sizeof(uint32_t),
-                        8 * bypp, dst_x, dst_y, width, height, color);
+        if ((width == 1 && height == 1) ||
+            !pixman_fill((uint32_t *)&s->local_mem[dst_base],
+                         dst_pitch * bypp / sizeof(uint32_t), 8 * bypp,
+                         dst_x, dst_y, width, height, color)) {
+            /* fallback when pixman failed or we don't want to call it */
+            uint8_t *d = s->local_mem + dst_base;
+            unsigned int x, y, i;
+            for (y = 0; y < height; y++, i += dst_pitch * bypp) {
+                i = (dst_x + (dst_y + y) * dst_pitch) * bypp;
+                for (x = 0; x < width; x++, i += bypp) {
+                    stn_he_p(&d[i], bypp, color);
+                }
+            }
         }
         break;
     }
-- 
2.30.7

[PATCH v3 3/8] hw/display/sm501: Add debug property to control pixman usage

[BALATON Zoltan]

Add a property to allow disabling pixman and always use the fallbacks
for different operations which is useful for testing different drawing
methods or debugging pixman related issues.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/sm501.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 23c4418e19..f2f7f26751 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -464,6 +464,7 @@ typedef struct SM501State {
     uint32_t last_width;
     uint32_t last_height;
     bool do_full_update; /* perform a full update next time */
+    uint8_t use_pixman;
     I2CBus *i2c_bus;
 
     /* mmio registers */
@@ -826,7 +827,7 @@ static void sm501_2d_operation(SM501State *s)
                 de = db + (width + (height - 1) * dst_pitch) * bypp;
                 overlap = (db < se && sb < de);
             }
-            if (overlap) {
+            if (overlap && (s->use_pixman & BIT(2))) {
                 /* pixman can't do reverse blit: copy via temporary */
                 int tmp_stride = DIV_ROUND_UP(width * bypp, sizeof(uint32_t));
                 uint32_t *tmp = tmp_buf;
@@ -851,13 +852,15 @@ static void sm501_2d_operation(SM501State *s)
                 if (tmp != tmp_buf) {
                     g_free(tmp);
                 }
-            } else {
+            } else if (!overlap && (s->use_pixman & BIT(1))) {
                 fallback = !pixman_blt((uint32_t *)&s->local_mem[src_base],
                                        (uint32_t *)&s->local_mem[dst_base],
                                        src_pitch * bypp / sizeof(uint32_t),
                                        dst_pitch * bypp / sizeof(uint32_t),
                                        8 * bypp, 8 * bypp, src_x, src_y,
                                        dst_x, dst_y, width, height);
+            } else {
+                fallback = true;
             }
             if (fallback) {
                 uint8_t *sp = s->local_mem + src_base;
@@ -890,7 +893,7 @@ static void sm501_2d_operation(SM501State *s)
             color = cpu_to_le16(color);
         }
 
-        if ((width == 1 && height == 1) ||
+        if (!(s->use_pixman & BIT(0)) || (width == 1 && height == 1) ||
             !pixman_fill((uint32_t *)&s->local_mem[dst_base],
                          dst_pitch * bypp / sizeof(uint32_t), 8 * bypp,
                          dst_x, dst_y, width, height, color)) {
@@ -2039,6 +2042,7 @@ static void sm501_realize_sysbus(DeviceState *dev, Error **errp)
 static Property sm501_sysbus_properties[] = {
     DEFINE_PROP_UINT32("vram-size", SM501SysBusState, vram_size, 0),
     DEFINE_PROP_UINT32("base", SM501SysBusState, base, 0),
+    DEFINE_PROP_UINT8("x-pixman", SM501SysBusState, state.use_pixman, 7),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2122,6 +2126,7 @@ static void sm501_realize_pci(PCIDevice *dev, Error **errp)
 
 static Property sm501_pci_properties[] = {
     DEFINE_PROP_UINT32("vram-size", SM501PCIState, vram_size, 64 * MiB),
+    DEFINE_PROP_UINT8("x-pixman", SM501PCIState, state.use_pixman, 7),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2162,11 +2167,18 @@ static void sm501_pci_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_sm501_pci;
 }
 
+static void sm501_pci_init(Object *o)
+{
+    object_property_set_description(o, "x-pixman", "Use pixman for: "
+                                    "1: fill, 2: blit, 4: overlap blit");
+}
+
 static const TypeInfo sm501_pci_info = {
     .name          = TYPE_PCI_SM501,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(SM501PCIState),
     .class_init    = sm501_pci_class_init,
+    .instance_init = sm501_pci_init,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
         { },
-- 
2.30.7

[PATCH v3 0/8] Pegasos2 fixes and audio output support

[BALATON Zoltan]

Hello,

This is marked v3 to avoid confusion with previously separate patches
that already had v2, even if the series had no v2 yet.

This series now includes all patches needed to get AmigaOS 4.1 run
well on pegasos2 and add audio output to this board. It has 3 parts:
patches 1-3 improve hw/display/sm501 model to avoid graphics problems
that were present with AmigaOS; patches 4-6 fix PCI interrupt routing
in VIA VT8231 model and in pegasos2 board that fixes PCI cards such as
network or sound card not working before; finally patches 7-8 add
basic implementation of the via-ac97 audio part of VT8231 (also used
on VT82C686B) for output that is the default audio device on pegasos2.

This version was re-tested by Rene Engel with latest AmigaOS version
and runs as well as my original series did (posted a video with that
before). This works now on an M1 MacStudio on macOS where it was
unusable before. I've also tested it on Linux x86_64 with older
AmigaOS version that also boots and makes sound and verified MorphOS
still boots and also has sound now.

One known problem with this version that includes Berhard's
alternative vt82c686 patches is with MorphOS which uses level
sensitive mode of the i8259 PIC that QEMU does not support so it hangs
when multiple devices try to raise a shared IRQ. I could work around
that in my otiginal series (see here:
https://lists.nongnu.org/archive/html/qemu-ppc/2023-02/msg00403.html )
where this works and was also tested, that version is available here:
https://osdn.net/projects/qmiga/scm/git/qemu/tree/pegasos2/
but I could not convince Bernhard so I now expect him to provide a
work around for that. This isn't a blocker though as MorphOS already
runs on mac99 and sam460ex and only available as a time limited demo
(they only sell licenses for real hardware) so not really usable apart
from testing anyway so getting it running on pegasos2 would be nice
but not a prioriey, more important is that AmigaOS runs for which this
is the only viable machine as sam460ex version runs much slower. So
I'd like this to be merged for 8.0 as it is now or only minor chnages
(or alternatively we can return to my series which was also tested the
same way and apart from different VIA IRQ router modelling contains
the same patches).

Please review and let me know who will take care of merging this for 8.0.

Regards,
BALATON Zoltan

BALATON Zoltan (6):
  hw/display/sm501: Implement more 2D raster operations
  hw/display/sm501: Add fallbacks to pixman routines
  hw/display/sm501: Add debug property to control pixman usage
  hw/ppc/pegasos2: Fix PCI interrupt routing
  hw/audio/ac97: Split off some definitions to a header
  hw/audio/via-ac97: Basic implementation of audio playback

Bernhard Beschow (2):
  hw/isa/vt82c686: Implement PCI IRQ routing
  hw/usb/vt82c686-uhci-pci: Use PCI IRQ routing

 hw/audio/ac97.c            |  43 +---
 hw/audio/ac97.h            |  65 ++++++
 hw/audio/trace-events      |   6 +
 hw/audio/via-ac97.c        | 455 ++++++++++++++++++++++++++++++++++++-
 hw/display/sm501.c         | 119 ++++++++--
 hw/isa/trace-events        |   1 +
 hw/isa/vt82c686.c          |  41 +++-
 hw/pci-host/mv64361.c      |   4 -
 hw/ppc/pegasos2.c          |  26 ++-
 hw/usb/vt82c686-uhci-pci.c |  12 -
 include/hw/isa/vt82c686.h  |  25 ++
 11 files changed, 706 insertions(+), 91 deletions(-)
 create mode 100644 hw/audio/ac97.h

-- 
2.30.7

Re: [PATCH v3 7/8] hw/audio/ac97: Split off some definitions to a header

[Philippe Mathieu-Daudé]

On 25/1/22 20:48, BALATON Zoltan wrote:
> These can be shared with other AC97 implementations.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/audio/ac97.c | 43 +-------------------------------
>   hw/audio/ac97.h | 65 +++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 66 insertions(+), 42 deletions(-)
>   create mode 100644 hw/audio/ac97.h

Please carry previous tags.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[Philippe Mathieu-Daudé]

On 25/2/23 19:11, BALATON Zoltan wrote:
> From: Bernhard Beschow <shentey@gmail.com>
> 
> The real VIA south bridges implement a PCI IRQ router which is configured
> by the BIOS or the OS. In order to respect these configurations, QEMU
> needs to implement it as well.
> 
> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>   be connected in board code; remove some empty lines]
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Tested-by: Rene Engel <ReneEngel80@emailn.de>
> ---
>   hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)

> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
> +{
> +    switch (irq_num) {
> +    case 0:
> +        return s->dev.config[0x55] >> 4;
> +    case 1:
> +        return s->dev.config[0x56] & 0xf;
> +    case 2:
> +        return s->dev.config[0x56] >> 4;
> +    case 3:
> +        return s->dev.config[0x57] >> 4;
> +    }
> +    return 0;
> +}
> +
> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
> +{
> +    ViaISAState *s = opaque;
> +    PCIBus *bus = pci_get_bus(&s->dev);
> +    int pic_irq;
> +
> +    /* now we change the pic irq level according to the via irq mappings */
> +    /* XXX: optimize */
> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
> +    if (pic_irq < ISA_NUM_IRQS) {

the ISA IRQ is stored in 4-bit so will always be in range.

> +        int i, pic_level;
> +
> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
> +        pic_level = 0;
> +        for (i = 0; i < PCI_NUM_PINS; i++) {
> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
> +                pic_level |= pci_bus_get_irq_level(bus, i);
> +            }
> +        }
> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
> +    }
> +}

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

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

On Sun, 26 Feb 2023, Philippe Mathieu-Daudé wrote:
> On 25/2/23 19:11, BALATON Zoltan wrote:
>> From: Bernhard Beschow <shentey@gmail.com>
>> 
>> The real VIA south bridges implement a PCI IRQ router which is configured
>> by the BIOS or the OS. In order to respect these configurations, QEMU
>> needs to implement it as well.
>> 
>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>   be connected in board code; remove some empty lines]
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>> ---
>>   hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>
>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>> +{
>> +    switch (irq_num) {
>> +    case 0:
>> +        return s->dev.config[0x55] >> 4;
>> +    case 1:
>> +        return s->dev.config[0x56] & 0xf;
>> +    case 2:
>> +        return s->dev.config[0x56] >> 4;
>> +    case 3:
>> +        return s->dev.config[0x57] >> 4;
>> +    }
>> +    return 0;
>> +}
>> +
>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>> +{
>> +    ViaISAState *s = opaque;
>> +    PCIBus *bus = pci_get_bus(&s->dev);
>> +    int pic_irq;
>> +
>> +    /* now we change the pic irq level according to the via irq mappings 
>> */
>> +    /* XXX: optimize */
>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>> +    if (pic_irq < ISA_NUM_IRQS) {
>
> the ISA IRQ is stored in 4-bit so will always be in range.

Complain at hw/isa/piix4 I guess or clean this up together later :-) Or 
maybe Bernhard has an idea or patch for this coming up that's why he 
pushed this in here. In any case, since this is now the same as piix4 
either both or nothing for this and both would be out of scope for this 
series.

Regards,
BALATON Zoltan

>> +        int i, pic_level;
>> +
>> +        /* The pic level is the logical OR of all the PCI irqs mapped to 
>> it. */
>> +        pic_level = 0;
>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>> +            }
>> +        }
>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>> +    }
>> +}
>
>
>
>

Re: [PATCH v3 5/8] hw/ppc/pegasos2: Fix PCI interrupt routing

[Bernhard Beschow]

Am 16. Februar 2023 20:27:32 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>According to the PegasosII schematics the PCI interrupt lines are
>connected to both the gpp pins of the Mv64361 north bridge and the
>PINT pins of the VT8231 south bridge so guests can get interrupts from
>either of these. So far we only had the MV64361 connections which
>worked for on board devices but for additional PCI devices (such as
>network or sound card added with -device) guest OSes expect interrupt
>from the ISA IRQ 9 where the firmware routes these PCI interrupts in
>VT8231 ISA bridge. After the previous patches we can now model this
>and also remove the board specific connection from mv64361. Also
>configure routing of these lines when using Virtual Open Firmware to
>match board firmware for guests that expect this.
>
>This fixes PCI interrupts on pegasos2 under Linux, MorphOS and AmigaOS.
>
>Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>Tested-by: Rene Engel <ReneEngel80@emailn.de>
>Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>---
> hw/pci-host/mv64361.c |  4 ----
> hw/ppc/pegasos2.c     | 26 +++++++++++++++++++++++++-
> 2 files changed, 25 insertions(+), 5 deletions(-)
>
>diff --git a/hw/pci-host/mv64361.c b/hw/pci-host/mv64361.c
>index f43f33fbd9..3d9132f989 100644
>--- a/hw/pci-host/mv64361.c
>+++ b/hw/pci-host/mv64361.c
>@@ -874,10 +874,6 @@ static void mv64361_realize(DeviceState *dev, Error **errp)
>     }
>     sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->cpu_irq);
>     qdev_init_gpio_in_named(dev, mv64361_gpp_irq, "gpp", 32);
>-    /* FIXME: PCI IRQ connections may be board specific */
>-    for (i = 0; i < PCI_NUM_PINS; i++) {
>-        s->pci[1].irq[i] = qdev_get_gpio_in_named(dev, "gpp", 12 + i);
>-    }
> }
> 
> static void mv64361_reset(DeviceState *dev)
>diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>index a9563f4fb2..4e1476673b 100644
>--- a/hw/ppc/pegasos2.c
>+++ b/hw/ppc/pegasos2.c
>@@ -74,6 +74,8 @@ struct Pegasos2MachineState {
>     MachineState parent_obj;
>     PowerPCCPU *cpu;
>     DeviceState *mv;
>+    qemu_irq mv_pirq[PCI_NUM_PINS];
>+    qemu_irq via_pirq[PCI_NUM_PINS];
>     Vof *vof;
>     void *fdt_blob;
>     uint64_t kernel_addr;
>@@ -96,6 +98,15 @@ static void pegasos2_cpu_reset(void *opaque)
>     }
> }
> 
>+static void pegasos2_pci_irq(void *opaque, int n, int level)
>+{
>+    Pegasos2MachineState *pm = opaque;
>+
>+    /* PCI interrupt lines are connected to both MV64361 and VT8231 */
>+    qemu_set_irq(pm->mv_pirq[n], level);
>+    qemu_set_irq(pm->via_pirq[n], level);
>+}
>+
> static void pegasos2_init(MachineState *machine)
> {
>     Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine);
>@@ -107,7 +118,7 @@ static void pegasos2_init(MachineState *machine)
>     I2CBus *i2c_bus;
>     const char *fwname = machine->firmware ?: PROM_FILENAME;
>     char *filename;
>-    int sz;
>+    int i, sz;
>     uint8_t *spd_data;
> 
>     /* init CPU */
>@@ -157,11 +168,18 @@ static void pegasos2_init(MachineState *machine)
>     /* Marvell Discovery II system controller */
>     pm->mv = DEVICE(sysbus_create_simple(TYPE_MV64361, -1,
>                           qdev_get_gpio_in(DEVICE(pm->cpu), PPC6xx_INPUT_INT)));
>+    for (i = 0; i < PCI_NUM_PINS; i++) {
>+        pm->mv_pirq[i] = qdev_get_gpio_in_named(pm->mv, "gpp", 12 + i);
>+    }
>     pci_bus = mv64361_get_pci_bus(pm->mv, 1);
>+    pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
> 
>     /* VIA VT8231 South Bridge (multifunction PCI device) */
>     via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
>                                                  true, TYPE_VT8231_ISA));
>+    for (i = 0; i < PCI_NUM_PINS; i++) {
>+        pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
>+    }
>     object_property_add_alias(OBJECT(machine), "rtc-time",
>                               object_resolve_path_component(via, "rtc"),
>                               "date");
>@@ -268,6 +286,12 @@ static void pegasos2_machine_reset(MachineState *machine, ShutdownCause reason)
>                               PCI_INTERRUPT_LINE, 2, 0x9);
>     pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
>                               0x50, 1, 0x2);
>+    pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
>+                              0x55, 1, 0x90);
>+    pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
>+                              0x56, 1, 0x99);
>+    pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
>+                              0x57, 1, 0x90);

Let's not mix aspects (implement VT82XX PCI IRQ routing) with Pegasos2 aspects. IOW let's keep these additions in a separate patch to make things clearer.

> 
>     pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 1) << 8) |
>                               PCI_INTERRUPT_LINE, 2, 0x109);

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[Bernhard Beschow]

Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>From: Bernhard Beschow <shentey@gmail.com>
>
>The real VIA south bridges implement a PCI IRQ router which is configured
>by the BIOS or the OS. In order to respect these configurations, QEMU
>needs to implement it as well.
>
>Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>
>Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>[balaton: declare gpio inputs instead of changing pci bus irqs so it can
> be connected in board code; remove some empty lines]
>Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>Tested-by: Rene Engel <ReneEngel80@emailn.de>
>---
> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
>diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>index 3f9bd0c04d..4025f9bcdc 100644
>--- a/hw/isa/vt82c686.c
>+++ b/hw/isa/vt82c686.c
>@@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
>     qemu_set_irq(s->cpu_intr, level);
> }
> 
>+static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>+{
>+    switch (irq_num) {
>+    case 0:
>+        return s->dev.config[0x55] >> 4;
>+    case 1:
>+        return s->dev.config[0x56] & 0xf;
>+    case 2:
>+        return s->dev.config[0x56] >> 4;
>+    case 3:
>+        return s->dev.config[0x57] >> 4;
>+    }
>+    return 0;
>+}
>+
>+static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>+{
>+    ViaISAState *s = opaque;
>+    PCIBus *bus = pci_get_bus(&s->dev);
>+    int pic_irq;
>+
>+    /* now we change the pic irq level according to the via irq mappings */
>+    /* XXX: optimize */
>+    pic_irq = via_isa_get_pci_irq(s, irq_num);
>+    if (pic_irq < ISA_NUM_IRQS) {
>+        int i, pic_level;
>+
>+        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>+        pic_level = 0;
>+        for (i = 0; i < PCI_NUM_PINS; i++) {
>+            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>+                pic_level |= pci_bus_get_irq_level(bus, i);
>+            }
>+        }
>+        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>+    }
>+}
>+
> static void via_isa_realize(PCIDevice *d, Error **errp)
> {
>     ViaISAState *s = VIA_ISA(d);
>@@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>     int i;
> 
>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>+    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", PCI_NUM_PINS);

This line is a Pegasos2 specific addition for fixing its IRQ handling. Since this code must also work with the Fuloong2e board we should aim for a minimal changeset here which renders this line out of scope.

Let's keep the two series separate since now I need to watch two series for comments. Please use Based-on: tag next time instead.

Thanks,
Bernhard

>     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
>                           errp);

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[Bernhard Beschow]

Am 26. Februar 2023 22:27:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 25/2/23 19:11, BALATON Zoltan wrote:
>> From: Bernhard Beschow <shentey@gmail.com>
>> 
>> The real VIA south bridges implement a PCI IRQ router which is configured
>> by the BIOS or the OS. In order to respect these configurations, QEMU
>> needs to implement it as well.
>> 
>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>   be connected in board code; remove some empty lines]
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>> ---
>>   hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>
>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>> +{
>> +    switch (irq_num) {
>> +    case 0:
>> +        return s->dev.config[0x55] >> 4;
>> +    case 1:
>> +        return s->dev.config[0x56] & 0xf;
>> +    case 2:
>> +        return s->dev.config[0x56] >> 4;
>> +    case 3:
>> +        return s->dev.config[0x57] >> 4;
>> +    }
>> +    return 0;
>> +}
>> +
>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>> +{
>> +    ViaISAState *s = opaque;
>> +    PCIBus *bus = pci_get_bus(&s->dev);
>> +    int pic_irq;
>> +
>> +    /* now we change the pic irq level according to the via irq mappings */
>> +    /* XXX: optimize */
>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>> +    if (pic_irq < ISA_NUM_IRQS) {
>
>the ISA IRQ is stored in 4-bit so will always be in range.

Indeed. I'd turn this into an assert to keep this assum visible. I'll do another iteration of the PCI IRQ router series.

Best regards,
Bernhard
>
>> +        int i, pic_level;
>> +
>> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>> +        pic_level = 0;
>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>> +            }
>> +        }
>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>> +    }
>> +}
>
>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

On Sun, 26 Feb 2023, Bernhard Beschow wrote:
> Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> From: Bernhard Beschow <shentey@gmail.com>
>>
>> The real VIA south bridges implement a PCI IRQ router which is configured
>> by the BIOS or the OS. In order to respect these configurations, QEMU
>> needs to implement it as well.
>>
>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>> be connected in board code; remove some empty lines]
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>> ---
>> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index 3f9bd0c04d..4025f9bcdc 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
>>     qemu_set_irq(s->cpu_intr, level);
>> }
>>
>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>> +{
>> +    switch (irq_num) {
>> +    case 0:
>> +        return s->dev.config[0x55] >> 4;
>> +    case 1:
>> +        return s->dev.config[0x56] & 0xf;
>> +    case 2:
>> +        return s->dev.config[0x56] >> 4;
>> +    case 3:
>> +        return s->dev.config[0x57] >> 4;
>> +    }
>> +    return 0;
>> +}
>> +
>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>> +{
>> +    ViaISAState *s = opaque;
>> +    PCIBus *bus = pci_get_bus(&s->dev);
>> +    int pic_irq;
>> +
>> +    /* now we change the pic irq level according to the via irq mappings */
>> +    /* XXX: optimize */
>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>> +    if (pic_irq < ISA_NUM_IRQS) {
>> +        int i, pic_level;
>> +
>> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>> +        pic_level = 0;
>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>> +            }
>> +        }
>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>> +    }
>> +}
>> +
>> static void via_isa_realize(PCIDevice *d, Error **errp)
>> {
>>     ViaISAState *s = VIA_ISA(d);
>> @@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>     int i;
>>
>>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>> +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", PCI_NUM_PINS);
>
> This line is a Pegasos2 specific addition for fixing its IRQ handling. Since this code must also work with the Fuloong2e board we should aim for a minimal changeset here which renders this line out of scope.
>
> Let's keep the two series separate since now I need to watch two series for comments. Please use Based-on: tag next time instead.

Well, it's not. It's part of the QDev model for VT8231 that allows it to 
be connected by boards so I think this belongs here otherwise this won't 
even compile because the function you've added would be unused and bail on 
-Werror. Let's not make this more difficult than it is. I'm OK with 
reasonable changes but what's your goal now? You can't get rid of this 
line as it's how QDev can model it. Either I have to call into this model 
or have to export these pins as gpios.

Regards,
BALATON Zoltan

> Thanks,
> Bernhard
>
>>     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>>     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
>>                           errp);
>
>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

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

On Sun, 26 Feb 2023, Bernhard Beschow wrote:
> Am 26. Februar 2023 22:27:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> On 25/2/23 19:11, BALATON Zoltan wrote:
>>> From: Bernhard Beschow <shentey@gmail.com>
>>>
>>> The real VIA south bridges implement a PCI IRQ router which is configured
>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>> needs to implement it as well.
>>>
>>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>>   be connected in board code; remove some empty lines]
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>> ---
>>>   hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 39 insertions(+)
>>
>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>> +{
>>> +    switch (irq_num) {
>>> +    case 0:
>>> +        return s->dev.config[0x55] >> 4;
>>> +    case 1:
>>> +        return s->dev.config[0x56] & 0xf;
>>> +    case 2:
>>> +        return s->dev.config[0x56] >> 4;
>>> +    case 3:
>>> +        return s->dev.config[0x57] >> 4;
>>> +    }
>>> +    return 0;
>>> +}
>>> +
>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>> +{
>>> +    ViaISAState *s = opaque;
>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>> +    int pic_irq;
>>> +
>>> +    /* now we change the pic irq level according to the via irq mappings */
>>> +    /* XXX: optimize */
>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>
>> the ISA IRQ is stored in 4-bit so will always be in range.
>
> Indeed. I'd turn this into an assert to keep this assum visible. I'll do another iteration of the PCI IRQ router series.

If you do that please don't break it and make me redo this series again. 
Sumbit a patch as a reply to this that can be substituted without changing 
other patches and I can include in later respins. We don't have time to 
make extensive changes now, the freeze is too close.

Regards,
BALATON Zoltan

> Best regards,
> Bernhard
>>
>>> +        int i, pic_level;
>>> +
>>> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>>> +        pic_level = 0;
>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>> +            }
>>> +        }
>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>> +    }
>>> +}
>>
>>
>
>

Re: [PATCH v3 5/8] hw/ppc/pegasos2: Fix PCI interrupt routing

[BALATON Zoltan]

On Sun, 26 Feb 2023, Bernhard Beschow wrote:
> Am 16. Februar 2023 20:27:32 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> According to the PegasosII schematics the PCI interrupt lines are
>> connected to both the gpp pins of the Mv64361 north bridge and the
>> PINT pins of the VT8231 south bridge so guests can get interrupts from
>> either of these. So far we only had the MV64361 connections which
>> worked for on board devices but for additional PCI devices (such as
>> network or sound card added with -device) guest OSes expect interrupt
>> from the ISA IRQ 9 where the firmware routes these PCI interrupts in
>> VT8231 ISA bridge. After the previous patches we can now model this
>> and also remove the board specific connection from mv64361. Also
>> configure routing of these lines when using Virtual Open Firmware to
>> match board firmware for guests that expect this.
>>
>> This fixes PCI interrupts on pegasos2 under Linux, MorphOS and AmigaOS.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>> hw/pci-host/mv64361.c |  4 ----
>> hw/ppc/pegasos2.c     | 26 +++++++++++++++++++++++++-
>> 2 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/pci-host/mv64361.c b/hw/pci-host/mv64361.c
>> index f43f33fbd9..3d9132f989 100644
>> --- a/hw/pci-host/mv64361.c
>> +++ b/hw/pci-host/mv64361.c
>> @@ -874,10 +874,6 @@ static void mv64361_realize(DeviceState *dev, Error **errp)
>>     }
>>     sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->cpu_irq);
>>     qdev_init_gpio_in_named(dev, mv64361_gpp_irq, "gpp", 32);
>> -    /* FIXME: PCI IRQ connections may be board specific */
>> -    for (i = 0; i < PCI_NUM_PINS; i++) {
>> -        s->pci[1].irq[i] = qdev_get_gpio_in_named(dev, "gpp", 12 + i);
>> -    }
>> }
>>
>> static void mv64361_reset(DeviceState *dev)
>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>> index a9563f4fb2..4e1476673b 100644
>> --- a/hw/ppc/pegasos2.c
>> +++ b/hw/ppc/pegasos2.c
>> @@ -74,6 +74,8 @@ struct Pegasos2MachineState {
>>     MachineState parent_obj;
>>     PowerPCCPU *cpu;
>>     DeviceState *mv;
>> +    qemu_irq mv_pirq[PCI_NUM_PINS];
>> +    qemu_irq via_pirq[PCI_NUM_PINS];
>>     Vof *vof;
>>     void *fdt_blob;
>>     uint64_t kernel_addr;
>> @@ -96,6 +98,15 @@ static void pegasos2_cpu_reset(void *opaque)
>>     }
>> }
>>
>> +static void pegasos2_pci_irq(void *opaque, int n, int level)
>> +{
>> +    Pegasos2MachineState *pm = opaque;
>> +
>> +    /* PCI interrupt lines are connected to both MV64361 and VT8231 */
>> +    qemu_set_irq(pm->mv_pirq[n], level);
>> +    qemu_set_irq(pm->via_pirq[n], level);
>> +}
>> +
>> static void pegasos2_init(MachineState *machine)
>> {
>>     Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine);
>> @@ -107,7 +118,7 @@ static void pegasos2_init(MachineState *machine)
>>     I2CBus *i2c_bus;
>>     const char *fwname = machine->firmware ?: PROM_FILENAME;
>>     char *filename;
>> -    int sz;
>> +    int i, sz;
>>     uint8_t *spd_data;
>>
>>     /* init CPU */
>> @@ -157,11 +168,18 @@ static void pegasos2_init(MachineState *machine)
>>     /* Marvell Discovery II system controller */
>>     pm->mv = DEVICE(sysbus_create_simple(TYPE_MV64361, -1,
>>                           qdev_get_gpio_in(DEVICE(pm->cpu), PPC6xx_INPUT_INT)));
>> +    for (i = 0; i < PCI_NUM_PINS; i++) {
>> +        pm->mv_pirq[i] = qdev_get_gpio_in_named(pm->mv, "gpp", 12 + i);
>> +    }
>>     pci_bus = mv64361_get_pci_bus(pm->mv, 1);
>> +    pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
>>
>>     /* VIA VT8231 South Bridge (multifunction PCI device) */
>>     via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
>>                                                  true, TYPE_VT8231_ISA));
>> +    for (i = 0; i < PCI_NUM_PINS; i++) {
>> +        pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
>> +    }
>>     object_property_add_alias(OBJECT(machine), "rtc-time",
>>                               object_resolve_path_component(via, "rtc"),
>>                               "date");
>> @@ -268,6 +286,12 @@ static void pegasos2_machine_reset(MachineState *machine, ShutdownCause reason)
>>                               PCI_INTERRUPT_LINE, 2, 0x9);
>>     pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
>>                               0x50, 1, 0x2);
>> +    pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
>> +                              0x55, 1, 0x90);
>> +    pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
>> +                              0x56, 1, 0x99);
>> +    pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
>> +                              0x57, 1, 0x90);
>
> Let's not mix aspects (implement VT82XX PCI IRQ routing) with Pegasos2 aspects. IOW let's keep these additions in a separate patch to make things clearer.

I guess this one hunk could be split out but not sure what would that 
bring.

Regards,
BALATON Zoltan

>>
>>     pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 1) << 8) |
>>                               PCI_INTERRUPT_LINE, 2, 0x109);
>
>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

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

On Sun, 26 Feb 2023, Bernhard Beschow wrote:
> Am 26. Februar 2023 22:27:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> On 25/2/23 19:11, BALATON Zoltan wrote:
>>> From: Bernhard Beschow <shentey@gmail.com>
>>>
>>> The real VIA south bridges implement a PCI IRQ router which is configured
>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>> needs to implement it as well.
>>>
>>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>>   be connected in board code; remove some empty lines]
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>> ---
>>>   hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 39 insertions(+)
>>
>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>> +{
>>> +    switch (irq_num) {
>>> +    case 0:
>>> +        return s->dev.config[0x55] >> 4;
>>> +    case 1:
>>> +        return s->dev.config[0x56] & 0xf;
>>> +    case 2:
>>> +        return s->dev.config[0x56] >> 4;
>>> +    case 3:
>>> +        return s->dev.config[0x57] >> 4;
>>> +    }
>>> +    return 0;
>>> +}
>>> +
>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>> +{
>>> +    ViaISAState *s = opaque;
>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>> +    int pic_irq;
>>> +
>>> +    /* now we change the pic irq level according to the via irq mappings */
>>> +    /* XXX: optimize */
>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>
>> the ISA IRQ is stored in 4-bit so will always be in range.
>
> Indeed. I'd turn this into an assert to keep this assum visible. I'll do another iteration of the PCI IRQ router series.

I don't like a useless assert in an irq handler that's potentially called 
a lot. If you want to keep that add a comment instead.

Also I can't use Based-on because having all patches in a single series 
helps maintainers to follow what belongs to here so this should be one 
series. You don't have to follow your one any more as it was fully 
incorporated here so this is the only version you'd have to watch.

Regards,
BALATON Zoltan

> Best regards,
> Bernhard
>>
>>> +        int i, pic_level;
>>> +
>>> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>>> +        pic_level = 0;
>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>> +            }
>>> +        }
>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>> +    }
>>> +}
>>
>>
>
>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[Bernhard Beschow]

Am 26. Februar 2023 23:33:20 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>> Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>>> From: Bernhard Beschow <shentey@gmail.com>
>>> 
>>> The real VIA south bridges implement a PCI IRQ router which is configured
>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>> needs to implement it as well.
>>> 
>>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>> 
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>> be connected in board code; remove some empty lines]
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>> ---
>>> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 39 insertions(+)
>>> 
>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>> index 3f9bd0c04d..4025f9bcdc 100644
>>> --- a/hw/isa/vt82c686.c
>>> +++ b/hw/isa/vt82c686.c
>>> @@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
>>>     qemu_set_irq(s->cpu_intr, level);
>>> }
>>> 
>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>> +{
>>> +    switch (irq_num) {
>>> +    case 0:
>>> +        return s->dev.config[0x55] >> 4;
>>> +    case 1:
>>> +        return s->dev.config[0x56] & 0xf;
>>> +    case 2:
>>> +        return s->dev.config[0x56] >> 4;
>>> +    case 3:
>>> +        return s->dev.config[0x57] >> 4;
>>> +    }
>>> +    return 0;
>>> +}
>>> +
>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>> +{
>>> +    ViaISAState *s = opaque;
>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>> +    int pic_irq;
>>> +
>>> +    /* now we change the pic irq level according to the via irq mappings */
>>> +    /* XXX: optimize */
>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>> +        int i, pic_level;
>>> +
>>> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>>> +        pic_level = 0;
>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>> +            }
>>> +        }
>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>> +    }
>>> +}
>>> +
>>> static void via_isa_realize(PCIDevice *d, Error **errp)
>>> {
>>>     ViaISAState *s = VIA_ISA(d);
>>> @@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>>     int i;
>>> 
>>>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>>> +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", PCI_NUM_PINS);
>> 
>> This line is a Pegasos2 specific addition for fixing its IRQ handling. Since this code must also work with the Fuloong2e board we should aim for a minimal changeset here which renders this line out of scope.
>> 
>> Let's keep the two series separate since now I need to watch two series for comments. Please use Based-on: tag next time instead.
>
>Well, it's not. It's part of the QDev model for VT8231 that allows it to be connected by boards so I think this belongs here otherwise this won't even compile because the function you've added would be unused and bail on -Werror. Let's not make this more difficult than it is. I'm OK with reasonable changes but what's your goal now? You can't get rid of this line as it's how QDev can model it. Either I have to call into this model or have to export these pins as gpios.

Exporting the pins is a separate aspect on top of implementing PCI IRQ routing. To make this clear and obvious this should be a dedicated patch. In case either patch has an issue this would also ease and therefore acellerate discussions.

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>> Thanks,
>> Bernhard
>> 
>>>     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
>>>     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
>>>                           errp);
>> 
>> 

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[Bernhard Beschow]

Am 26. Februar 2023 23:37:24 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>> Am 26. Februar 2023 22:27:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> On 25/2/23 19:11, BALATON Zoltan wrote:
>>>> From: Bernhard Beschow <shentey@gmail.com>
>>>> 
>>>> The real VIA south bridges implement a PCI IRQ router which is configured
>>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>>> needs to implement it as well.
>>>> 
>>>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>>> 
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>>>   be connected in board code; remove some empty lines]
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>>> ---
>>>>   hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>   1 file changed, 39 insertions(+)
>>> 
>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>>> +{
>>>> +    switch (irq_num) {
>>>> +    case 0:
>>>> +        return s->dev.config[0x55] >> 4;
>>>> +    case 1:
>>>> +        return s->dev.config[0x56] & 0xf;
>>>> +    case 2:
>>>> +        return s->dev.config[0x56] >> 4;
>>>> +    case 3:
>>>> +        return s->dev.config[0x57] >> 4;
>>>> +    }
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>>> +{
>>>> +    ViaISAState *s = opaque;
>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>>> +    int pic_irq;
>>>> +
>>>> +    /* now we change the pic irq level according to the via irq mappings */
>>>> +    /* XXX: optimize */
>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>> 
>>> the ISA IRQ is stored in 4-bit so will always be in range.
>> 
>> Indeed. I'd turn this into an assert to keep this assum visible. I'll do another iteration of the PCI IRQ router series.
>
>If you do that please don't break it and make me redo this series again. Sumbit a patch as a reply to this that can be substituted without changing other patches and I can include in later respins.

I will only do the mist minimal changes satisfying the review comments. This should minimize the risk of breakage. Also, you can minimize the chance of breakage on your side by not introducing more changes than needed, e.g. by not doing any formatting changes.

>We don't have time to make extensive changes now, the freeze is too close.

I don't intend to make extensive changes unless review comments requre it.

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>> Best regards,
>> Bernhard
>>> 
>>>> +        int i, pic_level;
>>>> +
>>>> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>>>> +        pic_level = 0;
>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>>> +            }
>>>> +        }
>>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>>> +    }
>>>> +}
>>> 
>>> 
>> 
>>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[Bernhard Beschow]

Am 26. Februar 2023 23:58:32 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>> Am 26. Februar 2023 22:27:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>> On 25/2/23 19:11, BALATON Zoltan wrote:
>>>> From: Bernhard Beschow <shentey@gmail.com>
>>>> 
>>>> The real VIA south bridges implement a PCI IRQ router which is configured
>>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>>> needs to implement it as well.
>>>> 
>>>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>>> 
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>>>   be connected in board code; remove some empty lines]
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>>> ---
>>>>   hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>   1 file changed, 39 insertions(+)
>>> 
>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>>> +{
>>>> +    switch (irq_num) {
>>>> +    case 0:
>>>> +        return s->dev.config[0x55] >> 4;
>>>> +    case 1:
>>>> +        return s->dev.config[0x56] & 0xf;
>>>> +    case 2:
>>>> +        return s->dev.config[0x56] >> 4;
>>>> +    case 3:
>>>> +        return s->dev.config[0x57] >> 4;
>>>> +    }
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>>> +{
>>>> +    ViaISAState *s = opaque;
>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>>> +    int pic_irq;
>>>> +
>>>> +    /* now we change the pic irq level according to the via irq mappings */
>>>> +    /* XXX: optimize */
>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>> 
>>> the ISA IRQ is stored in 4-bit so will always be in range.
>> 
>> Indeed. I'd turn this into an assert to keep this assum visible. I'll do another iteration of the PCI IRQ router series.
>
>I don't like a useless assert in an irq handler that's potentially called a lot. If you want to keep that add a comment instead.

It won't because it will only affect debug builds. If it fails it's immediately clear in which direction to look for the culprit.

>
>Also I can't use Based-on because having all patches in a single series helps maintainers to follow what belongs to here so this should be one series. You don't have to follow your one any more as it was fully incorporated here so this is the only version you'd have to watch.
>
>Regards,
>BALATON Zoltan
>
>> Best regards,
>> Bernhard
>>> 
>>>> +        int i, pic_level;
>>>> +
>>>> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>>>> +        pic_level = 0;
>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>>> +            }
>>>> +        }
>>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>>> +    }
>>>> +}
>>> 
>>> 
>> 
>>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> Am 26. Februar 2023 23:33:20 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>>> Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>>>> From: Bernhard Beschow <shentey@gmail.com>
>>>>
>>>> The real VIA south bridges implement a PCI IRQ router which is configured
>>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>>> needs to implement it as well.
>>>>
>>>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>>>
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>>> be connected in board code; remove some empty lines]
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>>> ---
>>>> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 39 insertions(+)
>>>>
>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>> index 3f9bd0c04d..4025f9bcdc 100644
>>>> --- a/hw/isa/vt82c686.c
>>>> +++ b/hw/isa/vt82c686.c
>>>> @@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
>>>>     qemu_set_irq(s->cpu_intr, level);
>>>> }
>>>>
>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>>> +{
>>>> +    switch (irq_num) {
>>>> +    case 0:
>>>> +        return s->dev.config[0x55] >> 4;
>>>> +    case 1:
>>>> +        return s->dev.config[0x56] & 0xf;
>>>> +    case 2:
>>>> +        return s->dev.config[0x56] >> 4;
>>>> +    case 3:
>>>> +        return s->dev.config[0x57] >> 4;
>>>> +    }
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>>> +{
>>>> +    ViaISAState *s = opaque;
>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>>> +    int pic_irq;
>>>> +
>>>> +    /* now we change the pic irq level according to the via irq mappings */
>>>> +    /* XXX: optimize */
>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>>> +        int i, pic_level;
>>>> +
>>>> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>>>> +        pic_level = 0;
>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>>> +            }
>>>> +        }
>>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>>> +    }
>>>> +}
>>>> +
>>>> static void via_isa_realize(PCIDevice *d, Error **errp)
>>>> {
>>>>     ViaISAState *s = VIA_ISA(d);
>>>> @@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>>>     int i;
>>>>
>>>>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>>>> +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", PCI_NUM_PINS);
>>>
>>> This line is a Pegasos2 specific addition for fixing its IRQ handling. Since this code must also work with the Fuloong2e board we should aim for a minimal changeset here which renders this line out of scope.
>>>
>>> Let's keep the two series separate since now I need to watch two series for comments. Please use Based-on: tag next time instead.
>>
>> Well, it's not. It's part of the QDev model for VT8231 that allows it to be connected by boards so I think this belongs here otherwise this won't even compile because the function you've added would be unused and bail on -Werror. Let's not make this more difficult than it is. I'm OK with reasonable changes but what's your goal now? You can't get rid of this line as it's how QDev can model it. Either I have to call into this model or have to export these pins as gpios.
>
> Exporting the pins is a separate aspect on top of implementing PCI IRQ 
> routing. To make this clear and obvious this should be a dedicated 
> patch. In case either patch has an issue this would also ease and 
> therefore acellerate discussions.

How would you do that? Introduce via_isa_set_pci_irq() as unused in this 
patch then have a one line patch to add connecting it to gpio pins? That 
just makes no sense. This is not modeling the chip with QDev and then the 
boatd can connect it as appropriate modeling the board that the chip is 
in. So if fuloon2e needs to do that then it should. I'll check that as I 
was focusing on pegasos2 for now. You can't use pci_bur_irqs here because 
that breaks pegasos2 so no point in doing that way.

Regards,
BALATON Zoltan

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

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

On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> Am 26. Februar 2023 23:37:24 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>>> Am 26. Februar 2023 22:27:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> On 25/2/23 19:11, BALATON Zoltan wrote:
>>>>> From: Bernhard Beschow <shentey@gmail.com>
>>>>>
>>>>> The real VIA south bridges implement a PCI IRQ router which is configured
>>>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>>>> needs to implement it as well.
>>>>>
>>>>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>>>>
>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>>>>   be connected in board code; remove some empty lines]
>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>>>> ---
>>>>>   hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>>   1 file changed, 39 insertions(+)
>>>>
>>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>>>> +{
>>>>> +    switch (irq_num) {
>>>>> +    case 0:
>>>>> +        return s->dev.config[0x55] >> 4;
>>>>> +    case 1:
>>>>> +        return s->dev.config[0x56] & 0xf;
>>>>> +    case 2:
>>>>> +        return s->dev.config[0x56] >> 4;
>>>>> +    case 3:
>>>>> +        return s->dev.config[0x57] >> 4;
>>>>> +    }
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>>>> +{
>>>>> +    ViaISAState *s = opaque;
>>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>>>> +    int pic_irq;
>>>>> +
>>>>> +    /* now we change the pic irq level according to the via irq mappings */
>>>>> +    /* XXX: optimize */
>>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>>>
>>>> the ISA IRQ is stored in 4-bit so will always be in range.
>>>
>>> Indeed. I'd turn this into an assert to keep this assum visible. I'll do another iteration of the PCI IRQ router series.
>>
>> If you do that please don't break it and make me redo this series again. Sumbit a patch as a reply to this that can be substituted without changing other patches and I can include in later respins.
>
> I will only do the mist minimal changes satisfying the review comments. This should minimize the risk of breakage.

Hopefully this gets reviewed soon, we only have this week to finalise it 
so I hope we get comments early and not at the weekend or next week. I can 
make changes but I need to know in time for that.

> Also, you can minimize the chance of breakage on your side by not 
> introducing more changes than needed, e.g. by not doing any formatting 
> changes.

Deleting empty lines is hardly a breaking change. Also your formatting 
looked like it's missing break statements and did not conveniently fit in 
my screen so I took the opportunity to make it clearer at least for me. 
Personal preferences may differ but this should not be a big deal.

Regards,
BALATON Zoltan

>> We don't have time to make extensive changes now, the freeze is too close.
>
> I don't intend to make extensive changes unless review comments requre it.
>
> Best regards,
> Bernhard
>
>>
>> Regards,
>> BALATON Zoltan
>>
>>> Best regards,
>>> Bernhard
>>>>
>>>>> +        int i, pic_level;
>>>>> +
>>>>> +        /* The pic level is the logical OR of all the PCI irqs mapped to it. */
>>>>> +        pic_level = 0;
>>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>>>> +            }
>>>>> +        }
>>>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>>>> +    }
>>>>> +}
>>>>
>>>>
>>>
>>>
>
>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

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

On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> Am 26. Februar 2023 23:58:32 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>>> Am 26. Februar 2023 22:27:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> On 25/2/23 19:11, BALATON Zoltan wrote:
>>>>> From: Bernhard Beschow <shentey@gmail.com>
>>>>>
>>>>> The real VIA south bridges implement a PCI IRQ router which is configured
>>>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>>>> needs to implement it as well.
>>>>>
>>>>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>>>>
>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>>>>   be connected in board code; remove some empty lines]
>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>>>> ---
>>>>>   hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>>   1 file changed, 39 insertions(+)
>>>>
>>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>>>> +{
>>>>> +    switch (irq_num) {
>>>>> +    case 0:
>>>>> +        return s->dev.config[0x55] >> 4;
>>>>> +    case 1:
>>>>> +        return s->dev.config[0x56] & 0xf;
>>>>> +    case 2:
>>>>> +        return s->dev.config[0x56] >> 4;
>>>>> +    case 3:
>>>>> +        return s->dev.config[0x57] >> 4;
>>>>> +    }
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>>>> +{
>>>>> +    ViaISAState *s = opaque;
>>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>>>> +    int pic_irq;
>>>>> +
>>>>> +    /* now we change the pic irq level according to the via irq mappings */
>>>>> +    /* XXX: optimize */
>>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>>>
>>>> the ISA IRQ is stored in 4-bit so will always be in range.
>>>
>>> Indeed. I'd turn this into an assert to keep this assum visible. I'll do another iteration of the PCI IRQ router series.
>>
>> I don't like a useless assert in an irq handler that's potentially called a lot. If you want to keep that add a comment instead.
>
> It won't because it will only affect debug builds. If it fails it's 
> immediately clear in which direction to look for the culprit.

You may think so but it's not. QEMU cannot be built with NDEBUG (there was 
even some discussion about this recently on the list) so all the asserts 
are there in release builds. Let's at least keep them out from frequently 
called parts especially if they can never fire.

Regards,
BALATON Zoltan

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

On Mon, 27 Feb 2023, BALATON Zoltan wrote:
> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>> Am 26. Februar 2023 23:33:20 UTC schrieb BALATON Zoltan 
>> <balaton@eik.bme.hu>:
>>> On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>>>> Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan 
>>>> <balaton@eik.bme.hu>:
>>>>> From: Bernhard Beschow <shentey@gmail.com>
>>>>> 
>>>>> The real VIA south bridges implement a PCI IRQ router which is 
>>>>> configured
>>>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>>>> needs to implement it as well.
>>>>> 
>>>>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>>>>> 
>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it can
>>>>> be connected in board code; remove some empty lines]
>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>>>> ---
>>>>> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 39 insertions(+)
>>>>> 
>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>>> index 3f9bd0c04d..4025f9bcdc 100644
>>>>> --- a/hw/isa/vt82c686.c
>>>>> +++ b/hw/isa/vt82c686.c
>>>>> @@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void *opaque, 
>>>>> int irq, int level)
>>>>>     qemu_set_irq(s->cpu_intr, level);
>>>>> }
>>>>> 
>>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>>>> +{
>>>>> +    switch (irq_num) {
>>>>> +    case 0:
>>>>> +        return s->dev.config[0x55] >> 4;
>>>>> +    case 1:
>>>>> +        return s->dev.config[0x56] & 0xf;
>>>>> +    case 2:
>>>>> +        return s->dev.config[0x56] >> 4;
>>>>> +    case 3:
>>>>> +        return s->dev.config[0x57] >> 4;
>>>>> +    }
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>>>> +{
>>>>> +    ViaISAState *s = opaque;
>>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>>>> +    int pic_irq;
>>>>> +
>>>>> +    /* now we change the pic irq level according to the via irq 
>>>>> mappings */
>>>>> +    /* XXX: optimize */
>>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>>>> +        int i, pic_level;
>>>>> +
>>>>> +        /* The pic level is the logical OR of all the PCI irqs mapped 
>>>>> to it. */
>>>>> +        pic_level = 0;
>>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>>>> +            }
>>>>> +        }
>>>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> static void via_isa_realize(PCIDevice *d, Error **errp)
>>>>> {
>>>>>     ViaISAState *s = VIA_ISA(d);
>>>>> @@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error 
>>>>> **errp)
>>>>>     int i;
>>>>>
>>>>>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>>>>> +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", 
>>>>> PCI_NUM_PINS);
>>>> 
>>>> This line is a Pegasos2 specific addition for fixing its IRQ handling. 
>>>> Since this code must also work with the Fuloong2e board we should aim for 
>>>> a minimal changeset here which renders this line out of scope.
>>>> 
>>>> Let's keep the two series separate since now I need to watch two series 
>>>> for comments. Please use Based-on: tag next time instead.
>>> 
>>> Well, it's not. It's part of the QDev model for VT8231 that allows it to 
>>> be connected by boards so I think this belongs here otherwise this won't 
>>> even compile because the function you've added would be unused and bail on 
>>> -Werror. Let's not make this more difficult than it is. I'm OK with 
>>> reasonable changes but what's your goal now? You can't get rid of this 
>>> line as it's how QDev can model it. Either I have to call into this model 
>>> or have to export these pins as gpios.
>> 
>> Exporting the pins is a separate aspect on top of implementing PCI IRQ 
>> routing. To make this clear and obvious this should be a dedicated patch. 
>> In case either patch has an issue this would also ease and therefore 
>> acellerate discussions.
>
> How would you do that? Introduce via_isa_set_pci_irq() as unused in this 
> patch then have a one line patch to add connecting it to gpio pins? That just 
> makes no sense. This is not modeling the chip with QDev and then the boatd

This is now modeling...

> can connect it as appropriate modeling the board that the chip is in. So if 
> fuloon2e needs to do that then it should. I'll check that as I was focusing

fuloong2e

> on pegasos2 for now. You can't use pci_bur_irqs here because that breaks 
> pegasos2 so no point in doing that way.
>
> Regards,
> BALATON Zoltan
>
>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

On Mon, 27 Feb 2023, BALATON Zoltan wrote:
> On Mon, 27 Feb 2023, BALATON Zoltan wrote:
>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>> Am 26. Februar 2023 23:33:20 UTC schrieb BALATON Zoltan 
>>> <balaton@eik.bme.hu>:
>>>> On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>>>>> Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan 
>>>>> <balaton@eik.bme.hu>:
>>>>>> From: Bernhard Beschow <shentey@gmail.com>
>>>>>> 
>>>>>> The real VIA south bridges implement a PCI IRQ router which is 
>>>>>> configured
>>>>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>>>>>> needs to implement it as well.
>>>>>> 
>>>>>> Note: The implementation was taken from piix4_set_irq() in 
>>>>>> hw/isa/piix4.
>>>>>> 
>>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it 
>>>>>> can
>>>>>> be connected in board code; remove some empty lines]
>>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
>>>>>> ---
>>>>>> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 39 insertions(+)
>>>>>> 
>>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>>>> index 3f9bd0c04d..4025f9bcdc 100644
>>>>>> --- a/hw/isa/vt82c686.c
>>>>>> +++ b/hw/isa/vt82c686.c
>>>>>> @@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void 
>>>>>> *opaque, int irq, int level)
>>>>>>     qemu_set_irq(s->cpu_intr, level);
>>>>>> }
>>>>>> 
>>>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>>>>>> +{
>>>>>> +    switch (irq_num) {
>>>>>> +    case 0:
>>>>>> +        return s->dev.config[0x55] >> 4;
>>>>>> +    case 1:
>>>>>> +        return s->dev.config[0x56] & 0xf;
>>>>>> +    case 2:
>>>>>> +        return s->dev.config[0x56] >> 4;
>>>>>> +    case 3:
>>>>>> +        return s->dev.config[0x57] >> 4;
>>>>>> +    }
>>>>>> +    return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>>>>>> +{
>>>>>> +    ViaISAState *s = opaque;
>>>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>>>>> +    int pic_irq;
>>>>>> +
>>>>>> +    /* now we change the pic irq level according to the via irq 
>>>>>> mappings */
>>>>>> +    /* XXX: optimize */
>>>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>>>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>>>>> +        int i, pic_level;
>>>>>> +
>>>>>> +        /* The pic level is the logical OR of all the PCI irqs mapped 
>>>>>> to it. */
>>>>>> +        pic_level = 0;
>>>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>>>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>>>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>>>>> +            }
>>>>>> +        }
>>>>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> static void via_isa_realize(PCIDevice *d, Error **errp)
>>>>>> {
>>>>>>     ViaISAState *s = VIA_ISA(d);
>>>>>> @@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error 
>>>>>> **errp)
>>>>>>     int i;
>>>>>>
>>>>>>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>>>>>> +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", 
>>>>>> PCI_NUM_PINS);
>>>>> 
>>>>> This line is a Pegasos2 specific addition for fixing its IRQ handling. 
>>>>> Since this code must also work with the Fuloong2e board we should aim 
>>>>> for a minimal changeset here which renders this line out of scope.
>>>>> 
>>>>> Let's keep the two series separate since now I need to watch two series 
>>>>> for comments. Please use Based-on: tag next time instead.
>>>> 
>>>> Well, it's not. It's part of the QDev model for VT8231 that allows it to 
>>>> be connected by boards so I think this belongs here otherwise this won't 
>>>> even compile because the function you've added would be unused and bail 
>>>> on -Werror. Let's not make this more difficult than it is. I'm OK with 
>>>> reasonable changes but what's your goal now? You can't get rid of this 
>>>> line as it's how QDev can model it. Either I have to call into this model 
>>>> or have to export these pins as gpios.
>>> 
>>> Exporting the pins is a separate aspect on top of implementing PCI IRQ 
>>> routing. To make this clear and obvious this should be a dedicated patch. 
>>> In case either patch has an issue this would also ease and therefore 
>>> acellerate discussions.
>> 
>> How would you do that? Introduce via_isa_set_pci_irq() as unused in this 
>> patch then have a one line patch to add connecting it to gpio pins? That 
>> just makes no sense. This is not modeling the chip with QDev and then the 
>> boatd
>
> This is now modeling...
>
>> can connect it as appropriate modeling the board that the chip is in. So if 
>> fuloon2e needs to do that then it should. I'll check that as I was focusing
>
> fuloong2e

I've checked fuloong2e and it still works as before. PCI bus is handled by 
bonito on that board so your patch would actually break it. The VIA chip 
is a PCIDevice. You're not supposed to replace the interrupts of the bus 
it's connected to from this model as that should be done by the pci-host 
or the board. Therefore modeling the chip's PIRQ/PINT pins as gpios which 
is the QDev concept for that is right and your usage of pci_set_irq here 
is wrong.

Please stop breaking my series. I had a working and tested series (still 
have that on my pegasos2 branch in case we end up chosing that) which was 
changed but you to something that is now conceptually wrong but still 
works because of guests don't change firmware defaults to share PCI 
interrupts with internal functions just to fulfill your assumption that 
internal functions are PCI devices (which now I have proof that they don't 
conform to that PCI standard doc, look at the comment in the last patch 
about PCI Command register in via-ac97 and compare that with the chip 
datasheet) but OK if this allows simlpler code in QEMU and still works I 
can accept that but don't push ideas that are wrong.

Regards,
BALATON Zoltan

Re: [PATCH v3 0/8] Pegasos2 fixes and audio output support

[BALATON Zoltan]

On Sun, 26 Feb 2023, BALATON Zoltan wrote:
> Hello,
>
> This is marked v3 to avoid confusion with previously separate patches
> that already had v2, even if the series had no v2 yet.
>
> This series now includes all patches needed to get AmigaOS 4.1 run
> well on pegasos2 and add audio output to this board. It has 3 parts:
> patches 1-3 improve hw/display/sm501 model to avoid graphics problems

Gerd, Peter, Daniel or Philippe,

Could you please review and merge the first 3 sm501 patches that's 
independent of the rest of the series and useful in themselves to reduce 
the size of this series and make it less confusing? The rest is still 
debated but at least these sm501 patches should be simple to review and 
merge now.

Regards,
BALATON Zoltan

> that were present with AmigaOS; patches 4-6 fix PCI interrupt routing
> in VIA VT8231 model and in pegasos2 board that fixes PCI cards such as
> network or sound card not working before; finally patches 7-8 add
> basic implementation of the via-ac97 audio part of VT8231 (also used
> on VT82C686B) for output that is the default audio device on pegasos2.
>
> This version was re-tested by Rene Engel with latest AmigaOS version
> and runs as well as my original series did (posted a video with that
> before). This works now on an M1 MacStudio on macOS where it was
> unusable before. I've also tested it on Linux x86_64 with older
> AmigaOS version that also boots and makes sound and verified MorphOS
> still boots and also has sound now.
>
> One known problem with this version that includes Berhard's
> alternative vt82c686 patches is with MorphOS which uses level
> sensitive mode of the i8259 PIC that QEMU does not support so it hangs
> when multiple devices try to raise a shared IRQ. I could work around
> that in my otiginal series (see here:
> https://lists.nongnu.org/archive/html/qemu-ppc/2023-02/msg00403.html )
> where this works and was also tested, that version is available here:
> https://osdn.net/projects/qmiga/scm/git/qemu/tree/pegasos2/
> but I could not convince Bernhard so I now expect him to provide a
> work around for that. This isn't a blocker though as MorphOS already
> runs on mac99 and sam460ex and only available as a time limited demo
> (they only sell licenses for real hardware) so not really usable apart
> from testing anyway so getting it running on pegasos2 would be nice
> but not a prioriey, more important is that AmigaOS runs for which this
> is the only viable machine as sam460ex version runs much slower. So
> I'd like this to be merged for 8.0 as it is now or only minor chnages
> (or alternatively we can return to my series which was also tested the
> same way and apart from different VIA IRQ router modelling contains
> the same patches).
>
> Please review and let me know who will take care of merging this for 8.0.
>
> Regards,
> BALATON Zoltan
>
> BALATON Zoltan (6):
>  hw/display/sm501: Implement more 2D raster operations
>  hw/display/sm501: Add fallbacks to pixman routines
>  hw/display/sm501: Add debug property to control pixman usage
>  hw/ppc/pegasos2: Fix PCI interrupt routing
>  hw/audio/ac97: Split off some definitions to a header
>  hw/audio/via-ac97: Basic implementation of audio playback
>
> Bernhard Beschow (2):
>  hw/isa/vt82c686: Implement PCI IRQ routing
>  hw/usb/vt82c686-uhci-pci: Use PCI IRQ routing
>
> hw/audio/ac97.c            |  43 +---
> hw/audio/ac97.h            |  65 ++++++
> hw/audio/trace-events      |   6 +
> hw/audio/via-ac97.c        | 455 ++++++++++++++++++++++++++++++++++++-
> hw/display/sm501.c         | 119 ++++++++--
> hw/isa/trace-events        |   1 +
> hw/isa/vt82c686.c          |  41 +++-
> hw/pci-host/mv64361.c      |   4 -
> hw/ppc/pegasos2.c          |  26 ++-
> hw/usb/vt82c686-uhci-pci.c |  12 -
> include/hw/isa/vt82c686.h  |  25 ++
> 11 files changed, 706 insertions(+), 91 deletions(-)
> create mode 100644 hw/audio/ac97.h
>
>

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[Bernhard Beschow]

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

On Mon, Feb 27, 2023 at 1:57 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:

> On Mon, 27 Feb 2023, BALATON Zoltan wrote:
> > On Mon, 27 Feb 2023, BALATON Zoltan wrote:
> >> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> >>> Am 26. Februar 2023 23:33:20 UTC schrieb BALATON Zoltan
> >>> <balaton@eik.bme.hu>:
> >>>> On Sun, 26 Feb 2023, Bernhard Beschow wrote:
> >>>>> Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan
> >>>>> <balaton@eik.bme.hu>:
> >>>>>> From: Bernhard Beschow <shentey@gmail.com>
> >>>>>>
> >>>>>> The real VIA south bridges implement a PCI IRQ router which is
> >>>>>> configured
> >>>>>> by the BIOS or the OS. In order to respect these configurations,
> QEMU
> >>>>>> needs to implement it as well.
> >>>>>>
> >>>>>> Note: The implementation was taken from piix4_set_irq() in
> >>>>>> hw/isa/piix4.
> >>>>>>
> >>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> >>>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so
> it
> >>>>>> can
> >>>>>> be connected in board code; remove some empty lines]
> >>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> >>>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de>
> >>>>>> ---
> >>>>>> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
> >>>>>> 1 file changed, 39 insertions(+)
> >>>>>>
> >>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> >>>>>> index 3f9bd0c04d..4025f9bcdc 100644
> >>>>>> --- a/hw/isa/vt82c686.c
> >>>>>> +++ b/hw/isa/vt82c686.c
> >>>>>> @@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void
> >>>>>> *opaque, int irq, int level)
> >>>>>>     qemu_set_irq(s->cpu_intr, level);
> >>>>>> }
> >>>>>>
> >>>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
> >>>>>> +{
> >>>>>> +    switch (irq_num) {
> >>>>>> +    case 0:
> >>>>>> +        return s->dev.config[0x55] >> 4;
> >>>>>> +    case 1:
> >>>>>> +        return s->dev.config[0x56] & 0xf;
> >>>>>> +    case 2:
> >>>>>> +        return s->dev.config[0x56] >> 4;
> >>>>>> +    case 3:
> >>>>>> +        return s->dev.config[0x57] >> 4;
> >>>>>> +    }
> >>>>>> +    return 0;
> >>>>>> +}
> >>>>>> +
> >>>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int
> level)
> >>>>>> +{
> >>>>>> +    ViaISAState *s = opaque;
> >>>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
> >>>>>> +    int pic_irq;
> >>>>>> +
> >>>>>> +    /* now we change the pic irq level according to the via irq
> >>>>>> mappings */
> >>>>>> +    /* XXX: optimize */
> >>>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
> >>>>>> +    if (pic_irq < ISA_NUM_IRQS) {
> >>>>>> +        int i, pic_level;
> >>>>>> +
> >>>>>> +        /* The pic level is the logical OR of all the PCI irqs
> mapped
> >>>>>> to it. */
> >>>>>> +        pic_level = 0;
> >>>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
> >>>>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
> >>>>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
> >>>>>> +            }
> >>>>>> +        }
> >>>>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
> >>>>>> +    }
> >>>>>> +}
> >>>>>> +
> >>>>>> static void via_isa_realize(PCIDevice *d, Error **errp)
> >>>>>> {
> >>>>>>     ViaISAState *s = VIA_ISA(d);
> >>>>>> @@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error
> >>>>>> **errp)
> >>>>>>     int i;
> >>>>>>
> >>>>>>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
> >>>>>> +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq",
> >>>>>> PCI_NUM_PINS);
> >>>>>
> >>>>> This line is a Pegasos2 specific addition for fixing its IRQ
> handling.
> >>>>> Since this code must also work with the Fuloong2e board we should
> aim
> >>>>> for a minimal changeset here which renders this line out of scope.
> >>>>>
> >>>>> Let's keep the two series separate since now I need to watch two
> series
> >>>>> for comments. Please use Based-on: tag next time instead.
> >>>>
> >>>> Well, it's not. It's part of the QDev model for VT8231 that allows it
> to
> >>>> be connected by boards so I think this belongs here otherwise this
> won't
> >>>> even compile because the function you've added would be unused and
> bail
> >>>> on -Werror. Let's not make this more difficult than it is. I'm OK
> with
> >>>> reasonable changes but what's your goal now? You can't get rid of
> this
> >>>> line as it's how QDev can model it. Either I have to call into this
> model
> >>>> or have to export these pins as gpios.
> >>>
> >>> Exporting the pins is a separate aspect on top of implementing PCI IRQ
> >>> routing. To make this clear and obvious this should be a dedicated
> patch.
> >>> In case either patch has an issue this would also ease and therefore
> >>> acellerate discussions.
> >>
> >> How would you do that? Introduce via_isa_set_pci_irq() as unused in
> this
> >> patch then have a one line patch to add connecting it to gpio pins?
> That
> >> just makes no sense. This is not modeling the chip with QDev and then
> the
> >> boatd
> >
> > This is now modeling...
> >
> >> can connect it as appropriate modeling the board that the chip is in.
> So if
> >> fuloon2e needs to do that then it should. I'll check that as I was
> focusing
> >
> > fuloong2e
>
> I've checked fuloong2e and it still works as before. PCI bus is handled by
> bonito on that board so your patch would actually break it. The VIA chip
> is a PCIDevice. You're not supposed to replace the interrupts of the bus
> it's connected to from this model as that should be done by the pci-host
> or the board. Therefore modeling the chip's PIRQ/PINT pins as gpios which
> is the QDev concept for that is right and your usage of pci_set_irq here
> is wrong.
>

Works for me:
(08/84)
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e:
PASS (2.77 s)


>
> Please stop breaking my series. I had a working and tested series (still
> have that on my pegasos2 branch in case we end up chosing that) which was
> changed but you to something that is now conceptually wrong but still
> works because of guests don't change firmware defaults to share PCI
> interrupts with internal functions just to fulfill your assumption that
> internal functions are PCI devices (which now I have proof that they don't
> conform to that PCI standard doc, look at the comment in the last patch
> about PCI Command register in via-ac97 and compare that with the chip
> datasheet) but OK if this allows simlpler code in QEMU and still works I
> can accept that but don't push ideas that are wrong.
>
> Regards,
> BALATON Zoltan
>

[-- Attachment #2: Type: text/html, Size: 10117 bytes --]

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[Mark Cave-Ayland]

On 27/02/2023 16:52, Bernhard Beschow wrote:

> On Mon, Feb 27, 2023 at 1:57 PM BALATON Zoltan <balaton@eik.bme.hu 
> <mailto:balaton@eik.bme.hu>> wrote:
> 
>     On Mon, 27 Feb 2023, BALATON Zoltan wrote:
>      > On Mon, 27 Feb 2023, BALATON Zoltan wrote:
>      >> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>      >>> Am 26. Februar 2023 23:33:20 UTC schrieb BALATON Zoltan
>      >>> <balaton@eik.bme.hu <mailto:balaton@eik.bme.hu>>:
>      >>>> On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>      >>>>> Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan
>      >>>>> <balaton@eik.bme.hu <mailto:balaton@eik.bme.hu>>:
>      >>>>>> From: Bernhard Beschow <shentey@gmail.com <mailto:shentey@gmail.com>>
>      >>>>>>
>      >>>>>> The real VIA south bridges implement a PCI IRQ router which is
>      >>>>>> configured
>      >>>>>> by the BIOS or the OS. In order to respect these configurations, QEMU
>      >>>>>> needs to implement it as well.
>      >>>>>>
>      >>>>>> Note: The implementation was taken from piix4_set_irq() in
>      >>>>>> hw/isa/piix4.
>      >>>>>>
>      >>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com
>     <mailto:shentey@gmail.com>>
>      >>>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it
>      >>>>>> can
>      >>>>>> be connected in board code; remove some empty lines]
>      >>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu
>     <mailto:balaton@eik.bme.hu>>
>      >>>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de <mailto:ReneEngel80@emailn.de>>
>      >>>>>> ---
>      >>>>>> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>      >>>>>> 1 file changed, 39 insertions(+)
>      >>>>>>
>      >>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>      >>>>>> index 3f9bd0c04d..4025f9bcdc 100644
>      >>>>>> --- a/hw/isa/vt82c686.c
>      >>>>>> +++ b/hw/isa/vt82c686.c
>      >>>>>> @@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void
>      >>>>>> *opaque, int irq, int level)
>      >>>>>>     qemu_set_irq(s->cpu_intr, level);
>      >>>>>> }
>      >>>>>>
>      >>>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
>      >>>>>> +{
>      >>>>>> +    switch (irq_num) {
>      >>>>>> +    case 0:
>      >>>>>> +        return s->dev.config[0x55] >> 4;
>      >>>>>> +    case 1:
>      >>>>>> +        return s->dev.config[0x56] & 0xf;
>      >>>>>> +    case 2:
>      >>>>>> +        return s->dev.config[0x56] >> 4;
>      >>>>>> +    case 3:
>      >>>>>> +        return s->dev.config[0x57] >> 4;
>      >>>>>> +    }
>      >>>>>> +    return 0;
>      >>>>>> +}
>      >>>>>> +
>      >>>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
>      >>>>>> +{
>      >>>>>> +    ViaISAState *s = opaque;
>      >>>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>      >>>>>> +    int pic_irq;
>      >>>>>> +
>      >>>>>> +    /* now we change the pic irq level according to the via irq
>      >>>>>> mappings */
>      >>>>>> +    /* XXX: optimize */
>      >>>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>      >>>>>> +    if (pic_irq < ISA_NUM_IRQS) {
>      >>>>>> +        int i, pic_level;
>      >>>>>> +
>      >>>>>> +        /* The pic level is the logical OR of all the PCI irqs mapped
>      >>>>>> to it. */
>      >>>>>> +        pic_level = 0;
>      >>>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>      >>>>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>      >>>>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>      >>>>>> +            }
>      >>>>>> +        }
>      >>>>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>      >>>>>> +    }
>      >>>>>> +}
>      >>>>>> +
>      >>>>>> static void via_isa_realize(PCIDevice *d, Error **errp)
>      >>>>>> {
>      >>>>>>     ViaISAState *s = VIA_ISA(d);
>      >>>>>> @@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error
>      >>>>>> **errp)
>      >>>>>>     int i;
>      >>>>>>
>      >>>>>>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>      >>>>>> +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq",
>      >>>>>> PCI_NUM_PINS);
>      >>>>>
>      >>>>> This line is a Pegasos2 specific addition for fixing its IRQ handling.
>      >>>>> Since this code must also work with the Fuloong2e board we should aim
>      >>>>> for a minimal changeset here which renders this line out of scope.
>      >>>>>
>      >>>>> Let's keep the two series separate since now I need to watch two series
>      >>>>> for comments. Please use Based-on: tag next time instead.
>      >>>>
>      >>>> Well, it's not. It's part of the QDev model for VT8231 that allows it to
>      >>>> be connected by boards so I think this belongs here otherwise this won't
>      >>>> even compile because the function you've added would be unused and bail
>      >>>> on -Werror. Let's not make this more difficult than it is. I'm OK with
>      >>>> reasonable changes but what's your goal now? You can't get rid of this
>      >>>> line as it's how QDev can model it. Either I have to call into this model
>      >>>> or have to export these pins as gpios.
>      >>>
>      >>> Exporting the pins is a separate aspect on top of implementing PCI IRQ
>      >>> routing. To make this clear and obvious this should be a dedicated patch.
>      >>> In case either patch has an issue this would also ease and therefore
>      >>> acellerate discussions.
>      >>
>      >> How would you do that? Introduce via_isa_set_pci_irq() as unused in this
>      >> patch then have a one line patch to add connecting it to gpio pins? That
>      >> just makes no sense. This is not modeling the chip with QDev and then the
>      >> boatd
>      >
>      > This is now modeling...
>      >
>      >> can connect it as appropriate modeling the board that the chip is in. So if
>      >> fuloon2e needs to do that then it should. I'll check that as I was focusing
>      >
>      > fuloong2e
> 
>     I've checked fuloong2e and it still works as before. PCI bus is handled by
>     bonito on that board so your patch would actually break it. The VIA chip
>     is a PCIDevice. You're not supposed to replace the interrupts of the bus
>     it's connected to from this model as that should be done by the pci-host
>     or the board. Therefore modeling the chip's PIRQ/PINT pins as gpios which
>     is the QDev concept for that is right and your usage of pci_set_irq here
>     is wrong.
> 
> 
> Works for me:
> (08/84) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e: 
> PASS(2.77 s)

The bonito code is interesting in that the IRQ is swizzled in pci_bonito_map_irq() to 
the internal IRQ, and then pci_bonito_set_irq() sets the output (CPU?) IRQ 
accordingly. This means that the routing is currently fixed based upon the slot 
number, rather than using the VIA PCI IRQ routing. This bit will need some thought as 
to how this interacts with pci_bus_irqs() in your proposed patch, feel free to 
suggest a suitable approach.

>     Please stop breaking my series. I had a working and tested series (still
>     have that on my pegasos2 branch in case we end up chosing that) which was
>     changed but you to something that is now conceptually wrong but still
>     works because of guests don't change firmware defaults to share PCI
>     interrupts with internal functions just to fulfill your assumption that
>     internal functions are PCI devices (which now I have proof that they don't
>     conform to that PCI standard doc, look at the comment in the last patch
>     about PCI Command register in via-ac97 and compare that with the chip
>     datasheet) but OK if this allows simlpler code in QEMU and still works I
>     can accept that but don't push ideas that are wrong.

I don't think this is fair on Bernhard since his patches are coming from the place of 
improving the VIA ISA bridge implementation so that it can be used as an alternative 
to PIIX. From my perspective the work on PIIX and VIA has been well thought out, with 
a good understanding of the relevant specifications: the intention has always been to 
improve the existing code based upon a working implementation, and not to 
deliberately cause more work.


ATB,

Mark.

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

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

On Wed, 1 Mar 2023, Mark Cave-Ayland wrote:
> On 27/02/2023 16:52, Bernhard Beschow wrote:
>> On Mon, Feb 27, 2023 at 1:57 PM BALATON Zoltan <balaton@eik.bme.hu 
>> <mailto:balaton@eik.bme.hu>> wrote:
>>
>>     On Mon, 27 Feb 2023, BALATON Zoltan wrote:
>>      > On Mon, 27 Feb 2023, BALATON Zoltan wrote:
>>      >> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>      >>> Am 26. Februar 2023 23:33:20 UTC schrieb BALATON Zoltan
>>      >>> <balaton@eik.bme.hu <mailto:balaton@eik.bme.hu>>:
>>      >>>> On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>>      >>>>> Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan
>>      >>>>> <balaton@eik.bme.hu <mailto:balaton@eik.bme.hu>>:
>>      >>>>>> From: Bernhard Beschow <shentey@gmail.com 
>> <mailto:shentey@gmail.com>>
>>      >>>>>>
>>      >>>>>> The real VIA south bridges implement a PCI IRQ router which is
>>      >>>>>> configured
>>      >>>>>> by the BIOS or the OS. In order to respect these 
>> configurations, QEMU
>>      >>>>>> needs to implement it as well.
>>      >>>>>>
>>      >>>>>> Note: The implementation was taken from piix4_set_irq() in
>>      >>>>>> hw/isa/piix4.
>>      >>>>>>
>>      >>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com
>>     <mailto:shentey@gmail.com>>
>>      >>>>>> [balaton: declare gpio inputs instead of changing pci bus irqs 
>> so it
>>      >>>>>> can
>>      >>>>>> be connected in board code; remove some empty lines]
>>      >>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu
>>     <mailto:balaton@eik.bme.hu>>
>>      >>>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de 
>> <mailto:ReneEngel80@emailn.de>>
>>      >>>>>> ---
>>      >>>>>> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>      >>>>>> 1 file changed, 39 insertions(+)
>>      >>>>>>
>>      >>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>      >>>>>> index 3f9bd0c04d..4025f9bcdc 100644
>>      >>>>>> --- a/hw/isa/vt82c686.c
>>      >>>>>> +++ b/hw/isa/vt82c686.c
>>      >>>>>> @@ -604,6 +604,44 @@ static void via_isa_request_i8259_irq(void
>>      >>>>>> *opaque, int irq, int level)
>>      >>>>>>     qemu_set_irq(s->cpu_intr, level);
>>      >>>>>> }
>>      >>>>>>
>>      >>>>>> +static int via_isa_get_pci_irq(const ViaISAState *s, int 
>> irq_num)
>>      >>>>>> +{
>>      >>>>>> +    switch (irq_num) {
>>      >>>>>> +    case 0:
>>      >>>>>> +        return s->dev.config[0x55] >> 4;
>>      >>>>>> +    case 1:
>>      >>>>>> +        return s->dev.config[0x56] & 0xf;
>>      >>>>>> +    case 2:
>>      >>>>>> +        return s->dev.config[0x56] >> 4;
>>      >>>>>> +    case 3:
>>      >>>>>> +        return s->dev.config[0x57] >> 4;
>>      >>>>>> +    }
>>      >>>>>> +    return 0;
>>      >>>>>> +}
>>      >>>>>> +
>>      >>>>>> +static void via_isa_set_pci_irq(void *opaque, int irq_num, int 
>> level)
>>      >>>>>> +{
>>      >>>>>> +    ViaISAState *s = opaque;
>>      >>>>>> +    PCIBus *bus = pci_get_bus(&s->dev);
>>      >>>>>> +    int pic_irq;
>>      >>>>>> +
>>      >>>>>> +    /* now we change the pic irq level according to the via 
>> irq
>>      >>>>>> mappings */
>>      >>>>>> +    /* XXX: optimize */
>>      >>>>>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>>      >>>>>> +    if (pic_irq < ISA_NUM_IRQS) {
>>      >>>>>> +        int i, pic_level;
>>      >>>>>> +
>>      >>>>>> +        /* The pic level is the logical OR of all the PCI irqs 
>> mapped
>>      >>>>>> to it. */
>>      >>>>>> +        pic_level = 0;
>>      >>>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>      >>>>>> +            if (pic_irq == via_isa_get_pci_irq(s, i)) {
>>      >>>>>> +                pic_level |= pci_bus_get_irq_level(bus, i);
>>      >>>>>> +            }
>>      >>>>>> +        }
>>      >>>>>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>>      >>>>>> +    }
>>      >>>>>> +}
>>      >>>>>> +
>>      >>>>>> static void via_isa_realize(PCIDevice *d, Error **errp)
>>      >>>>>> {
>>      >>>>>>     ViaISAState *s = VIA_ISA(d);
>>      >>>>>> @@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, 
>> Error
>>      >>>>>> **errp)
>>      >>>>>>     int i;
>>      >>>>>>
>>      >>>>>>     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>>      >>>>>> +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq",
>>      >>>>>> PCI_NUM_PINS);
>>      >>>>>
>>      >>>>> This line is a Pegasos2 specific addition for fixing its IRQ 
>> handling.
>>      >>>>> Since this code must also work with the Fuloong2e board we 
>> should aim
>>      >>>>> for a minimal changeset here which renders this line out of 
>> scope.
>>      >>>>>
>>      >>>>> Let's keep the two series separate since now I need to watch two 
>> series
>>      >>>>> for comments. Please use Based-on: tag next time instead.
>>      >>>>
>>      >>>> Well, it's not. It's part of the QDev model for VT8231 that 
>> allows it to
>>      >>>> be connected by boards so I think this belongs here otherwise 
>> this won't
>>      >>>> even compile because the function you've added would be unused 
>> and bail
>>      >>>> on -Werror. Let's not make this more difficult than it is. I'm OK 
>> with
>>      >>>> reasonable changes but what's your goal now? You can't get rid of 
>> this
>>      >>>> line as it's how QDev can model it. Either I have to call into 
>> this model
>>      >>>> or have to export these pins as gpios.
>>      >>>
>>      >>> Exporting the pins is a separate aspect on top of implementing PCI 
>> IRQ
>>      >>> routing. To make this clear and obvious this should be a dedicated 
>> patch.
>>      >>> In case either patch has an issue this would also ease and 
>> therefore
>>      >>> acellerate discussions.
>>      >>
>>      >> How would you do that? Introduce via_isa_set_pci_irq() as unused in 
>> this
>>      >> patch then have a one line patch to add connecting it to gpio pins? 
>> That
>>      >> just makes no sense. This is not modeling the chip with QDev and 
>> then the
>>      >> boatd
>>      >
>>      > This is now modeling...
>>      >
>>      >> can connect it as appropriate modeling the board that the chip is 
>> in. So if
>>      >> fuloon2e needs to do that then it should. I'll check that as I was 
>> focusing
>>      >
>>      > fuloong2e
>>
>>     I've checked fuloong2e and it still works as before. PCI bus is handled 
>> by
>>     bonito on that board so your patch would actually break it. The VIA 
>> chip
>>     is a PCIDevice. You're not supposed to replace the interrupts of the 
>> bus
>>     it's connected to from this model as that should be done by the 
>> pci-host
>>     or the board. Therefore modeling the chip's PIRQ/PINT pins as gpios 
>> which
>>     is the QDev concept for that is right and your usage of pci_set_irq 
>> here
>>     is wrong.
>> 
>> 
>> Works for me:
>> (08/84) 
>> tests/avocado/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e: 
>> PASS(2.77 s)
>
> The bonito code is interesting in that the IRQ is swizzled in 
> pci_bonito_map_irq() to the internal IRQ, and then pci_bonito_set_irq() sets 
> the output (CPU?) IRQ accordingly. This means that the routing is currently 
> fixed based upon the slot number, rather than using the VIA PCI IRQ routing. 
> This bit will need some thought as to how this interacts with pci_bus_irqs() 
> in your proposed patch, feel free to suggest a suitable approach.

I don't know if you've checked what that test does but it seems it tries 
to boot an old debian kernel from 
linux-image-3.16.0-6-loongson-2e_3.16.56-1+deb8u1_mipsel.deb
which even on master does not boot, it ends in:
[    0.256000] PCI: Enabling device 0000:00:06.0 (0000 -> 0003)
[    0.256000] Data bus error, epc == ffffffff8045aed8, ra == ffffffff8045aeb8
[    0.256000] Oops[#1]:

so this test likely won't notice if you break PCI interrupts replacing the 
bonito irq handler from VT82C686B and either this board does not seem to 
be working anyway or that kernel may not be the right one to boot (the 
board firmware does produxe picture and a boot console but maybe that's 
not using interrupts). I did not reply to this before because it's quite 
irrelevant to the series we were discussing so it would just be a 
distraction.

>>     Please stop breaking my series. I had a working and tested series 
>> (still
>>     have that on my pegasos2 branch in case we end up chosing that) which 
>> was
>>     changed but you to something that is now conceptually wrong but still
>>     works because of guests don't change firmware defaults to share PCI
>>     interrupts with internal functions just to fulfill your assumption that
>>     internal functions are PCI devices (which now I have proof that they 
>> don't
>>     conform to that PCI standard doc, look at the comment in the last patch
>>     about PCI Command register in via-ac97 and compare that with the chip
>>     datasheet) but OK if this allows simlpler code in QEMU and still works 
>> I
>>     can accept that but don't push ideas that are wrong.
>
> I don't think this is fair on Bernhard since his patches are coming from the 
> place of improving the VIA ISA bridge implementation so that it can be used 
> as an alternative to PIIX. From my perspective the work on PIIX and VIA has 
> been well thought out, with a good understanding of the relevant 
> specifications: the intention has always been to improve the existing code 
> based upon a working implementation, and not to deliberately cause more work.

I fully support that and tried to help with review or info that could help 
but this particular change and the way it was forced on me did cause me to 
do more work. Anyway I hope we could come to a conclusion now so don't 
start again.

Regards,
BALATON Zoltan

Re: [PATCH v3 4/8] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

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

On Wed, 1 Mar 2023, Mark Cave-Ayland wrote:
> On 27/02/2023 16:52, Bernhard Beschow wrote:
>> On Mon, Feb 27, 2023 at 1:57 PM BALATON Zoltan <balaton@eik.bme.hu 
>> <mailto:balaton@eik.bme.hu>> wrote:
>> in. So if
>>      >> fuloon2e needs to do that then it should. I'll check that as I was 
>> focusing
>>      >
>>      > fuloong2e
>>
>>     I've checked fuloong2e and it still works as before. PCI bus is handled by
>>     bonito on that board so your patch would actually break it. The VIA chip
>>     is a PCIDevice. You're not supposed to replace the interrupts of the bus
>>     it's connected to from this model as that should be done by the pci-host
>>     or the board. Therefore modeling the chip's PIRQ/PINT pins as gpios which
>>     is the QDev concept for that is right and your usage of pci_set_irq here
>>     is wrong.
>> 
>> 
>> Works for me:
>> (08/84) 
>> tests/avocado/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e: 
>> PASS(2.77 s)
>
> The bonito code is interesting in that the IRQ is swizzled in 
> pci_bonito_map_irq() to the internal IRQ, and then pci_bonito_set_irq() sets 
> the output (CPU?) IRQ accordingly. This means that the routing is currently 
> fixed based upon the slot number, rather than using the VIA PCI IRQ routing. 
> This bit will need some thought as to how this interacts with pci_bus_irqs() 
> in your proposed patch, feel free to suggest a suitable approach.

I believe the fuloong2e may be similarly connected as the pegasos2. The 
Marvell Discovery II mv64361 was based on a MIPS counterpart so the 
concepts may be similar in these just the CPU arch is different.

This doc https://wiki.qemu.org/images/0/09/Bonito64-spec.pdf says the 
bonito north bridge has some GPin and GPIO pins which are connected to the 
interrupt controller (see section 5.15). Probably you can infer which pins 
PCI IRQs should come in from the map_irq function in the bonito model. I'd 
expect GPIO0-3 based on description in the table in section 6.1

On the other hand the board's firmware suggests PCI interrupt lines are 
also connected to the PIRQ pins of th 686B:

https://github.com/loongson-community/pmon/blob/master/sys/dev/pci/vt82c686_devbd2e.c

(if this is the right file to look at as there are different versions but 
dev board 2e said to inlude fuloong2e in the main README). Then in 686B 
PCI interrupts are mapped to 9.10.11.13 with the PnP IRQ routing registers 
in 686B.

This could then be modeled similarly to how I did it in this series for 
pegasos2: One could add gpio inputs in bonito to model the pins where the 
PCI interrupt lines are connected then connect these together in the board 
code just like they are wired on the real board.

Although this board does not have any PCI slots so these are only for the 
on board PCI devices: https://www.linux-mips.org/wiki/Fuloong_2E but a 
similar dev board may have 4 PCI slots.

Regards,
BALATON Zoltan