Dedicated instances are currently disturbed by unnecessary jitter due
to the emulated lapic timers fire on the same pCPUs which vCPUs resident.
There is no hardware virtual timer on Intel for guest like ARM. Both
programming timer in guest and the emulated timer fires incur vmexits.
This patchset tries to avoid vmexit which is incurred by the emulated
timer fires in dedicated instance scenario.
When nohz_full is enabled in dedicated instances scenario, the unpinned
timer will be moved to the nearest busy housekeepers after commit 444969223c8
("sched/nohz: Fix affine unpinned timers mess"). However, KVM always makes
lapic timer pinned to the pCPU which vCPU residents, the reason is explained
by commit 61abdbe0 (kvm: x86: make lapic hrtimer pinned). Actually, these
emulated timers can be offload to the housekeeping cpus since APICv
is really common in recent years. The guest timer interrupt is injected by
posted-interrupt which is delivered by housekeeping cpu once the emulated
timer fires.
This patchset introduces a new kvm module parameter, it is false by default.
The host admin can enable it after fine tuned, e.g. dedicated instances
scenario w/ nohz_full cover the pCPUs which vCPUs resident, several pCPUs
surplus for housekeeping, disable mwait/hlt/pause vmexits to occupy the
pCPUs, fortunately preemption timer is disabled after mwait is exposed
to guest which makes emulated timer offload can be possible.
3%~5% redis performance benefit can be observed on Skylake server.
Wanpeng Li (3):
KVM: LAPIC: Make lapic timer unpinned when timer is injected by posted-interrupt
KVM: LAPIC: lapic timer is injected by posted interrupt
KVM: LAPIC: Ignore timer migration when lapic timer is injected by
posted-interrupt
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/lapic.c | 45 +++++++++++++++++++++++++++++++++++------
arch/x86/kvm/svm.c | 5 +++++
arch/x86/kvm/vmx/vmx.c | 9 +++++++++
4 files changed, 54 insertions(+), 6 deletions(-)
--
2.7.4
From: Wanpeng Li <[email protected]>
Make lapic timer unpinned when timer is injected by posted-interrupt,
the emulated timer can be offload to the housekeeping cpus.
This patch introduces a new kvm module parameter, it is false by default.
The host admin can enable it after fine tuned, e.g. dedicated instances
scenario w/ nohz_full cover the pCPUs which vCPUs resident, several pCPUs
surplus for housekeeping, disable mwait/hlt/pause vmexits to occupy the
pCPUs, fortunately preemption timer is disabled after mwait is exposed
to guest which makes emulated timer offload can be possible.
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/lapic.c | 20 ++++++++++++++++----
arch/x86/kvm/svm.c | 5 +++++
arch/x86/kvm/vmx/vmx.c | 9 +++++++++
4 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index aeadbc7..ccb3d61 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1090,6 +1090,7 @@ struct kvm_x86_ops {
void (*set_virtual_apic_mode)(struct kvm_vcpu *vcpu);
void (*set_apic_access_page_addr)(struct kvm_vcpu *vcpu, hpa_t hpa);
void (*deliver_posted_interrupt)(struct kvm_vcpu *vcpu, int vector);
+ bool (*pi_inject_timer_enabled)(struct kvm_vcpu *vcpu);
int (*sync_pir_to_irr)(struct kvm_vcpu *vcpu);
int (*set_tss_addr)(struct kvm *kvm, unsigned int addr);
int (*set_identity_map_addr)(struct kvm *kvm, u64 ident_addr);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index fcf42a3..8c9c14d 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -127,6 +127,12 @@ static inline u32 kvm_x2apic_id(struct kvm_lapic *apic)
return apic->vcpu->vcpu_id;
}
+static inline bool posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
+{
+ return (kvm_x86_ops->pi_inject_timer_enabled(vcpu) &&
+ kvm_mwait_in_guest(vcpu->kvm));
+}
+
static inline bool kvm_apic_map_get_logical_dest(struct kvm_apic_map *map,
u32 dest_id, struct kvm_lapic ***cluster, u16 *mask) {
switch (map->mode) {
@@ -1581,7 +1587,9 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic)
likely(ns > apic->lapic_timer.timer_advance_ns)) {
expire = ktime_add_ns(now, ns);
expire = ktime_sub_ns(expire, ktimer->timer_advance_ns);
- hrtimer_start(&ktimer->timer, expire, HRTIMER_MODE_ABS_PINNED);
+ hrtimer_start(&ktimer->timer, expire,
+ posted_interrupt_inject_timer(vcpu) ?
+ HRTIMER_MODE_ABS : HRTIMER_MODE_ABS_PINNED);
} else
apic_timer_expired(apic);
@@ -1683,7 +1691,8 @@ static void start_sw_period(struct kvm_lapic *apic)
hrtimer_start(&apic->lapic_timer.timer,
apic->lapic_timer.target_expiration,
- HRTIMER_MODE_ABS_PINNED);
+ posted_interrupt_inject_timer(apic->vcpu) ?
+ HRTIMER_MODE_ABS : HRTIMER_MODE_ABS_PINNED);
}
bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu)
@@ -2320,7 +2329,8 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
apic->vcpu = vcpu;
hrtimer_init(&apic->lapic_timer.timer, CLOCK_MONOTONIC,
- HRTIMER_MODE_ABS_PINNED);
+ posted_interrupt_inject_timer(vcpu) ?
+ HRTIMER_MODE_ABS : HRTIMER_MODE_ABS_PINNED);
apic->lapic_timer.timer.function = apic_timer_fn;
if (timer_advance_ns == -1) {
apic->lapic_timer.timer_advance_ns = 1000;
@@ -2509,7 +2519,9 @@ void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
timer = &vcpu->arch.apic->lapic_timer.timer;
if (hrtimer_cancel(timer))
- hrtimer_start_expires(timer, HRTIMER_MODE_ABS_PINNED);
+ hrtimer_start_expires(timer,
+ posted_interrupt_inject_timer(vcpu) ?
+ HRTIMER_MODE_ABS : HRTIMER_MODE_ABS_PINNED);
}
/*
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 302cb40..aee1d91 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -5173,6 +5173,11 @@ static void svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec)
kvm_vcpu_wake_up(vcpu);
}
+static inline bool svm_posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
+{
+ return false;
+}
+
static void svm_ir_list_del(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi)
{
unsigned long flags;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index da24f18..2b4fd61 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -101,6 +101,9 @@ module_param(fasteoi, bool, S_IRUGO);
static bool __read_mostly enable_apicv = 1;
module_param(enable_apicv, bool, S_IRUGO);
+bool __read_mostly pi_inject_timer = 0;
+module_param(pi_inject_timer, bool, S_IRUGO);
+
/*
* If nested=1, nested virtualization is supported, i.e., guests may use
* VMX and be a hypervisor for its own guests. If nested=0, guests may not
@@ -3724,6 +3727,11 @@ static void vmx_deliver_posted_interrupt(struct kvm_vcpu *vcpu, int vector)
kvm_vcpu_kick(vcpu);
}
+static bool vmx_pi_inject_timer_enabled(struct kvm_vcpu *vcpu)
+{
+ return pi_inject_timer && kvm_vcpu_apicv_active(vcpu);
+}
+
/*
* Set up the vmcs's constant host-state fields, i.e., host-state fields that
* will not change in the lifetime of the guest.
@@ -7671,6 +7679,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
.guest_apic_has_interrupt = vmx_guest_apic_has_interrupt,
.sync_pir_to_irr = vmx_sync_pir_to_irr,
.deliver_posted_interrupt = vmx_deliver_posted_interrupt,
+ .pi_inject_timer_enabled = vmx_pi_inject_timer_enabled,
.set_tss_addr = vmx_set_tss_addr,
.set_identity_map_addr = vmx_set_identity_map_addr,
--
2.7.4
From: Wanpeng Li <[email protected]>
When lapic timer is injected by posted-interrupt, the emulated timer is
offload to the housekeeping cpu. The timer interrupt will be delivered
properly, no need to migrate timer.
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
arch/x86/kvm/lapic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index e9db086..3bb4376 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2534,7 +2534,8 @@ void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
{
struct hrtimer *timer;
- if (!lapic_in_kernel(vcpu))
+ if (!lapic_in_kernel(vcpu) ||
+ posted_interrupt_inject_timer(vcpu))
return;
timer = &vcpu->arch.apic->lapic_timer.timer;
--
2.7.4
From: Wanpeng Li <[email protected]>
Dedicated instances are currently disturbed by unnecessary jitter due
to the emulated lapic timers fire on the same pCPUs which vCPUs resident.
There is no hardware virtual timer on Intel for guest like ARM. Both
programming timer in guest and the emulated timer fires incur vmexits.
This patchset tries to avoid vmexit which is incurred by the emulated
timer fires in dedicated instance scenario.
When nohz_full is enabled in dedicated instances scenario, the unpinned
timer will be moved to the nearest busy housekeepers after commit 444969223c8
("sched/nohz: Fix affine unpinned timers mess"). However, KVM always makes
lapic timer pinned to the pCPU which vCPU residents, the reason is explained
by commit 61abdbe0 (kvm: x86: make lapic hrtimer pinned). Actually, these
emulated timers can be offload to the housekeeping cpus since APICv
is really common in recent years. The guest timer interrupt is injected by
posted-interrupt which is delivered by housekeeping cpu once the emulated
timer fires.
3%~5% redis performance benefit can be observed on Skylake server.
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
arch/x86/kvm/lapic.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 8c9c14d..e9db086 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1465,6 +1465,23 @@ static void apic_timer_expired(struct kvm_lapic *apic)
}
/*
+ * On APICv, lapic timer is injected by posted interrupt
+ * to dedicated instance.
+ */
+static void apic_timer_expired_pi(struct kvm_lapic *apic)
+{
+ struct kvm_timer *ktimer = &apic->lapic_timer;
+
+ kvm_apic_local_deliver(apic, APIC_LVTT);
+ if (apic_lvtt_tscdeadline(apic))
+ ktimer->tscdeadline = 0;
+ if (apic_lvtt_oneshot(apic)) {
+ ktimer->tscdeadline = 0;
+ ktimer->target_expiration = 0;
+ }
+}
+
+/*
* On APICv, this test will cause a busy wait
* during a higher-priority task.
*/
@@ -2297,7 +2314,10 @@ static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
struct kvm_timer *ktimer = container_of(data, struct kvm_timer, timer);
struct kvm_lapic *apic = container_of(ktimer, struct kvm_lapic, lapic_timer);
- apic_timer_expired(apic);
+ if (unlikely(posted_interrupt_inject_timer(apic->vcpu)))
+ apic_timer_expired_pi(apic);
+ else
+ apic_timer_expired(apic);
if (lapic_is_periodic(apic)) {
advance_periodic_target_expiration(apic);
--
2.7.4
On 05/06/19 12:09, Wanpeng Li wrote:
> +static void apic_timer_expired_pi(struct kvm_lapic *apic)
> +{
> + struct kvm_timer *ktimer = &apic->lapic_timer;
> +
> + kvm_apic_local_deliver(apic, APIC_LVTT);
> + if (apic_lvtt_tscdeadline(apic))
> + ktimer->tscdeadline = 0;
> + if (apic_lvtt_oneshot(apic)) {
> + ktimer->tscdeadline = 0;
> + ktimer->target_expiration = 0;
> + }
> +}
Please rename this function to kvm_apic_inject_pending_timer_irqs and
call it from kvm_inject_apic_timer_irqs.
Then apic_timer_expired can just do
if (atomic_read(&apic->lapic_timer.pending))
return;
+ if (unlikely(posted_interrupt_inject_timer(apic->vcpu))) {
+ kvm_apic_inject_pending_timer_irqs(apic);
+ return;
+ }
etc.
Paolo
On 05/06/19 12:09, Wanpeng Li wrote:
> +static inline bool posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
> +{
> + return (kvm_x86_ops->pi_inject_timer_enabled(vcpu) &&
> + kvm_mwait_in_guest(vcpu->kvm));
> +}
> +
Here you need to check kvm_halt_in_guest, not kvm_mwait_in_guest,
because you need to go through kvm_apic_expired if the guest needs to be
woken up from kvm_vcpu_block.
There is a case when you get to kvm_vcpu_block with kvm_halt_in_guest,
which is when the guest disables asynchronous page faults. Currently,
timer interrupts are delivered while apf.halted = true, with this change
they wouldn't. I would just disable KVM_REQ_APF_HALT in
kvm_can_do_async_pf if kvm_halt_in_guest is true, let me send a patch
for that later.
When you do this, I think you don't need the
kvm_x86_ops->pi_inject_timer_enabled check at all, because if we know
that the vCPU cannot be asleep in kvm_vcpu_block, then we can inject the
timer interrupt immediately with __apic_accept_irq (if APICv is
disabled, it will set IRR and do kvm_make_request + kvm_vcpu_kick).
You can keep the module parameter, mostly for debugging reasons, but
please move it from kvm-intel to kvm, and add something like
diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index 123ea07a3f3b..1cc7973c382e 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -14,6 +14,11 @@
static cpumask_var_t housekeeping_mask;
static unsigned int housekeeping_flags;
+bool housekeeping_enabled(enum hk_flags flags)
+{
+ return !!(housekeeping_flags & flags);
+}
+
int housekeeping_any_cpu(enum hk_flags flags)
{
if (static_branch_unlikely(&housekeeping_overridden))
so that the default for the module parameter can be
housekeeping_enabled(HK_FLAG_TIMER).
Thanks,
Paolo
On Wed, 5 Jun 2019 at 20:30, Paolo Bonzini <[email protected]> wrote:
>
> On 05/06/19 12:09, Wanpeng Li wrote:
> > +static void apic_timer_expired_pi(struct kvm_lapic *apic)
> > +{
> > + struct kvm_timer *ktimer = &apic->lapic_timer;
> > +
> > + kvm_apic_local_deliver(apic, APIC_LVTT);
> > + if (apic_lvtt_tscdeadline(apic))
> > + ktimer->tscdeadline = 0;
> > + if (apic_lvtt_oneshot(apic)) {
> > + ktimer->tscdeadline = 0;
> > + ktimer->target_expiration = 0;
> > + }
> > +}
>
> Please rename this function to kvm_apic_inject_pending_timer_irqs and
> call it from kvm_inject_apic_timer_irqs.
>
> Then apic_timer_expired can just do
>
> if (atomic_read(&apic->lapic_timer.pending))
> return;
>
> + if (unlikely(posted_interrupt_inject_timer(apic->vcpu))) {
> + kvm_apic_inject_pending_timer_irqs(apic);
> + return;
> + }
Do it in v2.
Regards,
Wanpeng Li
On Wed, 5 Jun 2019 at 21:04, Paolo Bonzini <[email protected]> wrote:
>
> On 05/06/19 12:09, Wanpeng Li wrote:
> > +static inline bool posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
> > +{
> > + return (kvm_x86_ops->pi_inject_timer_enabled(vcpu) &&
> > + kvm_mwait_in_guest(vcpu->kvm));
> > +}
> > +
>
> Here you need to check kvm_halt_in_guest, not kvm_mwait_in_guest,
> because you need to go through kvm_apic_expired if the guest needs to be
> woken up from kvm_vcpu_block.
>
> There is a case when you get to kvm_vcpu_block with kvm_halt_in_guest,
> which is when the guest disables asynchronous page faults. Currently,
> timer interrupts are delivered while apf.halted = true, with this change
You are right. I check it in v2 2/3.
> they wouldn't. I would just disable KVM_REQ_APF_HALT in
> kvm_can_do_async_pf if kvm_halt_in_guest is true, let me send a patch
> for that later.
>
> When you do this, I think you don't need the
> kvm_x86_ops->pi_inject_timer_enabled check at all, because if we know
I still keep check mwait and apicv in v2, since w/o mwait exposed, the
emulated timer can't be offload(thanks to preemption timer is
disabled). In addition, w/o posted-interrupt, we can't avoid the
timer fire vmexit.
> that the vCPU cannot be asleep in kvm_vcpu_block, then we can inject the
> timer interrupt immediately with __apic_accept_irq (if APICv is
> disabled, it will set IRR and do kvm_make_request + kvm_vcpu_kick).
>
> You can keep the module parameter, mostly for debugging reasons, but
> please move it from kvm-intel to kvm, and add something like
>
> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> index 123ea07a3f3b..1cc7973c382e 100644
> --- a/kernel/sched/isolation.c
> +++ b/kernel/sched/isolation.c
> @@ -14,6 +14,11 @@
> static cpumask_var_t housekeeping_mask;
> static unsigned int housekeeping_flags;
>
> +bool housekeeping_enabled(enum hk_flags flags)
> +{
> + return !!(housekeeping_flags & flags);
> +}
> +
> int housekeeping_any_cpu(enum hk_flags flags)
> {
> if (static_branch_unlikely(&housekeeping_overridden))
>
> so that the default for the module parameter can be
> housekeeping_enabled(HK_FLAG_TIMER).
Agreed. Thanks for the quick review. :)
Regards,
Wanpeng Li