2022-04-27 16:27:46

by Sasha Levin

Subject: [PATCH MANUALSEL 5.10 1/4] x86/kvm: Preserve BSP MSR_KVM_POLL_CONTROL across suspend/resume

From: Wanpeng Li <[email protected]>

[ Upstream commit 0361bdfddca20c8855ea3bdbbbc9c999912b10ff ]

MSR_KVM_POLL_CONTROL is cleared on reset, thus reverting guests to
host-side polling after suspend/resume. Non-bootstrap CPUs are
restored correctly by the haltpoll driver because they are hot-unplugged
during suspend and hot-plugged during resume; however, the BSP
is not hotpluggable and remains in host-sde polling mode after
the guest resume. The makes the guest pay for the cost of vmexits
every time the guest enters idle.

Fix it by recording BSP's haltpoll state and resuming it during guest
resume.

Cc: Marcelo Tosatti <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
arch/x86/kernel/kvm.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 18e952fed021..6c3d38b5a8ad 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -66,6 +66,7 @@ static DEFINE_PER_CPU_DECRYPTED(struct kvm_vcpu_pv_apf_data, apf_reason) __align
DEFINE_PER_CPU_DECRYPTED(struct kvm_steal_time, steal_time) __aligned(64) __visible;
static int has_steal_clock = 0;

+static int has_guest_poll = 0;
/*
* No need for any "IO delay" on KVM
*/
@@ -624,14 +625,26 @@ static int kvm_cpu_down_prepare(unsigned int cpu)

static int kvm_suspend(void)
{
+ u64 val = 0;
+
kvm_guest_cpu_offline(false);

+#ifdef CONFIG_ARCH_CPUIDLE_HALTPOLL
+ if (kvm_para_has_feature(KVM_FEATURE_POLL_CONTROL))
+ rdmsrl(MSR_KVM_POLL_CONTROL, val);
+ has_guest_poll = !(val & 1);
+#endif
return 0;
}

static void kvm_resume(void)
{
kvm_cpu_online(raw_smp_processor_id());
+
+#ifdef CONFIG_ARCH_CPUIDLE_HALTPOLL
+ if (kvm_para_has_feature(KVM_FEATURE_POLL_CONTROL) && has_guest_poll)
+ wrmsrl(MSR_KVM_POLL_CONTROL, 0);
+#endif
}

static struct syscore_ops kvm_syscore_ops = {
--
2.35.1

2022-04-27 16:27:57

by Sasha Levin

Subject: [PATCH MANUALSEL 5.10 2/4] KVM: x86: Do not change ICR on write to APIC_SELF_IPI

From: Paolo Bonzini <[email protected]>

[ Upstream commit d22a81b304a27fca6124174a8e842e826c193466 ]

Emulating writes to SELF_IPI with a write to ICR has an unwanted side effect:
the value of ICR in vAPIC page gets changed. The lists SELF_IPI as write-only,
with no associated MMIO offset, so any write should have no visible side
effect in the vAPIC page.

Reported-by: Chao Gao <[email protected]>
Reviewed-by: Sean Christopherson <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
arch/x86/kvm/lapic.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index de11149e28e0..e45ebf0870b6 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2106,10 +2106,9 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
break;

case APIC_SELF_IPI:
- if (apic_x2apic_mode(apic)) {
- kvm_lapic_reg_write(apic, APIC_ICR,
- APIC_DEST_SELF | (val & APIC_VECTOR_MASK));
- } else
+ if (apic_x2apic_mode(apic))
+ kvm_apic_send_ipi(apic, APIC_DEST_SELF | (val & APIC_VECTOR_MASK), 0);
+ else
ret = 1;
break;
default:
--
2.35.1

2022-04-27 16:53:22

by Paolo Bonzini

Subject: Re: [PATCH MANUALSEL 5.10 1/4] x86/kvm: Preserve BSP MSR_KVM_POLL_CONTROL across suspend/resume

On 4/27/22 17:54, Sasha Levin wrote:
> From: Wanpeng Li <[email protected]>
>
> [ Upstream commit 0361bdfddca20c8855ea3bdbbbc9c999912b10ff ]
>
> MSR_KVM_POLL_CONTROL is cleared on reset, thus reverting guests to
> host-side polling after suspend/resume. Non-bootstrap CPUs are
> restored correctly by the haltpoll driver because they are hot-unplugged
> during suspend and hot-plugged during resume; however, the BSP
> is not hotpluggable and remains in host-sde polling mode after
> the guest resume. The makes the guest pay for the cost of vmexits
> every time the guest enters idle.
>
> Fix it by recording BSP's haltpoll state and resuming it during guest
> resume.
>
> Cc: Marcelo Tosatti <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> Message-Id: <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> arch/x86/kernel/kvm.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 18e952fed021..6c3d38b5a8ad 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -66,6 +66,7 @@ static DEFINE_PER_CPU_DECRYPTED(struct kvm_vcpu_pv_apf_data, apf_reason) __align
> DEFINE_PER_CPU_DECRYPTED(struct kvm_steal_time, steal_time) __aligned(64) __visible;
> static int has_steal_clock = 0;
>
> +static int has_guest_poll = 0;
> /*
> * No need for any "IO delay" on KVM
> */
> @@ -624,14 +625,26 @@ static int kvm_cpu_down_prepare(unsigned int cpu)
>
> static int kvm_suspend(void)
> {
> + u64 val = 0;
> +
> kvm_guest_cpu_offline(false);
>
> +#ifdef CONFIG_ARCH_CPUIDLE_HALTPOLL
> + if (kvm_para_has_feature(KVM_FEATURE_POLL_CONTROL))
> + rdmsrl(MSR_KVM_POLL_CONTROL, val);
> + has_guest_poll = !(val & 1);
> +#endif
> return 0;
> }
>
> static void kvm_resume(void)
> {
> kvm_cpu_online(raw_smp_processor_id());
> +
> +#ifdef CONFIG_ARCH_CPUIDLE_HALTPOLL
> + if (kvm_para_has_feature(KVM_FEATURE_POLL_CONTROL) && has_guest_poll)
> + wrmsrl(MSR_KVM_POLL_CONTROL, 0);
> +#endif
> }
>
> static struct syscore_ops kvm_syscore_ops = {

Acked-by: Paolo Bonzini <[email protected]>

2022-04-27 17:00:02

by Paolo Bonzini

Subject: Re: [PATCH MANUALSEL 5.10 2/4] KVM: x86: Do not change ICR on write to APIC_SELF_IPI

On 4/27/22 17:54, Sasha Levin wrote:
> From: Paolo Bonzini <[email protected]>
>
> [ Upstream commit d22a81b304a27fca6124174a8e842e826c193466 ]
>
> Emulating writes to SELF_IPI with a write to ICR has an unwanted side effect:
> the value of ICR in vAPIC page gets changed. The lists SELF_IPI as write-only,
> with no associated MMIO offset, so any write should have no visible side
> effect in the vAPIC page.
>
> Reported-by: Chao Gao <[email protected]>
> Reviewed-by: Sean Christopherson <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> arch/x86/kvm/lapic.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index de11149e28e0..e45ebf0870b6 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2106,10 +2106,9 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
> break;
>
> case APIC_SELF_IPI:
> - if (apic_x2apic_mode(apic)) {
> - kvm_lapic_reg_write(apic, APIC_ICR,
> - APIC_DEST_SELF | (val & APIC_VECTOR_MASK));
> - } else
> + if (apic_x2apic_mode(apic))
> + kvm_apic_send_ipi(apic, APIC_DEST_SELF | (val & APIC_VECTOR_MASK), 0);
> + else
> ret = 1;
> break;
> default:

Acked-by: Paolo Bonzini <[email protected]>