[Qemu-devel] Re: >2 serial ports?

[Qemu-devel] Re: >2 serial ports?

[Gerd Hoffmann]

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

On 03/17/10 09:38, Michael Tokarev wrote:
> Since 0.12, it appears that kvm does not allow more than
> 2 serial ports for a guest:
>
> $ kvm \
>   -serial unix:s1,server,nowait \
>   -serial unix:s2,server,nowait \
>   -serial unix:s3,server,nowait
> isa irq 4 already assigned
>
> Is there a work-around for this?

Oh, well, yes, I remember.  qemu is more strict on ISA irq sharing now. 
  A bit too strict.

/me goes dig out a old patch which never made it upstream for some 
reason I forgot.  Attached.

HTH,
   Gerd

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

>From 7d5d53e8a23544ac6413487a8ecdd43537ade9f3 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 4d489d2..bd2f69c 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;
 
@@ -71,7 +72,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);
@@ -82,10 +85,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.6.6.1

[Qemu-devel] Re: >2 serial ports?

[Michael Tokarev]

Gerd Hoffmann wrote:
> On 03/17/10 09:38, Michael Tokarev wrote:
>> Since 0.12, it appears that kvm does not allow more than
>> 2 serial ports for a guest:
>>
>> $ kvm \
>>   -serial unix:s1,server,nowait \
>>   -serial unix:s2,server,nowait \
>>   -serial unix:s3,server,nowait
>> isa irq 4 already assigned
>>
>> Is there a work-around for this?
> 
> Oh, well, yes, I remember.  qemu is more strict on ISA irq sharing now.
>  A bit too strict.
> 
> /me goes dig out a old patch which never made it upstream for some
> reason I forgot.  Attached.

I tried the patch, and it now appears to work.  I did not try
to run various stress tests so far, but basic tests are fine.

Thank you Gerd!  And I think it's time to push it finally :)

/mjt

Re: [Qemu-devel] Re: >2 serial ports?

[Paul Brook]

> Oh, well, yes, I remember.  qemu is more strict on ISA irq sharing now.
>   A bit too strict.
> 
> /me goes dig out a old patch which never made it upstream for some
> reason I forgot.  Attached.

This is wrong. Two devices should never be manipulating the same qemu_irq 
object.  If you want multiple devices connected to the same IRQ then you need 
an explicit multiplexer. e.g. arm_timer.c:sp804_set_irq.

Paul

Re: [Qemu-devel] Re: >2 serial ports?

[Michael Tokarev]

Paul Brook wrote at Wed, 17 Mar 2010 11:18:09 +0000:
>> Oh, well, yes, I remember.  qemu is more strict on ISA irq sharing now.
>>   A bit too strict.
>>
>> /me goes dig out a old patch which never made it upstream for some
>> reason I forgot.  Attached.
> 
> This is wrong. Two devices should never be manipulating the same qemu_irq 
> object.  If you want multiple devices connected to the same IRQ then you need 
> an explicit multiplexer. e.g. arm_timer.c:sp804_set_irq.

So... what we have to do here?

I've looked at the mentioned routine, here it is:

/* Merge the IRQs from the two component devices.  */
static void sp804_set_irq(void *opaque, int irq, int level)
{
    sp804_state *s = (sp804_state *)opaque;

    s->level[irq] = level;
    qemu_set_irq(s->irq, s->level[0] || s->level[1]);
}

But I know nothing about qemu internals, so don't quite
understand how to do this in case of serial ports.  I
see it is tracking two timers and raises the irq level
if at least one half is raised...  That to say - I've
got the idea, but how to apply it to serial ports?

Thanks!

/mjt

Re: [Qemu-devel] Re: >2 serial ports?

[Avi Kivity]

On 03/22/2010 10:35 AM, Michael Tokarev wrote:
> Paul Brook wrote at Wed, 17 Mar 2010 11:18:09 +0000:
>    
>>> Oh, well, yes, I remember.  qemu is more strict on ISA irq sharing now.
>>>    A bit too strict.
>>>
>>> /me goes dig out a old patch which never made it upstream for some
>>> reason I forgot.  Attached.
>>>        
>> This is wrong. Two devices should never be manipulating the same qemu_irq
>> object.  If you want multiple devices connected to the same IRQ then you need
>> an explicit multiplexer. e.g. arm_timer.c:sp804_set_irq.
>>      
> So... what we have to do here?
>
> I've looked at the mentioned routine, here it is:
>
> /* Merge the IRQs from the two component devices.  */
> static void sp804_set_irq(void *opaque, int irq, int level)
> {
>      sp804_state *s = (sp804_state *)opaque;
>
>      s->level[irq] = level;
>      qemu_set_irq(s->irq, s->level[0] || s->level[1]);
> }
>
> But I know nothing about qemu internals, so don't quite
> understand how to do this in case of serial ports.  I
> see it is tracking two timers and raises the irq level
> if at least one half is raised...  That to say - I've
> got the idea, but how to apply it to serial ports?
>    

Two devices have the same s->irq.  Give each on its own qemu_irq, and 
feed it into a multiplexer that ORs them together and sends the result 
to the interrupt controller's qemu_irq:


S1 ----qemu_irq---->+------+

                     |  mux  +---qemu_irq----->  irq controller

S2 ----qemu_irq---->+------+


(the ascii art will come out all wrong, I know it)

-- 

Do not meddle in the internals of kernels, for they are subtle and quick to panic.