* [PATCH v4 3/4] xen: Mark "xen_nopvspin" parameter obsolete
From: Zhenzhong Duan @ 2019-10-03 14:02 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, Zhenzhong Duan, Jonathan Corbet, H. Peter Anvin
In-Reply-To: <1570111335-12731-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>
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 | 3 +++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 89d77ea..df1eacc 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5303,8 +5303,9 @@
never -- do not unplug even if version check succeeds
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
@@ -5330,7 +5331,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..bae29a4 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -114,6 +114,8 @@ void xen_uninit_lock_cpu(int cpu)
*/
void __init xen_init_spinlocks(void)
{
+ if (nopvspin)
+ xen_pvspin = false;
/* Don't need to use pvqspinlock code if there is only 1 vCPU. */
if (num_possible_cpus() == 1)
@@ -137,6 +139,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
* Re: [PATCH v6 0/2] hv: vmbus: add fuzz testing to hv device
From: Sasha Levin @ 2019-10-04 14:46 UTC (permalink / raw)
To: Branden Bonaby; +Cc: kys, haiyangz, sthemmin, linux-hyperv, linux-kernel
In-Reply-To: <cover.1570130325.git.brandonbonaby94@gmail.com>
On Thu, Oct 03, 2019 at 05:01:36PM -0400, Branden Bonaby wrote:
>This patchset introduces a testing framework for Hyper-V drivers.
>This framework allows us to introduce delays in the packet receive
>path on a per-device basis. While the current code only supports
>introducing arbitrary delays in the host/guest communication path,
>we intend to expand this to support error injection in the future.
I've queued it up for hyperv-next, thanks!
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v4 1/4] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Boris Ostrovsky @ 2019-10-04 14:52 UTC (permalink / raw)
To: Zhenzhong Duan, linux-kernel
Cc: vkuznets, linux-hyperv, kvm, kys, haiyangz, sthemmin, sashal,
tglx, mingo, bp, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, jgross, sstabellini, peterz,
Jonathan Corbet, H. Peter Anvin, Will Deacon
In-Reply-To: <1570111335-12731-2-git-send-email-zhenzhong.duan@oracle.com>
On 10/3/19 10:02 AM, Zhenzhong Duan wrote:
> void __init kvm_spinlock_init(void)
> {
> - /* Does host kernel support KVM_FEATURE_PV_UNHALT? */
> - if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
> - return;
> -
> - if (kvm_para_has_hint(KVM_HINTS_REALTIME))
> + /*
> + * Don't use the pvqspinlock code if no KVM_FEATURE_PV_UNHALT feature
> + * support, or there is REALTIME hints or only 1 vCPU.
> + */
> + if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT) ||
> + kvm_para_has_hint(KVM_HINTS_REALTIME) ||
> + num_possible_cpus() == 1) {
> + pr_info("PV spinlocks disabled\n");
> 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);
Would it make sense to bring here the other site where the key is
disabled (in kvm_smp_prepare_cpus())?
(and, in fact, shouldn't all of the checks that result in early return
above disable the key?)
-boris
> return;
> + }
> + pr_info("PV spinlocks enabled\n");
>
> __pv_init_lock_hash();
> pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
>
^ permalink raw reply
* Re: [PATCH v4 3/4] xen: Mark "xen_nopvspin" parameter obsolete
From: Boris Ostrovsky @ 2019-10-04 14:57 UTC (permalink / raw)
To: Zhenzhong Duan, linux-kernel
Cc: vkuznets, linux-hyperv, kvm, kys, haiyangz, sthemmin, sashal,
tglx, mingo, bp, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, jgross, sstabellini, peterz,
Jonathan Corbet, H. Peter Anvin
In-Reply-To: <1570111335-12731-4-git-send-email-zhenzhong.duan@oracle.com>
On 10/3/19 10:02 AM, Zhenzhong Duan wrote:
> 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>
> 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>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
with a small nit
> void __init xen_init_spinlocks(void)
> {
> + if (nopvspin)
> + xen_pvspin = false;
>
> /* Don't need to use pvqspinlock code if there is only 1 vCPU. */
> if (num_possible_cpus() == 1)
I'd fold the change into this 'if' statement, I think it will still be
clear what the comment refers to.
-boris
^ permalink raw reply
* Re: [PATCH 0/2] Drivers: hv: Specify buffer size using Hyper-V page size
From: Sasha Levin @ 2019-10-04 15:29 UTC (permalink / raw)
To: Michael Kelley
Cc: Himadri Pandya, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
tglx@linutronix.de, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, himadri18.07
In-Reply-To: <DM5PR21MB01377E1E6DE541E902A12EEFD7B80@DM5PR21MB0137.namprd21.prod.outlook.com>
On Wed, Sep 04, 2019 at 11:40:21PM +0000, Michael Kelley wrote:
>From: Himadri Pandya <himadrispandya@gmail.com> Sent: Wednesday, July 24, 2019 10:03 PM
>>
>> recv_buffer and VMbus ring buffers are sized based on guest page size
>> which Hyper-V assumes to be 4KB. It might not be the case for some
>> architectures. Hence instead use the Hyper-V page size.
>>
>> Himadri Pandya (2):
>> Drivers: hv: Specify receive buffer size using Hyper-V page size
>> Drivers: hv: util: Specify ring buffer size using Hyper-V page size
>>
>> drivers/hv/hv_fcopy.c | 3 ++-
>> drivers/hv/hv_kvp.c | 3 ++-
>> drivers/hv/hv_snapshot.c | 3 ++-
>> drivers/hv/hv_util.c | 13 +++++++------
>> 4 files changed, 13 insertions(+), 9 deletions(-)
>>
>> --
>> 2.17.1
>
>Thomas -- can you pick up this patch set in the x86/hyperv branch
>of your tip tree along with the other patches to fix wrong page size
>assumptions?
I've queued these two for hyperv-next, thanks!
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH 0/2] Drivers: hv: Remove dependencies on guest page size
From: Sasha Levin @ 2019-10-04 15:41 UTC (permalink / raw)
To: Michael Kelley
Cc: Himadri Pandya, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, hpa@zytor.com,
x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, himadri18.07
In-Reply-To: <DM5PR21MB01377F433CD767AF5E917EA6D7B80@DM5PR21MB0137.namprd21.prod.outlook.com>
On Wed, Sep 04, 2019 at 11:41:43PM +0000, Michael Kelley wrote:
>From: Himadri Pandya <himadrispandya@gmail.com>
>>
>> Hyper-V assumes page size to be 4KB. This might not be the case on ARM64
>> architecture. The first patch in this patchset introduces a hyer-v
>> specific function for allocating a zeroed page which can have a
>> different implementation on ARM64 to address the issue of different
>> guest and host page sizes. The second patch removes dependencies on
>> guest page size in vmbus by using hyper-v specific page symbol and
>> functions.
>>
>> Himadri Pandya (2):
>> x86: hv: Add function to allocate zeroed page for Hyper-V
>> Drivers: hv: vmbus: Remove dependencies on guest page size
>>
>> arch/x86/hyperv/hv_init.c | 8 ++++++++
>> arch/x86/include/asm/mshyperv.h | 1 +
>> drivers/hv/connection.c | 14 +++++++-------
>> drivers/hv/vmbus_drv.c | 6 +++---
>> 4 files changed, 19 insertions(+), 10 deletions(-)
>>
>> --
>> 2.17.1
>
>Thomas -- can you pick up this patch in the x86/hyperv branch of your
>tip tree along with the other patches to fix wrong page size assumptions?
I'll take it through the hyper-v tree, there's a bunch of similar work
queued up there already.
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH] Drivers: hv: balloon: Remove dependencies on guest page size
From: Sasha Levin @ 2019-10-04 15:46 UTC (permalink / raw)
To: Michael Kelley
Cc: Himadri Pandya, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
tglx@linutronix.de, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, himadri18.07
In-Reply-To: <DM5PR21MB01379A15CEBBABFB0B165EDDD7B80@DM5PR21MB0137.namprd21.prod.outlook.com>
On Wed, Sep 04, 2019 at 11:37:12PM +0000, Michael Kelley wrote:
>From: Himadri Pandya <himadrispandya@gmail.com> Sent: Friday, August 16, 2019 9:09 PM
>>
>> Hyper-V assumes page size to be 4K. This might not be the case for
>> ARM64 architecture. Hence use hyper-v specific page size and page
>> shift definitions to avoid conflicts between different host and guest
>> page sizes on ARM64.
>>
>> Also, remove some old and incorrect comments and redefine ballooning
>> granularities to handle larger page sizes correctly.
>>
>> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
>> ---
>> drivers/hv/hv_balloon.c | 25 ++++++++++++-------------
>> 1 file changed, 12 insertions(+), 13 deletions(-)
>>
>
>Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>
>Thomas -- can you pick up this patch in the x86/hyperv branch of your
>tip tree along with the other patches to fix wrong page size assumptions?
I've queued this for hyperv-next, thanks!
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH] hv_sock: use HV_HYP_PAGE_SIZE instead of PAGE_SIZE_4K
From: Sasha Levin @ 2019-10-04 15:48 UTC (permalink / raw)
To: Michael Kelley
Cc: Himadri Pandya, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
davem@davemloft.net, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
himadri18.07
In-Reply-To: <MWHPR21MB078479F82BBA6D3E6527ECECD7DF0@MWHPR21MB0784.namprd21.prod.outlook.com>
On Wed, Jul 31, 2019 at 01:02:03AM +0000, Michael Kelley wrote:
>From: Himadri Pandya <himadrispandya@gmail.com> Sent: Wednesday, July 24, 2019 10:11 PM
>>
>> Older windows hosts require the hv_sock ring buffer to be defined
>> using 4K pages. This was achieved by using the symbol PAGE_SIZE_4K
>> defined specifically for this purpose. But now we have a new symbol
>> HV_HYP_PAGE_SIZE defined in hyperv-tlfs which can be used for this.
>>
>> This patch removes the definition of symbol PAGE_SIZE_4K and replaces
>> its usage with the symbol HV_HYP_PAGE_SIZE. This patch also aligns
>> sndbuf and rcvbuf to hyper-v specific page size using HV_HYP_PAGE_SIZE
>> instead of the guest page size(PAGE_SIZE) as hyper-v expects the page
>> size to be 4K and it might not be the case on ARM64 architecture.
>>
>> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
>> ---
>> net/vmw_vsock/hyperv_transport.c | 21 +++++++++++----------
>> 1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>> index f2084e3f7aa4..ecb5d72d8010 100644
>> --- a/net/vmw_vsock/hyperv_transport.c
>> +++ b/net/vmw_vsock/hyperv_transport.c
>> @@ -13,15 +13,16 @@
>> #include <linux/hyperv.h>
>> #include <net/sock.h>
>> #include <net/af_vsock.h>
>> +#include <asm/hyperv-tlfs.h>
>>
>
>Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>
>This patch depends on a prerequisite patch in
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/hyperv
>
>that defines HV_HYP_PAGE_SIZE.
David, the above prerequisite patch is now upstream, so this patch
should be good to go. Would you take it through the net tree or should I
do it via the hyperv tree?
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH 1/2] x86/hyperv: Allow guests to enable InvariantTSC
From: Vitaly Kuznetsov @ 2019-10-04 16:57 UTC (permalink / raw)
To: Andrea Parri, linux-kernel, linux-hyperv, x86
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H . Peter Anvin,
Michael Kelley, Andrea Parri
In-Reply-To: <20191003155200.22022-1-parri.andrea@gmail.com>
Andrea Parri <parri.andrea@gmail.com> writes:
> If the hardware supports TSC scaling, Hyper-V will set bit 15 of the
> HV_PARTITION_PRIVILEGE_MASK in guest VMs with a compatible Hyper-V
> configuration version. Bit 15 corresponds to the
> AccessTscInvariantControls privilege. If this privilege bit is set,
> guests can access the HvSyntheticInvariantTscControl MSR: guests can
> set bit 0 of this synthetic MSR to enable the InvariantTSC feature.
> After setting the synthetic MSR, CPUID will enumerate support for
> InvariantTSC.
I tried getting more information from TLFS but as of 5.0C this feature
is not described there. I'm really interested in why this additional
interface is needed, e.g. why can't Hyper-V just set InvariantTSC
unconditionally when TSC scaling is supported?
>
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
> arch/x86/include/asm/hyperv-tlfs.h | 5 +++++
> arch/x86/kernel/cpu/mshyperv.c | 7 ++++++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index 7741e211f7f51..5f10f7f2098db 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -86,6 +86,8 @@
> #define HV_X64_ACCESS_FREQUENCY_MSRS BIT(11)
> /* AccessReenlightenmentControls privilege */
> #define HV_X64_ACCESS_REENLIGHTENMENT BIT(13)
> +/* AccessTscInvariantControls privilege */
> +#define HV_X64_ACCESS_TSC_INVARIANT BIT(15)
>
> /*
> * Feature identification: indicates which flags were specified at partition
> @@ -278,6 +280,9 @@
> #define HV_X64_MSR_TSC_EMULATION_CONTROL 0x40000107
> #define HV_X64_MSR_TSC_EMULATION_STATUS 0x40000108
>
> +/* TSC invariant control */
> +#define HV_X64_MSR_TSC_INVARIANT_CONTROL 0x40000118
> +
> /*
> * Declare the MSR used to setup pages used to communicate with the hypervisor.
> */
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 267daad8c0360..105844d542e5c 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -286,7 +286,12 @@ static void __init ms_hyperv_init_platform(void)
> machine_ops.shutdown = hv_machine_shutdown;
> machine_ops.crash_shutdown = hv_machine_crash_shutdown;
> #endif
> - mark_tsc_unstable("running on Hyper-V");
> + if (ms_hyperv.features & HV_X64_ACCESS_TSC_INVARIANT) {
> + wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x1);
> + setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> + } else {
> + mark_tsc_unstable("running on Hyper-V");
> + }
>
> /*
> * Generation 2 instances don't support reading the NMI status from
--
Vitaly
^ permalink raw reply
* RE: [PATCH 1/2] x86/hyperv: Allow guests to enable InvariantTSC
From: Michael Kelley @ 2019-10-04 22:05 UTC (permalink / raw)
To: vkuznets, Andrea Parri, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, x86@kernel.org
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H . Peter Anvin,
Andrea Parri
In-Reply-To: <87k19k1mad.fsf@vitty.brq.redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Friday, October 4, 2019 9:57 AM
>
> Andrea Parri <parri.andrea@gmail.com> writes:
>
> > If the hardware supports TSC scaling, Hyper-V will set bit 15 of the
> > HV_PARTITION_PRIVILEGE_MASK in guest VMs with a compatible Hyper-V
> > configuration version. Bit 15 corresponds to the
> > AccessTscInvariantControls privilege. If this privilege bit is set,
> > guests can access the HvSyntheticInvariantTscControl MSR: guests can
> > set bit 0 of this synthetic MSR to enable the InvariantTSC feature.
> > After setting the synthetic MSR, CPUID will enumerate support for
> > InvariantTSC.
>
> I tried getting more information from TLFS but as of 5.0C this feature
> is not described there. I'm really interested in why this additional
> interface is needed, e.g. why can't Hyper-V just set InvariantTSC
> unconditionally when TSC scaling is supported?
>
Yes, this is very new functionality that is not yet available in a released
version of Hyper-V. And as you know, the Hyper-V TLFS has gotten
woefully out-of-date. :-(
Your question is the same question I asked. The reason given by
Hyper-V is to take the more cautious approach of not "automatically"
giving VMs an InvariantTSC due to updating the underlying Hyper-V
version. Instead, guest VMs must have been explicitly coded to take
advantage of the new InvariantTSC feature. It's not clear to me how
much of this caution is driven by Windows guests vs. Linux or FreeBSD
guests, but it is what it is.
Having to explicitly enable the InvariantTSC does give the Linux code
the opportunity to be a bit cleaner by doing things like not marking
the TSC as unstable when the InvariantTSC feature is present, and to
mark the TSC as reliable so we don't try to do TSC synchronization
(which Hyper-V does not want guests to try to do).
Michael
^ permalink raw reply
* RE: [PATCH v2] x86/hyperv: make vapic support x2apic mode
From: Michael Kelley @ 2019-10-04 22:33 UTC (permalink / raw)
To: Roman Kagan
Cc: vkuznets, kvm@vger.kernel.org, Tianyu Lan, Joerg Roedel,
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20191004091855.GA26970@rkaganb.sw.ru>
From: Roman Kagan <rkagan@virtuozzo.com> Sent: Friday, October 4, 2019 2:19 AM
>
> On Fri, Oct 04, 2019 at 03:01:51AM +0000, Michael Kelley wrote:
> > From: Roman Kagan <rkagan@virtuozzo.com> Sent: Thursday, October 3, 2019 5:53 AM
> > > >
> > > > AFAIU you're trying to mirror native_x2apic_icr_write() here but this is
> > > > different from what hv_apic_icr_write() does
> > > > (SET_APIC_DEST_FIELD(id)).
> > >
> > > Right. In xapic mode the ICR2 aka the high 4 bytes of ICR is programmed
> > > with the destination id in the highest byte; in x2apic mode the whole
> > > ICR2 is set to the 32bit destination id.
> > >
> > > > Is it actually correct? (I think you've tested this and it is but)
> > >
> > > As I wrote in the commit log, I haven't tested it in the sense that I
> > > ran a Linux guest in a Hyper-V VM exposing x2apic to the guest, because
> > > I didn't manage to configure it to do so. OTOH I did run a Windows
> > > guest in QEMU/KVM with hv_apic and x2apic enabled and saw it write
> > > destination ids unshifted to the ICR2 part of ICR, so I assume it's
> > > correct.
> > >
> > > > Michael, could you please shed some light here?
> > >
> > > Would be appreciated, indeed.
> > >
> >
> > The newest version of Hyper-V provides an x2apic in a guest VM when the
> > number of vCPUs in the VM is > 240. This version of Hyper-V is beginning
> > to be deployed in Azure to enable the M416v2 VM size, but the functionality
> > is not yet available for the on-premises version of Hyper-V. However, I can
> > test this configuration internally with the above patch -- give me a few days.
> >
> > An additional complication is that when running on Intel processors that offer
> > vAPIC functionality, the Hyper-V "hints" value does *not* recommend using the
> > MSR-based APIC accesses. In this case, memory-mapped access to the x2apic
> > registers is faster than the synthetic MSRs.
>
> I guess you mean "using regular x2apic MSRs compared to the synthetic
> MSRs".
Yes, of course you are correct.
> Indeed they do essentially the same thing, and there's no reason
> for one set of MSRs to be significantly faster than the other. However,
> hv_apic_eoi_write makes use of "apic assists" aka lazy EOI which is
> certainly a win, and I'm not sure if it works without hv_apic.
>
I've checked with the Hyper-V people and the presence of vAPIC makes
a difference. If vAPIC is present in the hardware:
1) Hyper-V does not set the HV_X64_APIC_ACCESS_RECOMMENDED flag
2) The architectural MSRs should be used instead of the Hyper-V
synthetic MSRs, as they are significantly faster. The architectural
MSRs do not cause a VMEXIT because they are handled entirely by
the vAPIC microcode in the CPU. The synthetic MSRs do cause a VMEXIT.
3) The lazy EOI functionality should not be used
If vAPIC is not present in the hardware:
1) Hyper-V will set HV_X64_APIC_ACCESS_RECOMMENDED
2) Either set of MSRs has about the same performance, but we
should use the synthetic MSRs.
3) The lazy EOI functionality has some value and should be used
The same will apply to the AMD AVIC in some Hyper-V updates that
are coming soon.
So I think your code makes sense given the above information. By
Monday I'll try to test it on a Hyper-V guest VM with x2APIC.
Michael
^ permalink raw reply
* RE: [PATCH v4 4/4] x86/hyperv: Mark "hv_nopvspin" parameter obsolete
From: Michael Kelley @ 2019-10-05 17:07 UTC (permalink / raw)
To: Zhenzhong Duan, linux-kernel@vger.kernel.org
Cc: vkuznets, linux-hyperv@vger.kernel.org, kvm@vger.kernel.org,
KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, pbonzini@redhat.com, rkrcmar@redhat.com,
sean.j.christopherson@intel.com, wanpengli@tencent.com,
jmattson@google.com, joro@8bytes.org, boris.ostrovsky@oracle.com,
jgross@suse.com, sstabellini@kernel.org, peterz@infradead.org,
Jonathan Corbet, H. Peter Anvin
In-Reply-To: <1570111335-12731-5-git-send-email-zhenzhong.duan@oracle.com>
From: Zhenzhong Duan <zhenzhong.duan@oracle.com> Sent: Thursday, October 3, 2019 7:02 AM
>
> Map "hv_nopvspin" to "nopvspin".
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.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(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* Re: [PATCH v4 1/4] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Zhenzhong Duan @ 2019-10-06 7:49 UTC (permalink / raw)
To: Boris Ostrovsky, linux-kernel
Cc: vkuznets, linux-hyperv, kvm, kys, haiyangz, sthemmin, sashal,
tglx, mingo, bp, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, jgross, sstabellini, peterz,
Jonathan Corbet, H. Peter Anvin, Will Deacon
In-Reply-To: <26ef7beb-dad0-13c9-fc2f-217a5e046e4d@oracle.com>
On 2019/10/4 22:52, Boris Ostrovsky wrote:
> On 10/3/19 10:02 AM, Zhenzhong Duan wrote:
>> void __init kvm_spinlock_init(void)
>> {
>> - /* Does host kernel support KVM_FEATURE_PV_UNHALT? */
>> - if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
>> - return;
>> -
>> - if (kvm_para_has_hint(KVM_HINTS_REALTIME))
>> + /*
>> + * Don't use the pvqspinlock code if no KVM_FEATURE_PV_UNHALT feature
>> + * support, or there is REALTIME hints or only 1 vCPU.
>> + */
>> + if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT) ||
>> + kvm_para_has_hint(KVM_HINTS_REALTIME) ||
>> + num_possible_cpus() == 1) {
>> + pr_info("PV spinlocks disabled\n");
>> 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);
> Would it make sense to bring here the other site where the key is
> disabled (in kvm_smp_prepare_cpus())?
Thanks for point out, I'll do it. Just not clear if I should do that in a separate patch,
there is a history about that code:
Its original place was here and then moved to kvm_smp_prepare_cpus() by below commit:
34226b6b ("KVM: X86: Fix setup the virt_spin_lock_key before static key get initialized")
which fixed jump_label_init() calling late issue.
Then 8990cac6 ("x86/jump_label: Initialize static branching early") move jump_label_init()
early, so commit 34226b6b could be reverted.
>
> (and, in fact, shouldn't all of the checks that result in early return
> above disable the key?)
I think we should enable he key for !kvm_para_has_feature(KVM_FEATURE_PV_UNHALT) case,
there is lock holder preemption issue as qspinlock is fair lock, virt_spin_lock()
is an optimization to that, imaging one pcpu running 10 vcpus of same guest
contending a same lock.
For kvm_para_has_hint(KVM_HINTS_REALTIME) case, hypervisor hints there is
no preemption and we should disable virt_spin_lock_key to use native qspinlock.
For the UP case, we don't care virt_spin_lock_key value.
For nopvspin case, we intentionally check native qspinlock code performance,
compare it with PV qspinlock, etc. So virt_spin_lock() optimization should be disabled.
Let me know if anything wrong with above understanding. Thanks
Zhenzhong
^ permalink raw reply
* Re: [PATCH v4 3/4] xen: Mark "xen_nopvspin" parameter obsolete
From: Zhenzhong Duan @ 2019-10-06 7:52 UTC (permalink / raw)
To: Boris Ostrovsky, linux-kernel
Cc: vkuznets, linux-hyperv, kvm, kys, haiyangz, sthemmin, sashal,
tglx, mingo, bp, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, jgross, sstabellini, peterz,
Jonathan Corbet, H. Peter Anvin
In-Reply-To: <2c644c4a-f562-3271-ce0b-e60a44d82d89@oracle.com>
On 2019/10/4 22:57, Boris Ostrovsky wrote:
> On 10/3/19 10:02 AM, Zhenzhong Duan wrote:
>> 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>
>> 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>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>
> with a small nit
>
>> void __init xen_init_spinlocks(void)
>> {
>> + if (nopvspin)
>> + xen_pvspin = false;
>>
>> /* Don't need to use pvqspinlock code if there is only 1 vCPU. */
>> if (num_possible_cpus() == 1)
> I'd fold the change into this 'if' statement, I think it will still be
> clear what the comment refers to.
Good suggestion, will do that. Thanks
Zhenzhong
^ permalink raw reply
* [PATCH 5.3 091/166] PCI: pci-hyperv: Fix build errors on non-SYSFS config
From: Greg Kroah-Hartman @ 2019-10-06 17:20 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Randy Dunlap, Lorenzo Pieralisi,
Haiyang Zhang, Matthew Wilcox, Jake Oshins, K. Y. Srinivasan,
Stephen Hemminger, Stephen Hemminger, Sasha Levin, Bjorn Helgaas,
linux-pci, linux-hyperv, Dexuan Cui
In-Reply-To: <20191006171212.850660298@linuxfoundation.org>
From: Randy Dunlap <rdunlap@infradead.org>
[ Upstream commit f58ba5e3f6863ea4486952698898848a6db726c2 ]
Fix build errors when building almost-allmodconfig but with SYSFS
not set (not enabled). Fixes these build errors:
ERROR: "pci_destroy_slot" [drivers/pci/controller/pci-hyperv.ko] undefined!
ERROR: "pci_create_slot" [drivers/pci/controller/pci-hyperv.ko] undefined!
drivers/pci/slot.o is only built when SYSFS is enabled, so
pci-hyperv.o has an implicit dependency on SYSFS.
Make that explicit.
Also, depending on X86 && X86_64 is not needed, so just change that
to depend on X86_64.
Fixes: a15f2c08c708 ("PCI: hv: support reporting serial number as slot information")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Jake Oshins <jakeo@microsoft.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Sasha Levin <sashal@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-hyperv@vger.kernel.org
Cc: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/pci/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 2ab92409210af..297bf928d6522 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -181,7 +181,7 @@ config PCI_LABEL
config PCI_HYPERV
tristate "Hyper-V PCI Frontend"
- depends on X86 && HYPERV && PCI_MSI && PCI_MSI_IRQ_DOMAIN && X86_64
+ depends on X86_64 && HYPERV && PCI_MSI && PCI_MSI_IRQ_DOMAIN && SYSFS
help
The PCI device frontend driver allows the kernel to import arbitrary
PCI devices from a PCI backend to support PCI driver domains.
--
2.20.1
^ permalink raw reply related
* [PATCH 5.2 098/137] PCI: pci-hyperv: Fix build errors on non-SYSFS config
From: Greg Kroah-Hartman @ 2019-10-06 17:21 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Randy Dunlap, Lorenzo Pieralisi,
Haiyang Zhang, Matthew Wilcox, Jake Oshins, K. Y. Srinivasan,
Stephen Hemminger, Stephen Hemminger, Sasha Levin, Bjorn Helgaas,
linux-pci, linux-hyperv, Dexuan Cui
In-Reply-To: <20191006171209.403038733@linuxfoundation.org>
From: Randy Dunlap <rdunlap@infradead.org>
[ Upstream commit f58ba5e3f6863ea4486952698898848a6db726c2 ]
Fix build errors when building almost-allmodconfig but with SYSFS
not set (not enabled). Fixes these build errors:
ERROR: "pci_destroy_slot" [drivers/pci/controller/pci-hyperv.ko] undefined!
ERROR: "pci_create_slot" [drivers/pci/controller/pci-hyperv.ko] undefined!
drivers/pci/slot.o is only built when SYSFS is enabled, so
pci-hyperv.o has an implicit dependency on SYSFS.
Make that explicit.
Also, depending on X86 && X86_64 is not needed, so just change that
to depend on X86_64.
Fixes: a15f2c08c708 ("PCI: hv: support reporting serial number as slot information")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Jake Oshins <jakeo@microsoft.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Sasha Levin <sashal@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-hyperv@vger.kernel.org
Cc: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/pci/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 2ab92409210af..297bf928d6522 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -181,7 +181,7 @@ config PCI_LABEL
config PCI_HYPERV
tristate "Hyper-V PCI Frontend"
- depends on X86 && HYPERV && PCI_MSI && PCI_MSI_IRQ_DOMAIN && X86_64
+ depends on X86_64 && HYPERV && PCI_MSI && PCI_MSI_IRQ_DOMAIN && SYSFS
help
The PCI device frontend driver allows the kernel to import arbitrary
PCI devices from a PCI backend to support PCI driver domains.
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2] PCI: PM: Move to D0 before calling pci_legacy_resume_early()
From: Bjorn Helgaas @ 2019-10-07 13:24 UTC (permalink / raw)
To: Dexuan Cui
Cc: lorenzo.pieralisi@arm.com, linux-pci@vger.kernel.org,
Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Sasha Levin,
Haiyang Zhang, KY Srinivasan, olaf@aepfle.de, apw@canonical.com,
jasowang@redhat.com, vkuznets, marcelo.cerri@canonical.com,
Stephen Hemminger, jackm@mellanox.com
In-Reply-To: <KU1P153MB016637CAEAD346F0AA8E3801BFAD0@KU1P153MB0166.APCP153.PROD.OUTLOOK.COM>
On Wed, Aug 14, 2019 at 01:06:55AM +0000, Dexuan Cui wrote:
>
> In pci_legacy_suspend_late(), the device state is moved to PCI_UNKNOWN.
>
> In pci_pm_thaw_noirq(), the state is supposed to be moved back to PCI_D0,
> but the current code misses the pci_legacy_resume_early() path, so the
> state remains in PCI_UNKNOWN in that path. As a result, in the resume
> phase of hibernation, this causes an error for the Mellanox VF driver,
> which fails to enable MSI-X because pci_msi_supported() is false due
> to dev->current_state != PCI_D0:
>
> mlx4_core a6d1:00:02.0: Detected virtual function - running in slave mode
> mlx4_core a6d1:00:02.0: Sending reset
> mlx4_core a6d1:00:02.0: Sending vhcr0
> mlx4_core a6d1:00:02.0: HCA minimum page size:512
> mlx4_core a6d1:00:02.0: Timestamping is not supported in slave mode
> 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
>
> To be more accurate, the "resume" phase means the "thaw" callbacks which
> run before the system enters hibernation: when the user runs the command
> "echo disk > /sys/power/state" for hibernation, first the kernel "freezes"
> all the devices and creates a hibernation image, then the kernel "thaws"
> the devices including the disk/NIC, writes the memory to the disk, and
> powers down. This patch fixes the error message for the Mellanox VF driver
> in this phase.
>
> When the system starts again, a fresh kernel starts to run, and when the
> kernel detects that a hibernation image was saved, the kernel "quiesces"
> the devices, and then "restores" the devices from the saved image. In this
> path:
> device_resume_noirq() -> ... ->
> pci_pm_restore_noirq() ->
> pci_pm_default_resume_early() ->
> pci_power_up() moves the device states back to PCI_D0. This path is
> not broken and doesn't need my patch.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
This looks like a bugfix for 5839ee7389e8 ("PCI / PM: Force devices to
D0 in pci_pm_thaw_noirq()") so maybe it should be marked for stable as
5839ee7389e8 was?
Rafael, could you confirm?
> ---
>
> changes in v2:
> Updated the changelog with more details.
>
> drivers/pci/pci-driver.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 36dbe960306b..27dfc68db9e7 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1074,15 +1074,16 @@ 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.
> */
> pci_set_power_state(pci_dev, PCI_D0);
> +
> + if (pci_has_legacy_pm_support(pci_dev))
> + return pci_legacy_resume_early(dev);
> +
> pci_restore_state(pci_dev);
>
> if (drv && drv->pm && drv->pm->thaw_noirq)
> --
> 2.19.1
>
^ permalink raw reply
* Re: [PATCH v4 1/4] x86/kvm: Add "nopvspin" parameter to disable PV spinlocks
From: Boris Ostrovsky @ 2019-10-07 14:46 UTC (permalink / raw)
To: Zhenzhong Duan, linux-kernel
Cc: vkuznets, linux-hyperv, kvm, kys, haiyangz, sthemmin, sashal,
tglx, mingo, bp, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, jgross, sstabellini, peterz,
Jonathan Corbet, H. Peter Anvin, Will Deacon
In-Reply-To: <b0d6df7b-00ff-cdd8-f9f2-26af73256f5b@oracle.com>
On 10/6/19 3:49 AM, Zhenzhong Duan wrote:
> On 2019/10/4 22:52, Boris Ostrovsky wrote:
>
>> On 10/3/19 10:02 AM, Zhenzhong Duan wrote:
>>> void __init kvm_spinlock_init(void)
>>> {
>>> - /* Does host kernel support KVM_FEATURE_PV_UNHALT? */
>>> - if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
>>> - return;
>>> -
>>> - if (kvm_para_has_hint(KVM_HINTS_REALTIME))
>>> + /*
>>> + * Don't use the pvqspinlock code if no KVM_FEATURE_PV_UNHALT
>>> feature
>>> + * support, or there is REALTIME hints or only 1 vCPU.
>>> + */
>>> + if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT) ||
>>> + kvm_para_has_hint(KVM_HINTS_REALTIME) ||
>>> + num_possible_cpus() == 1) {
>>> + pr_info("PV spinlocks disabled\n");
>>> 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);
>> Would it make sense to bring here the other site where the key is
>> disabled (in kvm_smp_prepare_cpus())?
>
> Thanks for point out, I'll do it. Just not clear if I should do that
> in a separate patch,
> there is a history about that code:
>
> Its original place was here and then moved to kvm_smp_prepare_cpus()
> by below commit:
> 34226b6b ("KVM: X86: Fix setup the virt_spin_lock_key before static
> key get initialized")
> which fixed jump_label_init() calling late issue.
>
> Then 8990cac6 ("x86/jump_label: Initialize static branching early")
> move jump_label_init()
> early, so commit 34226b6b could be reverted.
Which is similar to what you did earlier for Xen.
>
>>
>> (and, in fact, shouldn't all of the checks that result in early return
>> above disable the key?)
>
> I think we should enable he key for
> !kvm_para_has_feature(KVM_FEATURE_PV_UNHALT) case,
> there is lock holder preemption issue as qspinlock is fair lock,
> virt_spin_lock()
> is an optimization to that, imaging one pcpu running 10 vcpus of same
> guest
> contending a same lock.
Right. I conflated pv lock and virt_spin_lock_key, and that is wrong.
-boris
>
> For kvm_para_has_hint(KVM_HINTS_REALTIME) case, hypervisor hints there is
> no preemption and we should disable virt_spin_lock_key to use native
> qspinlock.
>
> For the UP case, we don't care virt_spin_lock_key value.
>
> For nopvspin case, we intentionally check native qspinlock code
> performance,
> compare it with PV qspinlock, etc. So virt_spin_lock() optimization
> should be disabled.
>
> Let me know if anything wrong with above understanding. Thanks
>
> Zhenzhong
>
^ permalink raw reply
* [PATCH 0/2] Drivers: hv: vmbus: Miscellaneous improvements
From: Andrea Parri @ 2019-10-07 16:31 UTC (permalink / raw)
To: linux-kernel, linux-hyperv
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Michael Kelley, Vitaly Kuznetsov, Andrea Parri
Hi all,
The patchset:
- simplifies/refactors the VMBus negotiation code by introducing
the table of VMBus protocol versions (patch 1/2),
- enables VMBus protocol versions 5.1 and 5.2 (patch 2/2).
Thanks,
Andrea
Andrea Parri (2):
Drivers: hv: vmbus: Introduce table of VMBus protocol versions
Drivers: hv: vmbus: Enable VMBus protocol versions 5.1 and 5.2
drivers/hv/connection.c | 63 +++++++++++++++++------------------------
include/linux/hyperv.h | 6 ++--
2 files changed, 30 insertions(+), 39 deletions(-)
--
2.23.0
^ permalink raw reply
* [PATCH 1/2] Drivers: hv: vmbus: Introduce table of VMBus protocol versions
From: Andrea Parri @ 2019-10-07 16:31 UTC (permalink / raw)
To: linux-kernel, linux-hyperv
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Michael Kelley, Vitaly Kuznetsov, Andrea Parri
In-Reply-To: <20191007163115.26197-1-parri.andrea@gmail.com>
The technique used to get the next VMBus version seems increasisly
clumsy as the number of VMBus versions increases. Performance is
not a concern since this is only done once during system boot; it's
just that we'll end up with more lines of code than is really needed.
As an alternative, introduce a table with the version numbers listed
in order (from the most recent to the oldest). vmbus_connect() loops
through the versions listed in the table until it gets an accepted
connection or gets to the end of the table (invalid version).
Suggested-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
drivers/hv/connection.c | 51 +++++++++++++++--------------------------
include/linux/hyperv.h | 2 --
2 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 6e4c015783ffc..90a32c9d79403 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -40,29 +40,19 @@ EXPORT_SYMBOL_GPL(vmbus_connection);
__u32 vmbus_proto_version;
EXPORT_SYMBOL_GPL(vmbus_proto_version);
-static __u32 vmbus_get_next_version(__u32 current_version)
-{
- switch (current_version) {
- case (VERSION_WIN7):
- return VERSION_WS2008;
-
- case (VERSION_WIN8):
- return VERSION_WIN7;
-
- case (VERSION_WIN8_1):
- return VERSION_WIN8;
-
- case (VERSION_WIN10):
- return VERSION_WIN8_1;
-
- case (VERSION_WIN10_V5):
- return VERSION_WIN10;
-
- case (VERSION_WS2008):
- default:
- return VERSION_INVAL;
- }
-}
+/*
+ * Table of VMBus versions listed from newest to oldest; the table
+ * must terminate with VERSION_INVAL.
+ */
+__u32 vmbus_versions[] = {
+ VERSION_WIN10_V5,
+ VERSION_WIN10,
+ VERSION_WIN8_1,
+ VERSION_WIN8,
+ VERSION_WIN7,
+ VERSION_WS2008,
+ VERSION_INVAL
+};
int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
{
@@ -169,8 +159,8 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
*/
int vmbus_connect(void)
{
- int ret = 0;
struct vmbus_channel_msginfo *msginfo = NULL;
+ int i, ret = 0;
__u32 version;
/* Initialize the vmbus connection */
@@ -244,21 +234,18 @@ int vmbus_connect(void)
* version.
*/
- version = VERSION_CURRENT;
+ for (i = 0; ; i++) {
+ version = vmbus_versions[i];
+ if (version == VERSION_INVAL)
+ goto cleanup;
- do {
ret = vmbus_negotiate_version(msginfo, version);
if (ret == -ETIMEDOUT)
goto cleanup;
if (vmbus_connection.conn_state == CONNECTED)
break;
-
- version = vmbus_get_next_version(version);
- } while (version != VERSION_INVAL);
-
- if (version == VERSION_INVAL)
- goto cleanup;
+ }
vmbus_proto_version = version;
pr_info("Vmbus version:%d.%d\n",
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index b4a017093b697..7073f1eb3618c 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -194,8 +194,6 @@ static inline u32 hv_get_avail_to_write_percent(
#define VERSION_INVAL -1
-#define VERSION_CURRENT VERSION_WIN10_V5
-
/* Make maximum size of pipe payload of 16K */
#define MAX_PIPE_DATA_PAYLOAD (sizeof(u8) * 16384)
--
2.23.0
^ permalink raw reply related
* [PATCH 2/2] Drivers: hv: vmbus: Enable VMBus protocol versions 5.1 and 5.2
From: Andrea Parri @ 2019-10-07 16:31 UTC (permalink / raw)
To: linux-kernel, linux-hyperv
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Michael Kelley, Vitaly Kuznetsov, Andrea Parri
In-Reply-To: <20191007163115.26197-1-parri.andrea@gmail.com>
Hyper-V has added VMBus protocol versions 5.1 and 5.2 in recent release
versions. Allow Linux guests to negotiate these new protocol versions
on versions of Hyper-V that support them.
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
drivers/hv/connection.c | 12 +++++++-----
include/linux/hyperv.h | 4 ++++
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 90a32c9d79403..d05fef3e09080 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -45,6 +45,8 @@ EXPORT_SYMBOL_GPL(vmbus_proto_version);
* must terminate with VERSION_INVAL.
*/
__u32 vmbus_versions[] = {
+ VERSION_WIN10_V5_2,
+ VERSION_WIN10_V5_1,
VERSION_WIN10_V5,
VERSION_WIN10,
VERSION_WIN8_1,
@@ -70,12 +72,12 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
msg->vmbus_version_requested = version;
/*
- * VMBus protocol 5.0 (VERSION_WIN10_V5) requires that we must use
- * VMBUS_MESSAGE_CONNECTION_ID_4 for the Initiate Contact Message,
+ * VMBus protocol 5.0 (VERSION_WIN10_V5) and higher require that we must
+ * use VMBUS_MESSAGE_CONNECTION_ID_4 for the Initiate Contact Message,
* and for subsequent messages, we must use the Message Connection ID
* field in the host-returned Version Response Message. And, with
- * VERSION_WIN10_V5, we don't use msg->interrupt_page, but we tell
- * the host explicitly that we still use VMBUS_MESSAGE_SINT(2) for
+ * VERSION_WIN10_V5 and higher, we don't use msg->interrupt_page, but we
+ * tell the host explicitly that we still use VMBUS_MESSAGE_SINT(2) for
* compatibility.
*
* On old hosts, we should always use VMBUS_MESSAGE_CONNECTION_ID (1).
@@ -400,7 +402,7 @@ int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep)
case HV_STATUS_INVALID_CONNECTION_ID:
/*
* See vmbus_negotiate_version(): VMBus protocol 5.0
- * requires that we must use
+ * and higher require that we must use
* VMBUS_MESSAGE_CONNECTION_ID_4 for the Initiate
* Contact message, but on old hosts that only
* support VMBus protocol 4.0 or lower, here we get
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 7073f1eb3618c..5ecb2ff7cc25d 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -183,6 +183,8 @@ static inline u32 hv_get_avail_to_write_percent(
* 3 . 0 (Windows 8 R2)
* 4 . 0 (Windows 10)
* 5 . 0 (Newer Windows 10)
+ * 5 . 1 (Windows 10 RS4)
+ * 5 . 2 (Windows Server 2019, RS5)
*/
#define VERSION_WS2008 ((0 << 16) | (13))
@@ -191,6 +193,8 @@ static inline u32 hv_get_avail_to_write_percent(
#define VERSION_WIN8_1 ((3 << 16) | (0))
#define VERSION_WIN10 ((4 << 16) | (0))
#define VERSION_WIN10_V5 ((5 << 16) | (0))
+#define VERSION_WIN10_V5_1 ((5 << 16) | (1))
+#define VERSION_WIN10_V5_2 ((5 << 16) | (2))
#define VERSION_INVAL -1
--
2.23.0
^ permalink raw reply related
* Re: [PATCH 1/2] Drivers: hv: vmbus: Introduce table of VMBus protocol versions
From: Vitaly Kuznetsov @ 2019-10-07 17:14 UTC (permalink / raw)
To: Andrea Parri, linux-kernel, linux-hyperv
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
Michael Kelley, Andrea Parri
In-Reply-To: <20191007163115.26197-2-parri.andrea@gmail.com>
Andrea Parri <parri.andrea@gmail.com> writes:
> The technique used to get the next VMBus version seems increasisly
> clumsy as the number of VMBus versions increases. Performance is
> not a concern since this is only done once during system boot; it's
> just that we'll end up with more lines of code than is really needed.
>
> As an alternative, introduce a table with the version numbers listed
> in order (from the most recent to the oldest). vmbus_connect() loops
> through the versions listed in the table until it gets an accepted
> connection or gets to the end of the table (invalid version).
>
> Suggested-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
> drivers/hv/connection.c | 51 +++++++++++++++--------------------------
> include/linux/hyperv.h | 2 --
> 2 files changed, 19 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 6e4c015783ffc..90a32c9d79403 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -40,29 +40,19 @@ EXPORT_SYMBOL_GPL(vmbus_connection);
> __u32 vmbus_proto_version;
> EXPORT_SYMBOL_GPL(vmbus_proto_version);
>
> -static __u32 vmbus_get_next_version(__u32 current_version)
> -{
> - switch (current_version) {
> - case (VERSION_WIN7):
> - return VERSION_WS2008;
> -
> - case (VERSION_WIN8):
> - return VERSION_WIN7;
> -
> - case (VERSION_WIN8_1):
> - return VERSION_WIN8;
> -
> - case (VERSION_WIN10):
> - return VERSION_WIN8_1;
> -
> - case (VERSION_WIN10_V5):
> - return VERSION_WIN10;
> -
> - case (VERSION_WS2008):
> - default:
> - return VERSION_INVAL;
> - }
> -}
> +/*
> + * Table of VMBus versions listed from newest to oldest; the table
> + * must terminate with VERSION_INVAL.
> + */
> +__u32 vmbus_versions[] = {
> + VERSION_WIN10_V5,
> + VERSION_WIN10,
> + VERSION_WIN8_1,
> + VERSION_WIN8,
> + VERSION_WIN7,
> + VERSION_WS2008,
> + VERSION_INVAL
> +};
>
> int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
> {
> @@ -169,8 +159,8 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
> */
> int vmbus_connect(void)
> {
> - int ret = 0;
> struct vmbus_channel_msginfo *msginfo = NULL;
> + int i, ret = 0;
> __u32 version;
>
> /* Initialize the vmbus connection */
> @@ -244,21 +234,18 @@ int vmbus_connect(void)
> * version.
> */
>
> - version = VERSION_CURRENT;
> + for (i = 0; ; i++) {
> + version = vmbus_versions[i];
> + if (version == VERSION_INVAL)
> + goto cleanup;
If you use e.g. ARRAY_SIZE() you can get rid of VERSION_INVAL - and make
this code look more natural.
>
> - do {
> ret = vmbus_negotiate_version(msginfo, version);
> if (ret == -ETIMEDOUT)
> goto cleanup;
>
> if (vmbus_connection.conn_state == CONNECTED)
> break;
> -
> - version = vmbus_get_next_version(version);
> - } while (version != VERSION_INVAL);
> -
> - if (version == VERSION_INVAL)
> - goto cleanup;
> + }
>
> vmbus_proto_version = version;
> pr_info("Vmbus version:%d.%d\n",
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index b4a017093b697..7073f1eb3618c 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -194,8 +194,6 @@ static inline u32 hv_get_avail_to_write_percent(
>
> #define VERSION_INVAL -1
>
> -#define VERSION_CURRENT VERSION_WIN10_V5
> -
> /* Make maximum size of pipe payload of 16K */
> #define MAX_PIPE_DATA_PAYLOAD (sizeof(u8) * 16384)
--
Vitaly
^ permalink raw reply
* RE: [PATCH 1/2] Drivers: hv: vmbus: Introduce table of VMBus protocol versions
From: Dexuan Cui @ 2019-10-07 17:25 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,
Michael Kelley, vkuznets
In-Reply-To: <20191007163115.26197-2-parri.andrea@gmail.com>
> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Andrea Parri
> Sent: Monday, October 7, 2019 9:31 AM
> ....
> +/*
> + * Table of VMBus versions listed from newest to oldest; the table
> + * must terminate with VERSION_INVAL.
> + */
> +__u32 vmbus_versions[] = {
> + VERSION_WIN10_V5,
This should be "static"?
Thanks,
Dexuan
^ permalink raw reply
* RE: [PATCH 0/2] Drivers: hv: vmbus: Miscellaneous improvements
From: Dexuan Cui @ 2019-10-07 17:41 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,
Michael Kelley, vkuznets
In-Reply-To: <20191007163115.26197-1-parri.andrea@gmail.com>
> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Andrea Parri
> Sent: Monday, October 7, 2019 9:31 AM
>
> Hi all,
>
> The patchset:
>
> - simplifies/refactors the VMBus negotiation code by introducing
> the table of VMBus protocol versions (patch 1/2),
>
> - enables VMBus protocol versions 5.1 and 5.2 (patch 2/2).
>
> Thanks,
> Andrea
>
> Andrea Parri (2):
> Drivers: hv: vmbus: Introduce table of VMBus protocol versions
> Drivers: hv: vmbus: Enable VMBus protocol versions 5.1 and 5.2
Should we add a module parameter to allow the user to specify a lower
protocol version, when the VM runs on the latest host?
This can be useful for testing and debugging purpose: the variable
"vmbus_proto_version" is referenced by the vmbus driver itself and
some VSC drivers: if we always use the latest available proto version,
some code paths can not be tested on the latest hosts.
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH v2] PCI: PM: Move to D0 before calling pci_legacy_resume_early()
From: Dexuan Cui @ 2019-10-07 18:57 UTC (permalink / raw)
To: Bjorn Helgaas, Rafael J. Wysocki
Cc: lorenzo.pieralisi@arm.com, linux-pci@vger.kernel.org,
Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Sasha Levin,
Haiyang Zhang, KY Srinivasan, olaf@aepfle.de, apw@canonical.com,
jasowang@redhat.com, vkuznets, marcelo.cerri@canonical.com,
Stephen Hemminger, jackm@mellanox.com
In-Reply-To: <20191007132414.GA19294@google.com>
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Monday, October 7, 2019 6:24 AM
> To: Dexuan Cui <decui@microsoft.com>
> Cc: lorenzo.pieralisi@arm.com; linux-pci@vger.kernel.org; Michael Kelley
> <mikelley@microsoft.com>; linux-hyperv@vger.kernel.org;
> linux-kernel@vger.kernel.org; driverdev-devel@linuxdriverproject.org; Sasha
> Levin <Alexander.Levin@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>;
> olaf@aepfle.de; apw@canonical.com; jasowang@redhat.com; vkuznets
> <vkuznets@redhat.com>; marcelo.cerri@canonical.com; Stephen Hemminger
> <sthemmin@microsoft.com>; jackm@mellanox.com
> Subject: Re: [PATCH v2] PCI: PM: Move to D0 before calling
> pci_legacy_resume_early()
>
> On Wed, Aug 14, 2019 at 01:06:55AM +0000, Dexuan Cui wrote:
> >
> > In pci_legacy_suspend_late(), the device state is moved to PCI_UNKNOWN.
> >
> > In pci_pm_thaw_noirq(), the state is supposed to be moved back to PCI_D0,
> > but the current code misses the pci_legacy_resume_early() path, so the
> > state remains in PCI_UNKNOWN in that path. As a result, in the resume
> > phase of hibernation, this causes an error for the Mellanox VF driver,
> > which fails to enable MSI-X because pci_msi_supported() is false due
> > to dev->current_state != PCI_D0:
> >
> > mlx4_core a6d1:00:02.0: Detected virtual function - running in slave mode
> > mlx4_core a6d1:00:02.0: Sending reset
> > mlx4_core a6d1:00:02.0: Sending vhcr0
> > mlx4_core a6d1:00:02.0: HCA minimum page size:512
> > mlx4_core a6d1:00:02.0: Timestamping is not supported in slave mode
> > 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
> >
> > To be more accurate, the "resume" phase means the "thaw" callbacks which
> > run before the system enters hibernation: when the user runs the command
> > "echo disk > /sys/power/state" for hibernation, first the kernel "freezes"
> > all the devices and creates a hibernation image, then the kernel "thaws"
> > the devices including the disk/NIC, writes the memory to the disk, and
> > powers down. This patch fixes the error message for the Mellanox VF driver
> > in this phase.
> >
> > When the system starts again, a fresh kernel starts to run, and when the
> > kernel detects that a hibernation image was saved, the kernel "quiesces"
> > the devices, and then "restores" the devices from the saved image. In this
> > path:
> > device_resume_noirq() -> ... ->
> > pci_pm_restore_noirq() ->
> > pci_pm_default_resume_early() ->
> > pci_power_up() moves the device states back to PCI_D0. This path is
> > not broken and doesn't need my patch.
> >
> > Signed-off-by: Dexuan Cui <decui@microsoft.com>
>
> This looks like a bugfix for 5839ee7389e8 ("PCI / PM: Force devices to
> D0 in pci_pm_thaw_noirq()") so maybe it should be marked for stable as
> 5839ee7389e8 was?
>
> Rafael, could you confirm?
>
> > ---
> >
> > changes in v2:
> > Updated the changelog with more details.
> >
> > drivers/pci/pci-driver.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index 36dbe960306b..27dfc68db9e7 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -1074,15 +1074,16 @@ 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.
> > */
> > pci_set_power_state(pci_dev, PCI_D0);
> > +
> > + if (pci_has_legacy_pm_support(pci_dev))
> > + return pci_legacy_resume_early(dev);
> > +
> > pci_restore_state(pci_dev);
> >
> > if (drv && drv->pm && drv->pm->thaw_noirq)
> > --
> > 2.19.1
> >
Added Rafael to "To".
Thanks,
-- Dexuan
^ 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