From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60798 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pr59s-0004tM-EN for qemu-devel@nongnu.org; Sun, 20 Feb 2011 04:00:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pr59q-0007iX-Uo for qemu-devel@nongnu.org; Sun, 20 Feb 2011 04:00:48 -0500 Received: from fmmailgate01.web.de ([217.72.192.221]:49622) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pr59q-0007iP-D7 for qemu-devel@nongnu.org; Sun, 20 Feb 2011 04:00:46 -0500 Message-ID: <4D60D83C.3050207@web.de> Date: Sun, 20 Feb 2011 10:00:44 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <4D600446.3070008@web.de> <20110219232548.GF4580@hall.aurel32.net> In-Reply-To: <20110219232548.GF4580@hall.aurel32.net> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigB1182A679E3CF6D300D21DD4" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [STABLE][PATCH] isa-bus: Remove bogus IRQ sharing check List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: Kevin Wolf , =?ISO-8859-15?Q?Andreas_F=E4rber?= , Gerd Hoffmann , qemu-devel , Alexander Graf This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigB1182A679E3CF6D300D21DD4 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2011-02-20 00:25, Aurelien Jarno wrote: > On Sat, Feb 19, 2011 at 06:56:22PM +0100, Jan Kiszka wrote: >> From: Jan Kiszka >> >> Nothing prevented IRQ sharing on the ISA bus in principle. Not all >> boards supported this, neither each and every card nor driver and OS. >> Still, there existed valid IRQ sharing scenarios, (at least) two of th= em >> can also be found in QEMU: >2 PC UARTs and the PREP IDE buses. >> >> So remove this artificial restriction from our ISA model and reenable >> both PREP IDE buses. >=20 > What about the following patch from Gerd Hoffmann? >=20 > http://www.mail-archive.com/qemu-devel@nongnu.org/msg55314.html >=20 > It only allow sharing ISA IRQ for the same kind of devices, which seems= > to also be the case for real ISA IRQ sharing, at least for the example > that comes to my mind. Such setups were probably the most common sharing scenarios on ISA as then the same driver was involved and could be made aware of the sharing more easily. Still, this has nothing to do with what the bus was capable of in general. I remember very well the fiddling with IRQ jumpers (later they became software-configurable) on sound cards, I/O extension adapters, etc. The worst you got from setting a conflict was a non-working device or a locked-up box. I do not remember ever blowing up some hardware this way. This also complies with what you find about ISA on the web. So, I don't see any need for this sharing check, with or without a restriction to different devices. Jan >=20 > I haven't found time to try it though. >=20 >> Signed-off-by: Jan Kiszka >> --- >> >> hw/hpet.c | 1 - >> hw/ide/piix.c | 2 +- >> hw/ide/via.c | 2 +- >> hw/isa-bus.c | 16 +++------------- >> hw/isa.h | 2 +- >> hw/mips_fulong2e.c | 2 +- >> hw/mips_malta.c | 4 ++-- >> hw/pc.c | 2 +- >> hw/pc_piix.c | 4 ++-- >> hw/ppc_prep.c | 2 +- >> 10 files changed, 13 insertions(+), 24 deletions(-) >> >> diff --git a/hw/hpet.c b/hw/hpet.c >> index 82a9a21..91ebb75 100644 >> --- a/hw/hpet.c >> +++ b/hw/hpet.c >> @@ -713,7 +713,6 @@ static int hpet_init(SysBusDevice *dev) >> s->capability |=3D (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT; >> s->capability |=3D ((HPET_CLK_PERIOD) << 32); >> =20 >> - isa_reserve_irq(RTC_ISA_IRQ); >> qdev_init_gpio_in(&dev->qdev, hpet_handle_rtc_irq, 1); >> =20 >> /* HPET Area */ >> diff --git a/hw/ide/piix.c b/hw/ide/piix.c >> index d4289af..c349644 100644 >> --- a/hw/ide/piix.c >> +++ b/hw/ide/piix.c >> @@ -122,7 +122,7 @@ static void pci_piix_init_ports(PCIIDEState *d) { >> for (i =3D 0; i < 2; i++) { >> ide_bus_new(&d->bus[i], &d->dev.qdev, i); >> ide_init_ioport(&d->bus[i], port_info[i].iobase, port_info[i]= =2Eiobase2); >> - ide_init2(&d->bus[i], isa_reserve_irq(port_info[i].isairq)); >> + ide_init2(&d->bus[i], isa_get_irq(port_info[i].isairq)); >> =20 >> bmdma_init(&d->bus[i], &d->bmdma[i]); >> d->bmdma[i].bus =3D &d->bus[i]; >> diff --git a/hw/ide/via.c b/hw/ide/via.c >> index 0e90679..04f3290 100644 >> --- a/hw/ide/via.c >> +++ b/hw/ide/via.c >> @@ -145,7 +145,7 @@ static void vt82c686b_init_ports(PCIIDEState *d) {= >> for (i =3D 0; i < 2; i++) { >> ide_bus_new(&d->bus[i], &d->dev.qdev, i); >> ide_init_ioport(&d->bus[i], port_info[i].iobase, port_info[i]= =2Eiobase2); >> - ide_init2(&d->bus[i], isa_reserve_irq(port_info[i].isairq)); >> + ide_init2(&d->bus[i], isa_get_irq(port_info[i].isairq)); >> =20 >> bmdma_init(&d->bus[i], &d->bmdma[i]); >> d->bmdma[i].bus =3D &d->bus[i]; >> diff --git a/hw/isa-bus.c b/hw/isa-bus.c >> index 6f349a5..d07aa41 100644 >> --- a/hw/isa-bus.c >> +++ b/hw/isa-bus.c >> @@ -25,7 +25,6 @@ >> struct ISABus { >> BusState qbus; >> qemu_irq *irqs; >> - uint32_t assigned; >> }; >> static ISABus *isabus; >> target_phys_addr_t isa_mem_base =3D 0; >> @@ -61,33 +60,24 @@ void isa_bus_irqs(qemu_irq *irqs) >> } >> =20 >> /* >> - * isa_reserve_irq() reserves the ISA irq and returns the correspondi= ng >> - * qemu_irq entry for the i8259. >> + * isa_get_irq() returns the corresponding qemu_irq entry for the i82= 59. >> * >> * This function is only for special cases such as the 'ferr', and >> * temporary use for normal devices until they are converted to qdev.= >> */ >> -qemu_irq isa_reserve_irq(int isairq) >> +qemu_irq isa_get_irq(int isairq) >> { >> if (isairq < 0 || isairq > 15) { >> hw_error("isa irq %d invalid", isairq); >> } >> - if (isabus->assigned & (1 << isairq)) { >> - hw_error("isa irq %d already assigned", isairq); >> - } >> - isabus->assigned |=3D (1 << isairq); >> return isabus->irqs[isairq]; >> } >> =20 >> void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq) >> { >> assert(dev->nirqs < ARRAY_SIZE(dev->isairq)); >> - if (isabus->assigned & (1 << isairq)) { >> - hw_error("isa irq %d already assigned", isairq); >> - } >> - isabus->assigned |=3D (1 << isairq); >> dev->isairq[dev->nirqs] =3D isairq; >> - *p =3D isabus->irqs[isairq]; >> + *p =3D isa_get_irq(isairq); >> dev->nirqs++; >> } >> =20 >> diff --git a/hw/isa.h b/hw/isa.h >> index e26abfa..d2b6126 100644 >> --- a/hw/isa.h >> +++ b/hw/isa.h >> @@ -26,7 +26,7 @@ struct ISADeviceInfo { >> =20 >> ISABus *isa_bus_new(DeviceState *dev); >> void isa_bus_irqs(qemu_irq *irqs); >> -qemu_irq isa_reserve_irq(int isairq); >> +qemu_irq isa_get_irq(int isairq); >> void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq); >> void isa_init_ioport(ISADevice *dev, uint16_t ioport); >> void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t l= ength); >> diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c >> index 2783ed5..3b0abdb 100644 >> --- a/hw/mips_fulong2e.c >> +++ b/hw/mips_fulong2e.c >> @@ -369,7 +369,7 @@ static void mips_fulong2e_init(ram_addr_t ram_size= , const char *boot_device, >> qdev_init_nofail(eeprom); >> =20 >> /* init other devices */ >> - pit =3D pit_init(0x40, isa_reserve_irq(0)); >> + pit =3D pit_init(0x40, isa_get_irq(0)); >> cpu_exit_irq =3D qemu_allocate_irqs(cpu_request_exit, NULL, 1); >> DMA_init(0, cpu_exit_irq); >> =20 >> diff --git a/hw/mips_malta.c b/hw/mips_malta.c >> index 930c51c..f86bcff 100644 >> --- a/hw/mips_malta.c >> +++ b/hw/mips_malta.c >> @@ -919,7 +919,7 @@ void mips_malta_init (ram_addr_t ram_size, >> isa_bus_irqs(i8259); >> pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1); >> usb_uhci_piix4_init(pci_bus, piix4_devfn + 2); >> - smbus =3D piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100, isa_res= erve_irq(9), >> + smbus =3D piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100, isa_get= _irq(9), >> NULL, NULL, 0); >> eeprom_buf =3D qemu_mallocz(8 * 256); /* XXX: make this persisten= t */ >> for (i =3D 0; i < 8; i++) { >> @@ -930,7 +930,7 @@ void mips_malta_init (ram_addr_t ram_size, >> qdev_prop_set_ptr(eeprom, "data", eeprom_buf + (i * 256)); >> qdev_init_nofail(eeprom); >> } >> - pit =3D pit_init(0x40, isa_reserve_irq(0)); >> + pit =3D pit_init(0x40, isa_get_irq(0)); >> cpu_exit_irq =3D qemu_allocate_irqs(cpu_request_exit, NULL, 1); >> DMA_init(0, cpu_exit_irq); >> =20 >> diff --git a/hw/pc.c b/hw/pc.c >> index 56bf1d6..81d7a87 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -1118,7 +1118,7 @@ void pc_basic_device_init(qemu_irq *isa_irq, >> =20 >> qemu_register_boot_set(pc_boot_set, *rtc_state); >> =20 >> - pit =3D pit_init(0x40, isa_reserve_irq(0)); >> + pit =3D pit_init(0x40, isa_get_irq(0)); >> pcspk_init(pit); >> =20 >> for(i =3D 0; i < MAX_SERIAL_PORTS; i++) { >> diff --git a/hw/pc_piix.c b/hw/pc_piix.c >> index 2918454..97c5754 100644 >> --- a/hw/pc_piix.c >> +++ b/hw/pc_piix.c >> @@ -114,7 +114,7 @@ static void pc_init1(ram_addr_t ram_size, >> } >> isa_bus_irqs(isa_irq); >> =20 >> - pc_register_ferr_irq(isa_reserve_irq(13)); >> + pc_register_ferr_irq(isa_get_irq(13)); >> =20 >> pc_vga_init(pci_enabled? pci_bus: NULL); >> =20 >> @@ -170,7 +170,7 @@ static void pc_init1(ram_addr_t ram_size, >> smi_irq =3D qemu_allocate_irqs(pc_acpi_smi_interrupt, first_c= pu, 1); >> /* TODO: Populate SPD eeprom data. */ >> smbus =3D piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100, >> - isa_reserve_irq(9), *cmos_s3, *smi_irq,= >> + isa_get_irq(9), *cmos_s3, *smi_irq, >> kvm_enabled()); >> for (i =3D 0; i < 8; i++) { >> DeviceState *eeprom; >> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c >> index 6c1499a..6b22122 100644 >> --- a/hw/ppc_prep.c >> +++ b/hw/ppc_prep.c >> @@ -690,7 +690,7 @@ static void ppc_prep_init (ram_addr_t ram_size, >> hd[i] =3D drive_get(IF_IDE, i / MAX_IDE_DEVS, i % MAX_IDE_DEV= S); >> } >> =20 >> - for(i =3D 0; i < 1/*MAX_IDE_BUS*/; i++) { >> + for(i =3D 0; i < MAX_IDE_BUS; i++) { >> isa_ide_init(ide_iobase[i], ide_iobase2[i], ide_irq[i], >> hd[2 * i], >> hd[2 * i + 1]); >> --=20 >> 1.7.1 >> >=20 --------------enigB1182A679E3CF6D300D21DD4 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/ iEYEARECAAYFAk1g2DwACgkQitSsb3rl5xQlgQCgyPF/75CISbK0rdKFvJ/j7QZp NkQAniJdi3Yyc2UKftigIqRQK30OBLz9 =Vxnp -----END PGP SIGNATURE----- --------------enigB1182A679E3CF6D300D21DD4--