* [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code
@ 2016-03-02 21:56 Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 01/16] KVM: x86: change PIT discard tick policy Radim Krčmář
` (16 more replies)
0 siblings, 17 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
v3
- mark patches reviewed by Paolo [1,9-14/16]
- properly categorize reinject struct member [Paolo] [7/16]
- use atomic_t to document reinject usage [Paolo] [15/16]
- document PIT reinject modes [16/16]
v2: http://www.spinics.net/lists/kvm/msg127927.html
Radim Krčmář (16):
KVM: x86: change PIT discard tick policy
KVM: x86: simplify atomics in kvm_pit_ack_irq
KVM: x86: add kvm_pit_reset_reinject
KVM: x86: use atomic_t instead of pit.inject_lock
KVM: x86: tone down WARN_ON pit.state_lock
KVM: x86: pass struct kvm_pit instead of kvm in PIT
KVM: x86: remove unnecessary uses of PIT state lock
KVM: x86: remove notifiers from PIT discard policy
KVM: x86: refactor kvm_create_pit
KVM: x86: refactor kvm_free_pit
KVM: x86: remove pit and kvm from kvm_kpit_state
KVM: x86: remove pointless dereference of PIT
KVM: x86: don't assume layout of kvm_kpit_state
KVM: x86: move PIT timer function initialization
KVM: x86: turn kvm_kpit_state.reinject into atomic_t
KVM: document KVM_REINJECT_CONTROL ioctl
Documentation/virtual/kvm/api.txt | 24 +++
arch/x86/kvm/i8254.c | 320 +++++++++++++++++---------------------
arch/x86/kvm/i8254.h | 17 +-
arch/x86/kvm/x86.c | 52 ++++---
4 files changed, 213 insertions(+), 200 deletions(-)
--
2.7.2
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 01/16] KVM: x86: change PIT discard tick policy
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 02/16] KVM: x86: simplify atomics in kvm_pit_ack_irq Radim Krčmář
` (15 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya, stable
From: Radim Krčmář <rkrcmar@redhat.com>
Discard policy uses ack_notifiers to prevent injection of PIT interrupts
before EOI from the last one.
This patch changes the policy to always try to deliver the interrupt,
which makes a difference when its vector is in ISR.
Old implementation would drop the interrupt, but proposed one injects to
IRR, like real hardware would.
The old policy breaks legacy NMI watchdogs, where PIT is used through
virtual wire (LVT0): PIT never sends an interrupt before receiving EOI,
thus a guest deadlock with disabled interrupts will stop NMIs.
Note that NMI doesn't do EOI, so PIT also had to send a normal interrupt
through IOAPIC. (KVM's PIT is deeply rotten and luckily not used much
in modern systems.)
Even though there is a chance of regressions, I think we can fix the
LVT0 NMI bug without introducing a new tick policy.
Cc: <stable@vger.kernel.org>
Reported-by: Yuki Shibuya <shibuya.yk@ncos.nec.co.jp>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index b0ea42b78ccd..ab5318727579 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -245,7 +245,7 @@ static void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian)
* PIC is being reset. Handle it gracefully here
*/
atomic_inc(&ps->pending);
- else if (value > 0)
+ else if (value > 0 && ps->reinject)
/* in this case, we had multiple outstanding pit interrupts
* that we needed to inject. Reinject
*/
@@ -288,7 +288,9 @@ static void pit_do_work(struct kthread_work *work)
* last one has been acked.
*/
spin_lock(&ps->inject_lock);
- if (ps->irq_ack) {
+ if (!ps->reinject)
+ inject = 1;
+ else if (ps->irq_ack) {
ps->irq_ack = 0;
inject = 1;
}
@@ -317,10 +319,10 @@ static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
struct kvm_kpit_state *ps = container_of(data, struct kvm_kpit_state, timer);
struct kvm_pit *pt = ps->kvm->arch.vpit;
- if (ps->reinject || !atomic_read(&ps->pending)) {
+ if (ps->reinject)
atomic_inc(&ps->pending);
- queue_kthread_work(&pt->worker, &pt->expired);
- }
+
+ queue_kthread_work(&pt->worker, &pt->expired);
if (ps->is_periodic) {
hrtimer_add_expires_ns(&ps->timer, ps->period);
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 02/16] KVM: x86: simplify atomics in kvm_pit_ack_irq
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 01/16] KVM: x86: change PIT discard tick policy Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 03/16] KVM: x86: add kvm_pit_reset_reinject Radim Krčmář
` (14 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
We already have a helper that does the same thing.
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index ab5318727579..7d694ac7f4a4 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -236,19 +236,9 @@ static void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian)
{
struct kvm_kpit_state *ps = container_of(kian, struct kvm_kpit_state,
irq_ack_notifier);
- int value;
spin_lock(&ps->inject_lock);
- value = atomic_dec_return(&ps->pending);
- if (value < 0)
- /* spurious acks can be generated if, for example, the
- * PIC is being reset. Handle it gracefully here
- */
- atomic_inc(&ps->pending);
- else if (value > 0 && ps->reinject)
- /* in this case, we had multiple outstanding pit interrupts
- * that we needed to inject. Reinject
- */
+ if (atomic_dec_if_positive(&ps->pending) > 0 && ps->reinject)
queue_kthread_work(&ps->pit->worker, &ps->pit->expired);
ps->irq_ack = 1;
spin_unlock(&ps->inject_lock);
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 03/16] KVM: x86: add kvm_pit_reset_reinject
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 01/16] KVM: x86: change PIT discard tick policy Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 02/16] KVM: x86: simplify atomics in kvm_pit_ack_irq Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 04/16] KVM: x86: use atomic_t instead of pit.inject_lock Radim Krčmář
` (13 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
pit_state.pending and pit_state.irq_ack are always reset at the same
time. Create a function for them.
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 7d694ac7f4a4..bdbb3f076e72 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -321,6 +321,12 @@ static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
return HRTIMER_NORESTART;
}
+static inline void kvm_pit_reset_reinject(struct kvm_pit *pit)
+{
+ atomic_set(&pit->pit_state.pending, 0);
+ pit->pit_state.irq_ack = 1;
+}
+
static void create_pit_timer(struct kvm *kvm, u32 val, int is_period)
{
struct kvm_kpit_state *ps = &kvm->arch.vpit->pit_state;
@@ -343,8 +349,7 @@ static void create_pit_timer(struct kvm *kvm, u32 val, int is_period)
ps->timer.function = pit_timer_fn;
ps->kvm = ps->pit->kvm;
- atomic_set(&ps->pending, 0);
- ps->irq_ack = 1;
+ kvm_pit_reset_reinject(ps->pit);
/*
* Do not allow the guest to program periodic timers with small
@@ -644,18 +649,15 @@ void kvm_pit_reset(struct kvm_pit *pit)
}
mutex_unlock(&pit->pit_state.lock);
- atomic_set(&pit->pit_state.pending, 0);
- pit->pit_state.irq_ack = 1;
+ kvm_pit_reset_reinject(pit);
}
static void pit_mask_notifer(struct kvm_irq_mask_notifier *kimn, bool mask)
{
struct kvm_pit *pit = container_of(kimn, struct kvm_pit, mask_notifier);
- if (!mask) {
- atomic_set(&pit->pit_state.pending, 0);
- pit->pit_state.irq_ack = 1;
- }
+ if (!mask)
+ kvm_pit_reset_reinject(pit);
}
static const struct kvm_io_device_ops pit_dev_ops = {
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 04/16] KVM: x86: use atomic_t instead of pit.inject_lock
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (2 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 03/16] KVM: x86: add kvm_pit_reset_reinject Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 05/16] KVM: x86: tone down WARN_ON pit.state_lock Radim Krčmář
` (12 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
The lock was an overkill, the same can be done with atomics.
A mb() was added in kvm_pit_ack_irq, to pair with implicit barrier
between pit_timer_fn and pit_do_work. The mb() prevents a race that
could happen if pending == 0 and irq_ack == 0:
kvm_pit_ack_irq: | pit_timer_fn:
p = atomic_read(&ps->pending); |
| atomic_inc(&ps->pending);
| queue_work(pit_do_work);
| pit_do_work:
| atomic_xchg(&ps->irq_ack, 0);
| return;
atomic_set(&ps->irq_ack, 1); |
if (p == 0) return; |
where the interrupt would not be delivered in this tick of pit_timer_fn.
PIT would have eventually delivered the interrupt, but we sacrifice
perofmance to make sure that interrupts are not needlessly delayed.
sfence isn't enough: atomic_dec_if_positive does atomic_read first and
x86 can reorder loads before stores. lfence isn't enough: store can
pass lfence, turning it into a nop. A compiler barrier would be more
than enough as CPU needs to stall for unbelievably long to use fences.
This patch doesn't do anything in kvm_pit_reset_reinject, because any
order of resets can race, but the result differs by at most one
interrupt, which is ok, because it's the same result as if the reset
happened at a slightly different time. (Original code didn't protect
the reset path with a proper lock, so users have to be robust.)
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 56 +++++++++++++++++++++-------------------------------
arch/x86/kvm/i8254.h | 3 +--
2 files changed, 24 insertions(+), 35 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index bdbb3f076e72..0f5655c50e0c 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -237,11 +237,13 @@ static void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian)
struct kvm_kpit_state *ps = container_of(kian, struct kvm_kpit_state,
irq_ack_notifier);
- spin_lock(&ps->inject_lock);
+ atomic_set(&ps->irq_ack, 1);
+ /* irq_ack should be set before pending is read. Order accesses with
+ * inc(pending) in pit_timer_fn and xchg(irq_ack, 0) in pit_do_work.
+ */
+ smp_mb();
if (atomic_dec_if_positive(&ps->pending) > 0 && ps->reinject)
queue_kthread_work(&ps->pit->worker, &ps->pit->expired);
- ps->irq_ack = 1;
- spin_unlock(&ps->inject_lock);
}
void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu)
@@ -272,36 +274,25 @@ static void pit_do_work(struct kthread_work *work)
struct kvm_vcpu *vcpu;
int i;
struct kvm_kpit_state *ps = &pit->pit_state;
- int inject = 0;
- /* Try to inject pending interrupts when
- * last one has been acked.
+ if (ps->reinject && !atomic_xchg(&ps->irq_ack, 0))
+ return;
+
+ kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1, false);
+ kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0, false);
+
+ /*
+ * Provides NMI watchdog support via Virtual Wire mode.
+ * The route is: PIT -> LVT0 in NMI mode.
+ *
+ * Note: Our Virtual Wire implementation does not follow
+ * the MP specification. We propagate a PIT interrupt to all
+ * VCPUs and only when LVT0 is in NMI mode. The interrupt can
+ * also be simultaneously delivered through PIC and IOAPIC.
*/
- spin_lock(&ps->inject_lock);
- if (!ps->reinject)
- inject = 1;
- else if (ps->irq_ack) {
- ps->irq_ack = 0;
- inject = 1;
- }
- spin_unlock(&ps->inject_lock);
- if (inject) {
- kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1, false);
- kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0, false);
-
- /*
- * Provides NMI watchdog support via Virtual Wire mode.
- * The route is: PIT -> PIC -> LVT0 in NMI mode.
- *
- * Note: Our Virtual Wire implementation is simplified, only
- * propagating PIT interrupts to all VCPUs when they have set
- * LVT0 to NMI delivery. Other PIC interrupts are just sent to
- * VCPU0, and only if its LVT0 is in EXTINT mode.
- */
- if (atomic_read(&kvm->arch.vapics_in_nmi_mode) > 0)
- kvm_for_each_vcpu(i, vcpu, kvm)
- kvm_apic_nmi_wd_deliver(vcpu);
- }
+ if (atomic_read(&kvm->arch.vapics_in_nmi_mode) > 0)
+ kvm_for_each_vcpu(i, vcpu, kvm)
+ kvm_apic_nmi_wd_deliver(vcpu);
}
static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
@@ -324,7 +315,7 @@ static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
static inline void kvm_pit_reset_reinject(struct kvm_pit *pit)
{
atomic_set(&pit->pit_state.pending, 0);
- pit->pit_state.irq_ack = 1;
+ atomic_set(&pit->pit_state.irq_ack, 1);
}
static void create_pit_timer(struct kvm *kvm, u32 val, int is_period)
@@ -691,7 +682,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
mutex_init(&pit->pit_state.lock);
mutex_lock(&pit->pit_state.lock);
- spin_lock_init(&pit->pit_state.inject_lock);
pid = get_pid(task_tgid(current));
pid_nr = pid_vnr(pid);
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index c84990b42b5b..f8cf4b84f435 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -33,8 +33,7 @@ struct kvm_kpit_state {
u32 speaker_data_on;
struct mutex lock;
struct kvm_pit *pit;
- spinlock_t inject_lock;
- unsigned long irq_ack;
+ atomic_t irq_ack;
struct kvm_irq_ack_notifier irq_ack_notifier;
};
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 05/16] KVM: x86: tone down WARN_ON pit.state_lock
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (3 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 04/16] KVM: x86: use atomic_t instead of pit.inject_lock Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 06/16] KVM: x86: pass struct kvm_pit instead of kvm in PIT Radim Krčmář
` (11 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
If the guest could hit this, it would hang the host kernel, bacause of
sheer number of those reports. Internal callers have to be sensible
anyway, so we now only check for it in an API function.
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 0f5655c50e0c..e5a3e8015e30 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -76,8 +76,6 @@ static void pit_set_gate(struct kvm *kvm, int channel, u32 val)
struct kvm_kpit_channel_state *c =
&kvm->arch.vpit->pit_state.channels[channel];
- WARN_ON(!mutex_is_locked(&kvm->arch.vpit->pit_state.lock));
-
switch (c->mode) {
default:
case 0:
@@ -99,8 +97,6 @@ static void pit_set_gate(struct kvm *kvm, int channel, u32 val)
static int pit_get_gate(struct kvm *kvm, int channel)
{
- WARN_ON(!mutex_is_locked(&kvm->arch.vpit->pit_state.lock));
-
return kvm->arch.vpit->pit_state.channels[channel].gate;
}
@@ -144,8 +140,6 @@ static int pit_get_count(struct kvm *kvm, int channel)
s64 d, t;
int counter;
- WARN_ON(!mutex_is_locked(&kvm->arch.vpit->pit_state.lock));
-
t = kpit_elapsed(kvm, c, channel);
d = muldiv64(t, KVM_PIT_FREQ, NSEC_PER_SEC);
@@ -174,8 +168,6 @@ static int pit_get_out(struct kvm *kvm, int channel)
s64 d, t;
int out;
- WARN_ON(!mutex_is_locked(&kvm->arch.vpit->pit_state.lock));
-
t = kpit_elapsed(kvm, c, channel);
d = muldiv64(t, KVM_PIT_FREQ, NSEC_PER_SEC);
@@ -207,8 +199,6 @@ static void pit_latch_count(struct kvm *kvm, int channel)
struct kvm_kpit_channel_state *c =
&kvm->arch.vpit->pit_state.channels[channel];
- WARN_ON(!mutex_is_locked(&kvm->arch.vpit->pit_state.lock));
-
if (!c->count_latched) {
c->latched_count = pit_get_count(kvm, channel);
c->count_latched = c->rw_mode;
@@ -220,8 +210,6 @@ static void pit_latch_status(struct kvm *kvm, int channel)
struct kvm_kpit_channel_state *c =
&kvm->arch.vpit->pit_state.channels[channel];
- WARN_ON(!mutex_is_locked(&kvm->arch.vpit->pit_state.lock));
-
if (!c->status_latched) {
/* TODO: Return NULL COUNT (bit 6). */
c->status = ((pit_get_out(kvm, channel) << 7) |
@@ -367,8 +355,6 @@ static void pit_load_count(struct kvm *kvm, int channel, u32 val)
{
struct kvm_kpit_state *ps = &kvm->arch.vpit->pit_state;
- WARN_ON(!mutex_is_locked(&ps->lock));
-
pr_debug("load_count val is %d, channel is %d\n", val, channel);
/*
@@ -406,6 +392,9 @@ static void pit_load_count(struct kvm *kvm, int channel, u32 val)
void kvm_pit_load_count(struct kvm *kvm, int channel, u32 val, int hpet_legacy_start)
{
u8 saved_mode;
+
+ WARN_ON_ONCE(!mutex_is_locked(&kvm->arch.vpit->pit_state.lock));
+
if (hpet_legacy_start) {
/* save existing mode for later reenablement */
WARN_ON(channel != 0);
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 06/16] KVM: x86: pass struct kvm_pit instead of kvm in PIT
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (4 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 05/16] KVM: x86: tone down WARN_ON pit.state_lock Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 07/16] KVM: x86: remove unnecessary uses of PIT state lock Radim Krčmář
` (10 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
This patch passes struct kvm_pit into internal PIT functions.
Those functions used to get PIT through kvm->arch.vpit, even though most
of them never used *kvm for other purposes. Another benefit is that we
don't need to set kvm->arch.vpit during initialization.
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 112 ++++++++++++++++++++++++---------------------------
arch/x86/kvm/i8254.h | 4 +-
arch/x86/kvm/x86.c | 26 +++++++-----
3 files changed, 70 insertions(+), 72 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index e5a3e8015e30..2afe09b054e7 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -71,10 +71,9 @@ static u64 muldiv64(u64 a, u32 b, u32 c)
return res.ll;
}
-static void pit_set_gate(struct kvm *kvm, int channel, u32 val)
+static void pit_set_gate(struct kvm_pit *pit, int channel, u32 val)
{
- struct kvm_kpit_channel_state *c =
- &kvm->arch.vpit->pit_state.channels[channel];
+ struct kvm_kpit_channel_state *c = &pit->pit_state.channels[channel];
switch (c->mode) {
default:
@@ -95,16 +94,16 @@ static void pit_set_gate(struct kvm *kvm, int channel, u32 val)
c->gate = val;
}
-static int pit_get_gate(struct kvm *kvm, int channel)
+static int pit_get_gate(struct kvm_pit *pit, int channel)
{
- return kvm->arch.vpit->pit_state.channels[channel].gate;
+ return pit->pit_state.channels[channel].gate;
}
-static s64 __kpit_elapsed(struct kvm *kvm)
+static s64 __kpit_elapsed(struct kvm_pit *pit)
{
s64 elapsed;
ktime_t remaining;
- struct kvm_kpit_state *ps = &kvm->arch.vpit->pit_state;
+ struct kvm_kpit_state *ps = &pit->pit_state;
if (!ps->period)
return 0;
@@ -124,23 +123,22 @@ static s64 __kpit_elapsed(struct kvm *kvm)
return elapsed;
}
-static s64 kpit_elapsed(struct kvm *kvm, struct kvm_kpit_channel_state *c,
+static s64 kpit_elapsed(struct kvm_pit *pit, struct kvm_kpit_channel_state *c,
int channel)
{
if (channel == 0)
- return __kpit_elapsed(kvm);
+ return __kpit_elapsed(pit);
return ktime_to_ns(ktime_sub(ktime_get(), c->count_load_time));
}
-static int pit_get_count(struct kvm *kvm, int channel)
+static int pit_get_count(struct kvm_pit *pit, int channel)
{
- struct kvm_kpit_channel_state *c =
- &kvm->arch.vpit->pit_state.channels[channel];
+ struct kvm_kpit_channel_state *c = &pit->pit_state.channels[channel];
s64 d, t;
int counter;
- t = kpit_elapsed(kvm, c, channel);
+ t = kpit_elapsed(pit, c, channel);
d = muldiv64(t, KVM_PIT_FREQ, NSEC_PER_SEC);
switch (c->mode) {
@@ -161,14 +159,13 @@ static int pit_get_count(struct kvm *kvm, int channel)
return counter;
}
-static int pit_get_out(struct kvm *kvm, int channel)
+static int pit_get_out(struct kvm_pit *pit, int channel)
{
- struct kvm_kpit_channel_state *c =
- &kvm->arch.vpit->pit_state.channels[channel];
+ struct kvm_kpit_channel_state *c = &pit->pit_state.channels[channel];
s64 d, t;
int out;
- t = kpit_elapsed(kvm, c, channel);
+ t = kpit_elapsed(pit, c, channel);
d = muldiv64(t, KVM_PIT_FREQ, NSEC_PER_SEC);
switch (c->mode) {
@@ -194,25 +191,23 @@ static int pit_get_out(struct kvm *kvm, int channel)
return out;
}
-static void pit_latch_count(struct kvm *kvm, int channel)
+static void pit_latch_count(struct kvm_pit *pit, int channel)
{
- struct kvm_kpit_channel_state *c =
- &kvm->arch.vpit->pit_state.channels[channel];
+ struct kvm_kpit_channel_state *c = &pit->pit_state.channels[channel];
if (!c->count_latched) {
- c->latched_count = pit_get_count(kvm, channel);
+ c->latched_count = pit_get_count(pit, channel);
c->count_latched = c->rw_mode;
}
}
-static void pit_latch_status(struct kvm *kvm, int channel)
+static void pit_latch_status(struct kvm_pit *pit, int channel)
{
- struct kvm_kpit_channel_state *c =
- &kvm->arch.vpit->pit_state.channels[channel];
+ struct kvm_kpit_channel_state *c = &pit->pit_state.channels[channel];
if (!c->status_latched) {
/* TODO: Return NULL COUNT (bit 6). */
- c->status = ((pit_get_out(kvm, channel) << 7) |
+ c->status = ((pit_get_out(pit, channel) << 7) |
(c->rw_mode << 4) |
(c->mode << 1) |
c->bcd);
@@ -306,9 +301,10 @@ static inline void kvm_pit_reset_reinject(struct kvm_pit *pit)
atomic_set(&pit->pit_state.irq_ack, 1);
}
-static void create_pit_timer(struct kvm *kvm, u32 val, int is_period)
+static void create_pit_timer(struct kvm_pit *pit, u32 val, int is_period)
{
- struct kvm_kpit_state *ps = &kvm->arch.vpit->pit_state;
+ struct kvm_kpit_state *ps = &pit->pit_state;
+ struct kvm *kvm = pit->kvm;
s64 interval;
if (!ioapic_in_kernel(kvm) ||
@@ -326,9 +322,9 @@ static void create_pit_timer(struct kvm *kvm, u32 val, int is_period)
ps->is_periodic = is_period;
ps->timer.function = pit_timer_fn;
- ps->kvm = ps->pit->kvm;
+ ps->kvm = pit->kvm;
- kvm_pit_reset_reinject(ps->pit);
+ kvm_pit_reset_reinject(pit);
/*
* Do not allow the guest to program periodic timers with small
@@ -351,9 +347,9 @@ static void create_pit_timer(struct kvm *kvm, u32 val, int is_period)
HRTIMER_MODE_ABS);
}
-static void pit_load_count(struct kvm *kvm, int channel, u32 val)
+static void pit_load_count(struct kvm_pit *pit, int channel, u32 val)
{
- struct kvm_kpit_state *ps = &kvm->arch.vpit->pit_state;
+ struct kvm_kpit_state *ps = &pit->pit_state;
pr_debug("load_count val is %d, channel is %d\n", val, channel);
@@ -378,32 +374,33 @@ static void pit_load_count(struct kvm *kvm, int channel, u32 val)
case 1:
/* FIXME: enhance mode 4 precision */
case 4:
- create_pit_timer(kvm, val, 0);
+ create_pit_timer(pit, val, 0);
break;
case 2:
case 3:
- create_pit_timer(kvm, val, 1);
+ create_pit_timer(pit, val, 1);
break;
default:
- destroy_pit_timer(kvm->arch.vpit);
+ destroy_pit_timer(pit);
}
}
-void kvm_pit_load_count(struct kvm *kvm, int channel, u32 val, int hpet_legacy_start)
+void kvm_pit_load_count(struct kvm_pit *pit, int channel, u32 val,
+ int hpet_legacy_start)
{
u8 saved_mode;
- WARN_ON_ONCE(!mutex_is_locked(&kvm->arch.vpit->pit_state.lock));
+ WARN_ON_ONCE(!mutex_is_locked(&pit->pit_state.lock));
if (hpet_legacy_start) {
/* save existing mode for later reenablement */
WARN_ON(channel != 0);
- saved_mode = kvm->arch.vpit->pit_state.channels[0].mode;
- kvm->arch.vpit->pit_state.channels[0].mode = 0xff; /* disable timer */
- pit_load_count(kvm, channel, val);
- kvm->arch.vpit->pit_state.channels[0].mode = saved_mode;
+ saved_mode = pit->pit_state.channels[0].mode;
+ pit->pit_state.channels[0].mode = 0xff; /* disable timer */
+ pit_load_count(pit, channel, val);
+ pit->pit_state.channels[0].mode = saved_mode;
} else {
- pit_load_count(kvm, channel, val);
+ pit_load_count(pit, channel, val);
}
}
@@ -429,7 +426,6 @@ static int pit_ioport_write(struct kvm_vcpu *vcpu,
{
struct kvm_pit *pit = dev_to_pit(this);
struct kvm_kpit_state *pit_state = &pit->pit_state;
- struct kvm *kvm = pit->kvm;
int channel, access;
struct kvm_kpit_channel_state *s;
u32 val = *(u32 *) data;
@@ -453,9 +449,9 @@ static int pit_ioport_write(struct kvm_vcpu *vcpu,
s = &pit_state->channels[channel];
if (val & (2 << channel)) {
if (!(val & 0x20))
- pit_latch_count(kvm, channel);
+ pit_latch_count(pit, channel);
if (!(val & 0x10))
- pit_latch_status(kvm, channel);
+ pit_latch_status(pit, channel);
}
}
} else {
@@ -463,7 +459,7 @@ static int pit_ioport_write(struct kvm_vcpu *vcpu,
s = &pit_state->channels[channel];
access = (val >> 4) & KVM_PIT_CHANNEL_MASK;
if (access == 0) {
- pit_latch_count(kvm, channel);
+ pit_latch_count(pit, channel);
} else {
s->rw_mode = access;
s->read_state = access;
@@ -480,17 +476,17 @@ static int pit_ioport_write(struct kvm_vcpu *vcpu,
switch (s->write_state) {
default:
case RW_STATE_LSB:
- pit_load_count(kvm, addr, val);
+ pit_load_count(pit, addr, val);
break;
case RW_STATE_MSB:
- pit_load_count(kvm, addr, val << 8);
+ pit_load_count(pit, addr, val << 8);
break;
case RW_STATE_WORD0:
s->write_latch = val;
s->write_state = RW_STATE_WORD1;
break;
case RW_STATE_WORD1:
- pit_load_count(kvm, addr, s->write_latch | (val << 8));
+ pit_load_count(pit, addr, s->write_latch | (val << 8));
s->write_state = RW_STATE_WORD0;
break;
}
@@ -506,7 +502,6 @@ static int pit_ioport_read(struct kvm_vcpu *vcpu,
{
struct kvm_pit *pit = dev_to_pit(this);
struct kvm_kpit_state *pit_state = &pit->pit_state;
- struct kvm *kvm = pit->kvm;
int ret, count;
struct kvm_kpit_channel_state *s;
if (!pit_in_range(addr))
@@ -543,20 +538,20 @@ static int pit_ioport_read(struct kvm_vcpu *vcpu,
switch (s->read_state) {
default:
case RW_STATE_LSB:
- count = pit_get_count(kvm, addr);
+ count = pit_get_count(pit, addr);
ret = count & 0xff;
break;
case RW_STATE_MSB:
- count = pit_get_count(kvm, addr);
+ count = pit_get_count(pit, addr);
ret = (count >> 8) & 0xff;
break;
case RW_STATE_WORD0:
- count = pit_get_count(kvm, addr);
+ count = pit_get_count(pit, addr);
ret = count & 0xff;
s->read_state = RW_STATE_WORD1;
break;
case RW_STATE_WORD1:
- count = pit_get_count(kvm, addr);
+ count = pit_get_count(pit, addr);
ret = (count >> 8) & 0xff;
s->read_state = RW_STATE_WORD0;
break;
@@ -577,14 +572,13 @@ static int speaker_ioport_write(struct kvm_vcpu *vcpu,
{
struct kvm_pit *pit = speaker_to_pit(this);
struct kvm_kpit_state *pit_state = &pit->pit_state;
- struct kvm *kvm = pit->kvm;
u32 val = *(u32 *) data;
if (addr != KVM_SPEAKER_BASE_ADDRESS)
return -EOPNOTSUPP;
mutex_lock(&pit_state->lock);
pit_state->speaker_data_on = (val >> 1) & 1;
- pit_set_gate(kvm, 2, val & 1);
+ pit_set_gate(pit, 2, val & 1);
mutex_unlock(&pit_state->lock);
return 0;
}
@@ -595,7 +589,6 @@ static int speaker_ioport_read(struct kvm_vcpu *vcpu,
{
struct kvm_pit *pit = speaker_to_pit(this);
struct kvm_kpit_state *pit_state = &pit->pit_state;
- struct kvm *kvm = pit->kvm;
unsigned int refresh_clock;
int ret;
if (addr != KVM_SPEAKER_BASE_ADDRESS)
@@ -605,8 +598,8 @@ static int speaker_ioport_read(struct kvm_vcpu *vcpu,
refresh_clock = ((unsigned int)ktime_to_ns(ktime_get()) >> 14) & 1;
mutex_lock(&pit_state->lock);
- ret = ((pit_state->speaker_data_on << 1) | pit_get_gate(kvm, 2) |
- (pit_get_out(kvm, 2) << 5) | (refresh_clock << 4));
+ ret = ((pit_state->speaker_data_on << 1) | pit_get_gate(pit, 2) |
+ (pit_get_out(pit, 2) << 5) | (refresh_clock << 4));
if (len > sizeof(ret))
len = sizeof(ret);
memcpy(data, (char *)&ret, len);
@@ -625,7 +618,7 @@ void kvm_pit_reset(struct kvm_pit *pit)
c = &pit->pit_state.channels[i];
c->mode = 0xff;
c->gate = (i != 2);
- pit_load_count(pit->kvm, i, 0);
+ pit_load_count(pit, i, 0);
}
mutex_unlock(&pit->pit_state.lock);
@@ -687,7 +680,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
}
init_kthread_work(&pit->expired, pit_do_work);
- kvm->arch.vpit = pit;
pit->kvm = kvm;
pit_state = &pit->pit_state;
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index f8cf4b84f435..a6aceaf08df5 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -56,9 +56,11 @@ struct kvm_pit {
#define KVM_MAX_PIT_INTR_INTERVAL HZ / 100
#define KVM_PIT_CHANNEL_MASK 0x3
-void kvm_pit_load_count(struct kvm *kvm, int channel, u32 val, int hpet_legacy_start);
struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags);
void kvm_free_pit(struct kvm *kvm);
+
void kvm_pit_reset(struct kvm_pit *pit);
+void kvm_pit_load_count(struct kvm_pit *pit, int channel, u32 val,
+ int hpet_legacy_start);
#endif
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f4654e4150b0..a88e1a3eeb69 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3613,11 +3613,13 @@ static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
static int kvm_vm_ioctl_set_pit(struct kvm *kvm, struct kvm_pit_state *ps)
{
int i;
- mutex_lock(&kvm->arch.vpit->pit_state.lock);
- memcpy(&kvm->arch.vpit->pit_state, ps, sizeof(struct kvm_pit_state));
+ struct kvm_pit *pit = kvm->arch.vpit;
+
+ mutex_lock(&pit->pit_state.lock);
+ memcpy(&pit->pit_state, ps, sizeof(struct kvm_pit_state));
for (i = 0; i < 3; i++)
- kvm_pit_load_count(kvm, i, ps->channels[i].count, 0);
- mutex_unlock(&kvm->arch.vpit->pit_state.lock);
+ kvm_pit_load_count(pit, i, ps->channels[i].count, 0);
+ mutex_unlock(&pit->pit_state.lock);
return 0;
}
@@ -3637,18 +3639,20 @@ static int kvm_vm_ioctl_set_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
int start = 0;
int i;
u32 prev_legacy, cur_legacy;
- mutex_lock(&kvm->arch.vpit->pit_state.lock);
- prev_legacy = kvm->arch.vpit->pit_state.flags & KVM_PIT_FLAGS_HPET_LEGACY;
+ struct kvm_pit *pit = kvm->arch.vpit;
+
+ mutex_lock(&pit->pit_state.lock);
+ prev_legacy = pit->pit_state.flags & KVM_PIT_FLAGS_HPET_LEGACY;
cur_legacy = ps->flags & KVM_PIT_FLAGS_HPET_LEGACY;
if (!prev_legacy && cur_legacy)
start = 1;
- memcpy(&kvm->arch.vpit->pit_state.channels, &ps->channels,
- sizeof(kvm->arch.vpit->pit_state.channels));
- kvm->arch.vpit->pit_state.flags = ps->flags;
+ memcpy(&pit->pit_state.channels, &ps->channels,
+ sizeof(pit->pit_state.channels));
+ pit->pit_state.flags = ps->flags;
for (i = 0; i < 3; i++)
- kvm_pit_load_count(kvm, i, kvm->arch.vpit->pit_state.channels[i].count,
+ kvm_pit_load_count(pit, i, pit->pit_state.channels[i].count,
start && i == 0);
- mutex_unlock(&kvm->arch.vpit->pit_state.lock);
+ mutex_unlock(&pit->pit_state.lock);
return 0;
}
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 07/16] KVM: x86: remove unnecessary uses of PIT state lock
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (5 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 06/16] KVM: x86: pass struct kvm_pit instead of kvm in PIT Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 08/16] KVM: x86: remove notifiers from PIT discard policy Radim Krčmář
` (9 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
- kvm_create_pit had to lock only because it exposed kvm->arch.vpit very
early, but initialization doesn't use kvm->arch.vpit since the last
patch, so we can drop locking.
- kvm_free_pit is only run after there are no users of KVM and therefore
is the sole actor.
- Locking in kvm_vm_ioctl_reinject doesn't do anything, because reinject
is only protected at that place.
- kvm_pit_reset isn't used anywhere and its locking can be dropped if we
hide it.
Removing useless locking allows to see what actually is being protected
by PIT state lock (values accessible from the guest).
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
v3: properly categorize reinject struct member [Paolo]
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 9 +--------
arch/x86/kvm/i8254.h | 9 +++++----
arch/x86/kvm/x86.c | 4 ++--
3 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 2afe09b054e7..b8582fbe4fcf 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -607,12 +607,11 @@ static int speaker_ioport_read(struct kvm_vcpu *vcpu,
return 0;
}
-void kvm_pit_reset(struct kvm_pit *pit)
+static void kvm_pit_reset(struct kvm_pit *pit)
{
int i;
struct kvm_kpit_channel_state *c;
- mutex_lock(&pit->pit_state.lock);
pit->pit_state.flags = 0;
for (i = 0; i < 3; i++) {
c = &pit->pit_state.channels[i];
@@ -620,7 +619,6 @@ void kvm_pit_reset(struct kvm_pit *pit)
c->gate = (i != 2);
pit_load_count(pit, i, 0);
}
- mutex_unlock(&pit->pit_state.lock);
kvm_pit_reset_reinject(pit);
}
@@ -663,7 +661,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
}
mutex_init(&pit->pit_state.lock);
- mutex_lock(&pit->pit_state.lock);
pid = get_pid(task_tgid(current));
pid_nr = pid_vnr(pid);
@@ -673,7 +670,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
pit->worker_task = kthread_run(kthread_worker_fn, &pit->worker,
"kvm-pit/%d", pid_nr);
if (IS_ERR(pit->worker_task)) {
- mutex_unlock(&pit->pit_state.lock);
kvm_free_irq_source_id(kvm, pit->irq_source_id);
kfree(pit);
return NULL;
@@ -689,7 +685,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
pit_state->irq_ack_notifier.irq_acked = kvm_pit_ack_irq;
kvm_register_irq_ack_notifier(kvm, &pit_state->irq_ack_notifier);
pit_state->reinject = true;
- mutex_unlock(&pit->pit_state.lock);
kvm_pit_reset(pit);
@@ -737,13 +732,11 @@ void kvm_free_pit(struct kvm *kvm)
&kvm->arch.vpit->mask_notifier);
kvm_unregister_irq_ack_notifier(kvm,
&kvm->arch.vpit->pit_state.irq_ack_notifier);
- mutex_lock(&kvm->arch.vpit->pit_state.lock);
timer = &kvm->arch.vpit->pit_state.timer;
hrtimer_cancel(timer);
flush_kthread_work(&kvm->arch.vpit->expired);
kthread_stop(kvm->arch.vpit->worker_task);
kvm_free_irq_source_id(kvm, kvm->arch.vpit->irq_source_id);
- mutex_unlock(&kvm->arch.vpit->pit_state.lock);
kfree(kvm->arch.vpit);
}
}
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index a6aceaf08df5..840fbb3cb626 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -22,17 +22,19 @@ struct kvm_kpit_channel_state {
};
struct kvm_kpit_state {
+ /* All members before "struct mutex lock" are protected by the lock. */
struct kvm_kpit_channel_state channels[3];
u32 flags;
bool is_periodic;
s64 period; /* unit: ns */
struct hrtimer timer;
- atomic_t pending; /* accumulated triggered timers */
- bool reinject;
- struct kvm *kvm;
u32 speaker_data_on;
+
struct mutex lock;
+ struct kvm *kvm;
struct kvm_pit *pit;
+ bool reinject;
+ atomic_t pending; /* accumulated triggered timers */
atomic_t irq_ack;
struct kvm_irq_ack_notifier irq_ack_notifier;
};
@@ -59,7 +61,6 @@ struct kvm_pit {
struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags);
void kvm_free_pit(struct kvm *kvm);
-void kvm_pit_reset(struct kvm_pit *pit);
void kvm_pit_load_count(struct kvm_pit *pit, int channel, u32 val,
int hpet_legacy_start);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a88e1a3eeb69..ce4e91db5bae 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3661,9 +3661,9 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
{
if (!kvm->arch.vpit)
return -ENXIO;
- mutex_lock(&kvm->arch.vpit->pit_state.lock);
+
kvm->arch.vpit->pit_state.reinject = control->pit_reinject;
- mutex_unlock(&kvm->arch.vpit->pit_state.lock);
+
return 0;
}
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 08/16] KVM: x86: remove notifiers from PIT discard policy
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (6 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 07/16] KVM: x86: remove unnecessary uses of PIT state lock Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 09/16] KVM: x86: refactor kvm_create_pit Radim Krčmář
` (8 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
Discard policy doesn't rely on information from notifiers, so we don't
need to register notifiers unconditionally. We kept correct counts in
case userspace switched between policies during runtime, but that can be
avoided by reseting the state.
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 38 +++++++++++++++++++++++++++-----------
arch/x86/kvm/i8254.h | 1 +
arch/x86/kvm/x86.c | 12 ++++++++++--
3 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index b8582fbe4fcf..7a2f14bdf4b5 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -225,7 +225,7 @@ static void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian)
* inc(pending) in pit_timer_fn and xchg(irq_ack, 0) in pit_do_work.
*/
smp_mb();
- if (atomic_dec_if_positive(&ps->pending) > 0 && ps->reinject)
+ if (atomic_dec_if_positive(&ps->pending) > 0)
queue_kthread_work(&ps->pit->worker, &ps->pit->expired);
}
@@ -301,6 +301,27 @@ static inline void kvm_pit_reset_reinject(struct kvm_pit *pit)
atomic_set(&pit->pit_state.irq_ack, 1);
}
+void kvm_pit_set_reinject(struct kvm_pit *pit, bool reinject)
+{
+ struct kvm_kpit_state *ps = &pit->pit_state;
+ struct kvm *kvm = pit->kvm;
+
+ if (ps->reinject == reinject)
+ return;
+
+ if (reinject) {
+ /* The initial state is preserved while ps->reinject == 0. */
+ kvm_pit_reset_reinject(pit);
+ kvm_register_irq_ack_notifier(kvm, &ps->irq_ack_notifier);
+ kvm_register_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
+ } else {
+ kvm_unregister_irq_ack_notifier(kvm, &ps->irq_ack_notifier);
+ kvm_unregister_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
+ }
+
+ ps->reinject = reinject;
+}
+
static void create_pit_timer(struct kvm_pit *pit, u32 val, int is_period)
{
struct kvm_kpit_state *ps = &pit->pit_state;
@@ -681,15 +702,14 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
pit_state = &pit->pit_state;
pit_state->pit = pit;
hrtimer_init(&pit_state->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
+
pit_state->irq_ack_notifier.gsi = 0;
pit_state->irq_ack_notifier.irq_acked = kvm_pit_ack_irq;
- kvm_register_irq_ack_notifier(kvm, &pit_state->irq_ack_notifier);
- pit_state->reinject = true;
+ pit->mask_notifier.func = pit_mask_notifer;
kvm_pit_reset(pit);
- pit->mask_notifier.func = pit_mask_notifer;
- kvm_register_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
+ kvm_pit_set_reinject(pit, true);
kvm_iodevice_init(&pit->dev, &pit_dev_ops);
ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, KVM_PIT_BASE_ADDRESS,
@@ -712,8 +732,7 @@ fail_unregister:
kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->dev);
fail:
- kvm_unregister_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
- kvm_unregister_irq_ack_notifier(kvm, &pit_state->irq_ack_notifier);
+ kvm_pit_set_reinject(pit, false);
kvm_free_irq_source_id(kvm, pit->irq_source_id);
kthread_stop(pit->worker_task);
kfree(pit);
@@ -728,10 +747,7 @@ void kvm_free_pit(struct kvm *kvm)
kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &kvm->arch.vpit->dev);
kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS,
&kvm->arch.vpit->speaker_dev);
- kvm_unregister_irq_mask_notifier(kvm, 0,
- &kvm->arch.vpit->mask_notifier);
- kvm_unregister_irq_ack_notifier(kvm,
- &kvm->arch.vpit->pit_state.irq_ack_notifier);
+ kvm_pit_set_reinject(kvm->arch.vpit, false);
timer = &kvm->arch.vpit->pit_state.timer;
hrtimer_cancel(timer);
flush_kthread_work(&kvm->arch.vpit->expired);
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index 840fbb3cb626..1945635904a7 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -63,5 +63,6 @@ void kvm_free_pit(struct kvm *kvm);
void kvm_pit_load_count(struct kvm_pit *pit, int channel, u32 val,
int hpet_legacy_start);
+void kvm_pit_set_reinject(struct kvm_pit *pit, bool reinject);
#endif
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ce4e91db5bae..76f9f48898a5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3659,10 +3659,18 @@ static int kvm_vm_ioctl_set_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
static int kvm_vm_ioctl_reinject(struct kvm *kvm,
struct kvm_reinject_control *control)
{
- if (!kvm->arch.vpit)
+ struct kvm_pit *pit = kvm->arch.vpit;
+
+ if (!pit)
return -ENXIO;
- kvm->arch.vpit->pit_state.reinject = control->pit_reinject;
+ /* pit->pit_state.lock was overloaded to prevent userspace from getting
+ * an inconsistent state after running multiple KVM_REINJECT_CONTROL
+ * ioctls in parallel. Use a separate lock if that ioctl isn't rare.
+ */
+ mutex_lock(&pit->pit_state.lock);
+ kvm_pit_set_reinject(pit, control->pit_reinject);
+ mutex_unlock(&pit->pit_state.lock);
return 0;
}
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 09/16] KVM: x86: refactor kvm_create_pit
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (7 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 08/16] KVM: x86: remove notifiers from PIT discard policy Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 10/16] KVM: x86: refactor kvm_free_pit Radim Krčmář
` (7 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
Locks are gone, so we don't need to duplicate error paths.
Use goto everywhere.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 7a2f14bdf4b5..c24735ae1871 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -676,10 +676,8 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
return NULL;
pit->irq_source_id = kvm_request_irq_source_id(kvm);
- if (pit->irq_source_id < 0) {
- kfree(pit);
- return NULL;
- }
+ if (pit->irq_source_id < 0)
+ goto fail_request;
mutex_init(&pit->pit_state.lock);
@@ -690,11 +688,9 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
init_kthread_worker(&pit->worker);
pit->worker_task = kthread_run(kthread_worker_fn, &pit->worker,
"kvm-pit/%d", pid_nr);
- if (IS_ERR(pit->worker_task)) {
- kvm_free_irq_source_id(kvm, pit->irq_source_id);
- kfree(pit);
- return NULL;
- }
+ if (IS_ERR(pit->worker_task))
+ goto fail_kthread;
+
init_kthread_work(&pit->expired, pit_do_work);
pit->kvm = kvm;
@@ -715,7 +711,7 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, KVM_PIT_BASE_ADDRESS,
KVM_PIT_MEM_LENGTH, &pit->dev);
if (ret < 0)
- goto fail;
+ goto fail_register_pit;
if (flags & KVM_PIT_SPEAKER_DUMMY) {
kvm_iodevice_init(&pit->speaker_dev, &speaker_dev_ops);
@@ -723,18 +719,19 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
KVM_SPEAKER_BASE_ADDRESS, 4,
&pit->speaker_dev);
if (ret < 0)
- goto fail_unregister;
+ goto fail_register_speaker;
}
return pit;
-fail_unregister:
+fail_register_speaker:
kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->dev);
-
-fail:
+fail_register_pit:
kvm_pit_set_reinject(pit, false);
- kvm_free_irq_source_id(kvm, pit->irq_source_id);
kthread_stop(pit->worker_task);
+fail_kthread:
+ kvm_free_irq_source_id(kvm, pit->irq_source_id);
+fail_request:
kfree(pit);
return NULL;
}
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 10/16] KVM: x86: refactor kvm_free_pit
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (8 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 09/16] KVM: x86: refactor kvm_create_pit Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 11/16] KVM: x86: remove pit and kvm from kvm_kpit_state Radim Krčmář
` (6 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
Could be easier to read, but git history will become deeper.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index c24735ae1871..055018e833b6 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -738,18 +738,16 @@ fail_request:
void kvm_free_pit(struct kvm *kvm)
{
- struct hrtimer *timer;
+ struct kvm_pit *pit = kvm->arch.vpit;
- if (kvm->arch.vpit) {
- kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &kvm->arch.vpit->dev);
- kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS,
- &kvm->arch.vpit->speaker_dev);
- kvm_pit_set_reinject(kvm->arch.vpit, false);
- timer = &kvm->arch.vpit->pit_state.timer;
- hrtimer_cancel(timer);
- flush_kthread_work(&kvm->arch.vpit->expired);
- kthread_stop(kvm->arch.vpit->worker_task);
- kvm_free_irq_source_id(kvm, kvm->arch.vpit->irq_source_id);
- kfree(kvm->arch.vpit);
+ if (pit) {
+ kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->dev);
+ kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->speaker_dev);
+ kvm_pit_set_reinject(pit, false);
+ hrtimer_cancel(&pit->pit_state.timer);
+ flush_kthread_work(&pit->expired);
+ kthread_stop(pit->worker_task);
+ kvm_free_irq_source_id(kvm, pit->irq_source_id);
+ kfree(pit);
}
}
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 11/16] KVM: x86: remove pit and kvm from kvm_kpit_state
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (9 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 10/16] KVM: x86: refactor kvm_free_pit Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 12/16] KVM: x86: remove pointless dereference of PIT Radim Krčmář
` (5 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
kvm isn't ever used and pit can be accessed with container_of.
If you *really* need kvm, pit_state_to_pit(ps)->kvm.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 14 +++++++++-----
arch/x86/kvm/i8254.h | 2 --
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 055018e833b6..37e665c5f307 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -215,10 +215,16 @@ static void pit_latch_status(struct kvm_pit *pit, int channel)
}
}
+static inline struct kvm_pit *pit_state_to_pit(struct kvm_kpit_state *ps)
+{
+ return container_of(ps, struct kvm_pit, pit_state);
+}
+
static void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian)
{
struct kvm_kpit_state *ps = container_of(kian, struct kvm_kpit_state,
irq_ack_notifier);
+ struct kvm_pit *pit = pit_state_to_pit(ps);
atomic_set(&ps->irq_ack, 1);
/* irq_ack should be set before pending is read. Order accesses with
@@ -226,7 +232,7 @@ static void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian)
*/
smp_mb();
if (atomic_dec_if_positive(&ps->pending) > 0)
- queue_kthread_work(&ps->pit->worker, &ps->pit->expired);
+ queue_kthread_work(&pit->worker, &pit->expired);
}
void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu)
@@ -281,7 +287,7 @@ static void pit_do_work(struct kthread_work *work)
static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
{
struct kvm_kpit_state *ps = container_of(data, struct kvm_kpit_state, timer);
- struct kvm_pit *pt = ps->kvm->arch.vpit;
+ struct kvm_pit *pt = pit_state_to_pit(ps);
if (ps->reinject)
atomic_inc(&ps->pending);
@@ -338,12 +344,11 @@ static void create_pit_timer(struct kvm_pit *pit, u32 val, int is_period)
/* TODO The new value only affected after the retriggered */
hrtimer_cancel(&ps->timer);
- flush_kthread_work(&ps->pit->expired);
+ flush_kthread_work(&pit->expired);
ps->period = interval;
ps->is_periodic = is_period;
ps->timer.function = pit_timer_fn;
- ps->kvm = pit->kvm;
kvm_pit_reset_reinject(pit);
@@ -696,7 +701,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
pit->kvm = kvm;
pit_state = &pit->pit_state;
- pit_state->pit = pit;
hrtimer_init(&pit_state->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
pit_state->irq_ack_notifier.gsi = 0;
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index 1945635904a7..f365dce4fb8d 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -31,8 +31,6 @@ struct kvm_kpit_state {
u32 speaker_data_on;
struct mutex lock;
- struct kvm *kvm;
- struct kvm_pit *pit;
bool reinject;
atomic_t pending; /* accumulated triggered timers */
atomic_t irq_ack;
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 12/16] KVM: x86: remove pointless dereference of PIT
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (10 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 11/16] KVM: x86: remove pit and kvm from kvm_kpit_state Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 13/16] KVM: x86: don't assume layout of kvm_kpit_state Radim Krčmář
` (4 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
PIT is known at that point.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 37e665c5f307..964902b33eed 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -267,8 +267,8 @@ static void pit_do_work(struct kthread_work *work)
if (ps->reinject && !atomic_xchg(&ps->irq_ack, 0))
return;
- kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1, false);
- kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0, false);
+ kvm_set_irq(kvm, pit->irq_source_id, 0, 1, false);
+ kvm_set_irq(kvm, pit->irq_source_id, 0, 0, false);
/*
* Provides NMI watchdog support via Virtual Wire mode.
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 13/16] KVM: x86: don't assume layout of kvm_kpit_state
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (11 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 12/16] KVM: x86: remove pointless dereference of PIT Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 14/16] KVM: x86: move PIT timer function initialization Radim Krčmář
` (3 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
channels has offset 0 and correct size now, but that can change.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/x86.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 76f9f48898a5..60d6c0036a98 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3604,9 +3604,13 @@ static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
{
- mutex_lock(&kvm->arch.vpit->pit_state.lock);
- memcpy(ps, &kvm->arch.vpit->pit_state, sizeof(struct kvm_pit_state));
- mutex_unlock(&kvm->arch.vpit->pit_state.lock);
+ struct kvm_kpit_state *kps = &kvm->arch.vpit->pit_state;
+
+ BUILD_BUG_ON(sizeof(*ps) != sizeof(kps->channels));
+
+ mutex_lock(&kps->lock);
+ memcpy(ps, &kps->channels, sizeof(*ps));
+ mutex_unlock(&kps->lock);
return 0;
}
@@ -3616,7 +3620,7 @@ static int kvm_vm_ioctl_set_pit(struct kvm *kvm, struct kvm_pit_state *ps)
struct kvm_pit *pit = kvm->arch.vpit;
mutex_lock(&pit->pit_state.lock);
- memcpy(&pit->pit_state, ps, sizeof(struct kvm_pit_state));
+ memcpy(&pit->pit_state.channels, ps, sizeof(*ps));
for (i = 0; i < 3; i++)
kvm_pit_load_count(pit, i, ps->channels[i].count, 0);
mutex_unlock(&pit->pit_state.lock);
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 14/16] KVM: x86: move PIT timer function initialization
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (12 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 13/16] KVM: x86: don't assume layout of kvm_kpit_state Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 15/16] KVM: x86: turn kvm_kpit_state.reinject into atomic_t Radim Krčmář
` (2 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
We can do it just once.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
arch/x86/kvm/i8254.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 964902b33eed..68af4445d51d 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -348,8 +348,6 @@ static void create_pit_timer(struct kvm_pit *pit, u32 val, int is_period)
ps->period = interval;
ps->is_periodic = is_period;
- ps->timer.function = pit_timer_fn;
-
kvm_pit_reset_reinject(pit);
/*
@@ -702,6 +700,7 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
pit_state = &pit->pit_state;
hrtimer_init(&pit_state->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
+ pit_state->timer.function = pit_timer_fn;
pit_state->irq_ack_notifier.gsi = 0;
pit_state->irq_ack_notifier.irq_acked = kvm_pit_ack_irq;
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 15/16] KVM: x86: turn kvm_kpit_state.reinject into atomic_t
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (13 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 14/16] KVM: x86: move PIT timer function initialization Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 16/16] KVM: document KVM_REINJECT_CONTROL ioctl Radim Krčmář
2016-03-03 14:57 ` [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Paolo Bonzini
16 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
Document possible races between readers and concurrent update to the
ioctl.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
v3: new
arch/x86/kvm/i8254.c | 8 ++++----
arch/x86/kvm/i8254.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 68af4445d51d..219ef855aae5 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -264,7 +264,7 @@ static void pit_do_work(struct kthread_work *work)
int i;
struct kvm_kpit_state *ps = &pit->pit_state;
- if (ps->reinject && !atomic_xchg(&ps->irq_ack, 0))
+ if (atomic_read(&ps->reinject) && !atomic_xchg(&ps->irq_ack, 0))
return;
kvm_set_irq(kvm, pit->irq_source_id, 0, 1, false);
@@ -289,7 +289,7 @@ static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
struct kvm_kpit_state *ps = container_of(data, struct kvm_kpit_state, timer);
struct kvm_pit *pt = pit_state_to_pit(ps);
- if (ps->reinject)
+ if (atomic_read(&ps->reinject))
atomic_inc(&ps->pending);
queue_kthread_work(&pt->worker, &pt->expired);
@@ -312,7 +312,7 @@ void kvm_pit_set_reinject(struct kvm_pit *pit, bool reinject)
struct kvm_kpit_state *ps = &pit->pit_state;
struct kvm *kvm = pit->kvm;
- if (ps->reinject == reinject)
+ if (atomic_read(&ps->reinject) == reinject)
return;
if (reinject) {
@@ -325,7 +325,7 @@ void kvm_pit_set_reinject(struct kvm_pit *pit, bool reinject)
kvm_unregister_irq_mask_notifier(kvm, 0, &pit->mask_notifier);
}
- ps->reinject = reinject;
+ atomic_set(&ps->reinject, reinject);
}
static void create_pit_timer(struct kvm_pit *pit, u32 val, int is_period)
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index f365dce4fb8d..2f5af0798326 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -31,7 +31,7 @@ struct kvm_kpit_state {
u32 speaker_data_on;
struct mutex lock;
- bool reinject;
+ atomic_t reinject;
atomic_t pending; /* accumulated triggered timers */
atomic_t irq_ack;
struct kvm_irq_ack_notifier irq_ack_notifier;
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 16/16] KVM: document KVM_REINJECT_CONTROL ioctl
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (14 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 15/16] KVM: x86: turn kvm_kpit_state.reinject into atomic_t Radim Krčmář
@ 2016-03-02 21:56 ` Radim Krčmář
2016-03-03 14:29 ` Paolo Bonzini
2016-03-03 14:57 ` [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Paolo Bonzini
16 siblings, 1 reply; 20+ messages in thread
From: Radim Krčmář @ 2016-03-02 21:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Yuki Shibuya
From: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
v3: new, one result of a long discussion with Paolo
Documentation/virtual/kvm/api.txt | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 4a661e555c09..777aefc77743 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3035,6 +3035,30 @@ Returns: 0 on success, -1 on error
Queues an SMI on the thread's vcpu.
+4.97 KVM_REINJECT_CONTROL
+
+Capability: KVM_CAP_REINJECT_CONTROL
+Architectures: x86
+Type: vm ioctl
+Parameters: struct kvm_reinject_control (in)
+Returns: 0 on success,
+ -EFAULT if struct kvm_reinject_control cannot be read,
+ -ENXIO if KVM_CREATE_PIT or KVM_CREATE_PIT2 didn't succeed earlier.
+
+i8254 (PIT) has two modes, reinject and !reinject. The default is reinject,
+where KVM queues elapsed i8254 ticks and monitors completion of interrupt from
+vector(s) that i8254 injects. Reinject mode dequeues a tick and injects its
+interrupt whenever there isn't a pending interrupt from i8254.
+!reinject mode injects an interrupt as soon as a tick arrives.
+
+struct kvm_reinject_control {
+ __u8 pit_reinject;
+ __u8 reserved[31];
+};
+
+pit_reinject = 0 (!reinject mode) is recommended.
+
+
5. The kvm_run structure
------------------------
--
2.7.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 16/16] KVM: document KVM_REINJECT_CONTROL ioctl
2016-03-02 21:56 ` [PATCH v3 16/16] KVM: document KVM_REINJECT_CONTROL ioctl Radim Krčmář
@ 2016-03-03 14:29 ` Paolo Bonzini
2016-03-03 16:10 ` Radim Krčmář
0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2016-03-03 14:29 UTC (permalink / raw)
To: Radim Krčmář, linux-kernel; +Cc: kvm, Yuki Shibuya
On 02/03/2016 22:56, Radim Krčmář wrote:
> +
> +pit_reinject = 0 (!reinject mode) is recommended.
What about:
pit_reinject = 0 (!reinject mode) is recommended, unless running an old
operating system that uses the PIT for timing (e.g. Linux 2.4.x).
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
` (15 preceding siblings ...)
2016-03-02 21:56 ` [PATCH v3 16/16] KVM: document KVM_REINJECT_CONTROL ioctl Radim Krčmář
@ 2016-03-03 14:57 ` Paolo Bonzini
16 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2016-03-03 14:57 UTC (permalink / raw)
To: Radim Krčmář, linux-kernel; +Cc: kvm, Yuki Shibuya
On 02/03/2016 22:56, Radim Krčmář wrote:
> From: Radim Krčmář <rkrcmar@redhat.com>
>
> v3
> - mark patches reviewed by Paolo [1,9-14/16]
> - properly categorize reinject struct member [Paolo] [7/16]
> - use atomic_t to document reinject usage [Paolo] [15/16]
> - document PIT reinject modes [16/16]
>
> v2: http://www.spinics.net/lists/kvm/msg127927.html
>
> Radim Krčmář (16):
> KVM: x86: change PIT discard tick policy
> KVM: x86: simplify atomics in kvm_pit_ack_irq
> KVM: x86: add kvm_pit_reset_reinject
> KVM: x86: use atomic_t instead of pit.inject_lock
> KVM: x86: tone down WARN_ON pit.state_lock
> KVM: x86: pass struct kvm_pit instead of kvm in PIT
> KVM: x86: remove unnecessary uses of PIT state lock
> KVM: x86: remove notifiers from PIT discard policy
> KVM: x86: refactor kvm_create_pit
> KVM: x86: refactor kvm_free_pit
> KVM: x86: remove pit and kvm from kvm_kpit_state
> KVM: x86: remove pointless dereference of PIT
> KVM: x86: don't assume layout of kvm_kpit_state
> KVM: x86: move PIT timer function initialization
> KVM: x86: turn kvm_kpit_state.reinject into atomic_t
> KVM: document KVM_REINJECT_CONTROL ioctl
>
> Documentation/virtual/kvm/api.txt | 24 +++
> arch/x86/kvm/i8254.c | 320 +++++++++++++++++---------------------
> arch/x86/kvm/i8254.h | 17 +-
> arch/x86/kvm/x86.c | 52 ++++---
> 4 files changed, 213 insertions(+), 200 deletions(-)
>
Applied to kvm/queue, thanks.
Paolo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 16/16] KVM: document KVM_REINJECT_CONTROL ioctl
2016-03-03 14:29 ` Paolo Bonzini
@ 2016-03-03 16:10 ` Radim Krčmář
0 siblings, 0 replies; 20+ messages in thread
From: Radim Krčmář @ 2016-03-03 16:10 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm, Yuki Shibuya
2016-03-03 15:29+0100, Paolo Bonzini:
> On 02/03/2016 22:56, Radim Krčmář wrote:
>> +
>> +pit_reinject = 0 (!reinject mode) is recommended.
>
> What about:
>
> pit_reinject = 0 (!reinject mode) is recommended, unless running an old
> operating system that uses the PIT for timing (e.g. Linux 2.4.x).
Not giving any recommendation in some cases is clever. Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-03-03 16:10 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-02 21:56 [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 01/16] KVM: x86: change PIT discard tick policy Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 02/16] KVM: x86: simplify atomics in kvm_pit_ack_irq Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 03/16] KVM: x86: add kvm_pit_reset_reinject Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 04/16] KVM: x86: use atomic_t instead of pit.inject_lock Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 05/16] KVM: x86: tone down WARN_ON pit.state_lock Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 06/16] KVM: x86: pass struct kvm_pit instead of kvm in PIT Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 07/16] KVM: x86: remove unnecessary uses of PIT state lock Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 08/16] KVM: x86: remove notifiers from PIT discard policy Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 09/16] KVM: x86: refactor kvm_create_pit Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 10/16] KVM: x86: refactor kvm_free_pit Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 11/16] KVM: x86: remove pit and kvm from kvm_kpit_state Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 12/16] KVM: x86: remove pointless dereference of PIT Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 13/16] KVM: x86: don't assume layout of kvm_kpit_state Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 14/16] KVM: x86: move PIT timer function initialization Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 15/16] KVM: x86: turn kvm_kpit_state.reinject into atomic_t Radim Krčmář
2016-03-02 21:56 ` [PATCH v3 16/16] KVM: document KVM_REINJECT_CONTROL ioctl Radim Krčmář
2016-03-03 14:29 ` Paolo Bonzini
2016-03-03 16:10 ` Radim Krčmář
2016-03-03 14:57 ` [PATCH v3 00/16] KVM: x86: change PIT discard policy and untangle related code Paolo Bonzini
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).