diff mbox

isa_reserve_irq()

Message ID 4A82D4E4.90200@sgi.com
State Superseded
Headers show

Commit Message

Jes Sorensen Aug. 12, 2009, 2:42 p.m. UTC 
On 08/11/2009 10:36 PM, Gerd Hoffmann wrote:
> Attached a patch. It will:
>
> (1) make isa-bus maintain isa irqs, complain when allocating
> already taken irqs.
> (2) note that (1) works only for isa devices converted to qdev
> already (floppy and ps2/kbd/mouse right now), so more work
> is needed to make this really useful.
> (3) split floppy init into isa and sysbus versions.
> (4) add sysbus->isa bridge & fix -M isapc breakage.

Hi Gerd,

This looks great! I have implemented isa_reserve_irq() on top of this.
It allows one to reserve ISA irqs without converting them to qdev, which
for certain interrupts makes sense, and I have also made the other users
call it until they are converted to qdev.

Let me know what you think. With this I get what I am after.

Cheers,
Jes

Comments

Gerd Hoffmann Aug. 12, 2009, 2:58 p.m. UTC | #1 
On 08/12/09 16:42, Jes Sorensen wrote:
> Hi Gerd,
>
> This looks great! I have implemented isa_reserve_irq() on top of this.
> It allows one to reserve ISA irqs without converting them to qdev, which
> for certain interrupts makes sense,

which ones?

> and I have also made the other users
> call it until they are converted to qdev.

I think that deserves a comment clearly stating that this interface is a 
(temporary?) thing for not-yet converted devices.

Otherwise it looks fine to me.

cheers,
   Gerd
Jes Sorensen Aug. 12, 2009, 3:10 p.m. UTC | #2 
On 08/12/2009 04:58 PM, Gerd Hoffmann wrote:
> On 08/12/09 16:42, Jes Sorensen wrote:
>> Hi Gerd,
>>
>> This looks great! I have implemented isa_reserve_irq() on top of this.
>> It allows one to reserve ISA irqs without converting them to qdev, which
>> for certain interrupts makes sense,
>
> which ones?

ferr

>> and I have also made the other users
>> call it until they are converted to qdev.
>
> I think that deserves a comment clearly stating that this interface is a
> (temporary?) thing for not-yet converted devices.
>
> Otherwise it looks fine to me.

If you think it makes sense to convert ferr as well, then I'll be happy
to add the comment.

Anthony also pointed out that hpet shouldn't be on the list - it makes
sense. The BIOS can filter out irq2 based on the irq0override flag, so I
will take out the hpet bits too.

Cheers,
Jes
Gerd Hoffmann Aug. 12, 2009, 3:28 p.m. UTC | #3 
On 08/12/09 17:10, Jes Sorensen wrote:
> On 08/12/2009 04:58 PM, Gerd Hoffmann wrote:
>> On 08/12/09 16:42, Jes Sorensen wrote:
>>> Hi Gerd,
>>>
>>> This looks great! I have implemented isa_reserve_irq() on top of this.
>>> It allows one to reserve ISA irqs without converting them to qdev, which
>>> for certain interrupts makes sense,
>>
>> which ones?
>
> ferr

Ah, that one.

>> I think that deserves a comment clearly stating that this interface is a
>> (temporary?) thing for not-yet converted devices.
>>
>> Otherwise it looks fine to me.
>
> If you think it makes sense to convert ferr as well, then I'll be happy
> to add the comment.

Hmm, I don't think it makes sense to pull a dummy device out of thin 
air, isa_reserve_irq() will do fine.  But it should be be clearly 
documented that this is for this special case and not-yet converted 
devices.  qdev-ified devices should use the other isa_*_irq functions 
instead.

cheers,
   Gerd
Jes Sorensen Aug. 12, 2009, 3:31 p.m. UTC | #4 
On 08/12/2009 05:28 PM, Gerd Hoffmann wrote:
> Hmm, I don't think it makes sense to pull a dummy device out of thin
> air, isa_reserve_irq() will do fine. But it should be be clearly
> documented that this is for this special case and not-yet converted
> devices. qdev-ified devices should use the other isa_*_irq functions
> instead.

Ok - do you want me to update the documentation, or is it fine as is
then?

Cheers,
Jes
Gerd Hoffmann Aug. 12, 2009, 3:41 p.m. UTC | #5 
On 08/12/09 17:31, Jes Sorensen wrote:
> On 08/12/2009 05:28 PM, Gerd Hoffmann wrote:
>> Hmm, I don't think it makes sense to pull a dummy device out of thin
>> air, isa_reserve_irq() will do fine. But it should be be clearly
>> documented that this is for this special case and not-yet converted
>> devices. qdev-ified devices should use the other isa_*_irq functions
>> instead.
>
> Ok - do you want me to update the documentation, or is it fine as is
> then?

Would be great to update the commit message and also place a comment 
into the source code.

thanks,
   Gerd
diff mbox

Patch

Introduce isa_reserve_irq() which marks an irq reserved and returns
the appropriate qemu_irq entry from the i8259 table.

isa_reserve_irq() is to be used to allocate ISA IRQs for devices which
have not yet, or are not meant to be converted to qdev. Like the ferr
irq.

This patch goes on top of Gerd Hoffmann's which makes isa-bus.c own
the ISA irq table.

Signed-off-by: Jes Sorensen <jes@sgi.com>

---
 hw/cs4231a.c |   12 +++++-------
 hw/hpet.c    |    5 +++++
 hw/ide.c     |    7 +++++--
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    1 +
 hw/pc.c      |   23 +++++++++++++----------
 hw/sb16.c    |   25 ++++++++++++-------------
 7 files changed, 55 insertions(+), 32 deletions(-)

Index: qemu/hw/cs4231a.c
===================================================================
--- qemu.orig/hw/cs4231a.c
+++ qemu/hw/cs4231a.c
@@ -60,10 +60,9 @@  static struct {
 
 typedef struct CSState {
     QEMUSoundCard card;
-    qemu_irq *pic;
+    qemu_irq pic;
     uint32_t regs[CS_REGS];
     uint8_t dregs[CS_DREGS];
-    int irq;
     int dma;
     int port;
     int shift;
@@ -483,7 +482,7 @@  IO_WRITE_PROTO (cs_write)
         case Alternate_Feature_Status:
             if ((s->dregs[iaddr] & PI) && !(val & PI)) {
                 /* XXX: TI CI */
-                qemu_irq_lower (s->pic[s->irq]);
+                qemu_irq_lower (s->pic);
                 s->regs[Status] &= ~INT;
             }
             s->dregs[iaddr] = val;
@@ -503,7 +502,7 @@  IO_WRITE_PROTO (cs_write)
 
     case Status:
         if (s->regs[Status] & INT) {
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         s->regs[Status] &= ~INT;
         s->dregs[Alternate_Feature_Status] &= ~(PI | CI | TI);
@@ -588,7 +587,7 @@  static int cs_dma_read (void *opaque, in
         s->regs[Status] |= INT;
         s->dregs[Alternate_Feature_Status] |= PI;
         s->transferred = 0;
-        qemu_irq_raise (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
     }
     else {
         s->transferred += written;
@@ -643,8 +642,7 @@  int cs4231a_init (qemu_irq *pic)
 
     s = qemu_mallocz (sizeof (*s));
 
-    s->pic = pic;
-    s->irq = conf.irq;
+    s->pic = isa_reserve_irq(conf.irq);
     s->dma = conf.dma;
     s->port = conf.port;
 
Index: qemu/hw/hpet.c
===================================================================
--- qemu.orig/hw/hpet.c
+++ qemu/hw/hpet.c
@@ -29,6 +29,7 @@ 
 #include "console.h"
 #include "qemu-timer.h"
 #include "hpet_emul.h"
+#include "isa.h"
 
 //#define HPET_DEBUG
 #ifdef HPET_DEBUG
@@ -574,6 +575,10 @@  void hpet_init(qemu_irq *irq) {
     s = qemu_mallocz(sizeof(HPETState));
     hpet_statep = s;
     s->irqs = irq;
+    /*
+     * Rely on irq2 being used by HPET
+     */
+    isa_reserve_irq(2);
     for (i=0; i<HPET_NUM_TIMERS; i++) {
         HPETTimer *timer = &s->timer[i];
         timer->qemu_timer = qemu_new_timer(vm_clock, hpet_timer, timer);
Index: qemu/hw/ide.c
===================================================================
--- qemu.orig/hw/ide.c
+++ qemu/hw/ide.c
@@ -3414,8 +3414,8 @@  void pci_piix3_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
-    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
-    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
+    ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], isa_reserve_irq(14));
+    ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], isa_reserve_irq(15));
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
     ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
 
@@ -3454,6 +3454,9 @@  void pci_piix4_ide_init(PCIBus *bus, Blo
     pci_register_bar((PCIDevice *)d, 4, 0x10,
                            PCI_ADDRESS_SPACE_IO, bmdma_map);
 
+    /*
+     * These should call isa_reserve_irq() instead when MIPS supports it
+     */
     ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], pic[14]);
     ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], pic[15]);
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
Index: qemu/hw/isa-bus.c
===================================================================
--- qemu.orig/hw/isa-bus.c
+++ qemu/hw/isa-bus.c
@@ -85,6 +85,20 @@  void isa_connect_irq(ISADevice *dev, int
     }
 }
 
+qemu_irq isa_reserve_irq(int isairq)
+{
+    if (isairq < 0 || isairq > 15) {
+        fprintf(stderr, "isa irq %d invalid\n", isairq);
+        exit(1);
+    }
+    if (isabus->assigned & (1 << isairq)) {
+        fprintf(stderr, "isa irq %d already assigned\n", isairq);
+        exit(1);
+    }
+    isabus->assigned |= (1 << isairq);
+    return isabus->irqs[isairq];
+}
+
 void isa_init_irq(ISADevice *dev, qemu_irq *p)
 {
     assert(dev->nirqs < ARRAY_SIZE(dev->irqs));
Index: qemu/hw/isa.h
===================================================================
--- qemu.orig/hw/isa.h
+++ qemu/hw/isa.h
@@ -27,6 +27,7 @@  struct ISADeviceInfo {
 ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 void isa_connect_irq(ISADevice *dev, int devirq, int isairq);
+qemu_irq isa_reserve_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create_simple(const char *name, uint32_t iobase, uint32_t iobase2);
Index: qemu/hw/pc.c
===================================================================
--- qemu.orig/hw/pc.c
+++ qemu/hw/pc.c
@@ -1037,13 +1037,14 @@  static void audio_init (PCIBus *pci_bus,
 }
 #endif
 
-static void pc_init_ne2k_isa(NICInfo *nd, qemu_irq *pic)
+static void pc_init_ne2k_isa(NICInfo *nd)
 {
     static int nb_ne2k = 0;
 
     if (nb_ne2k == NE2000_NB_MAX)
         return;
-    isa_ne2000_init(ne2000_io[nb_ne2k], pic[ne2000_irq[nb_ne2k]], nd);
+    isa_ne2000_init(ne2000_io[nb_ne2k],
+                    isa_reserve_irq(ne2000_irq[nb_ne2k]), nd);
     nb_ne2k++;
 }
 
@@ -1265,7 +1266,6 @@  static void pc_init1(ram_addr_t ram_size
 
     cpu_irq = qemu_allocate_irqs(pic_irq_request, NULL, 1);
     i8259 = i8259_init(cpu_irq[0]);
-    ferr_irq = i8259[13];
 
     if (pci_enabled) {
         pci_bus = i440fx_init(&i440fx_state, i8259);
@@ -1275,6 +1275,7 @@  static void pc_init1(ram_addr_t ram_size
         isa_bus_new(NULL);
     }
     isa_bus_irqs(i8259);
+    ferr_irq = isa_reserve_irq(13);
 
     /* init basic PC hardware */
     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
@@ -1300,7 +1301,7 @@  static void pc_init1(ram_addr_t ram_size
         }
     }
 
-    rtc_state = rtc_init(0x70, i8259[8], 2000);
+    rtc_state = rtc_init(0x70, isa_reserve_irq(8), 2000);
 
     qemu_register_boot_set(pc_boot_set, rtc_state);
 
@@ -1310,7 +1311,7 @@  static void pc_init1(ram_addr_t ram_size
     if (pci_enabled) {
         ioapic = ioapic_init();
     }
-    pit = pit_init(0x40, i8259[0]);
+    pit = pit_init(0x40, isa_reserve_irq(0));
     pcspk_init(pit);
     if (!no_hpet) {
         hpet_init(i8259);
@@ -1321,14 +1322,14 @@  static void pc_init1(ram_addr_t ram_size
 
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         if (serial_hds[i]) {
-            serial_init(serial_io[i], i8259[serial_irq[i]], 115200,
+            serial_init(serial_io[i], isa_reserve_irq(serial_irq[i]), 115200,
                         serial_hds[i]);
         }
     }
 
     for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
         if (parallel_hds[i]) {
-            parallel_init(parallel_io[i], i8259[parallel_irq[i]],
+            parallel_init(parallel_io[i], isa_reserve_irq(parallel_irq[i]),
                           parallel_hds[i]);
         }
     }
@@ -1339,7 +1340,7 @@  static void pc_init1(ram_addr_t ram_size
         NICInfo *nd = &nd_table[i];
 
         if (!pci_enabled || (nd->model && strcmp(nd->model, "ne2k_isa") == 0))
-            pc_init_ne2k_isa(nd, i8259);
+            pc_init_ne2k_isa(nd);
         else
             pci_nic_init(nd, "ne2k_pci", NULL);
     }
@@ -1360,7 +1361,8 @@  static void pc_init1(ram_addr_t ram_size
         pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1, i8259);
     } else {
         for(i = 0; i < MAX_IDE_BUS; i++) {
-            isa_ide_init(ide_iobase[i], ide_iobase2[i], i8259[ide_irq[i]],
+            isa_ide_init(ide_iobase[i], ide_iobase2[i],
+                         isa_reserve_irq(ide_irq[i]),
 	                 hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
         }
     }
@@ -1390,7 +1392,8 @@  static void pc_init1(ram_addr_t ram_size
         i2c_bus *smbus;
 
         /* TODO: Populate SPD eeprom data.  */
-        smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100, i8259[9]);
+        smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
+                              isa_reserve_irq(9));
         for (i = 0; i < 8; i++) {
             DeviceState *eeprom;
             eeprom = qdev_create((BusState *)smbus, "smbus-eeprom");
Index: qemu/hw/sb16.c
===================================================================
--- qemu.orig/hw/sb16.c
+++ qemu/hw/sb16.c
@@ -56,7 +56,7 @@  static struct {
 
 typedef struct SB16State {
     QEMUSoundCard card;
-    qemu_irq *pic;
+    qemu_irq pic;
     int irq;
     int dma;
     int hdma;
@@ -190,7 +190,7 @@  static void aux_timer (void *opaque)
 {
     SB16State *s = opaque;
     s->can_write = 1;
-    qemu_irq_raise (s->pic[s->irq]);
+    qemu_irq_raise (s->pic);
 }
 
 #define DMA8_AUTO 1
@@ -598,7 +598,7 @@  static void command (SB16State *s, uint8
         case 0xf3:
             dsp_out_data (s, 0xaa);
             s->mixer_regs[0x82] |= (cmd == 0xf2) ? 1 : 2;
-            qemu_irq_raise (s->pic[s->irq]);
+            qemu_irq_raise (s->pic);
             break;
 
         case 0xf9:
@@ -766,7 +766,7 @@  static void complete (SB16State *s)
                 bytes = samples << s->fmt_stereo << (s->fmt_bits == 16);
                 ticks = (bytes * ticks_per_sec) / freq;
                 if (ticks < ticks_per_sec / 1024) {
-                    qemu_irq_raise (s->pic[s->irq]);
+                    qemu_irq_raise (s->pic);
                 }
                 else {
                     if (s->aux_ts) {
@@ -858,10 +858,10 @@  static void legacy_reset (SB16State *s)
 
 static void reset (SB16State *s)
 {
-    qemu_irq_lower (s->pic[s->irq]);
+    qemu_irq_lower (s->pic);
     if (s->dma_auto) {
-        qemu_irq_raise (s->pic[s->irq]);
-        qemu_irq_lower (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
+        qemu_irq_lower (s->pic);
     }
 
     s->mixer_regs[0x82] = 0;
@@ -897,7 +897,7 @@  static IO_WRITE_PROTO (dsp_write)
             if (s->v2x6 == 1) {
                 if (0 && s->highspeed) {
                     s->highspeed = 0;
-                    qemu_irq_lower (s->pic[s->irq]);
+                    qemu_irq_lower (s->pic);
                     control (s, 0);
                 }
                 else {
@@ -1008,7 +1008,7 @@  static IO_READ_PROTO (dsp_read)
         if (s->mixer_regs[0x82] & 1) {
             ack = 1;
             s->mixer_regs[0x82] &= 1;
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         break;
 
@@ -1017,7 +1017,7 @@  static IO_READ_PROTO (dsp_read)
         if (s->mixer_regs[0x82] & 2) {
             ack = 1;
             s->mixer_regs[0x82] &= 2;
-            qemu_irq_lower (s->pic[s->irq]);
+            qemu_irq_lower (s->pic);
         }
         break;
 
@@ -1231,7 +1231,7 @@  static int SB_read_DMA (void *opaque, in
 
     if (s->left_till_irq <= 0) {
         s->mixer_regs[0x82] |= (nchan & 4) ? 2 : 1;
-        qemu_irq_raise (s->pic[s->irq]);
+        qemu_irq_raise (s->pic);
         if (0 == s->dma_auto) {
             control (s, 0);
             speaker (s, 0);
@@ -1408,8 +1408,7 @@  int SB16_init (qemu_irq *pic)
     s = qemu_mallocz (sizeof (*s));
 
     s->cmd = -1;
-    s->pic = pic;
-    s->irq = conf.irq;
+    s->pic = isa_reserve_irq(conf.irq);
     s->dma = conf.dma;
     s->hdma = conf.hdma;
     s->port = conf.port;