On Fri, Jun 4, 2010 at 8:30 PM, Artyom Tarasenko <atar4q...@googlemail.com> wrote: > 2010/6/4 Blue Swirl <blauwir...@gmail.com>: >> On Tue, Jun 1, 2010 at 8:16 PM, Artyom Tarasenko >> <atar4q...@googlemail.com> wrote: >>> 2010/6/1 Blue Swirl <blauwir...@gmail.com>: >>>> On Tue, Jun 1, 2010 at 7:56 PM, Artyom Tarasenko >>>> <atar4q...@googlemail.com> wrote: >>>>> 2010/6/1 Blue Swirl <blauwir...@gmail.com>: >>>>>> On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko >>>>>> <atar4q...@googlemail.com> 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.
What ESP does with its IRQ line does not stop DMA from using its line.