* 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 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 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 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
* [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
* [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
* 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
* 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 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-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
* [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
* [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 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 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 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 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] video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.
From: Wei Hu @ 2019-10-22 11:10 UTC (permalink / raw)
To: b.zolnierkie@samsung.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, hch@lst.de,
m.szyprowski@samsung.com, robin.murphy@arm.com,
mchehab+samsung@kernel.org, sam@ravnborg.org,
gregkh@linuxfoundation.org, alexandre.belloni@bootlin.com,
info@metux.net, arnd@arndb.de, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, iommu@lists.linux-foundation.org,
dcui@microsoft.com, Michael Kelley
Cc: Wei Hu
On Hyper-V, Generation 1 VMs can directly use VM's physical memory for
their framebuffers. This can improve the efficiency of framebuffer and
overall performence for VM. The physical memory assigned to framebuffer
must be contiguous. We use CMA allocator to get contiguouse physicial
memory when the framebuffer size is greater than 4MB. For size under
4MB, we use alloc_pages to achieve this.
To enable framebuffer memory allocation from CMA, supply a kernel
parameter to give enough space to CMA allocator at boot time. For
example:
cma=130m
This gives 130MB memory to CAM allocator that can be allocated to
framebuffer. If this fails, we fall back to the old way of using
mmio for framebuffer.
Signed-off-by: Wei Hu <weh@microsoft.com>
---
drivers/video/fbdev/Kconfig | 1 +
drivers/video/fbdev/hyperv_fb.c | 179 +++++++++++++++++++++++++-------
kernel/dma/contiguous.c | 2 +
3 files changed, 147 insertions(+), 35 deletions(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index aa9541bf964b..f534059461ee 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2215,6 +2215,7 @@ config FB_HYPERV
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
select FB_DEFERRED_IO
+ select DMA_CMA
help
This framebuffer driver supports Microsoft Hyper-V Synthetic Video.
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 3f60b7bc8589..ea2fd3481225 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -31,6 +31,16 @@
* "set-vmvideo" command. For example
* set-vmvideo -vmname name -horizontalresolution:1920 \
* -verticalresolution:1200 -resolutiontype single
+ *
+ * Gen 1 VMs also support directly using VM's phyiscal memory for framebuffer.
+ * It could improve the efficiency and performance for framebuffer and VM.
+ * This requires to allocate contiguous physical memory from Linux kernel's
+ * CMA memory allocator. To enable this, supply a kernel parameter to give
+ * enough memory space to CMA allocator for framebuffer. For example:
+ * cma=130m
+ * This gives 130MB memory to CMA allocator that can be allocated to
+ * framebuffer. For reference, 8K resolution (7680x4320) takes about
+ * 127MB memory.
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -42,6 +52,7 @@
#include <linux/fb.h>
#include <linux/pci.h>
#include <linux/efi.h>
+#include <linux/dma-contiguous.h>
#include <linux/hyperv.h>
@@ -227,7 +238,6 @@ struct synthvid_msg {
} __packed;
-
/* FB driver definitions and structures */
#define HVFB_WIDTH 1152 /* default screen width */
#define HVFB_HEIGHT 864 /* default screen height */
@@ -256,6 +266,9 @@ struct hvfb_par {
/* If true, the VSC notifies the VSP on every framebuffer change */
bool synchronous_fb;
+ /* If true, need to copy from deferred IO mem to framebuffer mem */
+ bool need_docopy;
+
struct notifier_block hvfb_panic_nb;
/* Memory for deferred IO and frame buffer itself */
@@ -432,7 +445,7 @@ static void synthvid_deferred_io(struct fb_info *p,
maxy = max_t(int, maxy, y2);
/* Copy from dio space to mmio address */
- if (par->fb_ready)
+ if (par->fb_ready && par->need_docopy)
hvfb_docopy(par, start, PAGE_SIZE);
}
@@ -749,12 +762,12 @@ static void hvfb_update_work(struct work_struct *w)
return;
/* Copy the dirty rectangle to frame buffer memory */
- for (j = y1; j < y2; j++) {
- hvfb_docopy(par,
- j * info->fix.line_length +
- (x1 * screen_depth / 8),
- (x2 - x1) * screen_depth / 8);
- }
+ if (par->need_docopy)
+ for (j = y1; j < y2; j++)
+ hvfb_docopy(par,
+ j * info->fix.line_length +
+ (x1 * screen_depth / 8),
+ (x2 - x1) * screen_depth / 8);
/* Refresh */
if (par->fb_ready && par->update)
@@ -799,7 +812,8 @@ static int hvfb_on_panic(struct notifier_block *nb,
par = container_of(nb, struct hvfb_par, hvfb_panic_nb);
par->synchronous_fb = true;
info = par->info;
- hvfb_docopy(par, 0, dio_fb_size);
+ if (par->need_docopy)
+ hvfb_docopy(par, 0, dio_fb_size);
synthvid_update(info, 0, 0, INT_MAX, INT_MAX);
return NOTIFY_DONE;
@@ -938,6 +952,62 @@ static void hvfb_get_option(struct fb_info *info)
return;
}
+/*
+ * Allocate enough contiguous physical memory.
+ * Return physical address if succeeded or -1 if failed.
+ */
+static unsigned long hvfb_get_phymem(unsigned int request_size)
+{
+ struct page *page = NULL;
+ unsigned int request_pages;
+ unsigned long paddr = 0;
+ unsigned int order = get_order(request_size);
+
+ if (request_size == 0)
+ return -1;
+
+ /* Try call alloc_pages if the size is less than 2^MAX_ORDER */
+ if (order < MAX_ORDER) {
+ page = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
+ if (!page)
+ return -1;
+
+ request_pages = (1 << order);
+ goto get_phymem1;
+ }
+
+ /* Allocate from CMA */
+ // request_pages = (request_size >> PAGE_SHIFT) + 1;
+ request_pages = (round_up(request_size, PAGE_SIZE) >> PAGE_SHIFT);
+ page = dma_alloc_from_contiguous(NULL, request_pages, 0, false);
+
+ if (page == NULL)
+ return -1;
+
+get_phymem1:
+ paddr = (page_to_pfn(page) << PAGE_SHIFT);
+
+ pr_info("Allocated %d pages starts at physical addr 0x%lx\n",
+ request_pages, paddr);
+
+ return paddr;
+}
+
+/* Release contiguous physical memory */
+static void hvfb_release_phymem(unsigned long paddr, unsigned int size)
+{
+ unsigned int order = get_order(size);
+
+ if (order < MAX_ORDER)
+ __free_pages(pfn_to_page(paddr >> PAGE_SHIFT), order);
+ else
+ dma_release_from_contiguous(NULL,
+ pfn_to_page(paddr >> PAGE_SHIFT),
+ (round_up(size, PAGE_SIZE) >>
+ PAGE_SHIFT));
+ // (size >> PAGE_SHIFT) + 1);
+}
+
/* Get framebuffer memory from Hyper-V video pci space */
static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
@@ -947,8 +1017,58 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
void __iomem *fb_virt;
int gen2vm = efi_enabled(EFI_BOOT);
resource_size_t pot_start, pot_end;
+ unsigned long paddr;
int ret;
+ if (!gen2vm) {
+ pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
+ PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
+ if (!pdev) {
+ pr_err("Unable to find PCI Hyper-V video\n");
+ return -ENODEV;
+ }
+ }
+
+ info->apertures = alloc_apertures(1);
+ if (!info->apertures)
+ goto err1;
+
+ if (gen2vm) {
+ info->apertures->ranges[0].base = screen_info.lfb_base;
+ info->apertures->ranges[0].size = screen_info.lfb_size;
+ } else {
+ info->apertures->ranges[0].base = pci_resource_start(pdev, 0);
+ info->apertures->ranges[0].size = pci_resource_len(pdev, 0);
+ }
+
+ /*
+ * For Gen 1 VM, we can directly use the contiguous memory
+ * from VM. If we success, deferred IO happens directly
+ * on this allocated framebuffer memory, avoiding extra
+ * memory copy.
+ */
+ if (!gen2vm) {
+ paddr = hvfb_get_phymem(screen_fb_size);
+ if (paddr != (unsigned long) -1) {
+ par->mmio_pp = paddr;
+ par->mmio_vp = par->dio_vp = __va(paddr);
+
+ info->fix.smem_start = paddr;
+ info->fix.smem_len = screen_fb_size;
+ info->screen_base = par->mmio_vp;
+ info->screen_size = screen_fb_size;
+
+ par->need_docopy = false;
+ goto getmem1;
+ } else {
+ pr_info("Unable to allocate enough contiguous physical memory on Gen 1 VM. Use MMIO instead.\n");
+ }
+ }
+
+ /*
+ * Cannot use the contiguous physical memory.
+ * Allocate mmio space for framebuffer.
+ */
dio_fb_size =
screen_width * screen_height * screen_depth / 8;
@@ -956,13 +1076,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
pot_start = 0;
pot_end = -1;
} else {
- pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
- PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
- if (!pdev) {
- pr_err("Unable to find PCI Hyper-V video\n");
- return -ENODEV;
- }
-
if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM) ||
pci_resource_len(pdev, 0) < screen_fb_size) {
pr_err("Resource not available or (0x%lx < 0x%lx)\n",
@@ -991,20 +1104,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
if (par->dio_vp == NULL)
goto err3;
- info->apertures = alloc_apertures(1);
- if (!info->apertures)
- goto err4;
-
- if (gen2vm) {
- info->apertures->ranges[0].base = screen_info.lfb_base;
- info->apertures->ranges[0].size = screen_info.lfb_size;
- remove_conflicting_framebuffers(info->apertures,
- KBUILD_MODNAME, false);
- } else {
- info->apertures->ranges[0].base = pci_resource_start(pdev, 0);
- info->apertures->ranges[0].size = pci_resource_len(pdev, 0);
- }
-
/* Physical address of FB device */
par->mmio_pp = par->mem->start;
/* Virtual address of FB device */
@@ -1015,13 +1114,17 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
info->screen_base = par->dio_vp;
info->screen_size = dio_fb_size;
+ par->need_docopy = true;
+
+getmem1:
+ remove_conflicting_framebuffers(info->apertures,
+ KBUILD_MODNAME, false);
+
if (!gen2vm)
pci_dev_put(pdev);
return 0;
-err4:
- vfree(par->dio_vp);
err3:
iounmap(fb_virt);
err2:
@@ -1039,9 +1142,14 @@ static void hvfb_putmem(struct fb_info *info)
{
struct hvfb_par *par = info->par;
- vfree(par->dio_vp);
- iounmap(info->screen_base);
- vmbus_free_mmio(par->mem->start, screen_fb_size);
+ if (par->need_docopy) {
+ vfree(par->dio_vp);
+ iounmap(info->screen_base);
+ vmbus_free_mmio(par->mem->start, screen_fb_size);
+ } else {
+ hvfb_release_phymem(info->fix.smem_start, screen_fb_size);
+ }
+
par->mem = NULL;
}
@@ -1060,6 +1168,7 @@ static int hvfb_probe(struct hv_device *hdev,
par = info->par;
par->info = info;
par->fb_ready = false;
+ par->need_docopy = false;
init_completion(&par->wait);
INIT_DELAYED_WORK(&par->dwork, hvfb_update_work);
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 69cfb4345388..4553f4cca80e 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -197,6 +197,7 @@ struct page *dma_alloc_from_contiguous(struct device *dev, size_t count,
return cma_alloc(dev_get_cma_area(dev), count, align, no_warn);
}
+EXPORT_SYMBOL(dma_alloc_from_contiguous);
/**
* dma_release_from_contiguous() - release allocated pages
@@ -213,6 +214,7 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages,
{
return cma_release(dev_get_cma_area(dev), pages, count);
}
+EXPORT_SYMBOL(dma_release_from_contiguous);
/**
* dma_alloc_contiguous() - allocate contiguous pages
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v7 3/5] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Vitaly Kuznetsov @ 2019-10-22 11:36 UTC (permalink / raw)
To: Zhenzhong Duan, linux-kernel
Cc: tglx, mingo, bp, x86, pbonzini, rkrcmar, sean.j.christopherson,
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-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>
> 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);
You probably need to add 'return' before 'out:' as it seems you're
disabling virt_spin_lock_key in all cases now).
> }
>
> #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
--
Vitaly
^ permalink raw reply
* Re: [PATCH v7 3/5] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Zhenzhong Duan @ 2019-10-22 12:46 UTC (permalink / raw)
To: Vitaly Kuznetsov, linux-kernel
Cc: tglx, mingo, bp, x86, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, boris.ostrovsky, jgross, peterz, will,
linux-hyperv, kvm, mikelley, kys, haiyangz, sthemmin, sashal,
Jonathan Corbet, H. Peter Anvin
In-Reply-To: <8736fl1071.fsf@vitty.brq.redhat.com>
Hi Vitaly,
On 2019/10/22 19:36, Vitaly Kuznetsov wrote:
> Zhenzhong Duan<zhenzhong.duan@oracle.com> writes:
>
...snip
>> 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);
> You probably need to add 'return' before 'out:' as it seems you're
> disabling virt_spin_lock_key in all cases now).
virt_spin_lock_key is kept enabled in !kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)
case which is the only case virt_spin_lock() optimization is used.
When PV qspinlock is enabled, virt_spin_lock() isn't called in
__pv_queued_spin_lock_slowpath() in which case we don't care
virt_spin_lock_key's value.
So adding 'return' or not are both ok, I chosed to save a line,
let me know if you prefer to add a 'return' and I'll change it.
btw: __pv_queued_spin_lock_slowpath() is alias of queued_spin_lock_slowpath()
Thanks
Zhenzhong
^ permalink raw reply
* Re: [PATCH v7 3/5] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Vitaly Kuznetsov @ 2019-10-22 13:11 UTC (permalink / raw)
To: Zhenzhong Duan, linux-kernel
Cc: tglx, mingo, bp, x86, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, boris.ostrovsky, jgross, peterz, will,
linux-hyperv, kvm, mikelley, kys, haiyangz, sthemmin, sashal,
Jonathan Corbet, H. Peter Anvin
In-Reply-To: <dbc50272-a4f5-ce7c-ba71-75031521f420@oracle.com>
Zhenzhong Duan <zhenzhong.duan@oracle.com> writes:
> Hi Vitaly,
>
> On 2019/10/22 19:36, Vitaly Kuznetsov wrote:
>
>> Zhenzhong Duan<zhenzhong.duan@oracle.com> writes:
>>
> ...snip
>
>>> 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);
>> You probably need to add 'return' before 'out:' as it seems you're
>> disabling virt_spin_lock_key in all cases now).
>
> virt_spin_lock_key is kept enabled in !kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)
> case which is the only case virt_spin_lock() optimization is used.
>
> When PV qspinlock is enabled, virt_spin_lock() isn't called in
> __pv_queued_spin_lock_slowpath() in which case we don't care
> virt_spin_lock_key's value.
>
True, my bad: I though we still need it enabled for something.
> So adding 'return' or not are both ok, I chosed to save a line,
> let me know if you prefer to add a 'return' and I'll change it.
No, please ignore.
>
> btw: __pv_queued_spin_lock_slowpath() is alias of queued_spin_lock_slowpath()
>
> Thanks
> Zhenzhong
>
--
Vitaly
^ permalink raw reply
* Re: [PATCH v7 2/5] x86/kvm: Change print code to use pr_*() format
From: Sean Christopherson @ 2019-10-22 21:01 UTC (permalink / raw)
To: Zhenzhong Duan
Cc: linux-kernel, tglx, mingo, bp, x86, pbonzini, rkrcmar, vkuznets,
wanpengli, jmattson, joro, boris.ostrovsky, jgross, peterz, will,
linux-hyperv, kvm, mikelley, kys, haiyangz, sthemmin, sashal,
H. Peter Anvin
In-Reply-To: <1571649076-2421-3-git-send-email-zhenzhong.duan@oracle.com>
On Mon, Oct 21, 2019 at 05:11:13PM +0800, Zhenzhong Duan wrote:
> 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
Sort of a silly nit, especially since I suggested kvm_guest...
What about using kvm-guest instead of kvm_guest to be consistent with
kvm-clock, the other prolific logger in a KVM guest.
E.g.
kvm-clock: cpu 1, msr 551e041, secondary cpu clock
kvm-guest: setup async PF for cpu 1
kvm-guest: stealtime: cpu 1, msr 277695f40
kvm-clock: cpu 2, msr 551e081, secondary cpu clock
kvm-guest: setup async PF for cpu 2
kvm-guest: stealtime: cpu 2, msr 277715f40
kvm-clock: cpu 3, msr 551e0c1, secondary cpu clock
kvm-guest: setup async PF for cpu 3
kvm-guest: stealtime: cpu 3, msr 277795f40
kvm-clock: cpu 4, msr 551e101, secondary cpu clock
instead of
kvm-clock: cpu 1, msr 551e041, secondary cpu clock
kvm_guest: setup async PF for cpu 1
kvm_guest: stealtime: cpu 1, msr 277695f40
kvm-clock: cpu 2, msr 551e081, secondary cpu clock
kvm_guest: setup async PF for cpu 2
kvm_guest: stealtime: cpu 2, msr 277715f40
kvm-clock: cpu 3, msr 551e0c1, secondary cpu clock
kvm_guest: setup async PF for cpu 3
kvm_guest: stealtime: cpu 3, msr 277795f40
kvm-clock: cpu 4, msr 551e101, secondary cpu clock
^ permalink raw reply
* Re: [PATCH v7 3/5] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Sean Christopherson @ 2019-10-22 21:03 UTC (permalink / raw)
To: Zhenzhong Duan
Cc: Vitaly Kuznetsov, linux-kernel, tglx, mingo, bp, x86, pbonzini,
rkrcmar, wanpengli, jmattson, joro, boris.ostrovsky, jgross,
peterz, will, linux-hyperv, kvm, mikelley, kys, haiyangz,
sthemmin, sashal, Jonathan Corbet, H. Peter Anvin
In-Reply-To: <dbc50272-a4f5-ce7c-ba71-75031521f420@oracle.com>
On Tue, Oct 22, 2019 at 08:46:46PM +0800, Zhenzhong Duan wrote:
> Hi Vitaly,
>
> On 2019/10/22 19:36, Vitaly Kuznetsov wrote:
>
> >Zhenzhong Duan<zhenzhong.duan@oracle.com> writes:
> >
> ...snip
>
> >>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);
> >You probably need to add 'return' before 'out:' as it seems you're
> >disabling virt_spin_lock_key in all cases now).
>
> virt_spin_lock_key is kept enabled in !kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)
> case which is the only case virt_spin_lock() optimization is used.
>
> When PV qspinlock is enabled, virt_spin_lock() isn't called in
> __pv_queued_spin_lock_slowpath() in which case we don't care
> virt_spin_lock_key's value.
>
> So adding 'return' or not are both ok, I chosed to save a line,
> let me know if you prefer to add a 'return' and I'll change it.
It'd be worth adding a comment here if you end up spinning another version
to change the logging prefix. The logic is sound and I like the end
result, but I had the same knee jerk "this can't be right!?!?" reaction as
Vitaly.
^ permalink raw reply
* Re: [PATCH v7 2/5] x86/kvm: Change print code to use pr_*() format
From: Zhenzhong Duan @ 2019-10-23 1:29 UTC (permalink / raw)
To: Sean Christopherson
Cc: linux-kernel, tglx, mingo, bp, x86, pbonzini, rkrcmar, vkuznets,
wanpengli, jmattson, joro, boris.ostrovsky, jgross, peterz, will,
linux-hyperv, kvm, mikelley, kys, haiyangz, sthemmin, sashal,
H. Peter Anvin
In-Reply-To: <20191022210120.GQ2343@linux.intel.com>
On 2019/10/23 5:01, Sean Christopherson wrote:
> On Mon, Oct 21, 2019 at 05:11:13PM +0800, Zhenzhong Duan wrote:
>> 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
> Sort of a silly nit, especially since I suggested kvm_guest...
>
> What about using kvm-guest instead of kvm_guest to be consistent with
> kvm-clock, the other prolific logger in a KVM guest.
>
> E.g.
>
> kvm-clock: cpu 1, msr 551e041, secondary cpu clock
> kvm-guest: setup async PF for cpu 1
> kvm-guest: stealtime: cpu 1, msr 277695f40
> kvm-clock: cpu 2, msr 551e081, secondary cpu clock
> kvm-guest: setup async PF for cpu 2
> kvm-guest: stealtime: cpu 2, msr 277715f40
> kvm-clock: cpu 3, msr 551e0c1, secondary cpu clock
> kvm-guest: setup async PF for cpu 3
> kvm-guest: stealtime: cpu 3, msr 277795f40
> kvm-clock: cpu 4, msr 551e101, secondary cpu clock
>
> instead of
>
> kvm-clock: cpu 1, msr 551e041, secondary cpu clock
> kvm_guest: setup async PF for cpu 1
> kvm_guest: stealtime: cpu 1, msr 277695f40
> kvm-clock: cpu 2, msr 551e081, secondary cpu clock
> kvm_guest: setup async PF for cpu 2
> kvm_guest: stealtime: cpu 2, msr 277715f40
> kvm-clock: cpu 3, msr 551e0c1, secondary cpu clock
> kvm_guest: setup async PF for cpu 3
> kvm_guest: stealtime: cpu 3, msr 277795f40
> kvm-clock: cpu 4, msr 551e101, secondary cpu clock
Good suggestion, will do, thanks for point out.
Zhenzhong
^ permalink raw reply
* Re: [PATCH v7 3/5] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Zhenzhong Duan @ 2019-10-23 1:36 UTC (permalink / raw)
To: Sean Christopherson
Cc: Vitaly Kuznetsov, linux-kernel, tglx, mingo, bp, x86, pbonzini,
rkrcmar, wanpengli, jmattson, joro, boris.ostrovsky, jgross,
peterz, will, linux-hyperv, kvm, mikelley, kys, haiyangz,
sthemmin, sashal, Jonathan Corbet, H. Peter Anvin
In-Reply-To: <20191022210355.GR2343@linux.intel.com>
On 2019/10/23 5:03, Sean Christopherson wrote:
> On Tue, Oct 22, 2019 at 08:46:46PM +0800, Zhenzhong Duan wrote:
>> Hi Vitaly,
>>
>> On 2019/10/22 19:36, Vitaly Kuznetsov wrote:
>>
>>> Zhenzhong Duan<zhenzhong.duan@oracle.com> writes:
>>>
>> ...snip
>>
>>>> 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);
>>> You probably need to add 'return' before 'out:' as it seems you're
>>> disabling virt_spin_lock_key in all cases now).
>> virt_spin_lock_key is kept enabled in !kvm_para_has_feature(KVM_FEATURE_PV_UNHALT)
>> case which is the only case virt_spin_lock() optimization is used.
>>
>> When PV qspinlock is enabled, virt_spin_lock() isn't called in
>> __pv_queued_spin_lock_slowpath() in which case we don't care
>> virt_spin_lock_key's value.
>>
>> So adding 'return' or not are both ok, I chosed to save a line,
>> let me know if you prefer to add a 'return' and I'll change it.
> It'd be worth adding a comment here if you end up spinning another version
> to change the logging prefix. The logic is sound and I like the end
> result, but I had the same knee jerk "this can't be right!?!?" reaction as
> Vitaly.
Sure, will do in next version.
Thanks
Zhenzhong
^ permalink raw reply
* Re: [PATCH] video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.
From: hch @ 2019-10-23 9:10 UTC (permalink / raw)
To: Wei Hu
Cc: b.zolnierkie@samsung.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, hch@lst.de,
m.szyprowski@samsung.com, robin.murphy@arm.com,
mchehab+samsung@kernel.org, sam@ravnborg.org,
gregkh@linuxfoundation.org, alexandre.belloni@bootlin.com,
info@metux.net, arnd@arndb.de, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, iommu@lists.linux-foundation.org,
dcui@microsoft.com, Michael Kelley
In-Reply-To: <20191022110905.4032-1-weh@microsoft.com>
> + select DMA_CMA
Thіs needs to be
select DMA_CMA if HAVE_DMA_CONTIGUOUS
> +#include <linux/dma-contiguous.h>
> + /* Allocate from CMA */
> + // request_pages = (request_size >> PAGE_SHIFT) + 1;
> + request_pages = (round_up(request_size, PAGE_SIZE) >> PAGE_SHIFT);
> + page = dma_alloc_from_contiguous(NULL, request_pages, 0, false);
dma_alloc_from_contiguous is an internal helper, you must use it
through dma_alloc_coherent and pass a struct device to that function.
> + if (!gen2vm) {
> + pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
> + PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
> + if (!pdev) {
> + pr_err("Unable to find PCI Hyper-V video\n");
> + return -ENODEV;
> + }
> + }
Please actually implement a pci_driver instead of hacks like this.
> + par->need_docopy = false;
> + goto getmem1;
> + } else {
No need for an else after a goto.
^ 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