Re: [PATCH 0/7] Rework irq injection infrastructure

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Dong, Eddie wrote:
>>> Mmm, how can you know if it is injected successfully?
>>> From the patch, it looks like you know this by checking
>>> IDT_Vectoring in next VM Exit. That means the virtual 
>>> interrupt controller state in memory is incorrect temply.
>>>
>>> If the injection success & we can get VM Exit before 
>>> next access to those virtual interrupt controller state,
>>> that will be OK, 
>>>       
>> That's the idea.
>>
>>     
>>> but that means we eliminate future HW
>>> optimization opportunity.  I think we just pay too much
>>> for coding style reason to narrow HW optimization space.
>>>   
>>>       
>> I don't know enough about future hardware...
>>     
>
> One of the optimization, which can be used in pure SW is that
> we can shadow APIC state as "RO" to guest so that all guest
> read to APIC registers can run in full speed. T
>
>   

This can still be done with unacked interrupts: modify the state to "as 
if" the interrupt was injected, but commit it only if the injection 
succeeds.  If injection fails, revert the change.  It isn't very nice 
though.

Another possibility is:

   get_interrupt()
    ... do all sleepy things prior to injection
    enter critical section
    ack interrupt
    enter guest

> This optimization is probably cleaner & less-intrusive. Dynamic
> patching to modify guest code could trigger mine sooner or later:(
>
>   

Dynamic patching is dangerous, but the only option to run Windows SMP on 
millions of machines out there.

>   
>> But the motivation here is not coding style, it's to be able to do the
>> injection non-atomically. The tpr-opt patchset wants to write 
>> the tpr to
>> memory, and that is a sleeping operation. Similarly, if we emulate big
>> real mode we need to simulate interrupt injection by writing 
>> to the stack.
>>     
>
> OK. For  big real mode has issue, that is very easy
>  to walk around, we can probably pre-load (pin) those 2 or 3 pages
> before 
> injecting real mode irq.
>   

Pinning has problems of its own.  It can be made to work though.  In any 
case, it can fail, so we need to ack the interrupt only after we have 
successfully pinned the page.

> For dynamic patch stuff, what is the issue? I saw your previous code
> works, right?
>
> BTW, w/o this patch, your previous TPR optimization patch can work,
> w/ this patch, it won't work. The key difference is when guest access
> TPR,
> per original patch, it will dynamically calcuate the vPPR value base on
> vISR & vTPR. Since vISR is incorrect now (though temply), the patch
> won't work correctly.
>
>   

Sure, it needs to be adjusted.

>> Moving the logic to common code helps, but it could have been done with
>> the original system as well.
>>     
>
> Yes, probably we can split this 2 efforts. The part to make it common
> can be 
> easily in first.
>
>   
>>> If the injection fails, the previous logic will inject it back 
>>> immediately; current logic will do normal irq inject path.
>>> The potential differences are:
>>> 	1: If a new higher interrupt arrives, we inject new vector.
>>> that is probably OK.
>>> 	2: If the fault VM Exit is due to nested exception, the
>>> previous logic will probably inject double fault with the original
>>> interrupt consumed (acked); the new logic will probably inject
>>> double fault too, but keep the IRQ not consumed. I don't have
>>> direct case, but worry about those kind of deviation from native
>>> behavor.
>>>   
>>>       
>> We can probably fix it to ack the interrupt when injecting a 
>> double fault.
>>     
>
> Yes, could be though not very clean. But I am not sure how many 
> other reason will cause interrupt injection failed beside shadow fault.
> We need to check all of them.
>   

Injection can fail (or rather, not happen at all) if we do sleepy 
operations and then get a signal.  In that case, it is better to leave 
the interrupt unacked.

What do you think about the first six patches?  I think we can merge 
them now, and continue discussion about the last, which actually splits 
the injection.  I'll think further about acking the interrupt before 
entry, but after possibly sleepy operations have been done.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4