From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MGuo9-0003T0-Oi for qemu-devel@nongnu.org; Wed, 17 Jun 2009 09:04:05 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MGuo5-0003Ph-Fp for qemu-devel@nongnu.org; Wed, 17 Jun 2009 09:04:05 -0400 Received: from [199.232.76.173] (port=50882 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MGuo5-0003PR-6j for qemu-devel@nongnu.org; Wed, 17 Jun 2009 09:04:01 -0400 Received: from mail-fx0-f209.google.com ([209.85.220.209]:59155) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MGuo4-0005ex-7U for qemu-devel@nongnu.org; Wed, 17 Jun 2009 09:04:00 -0400 Received: by fxm5 with SMTP id 5so262518fxm.34 for ; Wed, 17 Jun 2009 06:03:59 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20090617121209.GA19508@redhat.com> References: <20090616124702.GS19508@redhat.com> <5b31733c0906170207u1c553f6by67eb814644f55a10@mail.gmail.com> <20090617094318.GX19508@redhat.com> <5b31733c0906170317m67821bc0o74a656e1b7afef21@mail.gmail.com> <20090617110638.GY19508@redhat.com> <5b31733c0906170436m358c6ed1pdae0306df79a44e4@mail.gmail.com> <20090617121209.GA19508@redhat.com> Date: Wed, 17 Jun 2009 15:03:58 +0200 Message-ID: <5b31733c0906170603k34de010fu1f01e9cda1277c8f@mail.gmail.com> Subject: Re: [Qemu-devel] [PATCH] Register usb-uhci reset function. From: Filip Navara Content-Type: multipart/alternative; boundary=0016364c749bac146e046c8aecd7 List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gleb Natapov Cc: qemu-devel@nongnu.org --0016364c749bac146e046c8aecd7 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On Wed, Jun 17, 2009 at 2:12 PM, Gleb Natapov wrote: [snip] > > > > That's why all the paths where the level in device emulation can change > has > > to be covered, so the information in piix3 stays accurate. What I'm > saying > > is that the reset callback is not one of the points where you can call > > qemu_set_irq since the state of other devices is unknown at that point > (for > > the purpose of simulating interrupt levels). When the reset callbacks are > I understand what you are saying, but the logic that emulates level irqs > from qemu_irq() edge (in piix3) is not a part of device emulation, so it > is not obvious to me that system reset should reset it and bring it to > unknown state. Line state change may reach IRQ chip before or after reset, > but > this doesn't matter since IRQ chip will be in knows state in both cases. > Actually I want to bring it to a known state. It may not be equivalent to what happens in real HW, but it's compatible behavior and deterministic, which makes debugging easier. > done with their work, all the simulated interrupt levels at all layers > have > > to be restored to known state. This state is currently with all the > levels > > set to zero. If you call qemu_set_irq during the reset callbacks the > > interrupt levels recorded in the device tree may end up in uncertain > state. > That may happen on real HW too. OS will get spurious interrupt, that's it. Agreed. That's what I said earlier (or at least what I thought), you would get spurious interrupts. It may have other consequences which I can't think of, right now. [snip] > > To change the level you first have to have the whole device tree topology > in > > known state. In the reset callback that's not the case. If there was a > Level change (as seen by other device) can happen only after or before > reset, not during reset. In both cases the device state is known. Hmm, makes sense. I suppose that if none of the bus/PIC/etc. devices altered their "interrupt level" state during the reset and each individual device brought it's IRQ line down it would work too. As long as the interrupts are communicated to the CPU after the reset in a correct way. I'm just not convinced that it's the right way. My rationale is that on power-up the interrupt levels in PIC/PCI/etc. are set to zero and they only change as the emulation is executed. So why should the reset be different? Doing it in a different way on reset sounds fishy to me, especially since it involves more code for no obvious benefit. Moreover APIC and lot of other devices clear the interrupt state now, so you'd have to change them too. [snip] > > I'm suggesting not to call it at all and always reset the recorded > > "interrupt levels" all the way down the device tree on reset. If there is > a > By doing this you are setting device irq line without asking device. It > will work since assumption that irq level is zero after reset is usually > correct. Yes, but the emulation is not running at that point, so it's harmless. > > device which sets the IRQ line high on reset then we should introduce the > > "late" reset/init callback. > > > Yes, calling set_irq(1) on reset may not achieve this since irq chip may > be reset after set_irq() was called. Right, exactly my point. [snip] > > If it's not clear then let's make it clear. Real HW communicates > interrupt > > levels and detects edges, QEMU communicates edges and simulates levels, > so > > it has to work differently than on real HW in this particular case. > > > > The system reset callback would reset device registers to a known state > as > > specified by the HW documentation. The interrupt levels would be driven > low > > at this point on the devices which simulate "interrupt levels" (buses, > PIC, > > etc.) - as if "0" was sent to the copper wire ;) > > > > The "late" reset/init callback would drive the IRQs (and GPIOs) high if > > necessary once the system is in a known state, ie. after all reset > callbacks > > have successfully completed. > > > Basically you want to simulate "dead" period when devices ignore their > inputs for > some time after reset (like on real HW). Sort of, but in a deterministic way. > > Hot-unplug callback doesn't need reset the registers of the device, it > only > > has to drive the IRQ low on the bus - ie. call *_set_irq(0) for the > specific > > bus. In theory that could be done at the bus level. > > > I should be done at bus level indeed. The device is disconnected at that > point. Agreed. > > Feel free to disagree or correct me. Suggestions welcome. > > > Not disagreeing but this still is not enough, for instance there are > devices that have different state after power-up and reset. Let's document it once we (including the maintainers) agree on something. At least that would set the precedent for future. [snip] > > > > I don't remember the exact contents of 2/3, sorry. > > > http://lists.gnu.org/archive/html/qemu-devel/2009-06/msg00342.html > Looks good to me. Best regards, Filip Navara --0016364c749bac146e046c8aecd7 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
On Wed, Jun 17, 2009 at 2:12 PM, Gleb Natapov <gleb@redhat.com&= gt; wrote:
[snip]
>
> That's why all the paths where the level in device emulation can c= hange has
> to be covered, so the information in piix3 stays accurate. What I'= m saying
> is that the reset callback is not one of the points where you can call=
> qemu_set_irq since the state of other devices is unknown at that point= (for
> the purpose of simulating interrupt levels). When the reset callbacks = are
I understand what you are saying, but the logic that emulates l= evel irqs
from qemu_irq() edge (in piix3) is not a part of device emulation, so it is not obvious to me that system reset should reset it and bring it to
unknown state. Line state change may reach IRQ chip before or after reset, = but
this doesn't matter since IRQ chip will be in knows state in both cases= .

Actually I want to= bring it to a known state. It may not be equivalent to what happens in rea= l HW, but it's compatible behavior and deterministic, which makes debug= ging easier.=A0

> done = with their work, all the simulated interrupt levels at all layers have
> to be restored to known state. This state is currently with all the le= vels
> set to zero. If you call qemu_set_irq during the reset callbacks the > interrupt levels recorded in the device tree may end up in uncertain s= tate.
That may happen on real HW too. OS will get spurious interrupt, that&= #39;s it.

Agreed.

= That's what I said earlier (or at least what I thought), you would get = spurious interrupts. It may have other consequences which I can't think= of, right now.=A0
=A0
[snip]
> To change the level you first have to have the whole device tree topol= ogy in
> known state. In the reset callback that's not the case. If there w= as a
Level change (as seen by other device) can happen only after or befor= e
reset, not during reset. In both cases the device state is known.

Hmm, makes sense. I suppose that if none of the bus/= PIC/etc. devices altered their "interrupt level" state during the= reset and each individual device brought it's IRQ line down it would w= ork too. As long as the interrupts are communicated to the CPU after the re= set in a correct way. I'm just not convinced that it's the right wa= y. My rationale is that on power-up the interrupt levels in PIC/PCI/etc. ar= e set to zero and they only change as the emulation is executed. So why sho= uld the reset be different? Doing it in a different way on reset sounds fis= hy to me, especially since it involves more code for no obvious benefit. Mo= reover APIC and lot of other devices clear the interrupt state now, so you&= #39;d have to change them too.
=A0
[snip]
> I'm suggesting not to call it at all and always reset t= he recorded
> "interrupt levels" all the way down the device tree on reset= . If there is a
By doing this you are setting device irq line without asking de= vice. It
will work since assumption that irq level is zero after reset is usually correct.

Yes, but the emulation is not runn= ing at that point, so it's harmless.
=A0
> device which sets the IRQ line high on reset then we= should introduce the
> "late" reset/init callback.
>
Yes, calling set_irq(1) on reset may not achieve this since irq chip = may
be reset after set_irq() was called.

Right,= exactly my point.
=A0
[snip]
> If it's not clear then let's make it cl= ear. Real HW communicates interrupt
> levels and detects edges, QEMU communicates edges and simulates levels= , so
> it has to work differently than on real HW in this particular case. >
> The system reset callback would reset device registers to a known stat= e as
> specified by the HW documentation. The interrupt levels would be drive= n low
> at this point on the devices which simulate "interrupt levels&quo= t; (buses, PIC,
> etc.) - as if "0" was sent to the copper wire ;)
>
> The "late" reset/init callback would drive the IRQs (and GPI= Os) high if
> necessary once the system is in a known state, ie. after all reset cal= lbacks
> have successfully completed.
>
Basically you want to simulate "dead" period when dev= ices ignore their inputs for
some time after reset (like on real HW).

So= rt of, but in a deterministic way.
=A0
> Hot-unplug callback doesn't need reset the regis= ters of the device, it only
> has to drive the IRQ low on the bus - ie. call *_set_irq(0) for the sp= ecific
> bus. In theory that could be done at the bus level.
>
I should be done at bus level indeed. The device is disconnected at t= hat
point.

Agreed.
=A0
> Feel free to disagree or corr= ect me. Suggestions welcome.
>
Not disagreeing but this still is not enough, for instance there are<= br> devices that have different state after power-up and reset.

Let's document it once we (including the maintainers) = agree on something. At least that would set the precedent for future.

[snip]=A0
>
> I don't remember the exact contents of 2/3, sorry.
>
http://lists.gnu.org/archive/html/qemu-devel/20= 09-06/msg00342.html

Looks good to me.

Best regards,
Filip Navara
--0016364c749bac146e046c8aecd7--