[Qemu-devel] Re: [PATCH] sparc32 esp fix spurious interrupts in chip reset

[Artyom Tarasenko]

2010/6/4 Blue Swirl <address@hidden>:
> On Tue, Jun 1, 2010 at 8:16 PM, Artyom Tarasenko
> <address@hidden> wrote:
>> 2010/6/1 Blue Swirl <address@hidden>:
>>> On Tue, Jun 1, 2010 at 7:56 PM, Artyom Tarasenko
>>> <address@hidden> wrote:
>>>> 2010/6/1 Blue Swirl <address@hidden>:
>>>>> On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko
>>>>> <address@hidden> wrote:
>>>>>> lower interrupt during chip reset. Otherwise the ESP_RSTAT register
>>>>>> may get out of sync with the IRQ line status. This effect became
>>>>>> visible after commit 65899fe3
>>>>>
>>>>> Hard reset handlers should not touch qemu_irqs, because on cold start,
>>>>> the receiving end may be unprepared to handle the signal.
>>>>
>>>> Wouldn't the real hardware lower irq on the hardware reset?
>>>
>>> Yes, but since qemu_irqs have no state, and on a cold start or system
>>> reset all other devices are guaranteed to be reset, the callback would
>>> be useless.
>>>
>>>> And if it would not, would it still clear the corresponding bit in
>>>> the ESP_RSTAT register?
>>>
>>> All registers are set to zero in the lines below.
>>>
>>>>
>>>>> See
>>>>> 0d0a7e69e853639b123798877e019c3c7ee6634a,
>>>>> bc26e55a6615dc594be425d293db40d5cdcdb84b and
>>>>> 42f1ced228c9b616cfa2b69846025271618e4ef5.
>>>>>
>>>>> For ESP there are two other sources of reset: signal from DMA and chip
>>>>> reset command. On those cases, lowering IRQ makes sense.
>>>>>
>>>>> So the correct fix is to refactor the reset handling a bit. Does this
>>>>> patch also fix your test case?
>>>>
>>>> It does, but
>>>>
>>>> +static void esp_soft_reset(DeviceState *d)
>>>> +{
>>>> +    ESPState *s = container_of(d, ESPState, busdev.qdev);
>>>> +
>>>> +    qemu_irq_lower(s->irq);
>>>>
>>>> Shouldn't it be esp_lower_irq(s)? What's going to happen to the
>>>> DMA_INTR bit if dma was the source of the irq?
>>>
>>> Again, the registers are zeroed in esp_hard_reset().
>>
>> How does it zero the _DMA_ registers? And sparc32_dma does share the
>> IRQ line with ESP, doesn't it?
>
> I'd suppose DMA registers are separate and they would not be cleared
> by for example ESP chip reset command. The IRQ goes from ESP to DMA,
> DMA has another line going to interrupt controller.

But do we have separate DMA lines in qemu? If we do, I'm absolutely fine with
qemu_irq_lower(s->irq) . If we don't, imagine the following scenario: DMA
rises an IRQ, then esp chip reset happens, and then... DMA can't rise
the IRQ anymore.


-- 
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/