[RFC PATCH] kvmtool: remove 8250 IRQ line reset on device_init

[RFC PATCH] kvmtool: remove 8250 IRQ line reset on device_init

[Andre Przywara]

Currently we reset the KVM interrupt line on initializing the 8250
serial device emulation.
For ARM this creates a problem where we use the in-kernel IRQ chip
before having fully initialized it. But with the new kernel interface
we cannot finish the GIC initialization before we know the number of
used IRQs, so we have to wait until all devices have been created and
initialized.
Since the in-kernel GIC emulation resets the IRQ line anyway and also
QEMU gets away without resetting it, the easiest solution is to drop
the IRQ line reset.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
Hi Pekka,

this patch is an easy fix for our problems with Marc's kvmtool tree
and 3.19-rc (setting number of IRQs after the GIC has been used
already). I see that this basically reverts:

> commit 2ca9e37193ca5f5df5726e0061dc749829295435
> Author: Pekka Enberg <penberg@kernel.org>
> Date:   Sun Jan 23 11:49:39 2011 +0200
> 
>     kvm,8250: Fix device initial state
>     
>     This patch fixes 8250 device initial state for registers and IRQ based
>     on what Qemu does.

Do you (or does anyone) know of the issue that this patch fixed?
This is four years old and from what I see QEMU does no longer the
mentioned IRQ reset(?).
Reworking kvmtool to avoid this issue sound rather painful.

Cheers,
Andre.

 tools/kvm/hw/serial.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c
index 270e6182..2f19ba8 100644
--- a/tools/kvm/hw/serial.c
+++ b/tools/kvm/hw/serial.c
@@ -406,7 +406,6 @@ static int serial8250__device_init(struct kvm *kvm, struct serial8250_device *de
 
 	ioport__map_irq(&dev->irq);
 	r = ioport__register(kvm, dev->iobase, &serial8250_ops, 8, dev);
-	kvm__irq_line(kvm, dev->irq, 0);
 
 	return r;
 }
-- 
1.7.9.5

Re: [RFC PATCH] kvmtool: remove 8250 IRQ line reset on device_init

[Will Deacon]

On Fri, Jan 09, 2015 at 03:58:19PM +0000, Andre Przywara wrote:
> Currently we reset the KVM interrupt line on initializing the 8250
> serial device emulation.
> For ARM this creates a problem where we use the in-kernel IRQ chip
> before having fully initialized it. But with the new kernel interface
> we cannot finish the GIC initialization before we know the number of
> used IRQs, so we have to wait until all devices have been created and
> initialized.
> Since the in-kernel GIC emulation resets the IRQ line anyway and also
> QEMU gets away without resetting it, the easiest solution is to drop
> the IRQ line reset.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> Hi Pekka,
> 
> this patch is an easy fix for our problems with Marc's kvmtool tree
> and 3.19-rc (setting number of IRQs after the GIC has been used
> already). I see that this basically reverts:
> 
> > commit 2ca9e37193ca5f5df5726e0061dc749829295435
> > Author: Pekka Enberg <penberg@kernel.org>
> > Date:   Sun Jan 23 11:49:39 2011 +0200
> > 
> >     kvm,8250: Fix device initial state
> >     
> >     This patch fixes 8250 device initial state for registers and IRQ based
> >     on what Qemu does.
> 
> Do you (or does anyone) know of the issue that this patch fixed?
> This is four years old and from what I see QEMU does no longer the
> mentioned IRQ reset(?).
> Reworking kvmtool to avoid this issue sound rather painful.

I'm fine with this from an ARM point-of-view.

  Acked-by: Will Deacon <will.deacon@arm.com>

Have you tested this on x86?

Will


>  tools/kvm/hw/serial.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c
> index 270e6182..2f19ba8 100644
> --- a/tools/kvm/hw/serial.c
> +++ b/tools/kvm/hw/serial.c
> @@ -406,7 +406,6 @@ static int serial8250__device_init(struct kvm *kvm, struct serial8250_device *de
>  
>  	ioport__map_irq(&dev->irq);
>  	r = ioport__register(kvm, dev->iobase, &serial8250_ops, 8, dev);
> -	kvm__irq_line(kvm, dev->irq, 0);
>  
>  	return r;
>  }
> -- 
> 1.7.9.5
> 

Re: [RFC PATCH] kvmtool: remove 8250 IRQ line reset on device_init

[Andre Przywara]

On 12/01/15 17:41, Will Deacon wrote:
> On Fri, Jan 09, 2015 at 03:58:19PM +0000, Andre Przywara wrote:
>> Currently we reset the KVM interrupt line on initializing the 8250
>> serial device emulation.
>> For ARM this creates a problem where we use the in-kernel IRQ chip
>> before having fully initialized it. But with the new kernel interface
>> we cannot finish the GIC initialization before we know the number of
>> used IRQs, so we have to wait until all devices have been created and
>> initialized.
>> Since the in-kernel GIC emulation resets the IRQ line anyway and also
>> QEMU gets away without resetting it, the easiest solution is to drop
>> the IRQ line reset.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>> Hi Pekka,
>>
>> this patch is an easy fix for our problems with Marc's kvmtool tree
>> and 3.19-rc (setting number of IRQs after the GIC has been used
>> already). I see that this basically reverts:
>>
>>> commit 2ca9e37193ca5f5df5726e0061dc749829295435
>>> Author: Pekka Enberg <penberg@kernel.org>
>>> Date:   Sun Jan 23 11:49:39 2011 +0200
>>>
>>>     kvm,8250: Fix device initial state
>>>     
>>>     This patch fixes 8250 device initial state for registers and IRQ based
>>>     on what Qemu does.
>>
>> Do you (or does anyone) know of the issue that this patch fixed?
>> This is four years old and from what I see QEMU does no longer the
>> mentioned IRQ reset(?).
>> Reworking kvmtool to avoid this issue sound rather painful.
> 
> I'm fine with this from an ARM point-of-view.
> 
>   Acked-by: Will Deacon <will.deacon@arm.com>
> 
> Have you tested this on x86?

Yes, that worked as well. Booting into an initrd, flooding the console
with output: no issues.

Cheers,
Andre.

> 
>>  tools/kvm/hw/serial.c |    1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/tools/kvm/hw/serial.c b/tools/kvm/hw/serial.c
>> index 270e6182..2f19ba8 100644
>> --- a/tools/kvm/hw/serial.c
>> +++ b/tools/kvm/hw/serial.c
>> @@ -406,7 +406,6 @@ static int serial8250__device_init(struct kvm *kvm, struct serial8250_device *de
>>  
>>  	ioport__map_irq(&dev->irq);
>>  	r = ioport__register(kvm, dev->iobase, &serial8250_ops, 8, dev);
>> -	kvm__irq_line(kvm, dev->irq, 0);
>>  
>>  	return r;
>>  }
>> -- 
>> 1.7.9.5
>>
>