Re: [PATCH 2/3] Consolidate userspace and kernel interrupt injection for VMX.

[Gleb Natapov <gleb@redhat.com>]

On Sat, Apr 11, 2009 at 02:29:28PM +0300, Avi Kivity wrote:
> Gleb Natapov wrote:
>> Use the same callback to inject irq/nmi events no matter what irqchip is
>> in use. Only from VMX for now.
>>
>> Signed-off-by: Gleb Natapov <gleb@redhat.com>
>> ---
>>
>>  arch/x86/include/asm/kvm_host.h |    2 +
>>  arch/x86/kvm/svm.c              |    2 +
>>  arch/x86/kvm/vmx.c              |   71 +++++++++------------------------------
>>  arch/x86/kvm/x86.c              |    2 +
>>  4 files changed, 19 insertions(+), 58 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index e672ca5..4e39c40 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -520,7 +520,7 @@ struct kvm_x86_ops {
>>  	void (*queue_exception)(struct kvm_vcpu *vcpu, unsigned nr,
>>  				bool has_error_code, u32 error_code);
>>  	bool (*exception_injected)(struct kvm_vcpu *vcpu);
>> -	void (*inject_pending_irq)(struct kvm_vcpu *vcpu);
>> +	void (*inject_pending_irq)(struct kvm_vcpu *vcpu, struct kvm_run *run);
>>   
>
> kvm_run is available as vcpu->run, so this isn't needed.  But better to  
> keep it for now and drop it in a later patch.
>
>>  static int vmx_set_tss_addr(struct kvm *kvm, unsigned int addr)
>>  {
>>  	int ret;
>> @@ -3351,8 +3309,11 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx)
>>  	}
>>  }
>>  -static void vmx_intr_assist(struct kvm_vcpu *vcpu)
>> +static void vmx_intr_assist(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>  {
>> +	bool req_int_win = !irqchip_in_kernel(vcpu->kvm) &&
>> +		kvm_run->request_interrupt_window;
>> +
>>  	update_tpr_threshold(vcpu);
>>   	vmx_update_window_states(vcpu);
>> @@ -3373,25 +3334,25 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
>>  			return;
>>  		}
>>  	}
>>   
>
> Why not convert the 'enable_nmi_window(); return;' above to a 'goto out'  
> like you do elsewhere?
>
>
Mainly because of the next patch in the series that reworks this code
anyway.

--
			Gleb.