* [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.