KVM: move DR register access handling into generic code.
PATCH 1/1 KVM: move DR register access handling into generic code.
Currently both SVM and VMX have their own DR handling code. Move it to x86.c.
arch/x86/include/asm/kvm_host.h +3 -2
@@ -496,8 +496,7 @@ 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); -int (*get_dr)(struct kvm_vcpu *vcpu,int dr,unsigned long *dest); -int (*set_dr)(struct kvm_vcpu *vcpu,int dr,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);void (*set_rflags)(struct kvm_vcpu *vcpu,unsigned long rflags);
@@ -602,6 +601,8 @@ void kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0);void kvm_set_cr3 (struct kvm_vcpu *vcpu,unsigned long cr3);void kvm_set_cr4 (struct kvm_vcpu *vcpu,unsigned long cr4);void kvm_set_cr8 (struct kvm_vcpu *vcpu,unsigned long cr8); +int kvm_set_dr (struct kvm_vcpu *vcpu,int dr,unsigned long val); +int kvm_get_dr (struct kvm_vcpu *vcpu,int dr,unsigned long *val);unsigned long kvm_get_cr8 (struct kvm_vcpu *vcpu);void kvm_lmsw (struct kvm_vcpu *vcpu,unsigned long msw);void kvm_get_cs_db_l_bits (struct kvm_vcpu *vcpu,int *db,int *l);
arch/x86/kvm/svm.c +3 -63
81 lines not on this page · raw message
arch/x86/kvm/vmx.c +11 -67
103 lines not on this page · raw message
arch/x86/kvm/x86.c +76 -2
96 lines not on this page · raw message
--
Gleb.
Quoted 4 lines · Gleb Natapov
Currently both SVM and VMX have their own DR handling code. Move it to
x86.c.
The standard process is to make them identical first and finally merge identical code, but I guess we can skip it in this case (Jan?)
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Quoted 8 lines · Avi Kivity
Quoted 4 lines · Gleb Natapov
Currently both SVM and VMX have their own DR handling code. Move it to
x86.c.The standard process is to make them identical first and finally merge identical code, but I guess we can skip it in this case (Jan?)
Looks OK, I would just...
Quoted 8 lines
arch/x86/kvm/x86.c +8
-0+int kvm_set_dr (struct kvm_vcpu *vcpu,int dr,unsigned long val) +{ +switch (dr) { + case 0 ... 3: + vcpu->arch.db[dr] = val; +if (!(vcpu->guest_debug &KVM_GUESTDBG_USE_HW_BP )) + vcpu->arch.eff_db[dr] = val; +break ;
arch/x86/kvm/x86.c unsigned long kvm_get_cr8(struct kvm_vcpu *vcpu) · line 581
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+case 4 : +if (kvm_read_cr4_bits (vcpu,X86_CR4_DE )) { +kvm_queue_exception (vcpu,UD_VECTOR ); +return 1 ; + } +/* fall through */ +case 6 : +if (val &0xffffffff00000000ULL ) { +kvm_inject_gp (vcpu,0 ); +return -1 ;
...either return 1 or -1 on failure.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Quoted 24 lines · Jan Kiszka
Quoted 8 lines · Avi Kivity
Quoted 4 lines · Gleb Natapov
Currently both SVM and VMX have their own DR handling code. Move it to
x86.c.The standard process is to make them identical first and finally merge identical code, but I guess we can skip it in this case (Jan?)
Looks OK, I would just...
Quoted 12 lines
arch/x86/kvm/x86.c +12
-0+int kvm_set_dr (struct kvm_vcpu *vcpu,int dr,unsigned long val) +{ +switch (dr) { + case 0 ... 3: + vcpu->arch.db[dr] = val; +if (!(vcpu->guest_debug &KVM_GUESTDBG_USE_HW_BP )) + vcpu->arch.eff_db[dr] = val; +break ; +case 4 : +if (kvm_read_cr4_bits (vcpu,X86_CR4_DE )) { +kvm_queue_exception (vcpu,UD_VECTOR ); +return 1 ;
Quoted 9 lines
arch/x86/kvm/x86.c unsigned long kvm_get_cr8(struct kvm_vcpu *vcpu) · line 581
Quoted 6 lines
arch/x86/kvm/x86.c +6
-0+ } +/* fall through */ +case 6 : +if (val &0xffffffff00000000ULL ) { +kvm_inject_gp (vcpu,0 ); +return -1 ;...either return 1 or -1 on failure.
This is a preparation for the future :) Exception injection will be moved outside the function so different return values are needed to distinguish different error conditions. Moving exception injection out is needed so emulator can call the function without it injecting exception behind emulators back.
--
Gleb.
Quoted 39 lines · Gleb Natapov
Quoted 27 lines · Jan Kiszka
Quoted 10 lines · Avi Kivity
Quoted 5 lines · Gleb Natapov
Currently both SVM and VMX have their own DR handling code. Move it to
x86.c.The standard process is to make them identical first and finally merge identical code, but I guess we can skip it in this case (Jan?)
Looks OK, I would just...
Quoted 12 lines
arch/x86/kvm/x86.c +12
-0+int kvm_set_dr (struct kvm_vcpu *vcpu,int dr,unsigned long val) +{ +switch (dr) { + case 0 ... 3: + vcpu->arch.db[dr] = val; +if (!(vcpu->guest_debug&KVM_GUESTDBG_USE_HW_BP )) + vcpu->arch.eff_db[dr] = val; +break ; +case 4 : +if (kvm_read_cr4_bits (vcpu,X86_CR4_DE )) { +kvm_queue_exception (vcpu,UD_VECTOR ); +return 1 ;Quoted 10 lines
arch/x86/kvm/x86.c unsigned long kvm_get_cr8(struct kvm_vcpu *vcpu) · line 581
Quoted 7 lines
arch/x86/kvm/x86.c +6
-0+ } +/* fall through */ +case 6 : +if (val&0xffffffff00000000ULL ) { +kvm_inject_gp (vcpu,0 ); +return -1 ;...either return 1 or -1 on failure.
Quoted 7 lines
This is a preparation for the future :) Exception injection will be moved outside the function so different return values are needed to distinguish different error conditions. Moving exception injection out is needed so emulator can call the function without it injecting exception behind emulators back.
Ugh, use symbolic names please. And I think that can be deferred for that future patch.
Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Quoted 32 lines · Avi Kivity
Quoted 31 lines · Gleb Natapov
Quoted 30 lines · Jan Kiszka
Quoted 7 lines · Avi Kivity
Quoted 4 lines · Gleb Natapov
Currently both SVM and VMX have their own DR handling code. Move it to
x86.c.The standard process is to make them identical first and finally merge identical code, but I guess we can skip it in this case (Jan?)
Looks OK, I would just...
Quoted 8 lines
arch/x86/kvm/x86.c +8
-0+int kvm_set_dr (struct kvm_vcpu *vcpu,int dr,unsigned long val) +{ +switch (dr) { + case 0 ... 3: + vcpu->arch.db[dr] = val; +if (!(vcpu->guest_debug&KVM_GUESTDBG_USE_HW_BP )) + vcpu->arch.eff_db[dr] = val; +break ;arch/x86/kvm/x86.c unsigned long kvm_get_cr8(struct kvm_vcpu *vcpu) · line 581
Quoted 10 lines
arch/x86/kvm/x86.c +10
-0+case 4 : +if (kvm_read_cr4_bits (vcpu,X86_CR4_DE )) { +kvm_queue_exception (vcpu,UD_VECTOR ); +return 1 ; + } +/* fall through */ +case 6 : +if (val&0xffffffff00000000ULL ) { +kvm_inject_gp (vcpu,0 ); +return -1 ;...either return 1 or -1 on failure.
Quoted 8 lines
Quoted 6 lines
This is a preparation for the future :) Exception injection will be moved outside the function so different return values are needed to distinguish different error conditions. Moving exception injection out is needed so emulator can call the function without it injecting exception behind emulators back.
Ugh, use symbolic names please.
All others (set_msr()/set_cr()) can cause only one type of exception each, so returning 1 for exception, 0 for success seams logical (and this is what set_msr() already does). I didn't want to have much different calling convention for set_dr(). It's not like we will have one more exception type that mov dr can generate suddenly :)
Quoted 3 lines
And I think that can be deferred
for that future patch.
Agree, I'll repost with 1 for error in all cases.
--
Gleb.