Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Michael Kelley (EOSG)" <Michael.H.Kelley@microsoft.com>
To:     Vitaly Kuznetsov <vkuznets@redhat.com>
CC:     "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
        "olaf@aepfle.de" <olaf@aepfle.de>,
        "apw@canonical.com" <apw@canonical.com>,
        "jasowang@redhat.com" <jasowang@redhat.com>,
        "leann.ogasawara@canonical.com" <leann.ogasawara@canonical.com>,
        "marcelo.cerri@canonical.com" <marcelo.cerri@canonical.com>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        "KY Srinivasan" <kys@microsoft.com>,
        "x86@kernel.org" <x86@kernel.org>
Subject: RE: [PATCH char-misc 1/1] Drivers: hv: vmbus: Implement Direct Mode
 for stimer0
Thread-Topic: [PATCH char-misc 1/1] Drivers: hv: vmbus: Implement Direct Mode
 for stimer0
Thread-Index: AQHTUpZo6Qcg61vIvkmfgDtshVpGsaL/bMdngAAcRGaALcYFYA==
Date:   Thu, 30 Nov 2017 18:08:06 +0000
Message-ID: <DM5PR21MB074745A6683A35CAD008852ADC380@DM5PR21MB0747.namprd21.prod.outlook.com>
References: <20171031221849.12117-1-mikelley@exchange.microsoft.com>
        <87lgjqw6nz.fsf@vitty.brq.redhat.com>
 <87lgjqunf4.fsf@vitty.brq.redhat.com>
In-Reply-To: <87lgjqunf4.fsf@vitty.brq.redhat.com>
Accept-Language: en-US
Content-Language: en-US
msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;
 MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;
 MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Owner=mikelley@ntdev.microsoft.com;
 MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2017-11-30T18:08:02.8389083Z;
 MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=General;
 MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Application=Microsoft Azure
 Information Protection;
 MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Extended_MSFT_Method=Automatic;
 Sensitivity=General
x-originating-ip: [24.22.167.197]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1;DM5PR21MB0154;6:Ilz39mwQt0eNvVWXsq9ezx46+F1q3+bs2f6mAwjsv4JVihB5nyiqqN9WFg+Xr5eQNo60P3KeSN6FIRcadLbCkq9Ke4L3/eED7MY24pCPpH4CvbxPU7HClgP3wn04mRhj8M57hkN//0vGzib0gdHfX75lbRcA/Oy3lbSwqvYvmyun9zvwB+Zx9gTR+0+cCR5pN7Qh5MaNZr1NT2ifYydvehWh2qJHFx6y+anQRm6LGsiyWjpv82WxyD08U6BHme0PfXBF+/Ei7Xz0leNa6V4VvT3/pRhpmwVvT74WJsLnIhXWINFLyzW5Zl4J4fjOlurCT71bujOWq3psk6Uv5NnarNjkNzrJTzJCidKHGNWQWIU=;5:UTVbTsU0QfanNO8/tw+UvknkUTzmqiAf8yWcysrgPKgURpAKT7KKcBCFaz8qjcScwo6QTO9fhYING7d0ZbOu5Me34bG7rjiILTt4Oap6PecqobjTNMoYwR+d44CCAmhAa8GY1JJDPzQGJfurpspTmWYwODzuF5MGUwKOwKziYyQ=;24:AxQCVKQCu2i5M8loiFgN0+tjTyLMjGhy1hlWp6ud1381+AsTb0Ka1ANUrglUSe92c4h4FrJyLm/ZfRU/oRF9qyXUC6B56ZT+EwDjW3UWt+A=;7:A6ADcuPLZZeOuxZ11AYPFZLa7WoV/W1VXPw8HNCYspz7111PILCB+f8u7JQlMiOS9gxoDN/cbYk67m1dJUX3qzSNX0koHlmZh1Nj8uAA0ceYrOepsDTCouMdkfL89vm1axEVvfrci2P+y+WwYwntrT3hE23lU5H3IluKOA0IyAt9zAN5AX6g6Y3ctiw23eQ9l+nZ0mGbs6D5ec4GEMq66wExIZSqSgMiZ7uNCyl/Fhs2X9ezWT9pUUXZCMO2HabT
x-ms-exchange-antispam-srfa-diagnostics: SSOS;SSOR;
x-forefront-antispam-report: SFV:SKI;SCL:-1;SFV:NSPM;SFS:(10019020)(6009001)(366004)(39860400002)(346002)(376002)(47760400005)(199003)(189002)(189998001)(106356001)(105586002)(33656002)(2900100001)(8936002)(68736007)(76176010)(3660700001)(3280700002)(2906002)(8990500004)(14454004)(478600001)(54356010)(10290500003)(50986010)(72206003)(7696005)(229853002)(316002)(22452003)(54906003)(25786009)(99286004)(97736004)(7416002)(102836003)(6116002)(10090500001)(3846002)(5660300001)(8676002)(81166006)(81156014)(305945005)(7736002)(74316002)(55016002)(77096006)(6436002)(6506006)(53936002)(9686003)(6916009)(2950100002)(53946003)(4326008)(6246003)(86362001)(66066001)(86612001)(101416001);DIR:OUT;SFP:1102;SCL:1;SRVR:DM5PR21MB0154;H:DM5PR21MB0747.namprd21.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en;
x-ms-office365-filtering-ht: Tenant
x-ms-office365-filtering-correlation-id: 55201e91-057c-4a9c-51c5-08d5381d4e9f
x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(48565401081)(5600026)(4604075)(2017052603286);SRVR:DM5PR21MB0154;
x-ms-traffictypediagnostic: DM5PR21MB0154:
x-microsoft-antispam-prvs: <DM5PR21MB01547FD0DC65B6BC7FA1468EDC380@DM5PR21MB0154.namprd21.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:(89211679590171)(209352067349851)(140211028294663);
x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(61425038)(6040450)(2401047)(8121501046)(5005006)(93006095)(93001095)(3002001)(3231022)(10201501046)(6055026)(61426038)(61427038)(6041248)(20161123562025)(20161123558100)(20161123560025)(20161123564025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011);SRVR:DM5PR21MB0154;BCL:0;PCL:0;RULEID:(100000803101)(100110400095);SRVR:DM5PR21MB0154;
x-forefront-prvs: 05079D8470
received-spf: None (protection.outlook.com: microsoft.com does not designate
 permitted sender hosts)
authentication-results: spf=none (sender IP is )
 smtp.mailfrom=Michael.H.Kelley@microsoft.com; 
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: microsoft.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 55201e91-057c-4a9c-51c5-08d5381d4e9f
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Nov 2017 18:08:06.5032
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR21MB0154
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Vitaly Kuznetsov <vkuznets@redhat.com> writes:

> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>=20
> > mikelley@exchange.microsoft.com writes:
> >
> >> From: Michael Kelley <mikelley@microsoft.com>
> >>
> >> The 2016 version of Hyper-V offers the option to operate the guest VM
> >> per-vcpu stimer's in Direct Mode, which means the timer interupts on
> >> its own vector rather than queueing a VMbus message.  Direct Mode
> >> reduces timer processing overhead in both the hypervisor and the
> >> guest, and avoids having timer interrupts pollute the VMbus interrupt
> >> stream for the synthetic NIC and storage.  This patch enables Direct
> >> Mode by default on stimer0 (which is the only stimer used in Linux on
> >> Hyper-V) when running on a version of Hyper-V that supports it, with
> >> a hv_vmbus module parameter for disabling Direct Mode and reverting
> >> to the old behavior.
> >>
> >> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> >> ---
> >>  arch/x86/include/asm/mshyperv.h    | 14 ++++++++
> >>  arch/x86/include/uapi/asm/hyperv.h | 26 ++++++++++++++
> >>  arch/x86/kernel/cpu/mshyperv.c     | 64 +++++++++++++++++++++++++++++=
++++-
> >>  drivers/hv/hv.c                    | 71 +++++++++++++++++++++++++++++=
+++++++--
> >>  drivers/hv/hyperv_vmbus.h          |  4 ++-
> >>  5 files changed, 175 insertions(+), 4 deletions(-)
> >>
>=20
> (in addition to my previous comment)
>=20
> This patch is also x86-heavy so it probably makes sense to make x86 maint=
ainers (Thomas,
> Peter, Ingo) aware no matter which tree it will go through.
>=20
> CC: x86@kernel.org
>=20
> >> diff --git a/arch/x86/include/asm/mshyperv.h
> >> b/arch/x86/include/asm/mshyperv.h index 740dc97..1bba1d7 100644
> >> --- a/arch/x86/include/asm/mshyperv.h
> >> +++ b/arch/x86/include/asm/mshyperv.h
> >> @@ -4,6 +4,8 @@
> >>  #include <linux/types.h>
> >>  #include <linux/atomic.h>
> >>  #include <linux/nmi.h>
> >> +#include <linux/irq.h>
> >> +#include <linux/irqdesc.h>
> >>  #include <asm/io.h>
> >>  #include <asm/hyperv.h>
> >>
> >> @@ -374,4 +376,16 @@ static inline struct ms_hyperv_tsc_page *hv_get_t=
sc_page(void)
> >>  	return NULL;
> >>  }
> >>  #endif
> >> +
> >> +/* Per architecture routines for stimer0 Direct Mode handling.  On
> >> +x86/x64
> >> + * there are no percpu actions to take.
> >> + */
> >> +#if IS_ENABLED(CONFIG_HYPERV)
> >> +static inline void hv_enable_stimer0_percpu_irq(int irq) { } static
> >> +inline void hv_disable_stimer0_percpu_irq(int irq) { } extern int
> >> +hv_allocate_stimer0_irq(int *irq, int *vector); extern void
> >> +hv_deallocate_stimer0_irq(int irq); extern void
> >> +hv_ack_stimer0_interrupt(struct irq_desc *desc); #endif
> >> +
> >>  #endif
> >> diff --git a/arch/x86/include/uapi/asm/hyperv.h
> >> b/arch/x86/include/uapi/asm/hyperv.h
> >> index f65d125..408cf3e 100644
> >> --- a/arch/x86/include/uapi/asm/hyperv.h
> >> +++ b/arch/x86/include/uapi/asm/hyperv.h
> >> @@ -112,6 +112,22 @@
> >>  #define HV_X64_GUEST_IDLE_STATE_AVAILABLE		(1 << 5)
> >>  /* Guest crash data handler available */
> >>  #define HV_X64_GUEST_CRASH_MSR_AVAILABLE		(1 << 10)
> >> +/* Debug MSRs available */
> >> +#define HV_X64_DEBUG_MSR_AVAILABLE			(1 << 11)
> >> +/* Support for Non-Privileged Instruction Execution Prevention is ava=
ilable */
> >> +#define HV_X64_NPIEP_AVAILABLE				(1 << 12)
> >> +/* Support for DisableHypervisor is available */
> >> +#define HV_X64_DISABLE_HYPERVISOR_AVAILABLE		(1 << 13)
> >> +/* Extended GVA Ranges for Flush Virtual Address list is available */
> >> +#define HV_X64_EXTENDED_GVA_RANGE_AVAILABLE		(1 << 14)
> >> +/* Return Hypercall output via XMM registers is available */
> >> +#define HV_X64_HYPERCALL_XMM_OUTPUT_AVAILABLE		(1 << 15)
> >> +/* SINT polling mode available */
> >> +#define HV_X64_SINT_POLLING_MODE_AVAILABLE		(1 << 17)
> >> +/* Hypercall MSR lock is available */
> >> +#define HV_X64_HYPERCALL_MSR_LOCK_AVAILABLE		(1 << 18)
> >> +/* stimer direct mode is available */
> >> +#define HV_X64_STIMER_DIRECT_MODE_AVAILABLE		(1 << 19)
> >>
> >>  /*
> >>   * Implementation recommendations. Indicates which behaviors the
> >> hypervisor @@ -300,6 +316,16 @@ enum HV_GENERIC_SET_FORMAT {
> >>
> >>  #define HV_SYNIC_STIMER_COUNT		(4)
> >>
> >> +/* Hardware IRQ number to use for stimer0 in Direct Mode.  This IRQ
> >> +is a fake
> >> + * because stimer's in Direct Mode simply interrupt on the specified
> >> +vector,
> >> + * without using a particular IOAPIC pin. But we use the IRQ
> >> +allocation
> >> + * machinery, so we need a hardware IRQ #.  This value is somewhat
> >> +arbitrary,
> >> + * but it should not be a legacy IRQ (0 to 15), and should fit
> >> +within the
> >> + * single IOAPIC (0 to 23) that Hyper-V provides to a guest VM. So
> >> +any value
> >> + * between 16 and 23 should be good.
> >> + */
> >
> > (nitpick): all comments in the patch are in 'net' style:
> >
> >  /* This is a
> >   * comment.
> >   */
> >
> > While in Hyper-V files (and x86 in general) the 'normal' style is used:
> >
> >  /*
> >   * This is a
> >   * comment.
> >   */
> >
> > But checkpatch.pl doesn't complain.

Will fix in v2.

> >
> >> +#define HV_STIMER0_IRQNR		18
> >> +
> >>  /* Define synthetic interrupt controller message constants. */
> >>  #define HV_MESSAGE_SIZE			(256)
> >>  #define HV_MESSAGE_PAYLOAD_BYTE_COUNT	(240)
> >> diff --git a/arch/x86/kernel/cpu/mshyperv.c
> >> b/arch/x86/kernel/cpu/mshyperv.c index 236324e8..88dc243 100644
> >> --- a/arch/x86/kernel/cpu/mshyperv.c
> >> +++ b/arch/x86/kernel/cpu/mshyperv.c
> >> @@ -19,7 +19,10 @@
> >>  #include <linux/efi.h>
> >>  #include <linux/interrupt.h>
> >>  #include <linux/irq.h>
> >> +#include <linux/irqdomain.h>
> >> +#include <linux/irqdesc.h>
> >>  #include <linux/kexec.h>
> >> +#include <linux/acpi.h>
> >>  #include <asm/processor.h>
> >>  #include <asm/hypervisor.h>
> >>  #include <asm/hyperv.h>
> >> @@ -27,6 +30,7 @@
> >>  #include <asm/desc.h>
> >>  #include <asm/irq_regs.h>
> >>  #include <asm/i8259.h>
> >> +#include <asm/irqdomain.h>
> >>  #include <asm/apic.h>
> >>  #include <asm/timer.h>
> >>  #include <asm/reboot.h>
> >> @@ -69,6 +73,64 @@ void hv_remove_vmbus_irq(void)
> >> EXPORT_SYMBOL_GPL(hv_setup_vmbus_irq);
> >>  EXPORT_SYMBOL_GPL(hv_remove_vmbus_irq);
> >>
> >> +
> >> +/* Routines to do per-architecture handling of stimer0 when in
> >> +Direct Mode */
> >> +
> >> +void hv_ack_stimer0_interrupt(struct irq_desc *desc) {
> >> +	ack_APIC_irq();
> >> +}
> >> +
> >> +static void allonline_vector_allocation_domain(int cpu, struct cpumas=
k *retmask,
> >> +				const struct cpumask *mask)
> >> +{
> >> +	cpumask_copy(retmask, cpu_online_mask); }
> >> +
> >> +int hv_allocate_stimer0_irq(int *irq, int *vector) {
> >> +	int		localirq;
> >> +	int		result;
> >> +	struct irq_data *irq_data;
> >> +
> >> +	/* The normal APIC vector allocation domain allows allocation of vec=
tors
> >> +	 * only for the calling CPU.  So we change the allocation domain to =
one
> >> +	 * that allows vectors to be allocated in all online CPUs.  This
> >> +	 * change is fine in a Hyper-V VM because VMs don't have the usual
> >> +	 * complement of interrupting devices.
> >> +	 */
> >> +	apic->vector_allocation_domain =3D allonline_vector_allocation_domai=
n;
> >> +	localirq =3D acpi_register_gsi(NULL, HV_STIMER0_IRQNR,
> >> +				ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH);
> >> +	if (localirq < 0) {
> >> +		pr_err("Cannot register stimer0 gsi. Error %d", localirq);
> >> +		return -1;
> >> +	}
> >> +
> >> +	/* We pass in a dummy IRQ handler because architecture independent c=
ode
> >> +	 * will later override the IRQ domain interrupt handler and set it t=
o a
> >> +	 * Hyper-V specific handler.
> >> +	 */
> >> +	result =3D request_irq(localirq, (irq_handler_t)(-1), 0,
> >> +					"Hyper-V stimer0", NULL);
> >
> > I remember K. Y. told me he has patches in his stash to implement
> > Hyper-V-specific APIC. Is this still in works or there is a reason to
> > not do it? Just curious.
> >

I've looked at KY's pending patch.  It's only a partial APIC implementation=
 that overrides certain performance sensitive functions and doesn't really =
help what's needed here.   v2 of the patch will take a different approach t=
hat makes this moot.

> >> +	if (result) {
> >> +		pr_err("Cannot request stimer0 irq. Error %d", result);
> >> +		acpi_unregister_gsi(localirq);
> >> +		return -1;
> >> +	}
> >> +	irq_data =3D irq_domain_get_irq_data(x86_vector_domain, localirq);
> >
> > In theory irq_domain_get_irq_data() can return NULL.
> >

Per separate comments from Thomas Gleixner, shouldn't be doing this anyway.=
  Will take a different approach in v2 of the patch.=20

> >> +	*vector =3D irqd_cfg(irq_data)->vector;
> >> +	*irq =3D localirq;
> >> +	return 0;
> >> +}
> >> +
> >> +void hv_deallocate_stimer0_irq(int irq) {
> >> +	free_irq(irq, NULL);
> >> +	acpi_unregister_gsi(irq);
> >> +}
> >> +
> >> +
> >>  void hv_setup_kexec_handler(void (*handler)(void))  {
> >>  	hv_kexec_handler =3D handler;
> >> @@ -195,7 +257,7 @@ static void __init ms_hyperv_init_platform(void)
> >>  		hv_host_info_ecx =3D cpuid_ecx(HVCPUID_VERSION);
> >>  		hv_host_info_edx =3D cpuid_edx(HVCPUID_VERSION);
> >>
> >> -		pr_info("Hyper-V Host Build:%d-%d.%d-%d-%d.%d\n",
> >> +		pr_info("Hyper-V: Host Build %d-%d.%d-%d-%d.%d\n",
> >
> > While this definitely makes logs nicer uhe change is probably not
> > needed in this particular patch.

True.  Will move this to a separate patch.

> >
> >>  			hv_host_info_eax, hv_host_info_ebx >> 16,
> >>  			hv_host_info_ebx & 0xFFFF, hv_host_info_ecx,
> >>  			hv_host_info_edx >> 24, hv_host_info_edx & 0xFFFFFF); diff --git
> >> a/drivers/hv/hv.c b/drivers/hv/hv.c index fe96aab..68ac474 100644
> >> --- a/drivers/hv/hv.c
> >> +++ b/drivers/hv/hv.c
> >> @@ -27,8 +27,12 @@
> >>  #include <linux/vmalloc.h>
> >>  #include <linux/hyperv.h>
> >>  #include <linux/version.h>
> >> -#include <linux/interrupt.h>
> >> +#include <linux/irq.h>
> >> +#include <linux/irqdesc.h>
> >> +#include <linux/random.h>
> >> +#include <linux/kernel_stat.h>
> >>  #include <linux/clockchips.h>
> >> +#include <linux/module.h>
> >>  #include <asm/hyperv.h>
> >>  #include <asm/mshyperv.h>
> >>  #include "hyperv_vmbus.h"
> >> @@ -38,6 +42,21 @@ struct hv_context hv_context =3D {
> >>  	.synic_initialized	=3D false,
> >>  };
> >>
> >> +/* If true, we're using Direct Mode for stimer0, and the timer will
> >> +do it own
> >> + * interrupt when it expires. If false, stimer0 is not using Direct
> >> +Mode and
> >> + * will send a VMbus message when it expires. We prefer to use
> >> +Direct Mode,
> >> + * but not all versions of Hyper-V support Direct Mode.
> >> + *
> >> + * While Hyper-V provides a total of four stimer's per CPU, Linux
> >> +use only
> >> + * stimer0.
> >> + */
> >> +static bool	stimer_direct_mode;
> >> +static int	stimer0_irq;
> >> +static int	stimer0_vector;
> >> +static bool	direct_mode_disable;
> >> +module_param(direct_mode_disable, bool, 0444);
> >> +MODULE_PARM_DESC(direct_mode_disable, "Set to Y to disable Direct
> >> +Mode.");
> >> +
> >>  #define HV_TIMER_FREQUENCY (10 * 1000 * 1000) /* 100ns period */
> >> #define HV_MAX_MAX_DELTA_TICKS 0xffffffff  #define HV_MIN_DELTA_TICKS
> >> 1 @@ -52,7 +71,12 @@ int hv_init(void)
> >>  	hv_context.cpu_context =3D alloc_percpu(struct hv_per_cpu_context);
> >>  	if (!hv_context.cpu_context)
> >>  		return -ENOMEM;
> >> +	stimer_direct_mode =3D (ms_hyperv.misc_features &
> >> +		HV_X64_STIMER_DIRECT_MODE_AVAILABLE) ? true : false;
> >>
> >> +	/* Apply boot command line override to the Direct Mode setting */
> >> +	if (direct_mode_disable)
> >> +		stimer_direct_mode =3D false;
> >
> > Having both direct_mode_disable and stimer_direct_mode seems redundant.
> >
> > we may write something like
> >
> >      if (!direct_mode_disable)
> >          direct_mode_disable =3D (ms_hyperv.misc_features &
> > HV_X64_STIMER_DIRECT_MODE_AVAILABLE) ? false : true;
> >
> > and use it everywhere.

Got it.  Makes sense.

> >
> >>  	return 0;
> >>  }
> >>
> >> @@ -91,6 +115,23 @@ int hv_post_message(union hv_connection_id connect=
ion_id,
> >>  	return status & 0xFFFF;
> >>  }
> >>
> >> +/* ISR for when stimer0 is operating in Direct Mode.  Direct Mode
> >> +does
> >> + * not use VMBus or any VMBus messages, so process here and not in
> >> +the
> >> + * VMBus driver code.
> >> + */
> >> +
> >> +static void hv_stimer0_isr(struct irq_desc *desc) {
> >> +	struct hv_per_cpu_context *hv_cpu;
> >> +
> >> +	__this_cpu_inc(*desc->kstat_irqs);
> >> +	__this_cpu_inc(kstat.irqs_sum);
> >> +	hv_ack_stimer0_interrupt(desc);
> >> +	hv_cpu =3D this_cpu_ptr(hv_context.cpu_context);
> >> +	hv_cpu->clk_evt->event_handler(hv_cpu->clk_evt);
> >> +	add_interrupt_randomness(desc->irq_data.irq, 0);
> >
> > AFAIU implementing Hyper-V specific IRQ chip would allow us to drop
> > this as we would do this on 'normal' Linux IRQ path.
> >
> >> +}
> >> +
> >>  static int hv_ce_set_next_event(unsigned long delta,
> >>  				struct clock_event_device *evt)
> >>  {
> >> @@ -108,6 +149,8 @@ static int hv_ce_shutdown(struct
> >> clock_event_device *evt)  {
> >>  	hv_init_timer(HV_X64_MSR_STIMER0_COUNT, 0);
> >>  	hv_init_timer_config(HV_X64_MSR_STIMER0_CONFIG, 0);
> >> +	if (stimer_direct_mode)
> >> +		hv_disable_stimer0_percpu_irq(stimer0_irq);
> >>
> >
> > Both hv_disable_stimer0_percpu_irq() and
> > hv_enable_stimer0_percpu_irq() are no-ops, right? Why don't we
> > mask/unmask the IRQ? I may have missed something...

Yes, these functions are no-ops on x86 as there's really no need to mask/un=
mask.  But we have code in the works to enable Linux guests on Hyper-V on A=
RM64 hardware.  On the ARM64 side, stimer Direct Mode fits nicely with perc=
pu IRQs, and these functions are needed to call enable_percpu_irq() and dis=
able_percpu_irq().

> >
> >>  	return 0;
> >>  }
> >> @@ -116,9 +159,24 @@ static int hv_ce_set_oneshot(struct
> >> clock_event_device *evt)  {
> >>  	union hv_timer_config timer_cfg;
> >>
> >> +	timer_cfg.as_uint64 =3D 0; /* Zero everything */
> >>  	timer_cfg.enable =3D 1;
> >>  	timer_cfg.auto_enable =3D 1;
> >> -	timer_cfg.sintx =3D VMBUS_MESSAGE_SINT;
> >> +	if (stimer_direct_mode) {
> >> +
> >> +		/* When it expires, the timer will directly interrupt
> >> +		 * on the specific hardware vector.
> >> +		 */
> >> +		timer_cfg.direct_mode =3D 1;
> >> +		timer_cfg.apic_vector =3D stimer0_vector;
> >> +		hv_enable_stimer0_percpu_irq(stimer0_irq);
> >> +	} else {
> >> +		/* When it expires, the timer will generate a VMbus message,
> >> +		 * to be handled by the normal VMbus interrupt handler.
> >> +		 */
> >> +		timer_cfg.direct_mode =3D 0;
> >> +		timer_cfg.sintx =3D VMBUS_MESSAGE_SINT;
> >> +	}
> >>  	hv_init_timer_config(HV_X64_MSR_STIMER0_CONFIG,
> >> timer_cfg.as_uint64);
> >>
> >>  	return 0;
> >> @@ -191,6 +249,12 @@ int hv_synic_alloc(void)
> >>  		INIT_LIST_HEAD(&hv_cpu->chan_list);
> >>  	}
> >>
> >> +	if (stimer_direct_mode) {
> >> +		if (hv_allocate_stimer0_irq(&stimer0_irq, &stimer0_vector))
> >> +			goto err;
> >> +		irq_set_handler(stimer0_irq, hv_stimer0_isr);
> >> +	}
> >> +
> >>  	return 0;
> >>  err:
> >>  	return -ENOMEM;
> >> @@ -292,6 +356,9 @@ void hv_synic_clockevents_cleanup(void)
> >>  	if (!(ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE))
> >>  		return;
> >>
> >> +	if (stimer_direct_mode)
> >> +		hv_deallocate_stimer0_irq(stimer0_irq);
> >> +
> >>  	for_each_present_cpu(cpu) {
> >>  		struct hv_per_cpu_context *hv_cpu
> >>  			=3D per_cpu_ptr(hv_context.cpu_context, cpu); diff --git
> >> a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index
> >> de6f01d..ee8c89b 100644
> >> --- a/drivers/hv/hyperv_vmbus.h
> >> +++ b/drivers/hv/hyperv_vmbus.h
> >> @@ -55,7 +55,9 @@
> >>  		u64 periodic:1;
> >>  		u64 lazy:1;
> >>  		u64 auto_enable:1;
> >> -		u64 reserved_z0:12;
> >> +		u64 apic_vector:8;
> >> +		u64 direct_mode:1;
> >> +		u64 reserved_z0:3;
> >>  		u64 sintx:4;
> >>  		u64 reserved_z1:44;
> >>  	};
>=20
> --
>   Vitaly

From 1583087426381268187@xxx Fri Nov 03 22:56:59 +0000 2017
X-GM-THRID: 1582813368435034245
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread