[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [ PATCH 2/2] xen: enable Virtual-interrupt delivery
Jan Beulich wrote on 2012-08-31: >>>> On 31.08.12 at 11:30, "Li, Jiongxi" <jiongxi.li@xxxxxxxxx> wrote: >> +/* >> + * When "Virtual Interrupt Delivery" is enabled, this function is used >> + * to handle EOI-induced VM exit >> + */ >> +void vlapic_handle_EOI_induced_exit(struct vlapic *vlapic, int vector) >> +{ >> + ASSERT(cpu_has_vmx_virtual_intr_delivery); >> + >> + if ( vlapic_test_and_clear_vector(vector, > &vlapic->regs->data[APIC_TMR]) ) > > Why test_and_clear rather than just test? The current logic requires to clear it when guest writes EOI. When VCPU received an interrupt, it set the TMR if it is a level trigger interrupt and do nothing if it is edge triggered(it assumes the corresponding bit in TMR is clear). >> + { >> + vioapic_update_EOI(vlapic_domain(vlapic), vector); >> + } >> + >> + hvm_dpci_msi_eoi(current->domain, vector); >> +} >> ... >> --- a/xen/arch/x86/hvm/vmx/intr.c Fri Aug 31 09:30:38 2012 +0800 >> +++ b/xen/arch/x86/hvm/vmx/intr.c Fri Aug 31 09:49:39 2012 +0800 >> @@ -227,19 +227,43 @@ void vmx_intr_assist(void) >> goto out; >> intblk = hvm_interrupt_blocked(v, intack); >> - if ( intblk == hvm_intblk_tpr ) >> + if ( cpu_has_vmx_virtual_intr_delivery ) >> { >> - ASSERT(vlapic_enabled(vcpu_vlapic(v))); - >> ASSERT(intack.source == hvm_intsrc_lapic); - tpr_threshold = >> intack.vector >> 4; - goto out; + /* Set >> "Interrupt-window exiting" for ExtINT */ + if ( (intblk != >> hvm_intblk_none) && + ( (intack.source == >> hvm_intsrc_pic) || + ( intack.source == >> hvm_intsrc_vector) ) ) + { + >> enable_intr_window(v, intack); + goto out; + >> } + + if ( __vmread(VM_ENTRY_INTR_INFO) & >> INTR_INFO_VALID_MASK ) + { + if ( >> (intack.source == hvm_intsrc_pic) || + >> (intack.source == hvm_intsrc_nmi) || + >> (intack.source == hvm_intsrc_mce) ) + >> enable_intr_window(v, intack); + + goto out; + >> } >> } >> + else >> + { >> + if ( intblk == hvm_intblk_tpr ) >> + { >> + ASSERT(vlapic_enabled(vcpu_vlapic(v))); >> + ASSERT(intack.source == hvm_intsrc_lapic); >> + tpr_threshold = intack.vector >> 4; >> + goto out; >> + } >> >> - if ( (intblk != hvm_intblk_none) || - >> (__vmread(VM_ENTRY_INTR_INFO) & INTR_INFO_VALID_MASK) ) - { - >> enable_intr_window(v, intack); - goto out; + >> if ( (intblk != hvm_intblk_none) || + >> (__vmread(VM_ENTRY_INTR_INFO) & INTR_INFO_VALID_MASK) ) + { >> + enable_intr_window(v, intack); + goto >> out; + } >> } > > If you made the above and if()/else if() series, some of the > differences would go away, making the changes easier to > review. > >> >> intack = hvm_vcpu_ack_pending_irq(v, intack); >> @@ -253,6 +277,29 @@ void vmx_intr_assist(void) >> { >> hvm_inject_hw_exception(TRAP_machine_check, > HVM_DELIVER_NO_ERROR_CODE); >> } >> + else if ( cpu_has_vmx_virtual_intr_delivery && >> + intack.source != hvm_intsrc_pic && >> + intack.source != hvm_intsrc_vector ) >> + { >> + /* we need update the RVI field */ >> + unsigned long status = __vmread(GUEST_INTR_STATUS); >> + status &= ~(unsigned long)0x0FF; >> + status |= (unsigned long)0x0FF & >> + intack.vector; >> + __vmwrite(GUEST_INTR_STATUS, status); >> + if (v->arch.hvm_vmx.eoi_exitmap_changed) { >> +#define UPDATE_EOI_EXITMAP(v, e) { \ >> + if (test_and_clear_bit(e, &(v).eoi_exitmap_changed)) { \ > > Here and elsewhere - do you really need locked accesses? > >> + __vmwrite(EOI_EXIT_BITMAP##e, >> (v).eoi_exit_bitmap[e]);}} + + >> UPDATE_EOI_EXITMAP(v->arch.hvm_vmx, 0); > > This is not very logical: Passing just 'v' to the macro, and accessing > the full field there would be more consistent. > > Furthermore, here and in other places you fail to write the upper halves > for 32-bit, yet you also don't disable the code for 32-bit afaics. > >> + UPDATE_EOI_EXITMAP(v->arch.hvm_vmx, 1); >> + UPDATE_EOI_EXITMAP(v->arch.hvm_vmx, 2); >> + UPDATE_EOI_EXITMAP(v->arch.hvm_vmx, 3); >> + } >> + >> + pt_intr_post(v, intack); >> + } >> else >> { >> HVMTRACE_2D(INJ_VIRQ, intack.vector, /*fake=*/ 0); > > Jan > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel Best regards, Yang _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |