Re: [PATCH 8/9] Add VMEXIT handler and intercepts v2

Alexander Graf
2008-09-15T16:11:23Z

On Sep 10, 2008, at 9:12 PM, Joerg Roedel wrote:

> On Fri, Sep 05, 2008 at 09:51:23AM +0200, Alexander Graf wrote:
>> This adds the #VMEXIT intercept, so we return to the level 1 guest
>> when something happens in the level 2 guest that should return to
>> the level 1 guest.
>>
>> v2 implements HIF handling and cleans up exception interception
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> arch/x86/kvm/svm.c | 319 ++++++++++++++++++++++++++++++++++++++++++
>> ++++++++++
>> 1 files changed, 319 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index c47f039..8318a63 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -74,6 +74,13 @@ module_param(npt, int, S_IRUGO);
>> static void kvm_reput_irq(struct vcpu_svm *svm);
>> static void svm_flush_tlb(struct kvm_vcpu *vcpu);
>>
>> +static int nested_svm_exit_handled(struct vcpu_svm *svm, bool
>> kvm_override);
>> +static int nested_svm_vmexit(struct vcpu_svm *svm);
>> +static int nested_svm_vmsave(struct vcpu_svm *svm, void
>> *nested_vmcb,
>> + void *arg2, void *opaque);
>> +static int nested_svm_check_exception(struct vcpu_svm *svm,
>> unsigned nr,
>> + bool has_error_code, u32 error_code);
>> +
>> static inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu)
>> {
>> return container_of(vcpu, struct vcpu_svm, vcpu);
>> @@ -223,6 +230,11 @@ static void svm_queue_exception(struct
>> kvm_vcpu *vcpu, unsigned nr,
>> {
>> 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)
>> @@ -1185,6 +1197,43 @@ static int vmmcall_interception(struct
>> vcpu_svm *svm, struct kvm_run *kvm_run)
>> return 1;
>> }
>>
>> +static int nested_svm_check_exception(struct vcpu_svm *svm,
>> unsigned nr,
>> + bool has_error_code, u32 error_code)
>> +{
>> + if (is_nested(svm)) {
>> + svm->vmcb->control.exit_code = SVM_EXIT_EXCP_BASE + nr;
>> + svm->vmcb->control.exit_code_hi = 0;
>> + svm->vmcb->control.exit_info_1 = error_code;
>> + svm->vmcb->control.exit_info_2 = svm->vcpu.arch.cr2;
>> + if (nested_svm_exit_handled(svm, false)) {
>> + nsvm_printk("VMexit -> EXCP 0x%x\n", nr);
>> +
>> + nested_svm_vmexit(svm);
>> + return 1;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static inline int nested_svm_intr(struct vcpu_svm *svm)
>> +{
>> + if (is_nested(svm)) {
>> + if (!(svm->vcpu.arch.hflags & HF_HIF_MASK))
>
> HF_HIF_MASK?
> Do you really need to check GIF if the vcpu is in guest mode? The
> guest
> itself can't influence the GIF so it should always be set.

HIF is not GIF ;-). The HIF is the hidden interrupt flag, aka the IF
flag the host had when it started VMRUN.

>
>
>> + return 0;
>> +
>> + svm->vmcb->control.exit_code = SVM_EXIT_INTR;
>> +
>> + if (nested_svm_exit_handled(svm, false)) {
>> + nsvm_printk("VMexit -> INTR\n");
>> + nested_svm_vmexit(svm);
>> + return 1;
>> + }
>
> The VMEXIT is only required if the guest vmcb has set V_INTR_MASKING.
> Otherwise we can inject the interrupt directly into the l2 guest. This
> is no problem for running KVM-in-KVM because KVM always sets
> V_INTR_MASKING. But to keep the implementation close to real hardware
> behavior this bit should be checked.

Right now I always force V_INTR_MASK to on in VMRUN, so we don't have
to look at that case. It might be a good idea to simply implement the
behavior though ...

[snip]

Alex