Re: [Qemu-devel] [STABLE][PATCH] isa-bus: Remove bogus IRQ sharing check

[Jan Kiszka]

On 2011-02-19 23:45, Michael Tokarev wrote:
> 19.02.2011 20:56, Jan Kiszka wrote:
>> From: Jan Kiszka <address@hidden>
>>
>> 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 them
>> 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.
> 
> In qemu-kvm at least (I don't remember how different the relevant code
> has been between qemu and qemu-kvm at that time) this restriction exists

Both trees should not have had any differences in this area.

> since 0.12 version.  Before that it was at least possible to have >2 UARTS.
> Do you aware of this?  I don't know this code at all, so.. just asking :)

Oh, we have this since 0.13? Well...

> 
> I'd love to get this fixed for real instead of a hackish way we did this
> in the debian package, and it does not look like the change is problematic,
> but are you sure it should go to stable?

If we agree that removing is correct (I'm strongly convinced about
this), than at least 0.14.1 should benefit from it.

> 
> FWIW, I applied this patch to qemu-kvm 0.14 branch (had to hand-apply
> it to hw/pc.c, obvious s/isa_reserve_irq/isa_get_irq) and run some quick
> tests, it appears to work correctly here with multiple serial ports
> and e.g. serial console on ttyS4.
> 
> (Without the patch (and this is a separate bug), qemu-kvm prints:
> 
>  qemu: hardware error: isa irq 4 already assigned
>  CPU #0:
>  _
> 
> when asked to create 3 serial ports, and stays there till killed).

Yeah, saw this at well but somehow ignored it. It's a KVM bug, upstream
is fixed like this:

diff --git a/kvm-all.c b/kvm-all.c
index e6a7de4..e54b5bf 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -211,6 +211,7 @@ int kvm_init_vcpu(CPUState *env)

     env->kvm_fd = ret;
     env->kvm_state = s;
+    env->kvm_vcpu_dirty = 1;

     mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
     if (mmap_size < 0) {

Will queue this for uq/master and post a corresponding qemu-kvm patch.
Thanks for pointing out!

Jan

signature.asc
Description: OpenPGP digital signature