* [PATCH v7 3/5] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Zhenzhong Duan @ 2019-10-21 9:11 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, mingo, bp, x86, pbonzini, rkrcmar, sean.j.christopherson,
vkuznets, wanpengli, jmattson, joro, boris.ostrovsky, jgross,
peterz, will, linux-hyperv, kvm, mikelley, kys, haiyangz,
sthemmin, sashal, Zhenzhong Duan, Jonathan Corbet, H. Peter Anvin
In-Reply-To: <1571649076-2421-1-git-send-email-zhenzhong.duan@oracle.com>
There are cases where a guest tries to switch spinlocks to bare metal
behavior (e.g. by setting "xen_nopvspin" on XEN platform and
"hv_nopvspin" on HYPER_V).
That feature is missed on KVM, add a new parameter "nopvspin" to disable
PV spinlocks for KVM guest.
The new 'nopvspin' parameter will also replace Xen and Hyper-V specific
parameters in future patches.
Define variable nopvsin as global because it will be used in future
patches as above.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krcmar <rkrcmar@redhat.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>
Cc: Jim Mattson <jmattson@google.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
Documentation/admin-guide/kernel-parameters.txt | 5 ++++
arch/x86/include/asm/qspinlock.h | 1 +
arch/x86/kernel/kvm.c | 34 ++++++++++++++++++++-----
kernel/locking/qspinlock.c | 7 +++++
4 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a84a83f..bd49ed2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5334,6 +5334,11 @@
as generic guest with no PV drivers. Currently support
XEN HVM, KVM, HYPER_V and VMWARE guest.
+ nopvspin [X86,KVM]
+ Disables the qspinlock slow path using PV optimizations
+ which allow the hypervisor to 'idle' the guest on lock
+ contention.
+
xirc2ps_cs= [NET,PCMCIA]
Format:
<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
index 444d6fd..d86ab94 100644
--- a/arch/x86/include/asm/qspinlock.h
+++ b/arch/x86/include/asm/qspinlock.h
@@ -32,6 +32,7 @@ static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock *lo
extern void __pv_init_lock_hash(void);
extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
extern void __raw_callee_save___pv_queued_spin_unlock(struct qspinlock *lock);
+extern bool nopvspin;
#define queued_spin_unlock queued_spin_unlock
/**
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 249f14a..3945aa5 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -825,18 +825,36 @@ __visible bool __kvm_vcpu_is_preempted(long cpu)
*/
void __init kvm_spinlock_init(void)
{
- /* Does host kernel support KVM_FEATURE_PV_UNHALT? */
- if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
+ /*
+ * In case host doesn't support KVM_FEATURE_PV_UNHALT there is still an
+ * advantage of keeping virt_spin_lock_key enabled: virt_spin_lock() is
+ * preferred over native qspinlock when vCPU is preempted.
+ */
+ if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)) {
+ pr_info("PV spinlocks disabled, no host support.\n");
return;
+ }
+ /*
+ * Disable PV qspinlock and use native qspinlock when dedicated pCPUs
+ * are available.
+ */
if (kvm_para_has_hint(KVM_HINTS_REALTIME)) {
- static_branch_disable(&virt_spin_lock_key);
- return;
+ pr_info("PV spinlocks disabled with KVM_HINTS_REALTIME hints.\n");
+ goto out;
}
- /* Don't use the pvqspinlock code if there is only 1 vCPU. */
- if (num_possible_cpus() == 1)
- return;
+ if (num_possible_cpus() == 1) {
+ pr_info("PV spinlocks disabled, single CPU.\n");
+ goto out;
+ }
+
+ if (nopvspin) {
+ pr_info("PV spinlocks disabled, forced by \"nopvspin\" parameter.\n");
+ goto out;
+ }
+
+ pr_info("PV spinlocks enabled\n");
__pv_init_lock_hash();
pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
@@ -849,6 +867,8 @@ void __init kvm_spinlock_init(void)
pv_ops.lock.vcpu_is_preempted =
PV_CALLEE_SAVE(__kvm_vcpu_is_preempted);
}
+out:
+ static_branch_disable(&virt_spin_lock_key);
}
#endif /* CONFIG_PARAVIRT_SPINLOCKS */
diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index 2473f10..75193d6 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -580,4 +580,11 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
#include "qspinlock_paravirt.h"
#include "qspinlock.c"
+bool nopvspin __initdata;
+static __init int parse_nopvspin(char *arg)
+{
+ nopvspin = true;
+ return 0;
+}
+early_param("nopvspin", parse_nopvspin);
#endif
--
1.8.3.1
^ permalink raw reply related
* [PATCH v7 5/5] x86/hyperv: Mark "hv_nopvspin" parameter obsolete
From: Zhenzhong Duan @ 2019-10-21 9:11 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, mingo, bp, x86, pbonzini, rkrcmar, sean.j.christopherson,
vkuznets, wanpengli, jmattson, joro, boris.ostrovsky, jgross,
peterz, will, linux-hyperv, kvm, mikelley, kys, haiyangz,
sthemmin, sashal, Zhenzhong Duan, Jonathan Corbet, H. Peter Anvin
In-Reply-To: <1571649076-2421-1-git-send-email-zhenzhong.duan@oracle.com>
Map "hv_nopvspin" to "nopvspin".
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Sasha Levin <sashal@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
Documentation/admin-guide/kernel-parameters.txt | 6 +++++-
arch/x86/hyperv/hv_spinlock.c | 4 ++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 85059dd..78648bb 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1436,6 +1436,10 @@
hv_nopvspin [X86,HYPER_V] Disables the paravirt spinlock optimizations
which allow the hypervisor to 'idle' the
guest on lock contention.
+ This parameter is obsoleted by "nopvspin"
+ parameter, which has equivalent effect for
+ HYPER_V platform.
+
keep_bootcon [KNL]
Do not unregister boot console at start. This is only
@@ -5335,7 +5339,7 @@
as generic guest with no PV drivers. Currently support
XEN HVM, KVM, HYPER_V and VMWARE guest.
- nopvspin [X86,XEN,KVM]
+ nopvspin [X86,XEN,KVM,HYPER_V]
Disables the qspinlock slow path using PV optimizations
which allow the hypervisor to 'idle' the guest on lock
contention.
diff --git a/arch/x86/hyperv/hv_spinlock.c b/arch/x86/hyperv/hv_spinlock.c
index 07f21a0..47c7d6c 100644
--- a/arch/x86/hyperv/hv_spinlock.c
+++ b/arch/x86/hyperv/hv_spinlock.c
@@ -64,6 +64,9 @@ __visible bool hv_vcpu_is_preempted(int vcpu)
void __init hv_init_spinlocks(void)
{
+ if (nopvspin)
+ hv_pvspin = false;
+
if (!hv_pvspin || !apic ||
!(ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) ||
!(ms_hyperv.features & HV_X64_MSR_GUEST_IDLE_AVAILABLE)) {
@@ -82,6 +85,7 @@ void __init hv_init_spinlocks(void)
static __init int hv_parse_nopvspin(char *arg)
{
+ pr_notice("\"hv_nopvspin\" is deprecated, please use \"nopvspin\" instead\n");
hv_pvspin = false;
return 0;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v7 4/5] xen: Mark "xen_nopvspin" parameter obsolete
From: Zhenzhong Duan @ 2019-10-21 9:11 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, mingo, bp, x86, pbonzini, rkrcmar, sean.j.christopherson,
vkuznets, wanpengli, jmattson, joro, boris.ostrovsky, jgross,
peterz, will, linux-hyperv, kvm, mikelley, kys, haiyangz,
sthemmin, sashal, Zhenzhong Duan, Jonathan Corbet,
Stefano Stabellini, H. Peter Anvin
In-Reply-To: <1571649076-2421-1-git-send-email-zhenzhong.duan@oracle.com>
Map "xen_nopvspin" to "nopvspin", fix stale description of "xen_nopvspin"
as we use qspinlock now.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
Documentation/admin-guide/kernel-parameters.txt | 7 ++++---
arch/x86/xen/spinlock.c | 4 ++--
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index bd49ed2..85059dd 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5307,8 +5307,9 @@
panic() code such as dumping handler.
xen_nopvspin [X86,XEN]
- Disables the ticketlock slowpath using Xen PV
- optimizations.
+ Disables the qspinlock slowpath using Xen PV optimizations.
+ This parameter is obsoleted by "nopvspin" parameter, which
+ has equivalent effect for XEN platform.
xen_nopv [X86]
Disables the PV optimizations forcing the HVM guest to
@@ -5334,7 +5335,7 @@
as generic guest with no PV drivers. Currently support
XEN HVM, KVM, HYPER_V and VMWARE guest.
- nopvspin [X86,KVM]
+ nopvspin [X86,XEN,KVM]
Disables the qspinlock slow path using PV optimizations
which allow the hypervisor to 'idle' the guest on lock
contention.
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 6deb490..799f4eb 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -114,9 +114,8 @@ void xen_uninit_lock_cpu(int cpu)
*/
void __init xen_init_spinlocks(void)
{
-
/* Don't need to use pvqspinlock code if there is only 1 vCPU. */
- if (num_possible_cpus() == 1)
+ if (num_possible_cpus() == 1 || nopvspin)
xen_pvspin = false;
if (!xen_pvspin) {
@@ -137,6 +136,7 @@ void __init xen_init_spinlocks(void)
static __init int xen_parse_nopvspin(char *arg)
{
+ pr_notice("\"xen_nopvspin\" is deprecated, please use \"nopvspin\" instead\n");
xen_pvspin = false;
return 0;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v7 0/5] Add a unified parameter "nopvspin"
From: Zhenzhong Duan @ 2019-10-21 9:11 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, mingo, bp, x86, pbonzini, rkrcmar, sean.j.christopherson,
vkuznets, wanpengli, jmattson, joro, boris.ostrovsky, jgross,
peterz, will, linux-hyperv, kvm, mikelley, kys, haiyangz,
sthemmin, sashal, Zhenzhong Duan
All the patches have Reviewed-by now, I think v7 could be the final
version.
There are cases folks want to disable spinlock optimization for
debug/test purpose. Xen and hyperv already have parameters "xen_nopvspin"
and "hv_nopvspin" to support that, but kvm doesn't.
The first patch adds that feature to KVM guest with "nopvspin".
For compatibility reason original parameters "xen_nopvspin" and
"hv_nopvspin" are retained and marked obsolete.
v7:
PATCH3: update comment and use goto, add RB [Vitaly Kuznetsov]
v6:
PATCH1: add Reviewed-by [Vitaly Kuznetsov]
PATCH2: change 'pv' to 'PV', add Reviewed-by [Vitaly Kuznetsov]
PATCH3: refactor 'if' branch in kvm_spinlock_init() [Vitaly Kuznetsov]
v5:
PATCH1: new patch to revert a currently unnecessory commit,
code is simpler a bit after that change. [Boris Ostrovsky]
PATCH3: fold 'if' statement,add comments on virt_spin_lock_key,
reorder with PATCH2 to better reflect dependency
PATCH4: fold 'if' statement, add Reviewed-by [Boris Ostrovsky]
PATCH5: add Reviewed-by [Michael Kelley]
v4:
PATCH1: use variable name nopvspin instead of pvspin and
defined it as __initdata, changed print message,
updated patch description [Sean Christopherson]
PATCH2: remove Suggested-by, use "kvm-guest:" prefix [Sean Christopherson]
PATCH3: make variable nopvsin and xen_pvspin coexist
remove Reviewed-by due to code change [Sean Christopherson]
PATCH4: make variable nopvsin and hv_pvspin coexist [Sean Christopherson]
v3:
PATCH2: Fix indentation
v2:
PATCH1: pick the print code change into separate PATCH2,
updated patch description [Vitaly Kuznetsov]
PATCH2: new patch with print code change [Vitaly Kuznetsov]
PATCH3: add Reviewed-by [Juergen Gross]
Zhenzhong Duan (5):
Revert "KVM: X86: Fix setup the virt_spin_lock_key before static key
get initialized"
x86/kvm: Change print code to use pr_*() format
x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
xen: Mark "xen_nopvspin" parameter obsolete
x86/hyperv: Mark "hv_nopvspin" parameter obsolete
Documentation/admin-guide/kernel-parameters.txt | 14 ++++-
arch/x86/hyperv/hv_spinlock.c | 4 ++
arch/x86/include/asm/qspinlock.h | 1 +
arch/x86/kernel/kvm.c | 74 +++++++++++++++----------
arch/x86/xen/spinlock.c | 4 +-
kernel/locking/qspinlock.c | 7 +++
6 files changed, 71 insertions(+), 33 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH v7 1/5] Revert "KVM: X86: Fix setup the virt_spin_lock_key before static key get initialized"
From: Zhenzhong Duan @ 2019-10-21 9:11 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, mingo, bp, x86, pbonzini, rkrcmar, sean.j.christopherson,
vkuznets, wanpengli, jmattson, joro, boris.ostrovsky, jgross,
peterz, will, linux-hyperv, kvm, mikelley, kys, haiyangz,
sthemmin, sashal, Zhenzhong Duan, H. Peter Anvin
In-Reply-To: <1571649076-2421-1-git-send-email-zhenzhong.duan@oracle.com>
This reverts commit 34226b6b70980a8f81fff3c09a2c889f77edeeff.
Commit 8990cac6e5ea ("x86/jump_label: Initialize static branching
early") adds jump_label_init() call in setup_arch() to make static
keys initialized early, so we could use the original simpler code
again.
The similar change for XEN is in commit 090d54bcbc54 ("Revert
"x86/paravirt: Set up the virt_spin_lock_key after static keys get
initialized"")
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krcmar <rkrcmar@redhat.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>
Cc: Jim Mattson <jmattson@google.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
arch/x86/kernel/kvm.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index e820568..3bc6a266 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -527,13 +527,6 @@ static void kvm_smp_send_call_func_ipi(const struct cpumask *mask)
}
}
-static void __init kvm_smp_prepare_cpus(unsigned int max_cpus)
-{
- native_smp_prepare_cpus(max_cpus);
- if (kvm_para_has_hint(KVM_HINTS_REALTIME))
- static_branch_disable(&virt_spin_lock_key);
-}
-
static void __init kvm_smp_prepare_boot_cpu(void)
{
/*
@@ -633,7 +626,6 @@ static void __init kvm_guest_init(void)
apic_set_eoi_write(kvm_guest_apic_eoi_write);
#ifdef CONFIG_SMP
- smp_ops.smp_prepare_cpus = kvm_smp_prepare_cpus;
smp_ops.smp_prepare_boot_cpu = kvm_smp_prepare_boot_cpu;
if (kvm_para_has_feature(KVM_FEATURE_PV_SCHED_YIELD) &&
!kvm_para_has_hint(KVM_HINTS_REALTIME) &&
@@ -835,8 +827,10 @@ void __init kvm_spinlock_init(void)
if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
return;
- if (kvm_para_has_hint(KVM_HINTS_REALTIME))
+ if (kvm_para_has_hint(KVM_HINTS_REALTIME)) {
+ static_branch_disable(&virt_spin_lock_key);
return;
+ }
/* Don't use the pvqspinlock code if there is only 1 vCPU. */
if (num_possible_cpus() == 1)
--
1.8.3.1
^ permalink raw reply related
* [PATCH v7 2/5] x86/kvm: Change print code to use pr_*() format
From: Zhenzhong Duan @ 2019-10-21 9:11 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, mingo, bp, x86, pbonzini, rkrcmar, sean.j.christopherson,
vkuznets, wanpengli, jmattson, joro, boris.ostrovsky, jgross,
peterz, will, linux-hyperv, kvm, mikelley, kys, haiyangz,
sthemmin, sashal, Zhenzhong Duan, H. Peter Anvin
In-Reply-To: <1571649076-2421-1-git-send-email-zhenzhong.duan@oracle.com>
pr_*() is preferred than printk(KERN_* ...), after change all the print
in arch/x86/kernel/kvm.c will have "kvm_guest: xxx" style.
No functional change.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krcmar <rkrcmar@redhat.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>
Cc: Jim Mattson <jmattson@google.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
arch/x86/kernel/kvm.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 3bc6a266..249f14a 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -7,6 +7,8 @@
* Authors: Anthony Liguori <aliguori@us.ibm.com>
*/
+#define pr_fmt(fmt) "kvm_guest: " fmt
+
#include <linux/context_tracking.h>
#include <linux/init.h>
#include <linux/kernel.h>
@@ -286,8 +288,8 @@ static void kvm_register_steal_time(void)
return;
wrmsrl(MSR_KVM_STEAL_TIME, (slow_virt_to_phys(st) | KVM_MSR_ENABLED));
- pr_info("kvm-stealtime: cpu %d, msr %llx\n",
- cpu, (unsigned long long) slow_virt_to_phys(st));
+ pr_info("stealtime: cpu %d, msr %llx\n", cpu,
+ (unsigned long long) slow_virt_to_phys(st));
}
static DEFINE_PER_CPU_DECRYPTED(unsigned long, kvm_apic_eoi) = KVM_PV_EOI_DISABLED;
@@ -321,8 +323,7 @@ static void kvm_guest_cpu_init(void)
wrmsrl(MSR_KVM_ASYNC_PF_EN, pa);
__this_cpu_write(apf_reason.enabled, 1);
- printk(KERN_INFO"KVM setup async PF for cpu %d\n",
- smp_processor_id());
+ pr_info("setup async PF for cpu %d\n", smp_processor_id());
}
if (kvm_para_has_feature(KVM_FEATURE_PV_EOI)) {
@@ -347,8 +348,7 @@ static void kvm_pv_disable_apf(void)
wrmsrl(MSR_KVM_ASYNC_PF_EN, 0);
__this_cpu_write(apf_reason.enabled, 0);
- printk(KERN_INFO"Unregister pv shared memory for cpu %d\n",
- smp_processor_id());
+ pr_info("Unregister PV shared memory for cpu %d\n", smp_processor_id());
}
static void kvm_pv_guest_cpu_reboot(void *unused)
@@ -469,7 +469,8 @@ static void __send_ipi_mask(const struct cpumask *mask, int vector)
} else {
ret = kvm_hypercall4(KVM_HC_SEND_IPI, (unsigned long)ipi_bitmap,
(unsigned long)(ipi_bitmap >> BITS_PER_LONG), min, icr);
- WARN_ONCE(ret < 0, "KVM: failed to send PV IPI: %ld", ret);
+ WARN_ONCE(ret < 0, "kvm_guest: failed to send PV IPI: %ld",
+ ret);
min = max = apic_id;
ipi_bitmap = 0;
}
@@ -479,7 +480,8 @@ static void __send_ipi_mask(const struct cpumask *mask, int vector)
if (ipi_bitmap) {
ret = kvm_hypercall4(KVM_HC_SEND_IPI, (unsigned long)ipi_bitmap,
(unsigned long)(ipi_bitmap >> BITS_PER_LONG), min, icr);
- WARN_ONCE(ret < 0, "KVM: failed to send PV IPI: %ld", ret);
+ WARN_ONCE(ret < 0, "kvm_guest: failed to send PV IPI: %ld",
+ ret);
}
local_irq_restore(flags);
@@ -509,7 +511,7 @@ static void kvm_setup_pv_ipi(void)
{
apic->send_IPI_mask = kvm_send_ipi_mask;
apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
- pr_info("KVM setup pv IPIs\n");
+ pr_info("setup PV IPIs\n");
}
static void kvm_smp_send_call_func_ipi(const struct cpumask *mask)
@@ -631,11 +633,11 @@ static void __init kvm_guest_init(void)
!kvm_para_has_hint(KVM_HINTS_REALTIME) &&
kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
smp_ops.send_call_func_ipi = kvm_smp_send_call_func_ipi;
- pr_info("KVM setup pv sched yield\n");
+ pr_info("setup PV sched yield\n");
}
if (cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "x86/kvm:online",
kvm_cpu_online, kvm_cpu_down_prepare) < 0)
- pr_err("kvm_guest: Failed to install cpu hotplug callbacks\n");
+ pr_err("failed to install cpu hotplug callbacks\n");
#else
sev_map_percpu_data();
kvm_guest_cpu_init();
@@ -738,7 +740,7 @@ static __init int kvm_setup_pv_tlb_flush(void)
zalloc_cpumask_var_node(per_cpu_ptr(&__pv_tlb_mask, cpu),
GFP_KERNEL, cpu_to_node(cpu));
}
- pr_info("KVM setup pv remote TLB flush\n");
+ pr_info("setup PV remote TLB flush\n");
}
return 0;
@@ -866,8 +868,8 @@ static void kvm_enable_host_haltpoll(void *i)
void arch_haltpoll_enable(unsigned int cpu)
{
if (!kvm_para_has_feature(KVM_FEATURE_POLL_CONTROL)) {
- pr_err_once("kvm: host does not support poll control\n");
- pr_err_once("kvm: host upgrade recommended\n");
+ pr_err_once("host does not support poll control\n");
+ pr_err_once("host upgrade recommended\n");
return;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v6 3/5] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Zhenzhong Duan @ 2019-10-22 2:45 UTC (permalink / raw)
To: Vitaly Kuznetsov, linux-kernel
Cc: linux-hyperv, kvm, kys, haiyangz, sthemmin, sashal, tglx, mingo,
bp, pbonzini, rkrcmar, sean.j.christopherson, wanpengli, jmattson,
joro, boris.ostrovsky, jgross, sstabellini, peterz,
Jonathan Corbet, H. Peter Anvin, Will Deacon
In-Reply-To: <87k18y1hc1.fsf@vitty.brq.redhat.com>
On 2019/10/21 19:14, Vitaly Kuznetsov wrote:
>> index 249f14a..e9c76d8 100644
>> --- a/arch/x86/kernel/kvm.c
>> +++ b/arch/x86/kernel/kvm.c
>> @@ -825,18 +825,44 @@ __visible bool __kvm_vcpu_is_preempted(long cpu)
>> */
>> void __init kvm_spinlock_init(void)
>> {
>> - /* Does host kernel support KVM_FEATURE_PV_UNHALT? */
>> - if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
>> + /*
>> + * PV spinlocks is disabled if no host side support, then native
>> + * qspinlock will be used. As native qspinlock is a fair lock, there is
>> + * lock holder preemption issue using it in a guest, imaging one pCPU
>> + * running 10 vCPUs of same guest contending same lock.
>> + *
>> + * virt_spin_lock() is introduced as an optimization for that scenario
>> + * which is enabled by virt_spin_lock_key key. To use that optimization,
>> + * virt_spin_lock_key isn't disabled here.
>> + */
> My take (if I properly understood what you say) would be:
>
> "In case host doesn't support KVM_FEATURE_PV_UNHALT there is still an
> advantage of keeping virt_spin_lock_key enabled: virt_spin_lock() is
> preferred over native qspinlock when vCPU is preempted."
Yes, that's what I mean, maybe I didn't explain clearly due to my pool
english,
I'll use your explanation instead.
>
>> + if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)) {
>> + pr_info("PV spinlocks disabled, no host support.\n");
>> return;
>> + }
>>
>> + /*
>> + * Disable PV qspinlock and use native qspinlock when dedicated pCPUs
>> + * are available.
>> + */
>> if (kvm_para_has_hint(KVM_HINTS_REALTIME)) {
>> + pr_info("PV spinlocks disabled with KVM_HINTS_REALTIME hints.\n");
>> + static_branch_disable(&virt_spin_lock_key);
>> + return;
>> + }
>> +
>> + if (num_possible_cpus() == 1) {
>> + pr_info("PV spinlocks disabled, single CPU.\n");
>> static_branch_disable(&virt_spin_lock_key);
>> return;
>> }
>>
>> - /* Don't use the pvqspinlock code if there is only 1 vCPU. */
>> - if (num_possible_cpus() == 1)
>> + if (nopvspin) {
>> + pr_info("PV spinlocks disabled, forced by \"nopvspin\" parameter.\n");
>> + static_branch_disable(&virt_spin_lock_key);
>> return;
> You could've replaced this 'static_branch_disable(); return;' pattern
> with a goto to the end of the function to save a few lines but this
> looks good anyways.
>
> Reviewed-by: Vitaly Kuznetsov<vkuznets@redhat.com>
Ok, will do, thanks for review.
Zhenzhong
^ permalink raw reply
* Re: [PATCH v6 3/5] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Vitaly Kuznetsov @ 2019-10-21 11:14 UTC (permalink / raw)
To: Zhenzhong Duan, linux-kernel
Cc: linux-hyperv, kvm, kys, haiyangz, sthemmin, sashal, tglx, mingo,
bp, pbonzini, rkrcmar, sean.j.christopherson, wanpengli, jmattson,
joro, boris.ostrovsky, jgross, sstabellini, peterz,
Zhenzhong Duan, Jonathan Corbet, H. Peter Anvin, Will Deacon
In-Reply-To: <1571102367-31595-4-git-send-email-zhenzhong.duan@oracle.com>
Zhenzhong Duan <zhenzhong.duan@oracle.com> writes:
> There are cases where a guest tries to switch spinlocks to bare metal
> behavior (e.g. by setting "xen_nopvspin" on XEN platform and
> "hv_nopvspin" on HYPER_V).
>
> That feature is missed on KVM, add a new parameter "nopvspin" to disable
> PV spinlocks for KVM guest.
>
> The new 'nopvspin' parameter will also replace Xen and Hyper-V specific
> parameters in future patches.
>
> Define variable nopvsin as global because it will be used in future
> patches as above.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krcmar <rkrcmar@redhat.com>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: Wanpeng Li <wanpengli@tencent.com>
> Cc: Jim Mattson <jmattson@google.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Will Deacon <will@kernel.org>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 5 ++++
> arch/x86/include/asm/qspinlock.h | 1 +
> arch/x86/kernel/kvm.c | 34 ++++++++++++++++++++++---
> kernel/locking/qspinlock.c | 7 +++++
> 4 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a84a83f..bd49ed2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5334,6 +5334,11 @@
> as generic guest with no PV drivers. Currently support
> XEN HVM, KVM, HYPER_V and VMWARE guest.
>
> + nopvspin [X86,KVM]
> + Disables the qspinlock slow path using PV optimizations
> + which allow the hypervisor to 'idle' the guest on lock
> + contention.
> +
> xirc2ps_cs= [NET,PCMCIA]
> Format:
> <irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
> diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
> index 444d6fd..d86ab94 100644
> --- a/arch/x86/include/asm/qspinlock.h
> +++ b/arch/x86/include/asm/qspinlock.h
> @@ -32,6 +32,7 @@ static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock *lo
> extern void __pv_init_lock_hash(void);
> extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
> extern void __raw_callee_save___pv_queued_spin_unlock(struct qspinlock *lock);
> +extern bool nopvspin;
>
> #define queued_spin_unlock queued_spin_unlock
> /**
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 249f14a..e9c76d8 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -825,18 +825,44 @@ __visible bool __kvm_vcpu_is_preempted(long cpu)
> */
> void __init kvm_spinlock_init(void)
> {
> - /* Does host kernel support KVM_FEATURE_PV_UNHALT? */
> - if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
> + /*
> + * PV spinlocks is disabled if no host side support, then native
> + * qspinlock will be used. As native qspinlock is a fair lock, there is
> + * lock holder preemption issue using it in a guest, imaging one pCPU
> + * running 10 vCPUs of same guest contending same lock.
> + *
> + * virt_spin_lock() is introduced as an optimization for that scenario
> + * which is enabled by virt_spin_lock_key key. To use that optimization,
> + * virt_spin_lock_key isn't disabled here.
> + */
My take (if I properly understood what you say) would be:
"In case host doesn't support KVM_FEATURE_PV_UNHALT there is still an
advantage of keeping virt_spin_lock_key enabled: virt_spin_lock() is
preferred over native qspinlock when vCPU is preempted."
> + if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)) {
> + pr_info("PV spinlocks disabled, no host support.\n");
> return;
> + }
>
> + /*
> + * Disable PV qspinlock and use native qspinlock when dedicated pCPUs
> + * are available.
> + */
> if (kvm_para_has_hint(KVM_HINTS_REALTIME)) {
> + pr_info("PV spinlocks disabled with KVM_HINTS_REALTIME hints.\n");
> + static_branch_disable(&virt_spin_lock_key);
> + return;
> + }
> +
> + if (num_possible_cpus() == 1) {
> + pr_info("PV spinlocks disabled, single CPU.\n");
> static_branch_disable(&virt_spin_lock_key);
> return;
> }
>
> - /* Don't use the pvqspinlock code if there is only 1 vCPU. */
> - if (num_possible_cpus() == 1)
> + if (nopvspin) {
> + pr_info("PV spinlocks disabled, forced by \"nopvspin\" parameter.\n");
> + static_branch_disable(&virt_spin_lock_key);
> return;
You could've replaced this 'static_branch_disable(); return;' pattern
with a goto to the end of the function to save a few lines but this
looks good anyways.
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> + }
> +
> + pr_info("PV spinlocks enabled\n");
>
> __pv_init_lock_hash();
> pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index 2473f10..75193d6 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -580,4 +580,11 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
> #include "qspinlock_paravirt.h"
> #include "qspinlock.c"
>
> +bool nopvspin __initdata;
> +static __init int parse_nopvspin(char *arg)
> +{
> + nopvspin = true;
> + return 0;
> +}
> +early_param("nopvspin", parse_nopvspin);
> #endif
--
Vitaly
^ permalink raw reply
* Re: [PATCH v6 3/5] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Zhenzhong Duan @ 2019-10-21 2:36 UTC (permalink / raw)
To: linux-kernel
Cc: vkuznets, linux-hyperv, kvm, kys, haiyangz, sthemmin, sashal,
tglx, mingo, bp, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, boris.ostrovsky, jgross, sstabellini,
peterz, Jonathan Corbet, H. Peter Anvin, Will Deacon
In-Reply-To: <1571102367-31595-4-git-send-email-zhenzhong.duan@oracle.com>
Hi vitaly
This patch is based on your suggestion on v5, appreciate your further
review:) Thanks
Zhenzhong
On 2019/10/15 9:19, Zhenzhong Duan wrote:
> There are cases where a guest tries to switch spinlocks to bare metal
> behavior (e.g. by setting "xen_nopvspin" on XEN platform and
> "hv_nopvspin" on HYPER_V).
>
> That feature is missed on KVM, add a new parameter "nopvspin" to disable
> PV spinlocks for KVM guest.
>
> The new 'nopvspin' parameter will also replace Xen and Hyper-V specific
> parameters in future patches.
>
> Define variable nopvsin as global because it will be used in future
> patches as above.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krcmar <rkrcmar@redhat.com>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: Wanpeng Li <wanpengli@tencent.com>
> Cc: Jim Mattson <jmattson@google.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Will Deacon <will@kernel.org>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 5 ++++
> arch/x86/include/asm/qspinlock.h | 1 +
> arch/x86/kernel/kvm.c | 34 ++++++++++++++++++++++---
> kernel/locking/qspinlock.c | 7 +++++
> 4 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a84a83f..bd49ed2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5334,6 +5334,11 @@
> as generic guest with no PV drivers. Currently support
> XEN HVM, KVM, HYPER_V and VMWARE guest.
>
> + nopvspin [X86,KVM]
> + Disables the qspinlock slow path using PV optimizations
> + which allow the hypervisor to 'idle' the guest on lock
> + contention.
> +
> xirc2ps_cs= [NET,PCMCIA]
> Format:
> <irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
> diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
> index 444d6fd..d86ab94 100644
> --- a/arch/x86/include/asm/qspinlock.h
> +++ b/arch/x86/include/asm/qspinlock.h
> @@ -32,6 +32,7 @@ static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock *lo
> extern void __pv_init_lock_hash(void);
> extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
> extern void __raw_callee_save___pv_queued_spin_unlock(struct qspinlock *lock);
> +extern bool nopvspin;
>
> #define queued_spin_unlock queued_spin_unlock
> /**
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 249f14a..e9c76d8 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -825,18 +825,44 @@ __visible bool __kvm_vcpu_is_preempted(long cpu)
> */
> void __init kvm_spinlock_init(void)
> {
> - /* Does host kernel support KVM_FEATURE_PV_UNHALT? */
> - if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
> + /*
> + * PV spinlocks is disabled if no host side support, then native
> + * qspinlock will be used. As native qspinlock is a fair lock, there is
> + * lock holder preemption issue using it in a guest, imaging one pCPU
> + * running 10 vCPUs of same guest contending same lock.
> + *
> + * virt_spin_lock() is introduced as an optimization for that scenario
> + * which is enabled by virt_spin_lock_key key. To use that optimization,
> + * virt_spin_lock_key isn't disabled here.
> + */
> + if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)) {
> + pr_info("PV spinlocks disabled, no host support.\n");
> return;
> + }
>
> + /*
> + * Disable PV qspinlock and use native qspinlock when dedicated pCPUs
> + * are available.
> + */
> if (kvm_para_has_hint(KVM_HINTS_REALTIME)) {
> + pr_info("PV spinlocks disabled with KVM_HINTS_REALTIME hints.\n");
> + static_branch_disable(&virt_spin_lock_key);
> + return;
> + }
> +
> + if (num_possible_cpus() == 1) {
> + pr_info("PV spinlocks disabled, single CPU.\n");
> static_branch_disable(&virt_spin_lock_key);
> return;
> }
>
> - /* Don't use the pvqspinlock code if there is only 1 vCPU. */
> - if (num_possible_cpus() == 1)
> + if (nopvspin) {
> + pr_info("PV spinlocks disabled, forced by \"nopvspin\" parameter.\n");
> + static_branch_disable(&virt_spin_lock_key);
> return;
> + }
> +
> + pr_info("PV spinlocks enabled\n");
>
> __pv_init_lock_hash();
> pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index 2473f10..75193d6 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -580,4 +580,11 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
> #include "qspinlock_paravirt.h"
> #include "qspinlock.c"
>
> +bool nopvspin __initdata;
> +static __init int parse_nopvspin(char *arg)
> +{
> + nopvspin = true;
> + return 0;
> +}
> +early_param("nopvspin", parse_nopvspin);
> #endif
^ permalink raw reply
* Re: [PATCH -next] x86/hyperv: Fix build error while CONFIG_PARAVIRT=n
From: Andrea Parri @ 2019-10-18 10:39 UTC (permalink / raw)
To: YueHaibing
Cc: kys, haiyangz, sthemmin, sashal, tglx, mingo, bp, hpa, x86,
mikelley, wei.liu, linux-hyperv, linux-kernel
In-Reply-To: <20191018082921.28164-1-yuehaibing@huawei.com>
On Fri, Oct 18, 2019 at 04:29:21PM +0800, YueHaibing wrote:
> while CONFIG_PARAVIRT=n, building fails:
>
> arch/x86/kernel/cpu/mshyperv.c: In function ms_hyperv_init_platform:
> arch/x86/kernel/cpu/mshyperv.c:219:2: error: pv_info undeclared (first use in this function); did you mean pr_info?
> pv_info.name = "Hyper-V";
> ^~~~~~~
Ouch, sorry for this...
>
> Wrap it into a #ifdef to fix this.
>
> Fixes: 628270ef628a ("x86/hyperv: Set pv_info.name to "Hyper-V"")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Reviewed-by: Andrea Parri <parri.andrea@gmail.com>
Thanks,
Andrea
> ---
> arch/x86/kernel/cpu/mshyperv.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index e7f0776..c656d92 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -216,7 +216,9 @@ static void __init ms_hyperv_init_platform(void)
> int hv_host_info_ecx;
> int hv_host_info_edx;
>
> +#ifdef CONFIG_PARAVIRT
> pv_info.name = "Hyper-V";
> +#endif
>
> /*
> * Extract the features and hints
> --
> 2.7.4
>
>
^ permalink raw reply
* [PATCH -next] x86/hyperv: Fix build error while CONFIG_PARAVIRT=n
From: YueHaibing @ 2019-10-18 8:29 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, tglx, mingo, bp, hpa, x86,
mikelley, wei.liu, parri.andrea
Cc: linux-hyperv, linux-kernel, YueHaibing
while CONFIG_PARAVIRT=n, building fails:
arch/x86/kernel/cpu/mshyperv.c: In function ms_hyperv_init_platform:
arch/x86/kernel/cpu/mshyperv.c:219:2: error: pv_info undeclared (first use in this function); did you mean pr_info?
pv_info.name = "Hyper-V";
^~~~~~~
Wrap it into a #ifdef to fix this.
Fixes: 628270ef628a ("x86/hyperv: Set pv_info.name to "Hyper-V"")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
arch/x86/kernel/cpu/mshyperv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index e7f0776..c656d92 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -216,7 +216,9 @@ static void __init ms_hyperv_init_platform(void)
int hv_host_info_ecx;
int hv_host_info_edx;
+#ifdef CONFIG_PARAVIRT
pv_info.name = "Hyper-V";
+#endif
/*
* Extract the features and hints
--
2.7.4
^ permalink raw reply related
* [PATCH] drivers: iommu: hyperv: Make HYPERV_IOMMU only available on x86
From: Boqun Feng @ 2019-10-17 0:57 UTC (permalink / raw)
To: iommu, linux-kernel
Cc: Boqun Feng, Lan Tianyu, Michael Kelley, linux-hyperv,
Joerg Roedel
Currently hyperv-iommu is implemented in a x86 specific way, for
example, apic is used. So make the HYPERV_IOMMU Kconfig depend on X86
as a preparation for enabling HyperV on architecture other than x86.
Cc: Lan Tianyu <Tianyu.Lan@microsoft.com>
Cc: Michael Kelley <mikelley@microsoft.com>
Cc: linux-hyperv@vger.kernel.org
Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>
---
Without this patch, I could observe compile error:
| drivers/iommu/hyperv-iommu.c:17:10: fatal error: asm/apic.h: No such
| file or directory
| 17 | #include <asm/apic.h>
| | ^~~~~~~~~~~~
, after apply Michael's ARM64 on HyperV enablement patchset.
drivers/iommu/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index e3842eabcfdd..f1086eaed41c 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -467,7 +467,7 @@ config QCOM_IOMMU
config HYPERV_IOMMU
bool "Hyper-V x2APIC IRQ Handling"
- depends on HYPERV
+ depends on HYPERV && X86
select IOMMU_API
default HYPERV
help
--
2.23.0
^ permalink raw reply related
* Re: [PATCH 5/7] PCI/PM: Make power management op coding style consistent
From: Dan Carpenter @ 2019-10-16 13:50 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dexuan Cui, driverdev-devel, olaf, Lorenzo Pieralisi,
Stephen Hemminger, jackm, Haiyang Zhang, Rafael J . Wysocki,
linux-hyperv, Michael Kelley, Sasha Levin, marcelo.cerri,
linux-pci, apw, vkuznets, Bjorn Helgaas, jasowang, linux-kernel
In-Reply-To: <20191014230016.240912-6-helgaas@kernel.org>
On Mon, Oct 14, 2019 at 06:00:14PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Some of the power management ops use this style:
>
> struct device_driver *drv = dev->driver;
> if (drv && drv->pm && drv->pm->prepare(dev))
> drv->pm->prepare(dev);
>
> while others use this:
>
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
I like this patch a lot, especially the direct returns. But it
occurs to me that in the future this conditional would look better as
const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
or something.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH net-next v2] hv_sock: use HV_HYP_PAGE_SIZE for Hyper-V communication
From: David Miller @ 2019-10-16 0:27 UTC (permalink / raw)
To: mikelley
Cc: kys, haiyangz, sthemmin, sashal, linux-hyperv, netdev,
linux-kernel, himadrispandya
In-Reply-To: <1570926595-8877-1-git-send-email-mikelley@microsoft.com>
From: Michael Kelley <mikelley@microsoft.com>
Date: Sun, 13 Oct 2019 00:30:21 +0000
> From: Himadri Pandya <himadrispandya@gmail.com>
>
> Current code assumes PAGE_SIZE (the guest page size) is equal
> to the page size used to communicate with Hyper-V (which is
> always 4K). While this assumption is true on x86, it may not
> be true for Hyper-V on other architectures. For example,
> Linux on ARM64 may have PAGE_SIZE of 16K or 64K. A new symbol,
> HV_HYP_PAGE_SIZE, has been previously introduced to use when
> the Hyper-V page size is intended instead of the guest page size.
>
> Make this code work on non-x86 architectures by using the new
> HV_HYP_PAGE_SIZE symbol instead of PAGE_SIZE, where appropriate.
> Also replace the now redundant PAGE_SIZE_4K with HV_HYP_PAGE_SIZE.
> The change has no effect on x86, but lays the groundwork to run
> on ARM64 and others.
>
> Signed-off-by: Himadri Pandya <himadrispandya@gmail.com>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> ---
>
> Changes in v2:
> * Revised commit message and subject [Jakub Kicinski]
Applied, thank you.
^ permalink raw reply
* Re: [PATCH v3 0/7] PCI: PM: Move to D0 before calling pci_legacy_resume_early()
From: Bjorn Helgaas @ 2019-10-15 18:42 UTC (permalink / raw)
To: Dexuan Cui
Cc: Rafael J . Wysocki, Lorenzo Pieralisi, Michael Kelley,
Sasha Levin, Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
olaf, apw, jasowang, vkuznets, marcelo.cerri, jackm, linux-pci,
linux-hyperv, linux-kernel, driverdev-devel
In-Reply-To: <20191014230016.240912-1-helgaas@kernel.org>
On Mon, Oct 14, 2019 at 06:00:09PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Dexuan, the important thing here is the first patch, which is your [1],
> which I modified by doing pci_restore_state() as well as setting to D0:
>
> pci_set_power_state(pci_dev, PCI_D0);
> pci_restore_state(pci_dev);
>
> I'm proposing some more patches on top. None are relevant to the problem
> you're solving; they're just minor doc and other updates in the same area.
>
> Rafael, if you have a chance to look at these, I'd appreciate it. I tried
> to make the doc match the code, but I'm no PM expert.
>
> [1] https://lore.kernel.org/r/KU1P153MB016637CAEAD346F0AA8E3801BFAD0@KU1P153MB0166.APCP153.PROD.OUTLOOK.COM
>
>
> Dexuan Cui (1):
> PCI/PM: Always return devices to D0 when thawing
>
> Bjorn Helgaas (6):
> PCI/PM: Correct pci_pm_thaw_noirq() documentation
> PCI/PM: Clear PCIe PME Status even for legacy power management
> PCI/PM: Run resume fixups before disabling wakeup events
> PCI/PM: Make power management op coding style consistent
> PCI/PM: Wrap long lines in documentation
> PCI/MSI: Move power state check out of pci_msi_supported()
>
> Documentation/power/pci.rst | 38 +++++++-------
> drivers/pci/msi.c | 6 +--
> drivers/pci/pci-driver.c | 99 ++++++++++++++++++-------------------
> 3 files changed, 71 insertions(+), 72 deletions(-)
Thanks Dexuan and Rafael for taking a look at these!
I applied the first six to pci/pm and the last to pci/msi, all for
v5.5.
^ permalink raw reply
* RE: [PATCH v3 0/7] PCI: PM: Move to D0 before calling pci_legacy_resume_early()
From: Dexuan Cui @ 2019-10-15 18:24 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rafael J . Wysocki, Lorenzo Pieralisi, Michael Kelley,
Sasha Levin, Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, vkuznets,
marcelo.cerri@canonical.com, jackm@mellanox.com,
linux-pci@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Bjorn Helgaas
In-Reply-To: <20191014230016.240912-1-helgaas@kernel.org>
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Monday, October 14, 2019 4:00 PM
> ...
>
> Dexuan, the important thing here is the first patch, which is your [1],
> which I modified by doing pci_restore_state() as well as setting to D0:
>
> pci_set_power_state(pci_dev, PCI_D0);
> pci_restore_state(pci_dev);
>
> I'm proposing some more patches on top. None are relevant to the problem
> you're solving; they're just minor doc and other updates in the same area.
>
> Rafael, if you have a chance to look at these, I'd appreciate it. I tried
> to make the doc match the code, but I'm no PM expert.
Thank you very much, Bjorn! The patchset looks good to me.
Thanks,
-- Dexuan
^ permalink raw reply
* Re: [PATCH 7/7] PCI/MSI: Move power state check out of pci_msi_supported()
From: Rafael J. Wysocki @ 2019-10-15 17:24 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dexuan Cui, Rafael J . Wysocki, Lorenzo Pieralisi, Michael Kelley,
Sasha Levin, Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
olaf, apw, jasowang, vkuznets, marcelo.cerri, jackm, linux-pci,
linux-hyperv, linux-kernel, driverdev-devel, Bjorn Helgaas
In-Reply-To: <20191014230016.240912-8-helgaas@kernel.org>
On Tuesday, October 15, 2019 1:00:16 AM CEST Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> 27e20603c54b ("PCI/MSI: Move D0 check into pci_msi_check_device()")
> moved the power state check into pci_msi_check_device(), which was
> subsequently renamed to pci_msi_supported(). This didn't change the
> behavior, since both callers checked the power state.
>
> However, it doesn't fit the current "pci_msi_supported()" name, which
> should return what the device is capable of, independent of the power
> state.
>
> Move the power state check back into the callers for readability. No
> functional change intended.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
No issues found, so
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/pci/msi.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 0884bedcfc7a..20e9c729617c 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -861,7 +861,7 @@ static int pci_msi_supported(struct pci_dev *dev, int nvec)
> if (!pci_msi_enable)
> return 0;
>
> - if (!dev || dev->no_msi || dev->current_state != PCI_D0)
> + if (!dev || dev->no_msi)
> return 0;
>
> /*
> @@ -972,7 +972,7 @@ static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
> int nr_entries;
> int i, j;
>
> - if (!pci_msi_supported(dev, nvec))
> + if (!pci_msi_supported(dev, nvec) || dev->current_state != PCI_D0)
> return -EINVAL;
>
> nr_entries = pci_msix_vec_count(dev);
> @@ -1058,7 +1058,7 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
> int nvec;
> int rc;
>
> - if (!pci_msi_supported(dev, minvec))
> + if (!pci_msi_supported(dev, minvec) || dev->current_state != PCI_D0)
> return -EINVAL;
>
> /* Check whether driver already requested MSI-X IRQs */
>
^ permalink raw reply
* Re: [PATCH 6/7] PCI/PM: Wrap long lines in documentation
From: Rafael J. Wysocki @ 2019-10-15 17:23 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dexuan Cui, Rafael J . Wysocki, Lorenzo Pieralisi, Michael Kelley,
Sasha Levin, Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
olaf, apw, jasowang, vkuznets, marcelo.cerri, jackm, linux-pci,
linux-hyperv, linux-kernel, driverdev-devel, Bjorn Helgaas
In-Reply-To: <20191014230016.240912-7-helgaas@kernel.org>
On Tuesday, October 15, 2019 1:00:15 AM CEST Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Documentation/power/pci.rst is wrapped to fit in 80 columns, but directory
> structure changes made a few lines longer. Wrap them so they all fit in 80
> columns again.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Well, looks better this way. :-)
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> Documentation/power/pci.rst | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
> index 1525c594d631..db41a770a2f5 100644
> --- a/Documentation/power/pci.rst
> +++ b/Documentation/power/pci.rst
> @@ -426,12 +426,12 @@ pm->runtime_idle() callback.
> 2.4. System-Wide Power Transitions
> ----------------------------------
> There are a few different types of system-wide power transitions, described in
> -Documentation/driver-api/pm/devices.rst. Each of them requires devices to be handled
> -in a specific way and the PM core executes subsystem-level power management
> -callbacks for this purpose. They are executed in phases such that each phase
> -involves executing the same subsystem-level callback for every device belonging
> -to the given subsystem before the next phase begins. These phases always run
> -after tasks have been frozen.
> +Documentation/driver-api/pm/devices.rst. Each of them requires devices to be
> +handled in a specific way and the PM core executes subsystem-level power
> +management callbacks for this purpose. They are executed in phases such that
> +each phase involves executing the same subsystem-level callback for every device
> +belonging to the given subsystem before the next phase begins. These phases
> +always run after tasks have been frozen.
>
> 2.4.1. System Suspend
> ^^^^^^^^^^^^^^^^^^^^^
> @@ -636,12 +636,12 @@ System restore requires a hibernation image to be loaded into memory and the
> pre-hibernation memory contents to be restored before the pre-hibernation system
> activity can be resumed.
>
> -As described in Documentation/driver-api/pm/devices.rst, the hibernation image is loaded
> -into memory by a fresh instance of the kernel, called the boot kernel, which in
> -turn is loaded and run by a boot loader in the usual way. After the boot kernel
> -has loaded the image, it needs to replace its own code and data with the code
> -and data of the "hibernated" kernel stored within the image, called the image
> -kernel. For this purpose all devices are frozen just like before creating
> +As described in Documentation/driver-api/pm/devices.rst, the hibernation image
> +is loaded into memory by a fresh instance of the kernel, called the boot kernel,
> +which in turn is loaded and run by a boot loader in the usual way. After the
> +boot kernel has loaded the image, it needs to replace its own code and data with
> +the code and data of the "hibernated" kernel stored within the image, called the
> +image kernel. For this purpose all devices are frozen just like before creating
> the image during hibernation, in the
>
> prepare, freeze, freeze_noirq
> @@ -691,8 +691,8 @@ controlling the runtime power management of their devices.
>
> At the time of this writing there are two ways to define power management
> callbacks for a PCI device driver, the recommended one, based on using a
> -dev_pm_ops structure described in Documentation/driver-api/pm/devices.rst, and the
> -"legacy" one, in which the .suspend(), .suspend_late(), .resume_early(), and
> +dev_pm_ops structure described in Documentation/driver-api/pm/devices.rst, and
> +the "legacy" one, in which the .suspend(), .suspend_late(), .resume_early(), and
> .resume() callbacks from struct pci_driver are used. The legacy approach,
> however, doesn't allow one to define runtime power management callbacks and is
> not really suitable for any new drivers. Therefore it is not covered by this
>
^ permalink raw reply
* Re: [PATCH 5/7] PCI/PM: Make power management op coding style consistent
From: Rafael J. Wysocki @ 2019-10-15 17:22 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dexuan Cui, Rafael J . Wysocki, Lorenzo Pieralisi, Michael Kelley,
Sasha Levin, Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
olaf, apw, jasowang, vkuznets, marcelo.cerri, jackm, linux-pci,
linux-hyperv, linux-kernel, driverdev-devel, Bjorn Helgaas
In-Reply-To: <20191014230016.240912-6-helgaas@kernel.org>
On Tuesday, October 15, 2019 1:00:14 AM CEST Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Some of the power management ops use this style:
>
> struct device_driver *drv = dev->driver;
> if (drv && drv->pm && drv->pm->prepare(dev))
> drv->pm->prepare(dev);
>
> while others use this:
>
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> if (pm && pm->runtime_resume)
> pm->runtime_resume(dev);
>
> Convert the first style to the second so they're all consistent. Remove
> local "error" variables when unnecessary. No functional change intended.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
No issues found, so
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/pci/pci-driver.c | 76 +++++++++++++++++++---------------------
> 1 file changed, 36 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 55acb658273f..abbf5c39cb9c 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -679,11 +679,11 @@ static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
>
> static int pci_pm_prepare(struct device *dev)
> {
> - struct device_driver *drv = dev->driver;
> struct pci_dev *pci_dev = to_pci_dev(dev);
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>
> - if (drv && drv->pm && drv->pm->prepare) {
> - int error = drv->pm->prepare(dev);
> + if (pm && pm->prepare) {
> + int error = pm->prepare(dev);
> if (error < 0)
> return error;
>
> @@ -917,8 +917,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
> static int pci_pm_resume_noirq(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> - struct device_driver *drv = dev->driver;
> - int error = 0;
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>
> if (dev_pm_may_skip_resume(dev))
> return 0;
> @@ -946,17 +945,16 @@ static int pci_pm_resume_noirq(struct device *dev)
> if (pci_has_legacy_pm_support(pci_dev))
> return pci_legacy_resume_early(dev);
>
> - if (drv && drv->pm && drv->pm->resume_noirq)
> - error = drv->pm->resume_noirq(dev);
> + if (pm && pm->resume_noirq)
> + return pm->resume_noirq(dev);
>
> - return error;
> + return 0;
> }
>
> static int pci_pm_resume(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> - int error = 0;
>
> /*
> * This is necessary for the suspend error path in which resume is
> @@ -972,12 +970,12 @@ static int pci_pm_resume(struct device *dev)
>
> if (pm) {
> if (pm->resume)
> - error = pm->resume(dev);
> + return pm->resume(dev);
> } else {
> pci_pm_reenable_device(pci_dev);
> }
>
> - return error;
> + return 0;
> }
>
> #else /* !CONFIG_SUSPEND */
> @@ -1038,16 +1036,16 @@ static int pci_pm_freeze(struct device *dev)
> static int pci_pm_freeze_noirq(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> - struct device_driver *drv = dev->driver;
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>
> if (pci_has_legacy_pm_support(pci_dev))
> return pci_legacy_suspend_late(dev, PMSG_FREEZE);
>
> - if (drv && drv->pm && drv->pm->freeze_noirq) {
> + if (pm && pm->freeze_noirq) {
> int error;
>
> - error = drv->pm->freeze_noirq(dev);
> - suspend_report_result(drv->pm->freeze_noirq, error);
> + error = pm->freeze_noirq(dev);
> + suspend_report_result(pm->freeze_noirq, error);
> if (error)
> return error;
> }
> @@ -1066,8 +1064,8 @@ static int pci_pm_freeze_noirq(struct device *dev)
> static int pci_pm_thaw_noirq(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> - struct device_driver *drv = dev->driver;
> - int error = 0;
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> + int error;
>
> if (pcibios_pm_ops.thaw_noirq) {
> error = pcibios_pm_ops.thaw_noirq(dev);
> @@ -1091,10 +1089,10 @@ static int pci_pm_thaw_noirq(struct device *dev)
> if (pci_has_legacy_pm_support(pci_dev))
> return pci_legacy_resume_early(dev);
>
> - if (drv && drv->pm && drv->pm->thaw_noirq)
> - error = drv->pm->thaw_noirq(dev);
> + if (pm && pm->thaw_noirq)
> + return pm->thaw_noirq(dev);
>
> - return error;
> + return 0;
> }
>
> static int pci_pm_thaw(struct device *dev)
> @@ -1165,24 +1163,24 @@ static int pci_pm_poweroff_late(struct device *dev)
> static int pci_pm_poweroff_noirq(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> - struct device_driver *drv = dev->driver;
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>
> if (dev_pm_smart_suspend_and_suspended(dev))
> return 0;
>
> - if (pci_has_legacy_pm_support(to_pci_dev(dev)))
> + if (pci_has_legacy_pm_support(pci_dev))
> return pci_legacy_suspend_late(dev, PMSG_HIBERNATE);
>
> - if (!drv || !drv->pm) {
> + if (!pm) {
> pci_fixup_device(pci_fixup_suspend_late, pci_dev);
> return 0;
> }
>
> - if (drv->pm->poweroff_noirq) {
> + if (pm->poweroff_noirq) {
> int error;
>
> - error = drv->pm->poweroff_noirq(dev);
> - suspend_report_result(drv->pm->poweroff_noirq, error);
> + error = pm->poweroff_noirq(dev);
> + suspend_report_result(pm->poweroff_noirq, error);
> if (error)
> return error;
> }
> @@ -1208,8 +1206,8 @@ static int pci_pm_poweroff_noirq(struct device *dev)
> static int pci_pm_restore_noirq(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> - struct device_driver *drv = dev->driver;
> - int error = 0;
> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> + int error;
>
> if (pcibios_pm_ops.restore_noirq) {
> error = pcibios_pm_ops.restore_noirq(dev);
> @@ -1223,17 +1221,16 @@ static int pci_pm_restore_noirq(struct device *dev)
> if (pci_has_legacy_pm_support(pci_dev))
> return pci_legacy_resume_early(dev);
>
> - if (drv && drv->pm && drv->pm->restore_noirq)
> - error = drv->pm->restore_noirq(dev);
> + if (pm && pm->restore_noirq)
> + return pm->restore_noirq(dev);
>
> - return error;
> + return 0;
> }
>
> static int pci_pm_restore(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> - int error = 0;
>
> /*
> * This is necessary for the hibernation error path in which restore is
> @@ -1249,12 +1246,12 @@ static int pci_pm_restore(struct device *dev)
>
> if (pm) {
> if (pm->restore)
> - error = pm->restore(dev);
> + return pm->restore(dev);
> } else {
> pci_pm_reenable_device(pci_dev);
> }
>
> - return error;
> + return 0;
> }
>
> #else /* !CONFIG_HIBERNATE_CALLBACKS */
> @@ -1330,9 +1327,9 @@ static int pci_pm_runtime_suspend(struct device *dev)
>
> static int pci_pm_runtime_resume(struct device *dev)
> {
> - int rc = 0;
> struct pci_dev *pci_dev = to_pci_dev(dev);
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> + int error = 0;
>
> /*
> * Restoring config space is necessary even if the device is not bound
> @@ -1348,18 +1345,17 @@ static int pci_pm_runtime_resume(struct device *dev)
> pci_pm_default_resume(pci_dev);
>
> if (pm && pm->runtime_resume)
> - rc = pm->runtime_resume(dev);
> + error = pm->runtime_resume(dev);
>
> pci_dev->runtime_d3cold = false;
>
> - return rc;
> + return error;
> }
>
> static int pci_pm_runtime_idle(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> - int ret = 0;
>
> /*
> * If pci_dev->driver is not set (unbound), the device should
> @@ -1372,9 +1368,9 @@ static int pci_pm_runtime_idle(struct device *dev)
> return -ENOSYS;
>
> if (pm->runtime_idle)
> - ret = pm->runtime_idle(dev);
> + return pm->runtime_idle(dev);
>
> - return ret;
> + return 0;
> }
>
> static const struct dev_pm_ops pci_dev_pm_ops = {
>
^ permalink raw reply
* Re: [PATCH 4/7] PCI/PM: Run resume fixups before disabling wakeup events
From: Rafael J. Wysocki @ 2019-10-15 17:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dexuan Cui, Rafael J . Wysocki, Lorenzo Pieralisi, Michael Kelley,
Sasha Levin, Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
olaf, apw, jasowang, vkuznets, marcelo.cerri, jackm, linux-pci,
linux-hyperv, linux-kernel, driverdev-devel, Bjorn Helgaas
In-Reply-To: <20191014230016.240912-5-helgaas@kernel.org>
On Tuesday, October 15, 2019 1:00:13 AM CEST Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> pci_pm_resume() and pci_pm_restore() call pci_pm_default_resume(), which
> runs resume fixups before disabling wakeup events:
>
> static void pci_pm_default_resume(struct pci_dev *pci_dev)
> {
> pci_fixup_device(pci_fixup_resume, pci_dev);
> pci_enable_wake(pci_dev, PCI_D0, false);
> }
>
> pci_pm_runtime_resume() does both of these, but in the opposite order:
>
> pci_enable_wake(pci_dev, PCI_D0, false);
> pci_fixup_device(pci_fixup_resume, pci_dev);
>
> We should always use the same ordering unless there's a reason to do
> otherwise.
Right.
> Change pci_pm_runtime_resume() to call pci_pm_default_resume()
> instead of open-coding this, so the fixups are always done before disabling
> wakeup events.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
No concerns about this change, so
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/pci/pci-driver.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 0c3086793e4e..55acb658273f 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1345,8 +1345,7 @@ static int pci_pm_runtime_resume(struct device *dev)
> return 0;
>
> pci_fixup_device(pci_fixup_resume_early, pci_dev);
> - pci_enable_wake(pci_dev, PCI_D0, false);
> - pci_fixup_device(pci_fixup_resume, pci_dev);
> + pci_pm_default_resume(pci_dev);
>
> if (pm && pm->runtime_resume)
> rc = pm->runtime_resume(dev);
>
^ permalink raw reply
* Re: [PATCH 3/7] PCI/PM: Clear PCIe PME Status even for legacy power management
From: Rafael J. Wysocki @ 2019-10-15 17:20 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dexuan Cui, Rafael J . Wysocki, Lorenzo Pieralisi, Michael Kelley,
Sasha Levin, Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
olaf, apw, jasowang, vkuznets, marcelo.cerri, jackm, linux-pci,
linux-hyperv, linux-kernel, driverdev-devel, Bjorn Helgaas
In-Reply-To: <20191014230016.240912-4-helgaas@kernel.org>
On Tuesday, October 15, 2019 1:00:12 AM CEST Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Previously, pci_pm_resume_noirq() cleared the PME Status bit in the Root
> Status register only if the device had no driver or the driver did not
> implement legacy power management. It should clear PME Status regardless
> of what sort of power management the driver supports, so do this before
> checking for legacy power management.
>
> This affects Root Ports and Root Complex Event Collectors, for which the
> usual driver is the PCIe portdrv, which implements new power management, so
> this change is just on principle, not to fix any actual defects.
>
> Fixes: a39bd851dccf ("PCI/PM: Clear PCIe PME Status bit in core, not PCIe port driver")
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
This is a reasonable change in my view, so
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/pci/pci-driver.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index d4ac8ce8c1f9..0c3086793e4e 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -941,12 +941,11 @@ static int pci_pm_resume_noirq(struct device *dev)
> pci_pm_default_resume_early(pci_dev);
>
> pci_fixup_device(pci_fixup_resume_early, pci_dev);
> + pcie_pme_root_status_cleanup(pci_dev);
>
> if (pci_has_legacy_pm_support(pci_dev))
> return pci_legacy_resume_early(dev);
>
> - pcie_pme_root_status_cleanup(pci_dev);
> -
> if (drv && drv->pm && drv->pm->resume_noirq)
> error = drv->pm->resume_noirq(dev);
>
>
^ permalink raw reply
* Re: [PATCH 2/7] PCI/PM: Correct pci_pm_thaw_noirq() documentation
From: Rafael J. Wysocki @ 2019-10-15 17:17 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dexuan Cui, Rafael J . Wysocki, Lorenzo Pieralisi, Michael Kelley,
Sasha Levin, Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
olaf, apw, jasowang, vkuznets, marcelo.cerri, jackm, linux-pci,
linux-hyperv, linux-kernel, driverdev-devel, Bjorn Helgaas
In-Reply-To: <20191014230016.240912-3-helgaas@kernel.org>
On Tuesday, October 15, 2019 1:00:11 AM CEST Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> According to the documentation, pci_pm_thaw_noirq() did not put the device
> into the full-power state and restore its standard configuration registers.
> This is incorrect, so update the documentation to match the code.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Right, the documentation is outdated, so
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> Documentation/power/pci.rst | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
> index 0e2ef7429304..1525c594d631 100644
> --- a/Documentation/power/pci.rst
> +++ b/Documentation/power/pci.rst
> @@ -600,17 +600,17 @@ using the following PCI bus type's callbacks::
>
> respectively.
>
> -The first of them, pci_pm_thaw_noirq(), is analogous to pci_pm_resume_noirq(),
> -but it doesn't put the device into the full power state and doesn't attempt to
> -restore its standard configuration registers. It also executes the device
> -driver's pm->thaw_noirq() callback, if defined, instead of pm->resume_noirq().
> +The first of them, pci_pm_thaw_noirq(), is analogous to pci_pm_resume_noirq().
> +It puts the device into the full power state and restores its standard
> +configuration registers. It also executes the device driver's pm->thaw_noirq()
> +callback, if defined, instead of pm->resume_noirq().
>
> The pci_pm_thaw() routine is similar to pci_pm_resume(), but it runs the device
> driver's pm->thaw() callback instead of pm->resume(). It is executed
> asynchronously for different PCI devices that don't depend on each other in a
> known way.
>
> -The complete phase it the same as for system resume.
> +The complete phase is the same as for system resume.
>
> After saving the image, devices need to be powered down before the system can
> enter the target sleep state (ACPI S4 for ACPI-based systems). This is done in
>
^ permalink raw reply
* Re: [PATCH 1/7] PCI/PM: Always return devices to D0 when thawing
From: Rafael J. Wysocki @ 2019-10-15 17:15 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Dexuan Cui, Rafael J . Wysocki, Lorenzo Pieralisi, Michael Kelley,
Sasha Levin, Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
olaf, apw, jasowang, vkuznets, marcelo.cerri, jackm, linux-pci,
linux-hyperv, linux-kernel, driverdev-devel, Bjorn Helgaas,
Linux PM
In-Reply-To: <20191014230016.240912-2-helgaas@kernel.org>
On Tuesday, October 15, 2019 1:00:10 AM CEST Bjorn Helgaas wrote:
> From: Dexuan Cui <decui@microsoft.com>
>
> pci_pm_thaw_noirq() is supposed to return the device to D0 and restore its
> configuration registers, but previously it only did that for devices whose
> drivers implemented the new power management ops.
>
> Hibernation, e.g., via "echo disk > /sys/power/state", involves freezing
> devices, creating a hibernation image, thawing devices, writing the image,
> and powering off. The fact that thawing did not return devices with legacy
> power management to D0 caused errors, e.g., in this path:
>
> pci_pm_thaw_noirq
> if (pci_has_legacy_pm_support(pci_dev)) # true for Mellanox VF driver
> return pci_legacy_resume_early(dev) # ... legacy PM skips the rest
> pci_set_power_state(pci_dev, PCI_D0)
> pci_restore_state(pci_dev)
> pci_pm_thaw
> if (pci_has_legacy_pm_support(pci_dev))
> pci_legacy_resume
> drv->resume
> mlx4_resume
> ...
> pci_enable_msix_range
> ...
> if (dev->current_state != PCI_D0) # <---
> return -EINVAL;
>
> which caused these warnings:
>
> mlx4_core a6d1:00:02.0: INTx is not supported in multi-function mode, aborting
> PM: dpm_run_callback(): pci_pm_thaw+0x0/0xd7 returns -95
> PM: Device a6d1:00:02.0 failed to thaw: error -95
>
> Return devices to D0 and restore config registers for all devices, not just
> those whose drivers support new power management.
>
> [bhelgaas: also call pci_restore_state() before pci_legacy_resume_early(),
> update comment, add stable tag, commit log]
> Link: https://lore.kernel.org/r/KU1P153MB016637CAEAD346F0AA8E3801BFAD0@KU1P153MB0166.APCP153.PROD.OUTLOOK.COM
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: stable@vger.kernel.org # v4.13+
No issues found, so
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/pci/pci-driver.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index a8124e47bf6e..d4ac8ce8c1f9 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1076,17 +1076,22 @@ static int pci_pm_thaw_noirq(struct device *dev)
> return error;
> }
>
> - if (pci_has_legacy_pm_support(pci_dev))
> - return pci_legacy_resume_early(dev);
> -
> /*
> - * pci_restore_state() requires the device to be in D0 (because of MSI
> - * restoration among other things), so force it into D0 in case the
> - * driver's "freeze" callbacks put it into a low-power state directly.
> + * Both the legacy ->resume_early() and the new pm->thaw_noirq()
> + * callbacks assume the device has been returned to D0 and its
> + * config state has been restored.
> + *
> + * In addition, pci_restore_state() restores MSI-X state in MMIO
> + * space, which requires the device to be in D0, so return it to D0
> + * in case the driver's "freeze" callbacks put it into a low-power
> + * state.
> */
> pci_set_power_state(pci_dev, PCI_D0);
> pci_restore_state(pci_dev);
>
> + if (pci_has_legacy_pm_support(pci_dev))
> + return pci_legacy_resume_early(dev);
> +
> if (drv && drv->pm && drv->pm->thaw_noirq)
> error = drv->pm->thaw_noirq(dev);
>
>
^ permalink raw reply
* Re: [PATCH v3 0/3] Drivers: hv: vmbus: Miscellaneous improvements
From: Wei Liu @ 2019-10-15 14:26 UTC (permalink / raw)
To: Andrea Parri
Cc: Linux Kernel List, Linux on Hyper-V List, K . Y . Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Michael Kelley,
Vitaly Kuznetsov, Dexuan Cui, Wei Liu
In-Reply-To: <20191015114646.15354-1-parri.andrea@gmail.com>
On Tue, 15 Oct 2019 at 12:47, Andrea Parri <parri.andrea@gmail.com> wrote:
[...]
> [1] https://lkml.kernel.org/r/20191010154600.23875-1-parri.andrea@gmail.com
> [2] https://lkml.kernel.org/r/20191007163115.26197-1-parri.andrea@gmail.com
>
> Andrea Parri (3):
> Drivers: hv: vmbus: Introduce table of VMBus protocol versions
> Drivers: hv: vmbus: Enable VMBus protocol versions 4.1, 5.1 and 5.2
> Drivers: hv: vmbus: Add module parameter to cap the VMBus version
For the whole series:
Reviewed-by: Wei Liu <wei.liu@kernel.org>
^ permalink raw reply
* RE: [PATCH v3 3/3] Drivers: hv: vmbus: Add module parameter to cap the VMBus version
From: Michael Kelley @ 2019-10-15 13:15 UTC (permalink / raw)
To: Andrea Parri, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
vkuznets, Dexuan Cui, Wei Liu
In-Reply-To: <20191015114646.15354-4-parri.andrea@gmail.com>
From: Andrea Parri <parri.andrea@gmail.com> Sent: Tuesday, October 15, 2019 4:47 AM
>
> Currently, Linux guests negotiate the VMBus version with Hyper-V
> and use the highest available VMBus version they can connect to.
> This has some drawbacks: by using the highest available version,
> certain code paths are never executed and can not be tested when
> the guest runs on the newest host.
>
> Add the module parameter "max_version", to upper-bound the VMBus
> versions guests can negotiate.
>
> Suggested-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
> drivers/hv/connection.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox