On Mon, Feb 15, 2010 at 07:17:17PM +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 <[email protected]>
> ---
>  arch/x86/kvm/svm.c |   54 +++++++++++++++++++++++++++++++++++----------------
>  1 files changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 52f78dd..f63f1db 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,8 @@ struct vcpu_svm {
>       struct nested_state nested;
>  
>       bool nmi_singlestep;
> +
> +     bool int3_injected;
>  };
>  
>  /* enable NPT for AMD64 and X86 with PAE */
> @@ -234,23 +237,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;
> -}
> -
>  static int is_external_interrupt(u32 info)
>  {
>       info &= SVM_EVTINJ_TYPE_MASK | SVM_EVTINJ_VALID;
> @@ -296,6 +282,36 @@ 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)) {
> +             /*
> +              * For guest debugging where we have to reinject #BP if some
> +              * INT3 is guest-owned:
> +              * Emulate nRIP by moving RIP one forward. Will fail if
> +              * injection raises a fault that is not intercepted. Still
> +              * better than failing in all cases.
> +              */
> +             svm->next_rip = kvm_rip_read(&svm->vcpu) + 1;
> +             skip_emulated_instruction(&svm->vcpu);
if next_rip is zero skip_emulated_instruction() decodes instruction by
itself to properly calculate next rip so no need to guess instruction
length.

> +             svm->int3_injected = true;
> +     }
> +
> +     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;
> @@ -2653,6 +2669,9 @@ static void svm_complete_interrupts(struct vcpu_svm 
> *svm)
>               if (is_nested(svm))
>                       break;
>               if (kvm_exception_is_soft(vector))
> +                     if (vector == BP_VECTOR && svm->int3_injected)
> +                             kvm_rip_write(&svm->vcpu,
> +                                           kvm_rip_read(&svm->vcpu) - 1);
You don't even check current rip? So if fault happens during unrelated #BP you 
move
rip backwards and restart.

>                       break;
>               if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) {
>                       u32 err = svm->vmcb->control.exit_int_info_err;
> @@ -2667,6 +2686,7 @@ static void svm_complete_interrupts(struct vcpu_svm 
> *svm)
>       default:
>               break;
>       }
> +     svm->int3_injected =false;
Looks like the wrong place to clear this. It should be cleared on every
exit, not only with valid vectoring info.

>  }
>  
>  #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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to