On Mon, Feb 22, 2010 at 06:51:19PM +0100, Jan Kiszka wrote:
> When in guest debugging mode, we have to reinject those #BP software
> exceptions that are caused by guest-injected INT3. As older AMD
> processors to not support the required nRIP VMCB field, try to emulate
> it by moving RIP by one on injection. Fix it up again in case the
> injection failed and we were able to catch this. This does not work for
> unintercepted faults, but it is better than doing nothing.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> ---
> arch/x86/kvm/svm.c | 72 +++++++++++++++++++++++++++++++++++++--------------
> 1 files changed, 52 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 1d76899..754e2f7 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -46,6 +46,7 @@ MODULE_LICENSE("GPL");
> #define SVM_FEATURE_NPT (1 << 0)
> #define SVM_FEATURE_LBRV (1 << 1)
> #define SVM_FEATURE_SVML (1 << 2)
> +#define SVM_FEATURE_NRIP (1 << 3)
> #define SVM_FEATURE_PAUSE_FILTER (1 << 10)
>
> #define NESTED_EXIT_HOST 0 /* Exit handled on host level */
> @@ -109,6 +110,10 @@ struct vcpu_svm {
> struct nested_state nested;
>
> bool nmi_singlestep;
> +
> + unsigned int3_injected;
> + u16 int3_cs;
> + u64 int3_rip;
> };
>
> /* enable NPT for AMD64 and X86 with PAE */
> @@ -235,23 +240,6 @@ static void svm_set_efer(struct kvm_vcpu *vcpu, u64 efer)
> vcpu->arch.efer = efer;
> }
>
> -static void svm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
> - bool has_error_code, u32 error_code)
> -{
> - struct vcpu_svm *svm = to_svm(vcpu);
> -
> - /* If we are within a nested VM we'd better #VMEXIT and let the
> - guest handle the exception */
> - if (nested_svm_check_exception(svm, nr, has_error_code, error_code))
> - return;
> -
> - svm->vmcb->control.event_inj = nr
> - | SVM_EVTINJ_VALID
> - | (has_error_code ? SVM_EVTINJ_VALID_ERR : 0)
> - | SVM_EVTINJ_TYPE_EXEPT;
> - svm->vmcb->control.event_inj_err = error_code;
> -}
> -
Why have you moved svm_queue_exception() function?
> static int is_external_interrupt(u32 info)
> {
> info &= SVM_EVTINJ_TYPE_MASK | SVM_EVTINJ_VALID;
> @@ -297,6 +285,39 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu)
> svm_set_interrupt_shadow(vcpu, 0);
> }
>
> +static void svm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
> + bool has_error_code, u32 error_code)
> +{
> + struct vcpu_svm *svm = to_svm(vcpu);
> +
> + /* If we are within a nested VM we'd better #VMEXIT and let the
> + guest handle the exception */
> + if (nested_svm_check_exception(svm, nr, has_error_code, error_code))
> + return;
> +
> + if (nr == BP_VECTOR && !svm_has(SVM_FEATURE_NRIP)) {
> + u64 old_rip = kvm_rip_read(&svm->vcpu);
> +
> + /*
> + * For guest debugging where we have to reinject #BP if some
> + * INT3 is guest-owned:
> + * Emulate nRIP by moving RIP forward. Will fail if injection
> + * raises a fault that is not intercepted. Still better than
> + * failing in all cases.
> + */
> + skip_emulated_instruction(&svm->vcpu);
> + svm->int3_cs = svm->vmcb->save.cs.selector;
> + svm->int3_rip = kvm_rip_read(&svm->vcpu);
> + svm->int3_injected = svm->int3_rip - old_rip;
> + }
> +
> + svm->vmcb->control.event_inj = nr
> + | SVM_EVTINJ_VALID
> + | (has_error_code ? SVM_EVTINJ_VALID_ERR : 0)
> + | SVM_EVTINJ_TYPE_EXEPT;
> + svm->vmcb->control.event_inj_err = error_code;
> +}
> +
> static int has_svm(void)
> {
> const char *msg;
> @@ -2703,8 +2724,10 @@ static void svm_complete_interrupts(struct vcpu_svm *svm)
> kvm_clear_exception_queue(&svm->vcpu);
> kvm_clear_interrupt_queue(&svm->vcpu);
>
int int3_injected = svm->int3_injected;
svm->int3_injected = 0;
Will save us from zeroing svm->int3_injected in two places.
> - if (!(exitintinfo & SVM_EXITINTINFO_VALID))
> + if (!(exitintinfo & SVM_EXITINTINFO_VALID)) {
> + svm->int3_injected = 0;
> return;
> + }
>
> vector = exitintinfo & SVM_EXITINTINFO_VEC_MASK;
> type = exitintinfo & SVM_EXITINTINFO_TYPE_MASK;
> @@ -2714,12 +2737,20 @@ static void svm_complete_interrupts(struct vcpu_svm *svm)
> svm->vcpu.arch.nmi_injected = true;
> break;
> case SVM_EXITINTINFO_TYPE_EXEPT:
> - /* In case of software exception do not reinject an exception
> - vector, but re-execute and instruction instead */
> if (is_nested(svm))
> break;
> if (kvm_exception_is_soft(vector))
> + if (vector == BP_VECTOR && svm->int3_injected &&
> + svm->vmcb->save.cs.selector == svm->int3_cs &&
> + kvm_rip_read(&svm->vcpu) == svm->int3_rip)
> + kvm_rip_write(&svm->vcpu,
> + kvm_rip_read(&svm->vcpu) -
> + svm->int3_injected);
> break;
Something is very wrong here. break does not belong to any of above ifs,
so code below is unreachable.
> + /*
> + * In case of other software exceptions, do not reinject the
> + * vector, but re-execute the instruction instead.
> + */
> if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) {
> u32 err = svm->vmcb->control.exit_int_info_err;
> kvm_queue_exception_e(&svm->vcpu, vector, err);
> @@ -2733,6 +2764,7 @@ static void svm_complete_interrupts(struct vcpu_svm *svm)
> default:
> break;
> }
> + svm->int3_injected = 0;
> }
>
> #ifdef CONFIG_X86_64
> --
> 1.6.0.2
--
Gleb.
--
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
