From patchwork Tue Oct 16 21:29:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jim Mattson X-Patchwork-Id: 10644227 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 01B12109C for ; Tue, 16 Oct 2018 21:29:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E5FA32A9D3 for ; Tue, 16 Oct 2018 21:29:53 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D91CC2A9DF; Tue, 16 Oct 2018 21:29:53 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 513832A9D3 for ; Tue, 16 Oct 2018 21:29:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727074AbeJQFWK (ORCPT ); Wed, 17 Oct 2018 01:22:10 -0400 Received: from mail-io1-f74.google.com ([209.85.166.74]:32847 "EHLO mail-io1-f74.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726936AbeJQFWK (ORCPT ); Wed, 17 Oct 2018 01:22:10 -0400 Received: by mail-io1-f74.google.com with SMTP id c5-v6so22996671ioa.0 for ; Tue, 16 Oct 2018 14:29:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=5zndTkr8W4c/jKOuDMvmm1BdmYkimk+6/dr5INJij+c=; b=J673UMP8DKGdCI/6Bl/8nd/c6pCtt/hxUjSs9gRZs/AYaQYlMxIl30FZ431vw2Hnf9 Tloal/V+XB0x1OWPpjnnbjCJpqXgLIhzhxb1e64/88BaPHNn+4YtrM4Eu4hCzv9d+Ywt EIvuZPNyRBhQ+f4193/kZxQZoYIEx28q9i9QA5KkjVsgFsuhFcsW8SnFQYGR76oOr3KQ yPOihTA6GSfEWrp/KovrFvuaz4QJ66kfmUUWsVRvWE/Bwr1OxFodHqaG4KzBzzg3iTCh JZt6cOIuqEr8jC6TF7ByWLz5dRUlWmO1aLoeEW+dFHtX2xXOKyIitdqCKoxjUtE5fjn7 X45w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=5zndTkr8W4c/jKOuDMvmm1BdmYkimk+6/dr5INJij+c=; b=Visti7pEJjSxeMLBjjglxEJjgXOZPANMG1IYQWn9i4N4GNlHRPL7U/5wXx7nIScMm5 ZvTarizOpLzgjuA3GwumcEEIynvbcsP9pMzeYpmRS21ttihBzV9rjCPmWj4/7jUeZ5n9 ERKPYm7zPO6Ec0+Aq3Kmpk5aOhDc5lPC0i9Oym4eI3Hym2i694o1c5sunsS1hpWoHyWD ilKVN84md1mlhS1jraOC51XZQBmauEqgvU8obqBKkzCRnIwuFHVirSkQ4r4ddZ0YU6IC P8oQ4+oIbg0RuhZoOcJUX+BRpuFC+coU7aO+BM9VzS5I0HiiZnp5NlnjnpmRXP9kGNb+ l+kg== X-Gm-Message-State: ABuFfoiT1byhCec0yAY8a60VvkrdeJuaNl8ecsZwHrfw6YDaDP+rAz64 lLEzUE3gtR9i3jYLAVCknpWvglL7wlbfz+Wb17u3nh7OM6Oh1OnNHiDJvnniYaIT8EOPl29v5HA uC4MkZ2g5rG7cyqDLlV0J9whB8Ve0k0HqRDm6M1Y46ph8Ey7K4ZrUilMhNpJI4Zk= X-Google-Smtp-Source: ACcGV62J4kAIc2JtbcNkUbrysHBXd5KDshxlDIlEJG4sTVvGcOUOpgE4GQnUEW9bgXfriOzwc4tlWVqq0zY6wA== X-Received: by 2002:a24:7f05:: with SMTP id r5-v6mr17603012itc.2.1539725390374; Tue, 16 Oct 2018 14:29:50 -0700 (PDT) Date: Tue, 16 Oct 2018 14:29:23 -0700 In-Reply-To: <20181016212924.130307-1-jmattson@google.com> Message-Id: <20181016212924.130307-6-jmattson@google.com> Mime-Version: 1.0 References: <20181016212924.130307-1-jmattson@google.com> X-Mailer: git-send-email 2.19.1.331.ge82ca0e54c-goog Subject: [PATCH v2 6/7] kvm: vmx: Defer setting of DR6 until #DB delivery From: Jim Mattson To: kvm@vger.kernel.org Cc: Peter Shier , Liran Alon , Paolo Bonzini , Jim Mattson Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When exception payloads are enabled by userspace (which is not yet possible) and a #DB is raised in L2, defer the setting of DR6 until later. Under VMX, this allows the L1 hypervisor to intercept the fault before DR6 is modified. Under SVM, DR6 is modified before L1 can intercept the fault (as has always been the case with DR7). Note that the payload associated with a #DB exception includes only the "new DR6 bits." When the payload is delievered, DR6.B0-B3 will be cleared and DR6.RTM will be set prior to merging in the new DR6 bits. Also note that bit 16 in the "new DR6 bits" is set to indicate that a debug exception (#DB) or a breakpoint exception (#BP) occurred inside an RTM region while advanced debugging of RTM transactional regions was enabled. Though the reverse of DR6.RTM, this makes the #DB payload field compatible with both the pending debug exceptions field under VMX and the exit qualification for #DB exceptions under VMX. Reported-by: Jim Mattson Suggested-by: Paolo Bonzini Signed-off-by: Jim Mattson --- arch/x86/kvm/vmx.c | 18 ++++-------- arch/x86/kvm/x86.c | 69 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 58 insertions(+), 29 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 6d55a2213e12..fd61a0ad3e6d 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3285,18 +3285,12 @@ static int nested_vmx_check_exception(struct kvm_vcpu *vcpu, unsigned long *exit *exit_qual = has_payload ? payload : vcpu->arch.cr2; return 1; } - } else { - /* - * FIXME: we must not write DR6 when L1 intercepts an - * L2 #DB exception. - */ - if (vmcs12->exception_bitmap & (1u << nr)) { - if (nr == DB_VECTOR) - *exit_qual = vcpu->arch.dr6; - else - *exit_qual = 0; - return 1; - } + } else if (vmcs12->exception_bitmap & (1u << nr)) { + if (nr == DB_VECTOR) + *exit_qual = has_payload ? payload : vcpu->arch.dr6; + else + *exit_qual = 0; + return 1; } return 0; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 872da22c7514..d68c34a74590 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -410,6 +410,28 @@ void kvm_deliver_exception_payload(struct kvm_vcpu *vcpu) return; switch (nr) { + case DB_VECTOR: + /* + * "Certain debug exceptions may clear bit 0-3. The + * remaining contents of the DR6 register are never + * cleared by the processor". + */ + vcpu->arch.dr6 &= ~DR_TRAP_BITS; + /* + * DR6.RTM is set by all #DB exceptions that don't clear it. + */ + vcpu->arch.dr6 |= DR6_RTM; + vcpu->arch.dr6 |= payload; + /* + * Bit 16 should be set in the payload whenever the #DB + * exception should clear DR6.RTM. This makes the payload + * compatible with the pending debug exceptions under VMX. + * Though not currently documented in the SDM, this also + * makes the payload compatible with the exit qualification + * for #DB exceptions under VMX. + */ + vcpu->arch.dr6 ^= payload & DR6_RTM; + break; case PF_VECTOR: vcpu->arch.cr2 = payload; break; @@ -464,11 +486,13 @@ static void kvm_multiple_exception(struct kvm_vcpu *vcpu, /* * In guest mode, payload delivery should be deferred, * so that the L1 hypervisor can intercept #PF before - * CR2 is modified. However, for ABI compatibility - * with KVM_GET_VCPU_EVENTS and KVM_SET_VCPU_EVENTS, - * we can't delay payload delivery unless userspace - * has enabled this functionality via the per-VM - * capability, KVM_CAP_EXCEPTION_PAYLOAD. + * CR2 is modified (or intercept #DB before DR6 is + * modified under nVMX). However, for ABI + * compatibility with KVM_GET_VCPU_EVENTS and + * KVM_SET_VCPU_EVENTS, we can't delay payload + * delivery unless userspace has enabled this + * functionality via the per-VM capability, + * KVM_CAP_EXCEPTION_PAYLOAD. */ if (!vcpu->kvm->arch.exception_payload_enabled || !is_guest_mode(vcpu)) @@ -518,6 +542,12 @@ void kvm_requeue_exception(struct kvm_vcpu *vcpu, unsigned nr) } EXPORT_SYMBOL_GPL(kvm_requeue_exception); +static void kvm_queue_exception_p(struct kvm_vcpu *vcpu, unsigned nr, + unsigned long payload) +{ + kvm_multiple_exception(vcpu, nr, false, 0, true, payload, false); +} + static void kvm_queue_exception_e_p(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code, unsigned long payload) { @@ -6133,14 +6163,7 @@ static void kvm_vcpu_do_singlestep(struct kvm_vcpu *vcpu, int *r) kvm_run->exit_reason = KVM_EXIT_DEBUG; *r = EMULATE_USER_EXIT; } else { - /* - * "Certain debug exceptions may clear bit 0-3. The - * remaining contents of the DR6 register are never - * cleared by the processor". - */ - vcpu->arch.dr6 &= ~15; - vcpu->arch.dr6 |= DR6_BS | DR6_RTM; - kvm_queue_exception(vcpu, DB_VECTOR); + kvm_queue_exception_p(vcpu, DB_VECTOR, DR6_BS); } } @@ -7079,10 +7102,22 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win) __kvm_set_rflags(vcpu, kvm_get_rflags(vcpu) | X86_EFLAGS_RF); - if (vcpu->arch.exception.nr == DB_VECTOR && - (vcpu->arch.dr7 & DR7_GD)) { - vcpu->arch.dr7 &= ~DR7_GD; - kvm_update_dr7(vcpu); + if (vcpu->arch.exception.nr == DB_VECTOR) { + /* + * This code assumes that nSVM doesn't use + * check_nested_events(). If it does, the + * DR6/DR7 changes should happen before L1 + * gets a #VMEXIT for an intercepted #DB in + * L2. (Under VMX, on the other hand, the + * DR6/DR7 changes should not happen in the + * event of a VM-exit to L1 for an intercepted + * #DB in L2.) + */ + kvm_deliver_exception_payload(vcpu); + if (vcpu->arch.dr7 & DR7_GD) { + vcpu->arch.dr7 &= ~DR7_GD; + kvm_update_dr7(vcpu); + } } kvm_x86_ops->queue_exception(vcpu);