RE: [PATCH RESEND 09/27] drivers:hv: Export the API to invoke a hypercall on Hyper-V
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Monday, December 14, 2015 7:08 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com; > jasow...@redhat.com; Jake Oshins > Subject: Re: [PATCH RESEND 09/27] drivers:hv: Export the API to invoke a > hypercall on Hyper-V > > On Mon, Dec 14, 2015 at 04:01:40PM -0800, K. Y. Srinivasan wrote: > > From: Jake Oshins > > > > 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 > > Signed-off-by: K. Y. Srinivasan > > --- > > drivers/hv/hv.c | 20 ++-- > > drivers/hv/hyperv_vmbus.h |2 +- > > include/linux/hyperv.h|1 + > > 3 files changed, 12 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c > > index 6341be8..7a06933 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) > > @@ -315,7 +316,7 @@ int hv_post_message(union hv_connection_id > connection_id, > > { > > > > struct hv_input_post_message *aligned_msg; > > - u16 status; > > + u64 status; > > > > if (payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT) > > return -EMSGSIZE; > > @@ -329,11 +330,10 @@ 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) > > - & 0x; > > + status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, > NULL); > > > > put_cpu(); > > - return status; > > + return status & 0x; > > } > > > > > > @@ -343,13 +343,13 @@ int hv_post_message(union hv_connection_id > connection_id, > > * > > * This involves a hypercall. > > */ > > -u16 hv_signal_event(void *con_id) > > +int hv_signal_event(void *con_id) > > { > > - u16 status; > > + u64 status; > > > > - status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) & > 0x); > > + status = hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL); > > > > - return status; > > + return status & 0x; > > This "feels" odd, as the return value isn't the traditional 0 or -ERROR, > right? Why are you masking off the top bits? > > And, no one ever does anything with the return value of > hv_signal_event(), so why have it return anything anyway? > > I'll take this, but please fix up the area, it's messy... Thanks Greg. Will do. K. Y > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 09/27] drivers:hv: Export the API to invoke a hypercall on Hyper-V
On Mon, Dec 14, 2015 at 04:01:40PM -0800, K. Y. Srinivasan wrote: > From: Jake Oshins > > 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 > Signed-off-by: K. Y. Srinivasan > --- > drivers/hv/hv.c | 20 ++-- > drivers/hv/hyperv_vmbus.h |2 +- > include/linux/hyperv.h|1 + > 3 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c > index 6341be8..7a06933 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) > @@ -315,7 +316,7 @@ int hv_post_message(union hv_connection_id connection_id, > { > > struct hv_input_post_message *aligned_msg; > - u16 status; > + u64 status; > > if (payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT) > return -EMSGSIZE; > @@ -329,11 +330,10 @@ 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) > - & 0x; > + status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL); > > put_cpu(); > - return status; > + return status & 0x; > } > > > @@ -343,13 +343,13 @@ int hv_post_message(union hv_connection_id > connection_id, > * > * This involves a hypercall. > */ > -u16 hv_signal_event(void *con_id) > +int hv_signal_event(void *con_id) > { > - u16 status; > + u64 status; > > - status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) & 0x); > + status = hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL); > > - return status; > + return status & 0x; This "feels" odd, as the return value isn't the traditional 0 or -ERROR, right? Why are you masking off the top bits? And, no one ever does anything with the return value of hv_signal_event(), so why have it return anything anyway? I'll take this, but please fix up the area, it's messy... greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 09/27] drivers:hv: Export the API to invoke a hypercall on Hyper-V
On Mon, Dec 14, 2015 at 04:01:40PM -0800, K. Y. Srinivasan wrote: > From: Jake Oshins> > 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 > Signed-off-by: K. Y. Srinivasan > --- > drivers/hv/hv.c | 20 ++-- > drivers/hv/hyperv_vmbus.h |2 +- > include/linux/hyperv.h|1 + > 3 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c > index 6341be8..7a06933 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) > @@ -315,7 +316,7 @@ int hv_post_message(union hv_connection_id connection_id, > { > > struct hv_input_post_message *aligned_msg; > - u16 status; > + u64 status; > > if (payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT) > return -EMSGSIZE; > @@ -329,11 +330,10 @@ 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) > - & 0x; > + status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL); > > put_cpu(); > - return status; > + return status & 0x; > } > > > @@ -343,13 +343,13 @@ int hv_post_message(union hv_connection_id > connection_id, > * > * This involves a hypercall. > */ > -u16 hv_signal_event(void *con_id) > +int hv_signal_event(void *con_id) > { > - u16 status; > + u64 status; > > - status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) & 0x); > + status = hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL); > > - return status; > + return status & 0x; This "feels" odd, as the return value isn't the traditional 0 or -ERROR, right? Why are you masking off the top bits? And, no one ever does anything with the return value of hv_signal_event(), so why have it return anything anyway? I'll take this, but please fix up the area, it's messy... greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH RESEND 09/27] drivers:hv: Export the API to invoke a hypercall on Hyper-V
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Monday, December 14, 2015 7:08 PM > To: KY Srinivasan> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com; > jasow...@redhat.com; Jake Oshins > Subject: Re: [PATCH RESEND 09/27] drivers:hv: Export the API to invoke a > hypercall on Hyper-V > > On Mon, Dec 14, 2015 at 04:01:40PM -0800, K. Y. Srinivasan wrote: > > From: Jake Oshins > > > > 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 > > Signed-off-by: K. Y. Srinivasan > > --- > > drivers/hv/hv.c | 20 ++-- > > drivers/hv/hyperv_vmbus.h |2 +- > > include/linux/hyperv.h|1 + > > 3 files changed, 12 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c > > index 6341be8..7a06933 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) > > @@ -315,7 +316,7 @@ int hv_post_message(union hv_connection_id > connection_id, > > { > > > > struct hv_input_post_message *aligned_msg; > > - u16 status; > > + u64 status; > > > > if (payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT) > > return -EMSGSIZE; > > @@ -329,11 +330,10 @@ 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) > > - & 0x; > > + status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, > NULL); > > > > put_cpu(); > > - return status; > > + return status & 0x; > > } > > > > > > @@ -343,13 +343,13 @@ int hv_post_message(union hv_connection_id > connection_id, > > * > > * This involves a hypercall. > > */ > > -u16 hv_signal_event(void *con_id) > > +int hv_signal_event(void *con_id) > > { > > - u16 status; > > + u64 status; > > > > - status = (do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL) & > 0x); > > + status = hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL); > > > > - return status; > > + return status & 0x; > > This "feels" odd, as the return value isn't the traditional 0 or -ERROR, > right? Why are you masking off the top bits? > > And, no one ever does anything with the return value of > hv_signal_event(), so why have it return anything anyway? > > I'll take this, but please fix up the area, it's messy... Thanks Greg. Will do. K. Y > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/