diff mbox

[v5,3/6] KVM : Calculate destination vcpu on interrupt injection

Message ID 1363862964-25148-4-git-send-email-yang.z.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang, Yang Z March 21, 2013, 10:49 a.m. UTC
From: Yang Zhang <yang.z.zhang@Intel.com>

Add a new parameter to know vcpus who received the interrupt.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
 arch/x86/kvm/lapic.h |    5 +++--
 virt/kvm/ioapic.c    |    2 +-
 virt/kvm/ioapic.h    |    2 +-
 virt/kvm/irq_comm.c  |   12 ++++++------
 5 files changed, 27 insertions(+), 15 deletions(-)

Comments

Gleb Natapov March 21, 2013, 11:48 a.m. UTC | #1
On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> Add a new parameter to know vcpus who received the interrupt.
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
>  arch/x86/kvm/lapic.h |    5 +++--
>  virt/kvm/ioapic.c    |    2 +-
>  virt/kvm/ioapic.h    |    2 +-
>  virt/kvm/irq_comm.c  |   12 ++++++------
>  5 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index d3e322a..5f6b1d0 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>  			     int vector, int level, int trig_mode);
>  
> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long *dest_map)
> +{
> +	if (!kvm_lapic_enabled(vcpu))
> +		return;
Why this check here?

> +	__set_bit(vcpu->vcpu_id, dest_map);
> +}
> +
> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> +		unsigned long *dest_map)
>  {
>  	struct kvm_lapic *apic = vcpu->arch.apic;
>  
> +	if (dest_map)
> +		kvm_set_irq_dest_map(vcpu, dest_map);
> +
>  	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
>  			irq->level, irq->trig_mode);
>  }
> @@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>  }
>  
>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq, int *r)
> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
>  {
>  	struct kvm_apic_map *map;
>  	unsigned long bitmap = 1;
> @@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>  	*r = -1;
>  
>  	if (irq->shorthand == APIC_DEST_SELF) {
> -		*r = kvm_apic_set_irq(src->vcpu, irq);
> +		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
>  		return true;
>  	}
>  
> @@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>  			continue;
>  		if (*r < 0)
>  			*r = 0;
> -		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
> +		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
>  	}
>  
>  	ret = true;
> @@ -852,7 +863,7 @@ static void apic_send_ipi(struct kvm_lapic *apic)
>  		   irq.trig_mode, irq.level, irq.dest_mode, irq.delivery_mode,
>  		   irq.vector);
>  
> -	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
> +	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
>  }
>  
>  static u32 apic_get_tmcct(struct kvm_lapic *apic)
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 2c721b9..967519c 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
>  
>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
>  int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> +		unsigned long *dest_map);
>  int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
>  
>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq, int *r);
> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
>  
>  u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
>  void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index ed6f111..4767fa6 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
>  	irqe.level = 1;
>  	irqe.shorthand = 0;
>  
> -	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
> +	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
>  }
>  
>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
> index 6e5c88f..14e5289 100644
> --- a/virt/kvm/ioapic.h
> +++ b/virt/kvm/ioapic.h
> @@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
>  void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq);
> +		struct kvm_lapic_irq *irq, unsigned long *dest_map);
>  int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>  int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>  void kvm_ioapic_make_eoibitmap_request(struct kvm *kvm);
> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> index e9073cf..2f07d2e 100644
> --- a/virt/kvm/irq_comm.c
> +++ b/virt/kvm/irq_comm.c
> @@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq)
>  }
>  
>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq)
> +		struct kvm_lapic_irq *irq, unsigned long *dest_map)
>  {
>  	int i, r = -1;
>  	struct kvm_vcpu *vcpu, *lowest = NULL;
> @@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  		irq->delivery_mode = APIC_DM_FIXED;
>  	}
>  
> -	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
> +	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
>  		return r;
>  
>  	kvm_for_each_vcpu(i, vcpu, kvm) {
> @@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  		if (!kvm_is_dm_lowest_prio(irq)) {
>  			if (r < 0)
>  				r = 0;
> -			r += kvm_apic_set_irq(vcpu, irq);
> +			r += kvm_apic_set_irq(vcpu, irq, dest_map);
>  		} else if (kvm_lapic_enabled(vcpu)) {
>  			if (!lowest)
>  				lowest = vcpu;
> @@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  	}
>  
>  	if (lowest)
> -		r = kvm_apic_set_irq(lowest, irq);
> +		r = kvm_apic_set_irq(lowest, irq, dest_map);
>  
>  	return r;
>  }
> @@ -130,7 +130,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
>  
>  	kvm_set_msi_irq(e, &irq);
>  
> -	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
> +	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
>  }
>  
>  
> @@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct kvm_kernel_irq_routing_entry *e,
>  
>  	kvm_set_msi_irq(e, &irq);
>  
> -	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
> +	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
>  		return r;
>  	else
>  		return -EWOULDBLOCK;
> -- 
> 1.7.1

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang, Yang Z March 21, 2013, 11:56 a.m. UTC | #2
Gleb Natapov wrote on 2013-03-21:
> On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> Add a new parameter to know vcpus who received the interrupt.
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> ---
>>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
>>  arch/x86/kvm/lapic.h |    5 +++--
>>  virt/kvm/ioapic.c    |    2 +-
>>  virt/kvm/ioapic.h    |    2 +-
>>  virt/kvm/irq_comm.c  |   12 ++++++------
>>  5 files changed, 27 insertions(+), 15 deletions(-)
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index d3e322a..5f6b1d0 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
> *vcpu)
>>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>>  			     int vector, int level, int trig_mode);
>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
>> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long
>> *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu)) +		return;
> Why this check here?
The vcpu who didn't enable apic should not account as destination vcpu. 
Without this check, if broadcast interrupt, all cpus will treat as destination vcpu, but only those who enabled apic will receive the interrupt.
There are same check in __apic_accept_irq():
if (unlikely(!apic_enabled(apic)))
     break;

>> +	__set_bit(vcpu->vcpu_id, dest_map);
>> +}
>> +
>> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
>> +		unsigned long *dest_map)
>>  {
>>  	struct kvm_lapic *apic = vcpu->arch.apic;
>> +	if (dest_map)
>> +		kvm_set_irq_dest_map(vcpu, dest_map);
>> +
>>  	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
>>  			irq->level, irq->trig_mode);
>>  }
>> @@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu,
> struct kvm_lapic *source,
>>  }
>>  
>>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>> -		struct kvm_lapic_irq *irq, int *r)
>> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
>>  {
>>  	struct kvm_apic_map *map;
>>  	unsigned long bitmap = 1;
>> @@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> struct kvm_lapic *src,
>>  	*r = -1;
>>  
>>  	if (irq->shorthand == APIC_DEST_SELF) {
>> -		*r = kvm_apic_set_irq(src->vcpu, irq);
>> +		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
>>  		return true;
>>  	}
>> @@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> struct kvm_lapic *src,
>>  			continue;
>>  		if (*r < 0)
>>  			*r = 0;
>> -		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
>> +		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
>>  	}
>>  
>>  	ret = true; @@ -852,7 +863,7 @@ static void apic_send_ipi(struct
>>  kvm_lapic *apic) 		   irq.trig_mode, irq.level, irq.dest_mode,
>>  irq.delivery_mode, 		   irq.vector);
>> -	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
>> +	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
>>  }
>>  
>>  static u32 apic_get_tmcct(struct kvm_lapic *apic)
>> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
>> index 2c721b9..967519c 100644
>> --- a/arch/x86/kvm/lapic.h
>> +++ b/arch/x86/kvm/lapic.h
>> @@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
>> 
>>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
>>  int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
>> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
>> +		unsigned long *dest_map);
>>  int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
>>  
>>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>> -		struct kvm_lapic_irq *irq, int *r);
>> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
>> 
>>  u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
>>  void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
>> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
>> index ed6f111..4767fa6 100644
>> --- a/virt/kvm/ioapic.c
>> +++ b/virt/kvm/ioapic.c
>> @@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int
> irq)
>>  	irqe.level = 1;
>>  	irqe.shorthand = 0;
>> -	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
>> +	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
>>  }
>>  
>>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
>> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
>> index 6e5c88f..14e5289 100644
>> --- a/virt/kvm/ioapic.h
>> +++ b/virt/kvm/ioapic.h
>> @@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq,
> int irq_source_id,
>>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
>>  void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
>>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>> -		struct kvm_lapic_irq *irq);
>> +		struct kvm_lapic_irq *irq, unsigned long *dest_map);
>>  int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>>  int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>>  void kvm_ioapic_make_eoibitmap_request(struct kvm *kvm);
>> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
>> index e9073cf..2f07d2e 100644
>> --- a/virt/kvm/irq_comm.c
>> +++ b/virt/kvm/irq_comm.c
>> @@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct
> kvm_lapic_irq *irq)
>>  }
>>  
>>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>> -		struct kvm_lapic_irq *irq)
>> +		struct kvm_lapic_irq *irq, unsigned long *dest_map)
>>  {
>>  	int i, r = -1;
>>  	struct kvm_vcpu *vcpu, *lowest = NULL;
>> @@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> kvm_lapic *src,
>>  		irq->delivery_mode = APIC_DM_FIXED;
>>  	}
>> -	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
>> +	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
>>  		return r;
>>  
>>  	kvm_for_each_vcpu(i, vcpu, kvm) {
>> @@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> kvm_lapic *src,
>>  		if (!kvm_is_dm_lowest_prio(irq)) {
>>  			if (r < 0)
>>  				r = 0;
>> -			r += kvm_apic_set_irq(vcpu, irq);
>> +			r += kvm_apic_set_irq(vcpu, irq, dest_map);
>>  		} else if (kvm_lapic_enabled(vcpu)) {
>>  			if (!lowest)
>>  				lowest = vcpu;
>> @@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> kvm_lapic *src,
>>  	}
>>  
>>  	if (lowest)
>> -		r = kvm_apic_set_irq(lowest, irq);
>> +		r = kvm_apic_set_irq(lowest, irq, dest_map);
>> 
>>  	return r;
>>  }
>> @@ -130,7 +130,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry
>> *e,
>> 
>>  	kvm_set_msi_irq(e, &irq);
>> -	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
>> +	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
>>  }
>> 
>> @@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct
>> kvm_kernel_irq_routing_entry *e,
>> 
>>  	kvm_set_msi_irq(e, &irq);
>> -	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
>> +	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
>>  		return r;
>>  	else
>>  		return -EWOULDBLOCK;
>> --
>> 1.7.1
> 
> --
> 			Gleb.


Best regards,
Yang


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov March 21, 2013, 12:03 p.m. UTC | #3
On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-03-21:
> > On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
> >> From: Yang Zhang <yang.z.zhang@Intel.com>
> >> 
> >> Add a new parameter to know vcpus who received the interrupt.
> >> 
> >> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> >> ---
> >>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
> >>  arch/x86/kvm/lapic.h |    5 +++--
> >>  virt/kvm/ioapic.c    |    2 +-
> >>  virt/kvm/ioapic.h    |    2 +-
> >>  virt/kvm/irq_comm.c  |   12 ++++++------
> >>  5 files changed, 27 insertions(+), 15 deletions(-)
> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> >> index d3e322a..5f6b1d0 100644
> >> --- a/arch/x86/kvm/lapic.c
> >> +++ b/arch/x86/kvm/lapic.c
> >> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
> > *vcpu)
> >>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> >>  			     int vector, int level, int trig_mode);
> >> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
> >> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long
> >> *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu)) +		return;
> > Why this check here?
> The vcpu who didn't enable apic should not account as destination vcpu. 
> Without this check, if broadcast interrupt, all cpus will treat as destination vcpu, but only those who enabled apic will receive the interrupt.
> There are same check in __apic_accept_irq():
> if (unlikely(!apic_enabled(apic)))
>      break;
I see, but you use more strict check that also checks that apic is
emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets
move bitmap update into __apic_accept_irq().

> 
> >> +	__set_bit(vcpu->vcpu_id, dest_map);
> >> +}
> >> +
> >> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> >> +		unsigned long *dest_map)
> >>  {
> >>  	struct kvm_lapic *apic = vcpu->arch.apic;
> >> +	if (dest_map)
> >> +		kvm_set_irq_dest_map(vcpu, dest_map);
> >> +
> >>  	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
> >>  			irq->level, irq->trig_mode);
> >>  }
> >> @@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu,
> > struct kvm_lapic *source,
> >>  }
> >>  
> >>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> >> -		struct kvm_lapic_irq *irq, int *r)
> >> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
> >>  {
> >>  	struct kvm_apic_map *map;
> >>  	unsigned long bitmap = 1;
> >> @@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> > struct kvm_lapic *src,
> >>  	*r = -1;
> >>  
> >>  	if (irq->shorthand == APIC_DEST_SELF) {
> >> -		*r = kvm_apic_set_irq(src->vcpu, irq);
> >> +		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
> >>  		return true;
> >>  	}
> >> @@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm,
> > struct kvm_lapic *src,
> >>  			continue;
> >>  		if (*r < 0)
> >>  			*r = 0;
> >> -		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
> >> +		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
> >>  	}
> >>  
> >>  	ret = true; @@ -852,7 +863,7 @@ static void apic_send_ipi(struct
> >>  kvm_lapic *apic) 		   irq.trig_mode, irq.level, irq.dest_mode,
> >>  irq.delivery_mode, 		   irq.vector);
> >> -	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
> >> +	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
> >>  }
> >>  
> >>  static u32 apic_get_tmcct(struct kvm_lapic *apic)
> >> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> >> index 2c721b9..967519c 100644
> >> --- a/arch/x86/kvm/lapic.h
> >> +++ b/arch/x86/kvm/lapic.h
> >> @@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
> >> 
> >>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
> >>  int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
> >> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
> >> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> >> +		unsigned long *dest_map);
> >>  int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
> >>  
> >>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> >> -		struct kvm_lapic_irq *irq, int *r);
> >> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
> >> 
> >>  u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
> >>  void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
> >> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> >> index ed6f111..4767fa6 100644
> >> --- a/virt/kvm/ioapic.c
> >> +++ b/virt/kvm/ioapic.c
> >> @@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int
> > irq)
> >>  	irqe.level = 1;
> >>  	irqe.shorthand = 0;
> >> -	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
> >> +	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
> >>  }
> >>  
> >>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
> >> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
> >> index 6e5c88f..14e5289 100644
> >> --- a/virt/kvm/ioapic.h
> >> +++ b/virt/kvm/ioapic.h
> >> @@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq,
> > int irq_source_id,
> >>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
> >>  void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
> >>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> >> -		struct kvm_lapic_irq *irq);
> >> +		struct kvm_lapic_irq *irq, unsigned long *dest_map);
> >>  int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
> >>  int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
> >>  void kvm_ioapic_make_eoibitmap_request(struct kvm *kvm);
> >> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> >> index e9073cf..2f07d2e 100644
> >> --- a/virt/kvm/irq_comm.c
> >> +++ b/virt/kvm/irq_comm.c
> >> @@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct
> > kvm_lapic_irq *irq)
> >>  }
> >>  
> >>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> >> -		struct kvm_lapic_irq *irq)
> >> +		struct kvm_lapic_irq *irq, unsigned long *dest_map)
> >>  {
> >>  	int i, r = -1;
> >>  	struct kvm_vcpu *vcpu, *lowest = NULL;
> >> @@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> > kvm_lapic *src,
> >>  		irq->delivery_mode = APIC_DM_FIXED;
> >>  	}
> >> -	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
> >> +	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
> >>  		return r;
> >>  
> >>  	kvm_for_each_vcpu(i, vcpu, kvm) {
> >> @@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> > kvm_lapic *src,
> >>  		if (!kvm_is_dm_lowest_prio(irq)) {
> >>  			if (r < 0)
> >>  				r = 0;
> >> -			r += kvm_apic_set_irq(vcpu, irq);
> >> +			r += kvm_apic_set_irq(vcpu, irq, dest_map);
> >>  		} else if (kvm_lapic_enabled(vcpu)) {
> >>  			if (!lowest)
> >>  				lowest = vcpu;
> >> @@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> > kvm_lapic *src,
> >>  	}
> >>  
> >>  	if (lowest)
> >> -		r = kvm_apic_set_irq(lowest, irq);
> >> +		r = kvm_apic_set_irq(lowest, irq, dest_map);
> >> 
> >>  	return r;
> >>  }
> >> @@ -130,7 +130,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry
> >> *e,
> >> 
> >>  	kvm_set_msi_irq(e, &irq);
> >> -	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
> >> +	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
> >>  }
> >> 
> >> @@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct
> >> kvm_kernel_irq_routing_entry *e,
> >> 
> >>  	kvm_set_msi_irq(e, &irq);
> >> -	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
> >> +	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
> >>  		return r;
> >>  	else
> >>  		return -EWOULDBLOCK;
> >> --
> >> 1.7.1
> > 
> > --
> > 			Gleb.
> 
> 
> Best regards,
> Yang
> 

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang, Yang Z March 21, 2013, 12:12 p.m. UTC | #4
Gleb Natapov wrote on 2013-03-21:
> On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote:
>> Gleb Natapov wrote on 2013-03-21:
>>> On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
>>>> From: Yang Zhang <yang.z.zhang@Intel.com>
>>>> 
>>>> Add a new parameter to know vcpus who received the interrupt.
>>>> 
>>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>>>> ---
>>>>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
>>>>  arch/x86/kvm/lapic.h |    5 +++--
>>>>  virt/kvm/ioapic.c    |    2 +-
>>>>  virt/kvm/ioapic.h    |    2 +-
>>>>  virt/kvm/irq_comm.c  |   12 ++++++------
>>>>  5 files changed, 27 insertions(+), 15 deletions(-)
>>>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>>>> index d3e322a..5f6b1d0 100644
>>>> --- a/arch/x86/kvm/lapic.c
>>>> +++ b/arch/x86/kvm/lapic.c
>>>> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
>>> *vcpu)
>>>>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>>>>  			     int vector, int level, int trig_mode);
>>>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
>>>> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long
>>>> *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu)) +		return;
>>> Why this check here?
>> The vcpu who didn't enable apic should not account as destination vcpu.
>> Without this check, if broadcast interrupt, all cpus will treat as
>> destination vcpu, but only those who enabled apic will receive the
>> interrupt. There are same check in __apic_accept_irq(): if
>> (unlikely(!apic_enabled(apic)))
>>      break;
> I see, but you use more strict check that also checks that apic is
> emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets
Do you mean the check add in here will block "userspace apic"? Shouldn't only in-kernel apic will get here?

> move bitmap update into __apic_accept_irq().
Sure.

> 
>> 
>>>> +	__set_bit(vcpu->vcpu_id, dest_map);
>>>> +}
>>>> +
>>>> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
>>>> +		unsigned long *dest_map)
>>>>  {
>>>>  	struct kvm_lapic *apic = vcpu->arch.apic;
>>>> +	if (dest_map)
>>>> +		kvm_set_irq_dest_map(vcpu, dest_map);
>>>> +
>>>>  	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
>>>>  			irq->level, irq->trig_mode);
>>>>  }
>>>> @@ -611,7 +622,7 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu,
>>> struct kvm_lapic *source,
>>>>  }
>>>>  
>>>>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic
> *src,
>>>> -		struct kvm_lapic_irq *irq, int *r)
>>>> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
>>>>  {
>>>>  	struct kvm_apic_map *map;
>>>>  	unsigned long bitmap = 1;
>>>> @@ -622,7 +633,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm
> *kvm,
>>> struct kvm_lapic *src,
>>>>  	*r = -1;
>>>>  
>>>>  	if (irq->shorthand == APIC_DEST_SELF) {
>>>> -		*r = kvm_apic_set_irq(src->vcpu, irq);
>>>> +		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
>>>>  		return true;
>>>>  	}
>>>> @@ -667,7 +678,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm
> *kvm,
>>> struct kvm_lapic *src,
>>>>  			continue;
>>>>  		if (*r < 0)
>>>>  			*r = 0;
>>>> -		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
>>>> +		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
>>>>  	}
>>>>  
>>>>  	ret = true; @@ -852,7 +863,7 @@ static void apic_send_ipi(struct
>>>>  kvm_lapic *apic) 		   irq.trig_mode, irq.level, irq.dest_mode,
>>>>  irq.delivery_mode, 		   irq.vector);
>>>> -	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
>>>> +	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
>>>>  }
>>>>  
>>>>  static u32 apic_get_tmcct(struct kvm_lapic *apic)
>>>> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
>>>> index 2c721b9..967519c 100644
>>>> --- a/arch/x86/kvm/lapic.h
>>>> +++ b/arch/x86/kvm/lapic.h
>>>> @@ -55,11 +55,12 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
>>>> 
>>>>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
>>>>  int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
>>>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
>>>> +int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
>>>> +		unsigned long *dest_map);
>>>>  int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
>>>>  
>>>>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic
> *src,
>>>> -		struct kvm_lapic_irq *irq, int *r);
>>>> +		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
>>>> 
>>>>  u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
>>>>  void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
>>>> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
>>>> index ed6f111..4767fa6 100644
>>>> --- a/virt/kvm/ioapic.c
>>>> +++ b/virt/kvm/ioapic.c
>>>> @@ -217,7 +217,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int
>>> irq)
>>>>  	irqe.level = 1;
>>>>  	irqe.shorthand = 0;
>>>> -	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
>>>> +	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
>>>>  }
>>>>  
>>>>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
>>>> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
>>>> index 6e5c88f..14e5289 100644
>>>> --- a/virt/kvm/ioapic.h
>>>> +++ b/virt/kvm/ioapic.h
>>>> @@ -88,7 +88,7 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int
> irq,
>>> int irq_source_id,
>>>>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
>>>>  void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
>>>>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>>>> -		struct kvm_lapic_irq *irq);
>>>> +		struct kvm_lapic_irq *irq, unsigned long *dest_map);
>>>>  int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>>>>  int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>>>>  void kvm_ioapic_make_eoibitmap_request(struct kvm *kvm);
>>>> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
>>>> index e9073cf..2f07d2e 100644
>>>> --- a/virt/kvm/irq_comm.c
>>>> +++ b/virt/kvm/irq_comm.c
>>>> @@ -63,7 +63,7 @@ inline static bool kvm_is_dm_lowest_prio(struct
>>> kvm_lapic_irq *irq)
>>>>  }
>>>>  
>>>>  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>>>> -		struct kvm_lapic_irq *irq)
>>>> +		struct kvm_lapic_irq *irq, unsigned long *dest_map)
>>>>  {
>>>>  	int i, r = -1;
>>>>  	struct kvm_vcpu *vcpu, *lowest = NULL;
>>>> @@ -74,7 +74,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
>>> kvm_lapic *src,
>>>>  		irq->delivery_mode = APIC_DM_FIXED;
>>>>  	}
>>>> -	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
>>>> +	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
>>>>  		return r;
>>>>  
>>>>  	kvm_for_each_vcpu(i, vcpu, kvm) {
>>>> @@ -88,7 +88,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
>>> kvm_lapic *src,
>>>>  		if (!kvm_is_dm_lowest_prio(irq)) {
>>>>  			if (r < 0)
>>>>  				r = 0;
>>>> -			r += kvm_apic_set_irq(vcpu, irq);
>>>> +			r += kvm_apic_set_irq(vcpu, irq, dest_map);
>>>>  		} else if (kvm_lapic_enabled(vcpu)) {
>>>>  			if (!lowest)
>>>>  				lowest = vcpu;
>>>> @@ -98,7 +98,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
>>> kvm_lapic *src,
>>>>  	}
>>>>  
>>>>  	if (lowest)
>>>> -		r = kvm_apic_set_irq(lowest, irq);
>>>> +		r = kvm_apic_set_irq(lowest, irq, dest_map);
>>>> 
>>>>  	return r;
>>>>  }
>>>> @@ -130,7 +130,7 @@ int kvm_set_msi(struct
>>>> kvm_kernel_irq_routing_entry *e,
>>>> 
>>>>  	kvm_set_msi_irq(e, &irq);
>>>> -	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
>>>> +	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
>>>>  }
>>>> @@ -142,7 +142,7 @@ static int kvm_set_msi_inatomic(struct
>>>> kvm_kernel_irq_routing_entry *e,
>>>> 
>>>>  	kvm_set_msi_irq(e, &irq);
>>>> -	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
>>>> +	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
>>>>  		return r;
>>>>  	else
>>>>  		return -EWOULDBLOCK;
>>>> --
>>>> 1.7.1
>>> 
>>> --
>>> 			Gleb.
>> 
>> 
>> Best regards,
>> Yang
>> 
> 
> --
> 			Gleb.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Best regards,
Yang


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov March 21, 2013, 12:21 p.m. UTC | #5
On Thu, Mar 21, 2013 at 12:12:06PM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-03-21:
> > On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote:
> >> Gleb Natapov wrote on 2013-03-21:
> >>> On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
> >>>> From: Yang Zhang <yang.z.zhang@Intel.com>
> >>>> 
> >>>> Add a new parameter to know vcpus who received the interrupt.
> >>>> 
> >>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> >>>> ---
> >>>>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
> >>>>  arch/x86/kvm/lapic.h |    5 +++--
> >>>>  virt/kvm/ioapic.c    |    2 +-
> >>>>  virt/kvm/ioapic.h    |    2 +-
> >>>>  virt/kvm/irq_comm.c  |   12 ++++++------
> >>>>  5 files changed, 27 insertions(+), 15 deletions(-)
> >>>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> >>>> index d3e322a..5f6b1d0 100644
> >>>> --- a/arch/x86/kvm/lapic.c
> >>>> +++ b/arch/x86/kvm/lapic.c
> >>>> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
> >>> *vcpu)
> >>>>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> >>>>  			     int vector, int level, int trig_mode);
> >>>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
> >>>> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long
> >>>> *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu)) +		return;
> >>> Why this check here?
> >> The vcpu who didn't enable apic should not account as destination vcpu.
> >> Without this check, if broadcast interrupt, all cpus will treat as
> >> destination vcpu, but only those who enabled apic will receive the
> >> interrupt. There are same check in __apic_accept_irq(): if
> >> (unlikely(!apic_enabled(apic)))
> >>      break;
> > I see, but you use more strict check that also checks that apic is
> > emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets
> Do you mean the check add in here will block "userspace apic"? Shouldn't only in-kernel apic will get here?
> 
No, it will not block. It checks for in kernel apic needlessly. Since we
patch all those checks out anyway using jump labels it is not really
affects performance, but I prefer to make only necessary checks for
consistency.

> > move bitmap update into __apic_accept_irq().
> Sure.
> 

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang, Yang Z March 21, 2013, 12:25 p.m. UTC | #6
Gleb Natapov wrote on 2013-03-21:
> On Thu, Mar 21, 2013 at 12:12:06PM +0000, Zhang, Yang Z wrote:
>> Gleb Natapov wrote on 2013-03-21:
>>> On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote:
>>>> Gleb Natapov wrote on 2013-03-21:
>>>>> On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote:
>>>>>> From: Yang Zhang <yang.z.zhang@Intel.com>
>>>>>> 
>>>>>> Add a new parameter to know vcpus who received the interrupt.
>>>>>> 
>>>>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>>>>>> ---
>>>>>>  arch/x86/kvm/lapic.c |   21 ++++++++++++++++-----
>>>>>>  arch/x86/kvm/lapic.h |    5 +++--
>>>>>>  virt/kvm/ioapic.c    |    2 +-
>>>>>>  virt/kvm/ioapic.h    |    2 +-
>>>>>>  virt/kvm/irq_comm.c  |   12 ++++++------
>>>>>>  5 files changed, 27 insertions(+), 15 deletions(-)
>>>>>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>>>>>> index d3e322a..5f6b1d0 100644
>>>>>> --- a/arch/x86/kvm/lapic.c
>>>>>> +++ b/arch/x86/kvm/lapic.c
>>>>>> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu
>>>>> *vcpu)
>>>>>>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>>>>>>  			     int vector, int level, int trig_mode);
>>>>>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq
>>>>>> *irq) +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu,
>>>>>> unsigned long *dest_map) +{ +	if (!kvm_lapic_enabled(vcpu))
>>>>>> +		return;
>>>>> Why this check here?
>>>> The vcpu who didn't enable apic should not account as destination vcpu.
>>>> Without this check, if broadcast interrupt, all cpus will treat as
>>>> destination vcpu, but only those who enabled apic will receive the
>>>> interrupt. There are same check in __apic_accept_irq(): if
>>>> (unlikely(!apic_enabled(apic)))
>>>>      break;
>>> I see, but you use more strict check that also checks that apic is
>>> emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets
>> Do you mean the check add in here will block "userspace apic"?
>> Shouldn't only in-kernel apic will get here?
>> 
> No, it will not block. It checks for in kernel apic needlessly. Since we
> patch all those checks out anyway using jump labels it is not really
> affects performance, but I prefer to make only necessary checks for
> consistency.
Make sense.
 
>>> move bitmap update into __apic_accept_irq().
>> Sure.
>> 
> 
> --
> 			Gleb.


Best regards,
Yang


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d3e322a..5f6b1d0 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -433,10 +433,21 @@  int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
 static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 			     int vector, int level, int trig_mode);
 
-int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
+static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long *dest_map)
+{
+	if (!kvm_lapic_enabled(vcpu))
+		return;
+	__set_bit(vcpu->vcpu_id, dest_map);
+}
+
+int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
+		unsigned long *dest_map)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
+	if (dest_map)
+		kvm_set_irq_dest_map(vcpu, dest_map);
+
 	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
 			irq->level, irq->trig_mode);
 }
@@ -611,7 +622,7 @@  int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 }
 
 bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq, int *r)
+		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
 {
 	struct kvm_apic_map *map;
 	unsigned long bitmap = 1;
@@ -622,7 +633,7 @@  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 	*r = -1;
 
 	if (irq->shorthand == APIC_DEST_SELF) {
-		*r = kvm_apic_set_irq(src->vcpu, irq);
+		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
 		return true;
 	}
 
@@ -667,7 +678,7 @@  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 			continue;
 		if (*r < 0)
 			*r = 0;
-		*r += kvm_apic_set_irq(dst[i]->vcpu, irq);
+		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
 	}
 
 	ret = true;
@@ -852,7 +863,7 @@  static void apic_send_ipi(struct kvm_lapic *apic)
 		   irq.trig_mode, irq.level, irq.dest_mode, irq.delivery_mode,
 		   irq.vector);
 
-	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq);
+	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
 }
 
 static u32 apic_get_tmcct(struct kvm_lapic *apic)
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 2c721b9..967519c 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -55,11 +55,12 @@  void kvm_apic_set_version(struct kvm_vcpu *vcpu);
 
 int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
 int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
-int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq);
+int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
+		unsigned long *dest_map);
 int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
 
 bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq, int *r);
+		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
 
 u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
 void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index ed6f111..4767fa6 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -217,7 +217,7 @@  static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq)
 	irqe.level = 1;
 	irqe.shorthand = 0;
 
-	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
+	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
 }
 
 int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index 6e5c88f..14e5289 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -88,7 +88,7 @@  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
 void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
 void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
 int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq);
+		struct kvm_lapic_irq *irq, unsigned long *dest_map);
 int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
 int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
 void kvm_ioapic_make_eoibitmap_request(struct kvm *kvm);
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index e9073cf..2f07d2e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -63,7 +63,7 @@  inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq)
 }
 
 int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq)
+		struct kvm_lapic_irq *irq, unsigned long *dest_map)
 {
 	int i, r = -1;
 	struct kvm_vcpu *vcpu, *lowest = NULL;
@@ -74,7 +74,7 @@  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 		irq->delivery_mode = APIC_DM_FIXED;
 	}
 
-	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r))
+	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
 		return r;
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
@@ -88,7 +88,7 @@  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 		if (!kvm_is_dm_lowest_prio(irq)) {
 			if (r < 0)
 				r = 0;
-			r += kvm_apic_set_irq(vcpu, irq);
+			r += kvm_apic_set_irq(vcpu, irq, dest_map);
 		} else if (kvm_lapic_enabled(vcpu)) {
 			if (!lowest)
 				lowest = vcpu;
@@ -98,7 +98,7 @@  int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 	}
 
 	if (lowest)
-		r = kvm_apic_set_irq(lowest, irq);
+		r = kvm_apic_set_irq(lowest, irq, dest_map);
 
 	return r;
 }
@@ -130,7 +130,7 @@  int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 
 	kvm_set_msi_irq(e, &irq);
 
-	return kvm_irq_delivery_to_apic(kvm, NULL, &irq);
+	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
 }
 
 
@@ -142,7 +142,7 @@  static int kvm_set_msi_inatomic(struct kvm_kernel_irq_routing_entry *e,
 
 	kvm_set_msi_irq(e, &irq);
 
-	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r))
+	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
 		return r;
 	else
 		return -EWOULDBLOCK;