Re: [PATCH] Add irqdevice interface + userint implementation

Gregory Haskins
2007-04-10T11:36:16Z

>>> On Tue, Apr 10, 2007 at 3:52 AM, in message <461B4241.5030101-atKUWr5tajBWk0Htik3J/w@public.gmane.org>,
Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:
>
> Hmm. The current code is synchronous in nature (the vcpu is never
> executing while we raise an interrupt, so the INTA is never needed, as
> we can ensure the cpu can process the interrupt when we inject it. With
> smp/paravirt/whatever (I realize now), this is no longer true.

Exactly. This is in prep for the SMP/PV stuff. I anticipate that the vcpu INTR will effectively be a no-op until then. Eventually we can add the logic to host-IPI the vcpu if its running, but this state is an impossibility today due to the synchronous nature as you have pointed out.

> The NIC raises an interrupt, we have to interrupt the guest to see if its
> current IF/cr8 permit interrupt injection, and if not, we have to keep
> the interrupt in the irqdevice and request an exit when IF/cr8 permit
> injection.

Exactly. The INTR really just serves to kick the cpu into a VMEXIT so we can evaluate the IF/TPR. In many cases this may reveal that the CPU might not be able to take the interrupt right then and there, so it will remain queued by the irqdevice model until it can. If the interrupt cannot be dispatched, we would turn on any relevant EXIT feature (e.g., interrupt-window-exiting) if we dont do this already (I believe this is already in there)

>
> This means that - >pending() and - >read_vector() have to be called in a
> critical section, otherwise the pending interrupt might be change
> between the first and second call, resulting in an injected interrupt to
> software that is not ready to accept it. If we replace read_vector by
> - >ack(this, int vector) we avoid this need (and also save a pointless
> recalculation of the vector).

I am a little confused as to what you are proposing. The current design has pending only returning a boolean if there is something pending, and read_vector actually does the INTA cycle (i.e., returns the vector # and marks it accepted). I can certainly see how a lack of critical section between pending and read_vector could allow a new interrupt to be injected, but I am not seeing why this is a problem. If a new higher-priority interrupt comes in between the pending and read_vector, the new one will be returned, but I think this is desirable?

And if we replace read_vector() with ack(), how do we learn of the vector in the first place? I am guessing that you are thinking that both pending and read_vector return a vector and thus the confusion. Otherwise, I am missing your point completely :) Could you elaborate?

> Note that the original code did not work as intended. Because of the
> IF/tpr issues, there is at most interrupt queued, and only when it is
> ready to be accepted.

Ack

> With the in- kernel apic that changes of course.

Agreed

Regards,
-Greg