All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] KVM: x86: consolidate "has lapic" checks
@ 2016-02-08 16:15 Paolo Bonzini
  2016-02-08 16:15 ` [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence Paolo Bonzini
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Paolo Bonzini @ 2016-02-08 16:15 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar

This cleans up "has in-kernel LAPIC" checks in various ways,
especially avoiding checking things twice and providing a single
way to do the test.

Paolo

Paolo Bonzini (3):
  KVM: APIC: remove unnecessary double checks on APIC existence
  KVM: x86: consolidate "has lapic" checks into irq.c
  KVM: x86: consolidate different ways to test for in-kernel LAPIC

 arch/x86/kvm/irq.c   |  9 ++++++---
 arch/x86/kvm/irq.h   |  8 --------
 arch/x86/kvm/lapic.c | 41 ++++++++++++-----------------------------
 arch/x86/kvm/lapic.h |  8 ++++----
 arch/x86/kvm/pmu.c   |  2 +-
 arch/x86/kvm/x86.c   | 17 +++++++++--------
 6 files changed, 32 insertions(+), 53 deletions(-)

-- 
1.8.3.1

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

* [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence
  2016-02-08 16:15 [PATCH 0/3] KVM: x86: consolidate "has lapic" checks Paolo Bonzini
@ 2016-02-08 16:15 ` Paolo Bonzini
  2016-02-09 13:55   ` Radim Krčmář
  2016-02-08 16:15 ` [PATCH 2/3] KVM: x86: consolidate "has lapic" checks into irq.c Paolo Bonzini
  2016-02-08 16:15 ` [PATCH 3/3] KVM: x86: consolidate different ways to test for in-kernel LAPIC Paolo Bonzini
  2 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2016-02-08 16:15 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar

Usually the in-kernel APIC's existence is checked in the caller.  Do not
bother checking it again in lapic.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/lapic.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 1520d1acd0ad..b1029051f664 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -475,18 +475,12 @@ static inline void apic_clear_isr(int vec, struct kvm_lapic *apic)
 
 int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
 {
-	int highest_irr;
-
 	/* This may race with setting of irr in __apic_accept_irq() and
 	 * value returned may be wrong, but kvm_vcpu_kick() in __apic_accept_irq
 	 * will cause vmexit immediately and the value will be recalculated
 	 * on the next vmentry.
 	 */
-	if (!kvm_vcpu_has_lapic(vcpu))
-		return 0;
-	highest_irr = apic_find_highest_irr(vcpu->arch.apic);
-
-	return highest_irr;
+	return apic_find_highest_irr(vcpu->arch.apic);
 }
 
 static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
@@ -1601,8 +1595,7 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 
 void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu)
 {
-	if (kvm_vcpu_has_lapic(vcpu))
-		apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
+	apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
 }
 EXPORT_SYMBOL_GPL(kvm_lapic_set_eoi);
 
@@ -1676,9 +1669,6 @@ void kvm_lapic_set_tpr(struct kvm_vcpu *vcpu, unsigned long cr8)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
-		return;
-
 	apic_set_tpr(apic, ((cr8 & 0x0f) << 4)
 		     | (kvm_apic_get_reg(apic, APIC_TASKPRI) & 4));
 }
@@ -1687,9 +1677,6 @@ u64 kvm_lapic_get_cr8(struct kvm_vcpu *vcpu)
 {
 	u64 tpr;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
-		return 0;
-
 	tpr = (u64) kvm_apic_get_reg(vcpu->arch.apic, APIC_TASKPRI);
 
 	return (tpr & 0xf0) >> 4;
@@ -1912,7 +1899,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	struct kvm_lapic *apic = vcpu->arch.apic;
 	int highest_irr;
 
-	if (!kvm_vcpu_has_lapic(vcpu) || !apic_enabled(apic))
+	if (!apic_enabled(apic))
 		return -1;
 
 	apic_update_ppr(apic);
-- 
1.8.3.1

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

* [PATCH 2/3] KVM: x86: consolidate "has lapic" checks into irq.c
  2016-02-08 16:15 [PATCH 0/3] KVM: x86: consolidate "has lapic" checks Paolo Bonzini
  2016-02-08 16:15 ` [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence Paolo Bonzini
@ 2016-02-08 16:15 ` Paolo Bonzini
  2016-02-09 14:05   ` Radim Krčmář
  2016-02-08 16:15 ` [PATCH 3/3] KVM: x86: consolidate different ways to test for in-kernel LAPIC Paolo Bonzini
  2 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2016-02-08 16:15 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar

Do for kvm_cpu_has_pending_timer and kvm_inject_pending_timer_irqs
what the other irq.c routines have been doing.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/irq.c   | 9 ++++++---
 arch/x86/kvm/lapic.c | 6 +-----
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
index 3982b479bb5f..4056007e9fe1 100644
--- a/arch/x86/kvm/irq.c
+++ b/arch/x86/kvm/irq.c
@@ -33,7 +33,10 @@
  */
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
 {
-	return apic_has_pending_timer(vcpu);
+	if (lapic_in_kernel(vcpu))
+		return apic_has_pending_timer(vcpu);
+
+	return true;
 }
 EXPORT_SYMBOL(kvm_cpu_has_pending_timer);
 
@@ -137,8 +140,8 @@ EXPORT_SYMBOL_GPL(kvm_cpu_get_interrupt);
 
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu)
 {
-	kvm_inject_apic_timer_irqs(vcpu);
-	/* TODO: PIT, RTC etc. */
+	if (lapic_in_kernel(vcpu))
+		kvm_inject_apic_timer_irqs(vcpu);
 }
 EXPORT_SYMBOL_GPL(kvm_inject_pending_timer_irqs);
 
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index b1029051f664..57e3f27bdadb 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1801,8 +1801,7 @@ int apic_has_pending_timer(struct kvm_vcpu *vcpu)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
-	if (kvm_vcpu_has_lapic(vcpu) && apic_enabled(apic) &&
-			apic_lvt_enabled(apic, APIC_LVTT))
+	if (apic_enabled(apic) && apic_lvt_enabled(apic, APIC_LVTT))
 		return atomic_read(&apic->lapic_timer.pending);
 
 	return 0;
@@ -1927,9 +1926,6 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
-		return;
-
 	if (atomic_read(&apic->lapic_timer.pending) > 0) {
 		kvm_apic_local_deliver(apic, APIC_LVTT);
 		if (apic_lvtt_tscdeadline(apic))
-- 
1.8.3.1

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

* [PATCH 3/3] KVM: x86: consolidate different ways to test for in-kernel LAPIC
  2016-02-08 16:15 [PATCH 0/3] KVM: x86: consolidate "has lapic" checks Paolo Bonzini
  2016-02-08 16:15 ` [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence Paolo Bonzini
  2016-02-08 16:15 ` [PATCH 2/3] KVM: x86: consolidate "has lapic" checks into irq.c Paolo Bonzini
@ 2016-02-08 16:15 ` Paolo Bonzini
  2016-02-09 14:14   ` Radim Krčmář
  2 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2016-02-08 16:15 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: rkrcmar

We had checks on vcpu->arch.apic being (non-)NULL, kvm_vcpu_has_lapic
(more optimized) and lapic_in_kernel.  Replace everything with
lapic_in_kernel's name and kvm_vcpu_has_lapic's implementation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/irq.h   |  8 --------
 arch/x86/kvm/lapic.c | 16 ++++++++--------
 arch/x86/kvm/lapic.h |  8 ++++----
 arch/x86/kvm/pmu.c   |  2 +-
 arch/x86/kvm/x86.c   | 17 +++++++++--------
 5 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index ae5c78f2337d..61ebdc13a29a 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -109,14 +109,6 @@ static inline int irqchip_in_kernel(struct kvm *kvm)
 	return ret;
 }
 
-static inline int lapic_in_kernel(struct kvm_vcpu *vcpu)
-{
-	/* Same as irqchip_in_kernel(vcpu->kvm), but with less
-	 * pointer chasing and no unnecessary memory barriers.
-	 */
-	return vcpu->arch.apic != NULL;
-}
-
 void kvm_pic_reset(struct kvm_kpic_state *s);
 
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 57e3f27bdadb..1482a581a83c 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -281,7 +281,7 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu)
 	struct kvm_cpuid_entry2 *feat;
 	u32 v = APIC_VERSION;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
+	if (!lapic_in_kernel(vcpu))
 		return;
 
 	feat = kvm_find_cpuid_entry(apic->vcpu, 0x1, 0);
@@ -1319,7 +1319,7 @@ void wait_lapic_expire(struct kvm_vcpu *vcpu)
 	struct kvm_lapic *apic = vcpu->arch.apic;
 	u64 guest_tsc, tsc_deadline;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
+	if (!lapic_in_kernel(vcpu))
 		return;
 
 	if (apic->lapic_timer.expired_tscdeadline == 0)
@@ -1645,7 +1645,7 @@ u64 kvm_get_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
-	if (!kvm_vcpu_has_lapic(vcpu) || apic_lvtt_oneshot(apic) ||
+	if (!lapic_in_kernel(vcpu) || apic_lvtt_oneshot(apic) ||
 			apic_lvtt_period(apic))
 		return 0;
 
@@ -1656,7 +1656,7 @@ void kvm_set_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu, u64 data)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
-	if (!kvm_vcpu_has_lapic(vcpu) || apic_lvtt_oneshot(apic) ||
+	if (!lapic_in_kernel(vcpu) || apic_lvtt_oneshot(apic) ||
 			apic_lvtt_period(apic))
 		return;
 
@@ -2001,7 +2001,7 @@ void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
 {
 	struct hrtimer *timer;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
+	if (!lapic_in_kernel(vcpu))
 		return;
 
 	timer = &vcpu->arch.apic->lapic_timer.timer;
@@ -2174,7 +2174,7 @@ int kvm_hv_vapic_msr_write(struct kvm_vcpu *vcpu, u32 reg, u64 data)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
+	if (!lapic_in_kernel(vcpu))
 		return 1;
 
 	/* if this is ICR write vector before command */
@@ -2188,7 +2188,7 @@ int kvm_hv_vapic_msr_read(struct kvm_vcpu *vcpu, u32 reg, u64 *data)
 	struct kvm_lapic *apic = vcpu->arch.apic;
 	u32 low, high = 0;
 
-	if (!kvm_vcpu_has_lapic(vcpu))
+	if (!lapic_in_kernel(vcpu))
 		return 1;
 
 	if (apic_reg_read(apic, reg, 4, &low))
@@ -2220,7 +2220,7 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
 	u8 sipi_vector;
 	unsigned long pe;
 
-	if (!kvm_vcpu_has_lapic(vcpu) || !apic->pending_events)
+	if (!lapic_in_kernel(vcpu) || !apic->pending_events)
 		return;
 
 	/*
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index afccf4099b00..59610099af04 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -103,7 +103,7 @@ static inline u32 kvm_apic_get_reg(struct kvm_lapic *apic, int reg_off)
 
 extern struct static_key kvm_no_apic_vcpu;
 
-static inline bool kvm_vcpu_has_lapic(struct kvm_vcpu *vcpu)
+static inline bool lapic_in_kernel(struct kvm_vcpu *vcpu)
 {
 	if (static_key_false(&kvm_no_apic_vcpu))
 		return vcpu->arch.apic;
@@ -130,7 +130,7 @@ static inline bool kvm_apic_sw_enabled(struct kvm_lapic *apic)
 
 static inline bool kvm_apic_present(struct kvm_vcpu *vcpu)
 {
-	return kvm_vcpu_has_lapic(vcpu) && kvm_apic_hw_enabled(vcpu->arch.apic);
+	return lapic_in_kernel(vcpu) && kvm_apic_hw_enabled(vcpu->arch.apic);
 }
 
 static inline int kvm_lapic_enabled(struct kvm_vcpu *vcpu)
@@ -150,7 +150,7 @@ static inline bool kvm_vcpu_apicv_active(struct kvm_vcpu *vcpu)
 
 static inline bool kvm_apic_has_events(struct kvm_vcpu *vcpu)
 {
-	return kvm_vcpu_has_lapic(vcpu) && vcpu->arch.apic->pending_events;
+	return lapic_in_kernel(vcpu) && vcpu->arch.apic->pending_events;
 }
 
 static inline bool kvm_lowest_prio_delivery(struct kvm_lapic_irq *irq)
@@ -161,7 +161,7 @@ static inline bool kvm_lowest_prio_delivery(struct kvm_lapic_irq *irq)
 
 static inline int kvm_lapic_latched_init(struct kvm_vcpu *vcpu)
 {
-	return kvm_vcpu_has_lapic(vcpu) && test_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events);
+	return lapic_in_kernel(vcpu) && test_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events);
 }
 
 static inline int kvm_apic_id(struct kvm_lapic *apic)
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 31aa2c85dc97..06ce377dcbc9 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -257,7 +257,7 @@ int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned idx, u64 *data)
 
 void kvm_pmu_deliver_pmi(struct kvm_vcpu *vcpu)
 {
-	if (vcpu->arch.apic)
+	if (lapic_in_kernel(vcpu))
 		kvm_apic_local_deliver(vcpu->arch.apic, APIC_LVTPC);
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 63a53e358ba3..cf15bc5b0dff 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3010,7 +3010,7 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 	kvm_x86_ops->set_nmi_mask(vcpu, events->nmi.masked);
 
 	if (events->flags & KVM_VCPUEVENT_VALID_SIPI_VECTOR &&
-	    kvm_vcpu_has_lapic(vcpu))
+	    lapic_in_kernel(vcpu))
 		vcpu->arch.apic->sipi_vector = events->sipi_vector;
 
 	if (events->flags & KVM_VCPUEVENT_VALID_SMM) {
@@ -3023,7 +3023,7 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 			vcpu->arch.hflags |= HF_SMM_INSIDE_NMI_MASK;
 		else
 			vcpu->arch.hflags &= ~HF_SMM_INSIDE_NMI_MASK;
-		if (kvm_vcpu_has_lapic(vcpu)) {
+		if (lapic_in_kernel(vcpu)) {
 			if (events->smi.latched_init)
 				set_bit(KVM_APIC_INIT, &vcpu->arch.apic->pending_events);
 			else
@@ -3263,7 +3263,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 	switch (ioctl) {
 	case KVM_GET_LAPIC: {
 		r = -EINVAL;
-		if (!vcpu->arch.apic)
+		if (!lapic_in_kernel(vcpu))
 			goto out;
 		u.lapic = kzalloc(sizeof(struct kvm_lapic_state), GFP_KERNEL);
 
@@ -3281,7 +3281,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 	}
 	case KVM_SET_LAPIC: {
 		r = -EINVAL;
-		if (!vcpu->arch.apic)
+		if (!lapic_in_kernel(vcpu))
 			goto out;
 		u.lapic = memdup_user(argp, sizeof(*u.lapic));
 		if (IS_ERR(u.lapic))
@@ -4116,7 +4116,7 @@ static int vcpu_mmio_write(struct kvm_vcpu *vcpu, gpa_t addr, int len,
 
 	do {
 		n = min(len, 8);
-		if (!(vcpu->arch.apic &&
+		if (!(lapic_in_kernel(vcpu) &&
 		      !kvm_iodevice_write(vcpu, &vcpu->arch.apic->dev, addr, n, v))
 		    && kvm_io_bus_write(vcpu, KVM_MMIO_BUS, addr, n, v))
 			break;
@@ -4136,7 +4136,7 @@ static int vcpu_mmio_read(struct kvm_vcpu *vcpu, gpa_t addr, int len, void *v)
 
 	do {
 		n = min(len, 8);
-		if (!(vcpu->arch.apic &&
+		if (!(lapic_in_kernel(vcpu) &&
 		      !kvm_iodevice_read(vcpu, &vcpu->arch.apic->dev,
 					 addr, n, v))
 		    && kvm_io_bus_read(vcpu, KVM_MMIO_BUS, addr, n, v))
@@ -6033,7 +6033,7 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu)
 	if (!kvm_x86_ops->update_cr8_intercept)
 		return;
 
-	if (!vcpu->arch.apic)
+	if (!lapic_in_kernel(vcpu))
 		return;
 
 	if (vcpu->arch.apicv_active)
@@ -7061,7 +7061,7 @@ int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
 int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 				    struct kvm_mp_state *mp_state)
 {
-	if (!kvm_vcpu_has_lapic(vcpu) &&
+	if (!lapic_in_kernel(vcpu) &&
 	    mp_state->mp_state != KVM_MP_STATE_RUNNABLE)
 		return -EINVAL;
 
@@ -7616,6 +7616,7 @@ bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu)
 }
 
 struct static_key kvm_no_apic_vcpu __read_mostly;
+EXPORT_SYMBOL_GPL(kvm_no_apic_vcpu);
 
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 {
-- 
1.8.3.1

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

* Re: [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence
  2016-02-08 16:15 ` [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence Paolo Bonzini
@ 2016-02-09 13:55   ` Radim Krčmář
  2016-02-09 15:06     ` Paolo Bonzini
  0 siblings, 1 reply; 11+ messages in thread
From: Radim Krčmář @ 2016-02-09 13:55 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

2016-02-08 17:15+0100, Paolo Bonzini:
> Usually the in-kernel APIC's existence is checked in the caller.  Do not
> bother checking it again in lapic.c.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> @@ -1601,8 +1595,7 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>  
>  void kvm_lapic_set_eoi(struct kvm_vcpu *vcpu)
>  {
> -	if (kvm_vcpu_has_lapic(vcpu))
> -		apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);
> +	apic_reg_write(vcpu->arch.apic, APIC_EOI, 0);

This is most likely going to bug on the following path:
  handle_apic_access -> kvm_lapic_set_eoi

Before the change, handle_apic_access would just drop EOIs that should
have gone to user space ... I'm not sure if we tested it, or the path is
really never taken.

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

* Re: [PATCH 2/3] KVM: x86: consolidate "has lapic" checks into irq.c
  2016-02-08 16:15 ` [PATCH 2/3] KVM: x86: consolidate "has lapic" checks into irq.c Paolo Bonzini
@ 2016-02-09 14:05   ` Radim Krčmář
  2016-02-09 15:07     ` Paolo Bonzini
  0 siblings, 1 reply; 11+ messages in thread
From: Radim Krčmář @ 2016-02-09 14:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

2016-02-08 17:15+0100, Paolo Bonzini:
> Do for kvm_cpu_has_pending_timer and kvm_inject_pending_timer_irqs
> what the other irq.c routines have been doing.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
> @@ -33,7 +33,10 @@
>   */
>  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>  {
> -	return apic_has_pending_timer(vcpu);
> +	if (lapic_in_kernel(vcpu))
> +		return apic_has_pending_timer(vcpu);
> +
> +	return true;

Apart from int/bool mismatch, it returned 0 before and that was correct.

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

* Re: [PATCH 3/3] KVM: x86: consolidate different ways to test for in-kernel LAPIC
  2016-02-08 16:15 ` [PATCH 3/3] KVM: x86: consolidate different ways to test for in-kernel LAPIC Paolo Bonzini
@ 2016-02-09 14:14   ` Radim Krčmář
  0 siblings, 0 replies; 11+ messages in thread
From: Radim Krčmář @ 2016-02-09 14:14 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

2016-02-08 17:15+0100, Paolo Bonzini:
> We had checks on vcpu->arch.apic being (non-)NULL, kvm_vcpu_has_lapic
> (more optimized) and lapic_in_kernel.  Replace everything with
> lapic_in_kernel's name and kvm_vcpu_has_lapic's implementation.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Nice,

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

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

* Re: [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence
  2016-02-09 13:55   ` Radim Krčmář
@ 2016-02-09 15:06     ` Paolo Bonzini
  2016-02-09 15:18       ` Radim Krčmář
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2016-02-09 15:06 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: linux-kernel, kvm



On 09/02/2016 14:55, Radim Krčmář wrote:
> This is most likely going to bug on the following path:
>   handle_apic_access -> kvm_lapic_set_eoi
> 
> Before the change, handle_apic_access would just drop EOIs that should
> have gone to user space ... I'm not sure if we tested it, or the path is
> really never taken.

cpu_need_virtualize_apic_accesses is needed to set
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES (vmx_secondary_exec_control) and
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES is required to get to
handle_apic_access.

Paolo

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

* Re: [PATCH 2/3] KVM: x86: consolidate "has lapic" checks into irq.c
  2016-02-09 14:05   ` Radim Krčmář
@ 2016-02-09 15:07     ` Paolo Bonzini
  2016-02-09 15:21       ` Radim Krčmář
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2016-02-09 15:07 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: linux-kernel, kvm



On 09/02/2016 15:05, Radim Krčmář wrote:
> 2016-02-08 17:15+0100, Paolo Bonzini:
>> Do for kvm_cpu_has_pending_timer and kvm_inject_pending_timer_irqs
>> what the other irq.c routines have been doing.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
>> @@ -33,7 +33,10 @@
>>   */
>>  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>>  {
>> -	return apic_has_pending_timer(vcpu);
>> +	if (lapic_in_kernel(vcpu))
>> +		return apic_has_pending_timer(vcpu);
>> +
>> +	return true;
> 
> Apart from int/bool mismatch, it returned 0 before and that was correct.
> 

So that means "return 0;" implies "Reviewed-by"? :)

Paolo

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

* Re: [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence
  2016-02-09 15:06     ` Paolo Bonzini
@ 2016-02-09 15:18       ` Radim Krčmář
  0 siblings, 0 replies; 11+ messages in thread
From: Radim Krčmář @ 2016-02-09 15:18 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

2016-02-09 16:06+0100, Paolo Bonzini:
> On 09/02/2016 14:55, Radim Krčmář wrote:
>> This is most likely going to bug on the following path:
>>   handle_apic_access -> kvm_lapic_set_eoi
>> 
>> Before the change, handle_apic_access would just drop EOIs that should
>> have gone to user space ... I'm not sure if we tested it, or the path is
>> really never taken.
> 
> cpu_need_virtualize_apic_accesses is needed to set
> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES (vmx_secondary_exec_control) and
> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES is required to get to
> handle_apic_access.

Thanks,

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

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

* Re: [PATCH 2/3] KVM: x86: consolidate "has lapic" checks into irq.c
  2016-02-09 15:07     ` Paolo Bonzini
@ 2016-02-09 15:21       ` Radim Krčmář
  0 siblings, 0 replies; 11+ messages in thread
From: Radim Krčmář @ 2016-02-09 15:21 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

2016-02-09 16:07+0100, Paolo Bonzini:
> On 09/02/2016 15:05, Radim Krčmář wrote:
>> 2016-02-08 17:15+0100, Paolo Bonzini:
>>>  int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
>>> +	return true;
>> 
>> Apart from int/bool mismatch, it returned 0 before and that was correct.
>> 
> 
> So that means "return 0;" implies "Reviewed-by"? :)

Yes.  (Also for "false" and "bool kvm_cpu_has_pending_timer...". :])

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

end of thread, other threads:[~2016-02-09 15:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-08 16:15 [PATCH 0/3] KVM: x86: consolidate "has lapic" checks Paolo Bonzini
2016-02-08 16:15 ` [PATCH 1/3] KVM: APIC: remove unnecessary double checks on APIC existence Paolo Bonzini
2016-02-09 13:55   ` Radim Krčmář
2016-02-09 15:06     ` Paolo Bonzini
2016-02-09 15:18       ` Radim Krčmář
2016-02-08 16:15 ` [PATCH 2/3] KVM: x86: consolidate "has lapic" checks into irq.c Paolo Bonzini
2016-02-09 14:05   ` Radim Krčmář
2016-02-09 15:07     ` Paolo Bonzini
2016-02-09 15:21       ` Radim Krčmář
2016-02-08 16:15 ` [PATCH 3/3] KVM: x86: consolidate different ways to test for in-kernel LAPIC Paolo Bonzini
2016-02-09 14:14   ` Radim Krčmář

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.