2010/6/9 Blue Swirl <blauwir...@gmail.com>: > 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.
Then I'm fine with your patch. -- Regards, Artyom Tarasenko solaris/sparc under qemu blog: http://tyom.blogspot.com/