| Message ID | 1358833771-12564-1-git-send-email-yang.z.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 22, 2013 at 01:49:31PM +0800, Yang Zhang wrote: > From: Yang Zhang <yang.z.zhang@Intel.com> > > The "acknowledge interrupt on exit" feature controls processor behavior > for external interrupt acknowledgement. When this control is set, the > processor acknowledges the interrupt controller to acquire the > interrupt vector on VM exit. > > After enabling this feature, an interrupt which arrived when target cpu is > running in vmx non-root mode will be handled by vmx handler instead of handler > in idt. Currently, vmx handler only fakes an interrupt stack and jump to idt > table to let real handler to handle it. Further, we will recognize the interrupt > and only delivery the interrupt which not belong to current vcpu through idt table. > The interrupt which belonged to current vcpu will be handled inside vmx handler. > This will reduce the interrupt handle cost of KVM. > > Refer to Intel SDM volum 3, chapter 33.2. > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > --- > arch/x86/include/asm/kvm_host.h | 2 + > arch/x86/kvm/svm.c | 6 ++++ > arch/x86/kvm/vmx.c | 52 ++++++++++++++++++++++++++++++++++++++- > arch/x86/kvm/x86.c | 2 + > 4 files changed, 61 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index c431b33..0b73602 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -345,6 +345,7 @@ struct kvm_vcpu_arch { > unsigned long cr8; > u32 hflags; > u64 efer; > + struct desc_ptr host_idt; Enough to save only host_idt.address. > u64 apic_base; > struct kvm_lapic *apic; /* kernel irqchip context */ > unsigned long apic_attention; > @@ -723,6 +724,7 @@ struct kvm_x86_ops { > int (*check_intercept)(struct kvm_vcpu *vcpu, > struct x86_instruction_info *info, > enum x86_intercept_stage stage); > + void (*handle_external_intr)(struct kvm_vcpu *vcpu); > }; > > struct kvm_arch_async_pf { > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index d29d3cd..e286600 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -4227,6 +4227,11 @@ out: > return ret; > } > > +static void svm_handle_external_intr(struct kvm_vcpu *vcpu) > +{ > + return; > +} > + > static struct kvm_x86_ops svm_x86_ops = { > .cpu_has_kvm_support = has_svm, > .disabled_by_bios = is_disabled, > @@ -4318,6 +4323,7 @@ static struct kvm_x86_ops svm_x86_ops = { > .set_tdp_cr3 = set_tdp_cr3, > > .check_intercept = svm_check_intercept, > + .handle_external_intr = svm_handle_external_intr, > }; > > static int __init svm_init(void) > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index dd2a85c..ef98392 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2565,7 +2565,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf) > #ifdef CONFIG_X86_64 > min |= VM_EXIT_HOST_ADDR_SPACE_SIZE; > #endif > - opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT; > + opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT | > + VM_EXIT_ACK_INTR_ON_EXIT; > if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS, > &_vmexit_control) < 0) > return -EIO; > @@ -3933,6 +3934,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) > > vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); > set_cr4_guest_host_mask(vmx); > + native_store_idt(&vmx->vcpu.arch.host_idt); > We already call native_store_idt() in vmx_set_constant_host_state(). No need to do it twice. Add vcpu parameter to vmx_set_constant_host_state() to get idt address from there. > return 0; > } > @@ -6096,6 +6098,53 @@ static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx) > } > } > > + > +static void vmx_handle_external_intr(struct kvm_vcpu *vcpu) > +{ > + u32 exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); > + if ((exit_intr_info & (INTR_INFO_VALID_MASK | INTR_INFO_INTR_TYPE_MASK)) > + == (INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR)) { > + unsigned int vector; > + unsigned long entry; > + gate_desc *desc; > + > + vector = exit_intr_info & INTR_INFO_VECTOR_MASK; > +#ifdef CONFIG_X86_64 > + desc = (void *)vcpu->arch.host_idt.address + vector * 16; > +#else > + desc = (void *)vcpu->arch.host_idt.address + vector * 8; > +#endif > + > + entry = gate_offset(*desc); > + asm( > + "mov %0, %%" _ASM_DX " \n\t" > +#ifdef CONFIG_X86_64 > + "mov %%" _ASM_SP ", %%" _ASM_BX " \n\t" > + "and $0xfffffffffffffff0, %%" _ASM_SP " \n\t" > + "mov %%ss, %%" _ASM_AX " \n\t" > + "push %%" _ASM_AX " \n\t" > + "push %%" _ASM_BX " \n\t" > +#endif > + "pushf \n\t" > + "mov %%cs, %%" _ASM_AX " \n\t" > + "push %%" _ASM_AX " \n\t" > + "push intr_return \n\t" > + "jmp *%% " _ASM_DX " \n\t" > + "1: \n\t" > + ".pushsection .rodata \n\t" > + ".global intr_return \n\t" > + "intr_return: " _ASM_PTR " 1b \n\t" > + ".popsection \n\t" > + : :"m"(entry) : > +#ifdef CONFIG_X86_64 > + "rax", "rbx", "rdx" > +#else > + "eax", "ebx", "edx" ebx is not clobbered on 32bit as far as I see. > +#endif > + ); > + } > +} > + > static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx) > { > u32 exit_intr_info; > @@ -7363,6 +7412,7 @@ static struct kvm_x86_ops vmx_x86_ops = { > .set_tdp_cr3 = vmx_set_cr3, > > .check_intercept = vmx_check_intercept, > + .handle_external_intr = vmx_handle_external_intr, > }; > > static int __init vmx_init(void) > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 1c9c834..000140b 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5728,6 +5728,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > native_read_tsc()); > > vcpu->mode = OUTSIDE_GUEST_MODE; > + kvm_x86_ops->handle_external_intr(vcpu); > + Move it after smp_wmb(). Also to be close to how real interrupt is injected we can set IF in eflags copy that we push on a stack in vmx_handle_external_intr() and get rid of local_irq_enable() here (adding comment about it of course). svm_handle_external_intr() will call local_irq_enable(). > smp_wmb(); > local_irq_enable(); > > -- > 1.7.1 -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Gleb Natapov wrote on 2013-01-23: > On Tue, Jan 22, 2013 at 01:49:31PM +0800, Yang Zhang wrote: >> From: Yang Zhang <yang.z.zhang@Intel.com> >> >> The "acknowledge interrupt on exit" feature controls processor behavior >> for external interrupt acknowledgement. When this control is set, the >> processor acknowledges the interrupt controller to acquire the >> interrupt vector on VM exit. >> >> After enabling this feature, an interrupt which arrived when target cpu >> is running in vmx non-root mode will be handled by vmx handler instead >> of handler in idt. Currently, vmx handler only fakes an interrupt stack >> and jump to idt table to let real handler to handle it. Further, we >> will recognize the interrupt and only delivery the interrupt which not >> belong to current vcpu through idt table. The interrupt which belonged >> to current vcpu will be handled inside vmx handler. This will reduce >> the interrupt handle cost of KVM. >> >> Refer to Intel SDM volum 3, chapter 33.2. >> >> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> >> --- >> arch/x86/include/asm/kvm_host.h | 2 + arch/x86/kvm/svm.c >> | 6 ++++ arch/x86/kvm/vmx.c | 52 >> ++++++++++++++++++++++++++++++++++++++- arch/x86/kvm/x86.c >> | 2 + 4 files changed, 61 insertions(+), 1 deletions(-) >> diff --git a/arch/x86/include/asm/kvm_host.h >> b/arch/x86/include/asm/kvm_host.h index c431b33..0b73602 100644 --- >> a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h >> @@ -345,6 +345,7 @@ struct kvm_vcpu_arch { >> unsigned long cr8; >> u32 hflags; >> u64 efer; >> + struct desc_ptr host_idt; > Enough to save only host_idt.address. > >> u64 apic_base; struct kvm_lapic *apic; /* kernel irqchip context >> */ unsigned long apic_attention; @@ -723,6 +724,7 @@ struct >> kvm_x86_ops { int (*check_intercept)(struct kvm_vcpu *vcpu, >> struct x86_instruction_info *info, enum x86_intercept_stage >> stage); + void (*handle_external_intr)(struct kvm_vcpu *vcpu); }; >> >> struct kvm_arch_async_pf { >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index d29d3cd..e286600 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -4227,6 +4227,11 @@ out: >> return ret; >> } >> +static void svm_handle_external_intr(struct kvm_vcpu *vcpu) >> +{ >> + return; >> +} >> + >> static struct kvm_x86_ops svm_x86_ops = { .cpu_has_kvm_support = >> has_svm, .disabled_by_bios = is_disabled, @@ -4318,6 +4323,7 @@ >> static struct kvm_x86_ops svm_x86_ops = { .set_tdp_cr3 = set_tdp_cr3, >> >> .check_intercept = svm_check_intercept, + .handle_external_intr = >> svm_handle_external_intr, }; >> >> static int __init svm_init(void) >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index dd2a85c..ef98392 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2565,7 +2565,8 @@ static __init int setup_vmcs_config(struct > vmcs_config *vmcs_conf) >> #ifdef CONFIG_X86_64 >> min |= VM_EXIT_HOST_ADDR_SPACE_SIZE; >> #endif >> - opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT; >> + opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT | >> + VM_EXIT_ACK_INTR_ON_EXIT; >> if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS, >> &_vmexit_control) < 0) >> return -EIO; >> @@ -3933,6 +3934,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) >> >> vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); >> set_cr4_guest_host_mask(vmx); >> + native_store_idt(&vmx->vcpu.arch.host_idt); >> > We already call native_store_idt() in vmx_set_constant_host_state(). No > need to do it twice. Add vcpu parameter to vmx_set_constant_host_state() > to get idt address from there. Sure. >> return 0; >> } >> @@ -6096,6 +6098,53 @@ static void vmx_complete_atomic_exit(struct > vcpu_vmx *vmx) >> } >> } >> + +static void vmx_handle_external_intr(struct kvm_vcpu *vcpu) +{ + u32 >> exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); + if ((exit_intr_info >> & (INTR_INFO_VALID_MASK | INTR_INFO_INTR_TYPE_MASK)) + == >> (INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR)) { + unsigned int vector; >> + unsigned long entry; + gate_desc *desc; + + vector = >> exit_intr_info & INTR_INFO_VECTOR_MASK; +#ifdef CONFIG_X86_64 + desc = >> (void *)vcpu->arch.host_idt.address + vector * 16; +#else + desc = >> (void *)vcpu->arch.host_idt.address + vector * 8; +#endif + + entry = >> gate_offset(*desc); + asm( + "mov %0, %%" _ASM_DX " \n\t" +#ifdef >> CONFIG_X86_64 + "mov %%" _ASM_SP ", %%" _ASM_BX " \n\t" + "and >> $0xfffffffffffffff0, %%" _ASM_SP " \n\t" + "mov %%ss, %%" _ASM_AX " >> \n\t" + "push %%" _ASM_AX " \n\t" + "push %%" _ASM_BX " \n\t" >> +#endif + "pushf \n\t" + "mov %%cs, %%" _ASM_AX " \n\t" + "push >> %%" _ASM_AX " \n\t" + "push intr_return \n\t" + "jmp *%% " _ASM_DX >> " \n\t" + "1: \n\t" + ".pushsection .rodata \n\t" + ".global >> intr_return \n\t" + "intr_return: " _ASM_PTR " 1b \n\t" >> + ".popsection \n\t" + : :"m"(entry) : +#ifdef CONFIG_X86_64 >> + "rax", "rbx", "rdx" +#else + "eax", "ebx", "edx" > ebx is not clobbered on 32bit as far as I see. Right. ebx is not touched. >> +#endif >> + ); >> + } >> +} >> + >> static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx) { u32 >> exit_intr_info; @@ -7363,6 +7412,7 @@ static struct kvm_x86_ops >> vmx_x86_ops = { .set_tdp_cr3 = vmx_set_cr3, >> >> .check_intercept = vmx_check_intercept, + .handle_external_intr = >> vmx_handle_external_intr, }; >> >> static int __init vmx_init(void) >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 1c9c834..000140b 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5728,6 +5728,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >> native_read_tsc()); >> >> vcpu->mode = OUTSIDE_GUEST_MODE; >> + kvm_x86_ops->handle_external_intr(vcpu); >> + > Move it after smp_wmb(). Also to be close to how real interrupt is > injected we can set IF in eflags copy that we push on a stack in > vmx_handle_external_intr() and get rid of local_irq_enable() here > (adding comment about it of course). svm_handle_external_intr() will > call local_irq_enable(). Yes, we can set IF in this case. But for non external interrupt caused vmexit, we still need the local_irq_enable() to open interrupt. >> smp_wmb(); >> local_irq_enable(); >> -- >> 1.7.1 > > > -- > Gleb. Best regards, Yang -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 24, 2013 at 12:47:14AM +0000, Zhang, Yang Z wrote: > Gleb Natapov wrote on 2013-01-23: > > On Tue, Jan 22, 2013 at 01:49:31PM +0800, Yang Zhang wrote: > >> From: Yang Zhang <yang.z.zhang@Intel.com> > >> > >> The "acknowledge interrupt on exit" feature controls processor behavior > >> for external interrupt acknowledgement. When this control is set, the > >> processor acknowledges the interrupt controller to acquire the > >> interrupt vector on VM exit. > >> > >> After enabling this feature, an interrupt which arrived when target cpu > >> is running in vmx non-root mode will be handled by vmx handler instead > >> of handler in idt. Currently, vmx handler only fakes an interrupt stack > >> and jump to idt table to let real handler to handle it. Further, we > >> will recognize the interrupt and only delivery the interrupt which not > >> belong to current vcpu through idt table. The interrupt which belonged > >> to current vcpu will be handled inside vmx handler. This will reduce > >> the interrupt handle cost of KVM. > >> > >> Refer to Intel SDM volum 3, chapter 33.2. > >> > >> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > >> --- > >> arch/x86/include/asm/kvm_host.h | 2 + arch/x86/kvm/svm.c > >> | 6 ++++ arch/x86/kvm/vmx.c | 52 > >> ++++++++++++++++++++++++++++++++++++++- arch/x86/kvm/x86.c > >> | 2 + 4 files changed, 61 insertions(+), 1 deletions(-) > >> diff --git a/arch/x86/include/asm/kvm_host.h > >> b/arch/x86/include/asm/kvm_host.h index c431b33..0b73602 100644 --- > >> a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h > >> @@ -345,6 +345,7 @@ struct kvm_vcpu_arch { > >> unsigned long cr8; > >> u32 hflags; > >> u64 efer; > >> + struct desc_ptr host_idt; > > Enough to save only host_idt.address. > > > >> u64 apic_base; struct kvm_lapic *apic; /* kernel irqchip context > >> */ unsigned long apic_attention; @@ -723,6 +724,7 @@ struct > >> kvm_x86_ops { int (*check_intercept)(struct kvm_vcpu *vcpu, > >> struct x86_instruction_info *info, enum x86_intercept_stage > >> stage); + void (*handle_external_intr)(struct kvm_vcpu *vcpu); }; > >> > >> struct kvm_arch_async_pf { > >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > >> index d29d3cd..e286600 100644 > >> --- a/arch/x86/kvm/svm.c > >> +++ b/arch/x86/kvm/svm.c > >> @@ -4227,6 +4227,11 @@ out: > >> return ret; > >> } > >> +static void svm_handle_external_intr(struct kvm_vcpu *vcpu) > >> +{ > >> + return; > >> +} > >> + > >> static struct kvm_x86_ops svm_x86_ops = { .cpu_has_kvm_support = > >> has_svm, .disabled_by_bios = is_disabled, @@ -4318,6 +4323,7 @@ > >> static struct kvm_x86_ops svm_x86_ops = { .set_tdp_cr3 = set_tdp_cr3, > >> > >> .check_intercept = svm_check_intercept, + .handle_external_intr = > >> svm_handle_external_intr, }; > >> > >> static int __init svm_init(void) > >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > >> index dd2a85c..ef98392 100644 > >> --- a/arch/x86/kvm/vmx.c > >> +++ b/arch/x86/kvm/vmx.c > >> @@ -2565,7 +2565,8 @@ static __init int setup_vmcs_config(struct > > vmcs_config *vmcs_conf) > >> #ifdef CONFIG_X86_64 > >> min |= VM_EXIT_HOST_ADDR_SPACE_SIZE; > >> #endif > >> - opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT; > >> + opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT | > >> + VM_EXIT_ACK_INTR_ON_EXIT; > >> if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS, > >> &_vmexit_control) < 0) > >> return -EIO; > >> @@ -3933,6 +3934,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) > >> > >> vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); > >> set_cr4_guest_host_mask(vmx); > >> + native_store_idt(&vmx->vcpu.arch.host_idt); > >> > > We already call native_store_idt() in vmx_set_constant_host_state(). No > > need to do it twice. Add vcpu parameter to vmx_set_constant_host_state() > > to get idt address from there. > Sure. > > >> return 0; > >> } > >> @@ -6096,6 +6098,53 @@ static void vmx_complete_atomic_exit(struct > > vcpu_vmx *vmx) > >> } > >> } > >> + +static void vmx_handle_external_intr(struct kvm_vcpu *vcpu) +{ + u32 > >> exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); + if ((exit_intr_info > >> & (INTR_INFO_VALID_MASK | INTR_INFO_INTR_TYPE_MASK)) + == > >> (INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR)) { + unsigned int vector; > >> + unsigned long entry; + gate_desc *desc; + + vector = > >> exit_intr_info & INTR_INFO_VECTOR_MASK; +#ifdef CONFIG_X86_64 + desc = > >> (void *)vcpu->arch.host_idt.address + vector * 16; +#else + desc = > >> (void *)vcpu->arch.host_idt.address + vector * 8; +#endif + + entry = > >> gate_offset(*desc); + asm( + "mov %0, %%" _ASM_DX " \n\t" +#ifdef > >> CONFIG_X86_64 + "mov %%" _ASM_SP ", %%" _ASM_BX " \n\t" + "and > >> $0xfffffffffffffff0, %%" _ASM_SP " \n\t" + "mov %%ss, %%" _ASM_AX " > >> \n\t" + "push %%" _ASM_AX " \n\t" + "push %%" _ASM_BX " \n\t" > >> +#endif + "pushf \n\t" + "mov %%cs, %%" _ASM_AX " \n\t" + "push > >> %%" _ASM_AX " \n\t" + "push intr_return \n\t" + "jmp *%% " _ASM_DX > >> " \n\t" + "1: \n\t" + ".pushsection .rodata \n\t" + ".global > >> intr_return \n\t" + "intr_return: " _ASM_PTR " 1b \n\t" > >> + ".popsection \n\t" + : :"m"(entry) : +#ifdef CONFIG_X86_64 > >> + "rax", "rbx", "rdx" +#else + "eax", "ebx", "edx" > > ebx is not clobbered on 32bit as far as I see. > Right. ebx is not touched. > > >> +#endif > >> + ); > >> + } > >> +} > >> + > >> static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx) { u32 > >> exit_intr_info; @@ -7363,6 +7412,7 @@ static struct kvm_x86_ops > >> vmx_x86_ops = { .set_tdp_cr3 = vmx_set_cr3, > >> > >> .check_intercept = vmx_check_intercept, + .handle_external_intr = > >> vmx_handle_external_intr, }; > >> > >> static int __init vmx_init(void) > >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > >> index 1c9c834..000140b 100644 > >> --- a/arch/x86/kvm/x86.c > >> +++ b/arch/x86/kvm/x86.c > >> @@ -5728,6 +5728,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > >> native_read_tsc()); > >> > >> vcpu->mode = OUTSIDE_GUEST_MODE; > >> + kvm_x86_ops->handle_external_intr(vcpu); > >> + > > Move it after smp_wmb(). Also to be close to how real interrupt is > > injected we can set IF in eflags copy that we push on a stack in > > vmx_handle_external_intr() and get rid of local_irq_enable() here > > (adding comment about it of course). svm_handle_external_intr() will > > call local_irq_enable(). > Yes, we can set IF in this case. But for non external interrupt caused vmexit, we still need the local_irq_enable() to open interrupt. > Of course. You can do it in vmx_handle_external_intr() if not external interrupt detected. > >> smp_wmb(); > >> local_irq_enable(); > >> -- > >> 1.7.1 > > > > > > -- > > Gleb. > > > Best regards, > Yang > -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Gleb Natapov wrote on 2013-01-24: > On Thu, Jan 24, 2013 at 12:47:14AM +0000, Zhang, Yang Z wrote: >> Gleb Natapov wrote on 2013-01-23: >>> On Tue, Jan 22, 2013 at 01:49:31PM +0800, Yang Zhang wrote: >>>> From: Yang Zhang <yang.z.zhang@Intel.com> >>>> >>>> The "acknowledge interrupt on exit" feature controls processor behavior >>>> for external interrupt acknowledgement. When this control is set, the >>>> processor acknowledges the interrupt controller to acquire the >>>> interrupt vector on VM exit. >>>> >>>> After enabling this feature, an interrupt which arrived when target cpu >>>> is running in vmx non-root mode will be handled by vmx handler instead >>>> of handler in idt. Currently, vmx handler only fakes an interrupt stack >>>> and jump to idt table to let real handler to handle it. Further, we >>>> will recognize the interrupt and only delivery the interrupt which not >>>> belong to current vcpu through idt table. The interrupt which belonged >>>> to current vcpu will be handled inside vmx handler. This will reduce >>>> the interrupt handle cost of KVM. >>>> >>>> Refer to Intel SDM volum 3, chapter 33.2. >>>> >>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> >>>> --- >>>> arch/x86/include/asm/kvm_host.h | 2 + arch/x86/kvm/svm.c >>>> | 6 ++++ arch/x86/kvm/vmx.c | 52 >>>> ++++++++++++++++++++++++++++++++++++++- arch/x86/kvm/x86.c >>>> | 2 + 4 files changed, 61 insertions(+), 1 deletions(-) >>>> diff --git a/arch/x86/include/asm/kvm_host.h >>>> b/arch/x86/include/asm/kvm_host.h index c431b33..0b73602 100644 --- >>>> a/arch/x86/include/asm/kvm_host.h +++ >>>> b/arch/x86/include/asm/kvm_host.h @@ -345,6 +345,7 @@ struct >>>> kvm_vcpu_arch { >>>> unsigned long cr8; >>>> u32 hflags; >>>> u64 efer; >>>> + struct desc_ptr host_idt; >>> Enough to save only host_idt.address. >>> >>>> u64 apic_base; struct kvm_lapic *apic; /* kernel irqchip >>>> context */ unsigned long apic_attention; @@ -723,6 +724,7 @@ struct >>>> kvm_x86_ops { int (*check_intercept)(struct kvm_vcpu *vcpu, >>>> >>>> struct x86_instruction_info *info, enum >>>> x86_intercept_stage stage); + void (*handle_external_intr)(struct >>>> kvm_vcpu *vcpu); }; >>>> >>>> struct kvm_arch_async_pf { >>>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >>>> index d29d3cd..e286600 100644 >>>> --- a/arch/x86/kvm/svm.c >>>> +++ b/arch/x86/kvm/svm.c >>>> @@ -4227,6 +4227,11 @@ out: >>>> return ret; >>>> } >>>> +static void svm_handle_external_intr(struct kvm_vcpu *vcpu) >>>> +{ >>>> + return; >>>> +} >>>> + >>>> static struct kvm_x86_ops svm_x86_ops = { .cpu_has_kvm_support = >>>> has_svm, .disabled_by_bios = is_disabled, @@ -4318,6 +4323,7 @@ >>>> static struct kvm_x86_ops svm_x86_ops = { .set_tdp_cr3 = > set_tdp_cr3, >>>> >>>> .check_intercept = svm_check_intercept, + .handle_external_intr = >>>> svm_handle_external_intr, }; >>>> >>>> static int __init svm_init(void) >>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>>> index dd2a85c..ef98392 100644 >>>> --- a/arch/x86/kvm/vmx.c >>>> +++ b/arch/x86/kvm/vmx.c >>>> @@ -2565,7 +2565,8 @@ static __init int setup_vmcs_config(struct >>> vmcs_config *vmcs_conf) >>>> #ifdef CONFIG_X86_64 >>>> min |= VM_EXIT_HOST_ADDR_SPACE_SIZE; >>>> #endif >>>> - opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT; >>>> + opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT | >>>> + VM_EXIT_ACK_INTR_ON_EXIT; >>>> if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS, >>>> &_vmexit_control) < 0) >>>> return -EIO; >>>> @@ -3933,6 +3934,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) >>>> >>>> vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); >>>> set_cr4_guest_host_mask(vmx); >>>> + native_store_idt(&vmx->vcpu.arch.host_idt); >>>> >>> We already call native_store_idt() in vmx_set_constant_host_state(). No >>> need to do it twice. Add vcpu parameter to vmx_set_constant_host_state() >>> to get idt address from there. >> Sure. >> >>>> return 0; >>>> } >>>> @@ -6096,6 +6098,53 @@ static void vmx_complete_atomic_exit(struct >>> vcpu_vmx *vmx) >>>> } >>>> } >>>> + +static void vmx_handle_external_intr(struct kvm_vcpu *vcpu) +{ >>>> + u32 exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); + if >>>> ((exit_intr_info & (INTR_INFO_VALID_MASK | INTR_INFO_INTR_TYPE_MASK)) >>>> + == (INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR)) { + unsigned int >>>> vector; + unsigned long entry; + gate_desc *desc; + + vector = >>>> exit_intr_info & INTR_INFO_VECTOR_MASK; +#ifdef CONFIG_X86_64 + >>>> desc = (void *)vcpu->arch.host_idt.address + vector * 16; +#else >>>> + desc = (void *)vcpu->arch.host_idt.address + vector * 8; +#endif + >>>> + entry = gate_offset(*desc); + asm( + "mov %0, %%" _ASM_DX " >>>> \n\t" +#ifdef CONFIG_X86_64 + "mov %%" _ASM_SP ", %%" _ASM_BX " >>>> \n\t" + "and $0xfffffffffffffff0, %%" _ASM_SP " \n\t" + "mov >>>> %%ss, %%" _ASM_AX " \n\t" + "push %%" _ASM_AX " \n\t" + "push %%" >>>> _ASM_BX " \n\t" +#endif + "pushf \n\t" + "mov %%cs, %%" _ASM_AX " >>>> \n\t" + "push %%" _ASM_AX " \n\t" + "push intr_return \n\t" >>>> + "jmp *%% " _ASM_DX " \n\t" + "1: \n\t" + ".pushsection >>>> .rodata \n\t" + ".global intr_return \n\t" + "intr_return: " >>>> _ASM_PTR " 1b \n\t" + ".popsection \n\t" + : :"m"(entry) : >>>> +#ifdef CONFIG_X86_64 + "rax", "rbx", "rdx" +#else + "eax", >>>> "ebx", "edx" >>> ebx is not clobbered on 32bit as far as I see. >> Right. ebx is not touched. >> >>>> +#endif >>>> + ); >>>> + } >>>> +} >>>> + >>>> static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx) { u32 >>>> exit_intr_info; @@ -7363,6 +7412,7 @@ static struct kvm_x86_ops >>>> vmx_x86_ops = { .set_tdp_cr3 = vmx_set_cr3, >>>> >>>> .check_intercept = vmx_check_intercept, + .handle_external_intr = >>>> vmx_handle_external_intr, }; >>>> >>>> static int __init vmx_init(void) >>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>>> index 1c9c834..000140b 100644 >>>> --- a/arch/x86/kvm/x86.c >>>> +++ b/arch/x86/kvm/x86.c >>>> @@ -5728,6 +5728,8 @@ static int vcpu_enter_guest(struct kvm_vcpu > *vcpu) >>>> native_read_tsc()); >>>> >>>> vcpu->mode = OUTSIDE_GUEST_MODE; >>>> + kvm_x86_ops->handle_external_intr(vcpu); >>>> + >>> Move it after smp_wmb(). Also to be close to how real interrupt is >>> injected we can set IF in eflags copy that we push on a stack in >>> vmx_handle_external_intr() and get rid of local_irq_enable() here >>> (adding comment about it of course). svm_handle_external_intr() will >>> call local_irq_enable(). >> Yes, we can set IF in this case. But for non external interrupt caused >> vmexit, we still need the local_irq_enable() to open interrupt. >> > Of course. You can do it in vmx_handle_external_intr() if not external > interrupt detected. Sure. Best regards, Yang -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index c431b33..0b73602 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -345,6 +345,7 @@ struct kvm_vcpu_arch { unsigned long cr8; u32 hflags; u64 efer; + struct desc_ptr host_idt; u64 apic_base; struct kvm_lapic *apic; /* kernel irqchip context */ unsigned long apic_attention; @@ -723,6 +724,7 @@ struct kvm_x86_ops { int (*check_intercept)(struct kvm_vcpu *vcpu, struct x86_instruction_info *info, enum x86_intercept_stage stage); + void (*handle_external_intr)(struct kvm_vcpu *vcpu); }; struct kvm_arch_async_pf { diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index d29d3cd..e286600 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -4227,6 +4227,11 @@ out: return ret; } +static void svm_handle_external_intr(struct kvm_vcpu *vcpu) +{ + return; +} + static struct kvm_x86_ops svm_x86_ops = { .cpu_has_kvm_support = has_svm, .disabled_by_bios = is_disabled, @@ -4318,6 +4323,7 @@ static struct kvm_x86_ops svm_x86_ops = { .set_tdp_cr3 = set_tdp_cr3, .check_intercept = svm_check_intercept, + .handle_external_intr = svm_handle_external_intr, }; static int __init svm_init(void) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index dd2a85c..ef98392 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2565,7 +2565,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf) #ifdef CONFIG_X86_64 min |= VM_EXIT_HOST_ADDR_SPACE_SIZE; #endif - opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT; + opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT | + VM_EXIT_ACK_INTR_ON_EXIT; if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS, &_vmexit_control) < 0) return -EIO; @@ -3933,6 +3934,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); set_cr4_guest_host_mask(vmx); + native_store_idt(&vmx->vcpu.arch.host_idt); return 0; } @@ -6096,6 +6098,53 @@ static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx) } } + +static void vmx_handle_external_intr(struct kvm_vcpu *vcpu) +{ + u32 exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO); + if ((exit_intr_info & (INTR_INFO_VALID_MASK | INTR_INFO_INTR_TYPE_MASK)) + == (INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR)) { + unsigned int vector; + unsigned long entry; + gate_desc *desc; + + vector = exit_intr_info & INTR_INFO_VECTOR_MASK; +#ifdef CONFIG_X86_64 + desc = (void *)vcpu->arch.host_idt.address + vector * 16; +#else + desc = (void *)vcpu->arch.host_idt.address + vector * 8; +#endif + + entry = gate_offset(*desc); + asm( + "mov %0, %%" _ASM_DX " \n\t" +#ifdef CONFIG_X86_64 + "mov %%" _ASM_SP ", %%" _ASM_BX " \n\t" + "and $0xfffffffffffffff0, %%" _ASM_SP " \n\t" + "mov %%ss, %%" _ASM_AX " \n\t" + "push %%" _ASM_AX " \n\t" + "push %%" _ASM_BX " \n\t" +#endif + "pushf \n\t" + "mov %%cs, %%" _ASM_AX " \n\t" + "push %%" _ASM_AX " \n\t" + "push intr_return \n\t" + "jmp *%% " _ASM_DX " \n\t" + "1: \n\t" + ".pushsection .rodata \n\t" + ".global intr_return \n\t" + "intr_return: " _ASM_PTR " 1b \n\t" + ".popsection \n\t" + : :"m"(entry) : +#ifdef CONFIG_X86_64 + "rax", "rbx", "rdx" +#else + "eax", "ebx", "edx" +#endif + ); + } +} + static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx) { u32 exit_intr_info; @@ -7363,6 +7412,7 @@ static struct kvm_x86_ops vmx_x86_ops = { .set_tdp_cr3 = vmx_set_cr3, .check_intercept = vmx_check_intercept, + .handle_external_intr = vmx_handle_external_intr, }; static int __init vmx_init(void) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 1c9c834..000140b 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5728,6 +5728,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) native_read_tsc()); vcpu->mode = OUTSIDE_GUEST_MODE; + kvm_x86_ops->handle_external_intr(vcpu); + smp_wmb(); local_irq_enable();