kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] KVM: x86: some apic broadcast modes does not work
@ 2014-10-02 21:30 Nadav Amit
  2014-10-03 12:49 ` Radim Krčmář
  2014-10-08  3:01 ` Wanpeng Li
  0 siblings, 2 replies; 6+ messages in thread
From: Nadav Amit @ 2014-10-02 21:30 UTC (permalink / raw)
  To: pbonzini; +Cc: rkrcmar, kvm, nadav.amit, Nadav Amit

KVM does not deliver x2APIC broadcast messages with physical mode.  Intel SDM
(10.12.9 ICR Operation in x2APIC Mode) states: "A destination ID value of
FFFF_FFFFH is used for broadcast of interrupts in both logical destination and
physical destination modes."

In addition, the local-apic enables cluster mode broadcast. As Intel SDM
10.6.2.2 says: "Broadcast to all local APICs is achieved by setting all
destination bits to one." This patch enables cluster mode broadcast.

The fix tries to combine broadcast in different modes through a unified code.

One rare case occurs when the source of IPI has its APIC disabled.  In such
case, the source can still issue IPIs, but since the source is not obliged to
have the same LAPIC mode as the enabled ones, we cannot rely on it.
Since it is a rare case, it is unoptimized and done on the slow-path.

---

Changes v1->v2:
- Follow Radim's review: setting constants, preferring simplicity to marginal
  performance gain, etc.
- Combine the cluster mode and x2apic mode patches

Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
---
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/lapic.c            | 28 ++++++++++++++++++++++------
 arch/x86/kvm/lapic.h            |  4 ++--
 virt/kvm/ioapic.h               |  2 +-
 4 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 7d603a7..6f2be83 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -542,7 +542,7 @@ struct kvm_apic_map {
 	struct rcu_head rcu;
 	u8 ldr_bits;
 	/* fields bellow are used to decode ldr values in different modes */
-	u32 cid_shift, cid_mask, lid_mask;
+	u32 cid_shift, cid_mask, lid_mask, broadcast;
 	struct kvm_lapic *phys_map[256];
 	/* first index is cluster id second is cpu id in a cluster */
 	struct kvm_lapic *logical_map[16][16];
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index b8345dd..ee04adf 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -68,6 +68,9 @@
 #define MAX_APIC_VECTOR			256
 #define APIC_VECTORS_PER_REG		32
 
+#define APIC_BROADCAST			0xFF
+#define X2APIC_BROADCAST		0xFFFFFFFFul
+
 #define VEC_POS(v) ((v) & (32 - 1))
 #define REG_POS(v) (((v) >> 5) << 4)
 
@@ -149,6 +152,7 @@ static void recalculate_apic_map(struct kvm *kvm)
 	new->cid_shift = 8;
 	new->cid_mask = 0;
 	new->lid_mask = 0xff;
+	new->broadcast = APIC_BROADCAST;
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
 		struct kvm_lapic *apic = vcpu->arch.apic;
@@ -170,6 +174,7 @@ static void recalculate_apic_map(struct kvm *kvm)
 			new->cid_shift = 16;
 			new->cid_mask = (1 << KVM_X2APIC_CID_BITS) - 1;
 			new->lid_mask = 0xffff;
+			new->broadcast = X2APIC_BROADCAST;
 		} else if (kvm_apic_sw_enabled(apic) &&
 				!new->cid_mask /* flat mode */ &&
 				kvm_apic_get_reg(apic, APIC_DFR) == APIC_DFR_CLUSTER) {
@@ -558,16 +563,25 @@ static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
 	apic_update_ppr(apic);
 }
 
-int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
+static int kvm_apic_broadcast(struct kvm_lapic *apic, u32 dest)
+{
+	return dest == (apic_x2apic_mode(apic) ?
+			X2APIC_BROADCAST : APIC_BROADCAST);
+}
+
+int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u32 dest)
 {
-	return dest == 0xff || kvm_apic_id(apic) == dest;
+	return kvm_apic_id(apic) == dest || kvm_apic_broadcast(apic, dest);
 }
 
-int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda)
+int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda)
 {
 	int result = 0;
 	u32 logical_id;
 
+	if (kvm_apic_broadcast(apic, mda))
+		return 1;
+
 	if (apic_x2apic_mode(apic)) {
 		logical_id = kvm_apic_get_reg(apic, APIC_LDR);
 		return logical_id & mda;
@@ -595,7 +609,7 @@ int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda)
 }
 
 int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
-			   int short_hand, int dest, int dest_mode)
+			   int short_hand, unsigned int dest, int dest_mode)
 {
 	int result = 0;
 	struct kvm_lapic *target = vcpu->arch.apic;
@@ -657,9 +671,11 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 	if (!map)
 		goto out;
 
+	if (irq->dest_id == map->broadcast)
+		goto out;
+
 	if (irq->dest_mode == 0) { /* physical mode */
-		if (irq->delivery_mode == APIC_DM_LOWEST ||
-				irq->dest_id == 0xff)
+		if (irq->delivery_mode == APIC_DM_LOWEST)
 			goto out;
 		dst = &map->phys_map[irq->dest_id & 0xff];
 	} else {
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 6a11845..3fd6c22 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -55,8 +55,8 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
 
 void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
 void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
-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_match_physical_addr(struct kvm_lapic *apic, u32 dest);
+int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda);
 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);
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index e23b706..31725a3 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -83,7 +83,7 @@ static inline struct kvm_ioapic *ioapic_irqchip(struct kvm *kvm)
 
 void kvm_rtc_eoi_tracking_restore_one(struct kvm_vcpu *vcpu);
 int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
-		int short_hand, int dest, int dest_mode);
+		int short_hand, unsigned int dest, int dest_mode);
 int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2);
 void kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu, int vector,
 			int trigger_mode);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] KVM: x86: some apic broadcast modes does not work
  2014-10-02 21:30 [PATCH v2] KVM: x86: some apic broadcast modes does not work Nadav Amit
@ 2014-10-03 12:49 ` Radim Krčmář
  2014-10-06 15:29   ` Nadav Amit
  2014-10-08  3:01 ` Wanpeng Li
  1 sibling, 1 reply; 6+ messages in thread
From: Radim Krčmář @ 2014-10-03 12:49 UTC (permalink / raw)
  To: Nadav Amit; +Cc: pbonzini, kvm, nadav.amit

2014-10-03 00:30+0300, Nadav Amit:
> KVM does not deliver x2APIC broadcast messages with physical mode.  Intel SDM
> (10.12.9 ICR Operation in x2APIC Mode) states: "A destination ID value of
> FFFF_FFFFH is used for broadcast of interrupts in both logical destination and
> physical destination modes."
> 
> In addition, the local-apic enables cluster mode broadcast. As Intel SDM
> 10.6.2.2 says: "Broadcast to all local APICs is achieved by setting all
> destination bits to one." This patch enables cluster mode broadcast.
> 
> The fix tries to combine broadcast in different modes through a unified code.
> 
> One rare case occurs when the source of IPI has its APIC disabled.  In such
> case, the source can still issue IPIs, but since the source is not obliged to
> have the same LAPIC mode as the enabled ones, we cannot rely on it.
> Since it is a rare case, it is unoptimized and done on the slow-path.
> 
> ---

Thanks!

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

> Changes v1->v2:
> - Follow Radim's review: setting constants, preferring simplicity to marginal
>   performance gain, etc.
> - Combine the cluster mode and x2apic mode patches
> 
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index b8345dd..ee04adf 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -68,6 +68,9 @@
>  #define MAX_APIC_VECTOR			256
>  #define APIC_VECTORS_PER_REG		32
>  
> +#define APIC_BROADCAST			0xFF
> +#define X2APIC_BROADCAST		0xFFFFFFFFul

(int is better -- using long introduces an interesting feature with
 implicit retyping: (int)X2APIC_BROADCAST != X2APIC_BROADCAST, and we
 don't compile with -Wtype-limits to notice it.  It poses no problem
 now, so I can change it in an inevitable cleanup series / convince lkml
 to endorse stricter warnings.)

> +
>  #define VEC_POS(v) ((v) & (32 - 1))
>  #define REG_POS(v) (((v) >> 5) << 4)
>  
> @@ -558,16 +563,25 @@ static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
>  	apic_update_ppr(apic);
>  }
>  
> -int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
> +static int kvm_apic_broadcast(struct kvm_lapic *apic, u32 dest)

(bool is better.)

> +{
> +	return dest == (apic_x2apic_mode(apic) ?
> +			X2APIC_BROADCAST : APIC_BROADCAST);
> +}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] KVM: x86: some apic broadcast modes does not work
  2014-10-03 12:49 ` Radim Krčmář
@ 2014-10-06 15:29   ` Nadav Amit
  2014-10-06 19:08     ` Radim Krčmář
  0 siblings, 1 reply; 6+ messages in thread
From: Nadav Amit @ 2014-10-06 15:29 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: Nadav Amit, pbonzini, kvm

[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]


On Oct 3, 2014, at 3:49 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:

> 2014-10-03 00:30+0300, Nadav Amit:
>> KVM does not deliver x2APIC broadcast messages with physical mode.  Intel SDM
>> (10.12.9 ICR Operation in x2APIC Mode) states: "A destination ID value of
>> FFFF_FFFFH is used for broadcast of interrupts in both logical destination and
>> physical destination modes."
>> 
>> In addition, the local-apic enables cluster mode broadcast. As Intel SDM
>> 10.6.2.2 says: "Broadcast to all local APICs is achieved by setting all
>> destination bits to one." This patch enables cluster mode broadcast.
>> 
>> The fix tries to combine broadcast in different modes through a unified code.
>> 
>> One rare case occurs when the source of IPI has its APIC disabled.  In such
>> case, the source can still issue IPIs, but since the source is not obliged to
>> have the same LAPIC mode as the enabled ones, we cannot rely on it.
>> Since it is a rare case, it is unoptimized and done on the slow-path.
>> 
>> ---
> 
> Thanks!
> 
> Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> 
>> Changes v1->v2:
>> - Follow Radim's review: setting constants, preferring simplicity to marginal
>>  performance gain, etc.
>> - Combine the cluster mode and x2apic mode patches
>> 
>> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>> ---
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index b8345dd..ee04adf 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -68,6 +68,9 @@
>> #define MAX_APIC_VECTOR			256
>> #define APIC_VECTORS_PER_REG		32
>> 
>> +#define APIC_BROADCAST			0xFF
>> +#define X2APIC_BROADCAST		0xFFFFFFFFul
> 
> (int is better -- using long introduces an interesting feature with
> implicit retyping: (int)X2APIC_BROADCAST != X2APIC_BROADCAST, and we
> don't compile with -Wtype-limits to notice it.  It poses no problem
> now, so I can change it in an inevitable cleanup series / convince lkml
> to endorse stricter warnings.)
Would unsigned int ease your mind?

> 
>> +
>> #define VEC_POS(v) ((v) & (32 - 1))
>> #define REG_POS(v) (((v) >> 5) << 4)
>> 
>> @@ -558,16 +563,25 @@ static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
>> 	apic_update_ppr(apic);
>> }
>> 
>> -int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
>> +static int kvm_apic_broadcast(struct kvm_lapic *apic, u32 dest)
> 
> (bool is better.)
Ok. I will do it for v3.

Nadav

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] KVM: x86: some apic broadcast modes does not work
  2014-10-06 15:29   ` Nadav Amit
@ 2014-10-06 19:08     ` Radim Krčmář
  2014-10-08  8:38       ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Radim Krčmář @ 2014-10-06 19:08 UTC (permalink / raw)
  To: Nadav Amit; +Cc: Nadav Amit, pbonzini, kvm

2014-10-06 18:29+0300, Nadav Amit:
> On Oct 3, 2014, at 3:49 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> > 2014-10-03 00:30+0300, Nadav Amit:
> > Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> > 
> >> +#define X2APIC_BROADCAST		0xFFFFFFFFul
> > 
> > (int is better -- using long introduces an interesting feature with
> > implicit retyping: (int)X2APIC_BROADCAST != X2APIC_BROADCAST, and we
> > don't compile with -Wtype-limits to notice it.  It poses no problem
> > now, so I can change it in an inevitable cleanup series / convince lkml
> > to endorse stricter warnings.)
> Would unsigned int ease your mind?

That would be perfect.

> >> -int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
> >> +static int kvm_apic_broadcast(struct kvm_lapic *apic, u32 dest)
> > 
> > (bool is better.)
> Ok. I will do it for v3.

I'm fine with v2, which is why the nitpicking was in parentheses.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] KVM: x86: some apic broadcast modes does not work
  2014-10-02 21:30 [PATCH v2] KVM: x86: some apic broadcast modes does not work Nadav Amit
  2014-10-03 12:49 ` Radim Krčmář
@ 2014-10-08  3:01 ` Wanpeng Li
  1 sibling, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2014-10-08  3:01 UTC (permalink / raw)
  To: Nadav Amit; +Cc: pbonzini, rkrcmar, kvm, nadav.amit, Nadav Amit

On Fri, Oct 03, 2014 at 12:30:52AM +0300, Nadav Amit wrote:
>KVM does not deliver x2APIC broadcast messages with physical mode.  Intel SDM
>(10.12.9 ICR Operation in x2APIC Mode) states: "A destination ID value of
>FFFF_FFFFH is used for broadcast of interrupts in both logical destination and
>physical destination modes."
>
>In addition, the local-apic enables cluster mode broadcast. As Intel SDM
>10.6.2.2 says: "Broadcast to all local APICs is achieved by setting all
>destination bits to one." This patch enables cluster mode broadcast.
>
>The fix tries to combine broadcast in different modes through a unified code.
>
>One rare case occurs when the source of IPI has its APIC disabled.  In such
>case, the source can still issue IPIs, but since the source is not obliged to
>have the same LAPIC mode as the enabled ones, we cannot rely on it.
>Since it is a rare case, it is unoptimized and done on the slow-path.
>
>---
>
>Changes v1->v2:
>- Follow Radim's review: setting constants, preferring simplicity to marginal
>  performance gain, etc.
>- Combine the cluster mode and x2apic mode patches
>
>Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>---

Reviewed-by: Wanpeng Li <wanpeng.li@linux.intel.com>

> arch/x86/include/asm/kvm_host.h |  2 +-
> arch/x86/kvm/lapic.c            | 28 ++++++++++++++++++++++------
> arch/x86/kvm/lapic.h            |  4 ++--
> virt/kvm/ioapic.h               |  2 +-
> 4 files changed, 26 insertions(+), 10 deletions(-)
>
>diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>index 7d603a7..6f2be83 100644
>--- a/arch/x86/include/asm/kvm_host.h
>+++ b/arch/x86/include/asm/kvm_host.h
>@@ -542,7 +542,7 @@ struct kvm_apic_map {
> 	struct rcu_head rcu;
> 	u8 ldr_bits;
> 	/* fields bellow are used to decode ldr values in different modes */
>-	u32 cid_shift, cid_mask, lid_mask;
>+	u32 cid_shift, cid_mask, lid_mask, broadcast;
> 	struct kvm_lapic *phys_map[256];
> 	/* first index is cluster id second is cpu id in a cluster */
> 	struct kvm_lapic *logical_map[16][16];
>diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>index b8345dd..ee04adf 100644
>--- a/arch/x86/kvm/lapic.c
>+++ b/arch/x86/kvm/lapic.c
>@@ -68,6 +68,9 @@
> #define MAX_APIC_VECTOR			256
> #define APIC_VECTORS_PER_REG		32
> 
>+#define APIC_BROADCAST			0xFF
>+#define X2APIC_BROADCAST		0xFFFFFFFFul
>+
> #define VEC_POS(v) ((v) & (32 - 1))
> #define REG_POS(v) (((v) >> 5) << 4)
> 
>@@ -149,6 +152,7 @@ static void recalculate_apic_map(struct kvm *kvm)
> 	new->cid_shift = 8;
> 	new->cid_mask = 0;
> 	new->lid_mask = 0xff;
>+	new->broadcast = APIC_BROADCAST;
> 
> 	kvm_for_each_vcpu(i, vcpu, kvm) {
> 		struct kvm_lapic *apic = vcpu->arch.apic;
>@@ -170,6 +174,7 @@ static void recalculate_apic_map(struct kvm *kvm)
> 			new->cid_shift = 16;
> 			new->cid_mask = (1 << KVM_X2APIC_CID_BITS) - 1;
> 			new->lid_mask = 0xffff;
>+			new->broadcast = X2APIC_BROADCAST;
> 		} else if (kvm_apic_sw_enabled(apic) &&
> 				!new->cid_mask /* flat mode */ &&
> 				kvm_apic_get_reg(apic, APIC_DFR) == APIC_DFR_CLUSTER) {
>@@ -558,16 +563,25 @@ static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
> 	apic_update_ppr(apic);
> }
> 
>-int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
>+static int kvm_apic_broadcast(struct kvm_lapic *apic, u32 dest)
>+{
>+	return dest == (apic_x2apic_mode(apic) ?
>+			X2APIC_BROADCAST : APIC_BROADCAST);
>+}
>+
>+int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u32 dest)
> {
>-	return dest == 0xff || kvm_apic_id(apic) == dest;
>+	return kvm_apic_id(apic) == dest || kvm_apic_broadcast(apic, dest);
> }
> 
>-int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda)
>+int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda)
> {
> 	int result = 0;
> 	u32 logical_id;
> 
>+	if (kvm_apic_broadcast(apic, mda))
>+		return 1;
>+
> 	if (apic_x2apic_mode(apic)) {
> 		logical_id = kvm_apic_get_reg(apic, APIC_LDR);
> 		return logical_id & mda;
>@@ -595,7 +609,7 @@ int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda)
> }
> 
> int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>-			   int short_hand, int dest, int dest_mode)
>+			   int short_hand, unsigned int dest, int dest_mode)
> {
> 	int result = 0;
> 	struct kvm_lapic *target = vcpu->arch.apic;
>@@ -657,9 +671,11 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> 	if (!map)
> 		goto out;
> 
>+	if (irq->dest_id == map->broadcast)
>+		goto out;
>+
> 	if (irq->dest_mode == 0) { /* physical mode */
>-		if (irq->delivery_mode == APIC_DM_LOWEST ||
>-				irq->dest_id == 0xff)
>+		if (irq->delivery_mode == APIC_DM_LOWEST)
> 			goto out;
> 		dst = &map->phys_map[irq->dest_id & 0xff];
> 	} else {
>diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
>index 6a11845..3fd6c22 100644
>--- a/arch/x86/kvm/lapic.h
>+++ b/arch/x86/kvm/lapic.h
>@@ -55,8 +55,8 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu);
> 
> void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
> void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
>-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_match_physical_addr(struct kvm_lapic *apic, u32 dest);
>+int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda);
> 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);
>diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
>index e23b706..31725a3 100644
>--- a/virt/kvm/ioapic.h
>+++ b/virt/kvm/ioapic.h
>@@ -83,7 +83,7 @@ static inline struct kvm_ioapic *ioapic_irqchip(struct kvm *kvm)
> 
> void kvm_rtc_eoi_tracking_restore_one(struct kvm_vcpu *vcpu);
> int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>-		int short_hand, int dest, int dest_mode);
>+		int short_hand, unsigned int dest, int dest_mode);
> int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2);
> void kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu, int vector,
> 			int trigger_mode);
>-- 
>1.9.1
>
>--
>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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] KVM: x86: some apic broadcast modes does not work
  2014-10-06 19:08     ` Radim Krčmář
@ 2014-10-08  8:38       ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2014-10-08  8:38 UTC (permalink / raw)
  To: Radim Krčmář, Nadav Amit; +Cc: Nadav Amit, kvm

Il 06/10/2014 21:08, Radim Krčmář ha scritto:
> 2014-10-06 18:29+0300, Nadav Amit:
>> On Oct 3, 2014, at 3:49 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
>>> 2014-10-03 00:30+0300, Nadav Amit:
>>> Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
>>>
>>>> +#define X2APIC_BROADCAST		0xFFFFFFFFul
>>>
>>> (int is better -- using long introduces an interesting feature with
>>> implicit retyping: (int)X2APIC_BROADCAST != X2APIC_BROADCAST, and we
>>> don't compile with -Wtype-limits to notice it.  It poses no problem
>>> now, so I can change it in an inevitable cleanup series / convince lkml
>>> to endorse stricter warnings.)
>> Would unsigned int ease your mind?
> 
> That would be perfect.
> 
>>>> -int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
>>>> +static int kvm_apic_broadcast(struct kvm_lapic *apic, u32 dest)
>>>
>>> (bool is better.)
>> Ok. I will do it for v3.
> 
> I'm fine with v2, which is why the nitpicking was in parentheses.

I will fix up v2 with 0xFFFFFFFFu and static bool kvm_apic_broadcast.

Paolo

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-10-08  8:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-02 21:30 [PATCH v2] KVM: x86: some apic broadcast modes does not work Nadav Amit
2014-10-03 12:49 ` Radim Krčmář
2014-10-06 15:29   ` Nadav Amit
2014-10-06 19:08     ` Radim Krčmář
2014-10-08  8:38       ` Paolo Bonzini
2014-10-08  3:01 ` Wanpeng Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).