From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH 3/3] KVM: x86: move kvm_inject_gp up from kvm_set_dr to callers
Date: Tue, 2 Feb 2021 10:17:16 -0800 [thread overview]
Message-ID: <YBmXLJPPTS7yzClF@google.com> (raw)
In-Reply-To: <20210202165141.88275-4-pbonzini@redhat.com>
On Tue, Feb 02, 2021, Paolo Bonzini wrote:
> Push the injection of #GP up to the callers, so that they can just use
> kvm_complete_insn_gp. __kvm_set_dr is pretty much what the callers can use
> together with kvm_complete_insn_gp, so rename it to kvm_set_dr and drop
> the old kvm_set_dr wrapper.
>
> This allows nested VMX code, which really wanted to use __kvm_set_dr, to
> use the right function.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/kvm/svm/svm.c | 14 +++++++-------
> arch/x86/kvm/vmx/vmx.c | 19 ++++++++++---------
> arch/x86/kvm/x86.c | 19 +++++--------------
> 3 files changed, 22 insertions(+), 30 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index c0d41a6920f0..818cf3babef2 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2599,6 +2599,7 @@ static int dr_interception(struct vcpu_svm *svm)
> {
> int reg, dr;
> unsigned long val;
> + int err;
>
> if (svm->vcpu.guest_debug == 0) {
> /*
> @@ -2617,19 +2618,18 @@ static int dr_interception(struct vcpu_svm *svm)
> reg = svm->vmcb->control.exit_info_1 & SVM_EXITINFO_REG_MASK;
> dr = svm->vmcb->control.exit_code - SVM_EXIT_READ_DR0;
>
> + if (!kvm_require_dr(&svm->vcpu, dr & 15))
Purely because I suck at reading base-10 bitwise operations, can we do "dr & 0xf"?
This also creates separate logic for writes (AND versus SUB), can you also
convert the other 'dr - 16'?
Alternatively, grab the "mov to DR" flag early on, but that feels less
readable, e.g.
mov_to_dr = dr & 0x10;
dr &= 0xf;
> + return 1;
> +
> if (dr >= 16) { /* mov to DRn */
> - if (!kvm_require_dr(&svm->vcpu, dr - 16))
> - return 1;
> val = kvm_register_read(&svm->vcpu, reg);
> - kvm_set_dr(&svm->vcpu, dr - 16, val);
> + err = kvm_set_dr(&svm->vcpu, dr - 16, val);
> } else {
> - if (!kvm_require_dr(&svm->vcpu, dr))
> - return 1;
> - kvm_get_dr(&svm->vcpu, dr, &val);
> + err = kvm_get_dr(&svm->vcpu, dr, &val);
Technically, 'err' needs to be checked, else 'reg' will theoretically be
clobbered with garbage. I say "theoretically", because kvm_get_dr() always
returns '0'; the CR4.DE=1 behavior is handled by kvm_require_dr(), presumably
due to it being a #UD instead of #GP. AFAICT, you can simply add a prep patch
to change the return type to void.
Side topic, is the kvm_require_dr() check needed on SVM interception? The APM
states:
All normal exception checks take precedence over the by implicit DR6/DR7 writes.)
I can't find anything that would suggest the CR4.DE=1 #UD isn't a "normal"
exception.
> kvm_register_write(&svm->vcpu, reg, val);
> }
>
> - return kvm_skip_emulated_instruction(&svm->vcpu);
> + return kvm_complete_insn_gp(&svm->vcpu, err);
> }
>
> static int cr8_write_interception(struct vcpu_svm *svm)
next prev parent reply other threads:[~2021-02-02 18:20 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-02 16:51 [PATCH 0/3] use kvm_complete_insn_gp more Paolo Bonzini
2021-02-02 16:51 ` [PATCH 1/3] KVM: x86: move kvm_inject_gp up from kvm_set_xcr to callers Paolo Bonzini
2021-02-02 17:19 ` Sean Christopherson
2021-02-02 17:24 ` Paolo Bonzini
2021-02-02 16:51 ` [PATCH 2/3] KVM: x86: move kvm_inject_gp up from kvm_handle_invpcid " Paolo Bonzini
2021-02-02 17:38 ` Sean Christopherson
2021-02-02 17:44 ` Paolo Bonzini
2021-02-02 16:51 ` [PATCH 3/3] KVM: x86: move kvm_inject_gp up from kvm_set_dr " Paolo Bonzini
2021-02-02 18:17 ` Sean Christopherson [this message]
2021-02-03 9:24 ` Paolo Bonzini
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=YBmXLJPPTS7yzClF@google.com \
--to=seanjc@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).