Il 27/12/2013 19:17, Jan Kiszka ha scritto:
>
> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>
> In contrast to VMX, SVM dose not automatically transfer DR6 into the
> VCPU's arch.dr6. So if we face a DR6 read, we must consult a new vendor
> hook to obtain the current value. And as SVM now picks the DR6 state
> from its VMCB, we also need a set callback in order to write updates of
> DR6 back.
>
> Fixes a regression of 020df0794f.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> ---
> arch/x86/include/asm/kvm_host.h | 2 ++
> arch/x86/kvm/svm.c | 18 ++++++++++++++++++
> arch/x86/kvm/vmx.c | 11 +++++++++++
> arch/x86/kvm/x86.c | 3 ++-
> 4 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index ae5d783..e73651b 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -699,6 +699,8 @@ struct kvm_x86_ops {
> void (*set_idt)(struct kvm_vcpu *vcpu, struct desc_ptr *dt);
> void (*get_gdt)(struct kvm_vcpu *vcpu, struct desc_ptr *dt);
> void (*set_gdt)(struct kvm_vcpu *vcpu, struct desc_ptr *dt);
> + u64 (*get_dr6)(struct kvm_vcpu *vcpu);
> + void (*set_dr6)(struct kvm_vcpu *vcpu, unsigned long value);
> void (*set_dr7)(struct kvm_vcpu *vcpu, unsigned long value);
> void (*cache_reg)(struct kvm_vcpu *vcpu, enum kvm_reg reg);
> unsigned long (*get_rflags)(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index c7168a5..5987414 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1671,6 +1671,22 @@ static void new_asid(struct vcpu_svm *svm, struct svm_cpu_data *sd)
> mark_dirty(svm->vmcb, VMCB_ASID);
> }
>
> +static u64 svm_get_dr6(struct kvm_vcpu *vcpu)
> +{
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)
> + return vcpu->arch.dr6;
> + else
> + return to_svm(vcpu)->vmcb->save.dr6;
> +}
> +
> +static void svm_set_dr6(struct kvm_vcpu *vcpu, unsigned long value)
> +{
> + struct vcpu_svm *svm = to_svm(vcpu);
> +
> + svm->vmcb->save.dr6 = value;
> + mark_dirty(svm->vmcb, VMCB_DR);
The code here is a bit different from the existing DR7 hooks. Why isn't
get_dr7 needed? I cannot find anything in the AMD manuals that suggests
a difference between DR6 and DR7.
Also, set_dr7 is only called when !(vcpu->guest_debug &
KVM_GUESTDBG_USE_HW_BP). I think it makes sense to do the same for all
the new hooks you need to introduce (get_dr6, set_dr6 and possibly get_dr7).
And finally, is it necessary to write to both vcpu->arch.dr6/7 and
svm->vmcb->save.dr6/7, or is it simply okay to do that?
Paolo
> +}
> +
> static void svm_set_dr7(struct kvm_vcpu *vcpu, unsigned long value)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
> @@ -4286,6 +4302,8 @@ static struct kvm_x86_ops svm_x86_ops = {
> .set_idt = svm_set_idt,
> .get_gdt = svm_get_gdt,
> .set_gdt = svm_set_gdt,
> + .get_dr6 = svm_get_dr6,
> + .set_dr6 = svm_set_dr6,
> .set_dr7 = svm_set_dr7,
> .cache_reg = svm_cache_reg,
> .get_rflags = svm_get_rflags,
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ee3bf54..1d9b0ec 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5153,6 +5153,15 @@ static int handle_dr(struct kvm_vcpu *vcpu)
> return 1;
> }
>
> +static u64 vmx_get_dr6(struct kvm_vcpu *vcpu)
> +{
> + return vcpu->arch.dr6;
> +}
> +
> +static void vmx_set_dr6(struct kvm_vcpu *vcpu, unsigned long val)
> +{
> +}
> +
> static void vmx_set_dr7(struct kvm_vcpu *vcpu, unsigned long val)
> {
> vmcs_writel(GUEST_DR7, val);
> @@ -8573,6 +8582,8 @@ static struct kvm_x86_ops vmx_x86_ops = {
> .set_idt = vmx_set_idt,
> .get_gdt = vmx_get_gdt,
> .set_gdt = vmx_set_gdt,
> + .get_dr6 = vmx_get_dr6,
> + .set_dr6 = vmx_set_dr6,
> .set_dr7 = vmx_set_dr7,
> .cache_reg = vmx_cache_reg,
> .get_rflags = vmx_get_rflags,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1dc0359..be000ec 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -747,6 +747,7 @@ static int __kvm_set_dr(struct kvm_vcpu *vcpu, int dr, unsigned long val)
> if (val & 0xffffffff00000000ULL)
> return -1; /* #GP */
> vcpu->arch.dr6 = (val & DR6_VOLATILE) | DR6_FIXED_1;
> + kvm_x86_ops->set_dr6(vcpu, vcpu->arch.dr6);
> break;
> case 5:
> if (kvm_read_cr4_bits(vcpu, X86_CR4_DE))
> @@ -788,7 +789,7 @@ static int _kvm_get_dr(struct kvm_vcpu *vcpu, int dr, unsigned long *val)
> return 1;
> /* fall through */
> case 6:
> - *val = vcpu->arch.dr6;
> + *val = kvm_x86_ops->get_dr6(vcpu);
> break;
> case 5:
> if (kvm_read_cr4_bits(vcpu, X86_CR4_DE))
--
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
