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

[Blue Swirl]

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.