Gleb Natapov wrote:
> 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 <jan.kiszka@xxxxxxxxxxx>
>> ---
>> 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.
Just copied what all the instruction emulations do here. Can change, though.
>
>> + 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.
Yep, true. Will fix.
>
>> 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.
Right.
>
>> }
>>
>> #ifdef CONFIG_X86_64
>> --
>> 1.6.0.2
>
> --
> Gleb.
Thanks,
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
