Re: [Qemu-devel] [PATCH 11/22] i8259: Update IRQ state after reset

[Jan Kiszka <jan.kiszka@web.de>]

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

On 2011-09-29 21:45, Blue Swirl wrote:
> On Wed, Sep 28, 2011 at 9:18 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2011-09-28 20:01, Blue Swirl wrote:
>>>
>>> On Wed, Sep 28, 2011 at 11:00 AM, Jan Kiszka<jan.kiszka@siemens.com>
>>>  wrote:
>>>>
>>>> As we clearly modify the PIC state on pic_reset, we also have to update
>>>> the IRQ output. This only happened on init so far. Apply this
>>>> consistently.
>>>
>>> Nack, IRQ lines shouldn't be touched on reset. The other side may not
>>> be ready for receiving the interrupt change and qemu_irqs are
>>> stateless anyway.
>>
>> Sorry, but failing to clear the line (this is what pic_update_irq will
>> effectively do) is a clear bug in the current code. This patch is 100%
>> analogue to what, e.g. the PCI layer does on reset. Please re-read.
> 
> Reset will happen also when the devices are created. At that time,
> qemu_irq callback triggered by changing of the state may produce
> undesired effects on the other side.

All those potential effects will be cleared again when the receiver is
reset as well. If not, that would be a bug which requires fixing.

> There have been bugs earlier, see
> bc26e55a6615dc594be425d293db40d5cdcdb84b and
> 42f1ced228c9b616cfa2b69846025271618e4ef5 and discussion in
> http://lists.nongnu.org/archive/html/qemu-devel/2009-06/msg01024.html.

Deasserting IRQs at PCI device level is indeed useless as we already do
this in the PCI core.

There is no difference between system reset after power-up or later on.
And there should be no difference between per device, per group of
devices or system-wide reset. A device model cannot tell these apart.
That the devices reset handler is only called on system reset is an odd
and fragile assumption - not that fragile for platform devices like the
i8259, but definitely bogus for pluggable ones.

My series does not depend on this cleanup/fix, but the reason not to
apply remains wrong IMHO.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]