[Qemu-devel] [RFC] prep: enable irq sharing on ide again

[Qemu-devel] [RFC] prep: enable irq sharing on ide again

[Alexander Graf]

The new ISA infrastructure checks for potential irq sharing bugs on
interrupt lines, because usually irq lines on isa can't be shared.

The PREP spec however mandates that the irq lines for both IDE ports
are shared and according to Aurelien this also used to work just fine.

So let's add a way to enable this sharing again, so we don't introduce
unnecessary regressions over older versions of Qemu.

Signed-off-by: Alexander Graf <agraf@suse.de>
CC: Andreas Färber <andreas.faerber@web.de>
CC: Aurelien Jarno <aurelien@aurel32.net>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ide/isa.c  |    7 ++++++-
 hw/isa-bus.c  |   13 +++++++++----
 hw/isa.h      |    3 +++
 hw/ppc_prep.c |    5 +++--
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 8c59c5a..2316826 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -66,10 +66,15 @@ static const VMStateDescription vmstate_ide_isa = {
 static int isa_ide_initfn(ISADevice *dev)
 {
     ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
+    int irq = s->isairq;
 
     ide_bus_new(&s->bus, &s->dev.qdev, 0);
     ide_init_ioport(&s->bus, s->iobase, s->iobase2);
-    isa_init_irq(dev, &s->irq, s->isairq);
+    if (irq >= 0) {
+        isa_init_irq(dev, &s->irq, irq);
+    } else {
+        isa_init_irq_nocheck(dev, &s->irq, -irq);
+    }
     isa_init_ioport_range(dev, s->iobase, 8);
     isa_init_ioport(dev, s->iobase2);
     ide_init2(&s->bus, s->irq);
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 0cb1afb..84f9c81 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -79,18 +79,23 @@ qemu_irq isa_reserve_irq(int isairq)
     return isabus->irqs[isairq];
 }
 
-void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
+void isa_init_irq_nocheck(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 |= (1 << isairq);
     dev->isairq[dev->nirqs] = isairq;
     *p = isabus->irqs[isairq];
     dev->nirqs++;
 }
 
+void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
+{
+    if (isabus->assigned & (1 << isairq)) {
+        hw_error("isa irq %d already assigned", isairq);
+    }
+    isa_init_irq_nocheck(dev, p, isairq);
+}
+
 static void isa_init_ioport_one(ISADevice *dev, uint16_t ioport)
 {
     assert(dev->nioports < ARRAY_SIZE(dev->ioports));
diff --git a/hw/isa.h b/hw/isa.h
index 19aa94c..031eed7 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -28,6 +28,9 @@ ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 qemu_irq isa_reserve_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
+/* version of init_irq without check for line sharing - only there for PREP
+   target */
+void isa_init_irq_nocheck(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 length);
 void isa_qdev_register(ISADeviceInfo *info);
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 6c1499a..e8d531a 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -75,7 +75,8 @@ qemu_log_mask(CPU_LOG_IOPORT, fmt, ## __VA_ARGS__)
 /* Constants for devices init */
 static const int ide_iobase[2] = { 0x1f0, 0x170 };
 static const int ide_iobase2[2] = { 0x3f6, 0x376 };
-static const int ide_irq[2] = { 13, 13 };
+/* negative numbers mean forced share enable */
+static const int ide_irq[2] = { -13, -13 };
 
 #define NE2000_NB_MAX 6
 
@@ -690,7 +691,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
         hd[i] = drive_get(IF_IDE, i / MAX_IDE_DEVS, i % MAX_IDE_DEVS);
     }
 
-    for(i = 0; i < 1/*MAX_IDE_BUS*/; i++) {
+    for(i = 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]);
-- 
1.6.0.2

[Qemu-devel] Re: [RFC] prep: enable irq sharing on ide again

[Gerd Hoffmann]

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

On 02/08/11 00:26, Alexander Graf wrote:
> The new ISA infrastructure checks for potential irq sharing bugs on
> interrupt lines, because usually irq lines on isa can't be shared.
>
> The PREP spec however mandates that the irq lines for both IDE ports
> are shared and according to Aurelien this also used to work just fine.
>
> So let's add a way to enable this sharing again, so we don't introduce
> unnecessary regressions over older versions of Qemu.

Had a patch for that, got shoot down for reasons I don't remember, 
attached for reference.  It basically allows IRQ sharing in case the two 
devices sharing the IRQ are of the same kind.  In that case you usually 
have a single guest driver handling both devices and IRQ sharing works 
most of the time.

I don't mind much which approach we take ...

cheers,
   Gerd

[-- Attachment #2: 0001-isa-refine-irq-reservations.patch --]
[-- Type: text/plain, Size: 2390 bytes --]

From 44d7b59ee9cc8a57e9999a7b6ecdf53798d2c74d Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Fri, 11 Sep 2009 13:43:46 +0200
Subject: [PATCH] isa: refine irq reservations

There are a few cases where IRQ sharing on the ISA bus is used and
possible.  In general only devices of the same kind can do that.
A few use cases:

  * serial lines 1+3 share irq 4
  * serial lines 2+4 share irq 3
  * parallel ports share irq 7
  * ppc/prep: ide ports share irq 13

This patch refines the irq reservation mechanism for the isa bus to
handle those cases.  It keeps track of the driver which owns the IRQ in
question and allows irq sharing for devices handled by the same driver.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/isa-bus.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 4e306de..a99e793 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -26,6 +26,7 @@ struct ISABus {
     BusState qbus;
     qemu_irq *irqs;
     uint32_t assigned;
+    DeviceInfo *irq_owner[16];
 };
 static ISABus *isabus;
 target_phys_addr_t isa_mem_base = 0;
@@ -72,7 +73,9 @@ qemu_irq isa_reserve_irq(int isairq)
         exit(1);
     }
     if (isabus->assigned & (1 << isairq)) {
-        fprintf(stderr, "isa irq %d already assigned\n", isairq);
+        DeviceInfo *owner = isabus->irq_owner[isairq];
+        fprintf(stderr, "isa irq %d already assigned (%s)\n",
+                isairq, owner ? owner->name : "unknown");
         exit(1);
     }
     isabus->assigned |= (1 << isairq);
@@ -83,10 +86,17 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
 {
     assert(dev->nirqs < ARRAY_SIZE(dev->isairq));
     if (isabus->assigned & (1 << isairq)) {
-        fprintf(stderr, "isa irq %d already assigned\n", isairq);
-        exit(1);
+        DeviceInfo *owner = isabus->irq_owner[isairq];
+        if (owner == dev->qdev.info) {
+            /* irq sharing is ok in case the same driver handles both */;
+        } else {
+            fprintf(stderr, "isa irq %d already assigned (%s)\n",
+                    isairq, owner ? owner->name : "unknown");
+            exit(1);
+        }
     }
     isabus->assigned |= (1 << isairq);
+    isabus->irq_owner[isairq] = dev->qdev.info;
     dev->isairq[dev->nirqs] = isairq;
     *p = isabus->irqs[isairq];
     dev->nirqs++;
-- 
1.7.1

[Qemu-devel] Re: [RFC] prep: enable irq sharing on ide again

[Alexander Graf]

Gerd Hoffmann wrote:
> On 02/08/11 00:26, Alexander Graf wrote:
>> The new ISA infrastructure checks for potential irq sharing bugs on
>> interrupt lines, because usually irq lines on isa can't be shared.
>>
>> The PREP spec however mandates that the irq lines for both IDE ports
>> are shared and according to Aurelien this also used to work just fine.
>>
>> So let's add a way to enable this sharing again, so we don't introduce
>> unnecessary regressions over older versions of Qemu.
>
> Had a patch for that, got shoot down for reasons I don't remember,
> attached for reference.  It basically allows IRQ sharing in case the
> two devices sharing the IRQ are of the same kind.  In that case you
> usually have a single guest driver handling both devices and IRQ
> sharing works most of the time.
>
> I don't mind much which approach we take ...

I'm very indifferent myself :). This is not an issue we should waste too
much time/effort on.


Alex

[Qemu-devel] Re: [RFC] prep: enable irq sharing on ide again

[Jan Kiszka]

On 2011-02-14 12:22, Gerd Hoffmann wrote:
> On 02/08/11 00:26, Alexander Graf wrote:
>> The new ISA infrastructure checks for potential irq sharing bugs on
>> interrupt lines, because usually irq lines on isa can't be shared.
>>
>> The PREP spec however mandates that the irq lines for both IDE ports
>> are shared and according to Aurelien this also used to work just fine.
>>
>> So let's add a way to enable this sharing again, so we don't introduce
>> unnecessary regressions over older versions of Qemu.
> 
> Had a patch for that, got shoot down for reasons I don't remember,
> attached for reference.  It basically allows IRQ sharing in case the two
> devices sharing the IRQ are of the same kind.  In that case you usually
> have a single guest driver handling both devices and IRQ sharing works
> most of the time.

Yeah, 3x -serial XXX is also broken for x86 targets as the third device
shares its IRQ with the first - like on real HW...

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux