| Message ID | fbb016c7d0e19093335c237e15f5f6c62c4393b4.1678188711.git.balaton@eik.bme.hu |
|---|---|
| State | New |
| Headers | show |
| Series | Pegasos2 fixes and audio output support | expand |
Hi Zoltan, On 7/3/23 12:42, BALATON Zoltan wrote: > 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. The real chip may allow routing IRQs from > internal functions independently of PCI interrupts but since guests > usually configute it to a single shared interrupt we don't model that > here for simplicity. > > Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4. > > Suggested-by: Bernhard Beschow <shentey@gmail.com> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > Reviewed-by: Bernhard Beschow <shentey@gmail.com> > Tested-by: Rene Engel <ReneEngel80@emailn.de> > --- > hw/isa/vt82c686.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 42 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; Shouldn't this be & 0xf? (This is why I prefer extract8() over manual bits extraction) > + } > + return 0; > +}
On Mon, 30 Oct 2023, Philippe Mathieu-Daudé wrote: > On 7/3/23 12:42, BALATON Zoltan wrote: >> 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. The real chip may allow routing IRQs from >> internal functions independently of PCI interrupts but since guests >> usually configute it to a single shared interrupt we don't model that >> here for simplicity. >> >> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4. >> >> Suggested-by: Bernhard Beschow <shentey@gmail.com> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> Reviewed-by: Bernhard Beschow <shentey@gmail.com> >> Tested-by: Rene Engel <ReneEngel80@emailn.de> >> --- >> hw/isa/vt82c686.c | 42 ++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 42 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; > > Shouldn't this be & 0xf? No, the INTD value is actually in the high byte of reg 0x57. See e.g. page 73 in the VT8231 doc Revision 2.32. > (This is why I prefer extract8() over manual bits extraction) (I have two problems with deposit/extract. Have to remember which operand is what so it's less obvious to me and they have an assert which is not a good idea in an interrupt handlet that could be called millinos of times.) Regards, BALATON Zoltan
On Mon, 30 Oct 2023, BALATON Zoltan wrote: > On Mon, 30 Oct 2023, Philippe Mathieu-Daudé wrote: >> On 7/3/23 12:42, BALATON Zoltan wrote: >>> 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. The real chip may allow routing IRQs from >>> internal functions independently of PCI interrupts but since guests >>> usually configute it to a single shared interrupt we don't model that >>> here for simplicity. >>> >>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4. >>> >>> Suggested-by: Bernhard Beschow <shentey@gmail.com> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> Reviewed-by: Bernhard Beschow <shentey@gmail.com> >>> Tested-by: Rene Engel <ReneEngel80@emailn.de> >>> --- >>> hw/isa/vt82c686.c | 42 ++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 42 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; >> >> Shouldn't this be & 0xf? > > No, the INTD value is actually in the high byte of reg 0x57. See e.g. page 73 I mean high bits or nibble but you get what I mean. > in the VT8231 doc Revision 2.32. > >> (This is why I prefer extract8() over manual bits extraction) > > (I have two problems with deposit/extract. Have to remember which operand is > what so it's less obvious to me and they have an assert which is not a good > idea in an interrupt handlet that could be called millinos of times.) Also "handler" above... This is a very old message from when it was added, the current series just moves this unchanged. Regards, BALATON Zoltan
On 30/10/23 10:02, BALATON Zoltan wrote: > On Mon, 30 Oct 2023, Philippe Mathieu-Daudé wrote: >> On 7/3/23 12:42, BALATON Zoltan wrote: >>> 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. The real chip may allow routing IRQs from >>> internal functions independently of PCI interrupts but since guests >>> usually configute it to a single shared interrupt we don't model that >>> here for simplicity. >>> >>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4. >>> >>> Suggested-by: Bernhard Beschow <shentey@gmail.com> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> Reviewed-by: Bernhard Beschow <shentey@gmail.com> >>> Tested-by: Rene Engel <ReneEngel80@emailn.de> >>> --- >>> hw/isa/vt82c686.c | 42 ++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 42 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; >> >> Shouldn't this be & 0xf? > > No, the INTD value is actually in the high byte of reg 0x57. See e.g. > page 73 in the VT8231 doc Revision 2.32. Correct (I was looking at rev 0.8 which is incomplete there). Thanks, Phil.
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index 01e0148967..71da316052 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -604,6 +604,46 @@ 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 i, pic_level, pic_irq = via_isa_get_pci_irq(s, irq_num); + + /* IRQ 0: disabled, IRQ 2,8,13: reserved */ + if (!pic_irq) { + return; + } + if (unlikely(pic_irq == 2 || pic_irq == 8 || pic_irq == 13)) { + qemu_log_mask(LOG_GUEST_ERROR, "Invalid ISA IRQ routing"); + } + + /* 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); + } + } + /* Now we change the pic irq level according to the via irq mappings. */ + qemu_set_irq(s->isa_irqs_in[pic_irq], pic_level); +} + static void via_isa_realize(PCIDevice *d, Error **errp) { ViaISAState *s = VIA_ISA(d); @@ -627,6 +667,8 @@ static void via_isa_realize(PCIDevice *d, Error **errp) i8254_pit_init(isa_bus, 0x40, 0, NULL); i8257_dma_init(isa_bus, 0); + qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", PCI_NUM_PINS); + /* RTC */ qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000); if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {