From: Gregory Haskins <gregory.haskins@gmail.com>
To: Gleb Natapov <gleb@redhat.com>
Cc: avi@redhat.com, kvm@vger.kernel.org
Subject: Re: [PATCH 5/9] [VMX] Do not re-execute INTn instruction.
Date: Wed, 06 May 2009 06:59:05 -0400 [thread overview]
Message-ID: <4A016D79.9040905@gmail.com> (raw)
In-Reply-To: <1241511275-2261-5-git-send-email-gleb@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 8541 bytes --]
Hi Gleb,
Gleb Natapov wrote:
> Re-inject event instead. This is what Intel suggest. Also use correct
> instruction length when re-injecting soft fault/interrupt.
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
> arch/x86/include/asm/kvm_host.h | 5 ++++-
> arch/x86/kvm/svm.c | 6 +++---
> arch/x86/kvm/vmx.c | 29 ++++++++++++++++++++++-------
> arch/x86/kvm/x86.c | 13 ++++++++-----
> arch/x86/kvm/x86.h | 9 ++++++++-
> 5 files changed, 45 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index cc892f5..fea0429 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -319,6 +319,8 @@ struct kvm_vcpu_arch {
> struct kvm_pio_request pio;
> void *pio_data;
>
> + u8 event_exit_inst_len;
> +
> struct kvm_queued_exception {
> bool pending;
> bool has_error_code;
> @@ -328,6 +330,7 @@ struct kvm_vcpu_arch {
>
> struct kvm_queued_interrupt {
> bool pending;
> + bool soft;
> u8 nr;
> } interrupt;
>
> @@ -510,7 +513,7 @@ struct kvm_x86_ops {
> void (*skip_emulated_instruction)(struct kvm_vcpu *vcpu);
> void (*patch_hypercall)(struct kvm_vcpu *vcpu,
> unsigned char *hypercall_addr);
> - void (*set_irq)(struct kvm_vcpu *vcpu, int vec);
> + void (*set_irq)(struct kvm_vcpu *vcpu, int vec, bool soft);
> void (*set_nmi)(struct kvm_vcpu *vcpu);
> void (*queue_exception)(struct kvm_vcpu *vcpu, unsigned nr,
> bool has_error_code, u32 error_code);
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 14cdfce..d5173a2 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -2284,7 +2284,7 @@ static void svm_queue_irq(struct kvm_vcpu *vcpu, unsigned nr)
> SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_INTR;
> }
>
> -static void svm_set_irq(struct kvm_vcpu *vcpu, int irq)
> +static void svm_set_irq(struct kvm_vcpu *vcpu, int irq, bool soft)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
>
> @@ -2392,7 +2392,7 @@ static void svm_complete_interrupts(struct vcpu_svm *svm)
> case SVM_EXITINTINFO_TYPE_EXEPT:
> /* In case of software exception do not reinject an exception
> vector, but re-execute and instruction instead */
> - if (vector == BP_VECTOR || vector == OF_VECTOR)
> + if (kvm_exception_is_soft(vector))
> break;
> if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) {
> u32 err = svm->vmcb->control.exit_int_info_err;
> @@ -2402,7 +2402,7 @@ static void svm_complete_interrupts(struct vcpu_svm *svm)
> kvm_queue_exception(&svm->vcpu, vector);
> break;
> case SVM_EXITINTINFO_TYPE_INTR:
> - kvm_queue_interrupt(&svm->vcpu, vector);
> + kvm_queue_interrupt(&svm->vcpu, vector, false);
> break;
> default:
> break;
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index a9b30e6..092a3ee 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -779,8 +779,9 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
> return;
> }
>
> - if (nr == BP_VECTOR || nr == OF_VECTOR) {
> - vmcs_write32(VM_ENTRY_INSTRUCTION_LEN, 1);
> + if (kvm_exception_is_soft(nr)) {
> + vmcs_write32(VM_ENTRY_INSTRUCTION_LEN,
> + vmx->vcpu.arch.event_exit_inst_len);
> intr_info |= INTR_TYPE_SOFT_EXCEPTION;
> } else
> intr_info |= INTR_TYPE_HARD_EXCEPTION;
> @@ -2429,9 +2430,10 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
> vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control);
> }
>
> -static void vmx_inject_irq(struct kvm_vcpu *vcpu, int irq)
> +static void vmx_inject_irq(struct kvm_vcpu *vcpu, int irq, bool soft)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> + uint32_t intr;
>
> KVMTRACE_1D(INJ_VIRQ, vcpu, (u32)irq, handler);
>
> @@ -2446,8 +2448,14 @@ static void vmx_inject_irq(struct kvm_vcpu *vcpu, int irq)
> kvm_rip_write(vcpu, vmx->rmode.irq.rip - 1);
> return;
> }
> - vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
> - irq | INTR_TYPE_EXT_INTR | INTR_INFO_VALID_MASK);
> + intr = irq | INTR_INFO_VALID_MASK;
> + if (soft) {
> + intr |= INTR_TYPE_SOFT_INTR;
> + vmcs_write32(VM_ENTRY_INSTRUCTION_LEN,
> + vmx->vcpu.arch.event_exit_inst_len);
> + } else
> + intr |= INTR_TYPE_EXT_INTR;
> + vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, intr);
> }
>
> static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
> @@ -3008,6 +3016,7 @@ static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> GUEST_INTR_STATE_NMI);
> break;
> case INTR_TYPE_EXT_INTR:
> + case INTR_TYPE_SOFT_INTR:
> kvm_clear_interrupt_queue(vcpu);
> break;
> case INTR_TYPE_HARD_EXCEPTION:
> @@ -3279,16 +3288,22 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx)
> vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO,
> GUEST_INTR_STATE_NMI);
> break;
> - case INTR_TYPE_HARD_EXCEPTION:
> case INTR_TYPE_SOFT_EXCEPTION:
> + vmx->vcpu.arch.event_exit_inst_len =
> + vmcs_read32(VM_EXIT_INSTRUCTION_LEN);
>
Minor nit: Would suggest a "/* fall through */" comment here to denote
the break was intentionally omitted (assuming it was, but I think so IIUC).
> + case INTR_TYPE_HARD_EXCEPTION:
> if (idt_vectoring_info & VECTORING_INFO_DELIVER_CODE_MASK) {
> u32 err = vmcs_read32(IDT_VECTORING_ERROR_CODE);
> kvm_queue_exception_e(&vmx->vcpu, vector, err);
> } else
> kvm_queue_exception(&vmx->vcpu, vector);
> break;
> + case INTR_TYPE_SOFT_INTR:
> + vmx->vcpu.arch.event_exit_inst_len =
> + vmcs_read32(VM_EXIT_INSTRUCTION_LEN);
> case INTR_TYPE_EXT_INTR:
> - kvm_queue_interrupt(&vmx->vcpu, vector);
> + kvm_queue_interrupt(&vmx->vcpu, vector,
> + type == INTR_TYPE_SOFT_INTR);
> break;
> default:
> break;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 4596927..023842b 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1424,7 +1424,7 @@ static int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
> return -ENXIO;
> vcpu_load(vcpu);
>
> - kvm_queue_interrupt(vcpu, irq->irq);
> + kvm_queue_interrupt(vcpu, irq->irq, false);
>
> vcpu_put(vcpu);
>
> @@ -3136,7 +3136,8 @@ static void inject_irq(struct kvm_vcpu *vcpu)
> }
>
> if (vcpu->arch.interrupt.pending) {
> - kvm_x86_ops->set_irq(vcpu, vcpu->arch.interrupt.nr);
> + kvm_x86_ops->set_irq(vcpu, vcpu->arch.interrupt.nr,
> + vcpu->arch.interrupt.soft);
> return;
> }
>
> @@ -3149,8 +3150,10 @@ static void inject_irq(struct kvm_vcpu *vcpu)
> }
> } else if (kvm_cpu_has_interrupt(vcpu)) {
> if (kvm_x86_ops->interrupt_allowed(vcpu)) {
> - kvm_queue_interrupt(vcpu, kvm_cpu_get_interrupt(vcpu));
> - kvm_x86_ops->set_irq(vcpu, vcpu->arch.interrupt.nr);
> + kvm_queue_interrupt(vcpu, kvm_cpu_get_interrupt(vcpu),
> + false);
> + kvm_x86_ops->set_irq(vcpu, vcpu->arch.interrupt.nr,
> + false);
> }
> }
> }
> @@ -4077,7 +4080,7 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> pending_vec = find_first_bit(
> (const unsigned long *)sregs->interrupt_bitmap, max_bits);
> if (pending_vec < max_bits) {
> - kvm_queue_interrupt(vcpu, pending_vec);
> + kvm_queue_interrupt(vcpu, pending_vec, false);
> pr_debug("Set back pending irq %d\n", pending_vec);
> if (irqchip_in_kernel(vcpu->kvm))
> kvm_pic_clear_isr_ack(vcpu->kvm);
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index c1f1a8c..2817c86 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -8,9 +8,11 @@ static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu)
> vcpu->arch.exception.pending = false;
> }
>
> -static inline void kvm_queue_interrupt(struct kvm_vcpu *vcpu, u8 vector)
> +static inline void kvm_queue_interrupt(struct kvm_vcpu *vcpu, u8 vector,
> + bool soft)
> {
> vcpu->arch.interrupt.pending = true;
> + vcpu->arch.interrupt.soft = soft;
> vcpu->arch.interrupt.nr = vector;
> }
>
> @@ -24,4 +26,9 @@ static inline bool kvm_event_needs_reinjection(struct kvm_vcpu *vcpu)
> return vcpu->arch.exception.pending || vcpu->arch.interrupt.pending ||
> vcpu->arch.nmi_injected;
> }
> +
> +static inline bool kvm_exception_is_soft(unsigned int nr)
> +{
> + return (nr == BP_VECTOR) || (nr == OF_VECTOR) || (nr == DB_VECTOR);
> +}
> #endif
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 266 bytes --]
next prev parent reply other threads:[~2009-05-06 10:59 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-05 8:14 [PATCH 1/9] Unprotect a page if #PF happens during NMI injection Gleb Natapov
2009-05-05 8:14 ` [PATCH 2/9] Do not allow interrupt injection from userspace if there is a pending event Gleb Natapov
2009-05-05 8:14 ` [PATCH 3/9] Remove irq_pending bitmap Gleb Natapov
2009-05-06 5:55 ` Sheng Yang
2009-05-06 6:50 ` Sheng Yang
2009-05-05 8:14 ` [PATCH 4/9] [SVM] skip_emulated_instruction() decode an instruction if size is not known Gleb Natapov
2009-05-05 8:14 ` [PATCH 5/9] [VMX] Do not re-execute INTn instruction Gleb Natapov
2009-05-06 6:57 ` Sheng Yang
2009-05-06 9:27 ` Gleb Natapov
2009-05-06 9:30 ` Avi Kivity
2009-05-06 10:59 ` Gregory Haskins [this message]
2009-05-06 11:46 ` Gleb Natapov
2009-05-05 8:14 ` [PATCH 6/9] IRQ/NMI window should always be requested Gleb Natapov
2009-05-05 8:14 ` [PATCH 7/9] Drop interrupt shadow when single stepping should be done only on VMX Gleb Natapov
2009-05-05 8:14 ` [PATCH 8/9] Replace pending exception by PF if it happens serially Gleb Natapov
2009-05-05 8:14 ` [PATCH 9/9] [SVM] inject NMI after IRET from a previous NMI, not before Gleb Natapov
2009-05-05 8:45 ` Jan Kiszka
2009-05-05 9:03 ` Gleb Natapov
2009-05-05 9:25 ` Jan Kiszka
2009-05-05 9:32 ` Gleb Natapov
2009-05-05 9:47 ` Gleb Natapov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A016D79.9040905@gmail.com \
--to=gregory.haskins@gmail.com \
--cc=avi@redhat.com \
--cc=gleb@redhat.com \
--cc=kvm@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.