Re: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
On Mon, May 14, 2018 at 11:17:55AM -0700, Stephen Hemminger wrote: > On Mon, 14 May 2018 18:14:15 + > Dexuan Cuiwrote: > > > > From: devel On Behalf Of > > > Stephen Hemminger > > > Sent: Sunday, May 13, 2018 10:24 > > > > ... > > > > @@ -372,6 +400,18 @@ int vmbus_post_msg(void *buffer, size_t buflen, > > > bool can_sleep) > > > > ... > > > > + hdr = (struct vmbus_channel_message_header > > > > *)buffer; > > > > > > Hate to pick o the details, but buffer is void * so cast is not necessary > > > here. > > > > Yes, it's unnecessary in C, though it's necessary in C++. > > > > I found the patch went into char-misc 4 hours ago, so it looks we may > > as well leave it as is. IMHO an explicit cast is not a bad thing. :-) > > > > Thanks, > > -- Dexuan > > Kernel developers like to be concise. In fact there is a smatch script that > perodically > gets run and more cleanup patches get sent. It's a Coccinelle script, not Smatch. Coccinelle generates patches automatically so it's a better tool for cleanup than Smatch. I would generate a lot more Smatch information if there was a way to integrate it easily into a code editor. For example, we could highlight unecessary casts or pointer dereferences where Smatch wasn't 100% sure if it was correct. Or you could hover over function name to see what resources it allocates. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
> From: Stephen Hemminger> Sent: Monday, May 14, 2018 11:18 > To: Dexuan Cui > > > ... > > > Hate to pick o the details, but buffer is void * so cast is not necessary > > > here. > > > > Yes, it's unnecessary in C, though it's necessary in C++. > > > > I found the patch went into char-misc 4 hours ago, so it looks we may > > as well leave it as is. IMHO an explicit cast is not a bad thing. :-) > > > > Thanks, > > -- Dexuan > > Kernel developers like to be concise. In fact there is a smatch script that > perodically gets run and more cleanup patches get sent. I checked the "git log" and confimed you're correct: there are a lot of patches that removed the cast from "void *". :-) Then let me post a small patch for this. -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
On Mon, 14 May 2018 18:14:15 + Dexuan Cuiwrote: > > From: devel On Behalf Of > > Stephen Hemminger > > Sent: Sunday, May 13, 2018 10:24 > > > ... > > > @@ -372,6 +400,18 @@ int vmbus_post_msg(void *buffer, size_t buflen, > > bool can_sleep) > > > ... > > > + hdr = (struct vmbus_channel_message_header *)buffer; > > > > Hate to pick o the details, but buffer is void * so cast is not necessary > > here. > > Yes, it's unnecessary in C, though it's necessary in C++. > > I found the patch went into char-misc 4 hours ago, so it looks we may > as well leave it as is. IMHO an explicit cast is not a bad thing. :-) > > Thanks, > -- Dexuan Kernel developers like to be concise. In fact there is a smatch script that perodically gets run and more cleanup patches get sent. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
> From: develOn Behalf Of > Stephen Hemminger > Sent: Sunday, May 13, 2018 10:24 > > ... > > @@ -372,6 +400,18 @@ int vmbus_post_msg(void *buffer, size_t buflen, > bool can_sleep) > > ... > > + hdr = (struct vmbus_channel_message_header *)buffer; > > Hate to pick o the details, but buffer is void * so cast is not necessary > here. Yes, it's unnecessary in C, though it's necessary in C++. I found the patch went into char-misc 4 hours ago, so it looks we may as well leave it as is. IMHO an explicit cast is not a bad thing. :-) Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
On Sat, 12 May 2018 02:30:33 -0700 k...@linuxonhyperv.com wrote: > int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep) > { > + struct vmbus_channel_message_header *hdr; > union hv_connection_id conn_id; > int ret = 0; > int retries = 0; > u32 usec = 1; > > conn_id.asu32 = 0; > - conn_id.u.id = VMBUS_MESSAGE_CONNECTION_ID; > + conn_id.u.id = vmbus_connection.msg_conn_id; > > /* >* hv_post_message() can have transient failures because of > @@ -372,6 +400,18 @@ int vmbus_post_msg(void *buffer, size_t buflen, bool > can_sleep) > > switch (ret) { > case HV_STATUS_INVALID_CONNECTION_ID: > + /* > + * See vmbus_negotiate_version(): VMBus protocol 5.0 > + * requires 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 > + * HV_STATUS_INVALID_CONNECTION_ID and we should > + * return an error immediately without retrying. > + */ > + hdr = (struct vmbus_channel_message_header *)buffer; Hate to pick o the details, but buffer is void * so cast is not necessary here. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] Drivers: hv: vmbus: enable VMBus protocol version 5.0
From: Dexuan CuiWith VMBus protocol 5.0, we're able to better support new features, e.g. running two or more VMBus drivers simultaneously in a single VM -- note: we can't simply load the current VMBus driver twice, instead, a secondary VMBus driver must be implemented. This patch adds the support for the new VMBus protocol, which is available on new Windows hosts, by: 1) We still use SINT2 for compatibility; 2) We must use Connection ID 4 for the Initiate Contact Message, and for subsequent messages, we must use the Message Connection ID field in the host-returned VersionResponse Message. Notes for developers of the secondary VMBus driver: 1) Must use VMBus protocol 5.0 as well; 2) Must use a different SINT number that is not in use. 3) Must use Connection ID 4 for the Initiate Contact Message, and for subsequent messages, must use the Message Connection ID field in the host-returned VersionResponse Message. 4) It's possible that the primary VMBus driver using protocol version 4.0 can work with a secondary VMBus driver using protocol version 5.0, but it's recommended that both should use 5.0 for new Hyper-V features in the future. Signed-off-by: Dexuan Cui Cc: Stephen Hemminger Cc: K. Y. Srinivasan Cc: Michael Kelley Signed-off-by: K. Y. Srinivasan --- drivers/hv/connection.c | 44 ++-- drivers/hv/hyperv_vmbus.h | 3 +++ include/linux/hyperv.h| 26 -- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index 72855182b191..19e046820fda 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -63,6 +63,9 @@ static __u32 vmbus_get_next_version(__u32 current_version) case (VERSION_WIN10): return VERSION_WIN8_1; + case (VERSION_WIN10_V5): + return VERSION_WIN10; + case (VERSION_WS2008): default: return VERSION_INVAL; @@ -80,9 +83,29 @@ static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, msg = (struct vmbus_channel_initiate_contact *)msginfo->msg; + memset(msg, 0, sizeof(*msg)); msg->header.msgtype = CHANNELMSG_INITIATE_CONTACT; msg->vmbus_version_requested = version; - msg->interrupt_page = virt_to_phys(vmbus_connection.int_page); + + /* +* VMBus protocol 5.0 (VERSION_WIN10_V5) requires 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 +* compatibility. +* +* On old hosts, we should always use VMBUS_MESSAGE_CONNECTION_ID (1). +*/ + if (version >= VERSION_WIN10_V5) { + msg->msg_sint = VMBUS_MESSAGE_SINT; + vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID_4; + } else { + msg->interrupt_page = virt_to_phys(vmbus_connection.int_page); + vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID; + } + msg->monitor_page1 = virt_to_phys(vmbus_connection.monitor_pages[0]); msg->monitor_page2 = virt_to_phys(vmbus_connection.monitor_pages[1]); /* @@ -137,6 +160,10 @@ static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, /* Check if successful */ if (msginfo->response.version_response.version_supported) { vmbus_connection.conn_state = CONNECTED; + + if (version >= VERSION_WIN10_V5) + vmbus_connection.msg_conn_id = + msginfo->response.version_response.msg_conn_id; } else { return -ECONNREFUSED; } @@ -354,13 +381,14 @@ void vmbus_on_event(unsigned long data) */ int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep) { + struct vmbus_channel_message_header *hdr; union hv_connection_id conn_id; int ret = 0; int retries = 0; u32 usec = 1; conn_id.asu32 = 0; - conn_id.u.id = VMBUS_MESSAGE_CONNECTION_ID; + conn_id.u.id = vmbus_connection.msg_conn_id; /* * hv_post_message() can have transient failures because of @@ -372,6 +400,18 @@ int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep) switch (ret) { case HV_STATUS_INVALID_CONNECTION_ID: + /* +* See vmbus_negotiate_version(): VMBus protocol 5.0 +* requires that we must use +*