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@xxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxx>
>>>> ---
>>>> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
