Re: [Qemu-devel] Re: [Bug 599958] Re: Timedrift problems with Win7: hpet missing time drift fixups

[Blue Swirl]

On Mon, Jul 5, 2010 at 1:47 PM, Avi Kivity <address@hidden> wrote:
> On 07/05/2010 04:28 PM, Jan Kiszka wrote:
>>
>> Avi Kivity wrote:
>>
>>>
>>> On 07/05/2010 03:16 PM, Jan Kiszka wrote:
>>>
>>>>>
>>>>> It's also architecturally cleaner.  Masks and acks are architectural
>>>>> events.  Injections are not - there's the edge on the LINT0 or INTI2
>>>>> pins, generation of an APIC message, receipt of the APIC message, and
>>>>> assertion of the APIC-to-core interrupt interface.  I'm not sure how
>>>>> the
>>>>> proposed interface maps to that.
>>>>>
>>>>>
>>>>
>>>> Our emulation does not reflect every architectural detail of the
>>>> delivery path anyway.
>>>>
>>>
>>> Usually, when that happens, we get an obscure bug.
>>>
>>> So if we add a facility, especially across the user/kernel boundary,
>>> it's better to have it conform to the architecture.  That reduces the
>>> chance it has a serious hidden bug.
>>>
>>
>> Neither ack/mask notifiers (past the IRQ controller) nor injection
>> return values are part of any architecture we emulate.
>
> Right.  But placing a tap on something that exists architectually means it's
> likely to survive implementation changes.
>
>>  What is driving
>> us are the requirements of the de-coalescing workarounds we want to
>> build on top and the impact on existing design.
>>
>
> In the case of qemu<->kvm interfaces, also the longevity of these
> interfaces.
>
> Note I'm not advocating a mask/ack solution.  It's pretty complicated and
> I'm not sure the benefits outweigh the complexity.  But I want us to examine
> all options, especially as I don't like delivery checks very much.

Since you seem to have gone back to drawing board, how about the
following design:

Explicit qemu_irqs and muxes for the backwards channel

Each participating device has a set of GPIOs (implemented with
qemu_irq) for any backwards signals: delivered, coalesced, EOI'd,
masked (short D/C/E/M). For each incoming IRQ line, there would be
four backwards GPIOs (D/C/E/M).

When a forward IRQ signal is propagated, any intermediate devices turn
the internal 'mux' dial to point to the originating device GPIO.

When a backwards signal (one of D/C/E/M) must be delivered, the mux
forwards the D/C/E/M signal in question towards the last IRQ
originator as indicated by the dial.

I think this design would cover the coalescing needs, it would not
change qemu_irq design and it should be expandable. Drawbacks would be
that a lot of signals would be needed and all these signal's routes
should be set up by the board level.

>
>>>> The abstraction is always an IRQ line which can be
>>>> high or low (sometimes it is only high, but this is a bug).
>>>>
>>>>
>>>
>>> That's a bug in the use of qemu_irq, not qemu_irq itself.
>>>
>>> But qemu_irq needs to remember its state, otherwise when an irq
>>> controller unmasks a level-triggered line, it won't see the interrupt.
>>>
>>
>> I think it's currently the IRQ controller's job to keep track of the
>> line state during masked periods. Moving this to qemu_irq is definitely
>> better but requires some care (e.g. when vmstates are involved).
>>
>
> If everything is done correctly no new vmstates are needed.  The irq
> producer, when its state is reloaded, will output exactly the same value as
> when the state was saved.
>
> The irq line qemu_irq models doesn't have a flip-flop, so it doesn't have
> state.  Similarly, the irq inputs on the irq controllers don't have flip
> flops attached (some do), so ioapic->irr for example shouldn't be state.
>
> (some care is needed on load to avoid spurious edges)
>
> --
> error compiling committee.c: too many arguments to function
>
>