[PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[Dexuan Cui]

Before the line vmbus_open() returns, srv->util_cb can be already running
and the variables, like util_fw_version, are needed by the srv->util_cb.

So we have to make sure the variables are initialized before the vmbus_open().

CC: "K. Y. Srinivasan" <kys@microsoft.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---

v2:
This is a RESEND.
I just added Vitaly's Reviewed-by.

 drivers/hv/hv_util.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 3b9c9ef..c5be773 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -340,12 +340,8 @@ static int util_probe(struct hv_device *dev,
 
 	set_channel_read_state(dev->channel, false);
 
-	ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 0,
-			srv->util_cb, dev->channel);
-	if (ret)
-		goto error;
-
 	hv_set_drvdata(dev, srv);
+
 	/*
 	 * Based on the host; initialize the framework and
 	 * service version numbers we will negotiate.
@@ -365,6 +361,11 @@ static int util_probe(struct hv_device *dev,
 		hb_srv_version = HB_VERSION;
 	}
 
+	ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 0,
+			srv->util_cb, dev->channel);
+	if (ret)
+		goto error;
+
 	return 0;
 
 error:
-- 
1.9.1

Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[Jason Wang]

On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com> wrote:
> Before the line vmbus_open() returns, srv->util_cb can be already 
> running
> and the variables, like util_fw_version, are needed by the 
> srv->util_cb.

A questions is why we do this for util only? Can callbacks of other 
devices be called before vmbus_open() returns?

> 
> So we have to make sure the variables are initialized before the 
> vmbus_open().
> 
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> 
> v2:
> This is a RESEND.
> I just added Vitaly's Reviewed-by.
> 
>  drivers/hv/hv_util.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 3b9c9ef..c5be773 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -340,12 +340,8 @@ static int util_probe(struct hv_device *dev,
>  
>  	set_channel_read_state(dev->channel, false);
>  
> -	ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 
> 0,
> -			srv->util_cb, dev->channel);
> -	if (ret)
> -		goto error;
> -
>  	hv_set_drvdata(dev, srv);
> +

This seems unnecessary.
> 
>  	/*
>  	 * Based on the host; initialize the framework and
>  	 * service version numbers we will negotiate.
> @@ -365,6 +361,11 @@ static int util_probe(struct hv_device *dev,
>  		hb_srv_version = HB_VERSION;
>  	}
>  
> +	ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 
> 0,
> +			srv->util_cb, dev->channel);
> +	if (ret)
> +		goto error;
> +
>  	return 0;
>  
>  error:
> -- 
> 1.9.1
> 

RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[Dexuan Cui]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2653 bytes --]

> -----Original Message-----
> From: Jason Wang [mailto:jasowang@redhat.com]
> Sent: Monday, February 2, 2015 17:36 PM
> To: Dexuan Cui
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY
> Srinivasan; vkuznets@redhat.com; Haiyang Zhang
> Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
> 
> 
> 
> On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com> wrote:
> > Before the line vmbus_open() returns, srv->util_cb can be already
> > running
> > and the variables, like util_fw_version, are needed by the
> > srv->util_cb.
> 
> A questions is why we do this for util only? Can callbacks of other
> devices be called before vmbus_open() returns?
The variables are used in vmbus_prep_negotiate_resp(), which is only for
the util devices.

I think the other devices should already handle the similar issue properly.
If this is not the case, we need to fix them too.

> 
> >
> > So we have to make sure the variables are initialized before the
> > vmbus_open().
> >
> > CC: "K. Y. Srinivasan" <kys@microsoft.com>
> > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > Signed-off-by: Dexuan Cui <decui@microsoft.com>
> > ---
> >
> > v2:
> > This is a RESEND.
> > I just added Vitaly's Reviewed-by.
> >
> >  drivers/hv/hv_util.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> > index 3b9c9ef..c5be773 100644
> > --- a/drivers/hv/hv_util.c
> > +++ b/drivers/hv/hv_util.c
> > @@ -340,12 +340,8 @@ static int util_probe(struct hv_device *dev,
> >
> >  	set_channel_read_state(dev->channel, false);
> >
> > -	ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL,
> > 0,
> > -			srv->util_cb, dev->channel);
> > -	if (ret)
> > -		goto error;
> > -
> >  	hv_set_drvdata(dev, srv);
> > +
> 
> This seems unnecessary.
Yeah, it's an empty line, splitting the line and the below comment.
I'm Ok to not have it.

> >  	/*
> >  	 * Based on the host; initialize the framework and
> >  	 * service version numbers we will negotiate.
> > @@ -365,6 +361,11 @@ static int util_probe(struct hv_device *dev,
> >  		hb_srv_version = HB_VERSION;
> >  	}
> >
> > +	ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL,
> > 0,
> > +			srv->util_cb, dev->channel);
> > +	if (ret)
> > +		goto error;
> > +
> >  	return 0;
> >
> >  error:
> > --
> > 1.9.1
> >

-- Dexuan
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[Jason Wang]

On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com> wrote:
>>  -----Original Message-----
>>  From: Jason Wang [mailto:jasowang@redhat.com]
>>  Sent: Monday, February 2, 2015 17:36 PM
>>  To: Dexuan Cui
>>  Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; 
>> driverdev-
>>  devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY
>>  Srinivasan; vkuznets@redhat.com; Haiyang Zhang
>>  Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a 
>> later place
>>  
>>  
>>  
>>  On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com> 
>> wrote:
>>  > Before the line vmbus_open() returns, srv->util_cb can be already
>>  > running
>>  > and the variables, like util_fw_version, are needed by the
>>  > srv->util_cb.
>>  
>>  A questions is why we do this for util only? Can callbacks of other
>>  devices be called before vmbus_open() returns?
> The variables are used in vmbus_prep_negotiate_resp(), which is only 
> for
> the util devices.
> 
> I think the other devices should already handle the similar issue 
> properly.
> If this is not the case, we need to fix them too.

Better to check all the others, e.g in balloon_probe(), it call 
hv_set_drvdata() after vmbus_open() and dose several datas setups in 
the middle. If balloon_onchannelcallback() could be called before 
hv_set_drvdata(), the code looks wrong.

Thanks

RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[KY Srinivasan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2388 bytes --]



> -----Original Message-----
> From: Jason Wang [mailto:jasowang@redhat.com]
> Sent: Monday, February 2, 2015 7:09 PM
> To: Dexuan Cui
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY
> Srinivasan; vkuznets@redhat.com; Haiyang Zhang
> Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
> 
> 
> 
> On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com> wrote:
> >>  -----Original Message-----
> >>  From: Jason Wang [mailto:jasowang@redhat.com]
> >>  Sent: Monday, February 2, 2015 17:36 PM
> >>  To: Dexuan Cui
> >>  Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> >> driverdev-
> >>  devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY
> >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang
> >>  Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a
> >> later place
> >>
> >>
> >>
> >>  On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com>
> >> wrote:
> >>  > Before the line vmbus_open() returns, srv->util_cb can be already
> >> > running  > and the variables, like util_fw_version, are needed by
> >> the  > srv->util_cb.
> >>
> >>  A questions is why we do this for util only? Can callbacks of other
> >> devices be called before vmbus_open() returns?
> > The variables are used in vmbus_prep_negotiate_resp(), which is only
> > for the util devices.
> >
> > I think the other devices should already handle the similar issue
> > properly.
> > If this is not the case, we need to fix them too.
> 
> Better to check all the others, e.g in balloon_probe(), it call
> hv_set_drvdata() after vmbus_open() and dose several datas setups in the
> middle. If balloon_onchannelcallback() could be called before
> hv_set_drvdata(), the code looks wrong.

Jason,

For all other device types, the guest initiates the communication with the host and potentially
negotiates appropriate (supported) version with the host. For the services packaged in the util
driver, the flow is a little different - the host pushes the version information into the guest. So,
the fix Dexuan made is only needed in the util driver.

Regards,

K. Y 
> 
> Thanks
> 
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[Jason Wang]

On Tue, Feb 3, 2015 at 11:30 AM, KY Srinivasan <kys@microsoft.com> 
wrote:
> 
> 
>>  -----Original Message-----
>>  From: Jason Wang [mailto:jasowang@redhat.com]
>>  Sent: Monday, February 2, 2015 7:09 PM
>>  To: Dexuan Cui
>>  Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; 
>> driverdev-
>>  devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY
>>  Srinivasan; vkuznets@redhat.com; Haiyang Zhang
>>  Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a 
>> later place
>>  
>>  
>>  
>>  On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com> 
>> wrote:
>>  >>  -----Original Message-----
>>  >>  From: Jason Wang [mailto:jasowang@redhat.com]
>>  >>  Sent: Monday, February 2, 2015 17:36 PM
>>  >>  To: Dexuan Cui
>>  >>  Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
>>  >> driverdev-
>>  >>  devel@linuxdriverproject.org; olaf@aepfle.de; 
>> apw@canonical.com; KY
>>  >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang
>>  >>  Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a
>>  >> later place
>>  >>
>>  >>
>>  >>
>>  >>  On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui 
>> <decui@microsoft.com>
>>  >> wrote:
>>  >>  > Before the line vmbus_open() returns, srv->util_cb can be 
>> already
>>  >> > running  > and the variables, like util_fw_version, are needed 
>> by
>>  >> the  > srv->util_cb.
>>  >>
>>  >>  A questions is why we do this for util only? Can callbacks of 
>> other
>>  >> devices be called before vmbus_open() returns?
>>  > The variables are used in vmbus_prep_negotiate_resp(), which is 
>> only
>>  > for the util devices.
>>  >
>>  > I think the other devices should already handle the similar issue
>>  > properly.
>>  > If this is not the case, we need to fix them too.
>>  
>>  Better to check all the others, e.g in balloon_probe(), it call
>>  hv_set_drvdata() after vmbus_open() and dose several datas setups 
>> in the
>>  middle. If balloon_onchannelcallback() could be called before
>>  hv_set_drvdata(), the code looks wrong.
> 
> Jason,
> 
> For all other device types, the guest initiates the communication 
> with the host and potentially
> negotiates appropriate (supported) version with the host. For the 
> services packaged in the util
> driver, the flow is a little different - the host pushes the version 
> information into the guest. So,
> the fix Dexuan made is only needed in the util driver.
> 
> Regards,
> 
> K. Y 

Thanks, so you mean for other device, it won't get any interrupt before 
guest negotiate the version with host?
> 
>>  
>>  Thanks
>>  
>>  
> 

RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[KY Srinivasan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3311 bytes --]



> -----Original Message-----
> From: Jason Wang [mailto:jasowang@redhat.com]
> Sent: Monday, February 2, 2015 7:38 PM
> To: KY Srinivasan
> Cc: Dexuan Cui; gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> driverdev-devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; vkuznets@redhat.com; Haiyang Zhang
> Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place
> 
> 
> 
> On Tue, Feb 3, 2015 at 11:30 AM, KY Srinivasan <kys@microsoft.com>
> wrote:
> >
> >
> >>  -----Original Message-----
> >>  From: Jason Wang [mailto:jasowang@redhat.com]
> >>  Sent: Monday, February 2, 2015 7:09 PM
> >>  To: Dexuan Cui
> >>  Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> >> driverdev-
> >>  devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; KY
> >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang
> >>  Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a
> >> later place
> >>
> >>
> >>
> >>  On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com>
> >> wrote:
> >>  >>  -----Original Message-----
> >>  >>  From: Jason Wang [mailto:jasowang@redhat.com]  >>  Sent: Monday,
> >> February 2, 2015 17:36 PM  >>  To: Dexuan Cui  >>  Cc:
> >> gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;  >>
> >> driverdev-  >>  devel@linuxdriverproject.org; olaf@aepfle.de;
> >> apw@canonical.com; KY  >> Srinivasan; vkuznets@redhat.com; Haiyang
> >> Zhang  >>  Subject: Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open()
> >> to a  >> later place  >>  >>  >>  >>  On Mon, Feb 2, 2015 at 12:35
> >> PM, Dexuan Cui <decui@microsoft.com>  >> wrote:
> >>  >>  > Before the line vmbus_open() returns, srv->util_cb can be
> >> already  >> > running  > and the variables, like util_fw_version, are
> >> needed by  >> the  > srv->util_cb.
> >>  >>
> >>  >>  A questions is why we do this for util only? Can callbacks of
> >> other  >> devices be called before vmbus_open() returns?
> >>  > The variables are used in vmbus_prep_negotiate_resp(), which is
> >> only  > for the util devices.
> >>  >
> >>  > I think the other devices should already handle the similar issue
> >> > properly.
> >>  > If this is not the case, we need to fix them too.
> >>
> >>  Better to check all the others, e.g in balloon_probe(), it call
> >>  hv_set_drvdata() after vmbus_open() and dose several datas setups in
> >> the  middle. If balloon_onchannelcallback() could be called before
> >> hv_set_drvdata(), the code looks wrong.
> >
> > Jason,
> >
> > For all other device types, the guest initiates the communication with
> > the host and potentially negotiates appropriate (supported) version
> > with the host. For the services packaged in the util driver, the flow
> > is a little different - the host pushes the version information into
> > the guest. So, the fix Dexuan made is only needed in the util driver.
> >
> > Regards,
> >
> > K. Y
> 
> Thanks, so you mean for other device, it won't get any interrupt before guest
> negotiate the version with host?

Yes, the guest initiates the version negotiation. Are you seeing something different?

K. Y
> >
> >>
> >>  Thanks
> >>
> >>
> >

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[Jason Wang]

On Tue, Feb 3, 2015 at 11:40 AM, KY Srinivasan <kys@microsoft.com> 
wrote:
> 
> 
>>  -----Original Message-----
>>  From: Jason Wang [mailto:jasowang@redhat.com]
>>  Sent: Monday, February 2, 2015 7:38 PM
>>  To: KY Srinivasan
>>  Cc: Dexuan Cui; gregkh@linuxfoundation.org; 
>> linux-kernel@vger.kernel.org;
>>  driverdev-devel@linuxdriverproject.org; olaf@aepfle.de;
>>  apw@canonical.com; vkuznets@redhat.com; Haiyang Zhang
>>  Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a 
>> later place
>>  
>>  
>>  
>>  On Tue, Feb 3, 2015 at 11:30 AM, KY Srinivasan <kys@microsoft.com>
>>  wrote:
>>  >
>>  >
>>  >>  -----Original Message-----
>>  >>  From: Jason Wang [mailto:jasowang@redhat.com]
>>  >>  Sent: Monday, February 2, 2015 7:09 PM
>>  >>  To: Dexuan Cui
>>  >>  Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
>>  >> driverdev-
>>  >>  devel@linuxdriverproject.org; olaf@aepfle.de; 
>> apw@canonical.com; KY
>>  >> Srinivasan; vkuznets@redhat.com; Haiyang Zhang
>>  >>  Subject: RE: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a
>>  >> later place
>>  >>
>>  >>
>>  >>
>>  >>  On Mon, Feb 2, 2015 at 6:09 PM, Dexuan Cui <decui@microsoft.com>
>>  >> wrote:
>>  >>  >>  -----Original Message-----
>>  >>  >>  From: Jason Wang [mailto:jasowang@redhat.com]  >>  Sent: 
>> Monday,
>>  >> February 2, 2015 17:36 PM  >>  To: Dexuan Cui  >>  Cc:
>>  >> gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;  >>
>>  >> driverdev-  >>  devel@linuxdriverproject.org; olaf@aepfle.de;
>>  >> apw@canonical.com; KY  >> Srinivasan; vkuznets@redhat.com; 
>> Haiyang
>>  >> Zhang  >>  Subject: Re: [PATCH v2 1/3] hv: hv_util: move 
>> vmbus_open()
>>  >> to a  >> later place  >>  >>  >>  >>  On Mon, Feb 2, 2015 at 
>> 12:35
>>  >> PM, Dexuan Cui <decui@microsoft.com>  >> wrote:
>>  >>  >>  > Before the line vmbus_open() returns, srv->util_cb can be
>>  >> already  >> > running  > and the variables, like 
>> util_fw_version, are
>>  >> needed by  >> the  > srv->util_cb.
>>  >>  >>
>>  >>  >>  A questions is why we do this for util only? Can callbacks 
>> of
>>  >> other  >> devices be called before vmbus_open() returns?
>>  >>  > The variables are used in vmbus_prep_negotiate_resp(), which 
>> is
>>  >> only  > for the util devices.
>>  >>  >
>>  >>  > I think the other devices should already handle the similar 
>> issue
>>  >> > properly.
>>  >>  > If this is not the case, we need to fix them too.
>>  >>
>>  >>  Better to check all the others, e.g in balloon_probe(), it call
>>  >>  hv_set_drvdata() after vmbus_open() and dose several datas 
>> setups in
>>  >> the  middle. If balloon_onchannelcallback() could be called 
>> before
>>  >> hv_set_drvdata(), the code looks wrong.
>>  >
>>  > Jason,
>>  >
>>  > For all other device types, the guest initiates the communication 
>> with
>>  > the host and potentially negotiates appropriate (supported) 
>> version
>>  > with the host. For the services packaged in the util driver, the 
>> flow
>>  > is a little different - the host pushes the version information 
>> into
>>  > the guest. So, the fix Dexuan made is only needed in the util 
>> driver.
>>  >
>>  > Regards,
>>  >
>>  > K. Y
>>  
>>  Thanks, so you mean for other device, it won't get any interrupt 
>> before guest
>>  negotiate the version with host?
> 
> Yes, the guest initiates the version negotiation. Are you seeing 
> something different?
> 
> K. Y

No, thanks.

Re: [PATCH v2 1/3] hv: hv_util: move vmbus_open() to a later place

[Jason Wang]

On Mon, Feb 2, 2015 at 12:35 PM, Dexuan Cui <decui@microsoft.com> wrote:
> Before the line vmbus_open() returns, srv->util_cb can be already 
> running
> and the variables, like util_fw_version, are needed by the 
> srv->util_cb.
> 
> So we have to make sure the variables are initialized before the 
> vmbus_open().
> 
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> 
> v2:
> This is a RESEND.
> I just added Vitaly's Reviewed-by.
> 
>  drivers/hv/hv_util.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 3b9c9ef..c5be773 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -340,12 +340,8 @@ static int util_probe(struct hv_device *dev,
>  
>  	set_channel_read_state(dev->channel, false);
>  
> -	ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 
> 0,
> -			srv->util_cb, dev->channel);
> -	if (ret)
> -		goto error;
> -
>  	hv_set_drvdata(dev, srv);
> +
>  	/*
>  	 * Based on the host; initialize the framework and
>  	 * service version numbers we will negotiate.
> @@ -365,6 +361,11 @@ static int util_probe(struct hv_device *dev,
>  		hb_srv_version = HB_VERSION;
>  	}
>  
> +	ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 
> 0,
> +			srv->util_cb, dev->channel);
> +	if (ret)
> +		goto error;
> +
>  	return 0;
>  
>  error:
> -- 
> 1.9.1

Reviewed-by: Jason Wang <jasowang@redhat.com>