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@xxxxxxxxxx>
> ---
> 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:
signature.asc
Description: OpenPGP digital signature
