[PATCH 0/3] Drivers: hv: vmbus: Support PCI Express pass-through driver

[PATCH 0/3] Drivers: hv: vmbus: Support PCI Express pass-through driver

[K. Y. Srinivasan]

The patch set for supporting PCI Express pass through on Hyper-V has been posted and
is currently under review. These vmbus patches are needed for the PCI
Express pass through driver. Since these need to go through Greg's tree,
I am posting these separately  here.

Jake Oshins (3):
  drivers:hv: Export a function that maps Linux CPU num onto Hyper-V
    proc num
  drivers:hv: Export the API to invoke a hypercall on Hyper-V
  drivers:hv: Define the channel type for Hyper-V PCI Express
    pass-through

 drivers/hv/hv.c        |    9 +++++----
 drivers/hv/vmbus_drv.c |   17 +++++++++++++++++
 include/linux/hyperv.h |   14 ++++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)

-- 
1.7.4.1

[PATCH 1/3] drivers:hv: Export a function that maps Linux CPU num onto Hyper-V proc num

[K. Y. Srinivasan]

From: Jake Oshins <jakeo@microsoft.com>

This patch exposes the mapping between Linux CPU number and Hyper-V virtual
processor number.  This is necessary because the hypervisor needs to know which
virtual processors to target when making a mapping in the Interrupt Redirection
Table in the I/O MMU.

Signed-off-by: Jake Oshins <jakeo@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/vmbus_drv.c |   17 +++++++++++++++++
 include/linux/hyperv.h |    2 ++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 0a344fa..1a45302 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1191,6 +1191,23 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 }
 EXPORT_SYMBOL_GPL(vmbus_allocate_mmio);
 
+/**
+ * vmbus_cpu_number_to_vp_number() - Map CPU to VP.
+ * @cpu_number: CPU number in Linux terms
+ *
+ * This function returns the mapping between the Linux processor
+ * number and the hypervisor's virtual processor number, useful
+ * in making hypercalls and such that talk about specific
+ * processors.
+ *
+ * Return: Virtual processor number in Hyper-V terms
+ */
+int vmbus_cpu_number_to_vp_number(int cpu_number)
+{
+	return hv_context.vp_index[cpu_number];
+}
+EXPORT_SYMBOL_GPL(vmbus_cpu_number_to_vp_number);
+
 static int vmbus_acpi_add(struct acpi_device *device)
 {
 	acpi_status result;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 54733d5..02393b6 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -982,6 +982,8 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 			resource_size_t size, resource_size_t align,
 			bool fb_overlap_ok);
 
+int vmbus_cpu_number_to_vp_number(int cpu_number);
+
 /**
  * VMBUS_DEVICE - macro used to describe a specific hyperv vmbus device
  *
-- 
1.7.4.1

[PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on Hyper-V

[K. Y. Srinivasan]

From: Jake Oshins <jakeo@microsoft.com>

This patch exposes the function that hv_vmbus.ko uses to make hypercalls.  This
is necessary for retargeting an interrupt when it is given a new affinity.

Since we are exporting this API, rename the API as it will be visible outside
the hv.c file.

Signed-off-by: Jake Oshins <jakeo@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv.c        |    9 +++++----
 include/linux/hyperv.h |    1 +
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 6341be8..a7b6c6a 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -89,9 +89,9 @@ static int query_hypervisor_info(void)
 }
 
 /*
- * do_hypercall- Invoke the specified hypercall
+ * hv_do_hypercall- Invoke the specified hypercall
  */
-static u64 do_hypercall(u64 control, void *input, void *output)
+u64 hv_do_hypercall(u64 control, void *input, void *output)
 {
 	u64 input_address = (input) ? virt_to_phys(input) : 0;
 	u64 output_address = (output) ? virt_to_phys(output) : 0;
@@ -132,6 +132,7 @@ static u64 do_hypercall(u64 control, void *input, void *output)
 	return hv_status_lo | ((u64)hv_status_hi << 32);
 #endif /* !x86_64 */
 }
+EXPORT_SYMBOL_GPL(hv_do_hypercall);
 
 #ifdef CONFIG_X86_64
 static cycle_t read_hv_clock_tsc(struct clocksource *arg)
@@ -329,7 +330,7 @@ int hv_post_message(union hv_connection_id connection_id,
 	aligned_msg->payload_size = payload_size;
 	memcpy((void *)aligned_msg->payload, payload, payload_size);
 
-	status = do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL)
+	status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL)
 		& 0xFFFF;
 
 	put_cpu();
@@ -347,7 +348,7 @@ u16 hv_signal_event(void *con_id)
 {
 	u16 status;
 
-	status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) & 0xFFFF);
+	status = (hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) & 0xFFFF);
 
 	return status;
 }
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 02393b6..ea0a0e3 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -983,6 +983,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 			bool fb_overlap_ok);
 
 int vmbus_cpu_number_to_vp_number(int cpu_number);
+u64 hv_do_hypercall(u64 control, void *input, void *output);
 
 /**
  * VMBUS_DEVICE - macro used to describe a specific hyperv vmbus device
-- 
1.7.4.1

[PATCH 3/3] drivers:hv: Define the channel type for Hyper-V PCI Express pass-through

[K. Y. Srinivasan]

From: Jake Oshins <jakeo@microsoft.com>

This defines the channel type for PCI front-ends in Hyper-V VMs.

Signed-off-by: Jake Oshins <jakeo@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 include/linux/hyperv.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index ea0a0e3..5587899 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1140,6 +1140,17 @@ u64 hv_do_hypercall(u64 control, void *input, void *output);
 		}
 
 /*
+ * PCI Express Pass Through
+ * {44C4F61D-4444-4400-9D52-802E27EDE19F}
+ */
+
+#define HV_PCIE_GUID \
+	.guid = { \
+			0x1D, 0xF6, 0xC4, 0x44, 0x44, 0x44, 0x00, 0x44, \
+			0x9D, 0x52, 0x80, 0x2E, 0x27, 0xED, 0xE1, 0x9F \
+		}
+
+/*
  * Common header for Hyper-V ICs
  */
 
-- 
1.7.4.1

Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on Hyper-V

[Greg KH]

On Tue, Sep 15, 2015 at 06:26:48PM -0700, K. Y. Srinivasan wrote:
> From: Jake Oshins <jakeo@microsoft.com>
> 
> This patch exposes the function that hv_vmbus.ko uses to make hypercalls.  This
> is necessary for retargeting an interrupt when it is given a new affinity.
> 
> Since we are exporting this API, rename the API as it will be visible outside
> the hv.c file.
> 
> Signed-off-by: Jake Oshins <jakeo@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/hv.c        |    9 +++++----
>  include/linux/hyperv.h |    1 +
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 6341be8..a7b6c6a 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -89,9 +89,9 @@ static int query_hypervisor_info(void)
>  }
>  
>  /*
> - * do_hypercall- Invoke the specified hypercall
> + * hv_do_hypercall- Invoke the specified hypercall
>   */
> -static u64 do_hypercall(u64 control, void *input, void *output)
> +u64 hv_do_hypercall(u64 control, void *input, void *output)
>  {
>  	u64 input_address = (input) ? virt_to_phys(input) : 0;
>  	u64 output_address = (output) ? virt_to_phys(output) : 0;
> @@ -132,6 +132,7 @@ static u64 do_hypercall(u64 control, void *input, void *output)
>  	return hv_status_lo | ((u64)hv_status_hi << 32);
>  #endif /* !x86_64 */
>  }
> +EXPORT_SYMBOL_GPL(hv_do_hypercall);
>  
>  #ifdef CONFIG_X86_64
>  static cycle_t read_hv_clock_tsc(struct clocksource *arg)
> @@ -329,7 +330,7 @@ int hv_post_message(union hv_connection_id connection_id,
>  	aligned_msg->payload_size = payload_size;
>  	memcpy((void *)aligned_msg->payload, payload, payload_size);
>  
> -	status = do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL)
> +	status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL)
>  		& 0xFFFF;
>  
>  	put_cpu();
> @@ -347,7 +348,7 @@ u16 hv_signal_event(void *con_id)
>  {
>  	u16 status;
>  
> -	status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) & 0xFFFF);
> +	status = (hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) & 0xFFFF);

What's with the crazy () around a function call?

And why are you passing a 64bit return value into a 16bit status value?
That seems like something is broken.

Re: [PATCH 3/3] drivers:hv: Define the channel type for Hyper-V PCI Express pass-through

[Greg KH]

On Tue, Sep 15, 2015 at 06:26:49PM -0700, K. Y. Srinivasan wrote:
> From: Jake Oshins <jakeo@microsoft.com>
> 
> This defines the channel type for PCI front-ends in Hyper-V VMs.
> 
> Signed-off-by: Jake Oshins <jakeo@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  include/linux/hyperv.h |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index ea0a0e3..5587899 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1140,6 +1140,17 @@ u64 hv_do_hypercall(u64 control, void *input, void *output);
>  		}
>  
>  /*
> + * PCI Express Pass Through
> + * {44C4F61D-4444-4400-9D52-802E27EDE19F}
> + */
> +
> +#define HV_PCIE_GUID \
> +	.guid = { \
> +			0x1D, 0xF6, 0xC4, 0x44, 0x44, 0x44, 0x00, 0x44, \
> +			0x9D, 0x52, 0x80, 0x2E, 0x27, 0xED, 0xE1, 0x9F \
> +		}
> +
> +/*
>   * Common header for Hyper-V ICs
>   */

Yet you do nothing with this, so why add it to the code at this point in
time?  I can't take pointless things like this...

RE: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on Hyper-V

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Sunday, September 20, 2015 10:28 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> Subject: Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on
> Hyper-V
> 
> On Tue, Sep 15, 2015 at 06:26:48PM -0700, K. Y. Srinivasan wrote:
> > From: Jake Oshins <jakeo@microsoft.com>
> >
> > This patch exposes the function that hv_vmbus.ko uses to make hypercalls.
> This
> > is necessary for retargeting an interrupt when it is given a new affinity.
> >
> > Since we are exporting this API, rename the API as it will be visible outside
> > the hv.c file.
> >
> > Signed-off-by: Jake Oshins <jakeo@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  drivers/hv/hv.c        |    9 +++++----
> >  include/linux/hyperv.h |    1 +
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > index 6341be8..a7b6c6a 100644
> > --- a/drivers/hv/hv.c
> > +++ b/drivers/hv/hv.c
> > @@ -89,9 +89,9 @@ static int query_hypervisor_info(void)
> >  }
> >
> >  /*
> > - * do_hypercall- Invoke the specified hypercall
> > + * hv_do_hypercall- Invoke the specified hypercall
> >   */
> > -static u64 do_hypercall(u64 control, void *input, void *output)
> > +u64 hv_do_hypercall(u64 control, void *input, void *output)
> >  {
> >  	u64 input_address = (input) ? virt_to_phys(input) : 0;
> >  	u64 output_address = (output) ? virt_to_phys(output) : 0;
> > @@ -132,6 +132,7 @@ static u64 do_hypercall(u64 control, void *input,
> void *output)
> >  	return hv_status_lo | ((u64)hv_status_hi << 32);
> >  #endif /* !x86_64 */
> >  }
> > +EXPORT_SYMBOL_GPL(hv_do_hypercall);
> >
> >  #ifdef CONFIG_X86_64
> >  static cycle_t read_hv_clock_tsc(struct clocksource *arg)
> > @@ -329,7 +330,7 @@ int hv_post_message(union hv_connection_id
> connection_id,
> >  	aligned_msg->payload_size = payload_size;
> >  	memcpy((void *)aligned_msg->payload, payload, payload_size);
> >
> > -	status = do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL)
> > +	status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg,
> NULL)
> >  		& 0xFFFF;
> >
> >  	put_cpu();
> > @@ -347,7 +348,7 @@ u16 hv_signal_event(void *con_id)
> >  {
> >  	u16 status;
> >
> > -	status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) &
> 0xFFFF);
> > +	status = (hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) &
> 0xFFFF);
> 
> What's with the crazy () around a function call?

I will address this and resend.
> 
> And why are you passing a 64bit return value into a 16bit status value?
> That seems like something is broken.

The low order 16 bits have the valid status codes we are interested in.

K. Y

RE: [PATCH 3/3] drivers:hv: Define the channel type for Hyper-V PCI Express pass-through

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Sunday, September 20, 2015 10:29 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> Subject: Re: [PATCH 3/3] drivers:hv: Define the channel type for Hyper-V PCI
> Express pass-through
> 
> On Tue, Sep 15, 2015 at 06:26:49PM -0700, K. Y. Srinivasan wrote:
> > From: Jake Oshins <jakeo@microsoft.com>
> >
> > This defines the channel type for PCI front-ends in Hyper-V VMs.
> >
> > Signed-off-by: Jake Oshins <jakeo@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  include/linux/hyperv.h |   11 +++++++++++
> >  1 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> > index ea0a0e3..5587899 100644
> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -1140,6 +1140,17 @@ u64 hv_do_hypercall(u64 control, void *input,
> void *output);
> >  		}
> >
> >  /*
> > + * PCI Express Pass Through
> > + * {44C4F61D-4444-4400-9D52-802E27EDE19F}
> > + */
> > +
> > +#define HV_PCIE_GUID \
> > +	.guid = { \
> > +			0x1D, 0xF6, 0xC4, 0x44, 0x44, 0x44, 0x00, 0x44, \
> > +			0x9D, 0x52, 0x80, 0x2E, 0x27, 0xED, 0xE1, 0x9F \
> > +		}
> > +
> > +/*
> >   * Common header for Hyper-V ICs
> >   */
> 
> Yet you do nothing with this, so why add it to the code at this point in
> time?  I can't take pointless things like this...

Greg,

This is a valid GUID that the host can offer us today. The driver to handle this is currently under review (Jake has
posted the code). Traditionally, we have kept our header files in synch with the host functionality.

Regards,

K. Y

Re: [PATCH 3/3] drivers:hv: Define the channel type for Hyper-V PCI Express pass-through

[Greg KH]

On Mon, Sep 21, 2015 at 04:27:28PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Sunday, September 20, 2015 10:29 PM
> > To: KY Srinivasan <kys@microsoft.com>
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> > Subject: Re: [PATCH 3/3] drivers:hv: Define the channel type for Hyper-V PCI
> > Express pass-through
> > 
> > On Tue, Sep 15, 2015 at 06:26:49PM -0700, K. Y. Srinivasan wrote:
> > > From: Jake Oshins <jakeo@microsoft.com>
> > >
> > > This defines the channel type for PCI front-ends in Hyper-V VMs.
> > >
> > > Signed-off-by: Jake Oshins <jakeo@microsoft.com>
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > ---
> > >  include/linux/hyperv.h |   11 +++++++++++
> > >  1 files changed, 11 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> > > index ea0a0e3..5587899 100644
> > > --- a/include/linux/hyperv.h
> > > +++ b/include/linux/hyperv.h
> > > @@ -1140,6 +1140,17 @@ u64 hv_do_hypercall(u64 control, void *input,
> > void *output);
> > >  		}
> > >
> > >  /*
> > > + * PCI Express Pass Through
> > > + * {44C4F61D-4444-4400-9D52-802E27EDE19F}
> > > + */
> > > +
> > > +#define HV_PCIE_GUID \
> > > +	.guid = { \
> > > +			0x1D, 0xF6, 0xC4, 0x44, 0x44, 0x44, 0x00, 0x44, \
> > > +			0x9D, 0x52, 0x80, 0x2E, 0x27, 0xED, 0xE1, 0x9F \
> > > +		}
> > > +
> > > +/*
> > >   * Common header for Hyper-V ICs
> > >   */
> > 
> > Yet you do nothing with this, so why add it to the code at this point in
> > time?  I can't take pointless things like this...
> 
> Greg,
> 
> This is a valid GUID that the host can offer us today. The driver to handle this is currently under review (Jake has
> posted the code). Traditionally, we have kept our header files in synch with the host functionality.

But please don't add things that are not used, that's pretty pointless.

Also wrap your email lines properly :)

thanks,

greg k-h

Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on Hyper-V

[Greg KH]

On Mon, Sep 21, 2015 at 04:22:01PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Sunday, September 20, 2015 10:28 PM
> > To: KY Srinivasan <kys@microsoft.com>
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> > Subject: Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on
> > Hyper-V
> > 
> > On Tue, Sep 15, 2015 at 06:26:48PM -0700, K. Y. Srinivasan wrote:
> > > From: Jake Oshins <jakeo@microsoft.com>
> > >
> > > This patch exposes the function that hv_vmbus.ko uses to make hypercalls.
> > This
> > > is necessary for retargeting an interrupt when it is given a new affinity.
> > >
> > > Since we are exporting this API, rename the API as it will be visible outside
> > > the hv.c file.
> > >
> > > Signed-off-by: Jake Oshins <jakeo@microsoft.com>
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > ---
> > >  drivers/hv/hv.c        |    9 +++++----
> > >  include/linux/hyperv.h |    1 +
> > >  2 files changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > > index 6341be8..a7b6c6a 100644
> > > --- a/drivers/hv/hv.c
> > > +++ b/drivers/hv/hv.c
> > > @@ -89,9 +89,9 @@ static int query_hypervisor_info(void)
> > >  }
> > >
> > >  /*
> > > - * do_hypercall- Invoke the specified hypercall
> > > + * hv_do_hypercall- Invoke the specified hypercall
> > >   */
> > > -static u64 do_hypercall(u64 control, void *input, void *output)
> > > +u64 hv_do_hypercall(u64 control, void *input, void *output)
> > >  {
> > >  	u64 input_address = (input) ? virt_to_phys(input) : 0;
> > >  	u64 output_address = (output) ? virt_to_phys(output) : 0;
> > > @@ -132,6 +132,7 @@ static u64 do_hypercall(u64 control, void *input,
> > void *output)
> > >  	return hv_status_lo | ((u64)hv_status_hi << 32);
> > >  #endif /* !x86_64 */
> > >  }
> > > +EXPORT_SYMBOL_GPL(hv_do_hypercall);
> > >
> > >  #ifdef CONFIG_X86_64
> > >  static cycle_t read_hv_clock_tsc(struct clocksource *arg)
> > > @@ -329,7 +330,7 @@ int hv_post_message(union hv_connection_id
> > connection_id,
> > >  	aligned_msg->payload_size = payload_size;
> > >  	memcpy((void *)aligned_msg->payload, payload, payload_size);
> > >
> > > -	status = do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL)
> > > +	status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg,
> > NULL)
> > >  		& 0xFFFF;
> > >
> > >  	put_cpu();
> > > @@ -347,7 +348,7 @@ u16 hv_signal_event(void *con_id)
> > >  {
> > >  	u16 status;
> > >
> > > -	status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) &
> > 0xFFFF);
> > > +	status = (hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) &
> > 0xFFFF);
> > 
> > What's with the crazy () around a function call?
> 
> I will address this and resend.
> > 
> > And why are you passing a 64bit return value into a 16bit status value?
> > That seems like something is broken.
> 
> The low order 16 bits have the valid status codes we are interested in.

Then why even return a 64bit number if no one uses it?

greg k-h

RE: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on Hyper-V

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 21, 2015 9:41 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> Subject: Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on
> Hyper-V
> 
> On Mon, Sep 21, 2015 at 04:22:01PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Sunday, September 20, 2015 10:28 PM
> > > To: KY Srinivasan <kys@microsoft.com>
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > > jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> > > Subject: Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall
> on
> > > Hyper-V
> > >
> > > On Tue, Sep 15, 2015 at 06:26:48PM -0700, K. Y. Srinivasan wrote:
> > > > From: Jake Oshins <jakeo@microsoft.com>
> > > >
> > > > This patch exposes the function that hv_vmbus.ko uses to make
> hypercalls.
> > > This
> > > > is necessary for retargeting an interrupt when it is given a new affinity.
> > > >
> > > > Since we are exporting this API, rename the API as it will be visible
> outside
> > > > the hv.c file.
> > > >
> > > > Signed-off-by: Jake Oshins <jakeo@microsoft.com>
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > ---
> > > >  drivers/hv/hv.c        |    9 +++++----
> > > >  include/linux/hyperv.h |    1 +
> > > >  2 files changed, 6 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > > > index 6341be8..a7b6c6a 100644
> > > > --- a/drivers/hv/hv.c
> > > > +++ b/drivers/hv/hv.c
> > > > @@ -89,9 +89,9 @@ static int query_hypervisor_info(void)
> > > >  }
> > > >
> > > >  /*
> > > > - * do_hypercall- Invoke the specified hypercall
> > > > + * hv_do_hypercall- Invoke the specified hypercall
> > > >   */
> > > > -static u64 do_hypercall(u64 control, void *input, void *output)
> > > > +u64 hv_do_hypercall(u64 control, void *input, void *output)
> > > >  {
> > > >  	u64 input_address = (input) ? virt_to_phys(input) : 0;
> > > >  	u64 output_address = (output) ? virt_to_phys(output) : 0;
> > > > @@ -132,6 +132,7 @@ static u64 do_hypercall(u64 control, void *input,
> > > void *output)
> > > >  	return hv_status_lo | ((u64)hv_status_hi << 32);
> > > >  #endif /* !x86_64 */
> > > >  }
> > > > +EXPORT_SYMBOL_GPL(hv_do_hypercall);
> > > >
> > > >  #ifdef CONFIG_X86_64
> > > >  static cycle_t read_hv_clock_tsc(struct clocksource *arg)
> > > > @@ -329,7 +330,7 @@ int hv_post_message(union hv_connection_id
> > > connection_id,
> > > >  	aligned_msg->payload_size = payload_size;
> > > >  	memcpy((void *)aligned_msg->payload, payload, payload_size);
> > > >
> > > > -	status = do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL)
> > > > +	status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg,
> > > NULL)
> > > >  		& 0xFFFF;
> > > >
> > > >  	put_cpu();
> > > > @@ -347,7 +348,7 @@ u16 hv_signal_event(void *con_id)
> > > >  {
> > > >  	u16 status;
> > > >
> > > > -	status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) &
> > > 0xFFFF);
> > > > +	status = (hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) &
> > > 0xFFFF);
> > >
> > > What's with the crazy () around a function call?
> >
> > I will address this and resend.
> > >
> > > And why are you passing a 64bit return value into a 16bit status value?
> > > That seems like something is broken.
> >
> > The low order 16 bits have the valid status codes we are interested in.
> 
> Then why even return a 64bit number if no one uses it?

The hypervisor ABI specifies a 64 bit return value.

K. Y


> 
> greg k-h

Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on Hyper-V

[Greg KH]

On Mon, Sep 21, 2015 at 05:36:06PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, September 21, 2015 9:41 AM
> > To: KY Srinivasan <kys@microsoft.com>
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> > Subject: Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on
> > Hyper-V
> > 
> > On Mon, Sep 21, 2015 at 04:22:01PM +0000, KY Srinivasan wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Sunday, September 20, 2015 10:28 PM
> > > > To: KY Srinivasan <kys@microsoft.com>
> > > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > > > jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> > > > Subject: Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall
> > on
> > > > Hyper-V
> > > >
> > > > On Tue, Sep 15, 2015 at 06:26:48PM -0700, K. Y. Srinivasan wrote:
> > > > > From: Jake Oshins <jakeo@microsoft.com>
> > > > >
> > > > > This patch exposes the function that hv_vmbus.ko uses to make
> > hypercalls.
> > > > This
> > > > > is necessary for retargeting an interrupt when it is given a new affinity.
> > > > >
> > > > > Since we are exporting this API, rename the API as it will be visible
> > outside
> > > > > the hv.c file.
> > > > >
> > > > > Signed-off-by: Jake Oshins <jakeo@microsoft.com>
> > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > > ---
> > > > >  drivers/hv/hv.c        |    9 +++++----
> > > > >  include/linux/hyperv.h |    1 +
> > > > >  2 files changed, 6 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > > > > index 6341be8..a7b6c6a 100644
> > > > > --- a/drivers/hv/hv.c
> > > > > +++ b/drivers/hv/hv.c
> > > > > @@ -89,9 +89,9 @@ static int query_hypervisor_info(void)
> > > > >  }
> > > > >
> > > > >  /*
> > > > > - * do_hypercall- Invoke the specified hypercall
> > > > > + * hv_do_hypercall- Invoke the specified hypercall
> > > > >   */
> > > > > -static u64 do_hypercall(u64 control, void *input, void *output)
> > > > > +u64 hv_do_hypercall(u64 control, void *input, void *output)
> > > > >  {
> > > > >  	u64 input_address = (input) ? virt_to_phys(input) : 0;
> > > > >  	u64 output_address = (output) ? virt_to_phys(output) : 0;
> > > > > @@ -132,6 +132,7 @@ static u64 do_hypercall(u64 control, void *input,
> > > > void *output)
> > > > >  	return hv_status_lo | ((u64)hv_status_hi << 32);
> > > > >  #endif /* !x86_64 */
> > > > >  }
> > > > > +EXPORT_SYMBOL_GPL(hv_do_hypercall);
> > > > >
> > > > >  #ifdef CONFIG_X86_64
> > > > >  static cycle_t read_hv_clock_tsc(struct clocksource *arg)
> > > > > @@ -329,7 +330,7 @@ int hv_post_message(union hv_connection_id
> > > > connection_id,
> > > > >  	aligned_msg->payload_size = payload_size;
> > > > >  	memcpy((void *)aligned_msg->payload, payload, payload_size);
> > > > >
> > > > > -	status = do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL)
> > > > > +	status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg,
> > > > NULL)
> > > > >  		& 0xFFFF;
> > > > >
> > > > >  	put_cpu();
> > > > > @@ -347,7 +348,7 @@ u16 hv_signal_event(void *con_id)
> > > > >  {
> > > > >  	u16 status;
> > > > >
> > > > > -	status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) &
> > > > 0xFFFF);
> > > > > +	status = (hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) &
> > > > 0xFFFF);
> > > >
> > > > What's with the crazy () around a function call?
> > >
> > > I will address this and resend.
> > > >
> > > > And why are you passing a 64bit return value into a 16bit status value?
> > > > That seems like something is broken.
> > >
> > > The low order 16 bits have the valid status codes we are interested in.
> > 
> > Then why even return a 64bit number if no one uses it?
> 
> The hypervisor ABI specifies a 64 bit return value.

Then just fricken return a 32bit value, don't force everyone who calls
it to do themselves...

greg k-h

RE: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on Hyper-V

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, September 21, 2015 1:45 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: olaf@aepfle.de; jasowang@redhat.com; linux-kernel@vger.kernel.org;
> Jake Oshins <jakeo@microsoft.com>; apw@canonical.com;
> devel@linuxdriverproject.org
> Subject: Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall on
> Hyper-V
> 
> On Mon, Sep 21, 2015 at 05:36:06PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, September 21, 2015 9:41 AM
> > > To: KY Srinivasan <kys@microsoft.com>
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > > jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> > > Subject: Re: [PATCH 2/3] drivers:hv: Export the API to invoke a hypercall
> on
> > > Hyper-V
> > >
> > > On Mon, Sep 21, 2015 at 04:22:01PM +0000, KY Srinivasan wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Sunday, September 20, 2015 10:28 PM
> > > > > To: KY Srinivasan <kys@microsoft.com>
> > > > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > > > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > > > > jasowang@redhat.com; Jake Oshins <jakeo@microsoft.com>
> > > > > Subject: Re: [PATCH 2/3] drivers:hv: Export the API to invoke a
> hypercall
> > > on
> > > > > Hyper-V
> > > > >
> > > > > On Tue, Sep 15, 2015 at 06:26:48PM -0700, K. Y. Srinivasan wrote:
> > > > > > From: Jake Oshins <jakeo@microsoft.com>
> > > > > >
> > > > > > This patch exposes the function that hv_vmbus.ko uses to make
> > > hypercalls.
> > > > > This
> > > > > > is necessary for retargeting an interrupt when it is given a new
> affinity.
> > > > > >
> > > > > > Since we are exporting this API, rename the API as it will be visible
> > > outside
> > > > > > the hv.c file.
> > > > > >
> > > > > > Signed-off-by: Jake Oshins <jakeo@microsoft.com>
> > > > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > > > ---
> > > > > >  drivers/hv/hv.c        |    9 +++++----
> > > > > >  include/linux/hyperv.h |    1 +
> > > > > >  2 files changed, 6 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > > > > > index 6341be8..a7b6c6a 100644
> > > > > > --- a/drivers/hv/hv.c
> > > > > > +++ b/drivers/hv/hv.c
> > > > > > @@ -89,9 +89,9 @@ static int query_hypervisor_info(void)
> > > > > >  }
> > > > > >
> > > > > >  /*
> > > > > > - * do_hypercall- Invoke the specified hypercall
> > > > > > + * hv_do_hypercall- Invoke the specified hypercall
> > > > > >   */
> > > > > > -static u64 do_hypercall(u64 control, void *input, void *output)
> > > > > > +u64 hv_do_hypercall(u64 control, void *input, void *output)
> > > > > >  {
> > > > > >  	u64 input_address = (input) ? virt_to_phys(input) : 0;
> > > > > >  	u64 output_address = (output) ? virt_to_phys(output) : 0;
> > > > > > @@ -132,6 +132,7 @@ static u64 do_hypercall(u64 control, void
> *input,
> > > > > void *output)
> > > > > >  	return hv_status_lo | ((u64)hv_status_hi << 32);
> > > > > >  #endif /* !x86_64 */
> > > > > >  }
> > > > > > +EXPORT_SYMBOL_GPL(hv_do_hypercall);
> > > > > >
> > > > > >  #ifdef CONFIG_X86_64
> > > > > >  static cycle_t read_hv_clock_tsc(struct clocksource *arg)
> > > > > > @@ -329,7 +330,7 @@ int hv_post_message(union
> hv_connection_id
> > > > > connection_id,
> > > > > >  	aligned_msg->payload_size = payload_size;
> > > > > >  	memcpy((void *)aligned_msg->payload, payload,
> payload_size);
> > > > > >
> > > > > > -	status = do_hypercall(HVCALL_POST_MESSAGE,
> aligned_msg, NULL)
> > > > > > +	status = hv_do_hypercall(HVCALL_POST_MESSAGE,
> aligned_msg,
> > > > > NULL)
> > > > > >  		& 0xFFFF;
> > > > > >
> > > > > >  	put_cpu();
> > > > > > @@ -347,7 +348,7 @@ u16 hv_signal_event(void *con_id)
> > > > > >  {
> > > > > >  	u16 status;
> > > > > >
> > > > > > -	status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id,
> NULL) &
> > > > > 0xFFFF);
> > > > > > +	status = (hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id,
> NULL) &
> > > > > 0xFFFF);
> > > > >
> > > > > What's with the crazy () around a function call?
> > > >
> > > > I will address this and resend.
> > > > >
> > > > > And why are you passing a 64bit return value into a 16bit status value?
> > > > > That seems like something is broken.
> > > >
> > > > The low order 16 bits have the valid status codes we are interested in.
> > >
> > > Then why even return a 64bit number if no one uses it?
> >
> > The hypervisor ABI specifies a 64 bit return value.
> 
> Then just fricken return a 32bit value, don't force everyone who calls
> it to do themselves...

Sorry for the confusion. The generic hypercall primitive will return a 64 bit value
that includes the status bits and other information. As we export the ability to invoke
the hypercall, we should preserve the 64 bits of the return value as the potential
consumers of this hypercall interface may care about the other information
in the return value.

Currently, we have a couple of wrappers built on the hypercall primitive that just
need to return the status value and don't care about other bits in the return value. I will
cleanup the code in these wrappers functions (two to be precise).

Regards,

K. Y
 
> 
> greg k-h