On 2013-02-17 17:26, Gleb Natapov wrote: > On Sun, Feb 17, 2013 at 04:31:26PM +0100, Jan Kiszka wrote: >> On 2013-02-17 16:07, Gleb Natapov wrote: >>> On Sat, Feb 16, 2013 at 06:10:14PM +0100, Jan Kiszka wrote: >>>> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> >>>> >>>> If L1 does not set PIN_BASED_EXT_INTR_MASK, we incorrectly skipped >>>> vmx_complete_interrupts on L2 exits. This is required because, with >>>> direct interrupt injection from L0 to L2, L0 has to update its pending >>>> events. >>>> >>>> Also, we need to allow vmx_cancel_injection when entering L2 in we left >>>> to L0. This condition is indirectly derived from the absence of valid >>>> vectoring info in vmcs12. We no explicitly clear it if we find out that >>>> the L2 exit is not targeting L1 but L0. >>>> >>> We really need to overhaul how interrupt injection is emulated in nested >>> VMX. Why not put pending events into event queue instead of >>> get_vmcs12(vcpu)->idt_vectoring_info_field and inject them in usual way. >> >> I was thinking about the same step but felt unsure so far if >> vmx_complete_interrupts & Co. do not include any assumptions about the >> vmcs configuration that won't match what L1 does. So I went for a >> different path first, specifically to avoid impact on these hairy bits >> for non-nested mode. >> > Assumption made by those functions should be still correct since guest > VMCS configuration is not applied directly to real HW, but we should be > careful of course. For instance interrupt queues should be cleared > during nested vmexit and event transfered back to idt_vectoring_info_field. > IIRC this is how nested SVM works BTW. Checking __vmx_complete_interrupts, the first issue I find is that type 5 (privileged software exception) is not decoded, thus will be lost if L2 leaves this way. That's a reason why it might be better to re-inject the content of vmcs12 if it is valid. VMX is a bit more hairy than SVM, I guess. > > And with you patch you did a half of the job already :) When exiting to > L0 you transfer event information from get_vmcs12(vcpu)->idt_vectoring_info_field > to our internal event queues anyway. Hmm, but you do not clear the queue > during nested vmexit. So what happens if L2 exits to L0 with an exception > in idt_vectoring_info_field. Now interrupt is delivered so nested vm exit > is done, but exception is left in internal queue. I think it will be > delivered into L1 during next vmentry. Indeed. The queue is only cleared on L2->L0 exits (via the late vmx_complete_interrupts). It should be cleared on L2->L1 exists as well. Will fix. Jan
Attachment:
signature.asc
Description: OpenPGP digital signature